-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Implement scalable dialog search authorization #875
Conversation
...main.Dialogporten.Infrastructure/Altinn/Authorization/LocalDevelopmentAltinnAuthorization.cs
Outdated
Show resolved
Hide resolved
…ge local dev altinn auth impl
## Description This is the precursor to #875, adding the SubjectResource (was "RoleResource") entity and update mechanism that uses the new changefeed API in RR This adds a "Janitor" console project, which can be invoked in container app jobs (or manually) to perform the syncronization. ## Related Issue(s) - #42 ## Verification - [ ] **Your** code builds clean without any errors or warnings - [ ] Manual testing done (required) - [ ] Relevant automated test added (if you find this hard, leave it and we'll help out) ## Documentation - [ ] Documentation is updated (either in `docs`-directory, Altinnpedia or a separate linked PR in [altinn-studio-docs.](https://github.com/Altinn/altinn-studio-docs), if applicable) --------- Co-authored-by: Magnus Sandgren <[email protected]> Co-authored-by: Are Almaas <[email protected]> Co-authored-by: Ole Jørgen Skogstad <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Holder fremdeles på å reviewe, men endringen på DialogUserTypes trenger litt mer kjærlighet.
src/Digdir.Domain.Dialogporten.Application/Common/Extensions/ClaimsPrincipalExtensions.cs
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Application/Common/Extensions/QueryableExtensions.cs
Show resolved
Hide resolved
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogUserType.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved - bare ta en titt her #1091 Gi meg beskjed om du trenger en ny approval 🙂
...omain.Dialogporten.Infrastructure/Persistence/DevelopmentSubjectResourceSyncHostedService.cs
Show resolved
Hide resolved
Nå ble dette litt mer enn noen forslag om akkurat PRen din. Jeg så noe lavt hengende frukt i PaginatedList også som jeg håper du syns er greit at jeg bare slenger inn her 🙏
Quality Gate passedIssues Measures |
🤖 I have created a release *beep* *boop* --- ## [1.17.0](v1.16.0...v1.17.0) (2024-09-10) ### Features * Add SubjectResource entity and db migration ([#1048](#1048)) ([d04d764](d04d764)) * **graphQL:** Add subscription for dialog details ([#1072](#1072)) ([8214acb](8214acb)) * Implement scalable dialog search authorization ([#875](#875)) ([aa8f84d](aa8f84d)) * revise dialog status ([#1099](#1099)) ([0029f46](0029f46)) ### Bug Fixes * ensure correct appsettings is used ([#1086](#1086)) ([d43f6d7](d43f6d7)) * ensure jobs are run with correct arguments and parameters ([#1085](#1085)) ([e21de56](e21de56)) * **webapi:** Return 422 when existing transmission IDs are used in dialog update ([#1094](#1094)) ([7a8a933](7a8a933)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@coderabbitai full review |
Actions performedFull review triggered. |
Caution Review failedThe pull request is closed. WalkthroughWalkthroughDis big change, it involves removin' old support for Altinn 2 enterprise users and addin' new methods for handlin' party identifiers. Many legacy methods gone, and new ones introduced to make user type handling better. Also, some classes got new methods for filterin' and authorization. Overall, it’s a big shift toward modern user management and authorization processes. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Description
This implements a authorized-parties based approach to list/search authorization, which should scale better than the current approach.
This builds on the syncronization of data from RR, which contains a map of subjects (role codes and eventually access packages) and resources. This data is persisted in Dialogporten DB, and used as a cache.
A new predicate builder
PrefilterAuthorizedDialogs
replacesWhereUserIsAuthorizedFor
, and constructs a SQL manually in order to propertly handle the new propertySubjectsByParties
inDialogSearchAuthorizationResult
, which is a dict of party->subjects. Each of the roles grant access to a list of resources.This also removes legacy system users, as they cannot be authorized this way (not possible to get a list of parties from Authorization APIs for a legacy system user).
Related Issue(s)
Verification
Documentation
docs
-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores