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

AzureAD: use AD OID as username to ensure unicity #224

Conversation

thomas-rabiller-azimut
Copy link

@thomas-rabiller-azimut thomas-rabiller-azimut commented Nov 5, 2018

This PR requires admin review for compliance with project architecture and potential further testing

Background

This fix is a suggestion to solve issue #213 concerning Azure AD authentication.

Changed

  • Use OID from AD as JupyterHub username instead of name property as it is today because the name value is not immutable
  • Apply normalization when used with local authenticator to comply with unix username restrictions

@sdementen
Copy link

Hello, could it be possible to provide a function to extract the name from the decoded JWT ?
The oid is a uuid that is not human friendly when digging into the username/folders/etc created for jupyterhub.
In my case, I would rather prefer to use the 'unique_name' (userid@blable.com) and extract the userid, i.e. something like:
userdict = {"name": decoded['unique_name'].split("@")[0]}
So if we could provide a python expression in the config to extract the username, would be great!

@thomas-rabiller-azimut
Copy link
Author

I agree the oid isn't very human friendly when looking at the folder structure. However it looks like the unique_name property is not safe to use, according to AAD documentation:

unique_name: Only present in v1.0 tokens. Provides a human readable value that identifies the subject of the token. This value is not guaranteed to be unique within a tenant and should be used only for display purposes.

This has similar risks as the current name property:

name: Only present in v1.0 tokens. Provides a human readable value that identifies the subject of the token. This value is not guaranteed to be unique within a tenant and should be used only for display purposes.

@sdementen
Copy link

I would rather leave this choice to the user/configuration.
We just deployed a jupyterhub and as admin I see all users by their internal id (ie a part of the unique_name) which is not convenient either.
We will probably move to a mix of user friendly name + unique id.
Today, we are patching the azuread.py. would be great if tomorrow we could do this by configuration

@ghost
Copy link

ghost commented Jan 10, 2019

@thomas-rabiller-azimut, are you still working on this PR? I'd really like to see it merged.

@thomas-rabiller-azimut
Copy link
Author

@jfleury-eidos The PR is complete as it is as far as I am concerned (fix is as I expect and tests have passed) but I am not sure what needs to be done for the jupyter Hub team to review and integrate it in the official repo...

@ghost
Copy link

ghost commented Jan 11, 2019

@sdementen asked that the username value be configurable.

@dwaltrip
Copy link

dwaltrip commented Apr 15, 2019

Using the oid seems very suboptimal, as it becomes much more difficult for users to see who they are logged in as, as well as for admins trying to look at user info.

I understand the documented warnings around unique_name, but in practice how likely is it for the values to not be unique? Perhaps we could append a suffix such as "-1", "-2", etc, whenever a conflict is found?

Also, a nice configurable escape hatch might be an option such as azuread_username_key, which determines which key to use to set the username. One could set c.AzureAdOAuthenticator.azuread_username_key = 'oid' if they choose.

I'm glad this is being looked at! I'm currently having to patch oathenticator/azuread.py to fix this issue.

@timlod
Copy link

timlod commented Apr 24, 2019

We are facing the same issues.
I'm now patching to use unique_name, which in all of the current cases is equivalent to using email.

An interesting note to add to the discussion:
Azure offers the AADLoginForLinux extension for Azure Virtual Machines.
If you login using your AD credentials, at least in my case the system uses the email address prefix (firstname.lastname) as the username.
By using the email address in JupyterHub we get the advantage of being able to log into the same account using SSH AD login as well as JupyterHub Azure AD authentication.

The problem seems indeed to be that v2.0 tokens don't have an email field. The example says that preferred_name can be an email address, but it is mutable.

I agree that giving an option would be the best way to fix this.
If I can find time in the near future I may submit a PR.

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 this pull request may close these issues.

None yet

4 participants