Username & Password Script (C++)

Here is a code I made today! Can I get some opinions?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
// Log in to view secrat message script. (C++14)
// FireCoder 8/26/15.

#include <iostream>
#include <string>

std::string userName = "FakeUserName";
std::string passWord = "FakePassWord";
std::string iuserName, ipassWord;

bool goAgainU;
bool goAgainP;

void checkUser()
{
    if(iuserName != userName)
    {
        std::cout << "Incorrect!" << std::endl;
        
        goAgainU = true;
    }
    
    return;
}

void grabUser()
{
    std::cout << "Username:\t";
    std::cin  >> iuserName;\
    
    checkUser();
    
    if(goAgainU == true)
    {grabUser();}
    
    return;
}

void checkPass()
{
    if(ipassWord != passWord)
    {
        std::cout << "Incorrect!" << std::endl;
        
        goAgainP = true;
    }
    
    return;
}

void grabPass()
{
    std::cout << "Password:\t";
    std::cin  >> ipassWord;\
    
    checkPass();
    
    if(goAgainP == true)
    {grabPass();}
    
    return;
}

int main ()
{
       std::cout << "You have to log in to view the"
            << " the hidden message!" << std::endl; //Tell user to log in.
            
        grabUser();
        grabPass();
        
        std::cout << "[secret message goes here]\n\n\n\n\n\n\n\n I lied :)";
        
        return 0;
}
- global variables are bad, don't use them.
- passwords should never be stored as plain text - look up password hashing.
- you have superfluous backslashes on lines 29 and 54
- your formatting is unusual on line 34 and 59
- you have split this program into multiple functions in a way which makes the program worse instead of better
thx
- you have split this program into multiple functions in a way which makes the program worse instead of better
What do you suggest for him to fix it?
I suggest that he use only a single function to avoid having to use global variables and to avoid making a class to hold temporary local variables.
Last edited on
Agreed. Username+Password validation should work like this:

username> Joe
password> xyz

Incorrect. Try again.
username>
or
username> Joe
password> xyz

Success!

This is a single function, something like bool validate_user().

Hope this helps.

In a real program you will also want to turn off echo when obtaining the user's password.
Last edited on
Thanks guys (and girls if any.)

here is my new and improved code.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
// Log in to view secrat message script. (C++14)
// FireCoder 9/2/15.
// Version 2

#include <iostream>
#include <string>

std::string userName = "FakeUserName";
std::string passWord = "FakePassWord";
std::string iuserName, ipassWord;

bool userNamePass = false;
bool passWordPass = false;


int main ()
{
        std::cout << "You have to log in to view the"
            << " the hidden message!" << std::endl; //Tell user to log in.
            
        while(userNamePass == false)
        {
            std::cout << "Username:\t";
            getline(std::cin,iuserName);
            
            if(iuserName == userName)
            {
                break;
            }
            
            std::cout << "incorrect, try again.\n";
        }
        
        while(passWordPass == false)
        {
            std::cout << "Password:\t";
            getline(std::cin,ipassWord);
            
            if(ipassWord == passWord)
            {
                break;
            }
            
            std::cout << "incorrect, try again.\n";
        }
        
        std::cout << "\nLoged In!\n";
}
I have look at your code , it works fine . But here is what I could do with :
//main.cpp

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
// Log in to view secrat message script. (C++14)
// FireCoder 9/2/15. Ericool :)
// Version 3

#include <iostream>
#include <string>


#include "Server.h"

void ConsoleEntry(std::string & _u , std::string & _p)
{
	//use getline if you want as well

	std::cout << "Please enter Username" << std::endl;
	std::cin >> _u;
	std::cout << "Please enter Password" << std::endl;
	std::cin >> _p;
}
int main (int argc , char * argv[])
{

        
	Server server;

	std::string p;
	std::string u;
	
	//First catch
	ConsoleEntry( u , p );

	while( !server.send(u , p) )
	{
		ConsoleEntry( u , p );
	}

    std::cout << "\nLoged In!\n";

	system("pause");

	return EXIT_SUCCESS;
}


//Server.cpp
1
2
3
4
5
6
7
8
9
10
#include "Server.h"

    Server::Server(void)
		: db()
	{}

	bool Server::send(std::string _p , std::string _u )
	{
		return db.find( _p , _u );
	}

//Server.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
#ifndef SERVER_H
#define SERVER_H

#include "DataBase.h"

class Server
{
	DataBase db;

public:
	Server(void);

