Skip to content

Auth/defaults#445

Merged
PGijsbers merged 4 commits intodevelopfrom
auth/defaults
Feb 12, 2025
Merged

Auth/defaults#445
PGijsbers merged 4 commits intodevelopfrom
auth/defaults

Conversation

@PGijsbers
Copy link
Contributor

@PGijsbers PGijsbers commented Feb 11, 2025

Change

  • Adds the reviewer role to the aiod realm, and a reviewer user with that role (password is "password").
  • Store realm and user information separately.
  • Add documentation on exporting/importing keycloak.
  • ⚠️Fix the cleanup script so that it actually removes keycloak data.
  • Forward the KEYCLOAK_ADMIN variables when using the dev options.

To create the new realm files, I loaded the old ones, then did an export. I assume this preserves all the relevant information, the new exports only contains the aiod realm data and not any admin realm data (though as far as I could tell this was already the case in the old file).

How to Test

./scripts/down.sh
./scripts/clean.sh
./scripts.up.sh

Go to the admin console (http://localhost/aiod-auth) login with default credentials, and see that the reviewer role and user exist. On the REST API doc page, log in as reviewer and call the authorization_test endpoint to see the role is visible from the REST API.

This change is not covered by unit tests because keycloak configuration isn't tested.

Checklist

  • Tests have been added or updated to reflect the changes, or their absence is explicitly explained.
  • Documentation has been added or updated to reflect the changes, or their absence is explicitly explained.
  • A self-review has been conducted checking:
    • No unintended changes have been committed.
    • The changes in isolation seem reasonable.
    • Anything that may be odd or unintuitive is provided with a GitHub comment explaining it (but consider if this should not be a code comment or in the documentation instead).
  • All CI checks pass before pinging a reviewer, or provide an explanation if they do not.

Related Issues

Preparation for #435 which uses the reviewer role.

@PGijsbers PGijsbers requested a review from mrorro February 11, 2025 11:39
@PGijsbers PGijsbers mentioned this pull request Feb 11, 2025
4 tasks
@mrorro
Copy link
Collaborator

mrorro commented Feb 12, 2025

  1. USE_LOCAL_DEV in .env – Currently, USE_LOCAL_DEV is not set, so ./scripts/up.sh starts without applying local changes. We should either set it to true by default or update the documentation to clarify this.
  2. sudo commands in ./scripts/clean.sh – Are these necessary? The script works for me without sudo. If it's not required, we could remove them.
  3. Minor macOS issue – Port 5000 is already in use on macOS. Using an alternative, like 5001, works fine.

@PGijsbers
Copy link
Contributor Author

I agree with all those suggestions, but none of that is related to the changes of this PR. Can I assume that the proposed changes here are good?

@PGijsbers PGijsbers mentioned this pull request Feb 12, 2025
Copy link
Collaborator

@mrorro mrorro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PGijsbers PGijsbers merged commit 357ebb4 into develop Feb 12, 2025
1 check passed
@PGijsbers PGijsbers deleted the auth/defaults branch February 12, 2025 13:43
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