Skip to content
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
merged 10 commits into from
Feb 26, 2024

Conversation

MKodde
Copy link
Member

@MKodde MKodde commented Feb 20, 2024

This is a port of #417 to the main branch. See #417 for details

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 pmeulen requested a review from pablothedude February 22, 2024 13:50
@pmeulen pmeulen self-assigned this Feb 22, 2024
@pmeulen pmeulen requested review from pmeulen and removed request for pmeulen and pablothedude February 22, 2024 14:06
@pmeulen
Copy link
Member

pmeulen commented Feb 22, 2024

Waiting for review of #417

@MKodde MKodde merged commit fd4d739 into main Feb 26, 2024
4 checks passed
@MKodde MKodde deleted the feature/main-authz-bugfix branch February 26, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants