SMART on FHIR Token Introspection Endpoint

[mikaelweave]

Description

Implements RFC 7662 token introspection endpoint at /connect/introspect for SMART on FHIR server with swapple support for the introspection endpoint for alternate SMART configurations.

Key Features:

• RFC 7662 compliant introspection endpoint supporting both OpenIddict (development) and external IdP tokens
• Service abstraction pattern (ITokenIntrospectionService) enabling alternate authentication patterns
• SMART on FHIR v1/v2 claim support (patient, fhirUser, raw_scope)
• Bearer token authentication required via [Authorize] attribute
• Integrated with existing audit logging infrastructure
• Handles issuer variations (with/without trailing slash) for OpenIddict compatibility

Related issues

Addresses AB#174822

Testing

• Unit Tests (12 tests - all passing):
• Manual Testing

Test Coverage:

• Controller layer: Parameter validation, HTTP status codes
• Service layer: Token parsing, validation, claim extraction
• Integration: Audit logging, authentication filters

FHIR Team Checklist

• Update the title of the PR to be succinct and less than 65 characters
• Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
• Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
• Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
• Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
• When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
• CI is green before merge
• Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

To fix the problem, we should ensure that the RSA instance created at line 144 is properly disposed after use. The best approach is to use a using statement (or in modern C# a using declaration) to wrap the lifetime of differentRsa so it will be disposed of automatically after we're finished with it. This requires wrapping the block of code where differentRsa is used (lines 144–162) inside a using block or using declaration. This change will be localized to the test method GivenInvalidSignatureToken_WhenIntrospect_ThenReturnsInactive() and does not affect the functionality of the test.

---

[mikaelweave]

Need to look at an implementation here using DI and not typing to new

The best and most reliable way to fix the missing Dispose call on the local HttpRequestMessage is to wrap its construction and usage in a using block. This ensures that, regardless of exceptions or early returns, the object's resources are correctly disposed of. In the context of async code and C# 8.0 and above, using is supported with local variables even when the body of the using includes await expressions.

Within the method GivenMissingTokenParameter_WhenIntrospecting_ThenReturnsBadRequest, modify lines 177-184 to enclose the declaration of request and the following lines which require it within a using statement. This will involve placing a using declaration for the HttpRequestMessage and adjusting the code so the disposable is cleaned up after the async call, but before assertions that access only the response values.

No new imports are needed, and no method signatures need to be changed.

To fix the issue, we need to ensure that the HttpRequestMessage created on line 226, as well as its associated content (the StringContent object created on line 225), are properly disposed after use. The best and least intrusive way to do this is to wrap the creation and usage of both objects in a using statement. Since the request and content are both disposable and their scope is limited to this method, we can use nested or a single combined using declaration (C# 8.0 and above). This can be achieved by replacing the variable declarations and usage block with a using block that ensures disposal after the request has been sent and the assertion is performed. This approach maintains existing functionality and only affects the resource management pattern within the relevant test method.

To fix this problem, we need to ensure that the FormUrlEncodedContent instance created in GetAccessTokenAsync is disposed of properly—i.e., its Dispose method is invoked when it is no longer needed. The most concise and idiomatic way to do this is to wrap its usage in a using statement.

Specifically, in GetAccessTokenAsync, wrap the section of code that uses content from its creation at line 269 to the point where it is passed to _httpClient.PostAsync (and ensure that we do not use content beyond that point). Since PostAsync does not require content to be alive after invocation, we can make the using block encompass the call to PostAsync. In async code, this means using the C# 8.0 using declaration or otherwise nesting a regular using block. Since all usages are within the method and are awaited before further usage, this is straightforward.

No new imports or other changes are required beyond converting the code to use a using block for FormUrlEncodedContent inside GetAccessTokenAsync.

The best way to fix this issue is to wrap the creation and use of the HttpRequestMessage object in a using statement. This ensures that it will be disposed of as soon as it is no longer needed, even if exceptions occur. In this context, since the code creates the request, sends it via HttpClient.SendAsync, and returns the corresponding HttpResponseMessage, we must ensure that disposal happens after the send. However, since the method returns the HttpResponseMessage, not the request, using an async using block will suffice.

To implement the fix, in IntrospectTokenAsync, lines 295–302, replace the direct creation and sending of the request with a using block. This change does not require new imports, as using and IDisposable are built-in to .NET.

The best way to fix this issue is to replace the check for response.ContainsKey("exp") and subsequent access to response["exp"] with a single call to response.TryGetValue("exp", out JsonElement expElement). This only performs one dictionary lookup and is more efficient. The assertion should then use expElement.GetInt64(). The change should be made in the method GivenValidToken_WhenIntrospecting_ThenReturnsActiveWithStandardClaims in lines 63 and 67 of test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/TokenIntrospectionTests.cs. You may need to add a local variable for expElement. No new imports or external methods are required.

To fix this performance issue, replace the existing check:

if (response.ContainsKey("iat"))
{
    Assert.True(response["iat"].GetInt64() > 0);
}

with a TryGetValue-based idiom, which combines both the existence check and the access into a single operation. Specifically, declare a variable for the out value, use response.TryGetValue("iat", out var iatValue), and in the body use iatValue.GetInt64(). This change should be made directly in the test method, at the exact code location, keeping naming conventions and context identical.

No additional imports or helper methods are necessary, as TryGetValue is a method on Dictionary<TKey, TValue>.

To fix the problem, replace the use of response.ContainsKey("scope") followed by repeated dictionary lookups with a single call to response.TryGetValue("scope", out var scopeElement). Specifically, in the affected test method, replace the if block:

if (response.ContainsKey("scope"))
{
    Assert.Equal(JsonValueKind.String, response["scope"].ValueKind);
    var scope = response["scope"].GetString();
    Assert.NotEmpty(scope);
}

with:

if (response.TryGetValue("scope", out var scopeElement))
{
    Assert.Equal(JsonValueKind.String, scopeElement.ValueKind);
    var scope = scopeElement.GetString();
    Assert.NotEmpty(scope);
}

This change ensures that the key existence check and retrieval are combined efficiently, without changing functionality. No new methods or imports are necessary, as all required functionality is part of the standard library.

Make the edit only in the relevant section of the file test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/TokenIntrospectionTests.cs, and ensure any usages of response["scope"] in this scope are changed to use the local variable for correctness and efficiency.

To fix the problem, on lines 114-116 in TokenIntrospectionTests.cs, replace the use of ContainsKey followed by accessing via the indexer with a single call to TryGetValue. This involves declaring a local JsonElement variable to store the value, and then using it if the key existed. The scope of the fix only encompasses these lines, and no additional imports or helper methods are needed.

---

To fix the inefficiency, we replace the two operations on the dictionary (response.ContainsKey("active") and then accessing the value via response["active"]) in the test method with a single TryGetValue call. Specifically, we should use:

Assert.True(response.TryGetValue("active", out JsonElement activeValue));
Assert.False(activeValue.GetBoolean());

This maintains the two assertions, but avoids the double lookup, combining them for efficiency. No other changes to functionality are needed. The only change is in the assertion region (lines 141 and 142) of the file test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/TokenIntrospectionTests.cs. All necessary types are already imported, so no imports or additional methods are required.