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

Expired tokens do not get redirected to logout #39

Closed
jhopkins219 opened this issue Sep 25, 2023 · 3 comments · Fixed by #46
Closed

Expired tokens do not get redirected to logout #39

jhopkins219 opened this issue Sep 25, 2023 · 3 comments · Fixed by #46
Assignees

Comments

@jhopkins219
Copy link

Hello, I think we have identified a bug here in the latest version of flask-oidc. Several engineers noticed they were not presented with the login screen in our application after we upgraded to 2.0.3 and well-beyond their token expire time. After running it down I think there is a bug in this commit - https://github.com/fedora-infra/flask-oidc/pull/23/files#diff-ba9e98878e9cf879d9dc4f97d637849d516a4ddaf4d27c3409a2b05250ba992bR205

It attempts to refresh the token, expecting an exception to be thrown if it's no longer valid. However, the library will instead return None if there is no token to refresh - https://github.com/lepture/authlib/blob/a6e89f8e6cf6f6bebd63dcdc2665b7d22cf0fde3/authlib/oauth2/client.py#L260

I validated this by hooking into the authlib library to pull my token, editing the expire time to be in the past, and then passing it to this library's ensure_active_token call.

...
oidc = OpenIDConnect(app, prefix="/oidc")
token = OAuth2Token.from_dict(session.get("oidc_auth_token"))
token["expires_at"] = 1695673137
print(f"token expired? {token.is_expired()}")
print(f"token active? {oidc.ensure_active_token(token)}")
token expired? True
token active? None

This in turn makes the check_token_expiry call in the before_request handler not return a redirect, and allows users through because their old (expired) session exists, passing the require_login decorator.

The impact of this is being able to access anything behind the require_login decorator far past the token expire time, up until the cookie gets cleared in the browser

@abompard
Copy link
Member

abompard commented Oct 9, 2023

Thanks for reporting! I'll have a look.

@abompard abompard self-assigned this Oct 9, 2023
abompard added a commit to abompard/flask-oidc that referenced this issue Oct 9, 2023
…n URL

Fixes: fedora-infra#39
Signed-off-by: Aurélien Bompard <aurelien@bompard.org>
@abompard
Copy link
Member

abompard commented Oct 9, 2023

Thanks for the investigation, it made fixing much easier. I'll release a new version with the fix today.

abompard added a commit that referenced this issue Oct 9, 2023
…n URL

Fixes: #39
Signed-off-by: Aurélien Bompard <aurelien@bompard.org>
@github-actions
Copy link

github-actions bot commented Oct 9, 2023

Included in release Version 2.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants