Mechanism to detect if you've been logged out, prompt you to re-enter your password #253

Open
tobscure opened this Issue Aug 27, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@tobscure
Member

tobscure commented Aug 27, 2015

Since API tokens expire after 14 days, it's possible to be "logged out" while the client is open. You stop having permission to do things and stop getting non-public data, but the client still thinks it's logged in. We should detect if this has happened and get the client to prompt for a password so it can get a new token. (related to #219)

What needs to be done:

  • In Flarum\Api\LoginWithHeader, if an invalid token is presented, we should return a 401 or 403 + error message.
  • The client's app.request method should look for this specific error. If encountered, it should present a modal to the user asking them to re-enter their password. The modal should contain the username + avatar, a password input, a "login" button, and a "cancel" button.
  • If the login button is clicked, it should attempt to get a new token from the API. If successful, dismiss the modal and carry on.
  • If the cancel button is clicked, it should unset app.session.user and dismiss the modal.

@tobscure tobscure referenced this issue Aug 28, 2015

Closed

v0.1.0 roadmap (old) #74

19 of 53 tasks complete

@tobscure tobscure added the Feature label Aug 28, 2015

@RonnyO

This comment has been minimized.

Show comment
Hide comment
@RonnyO

RonnyO Aug 31, 2015

I believe this just happened to me (got logged out, action didn't work, had to copy-paste my message, refresh and login). What's weird is only registered yesterday, so this wasn't a 14 days expiration.

RonnyO commented Aug 31, 2015

I believe this just happened to me (got logged out, action didn't work, had to copy-paste my message, refresh and login). What's weird is only registered yesterday, so this wasn't a 14 days expiration.

@justjavac justjavac referenced this issue in justjavac/Flarum Sep 7, 2015

Open

Flarum v0.1.0 开发路线图 #3

18 of 53 tasks complete
@franzliedke

This comment has been minimized.

Show comment
Hide comment
@franzliedke

franzliedke Sep 9, 2015

Member

If we trust Wikipedia's list of HTTP status codes, we can use the HTTP 419 status code (although it's not standard).

Is that always caused by the InvalidConfirmationTokenException?

Member

franzliedke commented Sep 9, 2015

If we trust Wikipedia's list of HTTP status codes, we can use the HTTP 419 status code (although it's not standard).

Is that always caused by the InvalidConfirmationTokenException?

@tobscure

This comment has been minimized.

Show comment
Hide comment
@tobscure

tobscure Sep 9, 2015

Member

Sounds good. InvalidConfirmationTokenException is unrelated to this – that's for email confirmation tokens.

This is low priority though :)

Member

tobscure commented Sep 9, 2015

Sounds good. InvalidConfirmationTokenException is unrelated to this – that's for email confirmation tokens.

This is low priority though :)

@kirkbushell

This comment has been minimized.

Show comment
Hide comment
@kirkbushell

kirkbushell Sep 28, 2015

Contributor

Just touching on this class now while writing some tests for the core codebase. Happy to investigate and resolve but need a little more input/discussion :)

Contributor

kirkbushell commented Sep 28, 2015

Just touching on this class now while writing some tests for the core codebase. Happy to investigate and resolve but need a little more input/discussion :)

@tobscure

This comment has been minimized.

Show comment
Hide comment
@tobscure

tobscure Sep 29, 2015

Member

Actually on second thoughts I don't think we should use 419 if it's not standard. Let's stick with 401, but add some error information to distinguish an expired token from a non-existent token.

So Api\Middleware\LoginWithHeader needs to be refactored:

  • If a token is present in the request, find it. (It may be an access_token, or it may be a master api_key... I'm not sure if these two should be distinguished in terms of request input?)
    • If it doesn't exist, throw a PermissionDeniedException? (Currently we just continue silently)
    • If it does exist but has expired, throw a TokenExpiredException. This should implement JsonApiSerializable with 401 status code and some more info... probably a code and a title?

We might also want to consider allowing the token to be passed in the query string, as we've found during beta that some Apache installations don't pass the Authorization header along to PHP.

I am happy to take care of the front-end implementation after this is done.

Member

tobscure commented Sep 29, 2015

Actually on second thoughts I don't think we should use 419 if it's not standard. Let's stick with 401, but add some error information to distinguish an expired token from a non-existent token.

So Api\Middleware\LoginWithHeader needs to be refactored:

  • If a token is present in the request, find it. (It may be an access_token, or it may be a master api_key... I'm not sure if these two should be distinguished in terms of request input?)
    • If it doesn't exist, throw a PermissionDeniedException? (Currently we just continue silently)
    • If it does exist but has expired, throw a TokenExpiredException. This should implement JsonApiSerializable with 401 status code and some more info... probably a code and a title?

We might also want to consider allowing the token to be passed in the query string, as we've found during beta that some Apache installations don't pass the Authorization header along to PHP.

I am happy to take care of the front-end implementation after this is done.

@kirkbushell

This comment has been minimized.

Show comment
Hide comment
@kirkbushell

kirkbushell Sep 29, 2015

Contributor

I'll do this sometime today - I've been working with the middleware layer (specifically the json api error management).

Contributor

kirkbushell commented Sep 29, 2015

I'll do this sometime today - I've been working with the middleware layer (specifically the json api error management).

@franzliedke franzliedke added this to the 0.1.x milestone Apr 7, 2016

@tobscure tobscure removed this from the 0.1.x milestone Jul 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment