From c1ca13d0290f345b9b39bbcb3c669136331798a0 Mon Sep 17 00:00:00 2001 From: williambza Date: Thu, 23 Jul 2026 12:51:31 +0200 Subject: [PATCH 1/2] Allow for a missing offline_access scope --- docs/authentication-testing.md | 5 +- .../OpenIdConnect/OpenIdConnectAssertions.cs | 12 ++++ .../OpenIdConnectTestConfiguration.cs | 11 +++ .../When_authentication_is_enabled.cs | 1 + ..._pulse_offline_access_scope_is_disabled.cs | 67 +++++++++++++++++++ ...rovals.PlatformSampleSettings.approved.txt | 2 + .../OpenIdConnectSettings.cs | 24 ++++++- ...sTests.PlatformSampleSettings.approved.txt | 2 + ...rovals.PlatformSampleSettings.approved.txt | 2 + .../Settings/OpenIdConnectSettingsTests.cs | 32 +++++++++ src/ServiceControl/App.config | 2 + .../AuthenticationController.cs | 12 +++- 12 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_service_pulse_offline_access_scope_is_disabled.cs diff --git a/docs/authentication-testing.md b/docs/authentication-testing.md index 76f018c94b..f165e84105 100644 --- a/docs/authentication-testing.md +++ b/docs/authentication-testing.md @@ -282,11 +282,12 @@ curl http://localhost:33633/api/authentication/configuration | json "enabled": true, "clientId": "test-client-id", "audience": "api://servicecontrol-test", - "apiScopes": "[\"api://servicecontrol-test/access_as_user\"]" + "apiScopes": "[\"api://servicecontrol-test/access_as_user\"]", + "scopes": "[\"api://servicecontrol-test/access_as_user\"] openid profile email offline_access" } ``` -The authentication configuration endpoint is accessible without authentication and returns the configuration that clients need to authenticate. The `authority` field is omitted when `ServicePulse.Authority` is not explicitly set (it defaults to the main Authority for ServicePulse clients). The `audience` field is copied from the `ServiceControl/Authentication.Audience` value. +The authentication configuration endpoint is accessible without authentication and returns the configuration that clients need to authenticate. The `authority` field is omitted when `ServicePulse.Authority` is not explicitly set (it defaults to the main Authority for ServicePulse clients). The `audience` field is copied from the `ServiceControl/Authentication.Audience` value. The `scopes` field is the complete scope string ServicePulse should request, composed by ServiceControl from `apiScopes` plus the fixed `openid profile email` scopes and `offline_access` unless `ServiceControl/Authentication.ServicePulse.OfflineAccessScopeEnabled` is set to `false`. ### Scenario 3: Authentication with Invalid Token diff --git a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs index 5525b9271f..b48cc3ea87 100644 --- a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs +++ b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs @@ -106,6 +106,7 @@ public static async Task AssertAuthConfigurationResponse( string expectedAuthority = null, string expectedAudience = null, string expectedApiScopes = null, + string expectedScopes = null, bool expectedRoleBasedAuthorizationEnabled = false) { Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.OK), @@ -172,6 +173,17 @@ public static async Task AssertAuthConfigurationResponse( $"'api_scopes' should be '{expectedApiScopes}'"); } } + + if (expectedScopes != null) + { + using (Assert.EnterMultipleScope()) + { + Assert.That(root.TryGetProperty("scopes", out var scopesProperty), Is.True, + "Response should contain 'scopes' property"); + Assert.That(scopesProperty.GetString(), Is.EqualTo(expectedScopes), + $"'scopes' should be '{expectedScopes}'"); + } + } } /// diff --git a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs index 3148b18275..ace29cdced 100644 --- a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs +++ b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs @@ -159,6 +159,16 @@ public OpenIdConnectTestConfiguration WithServicePulseAuthority(string authority return this; } + /// + /// Configures whether ServicePulse should request the offline_access scope. + /// Default is true. Set to false to simulate an identity provider that disallows the scope. + /// + public OpenIdConnectTestConfiguration WithServicePulseOfflineAccessScopeEnabled(bool enabled) + { + SetEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED", enabled.ToString().ToLowerInvariant()); + return this; + } + /// /// Clears all OpenID Connect environment variables. /// Called automatically on Dispose. @@ -176,6 +186,7 @@ public void ClearConfiguration() ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_CLIENTID"); ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_APISCOPES"); ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_AUTHORITY"); + ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED"); ClearEnvironmentVariable("AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED"); ClearEnvironmentVariable("VALIDATECONFIG"); } diff --git a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs index 93342685f1..4eca6f5fe8 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -75,6 +75,7 @@ await OpenIdConnectAssertions.AssertAuthConfigurationResponse( expectedClientId: TestClientId, expectedAudience: TestAudience, expectedApiScopes: TestApiScopes, + expectedScopes: $"{TestApiScopes} openid profile email offline_access", expectedRoleBasedAuthorizationEnabled: true); } diff --git a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_service_pulse_offline_access_scope_is_disabled.cs b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_service_pulse_offline_access_scope_is_disabled.cs new file mode 100644 index 0000000000..42c0a7dc67 --- /dev/null +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_service_pulse_offline_access_scope_is_disabled.cs @@ -0,0 +1,67 @@ +namespace ServiceControl.AcceptanceTests.Security.OpenIdConnect +{ + using System.Net.Http; + using System.Threading.Tasks; + using AcceptanceTesting; + using AcceptanceTesting.OpenIdConnect; + using NServiceBus.AcceptanceTesting; + using NUnit.Framework; + + /// + /// ServicePulse Offline Access Scope Opt-Out + /// When Authentication.ServicePulse.OfflineAccessScopeEnabled is false, the composed scope + /// string returned by the authentication configuration endpoint should omit offline_access, + /// so ServicePulse does not request a scope the identity provider disallows. + /// + class When_service_pulse_offline_access_scope_is_disabled : AcceptanceTest + { + OpenIdConnectTestConfiguration configuration; + + const string TestAuthority = "https://login.example.com/tenant-id/v2.0"; + const string TestAudience = "api://test-audience"; + const string TestClientId = "test-client-id"; + const string TestApiScopes = "api://test-audience/.default"; + + [SetUp] + public void ConfigureAuth() => + configuration = new OpenIdConnectTestConfiguration(ServiceControlInstanceType.Primary) + .WithConfigurationValidationDisabled() + .WithAuthenticationEnabled() + .WithAuthority(TestAuthority) + .WithAudience(TestAudience) + .WithServicePulseClientId(TestClientId) + .WithServicePulseApiScopes(TestApiScopes) + .WithServicePulseOfflineAccessScopeEnabled(false) + .WithRequireHttpsMetadata(false); + + [TearDown] + public void CleanupAuth() => configuration?.Dispose(); + + [Test] + public async Task Should_omit_offline_access_from_composed_scopes() + { + HttpResponseMessage response = null; + + _ = await Define() + .Done(async ctx => + { + response = await OpenIdConnectAssertions.SendRequestWithoutAuth( + HttpClient, + HttpMethod.Get, + "/api/authentication/configuration"); + return response != null; + }) + .Run(); + + await OpenIdConnectAssertions.AssertAuthConfigurationResponse( + response, + expectedEnabled: true, + expectedClientId: TestClientId, + expectedAudience: TestAudience, + expectedApiScopes: TestApiScopes, + expectedScopes: $"{TestApiScopes} openid profile email"); + } + + class Context : ScenarioContext; + } +} diff --git a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 4a1b91b263..1f04237927 100644 --- a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -17,6 +17,8 @@ "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null, + "ServicePulseOfflineAccessScopeEnabled": false, + "ServicePulseScopes": null, "RolesClaim": "roles", "RoleBasedAuthorizationEnabled": false }, diff --git a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs index 058a946168..da7f189b2c 100644 --- a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs +++ b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs @@ -49,6 +49,7 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC ServicePulseClientId = SettingsReader.Read(rootNamespace, "Authentication.ServicePulse.ClientId"); ServicePulseApiScopes = SettingsReader.Read(rootNamespace, "Authentication.ServicePulse.ApiScopes"); ServicePulseAuthority = SettingsReader.Read(rootNamespace, "Authentication.ServicePulse.Authority"); + ServicePulseOfflineAccessScopeEnabled = SettingsReader.Read(rootNamespace, "Authentication.ServicePulse.OfflineAccessScopeEnabled", true); } if (validateConfiguration) @@ -138,6 +139,24 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC /// public string ServicePulseApiScopes { get; } + /// + /// Whether ServicePulse should request the offline_access scope. Defaults to true + /// to preserve existing behaviour. Some identity providers reject authorization requests that + /// include a scope they don't permit, so operators can disable it here rather than have + /// ServicePulse hard-code it into every request. + /// + public bool ServicePulseOfflineAccessScopeEnabled { get; } + + /// + /// The complete, space-separated scope string ServicePulse should request, composed from + /// plus the fixed openid profile email scopes required + /// to establish an OIDC session, and offline_access unless + /// is false. + /// + public string ServicePulseScopes => ServicePulseApiScopes is null + ? null + : $"{ServicePulseApiScopes} openid profile email{(ServicePulseOfflineAccessScopeEnabled ? " offline_access" : "")}"; + /// /// Path within the JWT where the user's role values live. Defaults to the flat roles /// claim, as emitted by Microsoft Entra ID app roles or Keycloak with a "User Realm Role" mapper. @@ -231,9 +250,10 @@ void LogConfiguration(bool requireServicePulseSettings) var servicePulseClientIdDisplay = requireServicePulseSettings ? (ServicePulseClientId ?? "(not configured)") : "(n/a)"; var servicePulseAuthorityDisplay = requireServicePulseSettings ? (ServicePulseAuthority ?? "(not configured)") : "(n/a)"; var servicePulseApiScopesDisplay = requireServicePulseSettings ? (ServicePulseApiScopes ?? "(not configured)") : "(n/a)"; + var servicePulseOfflineAccessScopeEnabledDisplay = requireServicePulseSettings ? ServicePulseOfflineAccessScopeEnabled.ToString() : "(n/a)"; - logger.LogInformation("Authentication settings: Enabled={Enabled}, Authority={Authority}, Audience={Audience}, ValidateIssuer={ValidateIssuer}, ValidateAudience={ValidateAudience}, ValidateLifetime={ValidateLifetime}, ValidateIssuerSigningKey={ValidateIssuerSigningKey}, RequireHttpsMetadata={RequireHttpsMetadata}, RolesClaim={RolesClaim}, SubjectIdClaim={SubjectIdClaim}, SubjectNameClaim={SubjectNameClaim}, ServicePulseClientId={ServicePulseClientId}, ServicePulseAuthority={ServicePulseAuthority}, ServicePulseApiScopes={ServicePulseApiScopes}", - Enabled, authorityDisplay, audienceDisplay, ValidateIssuer, ValidateAudience, ValidateLifetime, ValidateIssuerSigningKey, RequireHttpsMetadata, RolesClaim, SubjectIdClaim, SubjectNameClaim, servicePulseClientIdDisplay, servicePulseAuthorityDisplay, servicePulseApiScopesDisplay); + logger.LogInformation("Authentication settings: Enabled={Enabled}, Authority={Authority}, Audience={Audience}, ValidateIssuer={ValidateIssuer}, ValidateAudience={ValidateAudience}, ValidateLifetime={ValidateLifetime}, ValidateIssuerSigningKey={ValidateIssuerSigningKey}, RequireHttpsMetadata={RequireHttpsMetadata}, RolesClaim={RolesClaim}, SubjectIdClaim={SubjectIdClaim}, SubjectNameClaim={SubjectNameClaim}, ServicePulseClientId={ServicePulseClientId}, ServicePulseAuthority={ServicePulseAuthority}, ServicePulseApiScopes={ServicePulseApiScopes}, ServicePulseOfflineAccessScopeEnabled={ServicePulseOfflineAccessScopeEnabled}", + Enabled, authorityDisplay, audienceDisplay, ValidateIssuer, ValidateAudience, ValidateLifetime, ValidateIssuerSigningKey, RequireHttpsMetadata, RolesClaim, SubjectIdClaim, SubjectNameClaim, servicePulseClientIdDisplay, servicePulseAuthorityDisplay, servicePulseApiScopesDisplay, servicePulseOfflineAccessScopeEnabledDisplay); // Warn about potential misconfigurations var hasAuthConfig = !string.IsNullOrWhiteSpace(Authority) || !string.IsNullOrWhiteSpace(Audience); diff --git a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt index a35b4112e2..b853b75320 100644 --- a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt @@ -17,6 +17,8 @@ "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null, + "ServicePulseOfflineAccessScopeEnabled": false, + "ServicePulseScopes": null, "RolesClaim": "roles", "RoleBasedAuthorizationEnabled": false }, diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 6e7be8475f..13a979c0e0 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -17,6 +17,8 @@ "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null, + "ServicePulseOfflineAccessScopeEnabled": true, + "ServicePulseScopes": null, "RolesClaim": "roles", "RoleBasedAuthorizationEnabled": false }, diff --git a/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs b/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs index d1a588a517..7f1bbd6767 100644 --- a/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs +++ b/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs @@ -33,6 +33,7 @@ public void TearDown() Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID", null); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", null); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_AUTHORITY", null); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED", null); } [Test] @@ -54,6 +55,8 @@ public void Should_have_correct_defaults() Assert.That(settings.ServicePulseClientId, Is.Null); Assert.That(settings.ServicePulseApiScopes, Is.Null); Assert.That(settings.ServicePulseAuthority, Is.Null); + Assert.That(settings.ServicePulseOfflineAccessScopeEnabled, Is.True); + Assert.That(settings.ServicePulseScopes, Is.Null); } } @@ -115,6 +118,35 @@ public void Should_read_service_pulse_settings_when_required() } } + [Test] + public void Should_compose_service_pulse_scopes_with_offline_access_by_default() + { + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "api://my-api/.default"); + + var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: false, requireServicePulseSettings: true); + + using (Assert.EnterMultipleScope()) + { + Assert.That(settings.ServicePulseOfflineAccessScopeEnabled, Is.True); + Assert.That(settings.ServicePulseScopes, Is.EqualTo("api://my-api/.default openid profile email offline_access")); + } + } + + [Test] + public void Should_compose_service_pulse_scopes_without_offline_access_when_disabled() + { + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "api://my-api/.default"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED", "false"); + + var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: false, requireServicePulseSettings: true); + + using (Assert.EnterMultipleScope()) + { + Assert.That(settings.ServicePulseOfflineAccessScopeEnabled, Is.False); + Assert.That(settings.ServicePulseScopes, Is.EqualTo("api://my-api/.default openid profile email")); + } + } + [Test] public void Should_not_read_service_pulse_settings_when_not_required() { diff --git a/src/ServiceControl/App.config b/src/ServiceControl/App.config index 57bd3090f0..a086462123 100644 --- a/src/ServiceControl/App.config +++ b/src/ServiceControl/App.config @@ -45,6 +45,8 @@ These settings are only here so that we can debug ServiceControl while developin + + diff --git a/src/ServiceControl/Authentication/AuthenticationController.cs b/src/ServiceControl/Authentication/AuthenticationController.cs index 9a878f568a..31cea0553a 100644 --- a/src/ServiceControl/Authentication/AuthenticationController.cs +++ b/src/ServiceControl/Authentication/AuthenticationController.cs @@ -21,7 +21,8 @@ public ActionResult Configuration() ClientId = settings.OpenIdConnectSettings.ServicePulseClientId, Authority = settings.OpenIdConnectSettings.ServicePulseAuthority, Audience = settings.OpenIdConnectSettings.Audience, - ApiScopes = settings.OpenIdConnectSettings.ServicePulseApiScopes + ApiScopes = settings.OpenIdConnectSettings.ServicePulseApiScopes, + Scopes = settings.OpenIdConnectSettings.ServicePulseScopes }; return Ok(info); @@ -40,5 +41,14 @@ public class AuthConfig public string Authority { get; set; } public string Audience { get; set; } public string ApiScopes { get; set; } + + /// + /// The complete, space-separated scope string ServicePulse should request, composed by + /// ServiceControl from plus the fixed openid profile email + /// scopes and offline_access unless disabled via + /// Authentication.ServicePulse.OfflineAccessScopeEnabled. Older ServicePulse builds + /// that predate this field ignore it and fall back to assembling their own scope string. + /// + public string Scopes { get; set; } } } From 45486bb5631d029967c7b6a34bb83a1b66d0a7fc Mon Sep 17 00:00:00 2001 From: Warwick Schroeder Date: Fri, 24 Jul 2026 16:41:37 +0800 Subject: [PATCH 2/2] Fix OpenID Connect settings to support JSON array for API scopes and enable offline access by default --- docs/authentication-testing.md | 5 +- .../OpenIdConnectTestConfiguration.cs | 2 +- .../When_authentication_is_enabled.cs | 6 +- ..._pulse_offline_access_scope_is_disabled.cs | 6 +- ...rovals.PlatformSampleSettings.approved.txt | 2 +- .../OpenIdConnectSettings.cs | 69 ++++++++++++++++--- ...sTests.PlatformSampleSettings.approved.txt | 2 +- .../Settings/OpenIdConnectSettingsTests.cs | 49 ++++++++++--- src/ServiceControl/App.config | 2 +- .../AuthenticationController.cs | 9 ++- 10 files changed, 120 insertions(+), 32 deletions(-) diff --git a/docs/authentication-testing.md b/docs/authentication-testing.md index f165e84105..a2f30886e0 100644 --- a/docs/authentication-testing.md +++ b/docs/authentication-testing.md @@ -204,6 +204,7 @@ set SERVICECONTROL_AUTHENTICATION_AUDIENCE=api://servicecontrol-test set SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID=test-client-id set SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_AUTHORITY=https://login.microsoftonline.com/common/v2.0 set SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES=["api://servicecontrol-test/access_as_user"] +set SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED= set SERVICECONTROL_AUTHENTICATION_REQUIREHTTPSMETADATA= set SERVICECONTROL_AUTHENTICATION_VALIDATEISSUER= set SERVICECONTROL_AUTHENTICATION_VALIDATEAUDIENCE= @@ -283,11 +284,11 @@ curl http://localhost:33633/api/authentication/configuration | json "clientId": "test-client-id", "audience": "api://servicecontrol-test", "apiScopes": "[\"api://servicecontrol-test/access_as_user\"]", - "scopes": "[\"api://servicecontrol-test/access_as_user\"] openid profile email offline_access" + "scopes": "api://servicecontrol-test/access_as_user openid profile email offline_access" } ``` -The authentication configuration endpoint is accessible without authentication and returns the configuration that clients need to authenticate. The `authority` field is omitted when `ServicePulse.Authority` is not explicitly set (it defaults to the main Authority for ServicePulse clients). The `audience` field is copied from the `ServiceControl/Authentication.Audience` value. The `scopes` field is the complete scope string ServicePulse should request, composed by ServiceControl from `apiScopes` plus the fixed `openid profile email` scopes and `offline_access` unless `ServiceControl/Authentication.ServicePulse.OfflineAccessScopeEnabled` is set to `false`. +The authentication configuration endpoint is accessible without authentication and returns the configuration that clients need to authenticate. The `authority` field is omitted when `ServicePulse.Authority` is not explicitly set (it defaults to the main Authority for ServicePulse clients). The `audience` field is copied from the `ServiceControl/Authentication.Audience` value. The `apiScopes` field is the raw JSON array as configured. The `scopes` field is the complete, space-separated scope string ServicePulse should request, composed by ServiceControl by parsing the `apiScopes` JSON array and adding the fixed `openid profile email` scopes plus `offline_access` unless `ServiceControl/Authentication.ServicePulse.OfflineAccessScopeEnabled` is set to `false`. ### Scenario 3: Authentication with Invalid Token diff --git a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs index ace29cdced..9e105a6983 100644 --- a/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs +++ b/src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs @@ -141,7 +141,7 @@ public OpenIdConnectTestConfiguration WithServicePulseClientId(string clientId) /// Configures the API scopes that ServicePulse should request. /// Required on the primary ServiceControl instance when authentication is enabled. /// - /// Space-separated list of API scopes + /// JSON array of API scopes (e.g. ["api://my-api/access_as_user"]) public OpenIdConnectTestConfiguration WithServicePulseApiScopes(string scopes) { SetEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_APISCOPES", scopes); diff --git a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs index 4eca6f5fe8..a52077657f 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_authentication_is_enabled.cs @@ -24,7 +24,9 @@ class When_authentication_is_enabled : AcceptanceTest const string TestAudience = "api://test-audience"; const string TestClientId = "test-client-id"; - const string TestApiScopes = "api://test-audience/.default"; + const string TestApiScope = "api://test-audience/.default"; + // ApiScopes is configured as a JSON array (the format ServicePulse parses) + const string TestApiScopes = "[\"api://test-audience/.default\"]"; [SetUp] public void ConfigureAuth() @@ -75,7 +77,7 @@ await OpenIdConnectAssertions.AssertAuthConfigurationResponse( expectedClientId: TestClientId, expectedAudience: TestAudience, expectedApiScopes: TestApiScopes, - expectedScopes: $"{TestApiScopes} openid profile email offline_access", + expectedScopes: $"{TestApiScope} openid profile email offline_access", expectedRoleBasedAuthorizationEnabled: true); } diff --git a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_service_pulse_offline_access_scope_is_disabled.cs b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_service_pulse_offline_access_scope_is_disabled.cs index 42c0a7dc67..60231ce655 100644 --- a/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_service_pulse_offline_access_scope_is_disabled.cs +++ b/src/ServiceControl.AcceptanceTests/Security/OpenIdConnect/When_service_pulse_offline_access_scope_is_disabled.cs @@ -20,7 +20,9 @@ class When_service_pulse_offline_access_scope_is_disabled : AcceptanceTest const string TestAuthority = "https://login.example.com/tenant-id/v2.0"; const string TestAudience = "api://test-audience"; const string TestClientId = "test-client-id"; - const string TestApiScopes = "api://test-audience/.default"; + const string TestApiScope = "api://test-audience/.default"; + // ApiScopes is configured as a JSON array (the format ServicePulse parses) + const string TestApiScopes = "[\"api://test-audience/.default\"]"; [SetUp] public void ConfigureAuth() => @@ -59,7 +61,7 @@ await OpenIdConnectAssertions.AssertAuthConfigurationResponse( expectedClientId: TestClientId, expectedAudience: TestAudience, expectedApiScopes: TestApiScopes, - expectedScopes: $"{TestApiScopes} openid profile email"); + expectedScopes: $"{TestApiScope} openid profile email"); } class Context : ScenarioContext; diff --git a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt index 1f04237927..cb9fa30f77 100644 --- a/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt @@ -17,7 +17,7 @@ "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null, - "ServicePulseOfflineAccessScopeEnabled": false, + "ServicePulseOfflineAccessScopeEnabled": true, "ServicePulseScopes": null, "RolesClaim": "roles", "RoleBasedAuthorizationEnabled": false diff --git a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs index da7f189b2c..81f6080836 100644 --- a/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs +++ b/src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs @@ -1,6 +1,7 @@ namespace ServiceControl.Infrastructure; using System; +using System.Text.Json; using Microsoft.Extensions.Logging; using ServiceControl.Configuration; @@ -134,7 +135,8 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC public string ServicePulseClientId { get; } /// - /// Space-separated list of API scopes that ServicePulse should request during authentication. + /// JSON array of API scopes that ServicePulse should request during authentication + /// (e.g. ["api://my-api/access_as_user"]) — the format ServicePulse parses. /// Required on the primary ServiceControl instance when authentication is enabled. /// public string ServicePulseApiScopes { get; } @@ -145,17 +147,28 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC /// include a scope they don't permit, so operators can disable it here rather than have /// ServicePulse hard-code it into every request. /// - public bool ServicePulseOfflineAccessScopeEnabled { get; } + public bool ServicePulseOfflineAccessScopeEnabled { get; } = true; /// - /// The complete, space-separated scope string ServicePulse should request, composed from - /// plus the fixed openid profile email scopes required - /// to establish an OIDC session, and offline_access unless - /// is false. + /// The complete, space-separated scope string ServicePulse should request, composed by parsing the + /// JSON array and appending the fixed openid profile email + /// scopes required to establish an OIDC session, plus offline_access unless + /// is false. Returns null when no + /// API scopes are configured (e.g. on non-primary instances). /// - public string ServicePulseScopes => ServicePulseApiScopes is null - ? null - : $"{ServicePulseApiScopes} openid profile email{(ServicePulseOfflineAccessScopeEnabled ? " offline_access" : "")}"; + public string ServicePulseScopes + { + get + { + if (!TryParseApiScopes(ServicePulseApiScopes, out var apiScopes)) + { + return null; + } + + var offlineAccessScope = ServicePulseOfflineAccessScopeEnabled ? " offline_access" : ""; + return $"{apiScopes} openid profile email{offlineAccessScope}"; + } + } /// /// Path within the JWT where the user's role values live. Defaults to the flat roles @@ -234,6 +247,13 @@ void ValidateRequiredSettings(bool requireServicePulseSettings) throw new Exception(message); } + if (!TryParseApiScopes(ServicePulseApiScopes, out _)) + { + var message = $"Authentication.ServicePulse.ApiScopes must be a non-empty JSON array of scope strings (e.g. [\"api://my-api/access_as_user\"]). Current value: '{ServicePulseApiScopes}'"; + logger.LogCritical(message); + throw new Exception(message); + } + if (ServicePulseAuthority != null && !Uri.TryCreate(ServicePulseAuthority, UriKind.Absolute, out _)) { var message = $"Authentication.ServicePulse.Authority must be a valid absolute URI. Current value: '{ServicePulseAuthority}'"; @@ -243,6 +263,37 @@ void ValidateRequiredSettings(bool requireServicePulseSettings) } } + /// + /// Parses the ServicePulse.ApiScopes setting. A JSON array of scope strings, the format + /// ServicePulse expects, into a single space-separated scope string. Returns false for a + /// null/blank, malformed, or empty value. + /// + static bool TryParseApiScopes(string apiScopes, out string spaceSeparatedScopes) + { + spaceSeparatedScopes = null; + + if (string.IsNullOrWhiteSpace(apiScopes)) + { + return false; + } + + try + { + var scopes = JsonSerializer.Deserialize(apiScopes); + if (scopes is null || scopes.Length == 0) + { + return false; + } + + spaceSeparatedScopes = string.Join(' ', scopes); + return true; + } + catch (JsonException) + { + return false; + } + } + void LogConfiguration(bool requireServicePulseSettings) { var authorityDisplay = Authority ?? "(not configured)"; diff --git a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt index b853b75320..b8f0237609 100644 --- a/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt +++ b/src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt @@ -17,7 +17,7 @@ "ServicePulseAuthority": null, "ServicePulseClientId": null, "ServicePulseApiScopes": null, - "ServicePulseOfflineAccessScopeEnabled": false, + "ServicePulseOfflineAccessScopeEnabled": true, "ServicePulseScopes": null, "RolesClaim": "roles", "RoleBasedAuthorizationEnabled": false diff --git a/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs b/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs index 7f1bbd6767..a3e497415a 100644 --- a/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs +++ b/src/ServiceControl.UnitTests/Infrastructure/Settings/OpenIdConnectSettingsTests.cs @@ -105,7 +105,7 @@ public void Should_read_validation_flags() public void Should_read_service_pulse_settings_when_required() { Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID", "my-client-id"); - Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "api://my-api/.default"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "[\"api://my-api/.default\"]"); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_AUTHORITY", "https://pulse-auth.example.com"); var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: false, requireServicePulseSettings: true); @@ -113,7 +113,7 @@ public void Should_read_service_pulse_settings_when_required() using (Assert.EnterMultipleScope()) { Assert.That(settings.ServicePulseClientId, Is.EqualTo("my-client-id")); - Assert.That(settings.ServicePulseApiScopes, Is.EqualTo("api://my-api/.default")); + Assert.That(settings.ServicePulseApiScopes, Is.EqualTo("[\"api://my-api/.default\"]")); Assert.That(settings.ServicePulseAuthority, Is.EqualTo("https://pulse-auth.example.com")); } } @@ -121,7 +121,7 @@ public void Should_read_service_pulse_settings_when_required() [Test] public void Should_compose_service_pulse_scopes_with_offline_access_by_default() { - Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "api://my-api/.default"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "[\"api://my-api/.default\"]"); var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: false, requireServicePulseSettings: true); @@ -135,7 +135,7 @@ public void Should_compose_service_pulse_scopes_with_offline_access_by_default() [Test] public void Should_compose_service_pulse_scopes_without_offline_access_when_disabled() { - Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "api://my-api/.default"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "[\"api://my-api/.default\"]"); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED", "false"); var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: false, requireServicePulseSettings: true); @@ -147,11 +147,24 @@ public void Should_compose_service_pulse_scopes_without_offline_access_when_disa } } + [Test] + public void Should_compose_service_pulse_scopes_from_json_array_with_multiple_scopes() + { + // ApiScopes is a JSON array (the format ServicePulse parses); the composed scope string + // must be the space-separated scopes, not the raw JSON array text. + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "[\"api://my-api/scope1\", \"api://my-api/scope2\"]"); + + var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: false, requireServicePulseSettings: true); + + Assert.That(settings.ServicePulseScopes, Is.EqualTo("api://my-api/scope1 api://my-api/scope2 openid profile email offline_access")); + } + [Test] public void Should_not_read_service_pulse_settings_when_not_required() { Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID", "my-client-id"); - Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "api://my-api/.default"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "[\"api://my-api/.default\"]"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED", "false"); var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: false, requireServicePulseSettings: false); @@ -159,6 +172,11 @@ public void Should_not_read_service_pulse_settings_when_not_required() { Assert.That(settings.ServicePulseClientId, Is.Null); Assert.That(settings.ServicePulseApiScopes, Is.Null); + // The offline_access toggle is only read on the primary instance. When ServicePulse + // settings aren't required the env var is ignored and the property keeps its default, + // so ServicePulseScopes has no API scopes to compose from and stays null. + Assert.That(settings.ServicePulseOfflineAccessScopeEnabled, Is.True); + Assert.That(settings.ServicePulseScopes, Is.Null); } } @@ -247,13 +265,28 @@ public void Should_throw_when_service_pulse_api_scopes_missing() } [Test] - public void Should_throw_when_service_pulse_authority_is_invalid() + public void Should_throw_when_service_pulse_api_scopes_not_a_json_array() { Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_ENABLED", "true"); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_AUTHORITY", "https://login.example.com"); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_AUDIENCE", "my-audience"); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID", "my-client-id"); + // A bare scope string rather than the required JSON array (e.g. ["api://my-api/.default"]) Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "api://my-api/.default"); + + var ex = Assert.Throws(() => new OpenIdConnectSettings(TestNamespace, validateConfiguration: true, requireServicePulseSettings: true)); + + Assert.That(ex.Message, Does.Contain("ServicePulse.ApiScopes must be a non-empty JSON array")); + } + + [Test] + public void Should_throw_when_service_pulse_authority_is_invalid() + { + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_ENABLED", "true"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_AUTHORITY", "https://login.example.com"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_AUDIENCE", "my-audience"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID", "my-client-id"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "[\"api://my-api/.default\"]"); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_AUTHORITY", "not-a-valid-uri"); var ex = Assert.Throws(() => new OpenIdConnectSettings(TestNamespace, validateConfiguration: true, requireServicePulseSettings: true)); @@ -268,7 +301,7 @@ public void Should_succeed_with_valid_full_configuration() Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_AUTHORITY", "https://login.example.com"); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_AUDIENCE", "my-audience"); Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID", "my-client-id"); - Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "api://my-api/.default"); + Environment.SetEnvironmentVariable("SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES", "[\"api://my-api/.default\"]"); var settings = new OpenIdConnectSettings(TestNamespace, validateConfiguration: true, requireServicePulseSettings: true); @@ -278,7 +311,7 @@ public void Should_succeed_with_valid_full_configuration() Assert.That(settings.Authority, Is.EqualTo("https://login.example.com")); Assert.That(settings.Audience, Is.EqualTo("my-audience")); Assert.That(settings.ServicePulseClientId, Is.EqualTo("my-client-id")); - Assert.That(settings.ServicePulseApiScopes, Is.EqualTo("api://my-api/.default")); + Assert.That(settings.ServicePulseApiScopes, Is.EqualTo("[\"api://my-api/.default\"]")); } } diff --git a/src/ServiceControl/App.config b/src/ServiceControl/App.config index a086462123..cf671edde0 100644 --- a/src/ServiceControl/App.config +++ b/src/ServiceControl/App.config @@ -45,7 +45,7 @@ These settings are only here so that we can debug ServiceControl while developin - + diff --git a/src/ServiceControl/Authentication/AuthenticationController.cs b/src/ServiceControl/Authentication/AuthenticationController.cs index 31cea0553a..c6f3cecc3d 100644 --- a/src/ServiceControl/Authentication/AuthenticationController.cs +++ b/src/ServiceControl/Authentication/AuthenticationController.cs @@ -43,11 +43,10 @@ public class AuthConfig public string ApiScopes { get; set; } /// - /// The complete, space-separated scope string ServicePulse should request, composed by - /// ServiceControl from plus the fixed openid profile email - /// scopes and offline_access unless disabled via - /// Authentication.ServicePulse.OfflineAccessScopeEnabled. Older ServicePulse builds - /// that predate this field ignore it and fall back to assembling their own scope string. + /// The complete, space-separated scope string ServicePulse should request. Carries the value of + /// OpenIdConnectSettings.ServicePulseScopes (see there for how it is composed). Added as an + /// additive, non-breaking field; ServicePulse builds that don't recognize it fall back to + /// composing the scope string themselves from . /// public string Scopes { get; set; } }