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

OAuth2 ClientCredential grant custom expiration not being read from Flask configuration. #43

Closed
jperras opened this Issue Apr 20, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@jperras

jperras commented Apr 20, 2018

Summary

When registering a client credential grant for the authlib.flask.oauth2.AuthorizationServer in a Flask application and attempting to set a custom expiration time by setting OAUTH2_EXPIRES_CLIENT_CREDENTIAL as specified in the docs:

https://github.com/lepture/authlib/blob/23ea76a4d9099581cd1cb43e0a8a9a49a9328361/docs/flask/oauth2.rst#define-server

the specified custom expiration time is not being read.

Investigation

At first I believe that there was simply an error in the docs, in that the configuration value key should be pluralized, e.g. OAUTH2_EXPIRES_CLIENT_CREDENTIALS. However, upon a deeper look, it seems as though the default credential grant expiration time set in the authlib code base was not being used at all; the value being returned was 3600 instead of 864000 as specified in the mapping:

GRANT_TYPES_EXPIRES = {
'authorization_code': 864000,
'implicit': 3600,
'password': 864000,
'client_credential': 864000
}

After a bit more digging, it seems that the create_expires_generator is returning the default BearerToken.DEFAULT_EXPIRES_IN value because the calculated conf_key = 'OAUTH2_EXPIRES_{}'.format(grant_type.upper()) only produces client_credentials instead of the expected client_credential:

def create_expires_generator(self, app):
"""Create a generator function for generating ``expires_in`` value.
Developers can re-implement this method with a subclass if other means
required. The default expires_in value is defined by ``grant_type``,
different ``grant_type`` has different value. It can be configured
with: ``OAUTH2_EXPIRES_{{grant_type|upper}}``.
"""
def expires_in(client, grant_type):
conf_key = 'OAUTH2_EXPIRES_{}'.format(grant_type.upper())
return app.config.get(conf_key, BearerToken.DEFAULT_EXPIRES_IN)
return expires_in

Notes

A very subtle bug that took me a while to track down; I attempted to create a test case, but it's not very obvious as to where the test case should exist since tests/flask/test_oauth2/test_client_credentials_grant.py is composed of functional tests as opposed to integration/unit tests.

If this is at all not clear, please let me know and I'll attempt to provide additional information.

And thank you for all your hard work! Authlib is fantastic, and has been a pleasure to use even in it's not-completely-done state.

@jperras

This comment has been minimized.

jperras commented Apr 20, 2018

The current workaround is pretty simple: I simply set the pluralized config valueOAUTH2_EXPIRES_CLIENT_CREDENTIALS, which does get picked up and used for the expiration time of the client credential grant.

The issue, as described above, is that the currently specified default in the code base is not being used, and if you change the default bearer token expiration time then the default client credential grant expiration time will pick up the same value as a result.

@lepture

This comment has been minimized.

Owner

lepture commented Apr 20, 2018

Thanks for your report. I believe it is a typo, the grant_type should be client_credentials, not client_credential. I will fix it asap.

@jperras

This comment has been minimized.

jperras commented Apr 20, 2018

If the grant type should be client_credentials, might I suggest also changing the documentation to reflect that the config variable name should be OAUTH2_EXPIRES_CLIENT_CREDENTIALS to match as well?

lepture added a commit that referenced this issue Apr 20, 2018

@lepture

This comment has been minimized.

Owner

lepture commented Apr 20, 2018

Fixed.

@lepture lepture closed this Apr 20, 2018

@jperras

This comment has been minimized.

jperras commented Apr 20, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment