From c1ca13d0290f345b9b39bbcb3c669136331798a0 Mon Sep 17 00:00:00 2001 From: williambza Date: Thu, 23 Jul 2026 12:51:31 +0200 Subject: [PATCH] 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; } } }