PHP Login system - Best Practice

[Mr Scratch]

Your variable naming convention.

 

[Thor]

simple form based authentication is fine. no need to overkill with token based, or oauth.

those can be added as an additional layer anytime anyway.


please stop abbreviating things. what is the point of omitting the `e` in `usr_id` and `usr_name`?


also, pick a style for variable names, and stick to it. `$succcessmsg` vs `$password_error`. that is not consistent.

http://www.php-fig.org/psr/psr-1/
http://www.php-fig.org/psr/psr-2/

When ever I use sessions or cookies I use usr whenever I interact with the DB I use user it is for me I am the only working on it.

I am quite consistent in my variables

$name_error
$email_error
$password_error
$cpassword_error

then

$successmsg

or

$errormsg

Very easy to read, very easy to understand very self explanatory, but by all means refactor it



Something I do try to follow is that PSR guidelines, but still a work in progress as I am not OOP

 

[_kabal_]

When ever I use sessions or cookies I use usr whenever I interact with the DB I use user it is for me I am the only working on it.

what about from a file? double underscores, skipping each even index letter of the alphabet.

you do know that a session, or a cookie, is a database?

I am quite consistent in my variables

$name_error
$email_error
$password_error
$cpassword_error

then

$successmsg

or

$errormsg

so not consistent at all

in all seriousness though, for your validation I would rather user an `errors` array.

$errorMessages['name'] = //
$errorMessages['email'] = //
$errorMessages['password'] = //
$errorMessages['confirmPassword'] = //

now your validation output could easily change in the future.

eg

foreach ($errorMessages as $errorMessage) {
    echo '<span>'.$errorMessage.'</span>';
}

also, IMO, there is no such thing as an $errormsg, or a $successmsg.
There is a $status, and a $message

 

[Thor]

what about from a file? double underscores, skipping each even index letter of the alphabet.

you do know that a session, or a cookie, is a database?



so not consistent at all

in all seriousness though, for your validation I would rather user an `errors` array.

$errorMessages['name'] = //
$errorMessages['email'] = //
$errorMessages['password'] = //
$errorMessages['confirmPassword'] = //

now your validation output could easily change in the future.

eg

foreach ($errorMessages as $errorMessage) {
    echo '<span>'.$errorMessage.'</span>';
}

also, IMO, there is no such thing as an $errormsg, or a $successmsg.
There is a $status, and a $message

Appreciate the clarification but you look at my code in the post above the html part then you will see this:

foreach ($errorMessages as $errorMessage) {
    echo '<span>'.$errorMessage.'</span>';
}

Would not work for me.

Granted I am not a professional I might be missing the entire point, let me put all of this on github tonight and then one day when you are bored you can refactor all of it and then I can see in a whole what you meant since at the moment looking at it from a whole I still do not see what is wrong with my way I do not see how I can replace all the specific error messages in various parts of my html with a foreach loop.

 

[Thor]

what about from a file? double underscores, skipping each even index letter of the alphabet.

you do know that a session, or a cookie, is a database?

Session is not MySQL so for me the two is not the same.

MySQL is what I refer to as my DB

I do not store Sessions in my DB I assign DB values to my session.

Hope that makes sense?

 

[_kabal_]

I didn't say you should change to a forEach, I said you could.

Imagine one day this code was separated from the ui, and registration could happen via a rest API call OR a form post.

Imagine you want to return a required and a min length validation for the same field at the same time

Maybe then you would do something like.

$errors['password']['required'] = 'password is required';
$errors['password']['min'] = 'password must be 6 chars';
$errors['password']['regex '] = 'password must be 3 letters and 3 numbers';

now your existing UI can be updated with no effort

<div class="form-group">
    <label for="password>Password</label>
    <input type="password" name="password" />
    <?php
    foreach ($errorMessages['password'] as $errorMessage) {
        echo '<span class="text-danger">'.$errorMessage.'</span>';
    }
    ?>
</div>

and the JSON response for the REST client maybe looks something like:

{
    errors: [
        password: {
            required: "password is required",
            min: "password must be 6 chars",
            regex: "password must be 3 letters and 3 numbers"
        }
    ]
}

 

[IndigoIdentity]

simple form based authentication is fine. no need to overkill with token based, or oauth.

Sure but in my own experience, spending the time in learning and understanding a more structured approach to this brings benefits whilst creating more simplified authentication mechanisms. That was my point put simply.

 

[_kabal_]

That's all well and good, but as they say, learn to walk before running and all that.

I would worry about the mix of db, session and html, and the lack of structure there, before what auth mechanism was used.

 

