vikunja
/
vikunja
6
43
Fork 71
Code Issues 150 Pull Requests 16 Packages Releases 32 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 1 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
continuous-integration/drone/pr Build is passing Details
4f36184c98 fix: OIDC teams should not have admins
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
continuous-integration/drone/pr Build is passing Details
6781e5e6cd fix: naming change for parameter
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
continuous-integration/drone/pr Build is passing Details
07dcfd8249 fix: move event dispatch to new function
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

  
needs reproduction

   
question

More information is needed

  
security

   
wontfix

This won't be fixed

No Label
dependencies
duplicate
help wanted
invalid
kind/bug
kind/feature
needs reproduction
question
security
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

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?