vikunja/vikunja
8
54
Fork 94
Code Issues 101 Pull Requests 31 Packages Releases 37 Activity

fix: OIDC teams should not have admins #2161

Merged
konrad merged 3 commits from waza-ari/vikunja:fix-no-oidc-team-no-admin into main 2024-03-05 22:08:41 +00:00
Conversation 4 Commits 3 Files Changed 2 +40 -24
waza-ari commented 2024-03-04 21:46:03 +00:00
Contributor
Copy Link

This PR fixes an issue discussed in #2152. Before this PR, the user who triggered team creation automatically got the admin flag set for this group, which makes perfect sense for the normal UI workflow. OIDC managed teams cannot be edited in Vikunja, and they're created automatically by the first user logging in having this team assigned. This PR therefore makes sure that OIDC managed team members do not receive the admin flag.

This PR fixes an issue discussed in #2152. Before this PR, the user who triggered team creation automatically got the admin flag set for this group, which makes perfect sense for the normal UI workflow. OIDC managed teams cannot be edited in Vikunja, and they're created automatically by the first user logging in having this team assigned. This PR therefore makes sure that OIDC managed team members do not receive the admin flag.
waza-ari added 1 commit 2024-03-04 21:46:05 +00:00
fix: OIDC teams should not have admins
All checks were successful
continuous-integration/drone/pr Build is passing
Details
4f36184c98
frederick commented 2024-03-04 21:54:27 +00:00
Member
Copy Link

Hi waza-ari!

Thank you for creating a PR!

I've deployed the frontend changes of this PR on a preview environment under this URL: https://2161-fix-no-oidc-team-no-admin--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/.

This preview does not contain any changes made to the api, only the frontend.

Have a nice day!

Beep boop, I'm a bot.

Hi waza-ari! Thank you for creating a PR! I've deployed the frontend changes of this PR on a preview environment under this URL: https://2161-fix-no-oidc-team-no-admin--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/. This preview does not contain any changes made to the api, only the frontend. Have a nice day! > Beep boop, I'm a bot.
konrad reviewed 2024-03-05 08:33:28 +00:00
pkg/models/teams.go Outdated
@ -214,0 +216,4 @@
// If def_adm is true, the user will be an admin of the team
// Note: this function has been extracted from the Create method to allow
// an additional parameter to control whether the user should become admin of the team
func (t *Team) CreateNewTeam(s *xorm.Session, a web.Auth, adm bool) (err error) {
konrad commented 2024-03-05 08:32:53 +00:00
Owner
Copy Link

Please rename the adm parameter to something like firstUserShouldBeAdmin so that it's clear what it does.

Please rename the `adm` parameter to something like `firstUserShouldBeAdmin` so that it's clear what it does.
konrad marked this conversation as resolved
waza-ari added 1 commit 2024-03-05 14:00:18 +00:00
fix: naming change for parameter
All checks were successful
continuous-integration/drone/pr Build is passing
Details
6781e5e6cd
konrad reviewed 2024-03-05 18:23:12 +00:00
pkg/models/teams.go Outdated
@ -314,4 +332,1 @@
return err
}
return events.Dispatch(&TeamCreatedEvent{
konrad commented 2024-03-05 18:23:12 +00:00
Owner
Copy Link

Can you include the event dispatch in the new method as well?

Can you include the event dispatch in the new method as well?
waza-ari commented 2024-03-05 21:13:58 +00:00
Author
Contributor
Copy Link

Absolutely. I'm not sure how braindead I was to think that its a good idea to leave it there...

Absolutely. I'm not sure how braindead I was to think that its a good idea to leave it there...
konrad commented 2024-03-05 22:07:43 +00:00
Owner
Copy Link

No worries!

No worries!
konrad marked this conversation as resolved
waza-ari added 1 commit 2024-03-05 21:13:15 +00:00
fix: move event dispatch to new function
All checks were successful
continuous-integration/drone/pr Build is passing
Details
07dcfd8249
konrad approved these changes 2024-03-05 22:08:17 +00:00
konrad merged commit d69fc28125 into main 2024-03-05 22:08:41 +00:00
konrad deleted branch fix-no-oidc-team-no-admin 2024-03-05 22:08:41 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
konrad
Labels
Clear labels
dependencies
duplicate
This issue or pull request already exists
help wanted
Need some help
invalid
Something is wrong
kind/bug
Something is not working
kind/feature
New feature
moved-to-github
needs reproduction
question
More information is needed
security
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
dpschen frederick konrad renovate shilch
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vikunja/vikunja#2161
No description provided.
Delete Branch "waza-ari/vikunja:fix-no-oidc-team-no-admin"

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?