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

SAML enhancements #13316

Merged
merged 12 commits into from Jan 27, 2023
Merged

Conversation

john-westcott-iv
Copy link
Member

@john-westcott-iv john-westcott-iv commented Dec 8, 2022

SUMMARY

Split out SAML and Social Auth pipelines.
Pull common bits between SAML and LDAP auth into common file.

ISSUE TYPE
  • New or Enhanced Feature
COMPONENT NAME
  • API
AWX VERSION
awx: 21.9.1.dev65+g7ac513fa18.d20221208
ADDITIONAL INFORMATION

@john-westcott-iv john-westcott-iv force-pushed the saml-enhancements branch 2 times, most recently from a48768c to 864f6f7 Compare January 13, 2023 17:12
@john-westcott-iv john-westcott-iv changed the title [WIP] Saml enhancements SAML enhancements Jan 13, 2023
@john-westcott-iv john-westcott-iv self-assigned this Jan 16, 2023
@AlanCoding
Copy link
Member

@kdelee did you do some performance testing of this PR? Could you give like a really really brief summary? Because right now this just looks like a refactor.

@jay-steurer jay-steurer self-requested a review January 17, 2023 18:25
awx/sso/backends.py Outdated Show resolved Hide resolved
Copy link
Member

@kdelee kdelee left a comment

Choose a reason for hiding this comment

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

image

100 times faster in worst case
10 times faster in best case

SHIP IT!

awx/sso/common.py Outdated Show resolved Hide resolved
organization_name = organization_alias
else:
organization_name = org_name
org = get_or_create_with_default_galaxy_cred(name=organization_name)
Copy link
Member

Choose a reason for hiding this comment

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

huh, I'm surprised that we don't use defaults already, like, set the description of the org to "imported via SAML"

Copy link
Member Author

Choose a reason for hiding this comment

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

Its just a straight building of the org. This will be a slight change to how the LDAP adapter works because it didn't used to include adding the default galaxy credential at all.

Prefixing all internal functions with _
Modified subfunctions to not return values but instead manipulate multable objects
Modified all functions to not add duplicate orgs to the orgs_to_create list
This made testing easier and allows for any adapter with a flag the ability to simply pass it into a function
Removed orgs_to_create from being passed into user_team functions, common create orgs code will add any team orgs to list of orgs automatically

Passed SAML_AUTO_CREATE_OBJECTS flag into create_org_and_teams

Fix bug where we were looking at values instead of keys

Added loading of all teams if remove flag is set in update_user_teams_by_saml_attr
@jay-steurer jay-steurer merged commit 8fb831d into ansible:devel Jan 27, 2023
@john-westcott-iv
Copy link
Member Author

@tvo318 this PR has a minor change to the behavior of the LDAP adapter. It now shares common code with the SAML adapter so if an LDAP login creates an organization the default galaxy credential will be added to that organization.

TheRealHaoLiu pushed a commit to TheRealHaoLiu/awx that referenced this pull request Feb 8, 2023
* Moving reconcile_users_org_team_mappings into common library

* Renaming pipeline to social_pipeline

* Breaking out SAML and generic Social Auth

* Optimizing SMAL login process

* Moving extraction of org in teams from backends into sso/common.create_orgs_and_teams

* Altering saml_pipeline from testing

Prefixing all internal functions with _
Modified subfunctions to not return values but instead manipulate multable objects
Modified all functions to not add duplicate orgs to the orgs_to_create list

* Updating the common function to respect a teams organization name

* Added can_create flag to create_org_and_teams

This made testing easier and allows for any adapter with a flag the ability to simply pass it into a function

* Multiple changes to SAML pipeline

Removed orgs_to_create from being passed into user_team functions, common create orgs code will add any team orgs to list of orgs automatically

Passed SAML_AUTO_CREATE_OBJECTS flag into create_org_and_teams

Fix bug where we were looking at values instead of keys

Added loading of all teams if remove flag is set in update_user_teams_by_saml_attr

* Moving common items between SAML and Social into a 'base'

* Updating and adding testing

* Renamed get_or_create_with_default_galaxy_cred to get_or_create_org_...
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.

None yet

6 participants