Skip to content

Commit 6b01141

Browse files
authored
Add improvements to authorization settings (#5588)
1 parent 1377c82 commit 6b01141

5 files changed

Lines changed: 30 additions & 2 deletions

File tree

src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ public static async Task AssertAuthConfigurationResponse(
105105
string expectedClientId = null,
106106
string expectedAuthority = null,
107107
string expectedAudience = null,
108-
string expectedApiScopes = null)
108+
string expectedApiScopes = null,
109+
bool expectedRoleBasedAuthorizationEnabled = false)
109110
{
110111
Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.OK),
111112
"Authentication configuration endpoint should return 200 OK");
@@ -120,6 +121,11 @@ public static async Task AssertAuthConfigurationResponse(
120121
"Response should contain 'enabled' property");
121122
Assert.That(enabledProperty.GetBoolean(), Is.EqualTo(expectedEnabled),
122123
$"'enabled' should be {expectedEnabled}");
124+
125+
Assert.That(root.TryGetProperty("role_based_authorization_enabled", out var roleBasedAuthorizationEnabledProperty), Is.True,
126+
"Response should contain 'role_based_authorization_enabled' property");
127+
Assert.That(roleBasedAuthorizationEnabledProperty.GetBoolean(), Is.EqualTo(expectedRoleBasedAuthorizationEnabled),
128+
$"'role_based_authorization_enabled' should be {expectedRoleBasedAuthorizationEnabled}");
123129
}
124130

125131
// Note: API uses snake_case JSON serialization (client_id, api_scopes)

src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ await OpenIdConnectAssertions.AssertAuthConfigurationResponse(
7474
expectedEnabled: true,
7575
expectedClientId: TestClientId,
7676
expectedAudience: TestAudience,
77-
expectedApiScopes: TestApiScopes);
77+
expectedApiScopes: TestApiScopes,
78+
expectedRoleBasedAuthorizationEnabled: true);
7879
}
7980

8081
[Test]

src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,13 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC
154154

155155
void Validate(bool requireServicePulseSettings)
156156
{
157+
if (!Enabled && RoleBasedAuthorizationEnabled)
158+
{
159+
var message = "Authentication.RoleBasedAuthorizationEnabled cannot be true when Authentication.Enabled is false. Role-based authorization requires authentication to be enabled.";
160+
logger.LogCritical(message);
161+
throw new Exception(message);
162+
}
163+
157164
if (Enabled)
158165
{
159166
ValidateRequiredSettings(requireServicePulseSettings);

src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ public void TearDown()
2929
Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_VALIDATELIFETIME", null);
3030
Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_VALIDATEISSUERSIGNINGKEY", null);
3131
Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_REQUIREHTTPSMETADATA", null);
32+
Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED", null);
3233
Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID", null);
3334
Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", null);
3435
Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_AUTHORITY", null);
@@ -49,6 +50,7 @@ public void Should_have_correct_defaults()
4950
Assert.That(settings.ValidateLifetime, Is.True);
5051
Assert.That(settings.ValidateIssuerSigningKey, Is.True);
5152
Assert.That(settings.RequireHttpsMetadata, Is.True);
53+
Assert.That(settings.RoleBasedAuthorizationEnabled, Is.False);
5254
Assert.That(settings.ServicePulseClientId, Is.Null);
5355
Assert.That(settings.ServicePulseApiScopes, Is.Null);
5456
Assert.That(settings.ServicePulseAuthority, Is.Null);
@@ -265,6 +267,16 @@ public void Should_succeed_without_service_pulse_settings_when_not_required()
265267
}
266268
}
267269

270+
[Test]
271+
public void Should_throw_when_role_based_authorization_enabled_without_authentication_enabled()
272+
{
273+
Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED", "true");
274+
275+
var ex = Assert.Throws<Exception>(() => new OpenIdConnectSettings(TestNamespace, validateConfiguration: true, requireServicePulseSettings: false));
276+
277+
Assert.That(ex.Message, Does.Contain("RoleBasedAuthorizationEnabled cannot be true when Authentication.Enabled is false"));
278+
}
279+
268280
[Test]
269281
public void Should_not_validate_when_disabled()
270282
{

src/ServiceControl/Authentication/AuthenticationController.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public ActionResult<AuthConfig> Configuration()
1717
var info = new AuthConfig
1818
{
1919
Enabled = settings.OpenIdConnectSettings.Enabled,
20+
RoleBasedAuthorizationEnabled = settings.OpenIdConnectSettings.RoleBasedAuthorizationEnabled,
2021
ClientId = settings.OpenIdConnectSettings.ServicePulseClientId,
2122
Authority = settings.OpenIdConnectSettings.ServicePulseAuthority,
2223
Audience = settings.OpenIdConnectSettings.Audience,
@@ -34,6 +35,7 @@ public ActionResult<AuthConfig> Configuration()
3435
public class AuthConfig
3536
{
3637
public bool Enabled { get; set; }
38+
public bool RoleBasedAuthorizationEnabled { get; set; }
3739
public string ClientId { get; set; }
3840
public string Authority { get; set; }
3941
public string Audience { get; set; }

0 commit comments

Comments
 (0)