search

3rd-party-integrations / github-team-sync

Sync GitHub teams to groups in Active Directory, LDAP, Okta, OneLogin or AzureAD when using any authentication method for GitHub.
MIT License
199 stars 67 forks source link

Email address #11

Closed clgx-tform closed 3 years ago

clgx-tform commented 5 years ago

We would like to link email address from GitHub public profile to SMAL IDAP email attribute.

primetheus commented 4 years ago

will be tackling this shortly

timhirsh commented 4 years ago

Thanks @primetheus, this is important for my team as well. For orgs that have linked SAML Identities, I would expect this tool to be able to link my GitHub account to my AD account using the email address shown on https://github.com/orgs/[org-name]/people/[username]/sso even if my AD username doesn't match my GitHub username.

primetheus commented 4 years ago

@clgx-tform @timhirsh the ground work is laid for this one in #33. Initial testing worked, but we're looking at ways we can improve validation, particularly when users have private email addresses or the email isn't found in their account. The initial thought is that we can create an issue detailing the failure and listing the users, but I want to make sure that is a good approach. #33 introduces the ability to open issues on failure, and I'll be adding issue templates shortly as well. I may be able to have this feature kicked out by next week

primetheus commented 3 years ago

There is currently a USERNAME_ATTRIBUTE variable that can be set. This will allow for a custom username attribute to map to the GitHub username. Emails are returned, but since synchronizing via email depends on users marking their email as public, it would make this somewhat unreliable to implement.

Chocrates commented 3 years ago

Hey @primetheus,

If we were to implement this for LDAP as well is this the only line we would need to change, other than updating the env to get it?

https://github.com/github/github-team-sync/blob/ff290cca32aba6ca8fd64b53b4995c4329016f13/githubapp/ldap.py#L77

primetheus commented 3 years ago

@Chocrates no, that wouldn't be something we want to change...

https://github.com/github/github-team-sync/blob/main/githubapp/ldap.py#L80

☝🏾 already captures the email address for the user, and if we change that from uid to something else it will begin looking in LDAP for whatever attribute we set it to. For example, If we're using AD instead of OpenLDAP, the LDAP_USER_ATTRIBUTE will be sAMAccountName instead of uid.

What we want to update is the logic found in https://github.com/github/github-team-sync/blob/main/app.py#L61-L66

☝🏾 is hardcoded to look for username within the dictionary ({"username": "<value>", "email": "<value>"}), and we'll want to make that a new setting that is passed into the method