Awx: "Cleanup Expired OAuth 2 Tokens" management job doesn't work for tokens issued with Authorization Code flow

[vrevelas]

ISSUE TYPE

• Bug Report

SUMMARY

2701 added a "Cleanup Expired OAuth 2 Tokens" management job which runs the awx-manage cleartokens command. The Django OAuth Toolkit docs for that command note

Refresh tokens need to expire before AccessTokens can be removed from the database. Using cleartokens without REFRESH_TOKEN_EXPIRE_SECONDS has limited effect.

AWX currently does not have a default value for REFRESH_TOKEN_EXPIRE_SECONDS nor does it provide UI to set it, so the token clearing job pretty much always does nothing.

ENVIRONMENT

• AWX version: 6.1.0
• AWX install method: docker on linux
• Ansible version: 2.8.4
• Operating System: Ubuntu 18.04
• Web Browser: Firefox 69.0

STEPS TO REPRODUCE

Go to Settings -> System and set a relatively low value for "ACCESS TOKEN EXPIRATION", e.g. 60.

Configure an Application to use the "Authorization Code" Grant type, and set "Client Type" to "Confidential".

Obtain a token using the steps in the docs, and wait for it to expire.

Run the "Cleanup Expired OAuth 2 Tokens" management job

EXPECTED RESULTS

The expired token should have been removed.

ACTUAL RESULTS

The expired token is not removed.

ADDITIONAL INFORMATION

I created /etc/tower/conf.d/oauth-settings.py with the following contents

OAUTH2_PROVIDER["REFRESH_TOKEN_EXPIRE_SECONDS"] = 60

and confirmed that running awx-manage cleartokens worked as expected. The presence of this setting breaks the entire settings UI though, so is not really a useable workaround.

My proposal is to add UI for the value of REFRESH_TOKEN_EXPIRE_SECONDS.

Adding this setting has additional benefits: When using AWX with an LDAP server and the "Allow external users to create OAuth2 tokens" setting is enabled, tokens allow users to continue to access AWX even if their LDAP account is disabled as detailed in this comment on #2191. Allowing AWX admins to specify an expiration time for refresh tokens would help mitigate that security risk by using relatively short-lived access and refresh tokens.

If there's interest in this I will try find the time to raise a PR.

[fosterseth]

replicated the issue. Setting REFRESH_TOKEN_EXPIRE_SECONDS allows proper functioning of awx-manage cleartokens and cleanup_tokens

related to issue #3825 and the same solution here fixes that one.

temporary fix by modifying OAUTH2_PROVIDER in awx/settings/defaults.py to add the REFRESH_TOKEN_EXPIRE_SECONDS key

e.g.

OAUTH2_PROVIDER = {'ACCESS_TOKEN_EXPIRE_SECONDS': 31536000000,
                   'AUTHORIZATION_CODE_EXPIRE_SECONDS': 600,
                   'REFRESH_TOKEN_EXPIRE_SECONDS': 60}

I did not experience any UI issues after setting this key/value

[ryanpetrello]

@fosterseth would it make sense for _us_ to update this with a default for REFRESH_TOKEN_EXPIRE_SECONDS?

https://github.com/ansible/awx/blob/devel/awx/settings/defaults.py#L340

[vrevelas]

Thanks for picking this up all. Given that the front end allows admin users to set access token expiration times, I think it would make sense to allow allow admins to control the refresh token expiration time as well. Also, a default value of 60 seconds is certainly too low.

As far as I'm aware, OAuth generally recommends using short-lived access tokens, and longer-lived refresh tokens. For example, Okta's oauth.com site states:

Short-lived access tokens and long-lived refresh tokens

A common method of granting tokens is to use a combination of access tokens and refresh tokens for maximum security and flexibility. The OAuth 2.0 spec recommends this option, and several of the larger implementations have gone with this approach.

So, to summarize, I'd suggest three tasks here:

One, change the default access token lifetime to something more sensible than the current 1000 years. I'd suggest something like an hour.

Two, set a sensible default value for REFRESH_TOKEN_EXPIRE_SECONDS greater than ACCESS_TOKEN_EXPIRE_SECONDS. Maybe something like one week?

Three, expose REFRESH_TOKEN_EXPIRE_SECONDS as a configurable setting in the UI next to the current setting for ACCESS_TOKEN_EXPIRE_SECONDS.

If this approach sounds reasonable, I'd be happy to make the time to open a PR.

[rooftopcellist]

@vrevelas I agree that we should make the REFRESH_TOKEN_EXPIRE_SECONDS configurable in the UI as well, but I do not think we should lower the default value for ACCESS_TOKEN_EXPIRE_SECONDS as there are use cases for long-lived tokens as well (specifically personal access tokens which don't have refresh tokens) and some people may already rely on the "long-lived" default setting.

By making both settings configurable in the UI, you should be able to configure your token expiration times as needed for your application.

In an ideal world we would probably have 2 separate default values for PAT's and App tokens, or just set Personal Access Tokens to not expire, as GitHub does. I am looking in how best we could implement that now.

---

Either way, at a minimum, we are going to need to set a default value for REFRESH_TOKEN_EXPIRE_SECONDS.

[vrevelas]

That's fair enough. However I'd be wary of adding a default value for REFRESH_TOKEN_EXPIRE_SECONDS with no UI, as any users that rely on the current behaviour (refresh tokens never expire) will have their use-case broken with no recourse.

[rooftopcellist]

Good point @vrevelas. @fosterseth, let's hold off on merging https://github.com/ansible/awx/pull/4886 until the UI work is also complete.

[ryanpetrello]

^ WFM 👍

[fosterseth]

@vrevelas the API and UI pull requests related to this fix have now both been merged, so you should see the change in the latest devel branch. Thanks!

https://github.com/ansible/awx/pull/4886
https://github.com/ansible/awx/pull/4890

[vrevelas]

Thanks all!

I did some testing this weekend on daa6f35d02b8d542d8a9e31b85ad29a7d0a5aca3 and unfortunately the functionality is not working as expected. I set the values of "Refresh Token Expiration" and "Access Token Expiration" both to low values (60 seconds), but 2 minutes after issuing a token the management job to remove the expired access token continues to do nothing. Changing the value of REFRESH_TOKEN_EXPIRE_SECONDS in /var/lib/awx/venv/awx/lib/python3.6/site-packages/awx/settings/defaults.py in the awx_task container to the same value as the UI works fine, so it looks like the setting in the UI doesn't make it to the task container where the cleartokens job is run.

While testing I also ended up getting thoroughly confused regarding the REFRESH_TOKEN_EXPIRE_SECONDS setting because I was able to continue using refresh tokens beyond ACCESS_TOKEN_EXPIRE_SECONDS + REFRESH_TOKEN_EXPIRE_SECONDS. It turns out that refresh tokens don't actually ever expire - they only stop being valid after being used, revoked, or deleted from the database by the cleartokens command. I dug up Better documentation for refresh tokens expiry in the DOT project and have added a comment.

So to summarise, two issues:

1. Is it possible to get the REFRESH_TOKEN_EXPIRE_SECONDS setting from the UI to get applied correctly to the cleartokens command in the task container?
2. Given that refresh tokens never expire, it might be worth making the help tooltip for the new "Refresh Token Expiration" setting more clear: The setting only determines which refresh (and hence expired access) tokens the management job will remove if or when it is run.

[ryanpetrello]

@rooftopcellist @fosterseth

This isn't the first time DOT has hard-coded static settings imports in a way that broke things for us 😞. Here's the _least_ gross way I can think of for resolving this:

diff --git a/awx/conf/apps.py b/awx/conf/apps.py
index 4f9a36395..ac4a5009f 100644
--- a/awx/conf/apps.py
+++ b/awx/conf/apps.py
@@ -9,7 +9,26 @@ class ConfConfig(AppConfig):
     name = 'awx.conf'
     verbose_name = _('Configuration')
​
+    def configure_oauth2_provider(self, settings):
+        from django.conf import settings
+        from oauth2_provider import models
+
+        class DynamicOAuth2ProviderSettings(models.oauth2_settings.__class__):
+
+            def __getattr__(self, attr):
+                if attr in settings.OAUTH2_PROVIDER:
+                    return settings.OAUTH2_PROVIDER[attr]
+                return super(DynamicOAuth2ProviderSettings, self).__getattr__(attr)
+
+        models.oauth2_settings = DynamicOAuth2ProviderSettings(
+            models.oauth2_settings.user_settings, models.oauth2_settings.defaults,
+            models.oauth2_settings.import_strings, models.oauth2_settings.mandatory
+        )
+
     def ready(self):
         self.module.autodiscover()
         from .settings import SettingsWrapper
         SettingsWrapper.initialize()
+        from django.conf import settings
+        self.configure_oauth2_provider(settings)

If we do add something like this, let’s add a big comment explaining why we’ve done this, w/ links to any oauth2 toolkit issues and the AWX issue so people who find this in the future have context

[kdelee]

When you do all do close this up, can you please provide some summary of
1) Problems + steps to reproduce
2) PRs made to implement changes
3) Any additional verification criteria

[rooftopcellist]

@kdelee

Token with incorrect expires

1. Set ACCESS_TOKEN_EXPIRE_SECONDS in the UI/API
2. Make an application with the password grant_type and confidential -- keep the client_id & client_secret
3. Use the client_id & client_secret to post to the api/o/token endpoint to create a token

# Create App Token with api/o/token
curl -X POST -d "grant_type=password&username=root&password=reverse&scope=write" \
    -u "client_id:client_secret" https://localhost:8043/api/o/token/ -i -k

1. Observe that the token returned does not have the expiration value you specified in the setting.

This issue (https://github.com/ansible/awx/issues/4710) is a side effect of this as well.

Cleartokens manage function Issue

The second effect this settings import issue causes is that Refresh Tokens and Access Tokens don't get cleared properly:

1. Set the ACCESS_TOKEN_EXPIRE_SECONDS value to 30 seconds in the API/UI
2. Set the REFRESH_TOKEN_EXPIRE_SECONDS setting to 30 seconds in the API/UI
3. Create an application token by any means
4. Wait at least 60 seconds, then run awx-manage cleartokens

You would expect the refresh token and access token to be deleted by this point, instead the refresh token is not deleted (because it is using the wrong REFRESH_TOKEN_EXPIRE_SECONDS value.

Additional Verification

If possible, we should also check that the following functionality is unaffected:
• revoking tokens
• refreshing tokens
• authorization token flow

[fosterseth]

There is a fix for this downstream, and the changes will make its way to awx devel in the near future. @vrevelas

[kdelee]

I have tested this and now we have it set up so we should catch regressions. Closing