Repository navigation
Conversation
Extends an OAuth session silently whenever session_lifetime lapses. While the access token received at login is still valid, the session is extended locally. Once it expires, a new access token is requested with the stored refresh token, and the user is only sent back to the login page if the provider refuses it. Adds oauth_tokens.provider (model and migration) so the server knows which provider to contact, and exempts OAuth backed sessions from the unconditional expired session cleanup, which would otherwise delete their refresh token on server startup or on another login.
Extends the OAuth login test to also check the persisted provider value, not just access_token.
Teaches the mock provider's /token endpoint to honor a grant_type=refresh_token request by issuing a new, distinct access token and rejecting unknown refresh tokens.
xb058t
added
database 🗄️
| "expires at %s.", token[:8], oauth_token.expires_at) | ||
| return True | ||
| except Exception as e: | ||
| LOG.warning("OAuth session extension failed for %s...: %s", |
There was a problem hiding this comment.
Does an expired refresh_token throw an exception via authlib?
If it does, then that needs to be handled explicitly, because it's not a warning-level issue.
There was a problem hiding this comment.
Yes, OAuthError. Handled it separately at info level, other errors are logged at error level now. Added unit tests.
| if new_token.get('expires_in') is not None: | ||
| oauth_token.expires_at = \ | ||
| now + timedelta(seconds=new_token['expires_in']) |
There was a problem hiding this comment.
authlib constructs an expires_at value on its own inside new_token, even if the server does not return that value.
There was a problem hiding this comment.
Changed to authlib's expires_at.
| return local_session | ||
|
|
||
| if self.__try_extend_oauth_session(token): | ||
| local_session.last_access = datetime.now() |
There was a problem hiding this comment.
This does not update the database, does it? Use local_session.revalidate(), or do the full revalidation inside try_extend_oauth_session.
There was a problem hiding this comment.
DB was updated in __try_extend_oauth_session, but revalidate() skips expired sessions. Moved the in-memory update there too.
| oauth_config['token_url'], | ||
| refresh_token=oauth_token.refresh_token) | ||
|
|
||
| if not new_token.get('access_token'): |
There was a problem hiding this comment.
Have you tested that there's a non-Exception code path here that does not contain an access_token? I feel like both this and checking for expires_in is needlessly paranoid.
There was a problem hiding this comment.
Not tested, you're right. Removed both checks.
8334e5f to
1e859bb
Compare
1e859bb to
1a37f76
Compare
3 participants
Closes #5047