vikunja
/
vikunja
6
43
Fork 71
Code Issues 150 Pull Requests 17 Packages Releases 32 Activity

rework openid.go, add errors to return, make team deletion more robust

This commit is contained in:
viehlieb 2023-02-13 19:52:18 +01:00
parent 956d7971ca
commit dace429bb7
2 changed files with 52 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
pkg/models/team_members.go
View File

@ -44,7 +44,6 @@ func (tm *TeamMember) Create(s *xorm.Session, a web.Auth) (err error) {
if err != nil {
return err
}
// Check if the user exists
member, err := user2.GetUserByUsername(s, tm.Username)
if err != nil {
@ -113,12 +112,18 @@ func (tm *TeamMember) CheckMembership(s *xorm.Session) (exists bool, err error)
exists, err = s.
Where("team_id = ? AND user_id = ?", tm.TeamID, tm.UserID).
Get(&TeamMember{})
if exists && err == nil {
return
}
return exists, err
}
func (tm *TeamMember) GetMemberCount(s *xorm.Session) (memberCount int, err error) {
members := []TeamMember{}
err = s.
Where("team_id = ?", tm.TeamID).
Cols("user_id").
Find(&members)
return len(members), err
}
// Update toggles a team member's admin status
// @Summary Toggle a team member's admin status
// @Description If a user is team admin, this will make them member and vise-versa.

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

@ -20,24 +20,24 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"math/rand"
"net/http"
"strconv"
"time"
"code.vikunja.io/web/handler"
"code.vikunja.io/api/pkg/db"
"xorm.io/xorm"
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/api/pkg/modules/auth"
"code.vikunja.io/api/pkg/user"
"code.vikunja.io/api/pkg/utils"
"github.com/coreos/go-oidc/v3/oidc"
petname "github.com/dustinkirkland/golang-petname"
"github.com/labstack/echo/v4"
"golang.org/x/oauth2"
"xorm.io/xorm"
)
// Callback contains the callback after an auth request was made and redirected
@ -64,7 +64,6 @@ type claims struct {
Name string `json:"name"`
PreferredUsername string `json:"preferred_username"`
Nickname string `json:"nickname"`
Teams []string `json:"groups"`
VikunjaGroups interface{} `json:"vikunja_groups"`
}
@ -193,7 +192,6 @@ func HandleCallback(c echo.Context) error {
// Check if we have seen this user before
u, err := getOrCreateUser(s, cl, idToken.Issuer, idToken.Subject)
if err != nil {
_ = s.Rollback()
log.Errorf("Error creating new user for provider %s: %v", provider.Name, err)
@ -201,14 +199,18 @@ func HandleCallback(c echo.Context) error {
}
// does the oidc token contain well formed "vikunja_groups" through vikunja_scope
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)
teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, provider)
if len(errs) > 0 {
for _, err := range errs {
log.Errorf("Error creating teams for user and vikunja groups %s: %v", cl.VikunjaGroups, err)
}
}
//find old teams for user through oidc
oldOidcTeams, _ := models.FindAllOidcTeamIDsForUser(s, u.ID)
oldOidcTeams, err := models.FindAllOidcTeamIDsForUser(s, u.ID)
if err != nil {
log.Errorf("No Oidc Teams found for user", err)
}
var oidcTeams []int64
if len(teamData) > 0 {
// check if we have seen these teams before.
@ -220,20 +222,21 @@ func HandleCallback(c echo.Context) error {
return err
}
for _, team := range teams {
tm := models.TeamMember{TeamID: team.ID, Username: u.Username}
tm := models.TeamMember{TeamID: team.ID, UserID: u.ID, Username: u.Username}
exists, err := tm.CheckMembership(s)
if !exists {
err = tm.Create(s, u)
if err != nil {
log.Debugf("Could not assign %v to %v. %v", u.Username, team.Name, err)
log.Errorf("Could not assign %v to %v. %v", u.Username, team.Name, err)
}
} else {
log.Debugf("Team exists? %v or error: %v", exists, err)
}
oidcTeams = append(oidcTeams, team.ID)
}
}
SignOutFromOrDeleteTeamsByID(s, u, notIn(oldOidcTeams, oidcTeams))
errs = SignOutFromOrDeleteTeamsByID(s, u, utils.NotIn(oldOidcTeams, oidcTeams))
for _, err := range errs {
log.Errorf("Found Error while signing out from teams %v", err)
}
err = s.Commit()
if err != nil {
_ = s.Rollback()
@ -244,26 +247,35 @@ func HandleCallback(c echo.Context) error {
return auth.NewUserAuthTokenResponse(u, c, false)
}
func SignOutFromOrDeleteTeamsByID(s *xorm.Session, u *user.User, teamIDs []int64) {
func SignOutFromOrDeleteTeamsByID(s *xorm.Session, u *user.User, teamIDs []int64) (errs []error) {
errs = []error{}
for _, teamID := range teamIDs {
tm := models.TeamMember{TeamID: teamID, Username: u.Username}
err := tm.Delete(s, u)
if err != nil {
tm := models.TeamMember{TeamID: teamID, UserID: u.ID, Username: u.Username}
exists, err := tm.CheckMembership(s)
memberCount, _ := tm.GetMemberCount(s)
if !exists {
continue
}
err = tm.Delete(s, u)
// if you cannot delete the team_member
if err != nil || memberCount <= 1 {
team, err := models.GetTeamByID(s, teamID)
if err != nil {
log.Errorf("Cannot find team with id: %v, err: %v", teamID, err)
} else {
err = team.Delete(s, u)
if err != nil {
log.Errorf("Cannot delete team %v", err)
}
errs = append(errs, err)
continue
}
err = team.Delete(s, u)
if err != nil {
errs = append(errs, err)
}
}
}
return errs
}
func getTeamDataFromToken(groups interface{}, provider *Provider) (teamData []models.TeamData, err error) {
func getTeamDataFromToken(groups interface{}, provider *Provider) (teamData []models.TeamData, errs []error) {
teamData = []models.TeamData{}
errs = []error{}
if groups != nil {
el := groups.([]interface{})
for _, data := range el {
@ -280,24 +292,24 @@ func getTeamDataFromToken(groups interface{}, provider *Provider) (teamData []mo
if team["oidcID"] != nil {
switch t := team["oidcID"].(type) {
case int64:
oidcID = fmt.Sprintf("%v", team["oidcID"])
oidcID = strconv.FormatInt(team["oidcID"].(int64), 10)
case string:
oidcID = string(team["oidcID"].(string))
case float64:
fl := float64(team["oidcID"].(float64))
oidcID = fmt.Sprintf("%v", fl)
oidcID = strconv.FormatFloat(team["oidcID"].(float64), 'f', -1, 64)
default:
log.Errorf("No oidcID assigned for %v or type %v not supported", team, t)
}
}
if name == "" || oidcID == "" {
log.Errorf("Claim of your custom scope does not hold name or oidcID for automatic group assignment through oidc provider. Please check %s", provider.Name)
return teamData, &user.ErrOpenIDCustomScopeMalformed{}
errs = append(errs, &user.ErrOpenIDCustomScopeMalformed{})
continue
}
teamData = append(teamData, models.TeamData{TeamName: name, OidcID: oidcID, Description: description})
}
}
return teamData, nil
return teamData, errs
}
func CreateTeamWithData(s *xorm.Session, teamData models.TeamData, u *user.User) (team *models.Team, err error) {
@ -306,8 +318,6 @@ func CreateTeamWithData(s *xorm.Session, teamData models.TeamData, u *user.User)
Description: teamData.Description,
OidcID: teamData.OidcID,
}
log.Debugf("Teams: %v", tea.OidcID)
err = tea.Create(s, u)
return tea, err
}
@ -331,7 +341,6 @@ func GetOrCreateTeamsByOIDCAndNames(s *xorm.Session, teamData []models.TeamData,
}
}
log.Debugf("Array: %v", te)
return te, err
}
@ -407,23 +416,3 @@ func getOrCreateUser(s *xorm.Session, cl *claims, issuer, subject string) (u *us
return
}
// find the elements which appear in slice1,but not in slice2
func notIn(slice1 []int64, slice2 []int64) []int64 {
var diff []int64
for _, s1 := range slice1 {
found := false
for _, s2 := range slice2 {
if s1 == s2 {
found = true
break
}
}
// String not found. We add it to return slice
if !found {
diff = append(diff, s1)
}
}
return diff
}