brockallen / BrockAllen.MembershipReboot

MembershipReboot is a user identity management and authentication library.
Other
742 stars 238 forks source link

Creating external/linked users question #583

Closed CrescentFresh closed 8 years ago

CrescentFresh commented 8 years ago

It's unclear to me if UserAccountService.CreateAccount() requires that external user accounts be created with a valid email address.

If an external identity provider does not provide an email address upon login and I attempt to auto-create the user in MR, what should I pass in for the email parameter to UserAccountService.CreateAccount()?

Currently I am passing username as Guid.NewGuid().ToString() however I don't know what to do for Email.

I've been scratching my head with this one. I'm wondering if there shouldn't be a Create*() method tailored to Linked Accounts such that neither username nor email are considered but rather, that ProviderName and ProviderAccountID are validated to be non-empty and do not already represent a user in the repository.

brockallen commented 8 years ago

Hmm... well, if MR is configured to require an email and the external provider doesn't have one, then how would MR allow the account? I don't think that scenario should be allowed. OTOH, if the MR config says an email is not required, then pass null for email, as that seems allowed.

CrescentFresh commented 8 years ago

If MR is configured to require Email, that applies to local accounts only. It has to. Otherwise attempts to create a linked account with an email would trigger an email verification to be sent out, would it not?

My understanding is external accounts should not require verification.

brockallen commented 8 years ago

Well, email being verified is for password resets. If you only have an external account then you don't have a password.

CrescentFresh commented 8 years ago

I've investigated this further. MR's logic is as follows:

If asked to create an account and an email address is given, MR requests account verification (via verification key). Simple and expected.

I'm arguing that when configuring MR with RequireAccountVerification=false I very intentionally do not want this behavior.

In this case I'm configuring MR this way because I am creating a 3rd party external user locally, who happens to have an email address.

CrescentFresh commented 8 years ago

Our workaround looks similar to:

// Save off the email and tell MR email is null, then restore 
// the email property to its expected value after the save 
// (and hit the datastore a 2nd time to update this model).
var emailSaved = user.Email;

_mrConfiguredForExternalUsers
    .CreateAccount(TENANT, user.Username, null/*password*/,
        null/*email*/, // <-- HERE
        account: user);

user.Email = emailSaved;
_mrConfiguredForExternalUsers.Update(user);

This bypasses the verification key being set (and EmailAccountEventsHandler attempting to send a verification email)

brockallen commented 8 years ago

Really sorry for letting these issues get so old without attention.

So are you all set, or do you think there's something further in MR that you'd like fixed/changed?