Skip to content

Commit 5cbf25a

Browse files
Allow for a missing offline_access scope (#5643)
* Allow for a missing offline_access scope * Fix OpenID Connect settings to support JSON array for API scopes and enable offline access by default --------- Co-authored-by: Warwick Schroeder <warwick.schroeder@particular.net>
1 parent 5619e94 commit 5cbf25a

12 files changed

Lines changed: 264 additions & 14 deletions

File tree

docs/authentication-testing.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ set SERVICECONTROL_AUTHENTICATION_AUDIENCE=api://servicecontrol-test
204204
set SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_CLIENTID=test-client-id
205205
set SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_AUTHORITY=https://login.microsoftonline.com/common/v2.0
206206
set SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_APISCOPES=["api://servicecontrol-test/access_as_user"]
207+
set SERVICECONTROL_AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED=
207208
set SERVICECONTROL_AUTHENTICATION_REQUIREHTTPSMETADATA=
208209
set SERVICECONTROL_AUTHENTICATION_VALIDATEISSUER=
209210
set SERVICECONTROL_AUTHENTICATION_VALIDATEAUDIENCE=
@@ -282,11 +283,12 @@ curl http://localhost:33633/api/authentication/configuration | json
282283
"enabled": true,
283284
"clientId": "test-client-id",
284285
"audience": "api://servicecontrol-test",
285-
"apiScopes": "[\"api://servicecontrol-test/access_as_user\"]"
286+
"apiScopes": "[\"api://servicecontrol-test/access_as_user\"]",
287+
"scopes": "api://servicecontrol-test/access_as_user openid profile email offline_access"
286288
}
287289
```
288290

289-
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.
291+
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`.
290292

291293
### Scenario 3: Authentication with Invalid Token
292294

src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectAssertions.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ public static async Task AssertAuthConfigurationResponse(
106106
string expectedAuthority = null,
107107
string expectedAudience = null,
108108
string expectedApiScopes = null,
109+
string expectedScopes = null,
109110
bool expectedRoleBasedAuthorizationEnabled = false)
110111
{
111112
Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.OK),
@@ -172,6 +173,17 @@ public static async Task AssertAuthConfigurationResponse(
172173
$"'api_scopes' should be '{expectedApiScopes}'");
173174
}
174175
}
176+
177+
if (expectedScopes != null)
178+
{
179+
using (Assert.EnterMultipleScope())
180+
{
181+
Assert.That(root.TryGetProperty("scopes", out var scopesProperty), Is.True,
182+
"Response should contain 'scopes' property");
183+
Assert.That(scopesProperty.GetString(), Is.EqualTo(expectedScopes),
184+
$"'scopes' should be '{expectedScopes}'");
185+
}
186+
}
175187
}
176188

177189
/// <summary>

