Skip auth form if valid refresh token exists?

[madprime]

According to documentation on skipping the authorization form, auto can be used to skip the authorization form if an application has already been authorized with the same scope.

However as it's currently implemented, this only seems to work as long as an access token exists that is valid (default expiration: 10 hours) and has matching scope. But it doesn't check for a valid refresh token.

If a valid refresh token exists (and its corresponding access token has matching scopes), it seems the app has equivalent evidence of valid authorization from that user? And so one could (should?) safely skip the authorization form? Unless I'm missing something, this would seem to match the anticipated behavior as described in documentation.

If that all sounds correct, I think a change to oauth2_provider/views/base.py can make this happen, and I'll open a related PR. :)

[IvanAnishchuk]

Merged too quickly. Converting querysets to lists there is a bad idea.

[madprime]

I did expect more discussion – especially about the intended change in logic made on the code for auto-authorization, i.e. whether it is correct to change it in this way.

[IvanAnishchuk]

Yep. Let's continue the discussion then and figure out the correct way of doing it.

I think it is, generally speaking, a correct approach (it's common to skip authorization decision page if user has previously approved a request with the same scope for the same application, it also does make sense that refreshing a token or anything other than explicit user actions has side-effects on auto-authorization), but maybe we should look at the grants rather than/in addition to access/refresh tokens -- theoretically there can be access tokens issued for some other grant type that user didn't explicitly approve (not a common situation with DOT, applications support only a single grant type by default).

Existence of a Grant can confirm "user authorization decision" i.e. that user clicked the "authorize" button with the same scope and AccessToken or RefreshToken can confirm that user hasn't logged out (revoked access) since. I think we need to establish both facts to consider approval established "via other means" (4.2.1).

There is, however, a slight problem with refresh tokens in DOT: they are not expiring. Unless you revoke tokens manually or change the code or run the management command that clears them they are sitting in the db forever (there is a settings variable that sets expiration period but it's never used except for the management command). Which means many times the authorization page will never reappear no matter what you do which is probably not always ideal. We could also try to use Grant timestamp for excluding very old tokens. Or something, the point here is establishing that the decision made by user to authorize the app is still valid, it's not always time based and not always action-based, perhaps the logic could vary from situation to situation or even be customizable like most important things in these libraries.

In short, we need to fix the following here: 1) excessive fetching of stuff from db, 2) check for refresh token expiration if the corresponding settings variable is set 3) check grants and not only tokens (DOT doesn't currently link them though so the exact logic needs further discussion)

[IvanAnishchuk]

Oh, forgot to mention: grants are also deleted by default when access token is issued.

[madprime]

@IvanAnishchuk I created a new PR (#755) that tries to avoid the queryset-to-list, and also check for refresh token expiration. It doesn't try to check grants (I figured this didn't need to block on that?).

BUT this is work in progress – it's failing tests and I got stuck. Maybe you or others could help? I made a comment here on what seemed to be the issue: https://github.com/jazzband/django-oauth-toolkit/pull/755/files#diff-7cdc0c1a73bed47e5369f87bb98b38e6R195