Support delegating additional actions in runtime delegating signing.#1662
Support delegating additional actions in runtime delegating signing.#1662
Conversation
📝 WalkthroughWalkthroughAdds an optional AdditionalActionsToDelegate property to signing models, threads it through signee context creation, and includes the additional actions when building rights for delegation and revocation; corresponding unit tests and public API verification were updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/Altinn.App.Core/Features/Signing/Services/SigningDelegationService.cs`:
- Around line 185-197: In SigningDelegationService where additionalActions is
iterated, validate and deduplicate actions before adding RightRequest: for each
entry in additionalActions trim and skip null/empty or whitespace-only strings,
normalize casing (e.g. ToLowerInvariant), maintain a HashSet<string> to ignore
duplicates and also skip the default actions "read" and "sign"
(case-insensitive) or any action already represented in rights, and only then
construct and Add a new RightRequest with Action = new AltinnAction { Value =
action }.
🧹 Nitpick comments (3)
src/Altinn.App.Core/Features/Signing/ProvidedSignee.cs (1)
15-21: Consider documenting or guarding against duplicate default actions.If a caller passes
"read"or"sign"inAdditionalActionsToDelegate, the delegation request will contain duplicate rights entries for those actions. Consider either:
- Adding a note in the XML doc that
"read"and"sign"should not be included, or- Filtering out duplicates in
CreateRights(inSigningDelegationService.cs).This is unlikely to cause a runtime failure, but duplicate rights in the request payload could cause unexpected behavior from the access management API.
src/Altinn.App.Core/Features/Signing/Services/SigningDelegationService.cs (1)
158-200: Sharedresourceslist reference across allRightRequestobjects.All
RightRequestinstances (lines 173, 178, and those added in the loop at line 192) reference the sameresourcesList<Resource>instance. SinceRightRequest.Resourceis a mutableList<Resource>with a public setter, any downstream mutation of one request'sResourcelist would corrupt all others.This is safe today because the requests are serialized and sent over HTTP without mutation, but it's a fragile assumption. Consider either documenting this intentional sharing or creating the list per-request if defensiveness is preferred.
test/Altinn.App.Core.Tests/Features/Signing/SigningDelegationServiceTests.cs (1)
640-687: Revocation with additional actions is properly tested.For completeness, consider adding a symmetric
RevokeSigneeRights_WithNullAdditionalActions_RevokesOnlyReadAndSigntest to match the delegation null-case test at line 594. This isn't blocking.
src/Altinn.App.Core/Features/Signing/Services/SigningDelegationService.cs
Show resolved
Hide resolved
test/Altinn.App.Core.Tests/Features/Signing/SigningDelegationServiceTests.cs
Fixed
Show fixed
Hide fixed
|
|
/publish |




Support delegating additional actions in runtime delegating signing.
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
New Features
Tests