search

zamzterz / Flask-pyoidc

Flask extension for using pyoidc as authentication for Flask apps.
Apache License 2.0
79 stars 38 forks source link

Issues refreshing tokens when 'session_refresh_interval_seconds' is set #67

Closed fredldotme closed 4 years ago

fredldotme commented 4 years ago

With the current state of flask_pyoidc we're running into the issue of not silently refreshing the tokens when it's desired.

Our current setup is as follows: API endpoints are protected using flask_pyoidc, with the session_refresh_interval_seconds providing prompt=none refreshing after 5 minutes.

The first issue we encountered was running into the silent refreshing when the initial authentication has not been successfully established yet. The second issue was the refreshing being triggered based on the previous auth_time, which didn't change with the request. Instead, what worked was taking the expiry time provided by the OIDC instance to trigger the silent refresh that way.

To illustrate a proposed way of fixing it we've created this change (this is not PR-quality code by any means): https://github.com/fredldotme/Flask-pyoidc/commit/8a062a1d9d281a80421c1e3adadd84c50ae12c7a

This forces the refresh after 5 minutes. Note that we've added 20 seconds of headroom before the expiry time runs out.

I'd like to get your suggestions on this topic and maybe a way forward in the form of code being merged into upstream.

zamzterz commented 4 years ago

Nice catch finding those issues and solutions for it! 🙇

  1. Your solution to the first issue looks good to me as-is. 👍
  2. Have you debugged the requests? I'm wondering why the auth_time in the session was not updated with the silent refresh request. Was there no ID Token returned in that case? Another thing: since the ID Token lifetime has no relation to the end-user session lifetime, it's better to base the refresh interval on the last authentication timestamp than the expiry time of the ID Token. That's why the refresh_interval_seconds is necessary (which looks to currently be ignored your proposed solution).
fredldotme commented 4 years ago

We're using Keycloak, do you think there are implementation-specific differences here? What I can tell is that the ID token is refreshed but the auth_time stays the same every time.

zamzterz commented 4 years ago

@fredldotme I've finally been able to look into this a bit more 🙂. I figured out the second issue as well: since no re-authentication happens, the newly issued ID Token will contain the same 'auth_time', hence no update of last_authenticated in the session.

I've made the changes in #70, are you able to test it and confirm it solves your issues?

fredldotme commented 4 years ago

This looks good. Doesn't regress and it uncovered a different issue on our side (CORS inhibiting Flasgger/SwaggerUI to run requests through on API endpoints when the tokens have to be refreshed).

zamzterz commented 4 years ago

Fix has been released in v3.2.0, many thanks again for your help. 🎉