External login info is always null with GitHub

[haythem]

External login info is always null with GitHub

[kevinchalet]

Hey 😄

Try removing these two things, which are already added by ASP.NET Identity when you call services.AddIdentity()/app.UseIdentity() (with different schemes, which probably explains why Identity cannot see the identity produced by your own cookie/GitHub middleware):

services.Configure<SharedAuthenticationOptions>(options => {
    options.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
});

app.UseCookieAuthentication(options => {
    options.AutomaticAuthentication = true;
    options.AuthenticationScheme = CookieAuthenticationDefaults.AuthenticationScheme;
    options.LoginPath = new PathString("/account/login");
});

[haythem]

@PinpointTownes, thanks a lot for this great answer. Indeed, adding app.UseCookieAuthentication confuses identity because of the two different schemes. The problem is now solved.

Now, for the scenario where the user is not present in the database, Identity will redirect us to the ExternalLoginConfirmation view, which will ask for the email address if not present in the claims.

I think that we should add to the GitHub middleware the ability of adding an the email claim in the case where the developer adds the user:email scope.

var email = GitHubHelper.GetEmail(payload);
if (!string.IsNullOrEmpty(email))
{
    identity.AddClaim(new Claim(ClaimTypes.Email, email, ClaimValueTypes.String, Options.ClaimsIssuer));
}

I have another question regarding the addition of the claims to the ClaimsIdentity, why are you using AddOptionalClaim instead of simply AddClaim ?

I'll probably submit a PR (though I have to read the contribution guide before 😄 ) by the end of the day regrading the improvement above.

[kevinchalet]

I think that we should add to the GitHub middleware the ability of adding an the email claim in the case where the developer adds the user:email scope.

Good idea! Feel free to submit a PR to fix that 👍

I have another question regarding the addition of the claims to the ClaimsIdentity, why are you using AddOptionalClaim instead of simply AddClaim ?

For readability: AddOptionalClaim internally does exactly what your snippet does, but allows chained calls.

[kevinchalet]

I think that we should add to the GitHub middleware the ability of adding an the email claim in the case where the developer adds the user:email scope.

Done: 23b52e6