Skip to content

Commit d838165

Browse files
TratcherTratcher
authored
Fix up how OIDC errors flow (#4520)
* Add regression test for #4384 * Fix up how OIDC errors flow #4384
1 parent 86071c9 commit d838165

File tree

9 files changed

+128
-68
lines changed

9 files changed

+128
-68
lines changed

‎src/Azure/Azure.sln

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.TestHo
7373
EndProject
7474
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Authorization", "..\Security\Authorization\Core\src\Microsoft.AspNetCore.Authorization.csproj", "{C57DFBC2-A887-44B4-A149-7ABFA6D98F7E}"
7575
EndProject
76+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Authentication.OpenIdConnect", "..\Security\Authentication\OpenIdConnect\src\Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj", "{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}"
77+
EndProject
78+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Authentication.OAuth", "..\Security\Authentication\OAuth\src\Microsoft.AspNetCore.Authentication.OAuth.csproj", "{406DF28A-0B58-408E-96B0-2D373EE36352}"
79+
EndProject
80+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Authentication", "..\Security\Authentication\Core\src\Microsoft.AspNetCore.Authentication.csproj", "{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}"
81+
EndProject
7682
Global
7783
GlobalSection(SolutionConfigurationPlatforms) = preSolution
7884
Debug|Any CPU = Debug|Any CPU
@@ -383,6 +389,42 @@ Global
383389
{C57DFBC2-A887-44B4-A149-7ABFA6D98F7E}.Release|x64.Build.0 = Release|Any CPU
384390
{C57DFBC2-A887-44B4-A149-7ABFA6D98F7E}.Release|x86.ActiveCfg = Release|Any CPU
385391
{C57DFBC2-A887-44B4-A149-7ABFA6D98F7E}.Release|x86.Build.0 = Release|Any CPU
392+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
393+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Debug|Any CPU.Build.0 = Debug|Any CPU
394+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Debug|x64.ActiveCfg = Debug|Any CPU
395+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Debug|x64.Build.0 = Debug|Any CPU
396+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Debug|x86.ActiveCfg = Debug|Any CPU
397+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Debug|x86.Build.0 = Debug|Any CPU
398+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Release|Any CPU.ActiveCfg = Release|Any CPU
399+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Release|Any CPU.Build.0 = Release|Any CPU
400+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Release|x64.ActiveCfg = Release|Any CPU
401+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Release|x64.Build.0 = Release|Any CPU
402+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Release|x86.ActiveCfg = Release|Any CPU
403+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C}.Release|x86.Build.0 = Release|Any CPU
404+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
405+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Debug|Any CPU.Build.0 = Debug|Any CPU
406+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Debug|x64.ActiveCfg = Debug|Any CPU
407+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Debug|x64.Build.0 = Debug|Any CPU
408+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Debug|x86.ActiveCfg = Debug|Any CPU
409+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Debug|x86.Build.0 = Debug|Any CPU
410+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Release|Any CPU.ActiveCfg = Release|Any CPU
411+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Release|Any CPU.Build.0 = Release|Any CPU
412+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Release|x64.ActiveCfg = Release|Any CPU
413+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Release|x64.Build.0 = Release|Any CPU
414+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Release|x86.ActiveCfg = Release|Any CPU
415+
{406DF28A-0B58-408E-96B0-2D373EE36352}.Release|x86.Build.0 = Release|Any CPU
416+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
417+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Debug|Any CPU.Build.0 = Debug|Any CPU
418+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Debug|x64.ActiveCfg = Debug|Any CPU
419+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Debug|x64.Build.0 = Debug|Any CPU
420+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Debug|x86.ActiveCfg = Debug|Any CPU
421+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Debug|x86.Build.0 = Debug|Any CPU
422+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Release|Any CPU.ActiveCfg = Release|Any CPU
423+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Release|Any CPU.Build.0 = Release|Any CPU
424+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Release|x64.ActiveCfg = Release|Any CPU
425+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Release|x64.Build.0 = Release|Any CPU
426+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Release|x86.ActiveCfg = Release|Any CPU
427+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC}.Release|x86.Build.0 = Release|Any CPU
386428
EndGlobalSection
387429
GlobalSection(SolutionProperties) = preSolution
388430
HideSolutionNode = FALSE
@@ -418,6 +460,9 @@ Global
418460
{38027842-48A7-4A64-A44F-004BAF0AB450} = {84622717-F98A-4DE2-806E-1EF89C45C0EB}
419461
{C520D48E-87A0-463D-B4CF-3E6B68F8F4D0} = {84622717-F98A-4DE2-806E-1EF89C45C0EB}
420462
{C57DFBC2-A887-44B4-A149-7ABFA6D98F7E} = {84622717-F98A-4DE2-806E-1EF89C45C0EB}
463+
{F44054A2-DAC9-467F-B899-F35F9DCDAE9C} = {84622717-F98A-4DE2-806E-1EF89C45C0EB}
464+
{406DF28A-0B58-408E-96B0-2D373EE36352} = {84622717-F98A-4DE2-806E-1EF89C45C0EB}
465+
{A5E7BA46-B76B-467A-88FA-38E04D0A42FC} = {84622717-F98A-4DE2-806E-1EF89C45C0EB}
421466
EndGlobalSection
422467
GlobalSection(ExtensibilityGlobals) = postSolution
423468
SolutionGuid = {81AADD49-473B-43ED-8A08-F6B7A058AA39}

‎src/Azure/AzureAD/test/FunctionalTests/WebAuthenticationTests.cs

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.using Microsoft.AspNetCore.Authorization;
33

44
using System.Net;
@@ -11,7 +11,9 @@
1111
using Microsoft.AspNetCore.Mvc.Authorization;
1212
using Microsoft.AspNetCore.Mvc.Testing;
1313
using Microsoft.AspNetCore.TestHost;
14+
using Microsoft.AspNetCore.WebUtilities;
1415
using Microsoft.Extensions.DependencyInjection;
16+
using Microsoft.Extensions.Primitives;
1517
using Microsoft.IdentityModel.Protocols.OpenIdConnect;
1618
using Xunit;
1719

@@ -158,5 +160,60 @@ public async Task ADB2CEndpoints_AreAvailable_When_Authentication_IsAdded(string
158160
// Assert
159161
Assert.Equal(expectedStatusCode, response.StatusCode);
160162
}
163+
164+
[Fact]
165+
public async Task ADB2C_EndToEnd_PasswordReset()
166+
{
167+
var client = Factory.WithWebHostBuilder(builder => builder.ConfigureTestServices(
168+
services =>
169+
{
170+
services
171+
.AddAuthentication(AzureADB2CDefaults.AuthenticationScheme)
172+
.AddAzureADB2C(o =>
173+
{
174+
o.Instance = "https://login.microsoftonline.com/tfp/";
175+
o.ClientId = "ClientId";
176+
o.CallbackPath = "/signin-oidc";
177+
o.Domain = "test.onmicrosoft.com";
178+
o.SignUpSignInPolicyId = "B2C_1_SiUpIn";
179+
o.ResetPasswordPolicyId = "B2C_1_SSPR";
180+
o.EditProfilePolicyId = "B2C_1_SiPe";
181+
});
182+
183+
services.Configure<OpenIdConnectOptions>(AzureADB2CDefaults.OpenIdScheme, o =>
184+
{
185+
o.Configuration = new OpenIdConnectConfiguration()
186+
{
187+
Issuer = "https://www.example.com",
188+
TokenEndpoint = "https://www.example.com/token",
189+
AuthorizationEndpoint = "https://www.example.com/authorize",
190+
EndSessionEndpoint = "https://www.example.com/logout"
191+
};
192+
// CookieContainer doesn't allow cookies from other paths
193+
o.CorrelationCookie.Path = "/";
194+
o.NonceCookie.Path = "/";
195+
});
196+
197+
services.AddMvc(o => o.Filters.Add(
198+
new AuthorizeFilter(new AuthorizationPolicyBuilder(new[] { AzureADB2CDefaults.AuthenticationScheme })
199+
.RequireAuthenticatedUser().Build())));
200+
})).CreateClient(new WebApplicationFactoryClientOptions() { AllowAutoRedirect = false });
201+
202+
var response = await client.GetAsync("/api/get");
203+
Assert.Equal(HttpStatusCode.Redirect, response.StatusCode);
204+
205+
var location = response.Headers.Location;
206+
Assert.StartsWith("https://www.example.com/authorize", location.AbsoluteUri);
207+
var queryString = location.Query;
208+
var query = QueryHelpers.ParseQuery(queryString);
209+
var state = query["state"];
210+
Assert.False(StringValues.IsNullOrEmpty(state));
211+
212+
// Mock Authorization response
213+
response = await client.GetAsync($"/signin-oidc?error=access_denied&error_description=AADB2C90118&state={state}");
214+
215+
Assert.Equal(HttpStatusCode.Redirect, response.StatusCode);
216+
Assert.Equal("/AzureADB2C/Account/ResetPassword/AzureADB2C", response.Headers.Location.OriginalString);
217+
}
161218
}
162219
}

‎src/Azure/AzureAD/test/testassets/AzureAD.WebSite/AzureAD.WebSite.csproj

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk.Web">
1+
<Project Sdk="Microsoft.NET.Sdk.Web">
22

33
<PropertyGroup>
44
<TargetFramework>netcoreapp3.0</TargetFramework>
@@ -9,16 +9,8 @@
99
<Reference Include="Microsoft.AspNetCore.Authentication.AzureADB2C.UI" />
1010
<Reference Include="Microsoft.AspNetCore.Authorization" />
1111
<Reference Include="Microsoft.AspNetCore.DataProtection.Extensions" />
12-
<Reference Include="Microsoft.AspNetCore.Hosting" />
12+
<Reference Include="Microsoft.AspNetCore" />
1313
<Reference Include="Microsoft.AspNetCore.Mvc" />
14-
<Reference Include="Microsoft.AspNetCore.Server.IISIntegration" />
15-
<Reference Include="Microsoft.AspNetCore.Server.Kestrel" />
16-
<Reference Include="Microsoft.Extensions.Configuration.CommandLine" />
17-
<Reference Include="Microsoft.Extensions.Configuration.EnvironmentVariables" />
18-
<Reference Include="Microsoft.Extensions.Configuration.UserSecrets" />
19-
<Reference Include="Microsoft.Extensions.Logging.Configuration" />
20-
<Reference Include="Microsoft.Extensions.Logging.Console" />
21-
<Reference Include="Microsoft.Extensions.Logging.Debug" />
2214
<Reference Include="Microsoft.NET.Sdk.Razor" PrivateAssets="All" />
2315
</ItemGroup>
2416

‎src/Azure/AzureAD/test/testassets/AzureAD.WebSite/Program.cs

Lines changed: 3 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -23,55 +23,8 @@ public static void Main(string[] args)
2323

2424
public static IWebHostBuilder CreateWebHostBuilder(string[] args)
2525
{
26-
var builder = new WebHostBuilder()
27-
.UseKestrel((builderContext, options) =>
28-
{
29-
options.Configure(builderContext.Configuration.GetSection("Kestrel"));
30-
})
31-
.UseContentRoot(Directory.GetCurrentDirectory())
32-
.ConfigureAppConfiguration((hostingContext, config) =>
33-
{
34-
var env = hostingContext.HostingEnvironment;
35-
36-
config.AddJsonFile("appsettings.json", optional: true, reloadOnChange: true)
37-
.AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true, reloadOnChange: true);
38-
39-
if (env.IsDevelopment())
40-
{
41-
var appAssembly = Assembly.Load(new AssemblyName(env.ApplicationName));
42-
if (appAssembly != null)
43-
{
44-
config.AddUserSecrets(appAssembly, optional: true);
45-
}
46-
}
47-
48-
config.AddEnvironmentVariables();
49-
50-
if (args != null)
51-
{
52-
config.AddCommandLine(args);
53-
}
54-
})
55-
.ConfigureLogging((hostingContext, logging) =>
56-
{
57-
logging.AddConfiguration(hostingContext.Configuration.GetSection("Logging"));
58-
logging.AddConsole();
59-
logging.AddDebug();
60-
})
61-
.UseIISIntegration()
62-
.UseDefaultServiceProvider((context, options) =>
63-
{
64-
options.ValidateScopes = context.HostingEnvironment.IsDevelopment();
65-
});
66-
67-
if (args != null)
68-
{
69-
builder.UseConfiguration(new ConfigurationBuilder().AddCommandLine(args).Build());
70-
}
71-
72-
builder.UseStartup<Startup>();
73-
74-
return builder;
26+
return WebHost.CreateDefaultBuilder()
27+
.UseStartup<Startup>();
7528
}
7629
}
7730
}

‎src/Security/Authentication/Core/src/HandleRequestResult.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -92,5 +92,10 @@ public static HandleRequestResult SkipHandler()
9292
{
9393
return new HandleRequestResult() { Skipped = true };
9494
}
95+
96+
public new static HandleRequestResult NoResult()
97+
{
98+
return new HandleRequestResult() { None = true };
99+
}
95100
}
96101
}

‎src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ protected virtual async Task<HandleRequestResult> HandleAccessDeniedErrorAsync(A
283283
return HandleRequestResult.Handle();
284284
}
285285

286-
return HandleRequestResult.Fail("Access was denied by the resource owner or by the remote server.", properties);
286+
return HandleRequestResult.NoResult();
287287
}
288288
}
289-
}
289+
}

‎src/Security/Authentication/OAuth/src/OAuthHandler.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
7171
// Visit https://tools.ietf.org/html/rfc6749#section-4.1.2.1 for more information.
7272
if (StringValues.Equals(error, "access_denied"))
7373
{
74-
return await HandleAccessDeniedErrorAsync(properties);
74+
var result = await HandleAccessDeniedErrorAsync(properties);
75+
return !result.None ? result
76+
: HandleRequestResult.Fail("Access was denied by the resource owner or by the remote server.", properties);
7577
}
7678

7779
var failureMessage = new StringBuilder();

‎src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,11 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
560560
// Visit https://tools.ietf.org/html/rfc6749#section-4.1.2.1 for more information.
561561
if (string.Equals(authorizationResponse.Error, "access_denied", StringComparison.Ordinal))
562562
{
563-
return await HandleAccessDeniedErrorAsync(properties);
563+
var result = await HandleAccessDeniedErrorAsync(properties);
564+
if (!result.None)
565+
{
566+
return result;
567+
}
564568
}
565569

566570
return HandleRequestResult.Fail(CreateOpenIdConnectProtocolException(authorizationResponse, response: null), properties);

‎src/Security/Authentication/Twitter/src/TwitterHandler.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
6262
// approve the authorization demand requested by the remote authorization server.
6363
// Since it's a frequent scenario (that is not caused by incorrect configuration),
6464
// denied errors are handled differently using HandleAccessDeniedErrorAsync().
65-
return await HandleAccessDeniedErrorAsync(properties);
65+
var result = await HandleAccessDeniedErrorAsync(properties);
66+
return !result.None ? result
67+
: HandleRequestResult.Fail("Access was denied by the resource owner or by the remote server.", properties);
6668
}
6769

6870
var returnedToken = query["oauth_token"];
@@ -311,4 +313,4 @@ private string ComputeSignature(string consumerSecret, string tokenSecret, strin
311313
}
312314
}
313315
}
314-
}
316+
}

0 commit comments

Comments
 (0)