From f0e1d1eb540c36402c386e66e73b9b7736578281 Mon Sep 17 00:00:00 2001
From: Safia Abdalla <safia@microsoft.com>
Date: Tue, 28 Jun 2022 09:07:56 -0700
Subject: [PATCH] Fix up reading auth schemes and setting default scheme
(#42452)
* Fix up reading auth schemes and setting default scheme
* Address feedback from peer review
---
.../src/AuthenticationService.cs | 11 +-
...ticationConfigurationProviderExtensions.cs | 2 +-
.../test/AuthenticationMiddlewareTests.cs | 10 ++
.../src/Commands/ClearCommand.cs | 2 +-
.../JwtAuthenticationSchemeSettings.cs | 17 ++-
.../test/UserJwtsTestFixture.cs | 4 +-
.../dotnet-user-jwts/test/UserJwtsTests.cs | 107 +++++++++++++++---
7 files changed, 129 insertions(+), 24 deletions(-)
diff --git a/src/Http/Authentication.Core/src/AuthenticationService.cs b/src/Http/Authentication.Core/src/AuthenticationService.cs
index e949edcfac5..aa35ea30602 100644
--- a/src/Http/Authentication.Core/src/AuthenticationService.cs
+++ b/src/Http/Authentication.Core/src/AuthenticationService.cs
@@ -14,6 +14,7 @@ namespace Microsoft.AspNetCore.Authentication;
public class AuthenticationService : IAuthenticationService
{
private HashSet<ClaimsPrincipal>? _transformCache;
+ private const string defaultSchemesOptionsMsg = "The default schemes can be set using either AddAuthentication(string defaultScheme) or AddAuthentication(Action<AuthenticationOptions> configureOptions) or by setting the Authentication:DefaultScheme property in configuration.";
/// <summary>
/// Constructor.
@@ -64,7 +65,7 @@ public class AuthenticationService : IAuthenticationService
scheme = defaultScheme?.Name;
if (scheme == null)
{
- throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultAuthenticateScheme found. The default schemes can be set using either AddAuthentication(string defaultScheme) or AddAuthentication(Action<AuthenticationOptions> configureOptions).");
+ throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultAuthenticateScheme found. {defaultSchemesOptionsMsg}");
}
}
@@ -112,7 +113,7 @@ public class AuthenticationService : IAuthenticationService
scheme = defaultChallengeScheme?.Name;
if (scheme == null)
{
- throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultChallengeScheme found. The default schemes can be set using either AddAuthentication(string defaultScheme) or AddAuthentication(Action<AuthenticationOptions> configureOptions).");
+ throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultChallengeScheme found. {defaultSchemesOptionsMsg}");
}
}
@@ -140,7 +141,7 @@ public class AuthenticationService : IAuthenticationService
scheme = defaultForbidScheme?.Name;
if (scheme == null)
{
- throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultForbidScheme found. The default schemes can be set using either AddAuthentication(string defaultScheme) or AddAuthentication(Action<AuthenticationOptions> configureOptions).");
+ throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultForbidScheme found. {defaultSchemesOptionsMsg}");
}
}
@@ -186,7 +187,7 @@ public class AuthenticationService : IAuthenticationService
scheme = defaultScheme?.Name;
if (scheme == null)
{
- throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultSignInScheme found. The default schemes can be set using either AddAuthentication(string defaultScheme) or AddAuthentication(Action<AuthenticationOptions> configureOptions).");
+ throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultSignInScheme found. {defaultSchemesOptionsMsg}");
}
}
@@ -220,7 +221,7 @@ public class AuthenticationService : IAuthenticationService
scheme = defaultScheme?.Name;
if (scheme == null)
{
- throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultSignOutScheme found. The default schemes can be set using either AddAuthentication(string defaultScheme) or AddAuthentication(Action<AuthenticationOptions> configureOptions).");
+ throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultSignOutScheme found. {defaultSchemesOptionsMsg}");
}
}
diff --git a/src/Security/Authentication/Core/src/AuthenticationConfigurationProviderExtensions.cs b/src/Security/Authentication/Core/src/AuthenticationConfigurationProviderExtensions.cs
index ccca635be83..2002ff02f35 100644
--- a/src/Security/Authentication/Core/src/AuthenticationConfigurationProviderExtensions.cs
+++ b/src/Security/Authentication/Core/src/AuthenticationConfigurationProviderExtensions.cs
@@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Authentication;
/// </summary>
public static class AuthenticationConfigurationProviderExtensions
{
- private const string AuthenticationSchemesKey = "Authentication:Schemes";
+ private const string AuthenticationSchemesKey = "Schemes";
/// <summary>
/// Returns the specified <see cref="IConfiguration"/> object.
diff --git a/src/Security/Authentication/test/AuthenticationMiddlewareTests.cs b/src/Security/Authentication/test/AuthenticationMiddlewareTests.cs
index 422a4e633f4..9c11dc1e35c 100644
--- a/src/Security/Authentication/test/AuthenticationMiddlewareTests.cs
+++ b/src/Security/Authentication/test/AuthenticationMiddlewareTests.cs
@@ -11,6 +11,7 @@ using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
using Moq;
namespace Microsoft.AspNetCore.Authentication;
@@ -156,6 +157,11 @@ public class AuthenticationMiddlewareTests
public async Task WebApplicationBuilder_RegistersAuthenticationMiddlewares()
{
var builder = WebApplication.CreateBuilder();
+ builder.Configuration.AddInMemoryCollection(new[]
+ {
+ new KeyValuePair<string, string>("Authentication:Schemes:Bearer:ClaimsIssuer", "SomeIssuer"),
+ new KeyValuePair<string, string>("Authentication:Schemes:Bearer:Audiences:0", "https://localhost:5001")
+ });
builder.Authentication.AddJwtBearer();
await using var app = builder.Build();
@@ -169,6 +175,10 @@ public class AuthenticationMiddlewareTests
await app.StartAsync();
Assert.True(app.Properties.ContainsKey("__AuthenticationMiddlewareSet"));
+
+ var options = app.Services.GetService<IOptionsMonitor<JwtBearerOptions>>().Get(JwtBearerDefaults.AuthenticationScheme);
+ Assert.Equal(new[] { "SomeIssuer" }, options.TokenValidationParameters.ValidIssuers);
+ Assert.Equal(new[] { "https://localhost:5001" }, options.TokenValidationParameters.ValidAudiences);
}
private HttpContext GetHttpContext(
diff --git a/src/Tools/dotnet-user-jwts/src/Commands/ClearCommand.cs b/src/Tools/dotnet-user-jwts/src/Commands/ClearCommand.cs
index abc01f770e3..2b977e3f631 100644
--- a/src/Tools/dotnet-user-jwts/src/Commands/ClearCommand.cs
+++ b/src/Tools/dotnet-user-jwts/src/Commands/ClearCommand.cs
@@ -45,7 +45,7 @@ internal sealed class ClearCommand
if (!force)
{
- reporter.Output(Resources.ClearCommand_Permission);
+ reporter.Output(Resources.FormatClearCommand_Permission(count, project));
reporter.Output("[Y]es / [N]o");
if (Console.ReadLine().Trim().ToUpperInvariant() != "Y")
{
diff --git a/src/Tools/dotnet-user-jwts/src/Helpers/JwtAuthenticationSchemeSettings.cs b/src/Tools/dotnet-user-jwts/src/Helpers/JwtAuthenticationSchemeSettings.cs
index b8108f5294c..77f95e6df13 100644
--- a/src/Tools/dotnet-user-jwts/src/Helpers/JwtAuthenticationSchemeSettings.cs
+++ b/src/Tools/dotnet-user-jwts/src/Helpers/JwtAuthenticationSchemeSettings.cs
@@ -10,6 +10,7 @@ namespace Microsoft.AspNetCore.Authentication.JwtBearer.Tools;
internal sealed record JwtAuthenticationSchemeSettings(string SchemeName, List<string> Audiences, string ClaimsIssuer)
{
private const string AuthenticationKey = "Authentication";
+ private const string DefaultSchemeKey = "DefaultScheme";
private const string SchemesKey = "Schemes";
private static readonly JsonSerializerOptions _jsonSerializerOptions = new JsonSerializerOptions
@@ -35,7 +36,7 @@ internal sealed record JwtAuthenticationSchemeSettings(string SchemeName, List<s
{
// If a scheme with the same name has already been registered, we
// override with the latest token's options
- schemes[SchemeName] = settingsObject;
+ schemes[SchemeName] = settingsObject;
}
else
{
@@ -56,6 +57,15 @@ internal sealed record JwtAuthenticationSchemeSettings(string SchemeName, List<s
};
}
+ // Set the DefaultScheme if it has not already been set
+ // and only a single scheme has been configured thus far
+ if (config[AuthenticationKey][DefaultSchemeKey] is null
+ && config[AuthenticationKey][SchemesKey] is JsonObject setSchemes
+ && setSchemes.Count == 1)
+ {
+ config[AuthenticationKey][DefaultSchemeKey] = SchemeName;
+ }
+
using var writer = new FileStream(filePath, FileMode.Open, FileAccess.Write);
JsonSerializer.Serialize(writer, config, _jsonSerializerOptions);
}
@@ -70,6 +80,11 @@ internal sealed record JwtAuthenticationSchemeSettings(string SchemeName, List<s
authentication[SchemesKey] is JsonObject schemes)
{
schemes.Remove(name);
+ if (authentication[DefaultSchemeKey] is JsonValue defaultScheme
+ && defaultScheme.GetValue<string>() == name)
+ {
+ authentication.Remove(DefaultSchemeKey);
+ }
}
using var writer = new FileStream(filePath, FileMode.Create, FileAccess.Write);
diff --git a/src/Tools/dotnet-user-jwts/test/UserJwtsTestFixture.cs b/src/Tools/dotnet-user-jwts/test/UserJwtsTestFixture.cs
index c4364d33cb0..1e14b7d5d67 100644
--- a/src/Tools/dotnet-user-jwts/test/UserJwtsTestFixture.cs
+++ b/src/Tools/dotnet-user-jwts/test/UserJwtsTestFixture.cs
@@ -62,7 +62,7 @@ public class UserJwtsTestFixture : IDisposable
}
}";
- public string CreateProject(bool hasSecret = true)
+ public string CreateProject(bool hasSecret = true, string appSettingsContent = "{}")
{
var projectPath = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "userjwtstest", Guid.NewGuid().ToString()));
Directory.CreateDirectory(Path.Combine(projectPath.FullName, "Properties"));
@@ -81,7 +81,7 @@ public class UserJwtsTestFixture : IDisposable
File.WriteAllText(
Path.Combine(projectPath.FullName, "appsettings.Development.json"),
- "{}");
+ appSettingsContent);
if (hasSecret)
{
diff --git a/src/Tools/dotnet-user-jwts/test/UserJwtsTests.cs b/src/Tools/dotnet-user-jwts/test/UserJwtsTests.cs
index 8947aa2234f..3c35b0085ec 100644
--- a/src/Tools/dotnet-user-jwts/test/UserJwtsTests.cs
+++ b/src/Tools/dotnet-user-jwts/test/UserJwtsTests.cs
@@ -67,15 +67,61 @@ public class UserJwtsTests : IClassFixture<UserJwtsTestFixture>
}
[Fact]
- public void Create_WritesGeneratedTokenToDisk()
+ public async Task Create_SetsDefaultSchemeIfNoOtherSchemesSet()
{
var project = Path.Combine(_fixture.CreateProject(), "TestProject.csproj");
- var appsettings = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
+ var appSettingsPath = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
var app = new Program(_console);
app.Run(new[] { "create", "--project", project });
Assert.Contains("New JWT saved", _console.GetOutput());
- Assert.Contains("dotnet-user-jwts", File.ReadAllText(appsettings));
+
+ using FileStream openStream = File.OpenRead(appSettingsPath);
+ var appSettingsFile = await JsonSerializer.DeserializeAsync<JsonObject>(openStream);
+
+ Assert.True(appSettingsFile.TryGetPropertyValue("Authentication", out var authentication));
+ Assert.Equal("Bearer", authentication["DefaultScheme"].GetValue<string>());
+ Assert.Equal("dotnet-user-jwts", authentication["Schemes"]["Bearer"]["ClaimsIssuer"].GetValue<string>());
+ }
+
+ [Fact]
+ public async Task Create_DoesNotOverrideDefaultSchemeIfAlreadySet()
+ {
+ var project = Path.Combine(_fixture.CreateProject(
+ hasSecret: true,
+ appSettingsContent: @"{ ""Authentication"": { ""DefaultScheme"": ""foobar"" } }"), "TestProject.csproj");
+ var appSettingsPath = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
+ var app = new Program(_console);
+
+ app.Run(new[] { "create", "--project", project });
+ Assert.Contains("New JWT saved", _console.GetOutput());
+
+ using FileStream openStream = File.OpenRead(appSettingsPath);
+ var appSettingsFile = await JsonSerializer.DeserializeAsync<JsonObject>(openStream);
+
+ Assert.True(appSettingsFile.TryGetPropertyValue("Authentication", out var authentication));
+ Assert.Equal("foobar", authentication["DefaultScheme"].GetValue<string>()); //foobar not Bearer
+ Assert.Equal("dotnet-user-jwts", authentication["Schemes"]["Bearer"]["ClaimsIssuer"].GetValue<string>());
+ }
+
+ [Fact]
+ public async Task Create_DoesNotSetDefaultSchemeIfMultipleSchemesConfigured()
+ {
+ var project = Path.Combine(_fixture.CreateProject(
+ hasSecret: true,
+ appSettingsContent: @"{ ""Authentication"": { ""Schemes"": { ""foobar"" : { } } } }"), "TestProject.csproj");
+ var appSettingsPath = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
+ var app = new Program(_console);
+
+ app.Run(new[] { "create", "--project", project });
+ Assert.Contains("New JWT saved", _console.GetOutput());
+
+ using FileStream openStream = File.OpenRead(appSettingsPath);
+ var appSettingsFile = await JsonSerializer.DeserializeAsync<JsonObject>(openStream);
+
+ Assert.True(appSettingsFile.TryGetPropertyValue("Authentication", out var authentication));
+ Assert.Null(authentication["DefaultScheme"]); // Should not be set beause 2 schemes configured
+ Assert.Equal("dotnet-user-jwts", authentication["Schemes"]["Bearer"]["ClaimsIssuer"].GetValue<string>());
}
[Fact]
@@ -92,7 +138,6 @@ public class UserJwtsTests : IClassFixture<UserJwtsTestFixture>
public void List_ReturnsIdForGeneratedToken()
{
var project = Path.Combine(_fixture.CreateProject(), "TestProject.csproj");
- var appsettings = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
var app = new Program(_console);
app.Run(new[] { "create", "--project", project, "--scheme", "MyCustomScheme" });
@@ -103,10 +148,10 @@ public class UserJwtsTests : IClassFixture<UserJwtsTestFixture>
}
[Fact]
- public void Remove_RemovesGeneratedToken()
+ public async Task Remove_RemovesGeneratedToken()
{
var project = Path.Combine(_fixture.CreateProject(), "TestProject.csproj");
- var appsettings = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
+ var appSettingsPath = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
var app = new Program(_console);
app.Run(new[] { "create", "--project", project });
@@ -115,16 +160,45 @@ public class UserJwtsTests : IClassFixture<UserJwtsTestFixture>
app.Run(new[] { "create", "--project", project, "--scheme", "Scheme2" });
app.Run(new[] { "remove", id, "--project", project });
- var appsettingsContent = File.ReadAllText(appsettings);
- Assert.DoesNotContain("Bearer", appsettingsContent);
- Assert.Contains("Scheme2", appsettingsContent);
+
+ using FileStream openStream = File.OpenRead(appSettingsPath);
+ var appSettingsFile = await JsonSerializer.DeserializeAsync<JsonObject>(openStream);
+
+ Assert.True(appSettingsFile.TryGetPropertyValue("Authentication", out var authentication));
+ Assert.Null(authentication["Schemes"]["Bearer"]);
+ Assert.NotNull(authentication["Schemes"]["Scheme2"]);
+ Assert.Null(authentication["DefaultScheme"]);
+ }
+
+ [Fact]
+ public async Task Remove_DoesNotUnsetDefaultSchemeIfNoMatch()
+ {
+ var project = Path.Combine(_fixture.CreateProject(), "TestProject.csproj");
+ var appSettingsPath = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
+ var app = new Program(_console);
+
+ app.Run(new[] { "create", "--project", project });
+ _console.ClearOutput();
+ app.Run(new[] { "create", "--project", project, "--scheme", "Scheme2" });
+ var matches = Regex.Matches(_console.GetOutput(), "New JWT saved with ID '(.*?)'");
+ var id = matches.SingleOrDefault().Groups[1].Value;
+
+ app.Run(new[] { "remove", id, "--project", project });
+
+ using FileStream openStream = File.OpenRead(appSettingsPath);
+ var appSettingsFile = await JsonSerializer.DeserializeAsync<JsonObject>(openStream);
+
+ Assert.True(appSettingsFile.TryGetPropertyValue("Authentication", out var authentication));
+ Assert.NotNull(authentication["Schemes"]["Bearer"]);
+ Assert.Null(authentication["Schemes"]["Scheme2"]);
+ Assert.NotNull(authentication["DefaultScheme"]); // We haven't removed the Bearer scheme so it's still the default
}
[Fact]
- public void Clear_RemovesGeneratedTokens()
+ public async Task Clear_RemovesGeneratedTokens()
{
var project = Path.Combine(_fixture.CreateProject(), "TestProject.csproj");
- var appsettings = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
+ var appSettingsPath = Path.Combine(Path.GetDirectoryName(project), "appsettings.Development.json");
var app = new Program(_console);
app.Run(new[] { "create", "--project", project });
@@ -133,9 +207,14 @@ public class UserJwtsTests : IClassFixture<UserJwtsTestFixture>
Assert.Contains("New JWT saved", _console.GetOutput());
app.Run(new[] { "clear", "--project", project, "--force" });
- var appsettingsContent = File.ReadAllText(appsettings);
- Assert.DoesNotContain("Bearer", appsettingsContent);
- Assert.DoesNotContain("Scheme2", appsettingsContent);
+
+ using FileStream openStream = File.OpenRead(appSettingsPath);
+ var appSettingsFile = await JsonSerializer.DeserializeAsync<JsonObject>(openStream);
+
+ Assert.True(appSettingsFile.TryGetPropertyValue("Authentication", out var authentication));
+ Assert.Null(authentication["Schemes"]["Bearer"]);
+ Assert.Null(authentication["Schemes"]["Scheme2"]);
+ Assert.Null(authentication["DefaultScheme"]);
}
[Fact]
--
GitLab