abpframework / abp

Open-source web application framework for ASP.NET Core! Offers an opinionated architecture to build enterprise software solutions with best practices on top of the .NET. Provides the fundamental infrastructure, cross-cutting-concern implementations, startup templates, application modules, UI themes, tooling and documentation.
https://abp.io
GNU Lesser General Public License v3.0
12.94k stars 3.45k forks source link

External Login With Github User. The email claim can't be retrived even I set the scope to user/ #3570

Closed youlxs closed 4 years ago

youlxs commented 4 years ago

I try to integrate the abp vnext with external login for github provider. But I got this exception. System.ArgumentNullException: Value cannot be null. (Parameter 'userName') at Volo.Abp.Check.NotNull[T](T value, String parameterName) at Volo.Abp.Identity.IdentityUser..ctor(Guid id, String userName, String email, Nullable1 tenantId) at Volo.Abp.Account.Web.Pages.Account.LoginModel.CreateExternalUserAsync(ExternalLoginInfo info) at Castle.DynamicProxy.AsyncInterceptorBase.ProceedAsynchronous[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo) at Volo.Abp.Castle.DynamicProxy.CastleAbpMethodInvocationAdapterWithReturnValue1.ProceedAsync() at Volo.Abp.Uow.UnitOfWorkInterceptor.InterceptAsync(IAbpMethodInvocation invocation) at Volo.Abp.Castle.DynamicProxy.CastleAsyncAbpInterceptorAdapter`1.InterceptAsync[TResult](IInvocation invocation, IInvocationProceedInfo

The code here can not get the correct email claim.

image

here is my setup

image

olicooper commented 4 years ago

Have you looked at what the values in info.Principal are in a debugging session? It might help to see why the email address is null. I would also suggest not to use ABP's ClaimTypes as they won't match GitHub's claims which may change in the future too. https://github.com/abpframework/abp/blob/c2b77fa92c961b6e05e848832946b60ab3a09f5d/framework/src/Volo.Abp.Security/Volo/Abp/Security/Claims/AbpClaimTypes.cs#L6-L8

Also, maybe the scope you need is user:email (the user scope also gives write access which might not be needed?) seen here

realLiangshiwei commented 4 years ago

See https://github.com/abpframework/abp/blob/1e5f9111fde5bb7750ffdd53b11dbaef9207f3b7/docs/en/How-To/Azure-Active-Directory-Authentication-MVC.md#faq

youlxs commented 4 years ago

Have you looked at what the values in info.Principal are in a debugging session? It might help to see why the email address is null. I would also suggest not to use ABP's ClaimTypes as they won't match GitHub's claims which may change in the future too.

https://github.com/abpframework/abp/blob/c2b77fa92c961b6e05e848832946b60ab3a09f5d/framework/src/Volo.Abp.Security/Volo/Abp/Security/Claims/AbpClaimTypes.cs#L6-L8

Also, maybe the scope you need is user:email (the user scope also gives write access which might not be needed?) seen here

I just debug the ticket received event, the claims returned by github does not contain the email. The ClaimTypes.Name is returned. I think the sourcecode info.Principal.FindFirstValue(Email) should be change to Name for github.

youlxs commented 4 years ago

Does this work for you? image

do not work

realLiangshiwei commented 4 years ago

As @olicooper said , you need is user:email . You can consider override CreateExternalUserAsync method.

youlxs commented 4 years ago

Hi All, I resolved github integration. There are two issues here.

1> When we request the github the scope options should be added like this.

image Both user and user:email should be added.

2>The line code here is incorrect for github, we should use the ClaimsType.Email.

image

image