-
Notifications
You must be signed in to change notification settings - Fork 43
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
Identity provider login from mediator CLI #1014
Conversation
3bb1394
to
13c9fb8
Compare
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.
Awesome work!
I started to read the PR and left several comments but I haven't really even read the whole PR yet..
} | ||
|
||
// we can create the default user | ||
user, err := qtx.CreateUser(ctx, db.CreateUserParams{ |
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.
Did you have any thoughts on the org admin users going forward, e.g. did you plan on using roles instead of the org admin users?
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.
using roles assigned to projects/orgs would be ideal.
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.
I was thinking when a user logs in for the first time a new org, group and role would be created, making the user an admin of their own org. The same as what we used to do for self-enrolled users.
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.
@eleftherias let's go for that, it goes in accordance to our plans for the initial SaaS
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.
I'd like to do this in a separate PR. For now, all users get added to organization 1.
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.
Good progress! Left some comments.
} | ||
|
||
// we can create the default user | ||
user, err := qtx.CreateUser(ctx, db.CreateUserParams{ |
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.
using roles assigned to projects/orgs would be ideal.
cdc3c50
to
2698ce2
Compare
43927a4
to
a11c4ff
Compare
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.
I really dig that this also replaces a bunch of the tech debt we had in JWT validation.
I tried this out locally. Had a small hickup when logging in with a GitHub user the first time, but it worked on the second attempt.
There are some next steps to do:
- Generate Keycloak config programmatically
- Allow for a non-git tracked config that folks could overwrite
- roles for users that come from an idp
But this really moves us forward! great work!
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.
nit: will need a docs update (can be follow up PR, as long as it gets done at some point) https://mediator-docs.stacklok.dev/run-the-login_medic
a11c4ff
to
5d25ddc
Compare
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.
lgtm
Relates to #691.
To try out this PR:
make gen
make sqlc
make migrateup
docker-compose up -d keycloak
make build
identity
to yourconfig.yaml
(see updates toconfig/config.yaml.example
)./bin/medic auth login
. The superadmin credentials are the same.Things that you can try:
To add Social login:
http://localhost:8081/realms/stacklok/broker/github/endpoint
identity/import/stacklok-realm-with-user-and-client.json
”identityProviders" : [ ],
with the following, using your generated client ID and client secret:To add Google login, the steps are the same, but with a different provider ID. Feel free to reach out if you want to experiment with Google login and want help.
Alternatively, you can add social login via the Keycloak admin console, but note that this configuration will disappear if you restart Keycloak.
Things for a different PR: