feat: enable oidc authz code flow

[vmttn]

Description

#246 and #307

Technical details

• authz code flow
• mozilla_django_oidc for the client implementation

[tevariou]

Can you explain why storing the access token which is short lived ? And what about the refresh token ?

[vmttn]

I'm not sure if/why you think there is an issue.

The access token has an expiration time (expires_in field in a successful auth response). It can be an hour, it can be 12h. It's not a one shot token. Given the requests/response pattern of web frameworks (the framework will forget you in between requests) and the fact that we don't want to send the access token in the front (authz code flow), we need to store the access token in the session (i.e. in DB).

Maybe you're asking me : why even bother storing the access token ?

1. asking a new access token for each requests will add latency and resources consumption. The provider will become more of a single point of failure.
2. IIRC access tokens have an expiration time bc it's otherwise hard to revoke them. Applications that still need an even longer access can use a refresh token. The refresh token mechanism forces a token bearer to check in with the provider, so that the provider can deny them access. Do you think our apps need to store the token longer than 12h ?
3. access token are not one-shot tokens, according to the spec. It goes against the spirit of the spec.
4. maybe thats why mozilla-django-oidc doesnt support storing refresh token.

[tevariou]

1. I don't understand this. It's not what's happening with mozilla oidc if you don't store the access token. It won't try to get one at each request according to the code I've read. Or you can point me to where the access token is actually used after the session is created because I don't see it.
2. I agree with your point but I also think a refresh token can be useful in other ways (see my other comment)
3. Agreed
4. As they say in their readme, they keep it as minimal as possible.

[vmttn]

Regarding 1., you're also right. I was just saying that : not storing the access token implies that we'll need to make sure to fetch a new one each time a view requires access to a resource server. This has indeed nothing to do with mozilla-oidc. If many views need access to external resources, then we'd better take advantage of storing the access token and reuse it.

[tevariou]

Did you read the code ? From memory it performs a redirect or in our case (when using an SPA) returns a 403 along with a refresh url in the body. It's not silent. That's why I stored the refresh token and implemented a custom middleware based on this one. There's a couple of issues and PR related to this in the mozilla-django-oidc repo.

[vmttn]

Are you're suggesting there is a bug with SPAs ? Point me towards the issues you're referring to.

I did read the code.

It seems to me that what you're describing is not related to mozilla-django-oidc. Rather an issue with the provider implementation when receiving prompt=none. It either can be the expected outcome (the end-user do need to reauthenticate) or an issue with the provider implementation.

If you want to review this PR, you should also check arkhn/o-provider.

