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

[CILogon] Rename allowed_idps to idps in a non-breaking way? #683

Open
consideRatio opened this issue Sep 20, 2023 · 0 comments · May be fixed by #685
Open

[CILogon] Rename allowed_idps to idps in a non-breaking way? #683

consideRatio opened this issue Sep 20, 2023 · 0 comments · May be fixed by #685

Comments

@consideRatio
Copy link
Member

consideRatio commented Sep 20, 2023

I think the name allowed_idps causes some confusion in oauthenticator v16, and that it should be non-breakingly renamed to just idps with a deprecation warning for users still using the allowed_idps name.

With a name like idps, there is no hint about users from a specific ipd would be allowed or simiarly. Configuring idps is about describing what users we can authenticate/recognize, while allowed refers to what users we authorize/allow. Since configuring an idp to be used to authenticate a user doesn't go hand in hand with authorizing a user, I think it should be renamed for clarity.

Together with the proposal in #682, the config for CILogon that is associated with authorizing users then become the following:

  • OAuthenticator.allow_all
  • OAuthenticator.allowed_users
  • OAuthenticator.admin_users
  • OAuthenticator.allow_existing_users
  • CILogonOAuthenticator.idps[<some idp>].allowed_domains
  • CILogonOAuthenticator.idps[<some idp>].allow_all

Proposal

Rename allowed_idps to idps, making allowed_idps still work as an alias for idps, but come with a deprecation warning.

@consideRatio consideRatio changed the title [CILogon] allowed_idps is now a confusing name [CILogon] Rename allowed_idps to idps in a non-breaking way? Sep 20, 2023
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 a pull request may close this issue.

1 participant