Add versioned client delegation API support - #2408
Merged
Merged
Conversation
Introduce ClientDelegationClientV1 and V2 with feature flag-based routing via ClientDelegationClientSelector. The v2 client supports single rights resources and uses updated endpoint parameter names (client/agent instead of from/to). Configuration endpoints now store versionless base URLs; clients append their version suffix. Includes comprehensive tests and mock data updates.
Contributor
📝 WalkthroughWalkthroughThe change adds v1 and v2 client delegation implementations with feature-flagged routing, updates Access Management clients to append API versions, introduces compact resource models, changes endpoint configuration, and adds HTTP contract tests. ChangesClient delegation resource contract
Versioned API endpoint configuration
v1 and v2 delegation clients
Feature-flagged selection and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/AccessManagementClient.cs`:
- Line 51: Update the role-assignment request URLs in AccessManagementClient to
remain relative to the versioned BaseAddress, using assignment paths without a
leading slash while preserving existing IDs and query parameters. Add a contract
test that verifies the resulting absolute URLs retain the /v1/ base path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ad0f178-fc06-4859-93b6-292dc8e48c40
📒 Files selected for processing (29)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Models/ClientDelegation/ClientDelegation.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Altinn.AccessManagement.UI.Integration.csprojbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/AccessManagementClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/AccessManagementClientV0.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/AccessPackageClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/ClientDelegationClientSelector.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/ClientDelegationClientV1.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/ClientDelegationClientV2.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/ConnectionClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/ConsentClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/InstanceClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/MaskinportenClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/RequestClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/RoleClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/SingleRightClient.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Configuration/PlatformSettings.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Mocks/Data/ClientDelegation/myClients.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Clients/ClientDelegationClientTest.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Data/ExpectedResults/ClientDelegation/myClients.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/Program.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT21.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT22.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT23.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.AT24.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.Development.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.Prod.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.TT02.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.YT01.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.json
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Clients/ClientDelegationClientTest.cs (1)
130-139: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert POST payload contracts too.
These mutation tests verify only method and URI, so a missing or incorrectly serialized
DelegationBatchInputDtostill passes. Capture and assert the JSON body (and content type) for the add/remove access-package calls.Also applies to: 185-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Clients/ClientDelegationClientTest.cs` around lines 130 - 139, Extend the add and remove access-package tests, including RemoveMyClientAccessPackages_V2PostsToDeleteRoute and the corresponding tests around lines 185-205, to capture and validate the request body serialized from _payload as DelegationBatchInputDto JSON. Assert the request content type is application/json while preserving the existing HTTP method and URI assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Clients/ClientDelegationClientTest.cs`:
- Around line 130-139: Extend the add and remove access-package tests, including
RemoveMyClientAccessPackages_V2PostsToDeleteRoute and the corresponding tests
around lines 185-205, to capture and validate the request body serialized from
_payload as DelegationBatchInputDto JSON. Assert the request content type is
application/json while preserving the existing HTTP method and URI assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d502726c-34c2-4212-b3f8-bf776a90787b
📒 Files selected for processing (7)
backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Core/Models/ClientDelegation/ClientDelegation.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/ClientDelegationClientV1.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/ClientDelegationClientV2.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Mocks/Data/ClientDelegation/myClients.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Clients/ClientDelegationClientTest.csbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Data/ExpectedResults/ClientDelegation/myClients.jsonbackend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI/appsettings.Development.json
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Mocks/Data/ClientDelegation/myClients.json
- backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Tests/Data/ExpectedResults/ClientDelegation/myClients.json
- backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/ClientDelegationClientV1.cs
- backend/src/Altinn.AccessManagement.UI/Altinn.AccessManagement.UI.Integration/Clients/ClientDelegationClientV2.cs
|
5 tasks
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
2 participants
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.
Del av Altinn/altinn-authorization-tmp#3737.
BFF-en kaller nå v2 av clientdelegations-API-et når flagget
AccessManagementUI.UseNewSingleRightsClientDelegation er på. v2-lesesvarene
inneholder enkelttjenester (resources) per rolle, som eksponeres videre på
BFF-modellene. Med flagget av er v1-oppførselen uendret.
Description
ApiAccessManagementEndpointi env-filene og setter versjondynamisk der den brukes. Versjoner utvikler seg uavhengig per API-område, så
versjonen hører hjemme der strengen brukes.
ClientDelegationClientV2så V1 kan slettes når V2 er ferdig.ClientDelegationClientSelectorer eneste flaggsjekkpunkt og velger klient runtime.CompactResource { Id, RefId }som kun populeres av v2 (feat: return the resource registry id on v2 client delegation resources altinn-authorization-tmp#3829)Sonarqube klager på kodeduplisering, men tenker at vi kan ignorere det siden v1 kan slettes når vi har v2 på plass.
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
Summary
New Features
Bug Fixes
Documentation
Tests