feat: implement tenant isolation for anomalies and namespaces, enhanc…#138
Merged
Conversation
…ing security and ownership checks
There was a problem hiding this comment.
Pull request overview
This PR strengthens tenant isolation and security by enforcing namespace ownership checks in cloud-bridge endpoints and anomaly retrieval, and by hardening EasyAuth header trust to prevent identity spoofing outside Azure App Service.
Changes:
- Enforce namespace ownership checks (404-on-mismatch) in
CloudBridgeControllerendpoints that operate by namespace ID. - Enforce tenant isolation when retrieving anomalies by ensuring the anomaly’s namespace owner matches the current request owner.
- Harden EasyAuth header handling by only trusting
X-MS-CLIENT-PRINCIPAL-IDwhen Azure EasyAuth is detected or explicitly configured, plus add/adjust related unit tests and config.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| services/api/src/ServiceHub.Api/Controllers/V1/CloudBridgeController.cs | Adds namespace ownership verification before executing provider operations to prevent cross-tenant access. |
| services/api/src/ServiceHub.Api/Controllers/V1/AnomaliesController.cs | Adds tenant isolation check for anomaly visibility via namespace ownership. |
| services/api/src/ServiceHub.Api/Middleware/EasyAuthMiddleware.cs | Restricts when EasyAuth headers are trusted to mitigate spoofing on non-Azure hosts; logs guidance when disabled. |
| services/api/src/ServiceHub.Api/Middleware/RateLimitingMiddleware.cs | Throttles full dictionary cleanup to once per interval to reduce per-request O(n) overhead. |
| services/api/src/ServiceHub.Api/appsettings.Production.json | Adds TrustClientPrincipalHeader setting and updates production guidance text. |
| services/api/tests/ServiceHub.UnitTests/Infrastructure/CloudProviderRouterTests.cs | Updates tests to provide an INamespaceRepository mock so new ownership checks pass. |
| services/api/tests/ServiceHub.UnitTests/Api/Controllers/V1/CloudBridgeControllerTests.cs | Adds namespace ownership isolation tests and updates controller construction for new dependency. |
| services/api/tests/ServiceHub.UnitTests/Api/Controllers/V1/AnomaliesControllerTests.cs | Updates/extends tests to cover namespace ownership isolation for anomaly retrieval. |
| services/api/tests/ServiceHub.UnitTests/Api/Middleware/EasyAuthMiddlewareTests.cs | Adds new unit tests for EasyAuth trust gating behavior, including Azure-detected behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+150
to
+157
| var namespaceResult = await _namespaceRepository.GetByIdAsync(result.Value.NamespaceId, cancellationToken); | ||
| if (namespaceResult.IsFailure | ||
| || !string.Equals(namespaceResult.Value.OwnerId, OwnerId, StringComparison.Ordinal)) | ||
| { | ||
| return ToActionResult<AnomalyInfo>(ServiceHub.Shared.Results.Error.NotFound( | ||
| "Anomaly.NotFound", | ||
| $"Anomaly with ID '{id}' was not found.")); | ||
| } |
Comment on lines
+79
to
+96
| Environment.SetEnvironmentVariable("WEBSITE_AUTH_ENABLED", "True"); | ||
| try | ||
| { | ||
| RequestDelegate next = _ => Task.CompletedTask; | ||
| var middleware = CreateMiddleware(CreateConfig(enabled: true), next); | ||
|
|
||
| var context = new DefaultHttpContext(); | ||
| context.Request.Headers[PrincipalHeader] = "user-oid-456"; | ||
|
|
||
| await middleware.InvokeAsync(context); | ||
|
|
||
| context.Items["OwnerId"].Should().Be("entra:user-oid-456"); | ||
| context.Items["AuthMethod"].Should().Be("EasyAuth"); | ||
| } | ||
| finally | ||
| { | ||
| Environment.SetEnvironmentVariable("WEBSITE_AUTH_ENABLED", null); | ||
| } |
… corresponding unit tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ing security and ownership checks