5
\$\begingroup\$

I have just finished the sololearn C++ course and wanted to create a basic login program in the vein of a basic operating system.

#include "ubuntu_login"
#include <iostream>
#include <string>
using namespace std;

Ubuntu_Login::Ubuntu_Login()
{
    cout << "\t Welcome to Ubuntu!\n" << endl;
}

void Ubuntu_Login::usr_login()
{

cout << "Please enter a username" << endl;
getline(cin,username);

cout << "Please enter a password" << endl;
getline(cin,password);

while(username_input != username || password_input != password)
  {
    cout << "\n|Ubuntu Login|" << endl;

    cout << "Please enter your username" << endl;
    getline(cin, username_input);

    cout << "Please enter your password" << endl;
    getline(cin, password_input);

    if(username_input != username || password_input != password)
      {
        cout << "Incorrect username or password" << endl;
      }
  }
}




#include "ubuntu_login.h"
#include <iostream>
#include <string>
using namespace std;

int main()
{
  Ubuntu_Login UO;
  UO.usr_login();
}

Header file:

#ifndef UBUNTU_LOGIN_H
#define UBUNTU_LOGIN_H
#include <string>
using namespace std;

class Ubuntu_Login
{
public:
  Ubuntu_Login();
  void usr_login();
private:
  string username;
  string password;
  string username_input;
  string password_input;
};

#endif // UBUNTU_LOGIN_H

Any help on improving my code by getting rid of useless code or replacing it with better code would be greatly appreciated.

\$\endgroup\$
  • \$\begingroup\$ Also, you don't need backticks for these code blocks (observe my edits). \$\endgroup\$ – Jamal May 29 '15 at 16:42
  • \$\begingroup\$ It should be denoted that this code is not meant to be a real login program but just a simulation thereof. \$\endgroup\$ – edmz May 30 '15 at 14:06
12
\$\begingroup\$

Never write login code

This is much harder than you think. Leave it to the experts you will get some security piece wrong.

Code Review

Don't use this:

using namespace std;

See Why is “using namespace std;” considered bad practice?.

The reason the "standard namespace" is called std is so that it is not a big deal to type std:: before each standard object/function/type. Don't be lazy type the extra 5 characters.

I don't see declarations for username or password I hope they are not global variables.

cout << "Please enter a username" << endl;
getline(cin,username);

cout << "Please enter a password" << endl;
getline(cin,password);

You have used a while loop.

while(username_input != username || password_input != password)
{
}

You may want to look at a do while loop. As the test is not done till the end of the input code. If somebody had an empty username and password then it would match before the loop was entered and you would never get asked.

do
{
}
while(username_input != username || password_input != password);

Hint

Passwords should NEVER be stored (or encrypted). You should only ever store the hash of the password (properly salted). When a user enters a password hash it with the appropriate salt and compare it to the stored hash of the password.

A hash can be though of as one way encryption. You can encrypt it, but because it is lossy you can not decrypt the hash back to a password. So if people get hold of the your password file all they have is a set of hashes and no passwords.

\$\endgroup\$
  • 2
    \$\begingroup\$ Thanks for your advice ill revise the code and implement the changes you have suggested, I'll also look into using the hash system you have talked about. \$\endgroup\$ – Mr.Ubuntu May 29 '15 at 16:39
  • 1
    \$\begingroup\$ out of interest, how would I go about making it so if the user entered nothing or a space to loop the input process? \$\endgroup\$ – Mr.Ubuntu May 29 '15 at 16:50
7
\$\begingroup\$

Usability

When you enter the password, it's visible on the screen. That's unacceptable from a login program.

Class design

Why is Ubuntu_Login a class? What is the purpose of such "login object", and what can I do with it? Would I ever want to have 2 or more instances? This class doesn't seem to make much sense. A simple function could do this job.

All the data contained in the class (username, username_input, ...) are only used in the usr_login function, and as such it would be better to make them all local variables.

The job of a constructor is to initialize the state of the new object being created. Printing a welcome message doesn't belong in a constructor.

using namespace std

Others already told you to not do this again. If you want to save typing, this alternative is acceptable:

using std::cin;
using std::cout;
using std::endl;
using std::string;
\$\endgroup\$
  • \$\begingroup\$ How would I go about not echo-inf the password? \$\endgroup\$ – Mr.Ubuntu May 29 '15 at 19:32
  • \$\begingroup\$ @theredacteduser You need to manipulate the control structure of that terminal (termios). It's a pretty low level and platform dependent thing (Linux in this case). \$\endgroup\$ – edmz May 30 '15 at 12:03
5
\$\begingroup\$

Also, you be careful to clean out any storage that you are using. For example in your case the user name and password will be hanging around in memory probably till your program exits. If a malicious actor were to dump the process memory he could be able to recover this information.

At a minimum, you should overwrite those field with garbage data as soon as you are done with them to protect against attacks such as this.

I will second Loki's point that writing secure and correct log-in code (as well as any security related code) is very challenging.

\$\endgroup\$
2
\$\begingroup\$
  1. You are not using PAM. Pluggable authentication modules allows you to change authentication backends, which allows for different security for different needs.

  2. You are using passwords. Pet peeve of mine. Passwords have had security failures longer than they have been named. The problems with them certainly predate computers. Unfortunately I don't know what to recommend instead that does not involve a soldering iron.

\$\endgroup\$
  • \$\begingroup\$ Please note one might not want to use PAM for multiple reasons, including having the most lightweight possible system. Also, @the redacted user did not state its intent was to make his/her login program pluggable. \$\endgroup\$ – ElementW May 30 '15 at 17:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.