Skip to content
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

only restrict OAuth2 tokens for external accounts *at creation time* #5499

Conversation

ryanpetrello
Copy link
Contributor

@ryanpetrello ryanpetrello commented Dec 13, 2019

related: #5477

cc @gamuniz

@@ -146,5 +141,6 @@ def validate_external_users(self):
).format(external_account))

def save(self, *args, **kwargs):
self.validate_external_users()
Copy link
Contributor Author

@ryanpetrello ryanpetrello Dec 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only prevent OAuth2 tokens for external accounts at creation time. Changing this means people can:

  1. settings.ALLOW_OAUTH2_FOR_EXTERNAL_USERS = True
  2. Create an OAuth2 token for a certain LDAP-backed user.
  3. settings.ALLOW_OAUTH2_FOR_EXTERNAL_USERS = False
  4. At this point, LDAP users can no longer make new OAuth2 tokens
  5. The token from step 2 will continue to function (until it's revoked).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lines up with how we explain its use in the docs and honestly makes tokens more useful for auditing in larger orgs.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit da44046 into ansible:devel Jan 2, 2020
AlanCoding pushed a commit to AlanCoding/awx that referenced this pull request Feb 4, 2022
[4.1 Backport] Avoid duplicated entries when calling create_preload_data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants