Welcome Guest, Not a member yet? Register   Sign In
Core Auth Library
#19

[eluser]thody[/eluser]
[quote author="jpi" date="1250258195"]Hi,

A few feedback regarding you auth library :

create method :
what if $user['password'] isn't set ? You should trigger an error or a warning
You should check that each key in array $user is a valid field in the DB table user

get_user_id method : it should return an int not a string
delete method : if identifier is username, as your DB scheme is not UNIQUE for the field username, this method could delete several users...
login method : $username could possibly be NULL but the first line of this method is a call _test_user_credentials where $username shouldn't be NULL. That's not consistent.
_set_persistent_session : you use a custom cookie. It does not store the IP adresse or at least the user agent. Cookies can be stolen.

Regarding your SQL table. Why is `id` an int(11) ? Do you expect billions of users ? It should also be unsigned. password whould be a varchar(40) because dohash use sha1.

A lot to correct, but you really should think when you are coding "what I want this method to do but also what i dont want it to do and what could someone use it to do something unexpected". Also keep writing a lot of unit test, that's cool Smile[/quote]

Hey jpi, really appreciate the feedback, you're right on with your observations. What you're seeing there is really just a couple hours of work, more or less a proof of concept, and has a long way to go obviously. At this point the goal has essentially been "make it work", and now that the core functionality is there, I'll be going back and tidying things up.

Feel free to fork the project and contribute, or keep posting issues here or on the issues tab in GitHub. Any help is appreciated.

Cheers,
Adam


Messages In This Thread
Core Auth Library - by El Forum - 08-12-2009, 08:53 AM
Core Auth Library - by El Forum - 08-12-2009, 10:44 AM
Core Auth Library - by El Forum - 08-12-2009, 11:00 AM
Core Auth Library - by El Forum - 08-12-2009, 11:54 AM
Core Auth Library - by El Forum - 08-12-2009, 12:05 PM
Core Auth Library - by El Forum - 08-12-2009, 12:32 PM
Core Auth Library - by El Forum - 08-12-2009, 12:45 PM
Core Auth Library - by El Forum - 08-12-2009, 12:49 PM
Core Auth Library - by El Forum - 08-12-2009, 01:04 PM
Core Auth Library - by El Forum - 08-12-2009, 01:24 PM
Core Auth Library - by El Forum - 08-12-2009, 03:07 PM
Core Auth Library - by El Forum - 08-12-2009, 05:12 PM
Core Auth Library - by El Forum - 08-12-2009, 06:01 PM
Core Auth Library - by El Forum - 08-12-2009, 06:57 PM
Core Auth Library - by El Forum - 08-12-2009, 07:41 PM
Core Auth Library - by El Forum - 08-12-2009, 08:15 PM
Core Auth Library - by El Forum - 08-13-2009, 07:25 AM
Core Auth Library - by El Forum - 08-14-2009, 02:56 AM
Core Auth Library - by El Forum - 08-14-2009, 04:56 AM
Core Auth Library - by El Forum - 08-14-2009, 08:12 AM
Core Auth Library - by El Forum - 08-14-2009, 08:18 AM
Core Auth Library - by El Forum - 08-14-2009, 08:19 AM
Core Auth Library - by El Forum - 08-14-2009, 08:29 AM
Core Auth Library - by El Forum - 08-14-2009, 08:51 AM
Core Auth Library - by El Forum - 08-14-2009, 09:02 AM
Core Auth Library - by El Forum - 08-14-2009, 09:19 AM
Core Auth Library - by El Forum - 08-14-2009, 09:33 AM
Core Auth Library - by El Forum - 08-14-2009, 11:23 AM
Core Auth Library - by El Forum - 08-14-2009, 11:49 AM
Core Auth Library - by El Forum - 08-14-2009, 12:38 PM
Core Auth Library - by El Forum - 08-14-2009, 12:56 PM
Core Auth Library - by El Forum - 08-14-2009, 01:07 PM
Core Auth Library - by El Forum - 08-14-2009, 01:10 PM
Core Auth Library - by El Forum - 08-14-2009, 02:21 PM
Core Auth Library - by El Forum - 08-14-2009, 06:39 PM
Core Auth Library - by El Forum - 08-16-2009, 01:38 AM
Core Auth Library - by El Forum - 08-16-2009, 06:02 AM
Core Auth Library - by El Forum - 08-16-2009, 09:02 AM
Core Auth Library - by El Forum - 08-16-2009, 11:18 AM
Core Auth Library - by El Forum - 08-16-2009, 01:17 PM
Core Auth Library - by El Forum - 08-16-2009, 01:37 PM
Core Auth Library - by El Forum - 08-16-2009, 03:12 PM
Core Auth Library - by El Forum - 08-16-2009, 04:03 PM
Core Auth Library - by El Forum - 08-16-2009, 06:23 PM
Core Auth Library - by El Forum - 08-16-2009, 07:01 PM
Core Auth Library - by El Forum - 08-21-2009, 01:15 AM
Core Auth Library - by El Forum - 08-21-2009, 05:06 AM
Core Auth Library - by El Forum - 08-22-2009, 07:04 AM
Core Auth Library - by El Forum - 08-22-2009, 07:45 AM
Core Auth Library - by El Forum - 08-22-2009, 04:29 PM
Core Auth Library - by El Forum - 08-22-2009, 07:12 PM
Core Auth Library - by El Forum - 08-23-2009, 05:43 AM
Core Auth Library - by El Forum - 08-24-2009, 06:08 PM



Theme © iAndrew 2016 - Forum software by © MyBB