Use RA roles when testing authorizations #417
Merged
Conversation
This file contains 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
MKodde
force-pushed
the
feature/database-test-experiment
branch
from
5eb3493 to
1b35421
Compare
MKodde
changed the title
MKodde
force-pushed
the
feature/database-test-experiment
branch
from
1b35421 to
15b5c25
Compare
Using the Insitution configuration role setting is semantically incorrect. Using the RA role here is the right way here. In order to get the select raa input for the select list displayed on the forms that allow selection of RA(A) users, a new method was added on the AuthorizationContextService
Documentation is improved by sharpening the logging. Making logging much more verbose should give quick insights into which commands are executed by which identity. The in-line code documentation was also improved by adding insights to some of the classes where this was lacking.
Documentation is improved by sharpening the logging. Making logging much more verbose should give quick insights into which commands are executed by which identity. The in-line code documentation was also improved by adding insights to some of the classes where this was lacking.
This field was renamed in the new MW release and updated in the mw client bundle. But we need to allow the old self service to work with the new Middleware. So adding (temporal) support for the old field seems the easiest fix. See: https://www.pivotaltracker.com/story/show/184749323
The authorization to read recovery tokens (RT) for ones institution was set to require the RAA role. However, reading and removing RT is a RA action. This commit lowers authz to RA for searching the RT See: https://www.pivotaltracker.com/story/show/184938232
The RA user was not yet allowed to revoke a recovery token. This is now allowed. As this is a RA role. See: https://www.pivotaltracker.com/story/show/184938232
MKodde
force-pushed
the
feature/database-test-experiment
branch
from
15b5c25 to
305a963
Compare
The CommandAuthorizationServiceTest did not yet correctly support the RT revocation scenario. Resulting in two failing tests. They were previously under the radar as the phpunit scritp would still return a 0 exit code even tho the tests failed. That was remedied by adding a 'set -e' instruction to that file
pablothedude
approved these changes
Feb 22, 2024
| @@ -69,40 +69,46 @@ public function __construct( | |||
| $this->authorizationRepository = $authorizationRepository; | |||
| } | |||
|
|
|||
| public function buildSelectRaaInstitutionAuthorizationContext(IdentityId $actorId): InstitutionAuthorizationContext | |||
There was a problem hiding this comment.
💯 for splitting up the contexts
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.
Before these changes, the authorizations handed out to an Identity when logging in to RA could be escalated unjustly.
The FGA authorizations were previously evaluated to calculate the authz for an Identity. Where they (ofcourse) have to be based on the authorizations that are actually accredited to the Identity.
For many authz decisions in the applications, we do evaluate the Institution authorizations, but when it comes to the determination of the roles for an identity we now correctly look to the
ra_listingprojection.Merge targets:
release/5.1main