cilogon: rename allowed_idps to idps, deprecating old name

docs/source/tutorials/provider-specific-setup/providers/cilogon.md

@@ -21,4 +21,4 @@ c.OAuthenticator.client_secret = "[your oauth2 application secret]"
 CILogonOAuthenticator expands OAuthenticator with the following required config,
 read more about it in the configuration reference:
 
-- {attr}`.CILogonOAuthenticator.allowed_idps`
+- {attr}`.CILogonOAuthenticator.idps`

oauthenticator/cilogon.py

@@ -21,7 +21,7 @@ class CILogonLoginHandler(OAuthLoginHandler):
     def authorize_redirect(self, *args, **kwargs):
         """
         Optionally add "skin" to redirect params, and always add "selected_idp"
-        (aka. "idphint") based on allowed_idps config.
+        (aka. "idphint") based on idps config.
 
         Related documentation at https://www.cilogon.org/oidc#h.p_IWGvXH0okDI_.
         """
@@ -30,8 +30,8 @@ def authorize_redirect(self, *args, **kwargs):
         extra_params = kwargs.setdefault('extra_params', {})
 
         # selected_idp should be a comma separated string
-        allowed_idps = ",".join(self.authenticator.allowed_idps.keys())
-        extra_params["selected_idp"] = allowed_idps
+        idps = ",".join(self.authenticator.idps.keys())
+        extra_params["selected_idp"] = idps
 
         if self.authenticator.skin:
             extra_params["skin"] = self.authenticator.skin
@@ -104,7 +104,7 @@ def _validate_scope(self, proposal):
 
         return scopes
 
-    allowed_idps = Dict(
+    idps = Dict(
         config=True,
         help="""
         A dictionary of the only entity IDs that will be allowed to be used as
@@ -116,7 +116,7 @@ def _validate_scope(self, proposal):
 
         For example::
 
-            c.CILogonOAuthenticator.allowed_idps = {
+            c.CILogonOAuthenticator.idps = {
                 "https://idpz.utorauth.utoronto.ca/shibboleth": {
                     "username_derivation": {
                         "username_claim": "email",
@@ -148,7 +148,7 @@ def _validate_scope(self, proposal):
             c.Authenticator.allowed_users = ["github-user2"]
 
         This is a description of the configuration you can pass to
-        `allowed_idps`.
+        `idps`.
 
         * `username_derivation`: string (required)
             * `username_claim`: string (required)
@@ -180,12 +180,12 @@ def _validate_scope(self, proposal):
         """,
     )
 
-    @validate("allowed_idps")
-    def _validate_allowed_idps(self, proposal):
+    @validate("idps")
+    def _validate_idps(self, proposal):
         idps = proposal.value
 
         if not idps:
-            raise ValueError("One or more allowed_idps must be configured")
+            raise ValueError("One or more idps must be configured")
 
         for entity_id, idp_config in idps.items():
             # Validate `idp_config` config using the schema
@@ -196,7 +196,7 @@ def _validate_allowed_idps(self, proposal):
                 # Raises useful exception if validation fails
                 jsonschema.validate(idp_config, schema)
 
-            # Make sure allowed_idps contains EntityIDs and not domain names.
+            # Make sure idps contains EntityIDs and not domain names.
             accepted_entity_id_scheme = ["urn", "https", "http"]
             entity_id_scheme = urlparse(entity_id).scheme
             if entity_id_scheme not in accepted_entity_id_scheme:
@@ -205,7 +205,7 @@ def _validate_allowed_idps(self, proposal):
                     f"Trying to allow an auth provider: {entity_id}, that doesn't look like a valid CILogon EntityID.",
                 )
                 raise ValueError(
-                    "The keys of `allowed_idps` **must** be CILogon permitted EntityIDs. "
+                    "The keys of `idps` **must** be CILogon permitted EntityIDs. "
                     "See https://cilogon.org/idplist for the list of EntityIDs of each IDP."
                 )
 
@@ -223,83 +223,92 @@ def _validate_allowed_idps(self, proposal):
 
     # _deprecated_oauth_aliases is used by deprecation logic in OAuthenticator
     _deprecated_oauth_aliases = {
-        "idp_whitelist": ("allowed_idps", "0.12.0", False),
+        "idp_whitelist": ("idps", "0.12.0", False),
         "idp": ("shown_idps", "15.0.0", False),
-        "strip_idp_domain": ("allowed_idps", "15.0.0", False),
-        "shown_idps": ("allowed_idps", "16.0.0", False),
-        "additional_username_claims": ("allowed_idps", "16.0.0", False),
-        "username_claim": ("allowed_idps", "16.0.0", False),
+        "strip_idp_domain": ("idps", "15.0.0", False),
+        "shown_idps": ("idps", "16.0.0", False),
+        "additional_username_claims": ("idps", "16.0.0", False),
+        "username_claim": ("idps", "16.0.0", False),
+        "allowed_idps": ("idps", "16.1.0", True),
         **OAuthenticator._deprecated_oauth_aliases,
     }
     idp_whitelist = List(
         config=True,
         help="""
         .. versionremoved:: 0.12
 
-           Use :attr:`allowed_idps`.
+           Use :attr:`idps`.
         """,
     )
     idp = Unicode(
         config=True,
         help="""
         .. versionremoved:: 15.0
 
-           Use :attr:`allowed_idps`.
+           Use :attr:`idps`.
         """,
     )
     strip_idp_domain = Bool(
         config=True,
         help="""
         .. versionremoved:: 15.0
 
-           Use :attr:`allowed_idps`.
+           Use :attr:`idps`.
         """,
     )
     shown_idps = List(
         config=True,
         help="""
         .. versionremoved:: 16.0
 
-           Use :attr:`allowed_idps`.
+           Use :attr:`idps`.
         """,
     )
     additional_username_claims = List(
         config=True,
         help="""
         .. versionremoved:: 16.0
 
-           Use :attr:`allowed_idps`.
+           Use :attr:`idps`.
         """,
     )
     username_claim = Unicode(
         config=True,
         help="""
         .. versionremoved:: 16.0
 
-           Use :attr:`allowed_idps`.
+           Use :attr:`idps`.
+        """,
+    )
+    allowed_idps = Dict(
+        config=True,
+        help="""
+        .. versiondeprecated:: 16.1
+
+           Use :attr:`idps`.
         """,
     )
 
     def user_info_to_username(self, user_info):
         """
         Overrides OAuthenticator.user_info_to_username that relies on
         username_claim to instead consider idp specific config in under
-        allowed_idps[user_info["idp"]]["username_derivation"].
+        idps[user_info["idp"]]["username_derivation"].
 
-        Returns a username based on user_info and configuration in allowed_idps
+        Returns a username based on user_info and configuration in idps
         under the associated idp's username_derivation config.
         """
         # NOTE: The first time we have received user_info is when
         #       user_info_to_username is called by OAuthenticator.authenticate,
         #       so we make a check here that the "idp" claim is received and
-        #       that we allowed_idps is configured to handle that idp.
+        #       that we idps is configured to handle that idp.
         #
         user_idp = user_info.get("idp")
         if not user_idp:
             message = "'idp' claim was not part of the response to the userdata_url"
             self.log.error(message)
             raise web.HTTPError(500, message)
-        if not self.allowed_idps.get(user_idp):
+        if not self.idps.get(user_idp):
             message = f"Login with identity provider {user_idp} is not pre-configured"
             self.log.error(message)
             raise web.HTTPError(403, message)
@@ -315,7 +324,7 @@ def _user_info_to_unprocessed_username(self, user_info):
         specified under "username_derivation" for the associated idp.
         """
         user_idp = user_info["idp"]
-        username_derivation = self.allowed_idps[user_idp]["username_derivation"]
+        username_derivation = self.idps[user_idp]["username_derivation"]
         username_claim = username_derivation["username_claim"]
 
         username = user_info.get(username_claim)
@@ -332,7 +341,7 @@ def _get_processed_username(self, username, user_info):
         specified under "username_derivation" for the associated idp.
         """
         user_idp = user_info["idp"]
-        username_derivation = self.allowed_idps[user_idp]["username_derivation"]
+        username_derivation = self.idps[user_idp]["username_derivation"]
 
         # Optionally execute action "strip_idp_domain" or "prefix"
         action = username_derivation.get("action")
@@ -350,19 +359,19 @@ async def check_allowed(self, username, auth_model):
         """
         Overrides the OAuthenticator.check_allowed to also allow users based on
         idp specific config `allow_all` and `allowed_domains` as configured
-        under `allowed_idps`.
+        under `idps`.
         """
         if await super().check_allowed(username, auth_model):
             return True
 
         user_info = auth_model["auth_state"][self.user_auth_state_key]
         user_idp = user_info["idp"]
 
-        idp_allow_all = self.allowed_idps[user_idp].get("allow_all")
+        idp_allow_all = self.idps[user_idp].get("allow_all")
         if idp_allow_all:
             return True
 
-        idp_allowed_domains = self.allowed_idps[user_idp].get("allowed_domains")
+        idp_allowed_domains = self.idps[user_idp].get("allowed_domains")
         if idp_allowed_domains:
             unprocessed_username = self._user_info_to_unprocessed_username(user_info)
             user_domain = unprocessed_username.split("@", 1)[1].lower()

oauthenticator/schemas/cilogon-schema.yaml

@@ -1,5 +1,5 @@
 # This JSONSchema is used to validate the values in the
-# CILogonOAuthenticator.allowed_idps dictionary.
+# CILogonOAuthenticator.idps dictionary.
 #
 $schema: http://json-schema.org/draft-07/schema#
 type: object

oauthenticator/tests/test_cilogon.py

@@ -364,7 +364,7 @@ async def test_cilogon(
         ),
     ],
 )
-async def test_cilogon_allowed_idps(
+async def test_cilogon_idps(
     cilogon_client,
     test_variation_id,
     idp_config,
@@ -409,7 +409,7 @@ async def test_cilogon_allowed_idps(
             {"idp_whitelist": ["dummy"]},
             {},
             logging.ERROR,
-            "CILogonOAuthenticator.idp_whitelist is deprecated in CILogonOAuthenticator 0.12.0, use CILogonOAuthenticator.allowed_idps instead",
+            "CILogonOAuthenticator.idp_whitelist is deprecated in CILogonOAuthenticator 0.12.0, use CILogonOAuthenticator.idps instead",
         ),
         (
             "idp",
@@ -423,28 +423,47 @@ async def test_cilogon_allowed_idps(
             {"strip_idp_domain": True},
             {},
             logging.ERROR,
-            "CILogonOAuthenticator.strip_idp_domain is deprecated in CILogonOAuthenticator 15.0.0, use CILogonOAuthenticator.allowed_idps instead",
+            "CILogonOAuthenticator.strip_idp_domain is deprecated in CILogonOAuthenticator 15.0.0, use CILogonOAuthenticator.idps instead",
         ),
         (
             "shown_idps",
             {"shown_idps": ["dummy"]},
             {},
             logging.ERROR,
-            "CILogonOAuthenticator.shown_idps is deprecated in CILogonOAuthenticator 16.0.0, use CILogonOAuthenticator.allowed_idps instead",
+            "CILogonOAuthenticator.shown_idps is deprecated in CILogonOAuthenticator 16.0.0, use CILogonOAuthenticator.idps instead",
         ),
         (
             "username_claim",
             {"username_claim": "dummy"},
             {},
             logging.ERROR,
-            "CILogonOAuthenticator.username_claim is deprecated in CILogonOAuthenticator 16.0.0, use CILogonOAuthenticator.allowed_idps instead",
+            "CILogonOAuthenticator.username_claim is deprecated in CILogonOAuthenticator 16.0.0, use CILogonOAuthenticator.idps instead",
         ),
         (
             "additional_username_claims",
             {"additional_username_claims": ["dummy"]},
             {},
             logging.ERROR,
-            "CILogonOAuthenticator.additional_username_claims is deprecated in CILogonOAuthenticator 16.0.0, use CILogonOAuthenticator.allowed_idps instead",
+            "CILogonOAuthenticator.additional_username_claims is deprecated in CILogonOAuthenticator 16.0.0, use CILogonOAuthenticator.idps instead",
+        ),
+        (
+            "allowed_idps",
+            {
+                "allowed_idps": {
+                    "https://github.com/login/oauth/authorize": {
+                        "username_derivation": {"username_claim": "email"}
+                    }
+                }
+            },
+            {
+                "idps": {
+                    "https://github.com/login/oauth/authorize": {
+                        "username_derivation": {"username_claim": "email"}
+                    }
+                }
+            },
+            logging.WARNING,
+            "CILogonOAuthenticator.allowed_idps is deprecated in CILogonOAuthenticator 16.1.0, use CILogonOAuthenticator.idps instead",
         ),
     ],
 )
@@ -480,7 +499,7 @@ async def test_deprecated_config(
     assert expected_log_tuple in captured_log_tuples
 
 
-async def test_config_allowed_idps_wrong_type(caplog):
+async def test_config_idps_wrong_type(caplog):
     """
     Test alllowed_idps is a dict
     """
@@ -491,7 +510,7 @@ async def test_config_allowed_idps_wrong_type(caplog):
         CILogonOAuthenticator(config=c)
 
 
-async def test_config_allowed_idps_required_username_derivation(caplog):
+async def test_config_idps_required_username_derivation(caplog):
     # Test username_derivation is a required field of allowed_idps
     c = Config()
     c.CILogonOAuthenticator.allowed_idps = {
@@ -502,7 +521,7 @@ async def test_config_allowed_idps_required_username_derivation(caplog):
         CILogonOAuthenticator(config=c)
 
 
-async def test_config_allowed_idps_invalid_entity_id(caplog):
+async def test_config_idps_invalid_entity_id(caplog):
     """
     Test allowed_idps keys cannot be domains, but only valid CILogon entity ids,
     i.e. only fully formed URLs
@@ -531,7 +550,7 @@ async def test_config_allowed_idps_invalid_entity_id(caplog):
     assert expected_deprecation_error in log_msgs
 
 
-async def test_config_allowed_idps_invalid_type(caplog):
+async def test_config_idps_invalid_type(caplog):
     c = Config()
     c.CILogonOAuthenticator.allowed_idps = {
         'https://github.com/login/oauth/authorize': 'should-be-a-dict'
@@ -540,7 +559,7 @@ async def test_config_allowed_idps_invalid_type(caplog):
         CILogonOAuthenticator(config=c)
 
 
-async def test_config_allowed_idps_unrecognized_options(caplog):
+async def test_config_idps_unrecognized_options(caplog):
     c = Config()
     c.CILogonOAuthenticator.allowed_idps = {
         'https://github.com/login/oauth/authorize': {
@@ -551,7 +570,7 @@ async def test_config_allowed_idps_unrecognized_options(caplog):
         CILogonOAuthenticator(config=c)
 
 
-async def test_config_allowed_idps_domain_required(caplog):
+async def test_config_idps_domain_required(caplog):
     c = Config()
     c.CILogonOAuthenticator.allowed_idps = {
         'https://github.com/login/oauth/authorize': {
@@ -565,7 +584,7 @@ async def test_config_allowed_idps_domain_required(caplog):
         CILogonOAuthenticator(config=c)
 
 
-async def test_config_allowed_idps_prefix_required(caplog):
+async def test_config_idps_prefix_required(caplog):
     c = Config()
     c.CILogonOAuthenticator.allowed_idps = {
         'https://github.com/login/oauth/authorize': {