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

10
docs/content/doc/setup/openid.md
View File

@ -57,20 +57,20 @@ You should see "(sso: *your_oidcID*)" written next to each team you were asigned
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-10-18 15:11:01 +00:00
Outdated
Review
Copy Link

This is unnecessary, please remove it

This is unnecessary, please remove it
*All examples assume one team called "team 1"*
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-10-18 15:11:57 +00:00
Outdated
Review
Copy Link

What kind of behaviour? Please rename it to something more specific.

And please don't capitalize the heading, same for the other headings.

What kind of behaviour? Please rename it to something more specific. And please don't capitalize the heading, same for the other headings.
1. *Token delivers team.name +team.oidc_id and Vikunja team does not exist:* \
New team will be created called "team 1" with attribute oidcId: "33929"
1. *Token delivers team.name +team.oidcID and Vikunja team does not exist:* \
New team will be created called "team 1" with attribute oidcID: "33929"
2. *In Vikunja Team with name "team 1" already exists in vikunja, but has no oidcID set:* \
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-10-18 15:12:50 +00:00
Outdated
Review
Copy Link

Is the attribute called oidcId or oidcID? The Authentik example was using oidcId.

Is the attribute called `oidcId` or `oidcID`? The Authentik example was using `oidcId`.
viehlieb commented 2023-11-07 14:01:11 +00:00
Outdated
Review
Copy Link

in authentik it is called oidcID

in migration:

type teams20230104152903 struct {
	OidcID string `xorm:"varchar(250) null" maxLength:"250" json:"oidc_id"`
}

Therefore in the db it is called oidc_id

I agree, there is an inconsistency in the .md file.

I try to fit the naming to the context.
Do you have suggested changes in renaming?

in authentik it is called `oidcID` in migration: ``` type teams20230104152903 struct { OidcID string `xorm:"varchar(250) null" maxLength:"250" json:"oidc_id"` } ``` Therefore in the db it is called oidc_id I agree, there is an inconsistency in the .md file. I try to fit the naming to the context. Do you have suggested changes in renaming?
konrad commented 2023-11-07 15:04:08 +00:00
Outdated
Review
Copy Link

I think the name is fine, but please use the same one everywhere.

I think the name is fine, but please use the same one everywhere.
👍 1
new team will be created called "team 1" with attribute oidc_id: "33929"
new team will be created called "team 1" with attribute oidcID: "33929"
3. *In Vikunja Team with name "team 1" already exists in vikunja, but has different oidcID set:* \
new team will be created called "team 1" with attribute oidcId: "33929"
new team will be created called "team 1" with attribute oidcID: "33929"
4. *In Vikunja Team with oidcID "33929" already exists in vikunja, but has different name than "team1":* \
new team will be created called "team 1" with attribute oidcId: "33929"
new team will be created called "team 1" with attribute oidcID: "33929"
5. *Scope vikunja_scope is not set:* \

2
pkg/models/teams.go
View File

@ -136,8 +136,6 @@ func GetTeamByOidcIDAndName(s *xorm.Session, oidcID string, teamName string) (*T
has, err := s.
Table("teams").
Where("oidc_id = ? AND name = ?", oidcID, teamName).
Asc("id").
Limit(1).
Get(team)
konrad marked this conversation as resolved Outdated
konrad commented 2023-02-23 15:27:05 +00:00
Outdated
Review
Copy Link

Did you try passing the

Did you try passing the
viehlieb commented 2023-03-10 12:11:15 +00:00
Outdated
Review
Copy Link

Ups, I guess here is something missing

Ups, I guess here is something missing
if !has || err != nil {
return nil, ErrOIDCTeamDoesNotExist{teamName, oidcID}
viehlieb marked this conversation as resolved Outdated
konrad commented 2023-10-18 15:14:35 +00:00
Outdated
Review
Copy Link

Get will always return one entry, no need for Asc or Limit.

`Get` will always return one entry, no need for `Asc` or `Limit`.