src/ServiceControl.AcceptanceTesting/OpenIdConnect/OpenIdConnectTestConfiguration.cs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ public OpenIdConnectTestConfiguration WithServicePulseClientId(string clientId)
141141
/// Configures the API scopes that ServicePulse should request.
142142
/// Required on the primary ServiceControl instance when authentication is enabled.
143143
/// </summary>
144-
/// <param name="scopes">Space-separated list of API scopes</param>
144+
/// <param name="scopes">JSON array of API scopes (e.g. <c>["api://my-api/access_as_user"]</c>)</param>
145145
public OpenIdConnectTestConfiguration WithServicePulseApiScopes(string scopes)
146146
{
147147
SetEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_APISCOPES", scopes);
@@ -159,6 +159,16 @@ public OpenIdConnectTestConfiguration WithServicePulseAuthority(string authority
159159
return this;
160160
}
161161

162+
/// <summary>
163+
/// Configures whether ServicePulse should request the <c>offline_access</c> scope.
164+
/// Default is true. Set to false to simulate an identity provider that disallows the scope.
165+
/// </summary>
166+
public OpenIdConnectTestConfiguration WithServicePulseOfflineAccessScopeEnabled(bool enabled)
167+
{
168+
SetEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED", enabled.ToString().ToLowerInvariant());
169+
return this;
170+
}
171+
162172
/// <summary>
163173
/// Clears all OpenID Connect environment variables.
164174
/// Called automatically on Dispose.
@@ -176,6 +186,7 @@ public void ClearConfiguration()
176186
ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_CLIENTID");
177187
ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_APISCOPES");
178188
ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_AUTHORITY");
189+
ClearEnvironmentVariable("AUTHENTICATION_SERVICEPULSE_OFFLINEACCESSSCOPEENABLED");
179190
ClearEnvironmentVariable("AUTHENTICATION_ROLEBASEDAUTHORIZATIONENABLED");
180191
ClearEnvironmentVariable("VALIDATECONFIG");
181192
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ class When_authentication_is_enabled : AcceptanceTest
2424

2525
const string TestAudience = "api://test-audience";
2626
const string TestClientId = "test-client-id";
27-
const string TestApiScopes = "api://test-audience/.default";
27+
const string TestApiScope = "api://test-audience/.default";
28+
// ApiScopes is configured as a JSON array (the format ServicePulse parses)
29+
const string TestApiScopes = "[\"api://test-audience/.default\"]";
2830

2931
[SetUp]
3032
public void ConfigureAuth()
@@ -75,6 +77,7 @@ await OpenIdConnectAssertions.AssertAuthConfigurationResponse(
7577
expectedClientId: TestClientId,
7678
expectedAudience: TestAudience,
7779
expectedApiScopes: TestApiScopes,
80+
expectedScopes: $"{TestApiScope} openid profile email offline_access",
7881
expectedRoleBasedAuthorizationEnabled: true);
7982
}
8083

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
namespace ServiceControl.AcceptanceTests.Security.OpenIdConnect
2+
{
3+
using System.Net.Http;
4+
using System.Threading.Tasks;
5+
using AcceptanceTesting;
6+
using AcceptanceTesting.OpenIdConnect;
7+
using NServiceBus.AcceptanceTesting;
8+
using NUnit.Framework;
9+
10+
/// <summary>
11+
/// ServicePulse Offline Access Scope Opt-Out
12+
/// When Authentication.ServicePulse.OfflineAccessScopeEnabled is false, the composed scope
13+
/// string returned by the authentication configuration endpoint should omit offline_access,
14+
/// so ServicePulse does not request a scope the identity provider disallows.
15+
/// </summary>
16+
class When_service_pulse_offline_access_scope_is_disabled : AcceptanceTest
17+
{
18+
OpenIdConnectTestConfiguration configuration;
19+
20+
const string TestAuthority = "https://login.example.com/tenant-id/v2.0";
21+
const string TestAudience = "api://test-audience";
22+
const string TestClientId = "test-client-id";
23+
const string TestApiScope = "api://test-audience/.default";
24+
// ApiScopes is configured as a JSON array (the format ServicePulse parses)
25+
const string TestApiScopes = "[\"api://test-audience/.default\"]";
26+
27+
[SetUp]
28+
public void ConfigureAuth() =>
29+
configuration = new OpenIdConnectTestConfiguration(ServiceControlInstanceType.Primary)
30+
.WithConfigurationValidationDisabled()
31+
.WithAuthenticationEnabled()
32+
.WithAuthority(TestAuthority)
33+
.WithAudience(TestAudience)
34+
.WithServicePulseClientId(TestClientId)
35+
.WithServicePulseApiScopes(TestApiScopes)
36+
.WithServicePulseOfflineAccessScopeEnabled(false)
37+
.WithRequireHttpsMetadata(false);
38+
39+
[TearDown]
40+
public void CleanupAuth() => configuration?.Dispose();
41+
42+
[Test]
43+
public async Task Should_omit_offline_access_from_composed_scopes()
44+
{
45+
HttpResponseMessage response = null;
46+
47+
_ = await Define<Context>()
48+
.Done(async ctx =>
49+
{
50+
response = await OpenIdConnectAssertions.SendRequestWithoutAuth(
51+
HttpClient,
52+
HttpMethod.Get,
53+
"/api/authentication/configuration");
54+
return response != null;
55+
})
56+
.Run();
57+
58+
await OpenIdConnectAssertions.AssertAuthConfigurationResponse(
59+
response,
60+
expectedEnabled: true,
61+
expectedClientId: TestClientId,
62+
expectedAudience: TestAudience,
63+
expectedApiScopes: TestApiScopes,
64+
expectedScopes: $"{TestApiScope} openid profile email");
65+
}
66+
67+
class Context : ScenarioContext;
68+
}
69+
}

