davidjrh / dnn.azureadprovider

The DNN Azure Active Directory Provider is an Authentication provider for DNN Platform (formerly DotNetNuke) that uses Azure Active Directory OAuth2 authentication to authenticate users.
MIT License
35 stars 21 forks source link

Support UPN changes. #13

Closed swalker1595 closed 4 years ago

swalker1595 commented 6 years ago

[Breaking Change]

Right now when a user's UPN is changed in AAD, DNN throws a "User already exist" error because it attempts to register the user as the new user with their new UPN. This is problematic.

Using the AAD ObjectId claim as the UserId resolves this issue. ObjectId is guaranteed never to change in AAD.

"Contains a unique identifier of an object in Azure AD. This value is immutable and cannot be reassigned or reused. Use the object ID to identify an object in queries to Azure AD. " - https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-token-and-claims

This is a breaking change whereas users who have already registered will now have to re-register with their new Id, but same credentials. Their old account will have to be removed before. Alternatively, you may be able to get their ObjectId from AAD and make it match the UserId in the DNN Database and resolve the issue that way.

davidjrh commented 6 years ago

This sounds good to me, but don't like breaking changes :) I'm thinking on adding a private setting to specify which claim will be used for Ids. On this specific upgrade version scenario, the setting will default to JwtRegisteredClaimNames.UniqueName, while on later versions on first time installations will be "oid" (a migration tool will be needed to change current installations if desired).

I'm wondering if this has any collateral effect, and would like to test this on some sites before merging the pull request.

davidjrh commented 4 years ago

I have finally implemented the claim mappings on https://github.com/davidjrh/dnn.azureadprovider/tree/v4.0.0-preview with backwards compatibility.