Fix SMART system search scope filtering - #5788
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 43c8b4b3-3e41-45af-b117-06bce9ca5517
There was a problem hiding this comment.
🟡 Changes recommended
One access-control related behavioral regression was identified (include parsing can be silently skipped under FGAC in cases that previously threw) and should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes SMART (v2) system-level search authorization by preserving the resource/action pairing for scope restrictions when requests don’t have a concrete route resource type, ensuring search vs read-by-id semantics are enforced consistently across _type filtering, include/revinclude validation, and Cosmos DB include queries.
Changes:
- Introduces action-aware scope filtering (
ScopeDataActions) so searches apply only.read/.s-equivalent restrictions, while direct reads by ID apply.read/.r-equivalent restrictions. - Propagates the action context through search option creation, expression access checks, and Cosmos include query scope filtering.
- Adds/extends unit + integration test coverage for mixed wildcard read-by-id + resource-scoped search scenarios.
File summaries
| File | Description |
|---|---|
| test/Microsoft.Health.Fhir.Shared.Tests.Integration/Features/Smart/SmartSearchTests.cs | Adds an integration test covering mixed wildcard read-by-id + Patient search scopes in system searches. |
| src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs | Adds ScopeDataActions, filters applicable SMART scope restrictions by action, and threads filtered scopes through include parsing and access checks. |
| src/Microsoft.Health.Fhir.Shared.Core/Features/Resources/Get/GetResourceHandler.cs | Ensures GET-by-id uses `Read |
| src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchOptionsFactoryTests.cs | Adds tests validating action-aware scope filtering and include parsing behavior under mixed scopes. |
| src/Microsoft.Health.Fhir.CosmosDb/Features/Search/FhirCosmosSearchService.cs | Applies action-aware SMART scope filtering to Cosmos include/revinclude query filtering. |
| src/Microsoft.Health.Fhir.Core/Features/Search/SearchService.cs | Adds an overload to pass scopeDataActions into SearchOptionsFactory. |
| src/Microsoft.Health.Fhir.Core/Features/Search/SearchOptions.cs | Stores ScopeDataActions on SearchOptions and copies it in the copy ctor. |
| src/Microsoft.Health.Fhir.Core/Features/Search/ISearchService.cs | Adds a new overload for action-aware SMART scope filtering. |
| src/Microsoft.Health.Fhir.Core/Features/Search/ISearchOptionsFactory.cs | Adds a new overload for action-aware SMART scope filtering at options creation time. |
| src/Microsoft.Health.Fhir.Core/Features/Search/Access/ExpressionAccessControl.cs | Adds an overload to validate include/chained expressions against the action-filtered scope set. |
| src/Microsoft.Health.Fhir.Core/Features/Context/ScopeRestriction.cs | Adds AllowsAny(DataActions) helper used to filter restrictions by requested actions. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5788 +/- ##
==========================================
+ Coverage 78.28% 78.93% +0.64%
==========================================
Files 1016 1018 +2
Lines 36940 37000 +60
Branches 5619 5624 +5
==========================================
+ Hits 28920 29205 +285
+ Misses 6644 6407 -237
- Partials 1376 1388 +12 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Explicit include targets can bypass filtered SMART scopes, and malformed includes may now be silently ignored.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs:745
- This authorization-based early return happens before
ParseInclude, so malformed or unsupported include values whose first token is not scoped are silently ignored instead of producing the existing 400 response. Parse and validate the include first, then discard the validated expression if its resource types are unauthorized.
string includeSourceResourceType = p.query?.Split(':')[0];
if (!string.Equals(includeSourceResourceType, "*", StringComparison.Ordinal) &&
!string.Equals(includeSourceResourceType, KnownResourceTypes.All, StringComparison.Ordinal) &&
!allowedResourceTypesByScope.Contains(includeSourceResourceType))
{
return null;
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Balanced
Description
Preserves the resource/action pairing for SMART clinical scopes when search requests do not carry a concrete route resource type.
.reador SMART v2.srestrictions..reador SMART v2.rrestrictions._type, all-resource searches, history and compartment searches, include/revinclude validation, and Cosmos DB include queries.Related issues
AB#206787
Testing
FHIR Team Checklist
Semver Change (docs)
Patch