src/ServiceControl.Audit.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
"ServicePulseAuthority": null,
1818
"ServicePulseClientId": null,
1919
"ServicePulseApiScopes": null,
20+
"ServicePulseOfflineAccessScopeEnabled": true,
21+
"ServicePulseScopes": null,
2022
"RolesClaim": "roles",
2123
"RoleBasedAuthorizationEnabled": false
2224
},

src/ServiceControl.Infrastructure/OpenIdConnectSettings.cs

Lines changed: 74 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
namespace ServiceControl.Infrastructure;
22

33
using System;
4+
using System.Text.Json;
45
using Microsoft.Extensions.Logging;
56
using ServiceControl.Configuration;
67

@@ -49,6 +50,7 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC
4950
ServicePulseClientId = SettingsReader.Read<string>(rootNamespace, "Authentication.ServicePulse.ClientId");
5051
ServicePulseApiScopes = SettingsReader.Read<string>(rootNamespace, "Authentication.ServicePulse.ApiScopes");
5152
ServicePulseAuthority = SettingsReader.Read<string>(rootNamespace, "Authentication.ServicePulse.Authority");
53+
ServicePulseOfflineAccessScopeEnabled = SettingsReader.Read(rootNamespace, "Authentication.ServicePulse.OfflineAccessScopeEnabled", true);
5254
}
5355

5456
if (validateConfiguration)
@@ -133,11 +135,41 @@ public OpenIdConnectSettings(SettingsRootNamespace rootNamespace, bool validateC
133135
public string ServicePulseClientId { get; }
134136

135137
/// <summary>
136-
/// Space-separated list of API scopes that ServicePulse should request during authentication.
138+
/// JSON array of API scopes that ServicePulse should request during authentication
139+
/// (e.g. <c>["api://my-api/access_as_user"]</c>) — the format ServicePulse parses.
137140
/// Required on the primary ServiceControl instance when authentication is enabled.
138141
/// </summary>
139142
public string ServicePulseApiScopes { get; }
140143

144+
/// <summary>
145+
/// Whether ServicePulse should request the <c>offline_access</c> scope. Defaults to <c>true</c>
146+
/// to preserve existing behaviour. Some identity providers reject authorization requests that
147+
/// include a scope they don't permit, so operators can disable it here rather than have
148+
/// ServicePulse hard-code it into every request.
149+
/// </summary>
150+
public bool ServicePulseOfflineAccessScopeEnabled { get; } = true;
151+
152+
/// <summary>
153+
/// The complete, space-separated scope string ServicePulse should request, composed by parsing the
154+
/// <see cref="ServicePulseApiScopes"/> JSON array and appending the fixed <c>openid profile email</c>
155+
/// scopes required to establish an OIDC session, plus <c>offline_access</c> unless
156+
/// <see cref="ServicePulseOfflineAccessScopeEnabled"/> is <c>false</c>. Returns <c>null</c> when no
157+
/// API scopes are configured (e.g. on non-primary instances).
158+
/// </summary>
159+
public string ServicePulseScopes
160+
{
161+
get
162+
{
163+
if (!TryParseApiScopes(ServicePulseApiScopes, out var apiScopes))
164+
{
165+
return null;
166+
}
167+
168+
var offlineAccessScope = ServicePulseOfflineAccessScopeEnabled ? " offline_access" : "";
169+
return $"{apiScopes} openid profile email{offlineAccessScope}";
170+
}
171+
}
172+
141173
/// <summary>
142174
/// Path within the JWT where the user's role values live. Defaults to the flat <c>roles</c>
143175
/// claim, as emitted by Microsoft Entra ID app roles or Keycloak with a "User Realm Role" mapper.
@@ -215,6 +247,13 @@ void ValidateRequiredSettings(bool requireServicePulseSettings)
215247
throw new Exception(message);
216248
}
217249