I did have an issue with django-oauth-toolkit, but was able to move past with the following monkeypatch (I can explain what a monkeypatch is if you don't know the pattern):
https://github.com/arkhn/o-provider/blob/master/django/oauth/__init__.py

[tevariou]

I'm sorry my comment wasn't very clear. The request to the provider works flawlessly. But the issue is when using an SPA like our React app. When we will perform a redirect, it means we need to store the client-side state before the redirect and restore the state afterwards.

For instance, if the user is filling a form. When submitting this form, he will receive a 403 (because he reached the 15 minutes you set in the settings) and then the app will redirect him to the redirect_url which will redirect him back to the app. To make it silent, we need to store the user state before the redirect in some ways in the app and rollback this state after the redirect. In our example, it means restoring the form inputs and submitting again. It's a bit cumbersome.

What we can do alternatively is using the refresh token. No redirect is needed in this case. That's what I implemented on my other PR.

[vmttn]

Okay ! This is the kind of insightful feedback I was expecting from you !

1. The SessionRefresh middleware is only used for GET requests (https://github.com/mozilla/mozilla-django-oidc/blob/HEAD/mozilla_django_oidc/middleware.py#L114). Therefore I don't think there should not be a redirection when submitting a form. No ?
2. Still. Maybe there is a dropdown on the form that makes an ajax request on click. We could fetch the values when the form is first rendered or use what's in the store.
3. 15 min refresh is a very strict setting. I think we can increase that value significantly, hence preventing the risk of 2 from happening.

[tevariou]

1. Totally forgot about that! you're right!
2. The form example is a bit off. Let's take another one. Take a component with some local state within it, It could have a modal which will fetch some data to display that open if we click on a button.. when the user comes back the state will be reinitialised. And we can't put everything in the redux store. And for persisting the state in the store, we would need something like redux-persist. Or we could use an hidden iframe!
3. Why not! But I may have missed something but it looks way more convenient to me to just use the refresh token. I looked a bit into it (not much though haha so I may be wrong) and the case where it's not recommended is for SPA without a backend since you can't store the refresh token safely in a browser.

[vmttn]

3. silent reauth is the go-to (in the spec and almost all resources I've crossed)
   a. mozilla infosec does not recommend using the refresh token at all (and they just implement the authz code flow).
   b. auth0 recommends using refresh token (with rotation) (ref), but they do so in the context of a SPA without a backend (ref). They previously recommended silent reauth but changed their recommendation because :

Recent developments in browser privacy technology, such as Intelligent Tracking Prevention (ITP) prevent access to the Auth0 session cookie, thereby requiring users to reauthenticate.
[...]
Refresh token rotation offers a remediation to end-user sessions being lost due to side-effects of browser privacy mechanisms. Because refresh token rotation does not rely on access to the Auth0 session cookie, it is not affected by ITP or similar mechanisms.

In short, Auth0 is a centralized session management system and ITP has forced them to recommend refresh token (with rotation) to their users.

c. Refresh token are dangerous because they never expire. Using them securely implies using rotation among other things (cf here and here). Without these, poorly implemented 3rd party clients could leak refresh token that never expire.

2. Do you have a concrete example where it could be a problem with pyrog ? I understand there is a world in which it could happen, but the way I see it :
   a. meaningful data, that should not be lost (e.g. user work), should be progressively (but coherently, as units of user work) saved with the backend. Otherwise it might be a UX problem.
   b. for other non meaningful data, we should not bother
   c. given 3. the chances of the issue could be low
   d. at worst, we have the quick fallback: https://mozilla-django-oidc.readthedocs.io/en/stable/xhr.html

To sum up 2., (severity of the problem occuring) * (probability of the problem occuring) is low and to me it does not justify to preemptively implement something.

Maybe you disagree. For now, let's just see how it goes. If it become truly an issue, I'd be happy to review your implementation.

[tevariou]

a. I didn't talk about losing any meaningful data. I'm just saying it won't be silent for the user if we have any local state in our components and that's not like it's not a common thing. For a concrete example, take the drawer in my last PR on Pyrog. If I don't use redux-persist (which stores the state in local storage), that will just close it without warning (there's three steps, each one potentially fetches something). And yes I think we should bother because it just breaks the UX.

c. is not true. You can set an expiry on refresh token as you do on access token.

[simonvadee]

I feel like this debate needs arbitration lol. I think both of you are more knowledgeable than me on this topic but the way I see it:

(severity of the problem occuring) * (probability of the problem occuring) is low

is true if the token expiration time is long enough. I think users will be bothered by this at some point, and this is when we'll think of something to patch it (like using refresh token for instance). Central authentication is already a hell of a mess, so let's take it step by step 😉.
@tevariou let's not take any action in the webapp to prevent this and see how problematic it becomes for users.

[vmttn]

@tevariou

• Silent reauth only means no prompt for credentials. It's not unvisible reauth ;)
• If the UX is your concern, we can add a loader with "reauth..." before redirecting. Or we could open the redirect_url in an iframe to preserve the app state or idk use a popup.

c. is not true. You can set an expiry on refresh token as you do on access token.

• Granted. I rephrase : the refresh token CAN expire, depending on the provider implementation and configuration. We won't always have control on this.