brockallen / BrockAllen.MembershipReboot

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

Allow temporary access pre-email verification #611

Closed vinneyk closed 8 years ago

vinneyk commented 8 years ago

@brockallen I'm looking for some configuration option or combination which would allow me to programmatically log users in to their accounts upon creation but still require that they verify their email within some configuration defined window of time. Does this currently exist?

vinneyk commented 8 years ago

@brockallen PR incoming for this feature, should you choose to accept it. All UserAccountServiceTests tests, including a couple new tests to cover the new logic, are passing. https://github.com/vinneyk/BrockAllen.MembershipReboot/pull/1

brockallen commented 8 years ago

RequireAccountVerification: https://github.com/brockallen/BrockAllen.MembershipReboot/wiki/Security-Settings-Configuration

vinneyk commented 8 years ago

Are you suggesting that the RequireAccountVerification feature does what I'm looking for? Again, I want to require account authorization but I want to allow new users to access their account for some period of time before they verify the account. After the allotted time, users will not be able to access their account unless/until the account is verified.

When I set RequireAccountVerification=true, I am unable to authenticate the users who have not yet verified the account. If is set it to false then there is no verification requirement which is also not what I want. The supplied changeset does exactly what I'm proposing: grants temporary access to un-validated user accounts with a configurable access timeout.

brockallen commented 8 years ago

Well, RequireAccountVerification tells MR to allow logins even though email is not verified. If you want something in-between then you'll have to do something else.

Give me a high level of what you want to add before you do the work. Unless it's small, I don't know how much disruption I want to introduce.

vinneyk commented 8 years ago

What I'm looking for is an in-between but the required change is small and unobtrusive to existing code. I've already completed the necessary change and now I'm, sadly and rather victoriously, trying to grasp the difference between GitHub and BitBucket so I can create pull request. Check this, if you want to see the impending request's content: https://github.com/vinneyk/BrockAllen.MembershipReboot/commit/4966eaffef838260f7639c23c6704dc3b5a70c8e

vinneyk commented 8 years ago

I retracted my commit because it's incomplete but, before I pursue it any further, I'd like to know whether that's a feature you'd consider pulling into the project.

brockallen commented 8 years ago

Honestly I don't like letting users in until they've confirmed their email. It allows them to use the system and accumulate data in the system before they have a safe way to reset password. If the email that was entered was incorrect, and the user forgets their password, then they have no way to get back into their account without an admin helping.

Now not everyone cares, so that's why I added the RequireAccountVerification to allow people to relax that rule, but I think it's the wrong approach.

There might already be another hook somewhere in the MR pipeline at login time where you can enforce your logic. You'd have to look...

vinneyk commented 8 years ago

I agree it's a bit more of a complex pipeline and one that I might need to table for now. I was inspired by Slack's user onboarding process which goes something like this:

  1. Create account w/o password
  2. Send account activation email
  3. Log users in with persistent cookie
  4. After x days, automatically log the user out and prevent continued access to the site until the account is verified.
  5. Upon account verification, have the user set their password.

There's no hook that I can see because it always comes to the if(account.IsAccountVerified || !UserAccountService.Configuration.RequireAccountVerification) or some variation of that logic which prevents login and authentication. I've considered adding an override to a enable programmatic a bypass of that check but I'm not sure that's the best way about it either. I'd appreciate your thoughts.

brockallen commented 8 years ago

I don't see how Slack verifies the user's email -- do they require a password when verifying the code sent to the email?

vinneyk commented 8 years ago

The email contains an activation link. When the user's click the link within the email, they are presented with the page where they will first create their password. So, it is possible for someone to take over an account if the user enters the wrong email address and someone else get's the invitation. That's something to consider. I wonder how they would handle such an occurrence...

brockallen commented 8 years ago

I wonder how they would handle such an occurrence

Exactly. At that point the account is hosed until an admin intervenes.

vinneyk commented 8 years ago

@brockallen how would you handle users who can't successfully enter their password upon account activation? I'm pretty sure you wouldn't issue a password reset link to an unverified account. Would you consider the account to be hosed without admin intervention in that case? Perhaps that's where the Cancel Validation link would come into play?

brockallen commented 8 years ago

how would you handle users who can't successfully enter their password upon account activation?

An admin must intervene.