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

6
pkg/modules/auth/openid/providers.go
View File

@ -125,6 +125,10 @@ func getProviderFromMap(pi map[string]interface{}) (provider *Provider, err erro
logoutURL = ""
}
scope, _ := pi["scope"].(string)
if scope == "" {
scope = "openid profile email"
}
provider = &Provider{
Name: pi["name"].(string),
Key: k,
@ -132,7 +136,7 @@ func getProviderFromMap(pi map[string]interface{}) (provider *Provider, err erro
OriginalAuthURL: pi["authurl"].(string),
ClientSecret: pi["clientsecret"].(string),
LogoutURL: logoutURL,
Scope: pi["scope"].(string),
Scope: scope,
konrad commented 2023-12-01 15:05:21 +00:00
Outdated
Review
Copy Link

Aren't we always expecting the scope to be vikunja_groups? Why make it configurable then instead of always appending it?

Aren't we always expecting the scope to be `vikunja_groups`? Why make it configurable then instead of always appending it?
viehlieb commented 2023-12-06 13:05:55 +00:00
Outdated
Review
Copy Link

If you have a look at:

src/helpers/redirectToProvider.ts Line 18 in 3643ffe0d0
window.location.href = `${provider.authUrl}?client_id=${provider.clientId}&redirect_uri=${redirectUrl}${provider.key}&response_type=code&scope=openid email profile&state=${state}`

You'll see the hardcoded scope.
But to receive more information instead of openid profile email from the oidc provider you'll need an extra scope.

There was a PR on the frontend explaining this issue.

vikunja/frontend#2749

This part makes sure there is always the "openid profile email" scope set - as it was before, hardcoded.. But if you want to add a custom scope in the config.yml, then you have to tell the provider in back and frontend to actually use it.

	scope, _ := pi["scope"].(string)
	if scope == "" {
		scope = "openid profile email"
	}
If you have a look at: https://kolaente.dev/vikunja/frontend/src/commit/3643ffe0d0357c89cb3517fafbb0c438188ac88d/src/helpers/redirectToProvider.ts#L18 You'll see the hardcoded scope. But to receive more information instead of openid profile email from the oidc provider you'll need an extra scope. There was a PR on the frontend explaining this issue. https://kolaente.dev/vikunja/frontend/pulls/2749 This part makes sure there is always the "openid profile email" scope set - as it was before, hardcoded.. But if you want to add a custom scope in the config.yml, then you have to tell the provider in back and frontend to actually use it. ``` scope, _ := pi["scope"].(string) if scope == "" { scope = "openid profile email" } ```
konrad commented 2023-12-12 22:35:22 +00:00
Outdated
Review
Copy Link

Yes it is hardcoded in the frontend, but why make it a manual config setting instead of passing it automatically from the api to the provider? That would allow us to get rid of the extra config variable.

Yes it is hardcoded in the frontend, but why make it a manual config setting instead of passing it automatically from the api to the provider? That would allow us to get rid of the extra config variable.
viehlieb commented 2023-12-13 09:38:19 +00:00
Outdated
Review
Copy Link

So you suggest to add:
Scope: "openid profile email vikunja_scope"

Anyway it has to be explained what "vikunja_scope" is and how it is used. I think it might be helpful for admins to use the scopes explicitly.
But I do not have a strong opinion on that

So you suggest to add: `Scope: "openid profile email vikunja_scope"` Anyway it has to be explained what "vikunja_scope" is and how it is used. I think it might be helpful for admins to use the scopes explicitly. But I do not have a strong opinion on that
konrad commented 2023-12-13 16:55:34 +00:00
Outdated
Review
Copy Link

Okay, now I got you. Sorry for the confusion. Let's keep it the way you intended, please add an example scope to the config.yml.sample file.

Okay, now I got you. Sorry for the confusion. Let's keep it the way you intended, please add an example scope to the `config.yml.sample` file.
viehlieb commented 2024-02-23 14:23:49 +00:00
Outdated
Review
Copy Link

Done, it is also linked to the explanation in openid.md

Done, it is also linked to the explanation in openid.md
}
cl, is := pi["clientid"].(int)