how how to write a C source code for a Login system?

[anu123d]

Hello all!

I am trying to write a program in C , which works as an efficient Login function. It is expected to have following properties:

1.Should accept the username( valid usernames are : '11user1',
'12user2' & '13user3' ). The usernames are hardcoded in the program and not stored in the file.

2. A case insensitive comparison is to be done for usernames.

3. Should accept the password. the password should not be displayed on the screen.

4. A case sensitive comparison is to be done for password.

Please suggest me, that how can i perform the case insensitive comaprisons for usernames and case sensitive comparison for passwords

Thanks in advance.
Regards,
Anupama

[Narue]

strcmp and friends already do a case sensitive comparison. It's easy to write a function that does a case insensitive comparison; it's just the same algorithm but with tolower or toupper matching the case:

int nocase_strcmp ( const char *a, const char *b )
{
  while ( tolower ( *a ) == tolower ( *b ) ) {
    if ( *a == '\0' )
      break;

    ++a;
    ++b;
  }

  return *a - *b;
}

[anu123d]

oh thank you for replying!

Regards,
Anupama

[dileepkumar235]

written by dileep basam ..
******************************************

#include <stdio.h>
#include <stdlib.h>
#include <conio.h>
#include <string.h>
main()
{
int i,counter=0,flag=0;
char uid[25],pwd[25],s_uid[][25]={"11user1","12user2","13user3"};
char s_pwd[][25]={"Pwd1","Pwd2","Pwd3"},ch='a';/*dummy character in ch */
 clrscr();
printf("\n Enter the user id : ");
scanf("%s",uid);

printf("\n Enter the password : ");
i=0;
while(1)
{
	ch=getch();
	if(ch==13)
	break;
	else if(ch==8)
	{       if(i!=0) /*this is for avoiding the ENTER instructions getting deleted */
		{
			printf("\b");  /*printing backspace to move cursor 1 pos back*/
			printf("%c",32);/*making the char invisible which is already on console*/
			printf("\b"); /*printing backspace to move cursor 1 pos back*/
			i--;
			pwd[i]='\0';
		}
		else
		continue;

	}
	else
	{
	putchar('*');/* char - '*' will be printed instead of the character */
	pwd[i]=ch;
	i++;
	}
}
pwd[i]='\0';


for(i=0;i<=2;i++)
{
	if((stricmp(uid,s_uid[i]))==0 && (strcmp(pwd,s_pwd[i]))==0)
	{
		flag=1;
		break;
	}
}
if(flag) printf(" \n \n \t \t USER AUTHENTICATED ");
else printf("\n \n \n\t TRESSPASSERS WILL BE SHOT AND SURVIVORS WILL BE SHOT AGAIN .. ha ha :)");
printf("written by dileep basam .. [snipped email]");
getch();
}

[Narue]

>written by dileep basam ..
Holy crap. Okay, I guess correcting your errors will take a long post. :icon_rolleyes:

>main()
Let's get one thing straight. What you're doing is called implicit int. It means that if you don't supply a type where one is expected, int is assumed. If your compiler allows you to do this (implicit int was removed in C99 and will no longer compile), don't forget that the two following definitions of main are exactly the same:

main()

int main()

In other words, both are defined to return an int, and failing to return an int is undefined behavior. Your definition of main displays one of the biggest problems with implicit int, and that's the mistaken assumption that not providing a return type means there is no return type and no return value is needed.

There are two correct ways of defining main:

int main ( void )

int main ( int argc, char *argv[] )

The portable return values are 0 and EXIT_SUCCESS (defined in stdlib.h) for success, and EXIT_FAILURE (also defined in stdlib.h) for failure. Anything else that's not directly equivalent to the above is incorrect.

>int i,counter=0,flag=0;
>char uid[25],pwd[25],s_uid[][25]={"11user1","12user2","13user3"};
>char s_pwd[][25]={"Pwd1","Pwd2","Pwd3"},ch='a';/*dummy character in ch */
Stylistically this is very poor because it makes reading your code more difficult. Don't sacrifice readability for perceived brevity, it's a cheap tradeoff, and that's why most style guidelines will tell you to limit yourself to only one variable declaration per line.

