vikunja/vikunja
6
43
Fork 73
Code Issues 150 Pull Requests 21 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 +8 -8
2 changed files with 8 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a3e4449b7b - Show all commits

4
pkg/models/teams.go
View File

@ -94,8 +94,8 @@ type TeamUser struct {
TeamID int64 `json:"-"`
}
// TeamData is the relevant data for a team and is delivered by oidc token
type TeamData struct {
// OIDCTeamData is the relevant data for a team and is delivered by oidc token
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-02-13 10:18:58 +00:00
Outdated
Review
Copy Link

What is this used for? Please add a comment.

What is this used for? Please add a comment.
konrad commented 2023-02-14 11:39:47 +00:00
Outdated
Review
Copy Link

What's the difference to the Team struct?

What's the difference to the `Team` struct?
viehlieb commented 2023-02-23 14:54:33 +00:00
Outdated
Review
Copy Link

Intermediate struct which only holds TeamName and OidcId Description.
It does not exist as a Team yet or better: it is the data accessible via oidc, which the Team struct is compared against.

Intermediate struct which only holds TeamName and OidcId Description. It does not exist as a Team yet or better: it is the data accessible via oidc, which the Team struct is compared against.
konrad commented 2023-02-23 15:25:50 +00:00
Outdated
Review
Copy Link

Okay, but isn't it missing the json tags then? I'm not sure if I understood you correctly herre.

If it's only used for oidc data the name should reflect that. TeamData is too generic.

Okay, but isn't it missing the json tags then? I'm not sure if I understood you correctly herre. If it's only used for oidc data the name should reflect that. `TeamData` is too generic.
viehlieb commented 2023-02-23 15:33:00 +00:00
Outdated
Review
Copy Link

No, the teamData has to be pulled out of token via

getTeamDataFromToken

Called it OIDCTeamData now.

No, the teamData has to be pulled out of token via `getTeamDataFromToken` Called it OIDCTeamData now.
type OIDCTeamData struct {
TeamName string
OidcID string
Description string

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

@ -231,7 +231,7 @@ func HandleCallback(c echo.Context) error {
return auth.NewUserAuthTokenResponse(u, c, false)
}
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-04-17 16:21:36 +00:00
Outdated
Review
Copy Link

Replace with

if len(teamData) < 1 {
	return
}
Replace with ``` if len(teamData) < 1 { return }
viehlieb commented 2023-05-03 15:09:07 +00:00
Outdated
Review
Copy Link

It could well be, that only a single team is received through the oidc token.

It could well be, that only a single team is received through the oidc token.
konrad commented 2023-05-06 16:13:14 +00:00
Outdated
Review
Copy Link

But len(teamData) < 1 will only be true if there are 0 teams received?

I should have phrased it better, this is equivalent:

if len(teamData) == 0 {
	return
}
But `len(teamData) < 1` will only be true if there are 0 teams received? I should have phrased it better, this is equivalent: ``` if len(teamData) == 0 { return } ```
viehlieb commented 2023-05-08 11:43:19 +00:00
Outdated
Review
Copy Link

yes, that's true of course. i do not know why i read it differently.

yes, that's true of course. i do not know why i read it differently.
👍 1
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-02-13 10:25:53 +00:00
Outdated
Review
Copy Link

This should be an Error log message.

This should be an `Error` log message.
func AssignOrCreateUserToTeams(s *xorm.Session, u *user.User, teamData []models.TeamData) (oidcTeams []int64, err error) {
func AssignOrCreateUserToTeams(s *xorm.Session, u *user.User, teamData []models.OIDCTeamData) (oidcTeams []int64, err error) {
if len(teamData) > 0 {
// check if we have seen these teams before.
// find or create Teams and assign user as teammember.
@ -277,8 +277,8 @@ func SignOutFromTeamsByID(s *xorm.Session, u *user.User, teamIDs []int64) (errs
return errs
}
func getTeamDataFromToken(groups []map[string]interface{}, provider *Provider) (teamData []models.TeamData, errs []error) {
teamData = []models.TeamData{}
func getTeamDataFromToken(groups []map[string]interface{}, provider *Provider) (teamData []models.OIDCTeamData, errs []error) {
teamData = []models.OIDCTeamData{}
errs = []error{}
for _, team := range groups {
var name string
@ -307,12 +307,12 @@ func getTeamDataFromToken(groups []map[string]interface{}, provider *Provider) (
errs = append(errs, &user.ErrOpenIDCustomScopeMalformed{})
continue
}
teamData = append(teamData, models.TeamData{TeamName: name, OidcID: oidcID, Description: description})
teamData = append(teamData, models.OIDCTeamData{TeamName: name, OidcID: oidcID, Description: description})
}
return teamData, errs
}
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-04-17 16:24:11 +00:00
Outdated
Review
Copy Link

Please rename this to team.

Please rename this to `team`.
konrad commented 2023-10-18 15:20:28 +00:00
Outdated
Review
Copy Link

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

What's the advantage of this method over `team.Create`?
viehlieb commented 2023-11-07 14:16:57 +00:00
Outdated
Review
Copy Link

i think it was cyclomatic complexity.

i think it was cyclomatic complexity.
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-02-13 10:37:50 +00:00
Outdated
Review
Copy Link

Please make this debug message more descriptive.

Please make this debug message more descriptive.
func CreateTeamWithData(s *xorm.Session, teamData models.TeamData, u *user.User) (team *models.Team, err error) {
func CreateTeamWithData(s *xorm.Session, teamData models.OIDCTeamData, u *user.User) (team *models.Team, err error) {
tea := &models.Team{
Name: teamData.TeamName,
Description: teamData.Description,
@ -323,7 +323,7 @@ func CreateTeamWithData(s *xorm.Session, teamData models.TeamData, u *user.User)
}
// this functions creates an array of existing teams that was generated from the oidc data.
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-04-17 16:25:19 +00:00
Outdated
Review
Copy Link

Which oidcID? In the teamData? Isn't that always a string, due to the type system? Then the comment is redundant, please remove or clarify further.

Which oidcID? In the `teamData`? Isn't that always a string, due to the type system? Then the comment is redundant, please remove or clarify further.
func GetOrCreateTeamsByOIDCAndNames(s *xorm.Session, teamData []models.TeamData, u *user.User) (te []*models.Team, err error) {
func GetOrCreateTeamsByOIDCAndNames(s *xorm.Session, teamData []models.OIDCTeamData, u *user.User) (te []*models.Team, err error) {
te = []*models.Team{}
// Procedure can only be successful if oidcID is set and converted to string
for _, oidcTeam := range teamData {