verdan / flaskoidc

A wrapper of Flask with pre-configured OIDC support
Apache License 2.0
52 stars 35 forks source link

Infinite login/logout loop if `expires` of _earliest_ session is earlier than current time #28

Closed mosheduminer closed 2 years ago

mosheduminer commented 2 years ago

I'm facing an issue where after some time, visiting an endpoint results in an infinite loop of 302 redirects to login and logout. I believe the issue is as follows (if you think this doesn't make sense, please tell me):

Normally, the following happens:

  1. When a user first visits, a session is created for them in the database.
  2. When a user next visits, they are authenticated from the session in the database, and their token / session is refreshed as well.
  3. If the session has expired, the user is logged out, and redirected to login.

The problem is that a user may have multiple sessions in the table. The app uses .first() to get just one token from the table, but that means it will get the oldest token. So if a user's session has expired, then when they next visit they will be

  1. logged out (because the token has expired)
  2. logged in (as expected)
  3. logged out again (because the "first" / earliest token is still expired and wasn't deleted)
  4. they are logged back in again (and continue again from 3)

I don't know enough about the implementation to be sure as to the solution, but I imagine that either 1) you order the results (descending) by the expires_at column (.order_by(OAuth2Token.expires_at.desc())), or 2) make sure to delete the token if it has expired (I guess as an else block in _fetch_token).

If you prefer, I'm happy to submit a PR if you indicate which direction the solution should take.

airpaio commented 2 years ago

Confirming - I am having the same issue.

Getting an infinite loop of the errors below. I'm just kinda passing through here at the moment as I try to move away from flask-oidc and this flaskoidc package was working for a bit during my testing/evaluation until I came across this issue.

Unexpected Error
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/flaskoidc/__init__.py", line 214, in _fetch_token
  token_with_refresh_token = OAuth2Token.get_with_refresh_token(name=name, user_id=user_id).to_token()
AttributeError: 'NoneType' object has no attribute 'to_token'
User not logged in, redirecting to auth
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/flaskoidc/__init__.py", line 214, in _fetch_token
    token_with_refresh_token = OAuth2Token.get_with_refresh_token(name=name, user_id=user_id).to_token()
AttributeError: 'NoneType' object has no attribute 'to_token'
mosheduminer commented 2 years ago

@airpaio just for reference, my solution was to downgrade flaskoidc to the latest version below 1.1.0.

airpaio commented 2 years ago

@mosheduminer Thanks! Yes, I'm back in my app after downgrading to v1.0.6. I'll look back into this later, but I suppose with v1.0.6, once the token expires we just need to re-authenticate from Okta (or whatever provider) to "manually" have the token refreshed.

verdan commented 2 years ago

@ozandogrultan do you also experience this in your environment? Can you please help with this?

ozandogrultan commented 2 years ago

Hi, we are not facing the same issue but it might be due to Okta not returning refresh tokens after authentication. One possible fix I can think of is adding a guard check like this, so that you can re-authenticate as needed:

token_with_refresh_token = OAuth2Token.get_with_refresh_token(name=name, user_id=user_id)
if token_with_refresh_token is None:
    LOGGER.info("Refresh token could not be found, redirecting to login")
    raise LoginRequiredError
token_with_refresh_token_dict = token_with_refresh_token.to_token()
return self._update_token(name, token_with_refresh_token_dict, token_with_refresh_token_dict["refresh_token"],
                                          token_with_refresh_token_dict["access_token"])
ozandogrultan commented 2 years ago

@verdan Please review https://github.com/verdan/flaskoidc/pull/30 for the potential fix for this

verdan commented 2 years ago

Released a new version: https://pypi.org/project/flaskoidc/1.1.2/