lix-project/buildbot-nix
15
0
Fork 0
Code Issues 6 Pull requests 2 Packages Projects Releases Wiki Activity

feat: enable Lix admins to admin the Buildbot properly #16

Open
raito wants to merge 1 commit from authz into main
pull from: authz
merge into: lix-project:main
Conversation 9 Commits 1 Files changed 1 +15 -1
raito commented 2024-06-09 15:01:59 +00:00
Owner
Copy link

This removes the need for a proxy and rely on the groups property of
the userDetails passed at the authentication layer.

To add a certain role, add the group buildbot-$role to that user via
Keycloak.

The next steps are to slowly add ways to give trusted actions to trusted developers.

Signed-off-by: Raito Bezarius raito@lix.systems

This removes the need for a proxy and rely on the `groups` property of the `userDetails` passed at the authentication layer. To add a certain role, add the group `buildbot-$role` to that user via Keycloak. The next steps are to slowly add ways to give trusted actions to trusted developers. Signed-off-by: Raito Bezarius <raito@lix.systems>
raito added 1 commit 2024-06-09 15:02:04 +00:00 
This removes the need for a proxy and let *@lix.systems folks to
be able to admin the Buildbot instance.

Our authentication layer should confirm emails to avoid someone
impersonating a @lix.systems user and grant themselves trivial admin
rights.

Signed-off-by: Raito Bezarius <raito@lix.systems>
jade commented 2024-06-09 15:28:45 +00:00
Owner
Copy link

uh why does this imply it uses emails? i would like to just put a role in the oidc token.

uh why does this imply it uses emails? i would like to just put a role in the oidc token.
raito commented 2024-06-09 17:24:35 +00:00
Author
Owner
Copy link

uh why does this imply it uses emails? i would like to just put a role in the oidc token.

Because I'm not sure this will work out with the role in the OIDC token, but we can try with that first if you prefer.

> uh why does this imply it uses emails? i would like to just put a role in the oidc token. Because I'm not sure this will work out with the `role` in the OIDC token, but we can try with that first if you prefer.
jade commented 2024-06-09 23:57:10 +00:00
Owner
Copy link

I don't understand how this PR gets authorization information in the first place; how does it get emails? If it is getting emails, surely it sees other parts of the token, right?

I don't understand how this PR gets authorization information in the first place; how *does* it get emails? If it is getting emails, surely it sees other parts of the token, right?
raito commented 2024-06-10 13:15:09 +00:00
Author
Owner
Copy link

I don't understand how this PR gets authorization information in the first place; how does it get emails? If it is getting emails, surely it sees other parts of the token, right?

I assume that Buildbot pulls the standard fields from the OAuth 2 token yes, this, I am pretty sure it would ~work out of the box. As for the rest of the token, I would need to dig more in the Buildbot code and I would prefer to test it "live" and adjust or we need to deploy a staging Buildbot maybe…

Take the owner role, this is induced by looking at Gerrit commit's emails and comparing them over the authenticated's email address to give the "owner" role.

> I don't understand how this PR gets authorization information in the first place; how *does* it get emails? If it is getting emails, surely it sees other parts of the token, right? I assume that Buildbot pulls the standard fields from the OAuth 2 token yes, this, I am pretty sure it would ~work out of the box. As for the rest of the token, I would need to dig more in the Buildbot code and I would prefer to test it "live" and adjust or we need to deploy a staging Buildbot maybe… Take the owner role, this is induced by looking at Gerrit commit's emails and comparing them over the authenticated's email address to give the "owner" role.
raito force-pushed authz from 97a2b79dba to efa7a950c3 2024-06-10 18:49:36 +00:00 Compare
raito commented 2024-06-10 18:49:57 +00:00
Author
Owner
Copy link

I pushed an alternative proposal using groups, I do not have in-depth view in how Keycloak will transmit groups, but if that's helpful to you, here's a version without emails.

I pushed an alternative proposal using `groups`, I do not have in-depth view in how Keycloak will transmit `groups`, but if that's helpful to you, here's a version without emails.
jade commented 2024-06-10 20:55:16 +00:00
Owner
Copy link

I pushed an alternative proposal using groups, I do not have in-depth view in how Keycloak will transmit groups, but if that's helpful to you, here's a version without emails.

i know how that works. i think that groups isn't actually sent in our tokens at all which is perfect because we can just send client roles in there instead. or we can override it at least.

> I pushed an alternative proposal using `groups`, I do not have in-depth view in how Keycloak will transmit `groups`, but if that's helpful to you, here's a version without emails. i know how that works. i think that groups isn't actually sent in our tokens at all which is perfect because we can just send client roles in there instead. or we can override it at least.
jade commented 2024-06-10 23:28:49 +00:00
Owner
Copy link

I pushed an alternative proposal using groups, I do not have in-depth view in how Keycloak will transmit groups, but if that's helpful to you, here's a version without emails.

i know how that works. i think that groups isn't actually sent in our tokens at all which is perfect because we can just send client roles in there instead. or we can override it at least.

ok nope, we need to change our subclass of OAuth2Auth to actually read a groups claim out of the token. We should call the token field buildbot_roles and I can make that happen on the Keycloak end.

96c50524ed/master/buildbot/www/oauth2.py (L403-L423)

> > I pushed an alternative proposal using `groups`, I do not have in-depth view in how Keycloak will transmit `groups`, but if that's helpful to you, here's a version without emails. > > i know how that works. i think that groups isn't actually sent in our tokens at all which is perfect because we can just send client roles in there instead. or we can override it at least. ok nope, we need to change our subclass of OAuth2Auth to actually read a groups claim out of the token. We should call the token field `buildbot_roles` and I can make that happen on the Keycloak end. https://github.com/buildbot/buildbot.git/blob/96c50524ed224e68191bba7c38329bd155d2c1fe/master/buildbot/www/oauth2.py#L403-L423
👍 1
jade commented 2024-06-10 23:32:01 +00:00
Owner
Copy link

re "is passing scope necessary" comment, maybe, but it should give scope "email openid profile" for correctness.

re "is passing scope necessary" comment, maybe, but it should give scope "email openid profile" for correctness.
👍 1
raito commented 2024-06-11 08:32:00 +00:00
Author
Owner
Copy link

OK, I will rework this towards that target.

OK, I will rework this towards that target.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin authz:authz
git checkout authz

Merge

Merge the changes and update on Forgejo.
git checkout main
git merge --no-ff authz
git checkout main
git merge --ff-only authz
git checkout authz
git rebase main
git checkout main
git merge --no-ff authz
git checkout main
git merge --squash authz
git checkout main
git merge --ff-only authz
git checkout main
git merge authz
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
You do not have permission to read 1 dependency
Reference: lix-project/buildbot-nix#16
No description provided.
Delete branch "authz"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?