[CILogon] Stripping domains only for case of single allowed domain?

[matthew-brett]

This is just asking for the reasoning behind the logic at https://github.com/jupyterhub/oauthenticator/blob/master/oauthenticator/cilogon.py#L206.

In the CILogon authenticator, you can specify allowed identity provider suffixes in allowed_idps - in my case these will be ['bham.ac.uk', 'student.bham.ac.uk'].

You can also specify strip_idp_domain, which will strip the username from (e.g. in my case) m.brett@bham.ac.uk to just m.brett. This is what I want.

But the logic specifies that I can only strip_idp_domain if I have exactly one entry in allowed_idps. Why is that a requirement? It's a problem in my case, because I must allow two suffixes to allow for staff and students, but I want the bare username e.g. "m.brett" rather than "m.brett@bham.ac.uk".

Maybe strip_idp_domain could also also allow a list of domains that should be stripped?

[welcome]

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.

You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[manics]

See the discussion on the original PR #145

The idea of having multiple entries in self.idp_whitelist makes me nervous, because of the possibility of username collisions across multiple domains. You don't want jbasney@wisc.edu logging in to jbasney@illinois.edu's account. It seems safer to follow the example from google.py to allow only a single self.hosted_domain parameter when you have a single trusted domain.

[matthew-brett]

Ah - thanks - I knew there was a good reason, it's just I hadn't seen it.

But how about my suggestion of allowing a list for strip_idp_domain to cover my use-case?

[minrk]

I think it only makes sense in aliases for the 'same' domains, so maybe we should handle this in the normalize_username step? or a specific dedicated 'aliases' config for mapping multiple idps known to share a user namespace to a single key?

class MyAuthenticator(CILogonOAuthenticator):
    def normalize_username(self, username):
        email = username.lower()
        just_name, _, domain = email.partition("@")
        if domain in {'bham.ac.uk', 'student.bham.ac.uk'}:
            return just_name
        else:
            return email

I've no particular objection to adding an opt-in list of domains to be stripped, with a warning about the possibility of collisions if the length is greater than 1. It's explicit and solves a known problem.

[robkooper]

We actually ran into this as well, i was wondering if maybe we can add a section called aliases to username_derivation. This would require me as the person setting up the authenticator to define the aliases. In the above case it would be:

                "acukidp": {
                    "username_derivation": {
                        "username_claim": "email",
                        "action": "strip_idp_domain",
                        "domain": "bham.ac.uk",
                        "aliases": {
                            "bham.ac.uk": [ "student.bham.ac.uk", "staff.bham.ac.uk"]
                        }
                    },
                    "allow_all": True,
                }

this would allow me to specify that "bham.ac.uk" can be spelled multiple ways.