vikunja/vikunja
6
43
Fork 73
Code Issues 150 Pull Requests 20 Packages Releases 32 Activity

feat: assign users to teams via OIDC claims #1393

Merged
konrad merged 93 commits from viehlieb/api:950_reworked_assign_teams_via_oidc into main 2024-03-02 08:47:12 +00:00
Conversation 34 Commits 93 Files Changed 26 +13 -4
1 changed files with 13 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 274dbecae5 - Show all commits

17
pkg/modules/auth/openid/openid.go
View File

@ -208,10 +208,19 @@ func HandleCallback(c echo.Context) error {
}
// does the oidc token contain well formed "vikunja_groups" through vikunja_scope
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-10-18 15:18:17 +00:00
Outdated
Review
Copy Link

Typo: team and oidc should be lowercase.

Please check the other log messages as well for typos, I've seen a few more like this.

Typo: `team` and `oidc` should be lowercase. Please check the other log messages as well for typos, I've seen a few more like this.
teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, provider)
if teamData != nil {
for _, err := range errs {
log.Errorf("Error creating teams for user and vikunja groups %s: %v", cl.VikunjaGroups, err)
teamData, err := getTeamDataFromToken(cl.VikunjaGroups, provider)
if err != nil {
log.Errorf("Error creating teams for user and vikunja groups %s: %v", cl.VikunjaGroups, err)
return handler.HandleHTTPError(err, c)
}
// check if we have seen these teams before.
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-02-13 10:24:55 +00:00
Outdated
Review
Copy Link

Please don't ignore the error.

Please don't ignore the error.
// find or create Teams and assign user as teammember.
if len(teamData) > 0 {
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-10-18 15:17:33 +00:00
Outdated
Review
Copy Link

Please don't call this "Sign out". That's a different thing.

Please don't call this "Sign out". That's a different thing.
log.Debugf("TeamData is set %v", teamData)
teams, err := GetOrCreateTeamsByOIDCAndNames(s, teamData, u)
if err != nil {
log.Errorf("Error verifying team for name %v, got %v", cl.Name, teams, err)
return err
}
konrad marked this conversation as resolved Outdated
konrad commented 2023-12-01 14:59:58 +00:00
Outdated
Review
Copy Link

Doing this every time a user logs in probably has performance implications but that's a problem to care about when we'll hit it in the wild.

Doing this every time a user logs in probably has performance implications but that's a problem to care about when we'll hit it in the wild.
viehlieb commented 2023-12-06 13:37:36 +00:00
Outdated
Review
Copy Link

only if it has teamIDsToLeave, but yeah I agree.

The problem is more the oidc structure itself, that you "have" to check for changes "everytime" you sign in..

only if it has teamIDsToLeave, but yeah I agree. The problem is more the oidc structure itself, that you "have" to check for changes "everytime" you sign in..
//find old teams for user through oidc