The size of uid and password shouldn't be hard coded as a literal. This is why manifest constants are supported. Instead of 25, #define a macro called FIELD_SIZE, or something and use that. A hidden benefit of manifest constants is that it's more tedious to use scanf safely with them, so you'll be more inclined to choose fgets. ;)

You don't use counter, and flag is a poor choice of names. is_authenticated or is_auth is easier to understand at a glance. Likewise, uid and pwd make sense, but how are readers supposed to intuit how they're different from s_uid and s_pwd? Ignoring the fact that you shouldn't store user IDs and passwords like this, a structure would be better because the ID and password are related by an account.

ch is only used in a nested scope, so you can move the declaration to that scope. It's best to restrict the scope of a variable as much as possible so that it doesn't clash with the rest of your code. The code is also easier to follow when you know that a variable can't be used anywhere (which is one of the rationales for avoiding global variables).

If you change the loop from an infinite loop to a loop bound by the result of getch, you can't do this anymore though.

Finally, note that getch returns int, not char. The type of ch is incorrect.

>clrscr();
It's generally unwise to remove the output of every program that ran before yours in the console. You don't own that output, and you should leave it be. Calling clrscr is usually a programmer trying to be cute with a "cool" user interface, but it's very antisocial.

>printf("\n Enter the user id : ");
>scanf("%s",uid);
Three problems. First, stdout isn't required to be flushed before scanf blocks for input, so it's possible that your user won't see the prompt and won't know what to do with the cursor blinks waiting for his input. You fix this by printing a newline or calling fflush.

Second, you need to check that scanf succeeded. Always always always verify that input succeeded, that's a very likely point of failure for your program and those bugs can be hard to catch.

Finally, never use "%s" without a field width because it's no different from gets. The field width will ensure that scanf doesn't write beyond the boundaries of the array. The correct field width is the size of the array minus room for the null character: "%24s" in your case.

Of course, fgets would be a better choice here.

>printf("\n Enter the password : ");
Once again, because the newline is at the front of the string, the prompt may not be displayed before getch blocks for input.

>while(1)
I recommend using for ( ; ; ) because while ( 1 ) often causes compilers to throw a warning about a conditional constant. However, because the first thing you do in all cases for this loop is call getch, you can do this as a part of the condition.

I notice that you don't protect against buffer overflow in your password loop. You need to make sure that i doesn't exceed the size of the array, just as you made sure that i doesn't drop below 0.

>if(ch==13)
C has a character literal for this: '\r'. When you have a character literal, please don't use the ASCII code because you may not end up running on a system that uses ASCII.

>else if(ch==8)
C has a character literal for this too: '\b'. I would wager that 99% of C programmers can tell you that the backspace character is '\b', but a much smaller percentage will know that the ASCII code for '\b' is 8. Just because it's called source code doesn't mean you should make it impenetrable. Clearly you know about '\b', or you wouldn't have used it later on. Consistency is important too. :icon_rolleyes:

>printf("%c",32)
The character literal is ' '. You have no excuse for hard coding 32 instead of ' '. Seriously. You can merge the three printf calls into one by making the format string "\b%c\b" as well.

>pwd = '\0';
This is unnecessary inside the loop. The actually contents of pwd aren't used until after this loop, and you assign a null character there anyway. As long as the value of i is correct, you're solid.

>else continue;
This is quite pointless as following the normal flow of execution does exactly the same thing. You can safely remove this clause and the effect will be identical.

>for(i=0;i<=2;i++)
For starters, the move idiomatic way is to use an open ended range [0, N), which is translated to code in this case by saying for ( i = 0; i < 3; i++ ) . This is easier to validate.

Also, it's all well and good when you know exactly how many accounts there are, but in reality you don't, or there will be enough that counting them manually is tedious and error prone. Because it's an array currently, you can use the trick of sizeof array / sizeof array[0] to find out how many elements there are, and it will always be correct even when you decide to add or remove accounts.

I like to use a macro for this:

#define length(array) (sizeof (array) / sizeof (array)[i])

You'll have to programmatically count and store the size later on though, such as if you choose to move the accounts to a file or database (not forgetting to encrypt the data, of course, because plain text user names and passwords is stupid).

>(stricmp(uid,s_uid))==0
Note that stricmp is not a standard function, so like getch and clrscr, it may not be available and the code may not compile. It's easy to write your own case insensitive comparison though, so you might consider that as an alternative to non-standard libraries.

Also note that you're using redundant parentheses around the stricmp call (and strcmp call later on). Removing them will make the code easier to read.

>break;
Some people would recommend that you use is_auth as a part of the condition:

for ( i = 0; !is_auth && i < length ( s_uid ); i++ )

I don't care either way (though I would probably write it using a break myself), so I'm just offering it as an alternative.

>if(flag) printf( ... );
>else printf( ... );
I'm not a fan of this "all on one line" practice at all. In my opinion it's far easier to follow the flow of the code if you define the block on a separate line, even if the block is only a single simple statement:

if ( flag )
  printf ( ... );
else
  printf ( ... );

>getch();
And of course, the obligatory recommendation of not using getch to pause your program at the end. It's obligatory because you have a legitimate use of getch in this program, so you've already made the decision to be non-portable. Thus the portability argument is moot.

However, another argument still stands. If your program is not run from an IDE, but from the console, there will be a redundant pause to keep the console window open when it would have remained open anyway. I'll remove it in my corrected program, but you can add it back if you choose.

I imagine you wrote this quickly and didn't consider modularization, but you can easily break your main function up into a few functions to clean up the code a bit. For example, the authorization check is completely independent. It can practically be cut and pasted into another function without any changes.

The password reading code is useful enough that it should be in a function anyway, and it's also pretty well isolated.

Here's my version of your code. The logic is still the same:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <conio.h>

#define FIELD_SIZE 25
#define length(array) ( sizeof (array) / sizeof (array)[i] )

struct account {
  char *id;
  char *password;
};

static struct account accounts[] = {
  { "11user1", "Pwd1" },
  { "12user2", "Pwd2" },
  { "13user3", "Pwd3" }
};

int is_authorized ( const char *uid, const char *pwd )
{
  int i;

  for ( i = 0; i < length ( accounts ); i++ ) {
    if ( stricmp ( uid, accounts[i].id ) == 0 && 
         strcmp ( pwd, accounts[i].password ) ==0 ) 
    {
      return 1;
    }
  }

  return 0;
}

void get_password ( char *pwd, int size )
{
  int i = 0;
  int ch;

  while ( i < size - 1 && ( ch = getch() ) != '\r' ) {
    if ( ch == '\b' ) {
      /* Don't run off the front end of the array */
      if ( i != 0 ) {
        printf ( "\b%c\b", ' ' );
        --i;
      }
    }
    else {
      putchar ( '*' );
      pwd[i++] = (char)ch;
    }
  }

  pwd[i] = '\0';
}

int main ( void )
{
  char uid[FIELD_SIZE];
  char pwd[FIELD_SIZE];

  printf ( "User ID: " );
  fflush ( stdout );

  if ( fgets ( uid, sizeof uid, stdin ) != NULL ) {
    char *newline = strchr ( uid, '\n' );

    /* Trim the newline if it's present */
    if ( newline != NULL )
      *newline = '\0';

    printf ( "Password: " );
    fflush ( stdout );

    get_password ( pwd, sizeof pwd );

    if ( is_authorized ( uid, pwd ) )
      printf("\n\n\t\tUSER AUTHENTICATED\n");
    else
      printf("\n\n\t\tINVALID USER\n");
  }

  return 0;
}

[anu123d]

@dileepkumar235

good try.. thanks! :)

[anu123d]

@Narue
Thank you so much! the post is quite helpful. I am quite new to programming, and i love C :)