Should unverified accounts return their claims when UserInfo is requested?

[HenrikWM]

Hi,

I've come across an issue when implementing authentication with MembershipReboot users that go through a two-step login. They will not be marked as verified by my UserServiceuntil they have completed login with a 2fa-code, and I use MembershipReboot's 2fa-feature for this. Until they successfully authenticate, their UserAccountis marked as IsAccountVerified=false and will be deleted by a continually-running cleanup-task after a while.

Now, if you call the /connect/userinfo endpoint with an access_tokenbelonging to a user with an unverified account, should it return claims and 200 OK? The OpenID-spec says:

The UserInfo Endpoint is an OAuth 2.0 Protected Resource that returns Claims about the authenticated End-User.

So just wondering if the IsActiveAsyncmethod in MembershipRebootUserServiceshould be changed to be stricter and check the IsAccountVerified property as well.

ctx.IsActive = acct != null && !acct.IsAccountClosed && acct.IsLoginAllowed; to

ctx.IsActive = acct != null && !acct.IsAccountClosed && acct.IsLoginAllowed && acct.IsAccountVerified;

This will cause the ValidateAccessTokenAsynccalled from the GetUserInfoof UserInfoEndpointControllerin IdentityServer to return invalid_tokenfor unverified accounts.

Curious to hear your thoughts on this :)

[brockallen]

Well, it seems that if MR has "require email to be verified" and the user's email is not verified, then the user should not get pass the authentication stage, no?

[HenrikWM]

True, if I was using e-mail to verify accounts. My configuration requires their mobile phones to be verified, which happens then they provide a valid 2fa-code in step two. So my config is:

EmailIsUsername=False RequireAccountVerification=True EmailIsUsername=False (their mobile phone is their Username)

I'm verifying the accounts with SetConfirmedMobilePhoneof UserAccountService, which is the alternative to the SetConfirmedEmail

[brockallen]

I still don't follow -- GetProfile should not be called until after AuthenticateLocal has succeeded, right? So my point is that if you have logic that says they should not be allowed to authenticate then you should stop that in AuthenticateLocal.

Unless I am misunderstanding something (it's still early here and I'm only on my first coffee).

[HenrikWM]

I can see your point and I'm probably just nitpicking, I can see a possibility of a call to connect/userinfo being done before a user successfully completes AuthenticateLocal. Perhaps during a partial login and a user interaction on another page or even client. And a check on IsAccountVerified would add more protection to ensure expected behavior I think, irregardless of how likely or correct such a call would be :)

[brockallen]

Ok, I guess you can send a PR for it :)