	bool send(std::string _p , std::string _u );

};

#endif 


//DataBase.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
#include "DataBase.h"

#include "Client.h"
#include <string>

typedef std::vector<Client *>::iterator iterator;
typedef std::vector<Client *>::const_iterator const_iterator;

	DataBase::DataBase(void)
		: clts()
	{
		//imagine you read from a file ...
		std::string userName = "FakeUserName";
		std::string passWord = "FakePassWord";

		//create client instance based on the file
		Client* client = new Client(userName , passWord );
		clts.push_back(client);
	}

	bool DataBase::find(std::string _p , std::string _u)
	{
		iterator it = clts.begin();
		const_iterator ite = clts.end();
		while(it != ite)
		{
			if ( (* it ++ )->compare( _p , _u) )
				return true;
		}
		return false;
	}


//DataBase.h

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
#ifndef DATABASE_H
#define DATABASE_H

#include <vector>

class Client;


class DataBase
{

	std::vector < Client *> clts;

public:
	DataBase(void);

	bool find(std::string _p , std::string _u);
};

#endif 


//Client.cpp

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
#include "Client.h"

#include <iostream>

	Client::Client(std::string _pwrd , std::string _usr) 
		: m_pword(_pwrd) 
		, m_usrname(_usr)
	{}
	bool Client::compare(std::string _p , std::string _u )
	{
		std::cout << "Comparing..." << std::endl;

		bool samePassword = ( std::strcmp(_p.c_str() , m_pword.c_str()) == 0 );
		bool sameUserName = ( std::strcmp(_u.c_str() , m_usrname.c_str()) == 0 );
		
		return (samePassword && sameUserName);
	}


//Client.h

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
#ifndef CLIENT_H
#define CLIENT_H

#include <string>

class Client
{

	std::string m_pword;
	std::string m_usrname;

public:
	Client(std::string _pwrd , std::string _usr);
	bool compare(std::string _p , std::string _u );
};

#endif 
Last edited on
Thanks. ( I can only use cpp.sh until I get a laptop, I am on a XP) lol.
well then tell us if you find this more appropriate .
:)

> void ConsoleEntry(std::string & _u , std::string & _p)
use meaningful names

> while( !server.send(u , p) )
¿send?

> system("pause");
¿what for?

> std::vector < Client *> clts;
smells like leaks.

> Client* client = new Client(userName , passWord );
yep, leaks all over the place.
http://www.cplusplus.com/forum/general/138037/#msg731921

> std::strcmp(_p.c_str()
http://www.cplusplus.com/reference/string/string/operators/

1
2
3
	bool Client::compare(std::string _p , std::string _u )
	{
		std::cout << "Comparing..." << std::endl;

¿why did you feel the need to output a message there?


PS: ¿why are you using the `lounge' for this?
I did this in 5 mins , but for leaks here is the code to add. This is just a prototype of course. :)

in Client.h
1
2
3
4
5
#include <iostream>
..
..
..
~Client(void){std::cout << "I am deleted by my database" << std::endl;}


in DataBase.h

~DataBase(void);

in DataBase.cpp

1
2
3
4
5
6
7
8
9
	DataBase::~DataBase(void)
	{
		iterator it = clts.begin();
		const_iterator ite = clts.end();
		while(it != ite)
		{
			delete (* it ++);
		}
	}


in main.cpp , you can add scope to see the destruction

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
int main (int argc , char * argv[])
{

	{ 
	Server server;

	std::string p;
	std::string u;
	
	//First catch
	ConsoleEntry( u , p );

	while( !server.send(u , p) )
	{
		ConsoleEntry( u , p );
	}

    std::cout << "\nLoged In!\n";
	}
	system("pause");

	return EXIT_SUCCESS;
}


@ne555
you dont need to show me reference link BTW
Last edited on
Cool, again I cant use "***.h".
why ? cant you download a IDE on a machine ?
Been some nice suggestions so far.

Have you thought about saving usernames and passwords to an encrypted file?

If I wanted to view the secret message ( Now I have seen the source code ). Opening the binary in a hex editor will allow a user to see the clear text strings. Not only this, but you could insert a jmp call to skip right past the validation and come out the other side to start execution of displaying the secret message.

Some food for thought. :)
if you look at the code I published at DataBase.cpp , Line 13 , I suggest to read it from a file . It can be a binary file , a you could try to add another layer in the process to encrypt/decrypt the content of the file(s). You can even zip/unzip the file(s).
Topic archived. No new replies allowed.