search

cartalyst / sentinel

A framework agnostic authentication & authorization system.
BSD 3-Clause "New" or "Revised" License
1.52k stars 240 forks source link

remove redundant is_array check #481

Closed 8633brown closed 5 years ago

8633brown commented 5 years ago

bit of a weird one this one. i cant really figure out the logic behind this although i havent delved too far in. it looks to me like IlluminateUserRepository::paerseCredentials will never return logins as anything other than an array so checking its an array doesn't have much point. although i could have completely missed something.

https://github.com/cartalyst/sentinel/blob/401d36a10dcf0fa8dfb566fe5860166fba68274b/src/Users/IlluminateUserRepository.php#L290-L317

8633brown commented 5 years ago

whoops forgot to test. my bad. although i still cant follow the logic yet haha.

8633brown commented 5 years ago

my bad im just blind and couldnt see this line https://github.com/cartalyst/sentinel/blob/401d36a10dcf0fa8dfb566fe5860166fba68274b/src/Users/IlluminateUserRepository.php#L310 however looking at it now would it not be better to refactor that to pass the string into the logins array. as then my this PR would simplify things IMO.

guess i'm just thinking out loud here, but i think it's somewhat valid. reopened but your welcome to close if you think i'm wrong.

also sorry probably should have put an issue in instead was a little early on this one.

brunogaspar commented 5 years ago

Not sure about this one, as i believe there was a specific reason to have the code like this, but i prefer to not make it a lot more breaking changes on behaviour at the moment.