SessionGuard does not check remember_token column existence

[b-zee]

• Laravel Version: 5.3.24
• PHP Version: 7.0.13
• Database Driver & Version: MariaDB/MySQL 10.1.19

Description:

SessionGuard->logout() does not check whether a remember token field exists (logging out), while AuthenticateUsers->attemptLogin() does check by $request->has('remember').

Default LoginController with AuthenticateUsers trait. The call to AuthenticateUsers->logout() will call $this->guard()->logout() which calls $this->refreshRememberToken($user).

This means an error will be thrown when logging out. The error is an SQL error (or sqlite etc.) saying the remember_token column does not exist.

Steps To Reproduce:

Create a custom App/User and migration, without a 'remember me' token. Try logging out. Using default session guard etc. (mainly achieved by make:auth).

[srmklive]

@Bennoz Why would logout method have that? It is automatically refreshed when the user logs out. This is not an error in the framework. If you are not using the default user migration, then i think you need to implement your own custom auth provider in this regard.

[b-zee]

My point is basically about the ease of use of this framework. The framework allows us to use a lot of defaults, and even checks whether we have a remember_token column except for the SessionGuard. This essentially renders the SessionGuard as out of line.

In Laracasts for example something said a lot is that if we don't need it, we can cut it out! But, when we do in fact cut this specific part out, we get an error only because of the SessionGuard. I understand you can use a custom provider to solve this, but this is about the adaptability of this framework and therefore an issue of this framework.

[manan-jadhav]

https://github.com/laravel/framework/blob/5.3/src/Illuminate/Auth/SessionGuard.php#L578

Session Guard tries to set the remember token, without checking even if it exists or not.

[manan-jadhav]

Well basically logging in by setting the $remember parameter to true in $guard->attempt() should also fail, because that also does not check for existence of the remember token.

[b-zee]

That is indeed what would happen when you pass the parameter, but, again I'm talking about default behaviour when using the AuthenticatesUsers trait. Me, as a framework user does not even touch the $guard->attempt() method, as everything is set up by default to make use of the LoginController. So there is no way for me as a simple developer to make sure the remember token is not being set. Hence the sensible way to disable the feature, which is removing the token column, should be respected by the framework.

[srmklive]

@Bennoz Again, it is something you need to implement on your own. It's not an issue with the framework. Instead of criticizing the implementation, how about you create a pull request for inclusion into the framework.

[b-zee]

I unfortunately have no knowledge of both the pull requests and the internal workings of the framework. Anyway, if it's not an issue, then why would I? I'm merely pointing out an inconsistency within the use of the framework, which is from my (framework) user point of view.

If this really is not an issue (and the error is therefore expected behaviour), I expect an authorised developer to close this.

[manan-jadhav]

Fixing this is going to need breaking changes. It won't be an easy fix. Authenticatable and UserProvider will have to change. Maybe fix this in 5.4 ?

[srmklive]

Yeah i think 5.4 may the right one for this or this could go into 5.3 as well. But i feel that errors will happen when you are retrieving user details but are using DatabaseUserProvider and the remember me column name is not remember_token