-
Notifications
You must be signed in to change notification settings - Fork 2
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
Port to main
: Use RA roles when testing authorizations
#418
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
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
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
pmeulen
requested review from
pmeulen
and removed request for
pmeulen and
pablothedude
February 22, 2024 14:06
Waiting for review of #417 |
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.
This is a port of #417 to the
main
branch. See #417 for details