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

oidc_refresh_access_token_before_expiry? #1111

Closed
zandbelt opened this issue Sep 14, 2023 Discussed in #1109 · 3 comments
Closed

oidc_refresh_access_token_before_expiry? #1111

zandbelt opened this issue Sep 14, 2023 Discussed in #1109 · 3 comments

Comments

@zandbelt
Copy link
Member

Discussed in #1109

Originally posted by brandonk10 September 14, 2023
I'm working off current master, and I'm struggling with this function. The way I understand the logic, it's supposed to refresh the token if it's expired, and returns FALSE if there's an error trying to do that - you're either logged out or re-authenticated if that happens.

My problem seems to be here:
if (t_expires > apr_time_now()) return FALSE;

From what I can tell, this is simply trying to skip the refresh if the token hasn't expired(within TTL). In this case, shouldn't TRUE be returned? I might be doing something else wrong, but I get logged out immediately with this logic, and if I flip to TRUE, my logins start falling through to the application again.

This occurs when "logout_on_error" is included in OIDCRefreshAccessTokenBeforeExpiry.

@zandbelt
Copy link
Member Author

@brandonk10 I believe you're right and I think the fix is that needs_save needs to be passed in to achieve the desired logic

zandbelt added a commit that referenced this issue Sep 14, 2023
see #1111; thanks @brandonk10; bump to 2.4.14.4rc0

Signed-off-by: Hans Zandbelt <[email protected]>
@zandbelt
Copy link
Member Author

zandbelt commented Sep 14, 2023

we've added end to end tests in our CI/CD environment to confirm that this issue is fixed in 1cf0a98

@brandonk10
Copy link

Thanks for the confirmation and the quick commit! I've done some preliminary testing and things are looking good so far.

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

No branches or pull requests

2 participants