250+
if (!TryParseApiScopes(ServicePulseApiScopes, out _))
251+
{
252+
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}'";
253+
logger.LogCritical(message);
254+
throw new Exception(message);
255+
}
256+
218257
if (ServicePulseAuthority != null && !Uri.TryCreate(ServicePulseAuthority, UriKind.Absolute, out _))
219258
{
220259
var message = $"Authentication.ServicePulse.Authority must be a valid absolute URI. Current value: '{ServicePulseAuthority}'";
@@ -224,16 +263,48 @@ void ValidateRequiredSettings(bool requireServicePulseSettings)
224263
}
225264
}
226265

266+
/// <summary>
267+
/// Parses the <c>ServicePulse.ApiScopes</c> setting. A JSON array of scope strings, the format
268+
/// ServicePulse expects, into a single space-separated scope string. Returns <c>false</c> for a
269+
/// null/blank, malformed, or empty value.
270+
/// </summary>
271+
static bool TryParseApiScopes(string apiScopes, out string spaceSeparatedScopes)
272+
{
273+
spaceSeparatedScopes = null;
274+
275+
if (string.IsNullOrWhiteSpace(apiScopes))
276+
{
277+
return false;
278+
}
279+
280+
try
281+
{
282+
var scopes = JsonSerializer.Deserialize<string[]>(apiScopes);
283+
if (scopes is null || scopes.Length == 0)
284+
{
285+
return false;
286+
}
287+
288+
spaceSeparatedScopes = string.Join(' ', scopes);
289+
return true;
290+
}
291+
catch (JsonException)
292+
{
293+
return false;
294+
}
295+
}
296+
227297
void LogConfiguration(bool requireServicePulseSettings)
228298
{
229299
var authorityDisplay = Authority ?? "(not configured)";
230300
var audienceDisplay = Audience ?? "(not configured)";
231301
var servicePulseClientIdDisplay = requireServicePulseSettings ? (ServicePulseClientId ?? "(not configured)") : "(n/a)";
232302
var servicePulseAuthorityDisplay = requireServicePulseSettings ? (ServicePulseAuthority ?? "(not configured)") : "(n/a)";
233303
var servicePulseApiScopesDisplay = requireServicePulseSettings ? (ServicePulseApiScopes ?? "(not configured)") : "(n/a)";
304+
var servicePulseOfflineAccessScopeEnabledDisplay = requireServicePulseSettings ? ServicePulseOfflineAccessScopeEnabled.ToString() : "(n/a)";
234305

235-
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}",
236-
Enabled, authorityDisplay, audienceDisplay, ValidateIssuer, ValidateAudience, ValidateLifetime, ValidateIssuerSigningKey, RequireHttpsMetadata, RolesClaim, SubjectIdClaim, SubjectNameClaim, servicePulseClientIdDisplay, servicePulseAuthorityDisplay, servicePulseApiScopesDisplay);
306+
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}",
307+
Enabled, authorityDisplay, audienceDisplay, ValidateIssuer, ValidateAudience, ValidateLifetime, ValidateIssuerSigningKey, RequireHttpsMetadata, RolesClaim, SubjectIdClaim, SubjectNameClaim, servicePulseClientIdDisplay, servicePulseAuthorityDisplay, servicePulseApiScopesDisplay, servicePulseOfflineAccessScopeEnabledDisplay);
237308

238309
// Warn about potential misconfigurations
239310
var hasAuthConfig = !string.IsNullOrWhiteSpace(Authority) || !string.IsNullOrWhiteSpace(Audience);

src/ServiceControl.Monitoring.UnitTests/ApprovalFiles/SettingsTests.PlatformSampleSettings.approved.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
"ServicePulseAuthority": null,
1818
"ServicePulseClientId": null,
1919
"ServicePulseApiScopes": null,
20+
"ServicePulseOfflineAccessScopeEnabled": true,
21+
"ServicePulseScopes": null,
2022
"RolesClaim": "roles",
2123
"RoleBasedAuthorizationEnabled": false
2224
},

src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.PlatformSampleSettings.approved.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
"ServicePulseAuthority": null,
1818
"ServicePulseClientId": null,
1919
"ServicePulseApiScopes": null,
20+
"ServicePulseOfflineAccessScopeEnabled": true,
21+
"ServicePulseScopes": null,
2022
"RolesClaim": "roles",
2123
"RoleBasedAuthorizationEnabled": false
2224
},

0 commit comments

Comments
 (0)