-
-
Notifications
You must be signed in to change notification settings - Fork 404
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 OIDC sign-in #9251
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
…_BASE` and `DOCKER_COMPOSE_TEST` variables
…_BASE` and `DOCKER_COMPOSE_TEST` variables
Makefile
Outdated
# Ensure shared_network is referenced when running locally | ||
DOCKER_COMPOSE_RUN=COMPOSE_FILE="${COMPOSE_FILE};docker/run.yml" ${DOCKER_COMPOSE} |
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.
@hangy I think there might have been a merge problem.
I think we did this on the main branch and revert it after.
So I don't think you need this change, as well as other changes that are underneath.
redis-listener: | ||
<<: *backend-conf | ||
# we want the redis_listerner for off, but not for pro profile | ||
profiles: ["off"] | ||
# This service watches for new events on a redis stream to delete users | ||
command: ["perl", "scripts/listen_to_redis_stream.pl"] |
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.
@john-gom this means we have to define a new systemd service in prod to do this ?
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.
Hi @alexgarel . Yes, I had created an issue in the project here: https://github.com/orgs/openfoodfacts/projects/119?pane=issue&itemId=89376373&issue=openfoodfacts%7Copenfoodfacts-server%7C11082
by fixing Makefile
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Signed-off-by: John Gomersall <[email protected]>
Quality Gate passedIssues Measures |
What
The main aim is to use Keycloak as an IdP, and connect Product Opener as a RP. We want to move the basics of user management out of Product Opener.
In the future, this can be used as a base to enable #1204 to be implemented, even though it will probably not be part of the initial change as to not blow up the PR too much. One important requirement is to keep compatibility with the current Basic Auth mechanism.
Screenshot
TBD
Related issue(s) and discussion
Adding a task list to track progress:
main
currently has a lot of hard coded URLs)time ./scripts/migrate_users_to_keycloak.pl
with around 400.000 synthetic accounts took about 45 minutes on my desktop PC in WSL2.Save for a later phase:
Example: If the users was on
fr.openfoodfacts.org
before signing up, and they change their locale to Spanish during registration in Keycloak, do we want them to be redirected to a Spanish page instead?Problem: Differentiating between first login (because of registration) and subsequent login would have to be done based on whether or not
${userid}.sto
exists.Advantage: The client secret does not have to be exposed to the admin. However, they'll have access to the secret anyways, unless we were to store the secret in some kind of HSM.
Disadvantage: Some client configuration can be done during self-registration, but some necessary permissions like realm-management need to be configured manually by an admin, anyways.
To consider: This file currently contains information about the users' active sessions. Do we want to replace the opaque session identifier by an encrypted cookie? Also, we need some kind of directory to identify users that have used ProductOpener know if we have to do something when a deletion event comes in through Redis. (Obviously, this could be done in MongoDB or PostgreSQL instead, but is there a huge difference?)
Figure out how/if org management should be done in Keycloak(Done in MongoDB)Moved to
openfoodfacts-auth
: