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
3 changed files with 11 additions and 13 deletions
Showing only changes of commit ec39d9f02a - Show all commits

View File

@ -203,7 +203,7 @@ ratelimit:
# Possible values are "keyvalue", "memory" or "redis".
# When choosing "keyvalue" this setting follows the one configured in the "keyvalue" section.
store: keyvalue
# The number of requests a user can make from the same IP to all unauthenticated routes (login, register,
# The number of requests a user can make from the same IP to all unauthenticated routes (login, register,
# password confirmation, email verification, password reset request) per minute. This limit cannot be disabled.
# You should only change this if you know what you're doing.
noauthlimit: 10
@ -326,7 +326,7 @@ auth:
# The client secret used to authenticate Vikunja at the OpenID Connect provider.
clientsecret:
# The scope necessary to use oidc.
# If you want to use the Feature to create and assign to vikunja teams via oidc, you have to add the custom "vikunja_scope" and check [openid.md](https://kolaente.dev/vikunja/api/src/branch/main/pkg/modules/auth/openid/openid.md)
# If you want to use the Feature to create and assign to vikunja teams via oidc, you have to add the custom "vikunja_scope" and check [openid.md](https://vikunja.io/docs/openid/)
# e.g. scope: openid email profile vikunja_scope
scope: openid email profile

View File

@ -1081,7 +1081,7 @@ func (err ErrTeamDoesNotExist) HTTPError() web.HTTPError {
return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTeamDoesNotExist, Message: "This team does not exist."}
}
// ErrTeamAlreadyHasAccess represents an error where a team already has access to a list/namespace
// ErrTeamAlreadyHasAccess represents an error where a team already has access to a project
type ErrTeamAlreadyHasAccess struct {
TeamID int64
ID int64

View File

@ -286,13 +286,8 @@ func RemoveUserFromTeamsByIds(s *xorm.Session, u *user.User, teamIDs []int64) (e
return nil
}
viehlieb marked this conversation as resolved Outdated

Please use `strconv.FormatInt

Please use `strconv.FormatInt

done

done
strSlice := make([]string, len(teamIDs))
for i, num := range teamIDs {
strSlice[i] = strconv.FormatInt(num, 10)
}
log.Debugf("Removing team_member with user_id %v from team_ids %v", u.ID, strings.Join(strSlice, ","))
_, err = s.Where("team_id IN (?) AND user_id = ?", strings.Join(strSlice, ","), u.ID).Delete(&models.TeamMember{})
log.Debugf("Removing team_member with user_id %v from team_ids %v", u.ID, teamIDs)
_, err = s.In("team_id", teamIDs).And("user_id = ?", u.ID).Delete(&models.TeamMember{})
return err
}
viehlieb marked this conversation as resolved Outdated

Please use xorm's In function because that allows you to pass an int slice directly and avoids the need for the conversion you're doing.

Please use xorm's `In` function because that allows you to pass an int slice directly and avoids the need for the conversion you're doing.
viehlieb marked this conversation as resolved Outdated

Please use strconv.FormatFloat (not 100% sure if the function is really called that)

Please use `strconv.FormatFloat` (not 100% sure if the function is really called that)
@ -303,13 +298,16 @@ func getTeamDataFromToken(groups []map[string]interface{}, provider *Provider) (
var name string
var description string
var oidcID string
viehlieb marked this conversation as resolved Outdated

The return here will mean if one team claim is invalid, it will not try to validate any others. Is that what you intended?

The return here will mean if one team claim is invalid, it will not try to validate any others. Is that what you intended?

changed behaviour to skipping the invalid team

changed behaviour to skipping the invalid team
if team["name"] != nil {
_, exists := team["name"]
if exists {
name = team["name"].(string)
viehlieb marked this conversation as resolved Outdated

Please use _, exists := team["name"] to check if the field exists (exists is a boolean indicating whether the field exists). Same for the other fields.

Please use `_, exists := team["name"]` to check if the field exists (`exists` is a boolean indicating whether the field exists). Same for the other fields.
}
if team["description"] != nil {
_, exists = team["description"]
if exists {
description = team["description"].(string)
}
if team["oidcID"] != nil {
_, exists = team["oidcID"]
if exists {
switch t := team["oidcID"].(type) {
case int64:
oidcID = strconv.FormatInt(team["oidcID"].(int64), 10)
viehlieb marked this conversation as resolved Outdated

Please rename this to team.

Please rename this to `team`.

What's the advantage of this method over team.Create?

What's the advantage of this method over `team.Create`?

i think it was cyclomatic complexity.

i think it was cyclomatic complexity.