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

Add option to automatically move users to a new authenticator #601

Merged

Conversation

newswangerd
Copy link
Member

This adds a new field on the authenticator model called auto_migrate_users_to which accepts a foreign key to another authenticator. When this field is configured we will do the following:

  • On login, collect any authenticator users from authenticators configured to migrate to the currently in use authenticator
  • For each of the discovered authenticators, call the move_authenticator_user_to method on the authenticator plugin. This lets the authenticator plugin know that the user is being moved and gives it a chance to perform any cleanup before that happens.

@newswangerd newswangerd marked this pull request as draft September 4, 2024 15:05
@newswangerd newswangerd force-pushed the feature/AAP-30012-merge-auth-users branch from 70af940 to 5697f2b Compare September 5, 2024 21:16
@newswangerd newswangerd marked this pull request as ready for review September 5, 2024 21:17
@newswangerd
Copy link
Member Author

Right now you can auto migrate users to any authenticator. I'm considering adding a field on the authenticator plugin where plugin authors can declare which authenticators are supported for auto migration. For example, we can declare that the generic oidc authenticator can be set to auto migrate users to keycloak, github, google etc (because these are all odic based authenticators), but can't be set to auto migrate users to saml.

Ultimately all of the authenticators that can be configured to use active directory as a backend (LDAP, OIDC, SAML, Keycloak) are potentially compatible with one another as long as they have the same user database as a backend.

@newswangerd
Copy link
Member Author

@elyezer I can't add you as a reviewer, but I'd like it if you could take a look at this

Copy link
Member

@john-westcott-iv john-westcott-iv left a comment

Choose a reason for hiding this comment

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

Can we squash the 2 migration files in the authentication app?

ansible_base/authentication/serializers/authenticator.py Outdated Show resolved Hide resolved
ansible_base/authentication/utils/authentication.py Outdated Show resolved Hide resolved
ansible_base/authentication/utils/authentication.py Outdated Show resolved Hide resolved
ansible_base/authentication/utils/authentication.py Outdated Show resolved Hide resolved
@newswangerd newswangerd force-pushed the feature/AAP-30012-merge-auth-users branch from fbd8e8b to 18b8a3f Compare September 10, 2024 14:57
Copy link
Member

@AlanCoding AlanCoding left a comment

Choose a reason for hiding this comment

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

I'm understanding high-level what this is doing and tests look fairly thorough.

@newswangerd newswangerd force-pushed the feature/AAP-30012-merge-auth-users branch from 6c52ba4 to 50bf6b3 Compare September 12, 2024 15:04
@newswangerd newswangerd force-pushed the feature/AAP-30012-merge-auth-users branch from 50bf6b3 to a487d39 Compare September 12, 2024 15:24
Copy link

sonarcloud bot commented Sep 12, 2024

@newswangerd newswangerd merged commit 2a85c91 into ansible:devel Sep 12, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants