-
Notifications
You must be signed in to change notification settings - Fork 223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug] signInManager.GetExternalLoginInfoAsync() always returns null #133
Comments
Do you want to change the scheme name? (for instance use "AzureAd") services.AddSignIn(Configuration, "AzureAd", "SomeOtherSchemeName") |
@jmprieur Thank you for your reply. Unfortunately this did not bring success: I tried both: services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
.AddSignIn(Configuration, "AzureAd", openIdConnectScheme:AzureADDefaults.AuthenticationScheme, subscribeToOpenIdConnectMiddlewareDiagnosticsEvents: true); and services.AddAuthentication(AzureADDefaults.AuthenticationScheme)
.AddSignIn(Configuration, "AzureAd", openIdConnectScheme:AzureADDefaults.AuthenticationScheme, subscribeToOpenIdConnectMiddlewareDiagnosticsEvents: true); Both leads to the result:
|
I created a WebApplication sample App for you to reproduce the bug. (Create new project -> ASP.NET Core Web Applikation -> Web Applikation (Model-View-Controller). Authentication: Individual User Accounts (Store user accounts in-app). In this I only added two things:
services.AddAuthentication(AzureADDefaults.AuthenticationScheme)
.AddAzureAD(options => Configuration.Bind("AzureAd", options));
//services.AddAuthentication(AzureADDefaults.AuthenticationScheme)
// .AddSignIn(Configuration, "AzureAd", openIdConnectScheme: AzureADDefaults.AuthenticationScheme); Try the "traditional" one first. That works. (Don't forget to click on the Migrate database button) What struck me immediately is that the button for the login with the traditional code is labeled "Azure Active Directory". With your code it is labeled "OpenIdConnect". This although I set the schema to AzureADDefaults.AuthenticationScheme. The provided sample: |
@ManuelHaas thanks for your sample |
@jmprieur This is a pity, the library is then unfortunately useless for me and not only for me I think. |
@ManuelHaas : Do you want to call downstreams APIs from your web app? |
@jmprieur |
This makes sense @ManuelHaas : it's super hard to do without code similar to what is in Microsoft.Identity.Web. I investigated this morning, and don't understand yet why this does not work. but I'll work with the ASP.NET Core team on this. BTW thanks for raising an issue on the docs.ms page |
@jmprieur Thanks for going to ASP.NET Core team with it! |
btw. we stumbled upon that aswell, since we wanted to create meetings when logged in via AzureAD. |
Hi, |
@pmaytak could you take a look at this when you have time (after your current task is done)? thanks. |
@schmitch @ManuelHaas @messerke is this still reproducible with the current version (1.2.0)? |
Hello, I have a setup where I store identities in the app and use my organizations azure ad to create and log users in. I just recently upgraded to Core 5 and now I am getting an obsolete warning on the current code. |
@erik1988 same boat here |
@jennyf19 no it still is broken, we sadly still need to use |
Thanks for the update. we will take a look. @schmitch @erik1988 @khanhvu161188 |
Start with: Don't set any of the schemes in AddAuthentication, let Identity manage those. AddMicrosoftIdentityWebApp will use the default sign-in scheme which is Identity's external cookie. It should all compose from there. This is the same pattern used by other external providers: |
Sorry, there's a deeper issue here. AddMicrosoftIdentityWebApp adds its own cookie auth provider rather than use the existing one. Line 238 in c75ea22
There will need to be a factored out overload of AddMicrosoftIdentityWebApp that does not add its own cookies. |
@schmitch @khanhvu161188 @ManuelHaas @erik1988 do you mind trying this branch, which does offers the possibility of not defining the cookie scheme, if you do something like this: services.AddAuthentication().AddMicrosoftIdentityWebApp(Configuration, cookieScheme: null); cc: @jmprieur |
@jennyf19 thanks for your PR. I just tested the scenario and this works fine with your modification when ensuring that the cookieScheme is set to dotnet new mvc --auth Individual Change the // This method gets called by the runtime. Use this method to add services to the container.
public void ConfigureServices(IServiceCollection services)
{
services.AddDbContext<ApplicationDbContext>(options =>
options.UseSqlite(Configuration.GetConnectionString("DefaultConnection")));
services.AddDatabaseDeveloperPageExceptionFilter();
services.AddDefaultIdentity<IdentityUser>(options => options.SignIn.RequireConfirmedAccount = true)
.AddEntityFrameworkStores<ApplicationDbContext>();
services.AddControllersWithViews();
services.AddAuthentication()
.AddMicrosoftIdentityWebApp(Configuration, cookieScheme: null);
} And this works! This proposes the OpenIdConnect button, which registers the user. When the PR is merged, we'd want to document the scenario in the wiki |
Included in 1.4 Release. |
Thanks! With the "AzureAD" it will look for the existing entry in "AspNetUserLogins" table, without it you may get some issues as it will try to register the user and then give an error saying that the email already exist. |
Hi there, got Login to work now with define cookieScheme: null, but how about Logout method? How you do ? to logout from ad too
|
Which Version of Microsoft Identity Web are you using ?
Microsoft Identity Web 0.1.1-preview
Application
for details see #133 (comment) below
In this I only added two things:
Where is the issue?
-> Sign-in user
In our application we use .NET Core Identity Framework (local DB) and external login provider. So we have multiple authentication provider and some users sign in directly (via .NET Core Identity Framework) and some via AAD.
Calling signInManager.GetExternalLoginInfoAsync() leads to null return value with the updated code.
Maybe it has to do with the appsettings property
"CookieSchemeName": "Identity.External"
. Because this was necessary to get external AAD login working. As stated here. Maybe it will be ignored by now.So we replaced the following code:
with that one:
appsettings.json section was not modified:
Login with the "old" code leads to the following log entries:
Login with the "new" code leads to the following log entries:
The text was updated successfully, but these errors were encountered: