User logged out, but still appears to be logged in

[wansco]

I'm not sure if I'm doing something wrong, so please bear with me.

I'm running in dev mode and every so often, I get sent to the /forbidden page on what should have been a valid request. On further investigation, I am actually logged out on the server, but the client UI shows the menu bar and user as if I am still logged in.

I've tracked it down to this:
The server session expires somehow (although I haven't figured out why, exactly).

When transitioning to another state, the server policy isAllowed() fires back a 403, which is reasonable given the unauthenticated user.

On the client, auth.interceptor.client.service.js catches the 403 and sends the user to /forbidden, which is also sort of reasonable, given the unauthenticated user. The problem is that the client still thinks they are authenticated because auth.interceptor.client.service.js doesn't clear out the client-side authentication in that case (on a 403).

I'm not sure if the solution would be to change the 403 to 401 in all the policy isAllowed()'s or to attach a message to the 403 that would clue auth.interceptor.client.service.js into the user's server login state (which would then transition to signin rather than forbidden). Changing to a 401 seems the cleanest solution, but I dont know for sure.

To reproduce this: Login, and then flush out the sessions collection on the backend DB. Also, the stock install of the articles module allows guest access in articles.server.policy.js... you should change line 34 to "user", or go to a page that requires an authenticated user to view.

Incidentally, some of the user management pages do not exhibit this behavior, which might be a bit of a security issue. In this unauthenticated state, I am still able to "view" the Edit Profile page, although I cannot make any changes. That throws back a 400 error, which would probably be better served with a 401 or 403. Either way, its sort of inconsistent as to what happens on the client when the server has logged the user out.

Also, a hard refresh solves the issue, but transitioning between states does not reload the client side authentication.

[wansco]

I think the best approach would be to check if the user is set in isAllowed() (on the server policy), and return 403 if the user is set (which then would mean they are both logged in and forbidden), or 401 if the user is not set (which only says they are not logged in, we cant know know if they are forbidden).

The idea there would be a logged in user is forbidden (403), an un-logged in user would ultimately be bounced to the signin page (401, which auth.interceptor would handle, sending the browser to the signin page).

I gave that a shot (401 on an unauthenticated user on the server, in isAllowed()) and now the following line in auth.interceptor.client.service.js is giving me problems.
$injector.get('$state').transitionTo('authentication.signin');

Its loading "/signin" instead of "/authentication/signin"

I also tried .go('authentication.signin'); instead of transitionTo(), but no dice.

[lirantal]

@wansco the problem is well understood, and probably this happens because the user is AFK for quite some time so the session indeed expires. One quick way around it is to increase the session expiration time in the relevant environment config file.

@ilanbiala looks like a valid bug, although a bit of an edge case. How about we have some kind of ping to the server every now and then, or if an action is performed on the page and the user is not allowed to do it (due to not logged in) then we pop-up a login window?

[ilanbiala]

@lirantal I'd rather just redirect them to the login page because that exists and we don't need to implement new handlers and such.

[wansco]

Redirecting to the login page, with a return url to the previous page, would be ideal. I'm not quite sure how to handle that return url since the offending request, in this case, is the API call, not the page load (or state) itself. Either way, this looks to be an issue that will need to be addressed with both server-side and client-side coordination (i.e. consistent use of http error codes 400, 401, or 403 across all api calls, and client side action to flush out the current user when appropriate).

In my 2nd post, it doesn't seem enough to simply return a 401 from isAllowed() (vs 403), because the client still thinks they are logged in. Redirecting to "/authentication/signin" has no meaning unless you clear out the client authentication before going to the login page. We could maybe attach a flag to the http return i.e. {message: 'User is not authorized', useractive:false} that the client could act on in auth.interceptor?

Another possible approach:
Glancing through the HTTP 4xx error codes for a condition that matches this case, it looks like 440 (440 Login Timeout (Microsoft)) would be appropriate. Although I'd prefer to avoid vendor specific extensions, I don't think that would matter much since we could send back just about any error code as long as the client knows what to do with it. Using a different error code would avoid confusion if it could be pulled off. Unfortunately, I don't know how we would identify on the server if the user was logged out, vs never logged in, so this might not be a viable solution.

https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#4xx_Client_Error

[mleanos]

@wansco Looking at this issue, and testing the removal of the user.client.config interceptor, has lead me to similar thoughts that you've shared here.
#1096

Which actually happens to be why you were experiencing issues with changing the server policy to return 401. See the user.client.config interceptor here is catching the 401, and sending to an invalid route. That's why your changes had no effect. This whole interceptor should be removed, and we'll use this core interceptor service

We'll also need to add Authentication.user = null; to the core client interceptor service 401 catch. This will clear the user out of the browser, when you make a request to the back-end API.

For the above to be effective we need to check for the user in the server-side policies, when we get a "access denied" response from areAnyRolesAllowed, like this:

var statusCode = 403; // default status for access denied

// check if non-authenticated request caused access denied
if (!req.user) {
  statusCode = 401;
}

return res.status(statusCode).json({
  message: 'User is not authorized'
});

Once we take care of the above issues, we should be good. The issue of transitioning to a new state will no longer be a problem. Anytime the user transitions to a state that makes a request to the API, and the request is denied (either 401 or 403) then the interceptor will handle it effectively. In the case that a 401 is returned, the Authentication.user = null; line will satisfy the "lingering perception of being logged in".

Lastly, I'm not exactly sure how to handle the fact that the browser still "thinks" the user is logged in because the Authentication.user object is still in scope. Since we're planning to move to Token-Based authentication (#389) we don't really need to worry about this edge case.

The other above issues are legitimate bugs/issues that should be addressed.

[mleanos]

This was already fixed with b12be5f, and can be closed.

[mg1075]

Looks as though this was never implemented, but sure seems like it should be.

// check if non-authenticated request caused access denied
if (!req.user) {
  statusCode = 401;
}