Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authentication token renewal #3079

Closed
mssalvatore opened this issue Mar 9, 2023 · 3 comments
Closed

Authentication token renewal #3079

mssalvatore opened this issue Mar 9, 2023 · 3 comments
Labels
Impact: High Impact: Medium Spike A small chunk of work with the objective of gathering information.
Milestone

Comments

@mssalvatore
Copy link
Collaborator

mssalvatore commented Mar 9, 2023

Spike

Objective

  1. Users shouldn't be logged out automatically while they are actively using the Island.
  2. Agent's shouldn't suddenly be unable to contact the Island when their authentication tokens expire

Understand how we can achieve this with flask-security-too

Output

A new issue that describes the modifications that need to be made to the Island.

Time Remaining

0d - @VakarisZ

@mssalvatore mssalvatore added Impact: High Impact: Medium Spike A small chunk of work with the objective of gathering information. labels Mar 9, 2023
@VakarisZ
Copy link
Contributor

VakarisZ commented Mar 10, 2023

Token refresh options

Use sessions with cookies

The way flask-login handles token refresh is by returning a response with set cookies header. This enforces the client to set the new, refreshed session token into browser's cookies. This is not a good solution for us though, as we don't want to enforce clients to use the cookies for one because of CSRF and because it's custom code to extract tokens from cookies for non-browser users.

Create a custom endpoint to refresh the token.

There are a couple of ways this could be handled. The [token we are using](https://flask-login.readthedocs.io/en/latest/#alternative-tokens) ATM comes with a b64 encoded timestamp. This means that the client could decode a timestamp and refresh the token himself. However, this enforces all clients to run an asynchronous loop that would periodically check if the token is about to expire.

The better solution would be to provide a refresh token, that's identical to the authentication token, but is valid for X more time. This simplifies the refresh process for the client (if client gets 401, he tries to refresh the token and retries the request). 

Refresh the token on the server side

Even though token timestamps are signed and unchangeable, we can store the last access time of the user in the database.

Now the token is deemed expired if max_token_lifespan < current_time_timestamp - token_timestamp.

We could change this to max_token_lifespan < current_time_timestamp - token_timestamp AND max_token_lifespan < current_time_timestamp - last_access_timestamp

This would require 0 work on the client part, but the following work would be needed on the server side:

  1. Update last_access_timestamp on each token validation
  2. Change the validation to include the last access timestamp condition

@VakarisZ
Copy link
Contributor

I've opened an issue on flask-security-too to ask for guidance. If they seem thrilled by the idea of refreshing the tokens server side we should open up a request and implement that on the flask-security-too. If not, we should go with the route of a custom refresh endpoint.

@VakarisZ
Copy link
Contributor

Based on the discussion with the maintainers of flask-security-too it seems that the intended way to avoid stale tokens is to revoke them on the server side. That being said, if we don't trust our server-side code to do the revocation, there are two options to revoke stale tokens:

Generate our own refresh token:

We could generate a token that's exactly the same as the authentication token, but with an extended lifespan (we need to use a custom TimestampSigner that uses not the current time, but current time + REFRESH_TIMEDELTA as it's timestamp). The only difference between the authentication token and the refresh token would be that the refresh token has a longer lifespan. Then, we would add another endpoint.

Pros: Easier refresh procedure for the client. Does not rely on the flawed "refresh" behavior of the flask-security-too (there's risk that this refresh behavior will be changed)
Cons: We need to generate a token manually (complexity, manual work if flask-security-too changes their interface or the structure of the token)

Tasks:

  • Create a refresh token (1.25d)
  • Modify login and registration to return refresh token (0.5d)
  • Add logic to the agent to use the refresh token (0.5d)
  • Add logic to the UI to use the refresh token (0.5d)
  • Adjust the times for refresh and authentication token (0.25d)

Adjust the clients to refresh the token flask-security-too way.

We would change the Agent to periodically fetch a new token using the old one.

Pros: Cheaper
Cons: Clients have to know about the token lifespan (hard code this value, because it's not present in the token), this refreshing procedure seems flawed and risks being removed (that's my opinion at least).

Tasks:

  • Create a "refresh_token" method in HTTPIslandAPIClient that would refresh the token for the agent (0.5d)
  • Make sure that "refresh_token" is being called before each and every usage of self._http_client (0.5d)
  • Cache "refresh_token" for a period of time shorter than the token lifespan (0.5d)
  • (Optional) Repeat the same procedure for the UI
  • (Optional) Create an endpoint that would return the token lifespan. This way the client doesn't need to hard-code the refresh time

@mssalvatore mssalvatore added this to the v2.1.0 milestone Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: High Impact: Medium Spike A small chunk of work with the objective of gathering information.
Projects
None yet
Development

No branches or pull requests

2 participants