CUTCODEDOWN
Minimalist Semantic Markup

Welcome Guest
Please Login or Register

If you have registered but not recieved your activation e-mail in a reasonable amount of time, or have issues with using the registration form, please use our Contact Form for assistance. Include both your username and the e-mail you tried to register with.

Author Topic: My LogIn Function - Safe & Secure?  (Read 1078 times)

GrumpyYoungMan

  • Hero Member
  • *****
  • Posts: 787
  • Karma: +8/-0
    • Grumpy Young Man
My LogIn Function - Safe & Secure?
« on: 2 Feb 2020, 07:55:32 am »
Is this safe and secure?
Code: [Select]
function loginout_LogInType($type) {

global $DB;

if( $type = "user" ) {

// Fetch User from DB - by User
$fetchUser = $DB->prepare("
SELECT
u_id, u_user, u_email, u_password, u_login_attempts, u_locked
FROM
{$DB->CONFIG['sql_tbl_prefix']}users
WHERE
u_user=:username
");

$fetchUser->execute( [
':username' => $_POST['username'],
] );

}
else {

global $USER;

// Fetch User from DB - by UserId
$fetchUser = $DB->prepare("
SELECT
u_id, u_password, u_login_attempts, u_locked
FROM
{$DB->CONFIG['sql_tbl_prefix']}users
WHERE
u_id=:uid
");

$fetchUser->execute( [
':uid' => $USER['u_id'],
] );


}

return $fetchUser->fetch();

}

function loginout_LogIn() {

global $CONFIG, $DB;

$errors = [];

$maxLogInAttempts = 3;

// Fetch the User Details:
$dbUser = loginout_LogInType('user');

// +------------------------------------------------------------------------------------------------------------------------------------------------


if ( false !== $dbUser ) {

// Is the account locked:
if( $dbUser['u_locked'] != 1 ) {

if( ! password_verify($_POST['password'], $dbUser['u_password'] ) ) {

$errors[] = "invalid_login_2_credentials";

if( $dbUser['u_login_attempts'] >= 0 && $dbUser['u_login_attempts'] < $maxLogInAttempts ) {

// Update FAILED login attempts:
$userLogInAttempts = $DB->prepare("
UPDATE
{$DB->CONFIG['sql_tbl_prefix']}users
SET
u_login_attempts = Coalesce(u_login_attempts, 0)+1, u_last_login_attempt = NOW()
WHERE
u_id=:uid
");

$userLogInAttempts->execute( [
':uid' => $dbUser['u_id'],
] );

}
else {

// Incorrect PASSWORD:
// Update FAILED login attempts:
$userLogInAttempts = $DB->prepare("
UPDATE
{$DB->CONFIG['sql_tbl_prefix']}users
SET
u_login_attempts = Coalesce(u_login_attempts, 0)+1, u_last_login_attempt = NOW(), u_locked=1
WHERE
u_id=:uid
");

$userLogInAttempts->execute( [
':uid' => $dbUser['u_id'],
] );

// ADD EMAIL TO QUEUE
$lockedEmail = $DB->prepare("
INSERT INTO
{$DB->CONFIG['sql_tbl_prefix']}email_queue (
eq_id, eq_to, eq_from, eq_subject, eq_message, eq_added
)
VALUES (
:id, :to, :from, :subject, :message, NOW()
)
");

$lockedEmail->execute( [
':id' => uniqid(),
':to' => $dbUser['u_email'],
':from' => $CONFIG['email_from'],
':subject' => "Account Locked",
':message' =>"Your account has now been locked for security reasons. <p>Sorry!</p>Test Message!"
] );

}

}
else {

// Login Successful:

// Update Last Login:
$userLastLogIn = $DB->prepare("
UPDATE
{$DB->CONFIG['sql_tbl_prefix']}users
SET
u_last_login = NOW()
WHERE
u_id=:uid
");

$userLastLogIn->execute( [
':uid' => $dbUser['u_id'],
] );

$_SESSION['user_id'] = $dbUser['u_id'];

// Return nothing, if we are a valid user...

}

}
else {

$errors[] = 'invalid_login_3_credentials';

}

}
else {

// Invalid DB User
$errors[] = "invalid_login_1_credentials";

}

// Return Errors - if any?
return $errors;

}
I've moved it into a function as the admin/user panels will also use this function to validate the user before changing things like email address and password changes?
« Last Edit: 2 Feb 2020, 08:16:16 am by GrumpyYoungMan »
Trying to learn a new trick to prove old dogs can learn new ones...

Total Novice have-a go Amateur Programmer - not sure that is the right thing to say... but trying to learn...

Dave

  • Junior Member
  • *
  • Posts: 38
  • Karma: +12/-0
Re: My LogIn Function - Safe & Secure?
« Reply #1 on: 2 Feb 2020, 09:33:39 am »
Right off the bat, if( $type = "user" ) will always be true since you are assigning 'user' to $type. I know you know that that should be == :) Also, no need to use double quotes there. Use single quotes instead.

Pass $DB, $USER and $CONFIG to your functions instead of using global.

Looks OK otherwise though the formatting of it in the question is terrible (probably not your fault).
Dave

GrumpyYoungMan

  • Hero Member
  • *****
  • Posts: 787
  • Karma: +8/-0
    • Grumpy Young Man
My LogIn Function - Safe & Secure?
« Reply #2 on: 2 Feb 2020, 10:47:44 am »
Right off the bat, if( $type = "user" ) will always be true since you are assigning 'user' to $type. I know you know that that should be == :) Also, no need to use double quotes there. Use single quotes instead.
Thanks, yes indeed
Code: [Select]
if ( $type == 'user' ) I spotted that after I tested and posted the code!
Pass $DB, $USER and $CONFIG to your functions instead of using global.
Will look into that... I guess that is more secure?
Looks OK otherwise though the formatting of it in the question is terrible (probably not your fault).
I am pleased - it made sense to me to create an universal login function.... when I started to created the UserCP and wanted to check there passwords before actioning there changes...

yeah the formatting plays up on this forum - it is laid out perfectly in my editor.

Thank you for taking the time to reply Dave, I appreciate it! :)
Trying to learn a new trick to prove old dogs can learn new ones...

Total Novice have-a go Amateur Programmer - not sure that is the right thing to say... but trying to learn...

Dave

  • Junior Member
  • *
  • Posts: 38
  • Karma: +12/-0
Re: My LogIn Function - Safe & Secure?
« Reply #3 on: 2 Feb 2020, 01:19:03 pm »
Be aware that "universal" will probably change and you'll want to keep an eye out for scope creep. It's almost guaranteed that things will need changing at some point.
Dave

GrumpyYoungMan

  • Hero Member
  • *****
  • Posts: 787
  • Karma: +8/-0
    • Grumpy Young Man
Re: My LogIn Function - Safe & Secure?
« Reply #4 on: 2 Feb 2020, 02:14:14 pm »
So do you think it’s a bad idea having an general master login function?!
Trying to learn a new trick to prove old dogs can learn new ones...

Total Novice have-a go Amateur Programmer - not sure that is the right thing to say... but trying to learn...

Dave

  • Junior Member
  • *
  • Posts: 38
  • Karma: +12/-0
Re: My LogIn Function - Safe & Secure?
« Reply #5 on: 3 Feb 2020, 10:16:28 am »
Didn't say that. I have probably a dozen or so "standard" functions in a class that I usually include when I start a new project. Some of them haven't  been modified in 10 years, some of them are still getting refined and tweaked.
Dave

 

SMF spam blocked by CleanTalk

Advertisement