[IndigoIdentity]

learn to walk before running and all that.

Failing to see how learning to implement oAuth is any more complicated than trying to build your own authentication mechanism from scratch. Then there's the point of how easy it is to pick up bad habits whilst learning and how they can stick around for a long time.

I worry too... That the mix of things there will end up in a production environment and someone ends up with a new clone.

 

[IndigoIdentity]

maybe quote me with the rest of my post as context??

Now you're making even more sense...

Cat rat dog hat sat!

 

[_kabal_]

 

[IndigoIdentity]

Lol you're a troll, I should have known it...

But seriously I understand why he wants to do what hes doing and yes I think it would be good experience so he should carry on but I was simply implying that in the long run... He would be better off spending some time learning about authentication mechanisms, make use of that because it's a solid point to start off on and then if you want to explore other ways of doing this kind of then atleast you have the backing of the understanding of how things generally work vs what your implementation is doing.

As I said, sure I can appreciate the issues with the code and it all comes with learning but I mean... The topic here contains best practice and i'd say that would be not to try to roll your own authentication things in a day and age where where is heaps of these out there that have been tested and proven to work. Sticking to that come hell or high water so troll on!

 

[rward]

Agreed!

Best Practice for Authentication -> Don't Roll your own.

 

[Thor]

Agreed!

Best Practice for Authentication -> Don't Roll your own.

I want to learn, I want to see and understand what the process involves I want be low level I want to write and build APIs for the language one day. I really feel if we keep telling people don't do it yourself copy and paste leverage the community then 10 years from now you won't have programmers anymore you'll just have a bunch of monkies that's my view anyway.

 

[rward]

I really feel if we keep telling people don't do it yourself copy and paste leverage the community then 10 years from now you won't have programmers anymore you'll just have a bunch of monkies that's my view anyway.

I agree to an extent but there are some pieces where using an already created best practice is best and user authentication and management and session management are one (three) of them.

Most hacks happen around user management and handling. Stick to best practices and hopeful you'll avoid the common pitfalls that you would probably hit in a'roll your own'

 

[[)roi(]]

I want to learn, I want to see and understand what the process involves I want be low level I want to write and build APIs for the language one day. I really feel if we keep telling people don't do it yourself copy and paste leverage the community then 10 years from now you won't have programmers anymore you'll just have a bunch of monkies that's my view anyway.

That's a good approach to take... building something yourself always implies an informed decision. Just do your research, there is no reason why you can't build something secure.

 

[SBSP]

My2c

Instead of trying to slap an authentication system together for an existing project. Rather totally forget about the existing project and treat the authentication part as a totally new project.
Design it in such a way that it can be re-used in future projects.

The reason why people give you the “Leave it to the pros” attitude is because security should not be taken lightly. They are afraid, Don’t blame them. But don’t let that stop you from building your own system.
Being the purest that I am I don’t like depending on other people’s code. Non-standard practices aren’t necessarily a sin and can be more secure than system developed by people like Google and FB. The bigger the company the Bigger the target the bigger the reward for hacking it.

 

[rward]

They are afraid, Don’t blame them. But don’t let that stop you from building your own system.

Not afraid. When your end goal is to have whatever Thor is planning on building (I can't reacall if he has said what it is but I don't think it's a login component) then you don't want to be spending 4 months developing and testing your login component....

and can be more secure than system developed by people like Google and FB. The bigger the company the Bigger the target the bigger the reward for hacking it.

The bigger the company, the bigger the budget, the more developers they have, the better their skills are, the better their practices, the more code reviews, the more QA people they have, the better their bug finding potential is...

By yourself, you're writing the code, you're doing the review (if there even is one) and you're doing the testing. It takes a lot of 'out the box' thinking to start getting edge cases..

I do agree that it is a good idea to write a login component that he can reuse. It's in Javascript but check out http://passportjs.org/ or php - http://hybridauth.sourceforge.net/

 

[Thor]

My2c

Instead of trying to slap an authentication system together for an existing project. Rather totally forget about the existing project and treat the authentication part as a totally new project.
Design it in such a way that it can be re-used in future projects.

The reason why people give you the “Leave it to the pros” attitude is because security should not be taken lightly. They are afraid, Don’t blame them. But don’t let that stop you from building your own system.
Being the purest that I am I don’t like depending on other people’s code. Non-standard practices aren’t necessarily a sin and can be more secure than system developed by people like Google and FB. The bigger the company the Bigger the target the bigger the reward for hacking it.

That is what this is, totally from scratch from the ground up for me by me

 

[Thor]

php - http://hybridauth.sourceforge.net/

I like this!