From 75ff537915b84e1113446b3e5bacbc8b3ee5a741 Mon Sep 17 00:00:00 2001 From: Vivek S Sejpal Date: Fri, 13 Mar 2026 19:05:57 -0700 Subject: [PATCH 1/6] fix: backend logout URL call on sign out (#3172) (#3352) * Fix backend logout URL call on sign out (#3172) Signed-off-by: Vivek Sejpal * doc: changelog entry for #3352 Signed-off-by: Jan Larwig --------- Signed-off-by: Vivek Sejpal Signed-off-by: Jan Larwig Co-authored-by: Jan Larwig --- CHANGELOG.md | 2 ++ oauthproxy.go | 6 ++++-- oauthproxy_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4542945f..6a284fa6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.14.3 +- [#3352](https://github.com/oauth2-proxy/oauth2-proxy/pull/3352) fix: backend logout URL call on sign out (#3172)(@vsejpal) + # V7.14.3 ## Release Highlights diff --git a/oauthproxy.go b/oauthproxy.go index 508084c8..d140f6a7 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -746,6 +746,10 @@ func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) { p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) return } + // Call backend logout before clearing the session so we still have the session + // (and id_token) available to invoke the provider's logout endpoint + p.backendLogout(rw, req) + err = p.ClearSessionCookie(rw, req) if err != nil { logger.Errorf("Error clearing session cookie: %v", err) @@ -753,8 +757,6 @@ func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) { return } - p.backendLogout(rw, req) - http.Redirect(rw, req, redirect, http.StatusFound) } diff --git a/oauthproxy_test.go b/oauthproxy_test.go index ccabdbbd..b33bdfe5 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -2028,6 +2028,44 @@ func Test_noCacheHeaders(t *testing.T) { }) } +func TestSignOutCallsBackendLogoutURL(t *testing.T) { + const testIDToken = "test-id-token-12345" + var receivedURL string + backendLogoutServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + receivedURL = req.URL.String() + rw.WriteHeader(http.StatusOK) + })) + defer backendLogoutServer.Close() + + opts := baseTestOptions() + opts.Providers[0].BackendLogoutURL = backendLogoutServer.URL + "/logout?id_token_hint={id_token}" + err := validation.Validate(opts) + require.NoError(t, err) + + proxy, err := NewOAuthProxy(opts, func(string) bool { return true }) + require.NoError(t, err) + + // Save a session with IDToken so backend logout can use it + rw := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/", nil) + err = proxy.sessionStore.Save(rw, req, &sessions.SessionState{ + Email: "user@example.com", + IDToken: testIDToken, + }) + require.NoError(t, err) + cookie := rw.Header().Values("Set-Cookie")[0] + + // Hit sign_out with the session cookie; backend logout should be called before session is cleared + signOutReq := httptest.NewRequest(http.MethodGet, "/oauth2/sign_out", nil) + signOutReq.Header.Set("Cookie", cookie) + rec := httptest.NewRecorder() + proxy.ServeHTTP(rec, signOutReq) + + assert.Equal(t, http.StatusFound, rec.Code, "sign_out should redirect") + assert.Contains(t, receivedURL, "id_token_hint="+testIDToken, + "backend logout URL should have been called with id_token from session") +} + func baseTestOptions() *options.Options { opts := options.NewOptions() opts.Cookie.Secret = rawCookieSecret From 5f446c3e00674f032e2e13491dd7020466701a25 Mon Sep 17 00:00:00 2001 From: Br1an <932039080@qq.com> Date: Sat, 14 Mar 2026 10:08:19 +0800 Subject: [PATCH 2/6] fix(devcontainer): bump Go version to 1.25 in devcontainer base image (#3366) The devcontainer was using Go 1.23 but go.mod requires Go 1.25.0. This caused 'go mod tidy' to fail in the devcontainer environment. Signed-off-by: Jan Larwig --- .devcontainer/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 89d97f30..7b6d5bba 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -1,4 +1,4 @@ -FROM mcr.microsoft.com/vscode/devcontainers/go:1-1.23 +FROM mcr.microsoft.com/vscode/devcontainers/go:1-1.25 SHELL ["/bin/bash", "-o", "pipefail", "-c"] From 566b3aac9ff253f002cfe801664bdfdee7c4a27d Mon Sep 17 00:00:00 2001 From: Francois Botha Date: Sat, 14 Mar 2026 04:36:24 +0200 Subject: [PATCH 3/6] ci: distribute windows binary with .exe extension (#3332) * Ensure Windows binary has .exe extension Signed-off-by: Jan Larwig * doc: add changelog for #3332 Signed-off-by: Jan Larwig --------- Signed-off-by: Jan Larwig Co-authored-by: Jan Larwig --- CHANGELOG.md | 1 + dist.sh | 13 ++++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a284fa6..db0be632 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ## Changes since v7.14.3 - [#3352](https://github.com/oauth2-proxy/oauth2-proxy/pull/3352) fix: backend logout URL call on sign out (#3172)(@vsejpal) +- [#3332](https://github.com/oauth2-proxy/oauth2-proxy/pull/3332) ci: distribute windows binary with .exe extension (@igitur) # V7.14.3 diff --git a/dist.sh b/dist.sh index 0990fff2..bcbd2292 100755 --- a/dist.sh +++ b/dist.sh @@ -30,16 +30,23 @@ for ARCH in "${ARCHS[@]}"; do GO_OS=$(echo $ARCH | awk -F- '{print $1}') GO_ARCH=$(echo $ARCH | awk -F- '{print $2}') + # Determine binary name based on OS + if [[ "${GO_OS}" == "windows" ]]; then + BINARY_NAME="${BINARY}.exe" + else + BINARY_NAME="${BINARY}" + fi + # Create architecture specific binaries if [[ ${GO_ARCH} == armv* ]]; then GO_ARM=$(echo $GO_ARCH | awk -Fv '{print $2}') GO111MODULE=on GOOS=${GO_OS} GOARCH=arm GOARM=${GO_ARM} CGO_ENABLED=0 go build \ -ldflags="-X github.com/oauth2-proxy/oauth2-proxy/v7/pkg/version.VERSION=${VERSION}" \ - -o release/${BINARY}-${VERSION}.${ARCH}/${BINARY} . + -o release/${BINARY}-${VERSION}.${ARCH}/${BINARY_NAME} . else GO111MODULE=on GOOS=${GO_OS} GOARCH=${GO_ARCH} CGO_ENABLED=0 go build \ -ldflags="-X github.com/oauth2-proxy/oauth2-proxy/v7/pkg/version.VERSION=${VERSION}" \ - -o release/${BINARY}-${VERSION}.${ARCH}/${BINARY} . + -o release/${BINARY}-${VERSION}.${ARCH}/${BINARY_NAME} . fi cd release @@ -51,7 +58,7 @@ for ARCH in "${ARCHS[@]}"; do sha256sum ${BINARY}-${VERSION}.${ARCH}.tar.gz > ${BINARY}-${VERSION}.${ARCH}.tar.gz-sha256sum.txt # Create sha256sum for architecture specific binary - sha256sum ${BINARY}-${VERSION}.${ARCH}/${BINARY} > ${BINARY}-${VERSION}.${ARCH}-sha256sum.txt + sha256sum ${BINARY}-${VERSION}.${ARCH}/${BINARY_NAME} > ${BINARY}-${VERSION}.${ARCH}-sha256sum.txt cd .. done From 6d272214e1d17e1af79b97195e76e7b4c4e158d2 Mon Sep 17 00:00:00 2001 From: Mridul <111583945+YMridul18@users.noreply.github.com> Date: Sat, 14 Mar 2026 08:28:26 +0530 Subject: [PATCH 4/6] docs: fix plural typo in gitlab provider flag (#3363) * doc: fix plural typo in gitlab provider flag Signed-off-by: Mridul Yadav * doc: fix plural typo in gitlab provider flag in versioned docs Signed-off-by: Jan Larwig --------- Signed-off-by: Mridul Yadav Signed-off-by: Jan Larwig Co-authored-by: Mridul Yadav Co-authored-by: Jan Larwig --- docs/docs/configuration/providers/gitlab.md | 2 +- .../version-7.10.x/configuration/providers/gitlab.md | 2 +- .../version-7.11.x/configuration/providers/gitlab.md | 2 +- .../version-7.12.x/configuration/providers/gitlab.md | 2 +- .../version-7.13.x/configuration/providers/gitlab.md | 2 +- .../version-7.14.x/configuration/providers/gitlab.md | 2 +- .../version-7.6.x/configuration/providers/gitlab.md | 2 +- .../version-7.7.x/configuration/providers/gitlab.md | 2 +- .../version-7.8.x/configuration/providers/gitlab.md | 2 +- .../version-7.9.x/configuration/providers/gitlab.md | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/docs/configuration/providers/gitlab.md b/docs/docs/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/docs/configuration/providers/gitlab.md +++ b/docs/docs/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage diff --git a/docs/versioned_docs/version-7.10.x/configuration/providers/gitlab.md b/docs/versioned_docs/version-7.10.x/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/versioned_docs/version-7.10.x/configuration/providers/gitlab.md +++ b/docs/versioned_docs/version-7.10.x/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage diff --git a/docs/versioned_docs/version-7.11.x/configuration/providers/gitlab.md b/docs/versioned_docs/version-7.11.x/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/versioned_docs/version-7.11.x/configuration/providers/gitlab.md +++ b/docs/versioned_docs/version-7.11.x/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage diff --git a/docs/versioned_docs/version-7.12.x/configuration/providers/gitlab.md b/docs/versioned_docs/version-7.12.x/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/versioned_docs/version-7.12.x/configuration/providers/gitlab.md +++ b/docs/versioned_docs/version-7.12.x/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage diff --git a/docs/versioned_docs/version-7.13.x/configuration/providers/gitlab.md b/docs/versioned_docs/version-7.13.x/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/versioned_docs/version-7.13.x/configuration/providers/gitlab.md +++ b/docs/versioned_docs/version-7.13.x/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage diff --git a/docs/versioned_docs/version-7.14.x/configuration/providers/gitlab.md b/docs/versioned_docs/version-7.14.x/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/versioned_docs/version-7.14.x/configuration/providers/gitlab.md +++ b/docs/versioned_docs/version-7.14.x/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage diff --git a/docs/versioned_docs/version-7.6.x/configuration/providers/gitlab.md b/docs/versioned_docs/version-7.6.x/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/versioned_docs/version-7.6.x/configuration/providers/gitlab.md +++ b/docs/versioned_docs/version-7.6.x/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage diff --git a/docs/versioned_docs/version-7.7.x/configuration/providers/gitlab.md b/docs/versioned_docs/version-7.7.x/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/versioned_docs/version-7.7.x/configuration/providers/gitlab.md +++ b/docs/versioned_docs/version-7.7.x/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage diff --git a/docs/versioned_docs/version-7.8.x/configuration/providers/gitlab.md b/docs/versioned_docs/version-7.8.x/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/versioned_docs/version-7.8.x/configuration/providers/gitlab.md +++ b/docs/versioned_docs/version-7.8.x/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage diff --git a/docs/versioned_docs/version-7.9.x/configuration/providers/gitlab.md b/docs/versioned_docs/version-7.9.x/configuration/providers/gitlab.md index 4cdbbbe1..fe259ab2 100644 --- a/docs/versioned_docs/version-7.9.x/configuration/providers/gitlab.md +++ b/docs/versioned_docs/version-7.9.x/configuration/providers/gitlab.md @@ -8,7 +8,7 @@ title: GitLab | Flag | Toml Field | Type | Description | Default | | ------------------- | ----------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `--gitlab-group` | `gitlab_groups` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | | -| `--gitlab-projects` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | +| `--gitlab-project` | `gitlab_projects` | string \| list | restrict logins to members of any of these projects (may be given multiple times) formatted as `orgname/repo=accesslevel`. Access level should be a value matching [Gitlab access levels](https://docs.gitlab.com/ee/api/members.html#valid-access-levels), defaulted to 20 if absent | | ## Usage From c6355ee402ac906aca383f21b8794911a1122d64 Mon Sep 17 00:00:00 2001 From: Nick Nikolakakis Date: Sat, 14 Mar 2026 05:09:25 +0200 Subject: [PATCH 5/6] docs: add statusRewrites to Traefik Errors middleware example (#3360) Add statusRewrites (401 -> 302) to the ForwardAuth with Errors middleware configuration and a troubleshooting note explaining that without it, browsers may show a "Found." link instead of auto-redirecting to the identity provider. Fixes #3359 Signed-off-by: Nick Nikolakakis Signed-off-by: Jan Larwig Co-authored-by: Jan Larwig --- docs/docs/configuration/integrations/traefik.md | 8 ++++++++ .../version-7.10.x/configuration/integration.md | 8 ++++++++ .../version-7.11.x/configuration/integration.md | 8 ++++++++ .../version-7.12.x/configuration/integration.md | 8 ++++++++ .../version-7.13.x/configuration/integrations/traefik.md | 8 ++++++++ .../version-7.14.x/configuration/integrations/traefik.md | 8 ++++++++ 6 files changed, 48 insertions(+) diff --git a/docs/docs/configuration/integrations/traefik.md b/docs/docs/configuration/integrations/traefik.md index e4b64b94..43830d43 100644 --- a/docs/docs/configuration/integrations/traefik.md +++ b/docs/docs/configuration/integrations/traefik.md @@ -79,8 +79,16 @@ http: - "401-403" service: oauth-backend query: "/oauth2/sign_in?rd={url}" + statusRewrites: + "401": 302 ``` +:::caution Troubleshooting: Browser shows "Found." instead of redirecting +When using the Errors middleware without `statusRewrites`, the redirect response from oauth2-proxy can be served within the original 401/403 status context. This causes some browsers to display a "Found." link instead of automatically following the redirect to the identity provider. + +Adding `statusRewrites` to rewrite `401 -> 302` ensures the browser treats the response as a proper redirect and follows it automatically. +::: + ### ForwardAuth with static upstreams configuration Redirect to sign_in functionality provided without the use of `errors` middleware with [Traefik v2 `ForwardAuth` middleware](https://doc.traefik.io/traefik/middlewares/http/forwardauth/) pointing to oauth2-proxy service's `/` endpoint diff --git a/docs/versioned_docs/version-7.10.x/configuration/integration.md b/docs/versioned_docs/version-7.10.x/configuration/integration.md index a5afeda4..181cdbf9 100644 --- a/docs/versioned_docs/version-7.10.x/configuration/integration.md +++ b/docs/versioned_docs/version-7.10.x/configuration/integration.md @@ -225,8 +225,16 @@ http: - "401-403" service: oauth-backend query: "/oauth2/sign_in?rd={url}" + statusRewrites: + "401": 302 ``` +:::info Troubleshooting: Browser shows "Found." instead of redirecting +When using the Errors middleware without `statusRewrites`, the redirect response from oauth2-proxy can be served within the original 401/403 status context. This causes some browsers to display a "Found." link instead of automatically following the redirect to the identity provider. + +Adding `statusRewrites` to rewrite `401 -> 302` ensures the browser treats the response as a proper redirect and follows it automatically. +::: + ### ForwardAuth with static upstreams configuration Redirect to sign_in functionality provided without the use of `errors` middleware with [Traefik v2 `ForwardAuth` middleware](https://doc.traefik.io/traefik/middlewares/http/forwardauth/) pointing to oauth2-proxy service's `/` endpoint diff --git a/docs/versioned_docs/version-7.11.x/configuration/integration.md b/docs/versioned_docs/version-7.11.x/configuration/integration.md index a5afeda4..181cdbf9 100644 --- a/docs/versioned_docs/version-7.11.x/configuration/integration.md +++ b/docs/versioned_docs/version-7.11.x/configuration/integration.md @@ -225,8 +225,16 @@ http: - "401-403" service: oauth-backend query: "/oauth2/sign_in?rd={url}" + statusRewrites: + "401": 302 ``` +:::info Troubleshooting: Browser shows "Found." instead of redirecting +When using the Errors middleware without `statusRewrites`, the redirect response from oauth2-proxy can be served within the original 401/403 status context. This causes some browsers to display a "Found." link instead of automatically following the redirect to the identity provider. + +Adding `statusRewrites` to rewrite `401 -> 302` ensures the browser treats the response as a proper redirect and follows it automatically. +::: + ### ForwardAuth with static upstreams configuration Redirect to sign_in functionality provided without the use of `errors` middleware with [Traefik v2 `ForwardAuth` middleware](https://doc.traefik.io/traefik/middlewares/http/forwardauth/) pointing to oauth2-proxy service's `/` endpoint diff --git a/docs/versioned_docs/version-7.12.x/configuration/integration.md b/docs/versioned_docs/version-7.12.x/configuration/integration.md index a5afeda4..181cdbf9 100644 --- a/docs/versioned_docs/version-7.12.x/configuration/integration.md +++ b/docs/versioned_docs/version-7.12.x/configuration/integration.md @@ -225,8 +225,16 @@ http: - "401-403" service: oauth-backend query: "/oauth2/sign_in?rd={url}" + statusRewrites: + "401": 302 ``` +:::info Troubleshooting: Browser shows "Found." instead of redirecting +When using the Errors middleware without `statusRewrites`, the redirect response from oauth2-proxy can be served within the original 401/403 status context. This causes some browsers to display a "Found." link instead of automatically following the redirect to the identity provider. + +Adding `statusRewrites` to rewrite `401 -> 302` ensures the browser treats the response as a proper redirect and follows it automatically. +::: + ### ForwardAuth with static upstreams configuration Redirect to sign_in functionality provided without the use of `errors` middleware with [Traefik v2 `ForwardAuth` middleware](https://doc.traefik.io/traefik/middlewares/http/forwardauth/) pointing to oauth2-proxy service's `/` endpoint diff --git a/docs/versioned_docs/version-7.13.x/configuration/integrations/traefik.md b/docs/versioned_docs/version-7.13.x/configuration/integrations/traefik.md index e4b64b94..f6ea35f8 100644 --- a/docs/versioned_docs/version-7.13.x/configuration/integrations/traefik.md +++ b/docs/versioned_docs/version-7.13.x/configuration/integrations/traefik.md @@ -79,8 +79,16 @@ http: - "401-403" service: oauth-backend query: "/oauth2/sign_in?rd={url}" + statusRewrites: + "401": 302 ``` +:::info Troubleshooting: Browser shows "Found." instead of redirecting +When using the Errors middleware without `statusRewrites`, the redirect response from oauth2-proxy can be served within the original 401/403 status context. This causes some browsers to display a "Found." link instead of automatically following the redirect to the identity provider. + +Adding `statusRewrites` to rewrite `401 -> 302` ensures the browser treats the response as a proper redirect and follows it automatically. +::: + ### ForwardAuth with static upstreams configuration Redirect to sign_in functionality provided without the use of `errors` middleware with [Traefik v2 `ForwardAuth` middleware](https://doc.traefik.io/traefik/middlewares/http/forwardauth/) pointing to oauth2-proxy service's `/` endpoint diff --git a/docs/versioned_docs/version-7.14.x/configuration/integrations/traefik.md b/docs/versioned_docs/version-7.14.x/configuration/integrations/traefik.md index e4b64b94..f6ea35f8 100644 --- a/docs/versioned_docs/version-7.14.x/configuration/integrations/traefik.md +++ b/docs/versioned_docs/version-7.14.x/configuration/integrations/traefik.md @@ -79,8 +79,16 @@ http: - "401-403" service: oauth-backend query: "/oauth2/sign_in?rd={url}" + statusRewrites: + "401": 302 ``` +:::info Troubleshooting: Browser shows "Found." instead of redirecting +When using the Errors middleware without `statusRewrites`, the redirect response from oauth2-proxy can be served within the original 401/403 status context. This causes some browsers to display a "Found." link instead of automatically following the redirect to the identity provider. + +Adding `statusRewrites` to rewrite `401 -> 302` ensures the browser treats the response as a proper redirect and follows it automatically. +::: + ### ForwardAuth with static upstreams configuration Redirect to sign_in functionality provided without the use of `errors` middleware with [Traefik v2 `ForwardAuth` middleware](https://doc.traefik.io/traefik/middlewares/http/forwardauth/) pointing to oauth2-proxy service's `/` endpoint From e59f7c1549aeee85e96f771694ac55cf8a594edb Mon Sep 17 00:00:00 2001 From: af su Date: Sat, 14 Mar 2026 12:04:33 +0800 Subject: [PATCH 6/6] feat: allow arbitrary claims from the IDToken and IdentityProvider UserInfo endpoint to be added to the session state (#2685) * feat: support additional claims Signed-off-by: afsu Signed-off-by: af su * docs: clarify that AdditionalClaims may come from id_token or userinfo endpoint Signed-off-by: afsu Signed-off-by: af su * feat: include AdditionalClaims in /oauth2/userinfo response (#834) Signed-off-by: afsu Signed-off-by: af su * refactor: extract coerceClaim logic into util Signed-off-by: afsu Signed-off-by: af su * doc: add changelog entry for #2685 Signed-off-by: Jan Larwig * refactor: added more verbose comments to some struct fields and minor code cleanup Signed-off-by: Jan Larwig --------- Signed-off-by: afsu Signed-off-by: af su Signed-off-by: Jan Larwig Co-authored-by: af su Co-authored-by: Jan Larwig --- CHANGELOG.md | 1 + docs/docs/configuration/alpha_config.md | 1 + oauthproxy.go | 10 +-- oauthproxy_test.go | 14 ++++ pkg/apis/options/providers.go | 3 + pkg/apis/sessions/session_state.go | 16 ++++- pkg/apis/sessions/session_state_test.go | 64 +++++++++++++++++ pkg/providers/util/claim_extractor.go | 84 +++------------------- pkg/providers/util/claim_extractor_test.go | 55 +------------- pkg/util/util.go | 64 +++++++++++++++++ pkg/util/util_test.go | 70 ++++++++++++++++++ providers/provider_data.go | 45 ++++++++++-- providers/provider_data_test.go | 23 ++++++ providers/providers.go | 1 + 14 files changed, 314 insertions(+), 137 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db0be632..0470479c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - [#3352](https://github.com/oauth2-proxy/oauth2-proxy/pull/3352) fix: backend logout URL call on sign out (#3172)(@vsejpal) - [#3332](https://github.com/oauth2-proxy/oauth2-proxy/pull/3332) ci: distribute windows binary with .exe extension (@igitur) +- [#2685](https://github.com/oauth2-proxy/oauth2-proxy/pull/2685) feat: allow arbitrary claims from the IDToken and IdentityProvider UserInfo endpoint to be added to the session state (@vegetablest) # V7.14.3 diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index b4a75582..b92b42f1 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -526,6 +526,7 @@ Provider holds all configuration for a single provider | `scope` | _string_ | Scope is the OAuth scope specification | | `allowedGroups` | _[]string_ | AllowedGroups is a list of restrict logins to members of this group | | `code_challenge_method` | _string_ | The code challenge method | +| `additionalClaims` | _[]string_ | Additional claims to be obtained from the upstream IDP, either from the id_token or from the userinfo endpoint if configured. | | `backendLogoutURL` | _string_ | URL to call to perform backend logout, `{id_token}` would be replaced by the actual `id_token` if available in the session | ### ProviderType diff --git a/oauthproxy.go b/oauthproxy.go index d140f6a7..895f61a2 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -721,15 +721,17 @@ func (p *OAuthProxy) UserInfo(rw http.ResponseWriter, req *http.Request) { } userInfo := struct { - User string `json:"user"` - Email string `json:"email"` - Groups []string `json:"groups,omitempty"` - PreferredUsername string `json:"preferredUsername,omitempty"` + User string `json:"user"` + Email string `json:"email"` + Groups []string `json:"groups,omitempty"` + PreferredUsername string `json:"preferredUsername,omitempty"` + AdditionalClaims map[string]interface{} `json:"additionalClaims,omitempty"` }{ User: session.User, Email: session.Email, Groups: session.Groups, PreferredUsername: session.PreferredUsername, + AdditionalClaims: session.AdditionalClaims, } if err := json.NewEncoder(rw).Encode(userInfo); err != nil { diff --git a/oauthproxy_test.go b/oauthproxy_test.go index b33bdfe5..69951375 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -1032,6 +1032,20 @@ func TestUserInfoEndpointAccepted(t *testing.T) { }, expectedResponse: "{\"user\":\"john.doe\",\"email\":\"john.doe@example.com\",\"groups\":[\"example\",\"groups\"],\"preferredUsername\":\"john\"}\n", }, + { + name: "With Additional Claim", + session: &sessions.SessionState{ + User: "john.doe", + PreferredUsername: "john", + Email: "john.doe@example.com", + Groups: []string{"example", "groups"}, + AccessToken: "my_access_token", + AdditionalClaims: map[string]interface{}{ + "foo": "bar", + }, + }, + expectedResponse: "{\"user\":\"john.doe\",\"email\":\"john.doe@example.com\",\"groups\":[\"example\",\"groups\"],\"preferredUsername\":\"john\",\"additionalClaims\":{\"foo\":\"bar\"}}\n", + }, } for _, tc := range testCases { diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index 94bdb592..55965ed9 100644 --- a/pkg/apis/options/providers.go +++ b/pkg/apis/options/providers.go @@ -134,6 +134,9 @@ type Provider struct { // The code challenge method CodeChallengeMethod string `yaml:"code_challenge_method,omitempty"` + // Additional claims to be obtained from the upstream IDP, either from the id_token or from the userinfo endpoint if configured. + AdditionalClaims []string `yaml:"additionalClaims,omitempty"` + // URL to call to perform backend logout, `{id_token}` would be replaced by the actual `id_token` if available in the session BackendLogoutURL string `yaml:"backendLogoutURL"` } diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go index a1f807ab..fef20aab 100644 --- a/pkg/apis/sessions/session_state.go +++ b/pkg/apis/sessions/session_state.go @@ -8,6 +8,7 @@ import ( "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util" "github.com/pierrec/lz4/v4" "github.com/vmihailenco/msgpack/v5" ) @@ -28,6 +29,9 @@ type SessionState struct { Groups []string `msgpack:"g,omitempty"` PreferredUsername string `msgpack:"pu,omitempty"` + // Additional claims + AdditionalClaims map[string]interface{} `msgpack:"ac,omitempty"` + // Internal helpers, not serialized Clock func() time.Time `msgpack:"-"` // override for time.Now, for testing Lock Lock `msgpack:"-"` @@ -156,10 +160,20 @@ func (s *SessionState) GetClaim(claim string) []string { case "preferred_username": return []string{s.PreferredUsername} default: - return []string{} + return s.getAdditionalClaim(claim) } } +func (s *SessionState) getAdditionalClaim(claim string) []string { + if value, ok := s.AdditionalClaims[claim]; ok { + var result []string + if err := util.CoerceClaim(value, &result); err == nil { + return result + } + } + return []string{} +} + // CheckNonce compares the Nonce against a potential hash of it func (s *SessionState) CheckNonce(hashed string) bool { return encryption.CheckNonce(s.Nonce, hashed) diff --git a/pkg/apis/sessions/session_state_test.go b/pkg/apis/sessions/session_state_test.go index 87b97614..1dc6d3ad 100644 --- a/pkg/apis/sessions/session_state_test.go +++ b/pkg/apis/sessions/session_state_test.go @@ -222,6 +222,23 @@ func TestEncodeAndDecodeSessionState(t *testing.T) { Nonce: []byte("abcdef1234567890abcdef1234567890"), Groups: []string{"group-a", "group-b"}, }, + "With additional claims": { + Email: "username@example.com", + User: "username", + PreferredUsername: "preferred.username", + AccessToken: "AccessToken.12349871293847fdsaihf9238h4f91h8fr.1349f831y98fd7", + IDToken: "IDToken.12349871293847fdsaihf9238h4f91h8fr.1349f831y98fd7", + CreatedAt: &created, + ExpiresOn: &expires, + RefreshToken: "RefreshToken.12349871293847fdsaihf9238h4f91h8fr.1349f831y98fd7", + Nonce: []byte("abcdef1234567890abcdef1234567890"), + Groups: []string{"group-a", "group-b"}, + AdditionalClaims: map[string]interface{}{ + "custom_claim_1": "value1", + "custom_claim_2": true, + "custom_claim_3": []interface{}{"item1", "item2"}, + }, + }, } for _, secretSize := range []int{16, 24, 32} { @@ -289,3 +306,50 @@ func compareSessionStates(t *testing.T, expected *SessionState, actual *SessionS act.ExpiresOn = nil assert.Equal(t, exp, act) } + +func TestGetClaim(t *testing.T) { + createdAt := time.Now() + expiresOn := createdAt.Add(1 * time.Hour) + + ss := &SessionState{ + CreatedAt: &createdAt, + ExpiresOn: &expiresOn, + AccessToken: "AccessToken.12349871293847fdsaihf9238h4f91h8fr.1349f831y98fd7", + IDToken: "IDToken.12349871293847fdsaihf9238h4f91h8fr.1349f831y98fd7", + RefreshToken: "RefreshToken.12349871293847fdsaihf9238h4f91h8fr.1349f831y98fd7", + Email: "user@example.com", + User: "user123", + Groups: []string{"group1", "group2"}, + PreferredUsername: "preferred_user", + AdditionalClaims: map[string]interface{}{ + "custom_claim_1": "value1", + "custom_claim_2": true, + "custom_claim_3": []string{"item1", "item2"}, + }, + } + + tests := []struct { + claim string + want []string + }{ + {"access_token", []string{"AccessToken.12349871293847fdsaihf9238h4f91h8fr.1349f831y98fd7"}}, + {"id_token", []string{"IDToken.12349871293847fdsaihf9238h4f91h8fr.1349f831y98fd7"}}, + {"refresh_token", []string{"RefreshToken.12349871293847fdsaihf9238h4f91h8fr.1349f831y98fd7"}}, + {"created_at", []string{createdAt.String()}}, + {"expires_on", []string{expiresOn.String()}}, + {"email", []string{"user@example.com"}}, + {"user", []string{"user123"}}, + {"groups", []string{"group1", "group2"}}, + {"preferred_username", []string{"preferred_user"}}, + {"custom_claim_1", []string{"value1"}}, + {"custom_claim_2", []string{"true"}}, + {"custom_claim_3", []string{"[\"item1\",\"item2\"]"}}, + } + + for _, tt := range tests { + t.Run(tt.claim, func(t *testing.T) { + gs := NewWithT(t) + gs.Expect(ss.GetClaim(tt.claim)).To(Equal(tt.want)) + }) + } +} diff --git a/pkg/providers/util/claim_extractor.go b/pkg/providers/util/claim_extractor.go index 9ab7a8c8..469a4f54 100644 --- a/pkg/providers/util/claim_extractor.go +++ b/pkg/providers/util/claim_extractor.go @@ -3,7 +3,6 @@ package util import ( "context" "encoding/base64" - "encoding/json" "fmt" "mime" "net/http" @@ -12,17 +11,17 @@ import ( "github.com/bitly/go-simplejson" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" - "github.com/spf13/cast" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util" ) // ClaimExtractor is used to extract claim values from an ID Token, or, if not // present, from the profile URL. type ClaimExtractor interface { // GetClaim fetches a named claim and returns the value. - GetClaim(claim string) (interface{}, bool, error) + GetClaim(claim string) (any, bool, error) // GetClaimInto fetches a named claim and puts the value into the destination. - GetClaimInto(claim string, dst interface{}) (bool, error) + GetClaimInto(claim string, dst any) (bool, error) } // NewClaimExtractor constructs a new ClaimExtractor from the raw ID Token. @@ -31,12 +30,12 @@ type ClaimExtractor interface { func NewClaimExtractor(ctx context.Context, idToken string, profileURL *url.URL, profileRequestHeaders http.Header) (ClaimExtractor, error) { payload, err := parseJWT(idToken) if err != nil { - return nil, fmt.Errorf("failed to parse ID Token: %v", err) + return nil, fmt.Errorf("failed to parse ID Token: %w", err) } tokenClaims, err := simplejson.NewJson(payload) if err != nil { - return nil, fmt.Errorf("failed to parse ID Token payload: %v", err) + return nil, fmt.Errorf("failed to parse ID Token payload: %w", err) } return &claimExtractor{ @@ -59,7 +58,7 @@ type claimExtractor struct { // GetClaim will return the value claim if it exists. // It will only return an error if the profile URL needs to be fetched due to // the claim not being present in the ID Token. -func (c *claimExtractor) GetClaim(claim string) (interface{}, bool, error) { +func (c *claimExtractor) GetClaim(claim string) (any, bool, error) { if claim == "" { return nil, false, nil } @@ -124,7 +123,7 @@ func (c *claimExtractor) loadProfileClaims() (*simplejson.Json, error) { // GetClaimInto loads a claim and places it into the destination interface. // This will attempt to coerce the claim into the specified type. // If it cannot be coerced, an error may be returned. -func (c *claimExtractor) GetClaimInto(claim string, dst interface{}) (bool, error) { +func (c *claimExtractor) GetClaimInto(claim string, dst any) (bool, error) { value, exists, err := c.GetClaim(claim) if err != nil { return false, fmt.Errorf("could not get claim %q: %v", claim, err) @@ -132,8 +131,8 @@ func (c *claimExtractor) GetClaimInto(claim string, dst interface{}) (bool, erro if !exists { return false, nil } - if err := coerceClaim(value, dst); err != nil { - return false, fmt.Errorf("could no coerce claim: %v", err) + if err := util.CoerceClaim(value, dst); err != nil { + return false, fmt.Errorf("could not coerce claim: %v", err) } return true, nil @@ -156,73 +155,10 @@ func parseJWT(p string) ([]byte, error) { // getClaimFrom gets a claim from a Json object. // It can accept either a single claim name or a json path. The claim is always evaluated first as a single claim name. // Paths with indexes are not supported. -func getClaimFrom(claim string, src *simplejson.Json) interface{} { +func getClaimFrom(claim string, src *simplejson.Json) any { if value, ok := src.CheckGet(claim); ok { return value.Interface() } claimParts := strings.Split(claim, ".") return src.GetPath(claimParts...).Interface() } - -// coerceClaim tries to convert the value into the destination interface type. -// If it can convert the value, it will then store the value in the destination -// interface. -func coerceClaim(value, dst interface{}) error { - switch d := dst.(type) { - case *string: - str, err := toString(value) - if err != nil { - return fmt.Errorf("could not convert value to string: %v", err) - } - *d = str - case *[]string: - strSlice, err := toStringSlice(value) - if err != nil { - return fmt.Errorf("could not convert value to string slice: %v", err) - } - *d = strSlice - case *bool: - *d = cast.ToBool(value) - default: - return fmt.Errorf("unknown type for destination: %T", dst) - } - return nil -} - -// toStringSlice converts an interface (either a slice or single value) into -// a slice of strings. -func toStringSlice(value interface{}) ([]string, error) { - var sliceValues []interface{} - switch v := value.(type) { - case []interface{}: - sliceValues = v - case interface{}: - sliceValues = []interface{}{v} - default: - sliceValues = cast.ToSlice(value) - } - - out := []string{} - for _, v := range sliceValues { - str, err := toString(v) - if err != nil { - return nil, fmt.Errorf("could not convert slice entry to string %v: %v", v, err) - } - out = append(out, str) - } - return out, nil -} - -// toString coerces a value into a string. -// If it is non-string, marshal it into JSON. -func toString(value interface{}) (string, error) { - if str, err := cast.ToStringE(value); err == nil { - return str, nil - } - - jsonStr, err := json.Marshal(value) - if err != nil { - return "", err - } - return string(jsonStr), nil -} diff --git a/pkg/providers/util/claim_extractor_test.go b/pkg/providers/util/claim_extractor_test.go index 4ce4606f..57e2e1dc 100644 --- a/pkg/providers/util/claim_extractor_test.go +++ b/pkg/providers/util/claim_extractor_test.go @@ -76,7 +76,7 @@ var _ = Describe("Claim Extractor Suite", func() { func(in newClaimExtractorTableInput) { _, err := NewClaimExtractor(context.Background(), in.idToken, nil, nil) if in.expectedError != nil { - Expect(err).To(MatchError(in.expectedError)) + Expect(err).To(MatchError(in.expectedError.Error())) } else { Expect(err).ToNot(HaveOccurred()) } @@ -405,7 +405,7 @@ var _ = Describe("Claim Extractor Suite", func() { into: "", expectExists: false, expectedValue: "", - expectedError: errors.New("could no coerce claim: unknown type for destination: string"), + expectedError: errors.New("could not coerce claim: unknown type for destination: string"), }), Entry("flattens a complex claim value into a JSON string", getClaimIntoTableInput{ testClaimExtractorOpts: testClaimExtractorOpts{ @@ -451,53 +451,6 @@ var _ = Describe("Claim Extractor Suite", func() { }), ) - type coerceClaimTableInput struct { - value interface{} - dst interface{} - expectedDst interface{} - expectedError error - } - - DescribeTable("coerceClaim", - func(in coerceClaimTableInput) { - err := coerceClaim(in.value, in.dst) - if in.expectedError != nil { - Expect(err).To(MatchError(in.expectedError)) - return - } - - Expect(err).ToNot(HaveOccurred()) - Expect(in.dst).To(Equal(in.expectedDst)) - }, - Entry("coerces a string to a string", coerceClaimTableInput{ - value: "some_string", - dst: stringPointer(""), - expectedDst: stringPointer("some_string"), - }), - Entry("coerces a slice to a string slice", coerceClaimTableInput{ - value: []interface{}{"a", "b"}, - dst: stringSlicePointer([]string{}), - expectedDst: stringSlicePointer([]string{"a", "b"}), - }), - Entry("coerces a bool to a bool", coerceClaimTableInput{ - value: true, - dst: boolPointer(false), - expectedDst: boolPointer(true), - }), - Entry("coerces a string to a bool", coerceClaimTableInput{ - value: "true", - dst: boolPointer(false), - expectedDst: boolPointer(true), - }), - Entry("coerces a map to a string", coerceClaimTableInput{ - value: map[string]interface{}{ - "foo": []interface{}{"bar", "baz"}, - }, - dst: stringPointer(""), - expectedDst: stringPointer("{\"foo\":[\"bar\",\"baz\"]}"), - }), - ) - It("should extract claims from a JWT response", func() { jwtResponsePayload := `{ "user": "jwtUser", @@ -605,10 +558,6 @@ func stringSlicePointer(in []string) *[]string { return &in } -func boolPointer(in bool) *bool { - return &in -} - // ****************************** // Different profile URL handlers // ****************************** diff --git a/pkg/util/util.go b/pkg/util/util.go index 0f3d70ad..207316e7 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -5,6 +5,7 @@ import ( "crypto/rsa" "crypto/x509" "crypto/x509/pkix" + "encoding/json" "fmt" "math/big" "net" @@ -12,6 +13,8 @@ import ( "os" "strings" "time" + + "github.com/spf13/cast" ) func GetCertPool(paths []string, useSystemPool bool) (*x509.CertPool, error) { @@ -191,3 +194,64 @@ func RemoveDuplicateStr(strSlice []string) []string { } return list } + +// CoerceClaim tries to convert the value into the destination interface type. +// If it can convert the value, it will then store the value in the destination +// interface. +func CoerceClaim(value, dst any) error { + switch d := dst.(type) { + case *string: + str, err := toString(value) + if err != nil { + return fmt.Errorf("could not convert value to string: %v", err) + } + *d = str + case *[]string: + strSlice, err := toStringSlice(value) + if err != nil { + return fmt.Errorf("could not convert value to string slice: %v", err) + } + *d = strSlice + case *bool: + *d = cast.ToBool(value) + default: + return fmt.Errorf("unknown type for destination: %T", dst) + } + return nil +} + +// toStringSlice converts an interface (either a slice or single value) into +// a slice of strings. +func toStringSlice(value any) ([]string, error) { + var sliceValues []any + switch v := value.(type) { + case []any: + sliceValues = v + default: + sliceValues = []any{v} + } + + out := []string{} + for _, v := range sliceValues { + str, err := toString(v) + if err != nil { + return nil, fmt.Errorf("could not convert slice entry to string %v: %v", v, err) + } + out = append(out, str) + } + return out, nil +} + +// toString coerces a value into a string. +// If it is non-string, marshal it into JSON. +func toString(value any) (string, error) { + if str, err := cast.ToStringE(value); err == nil { + return str, nil + } + + jsonStr, err := json.Marshal(value) + if err != nil { + return "", err + } + return string(jsonStr), nil +} diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 167c3e59..d2a2eeca 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -2,10 +2,13 @@ package util import ( "crypto/x509" + "encoding/json" "encoding/pem" "os" + "reflect" "testing" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" "github.com/stretchr/testify/assert" ) @@ -253,3 +256,70 @@ func TestGetCertPool(t *testing.T) { assert.Error(t, err3) } } + +type coerceClaimTableInput struct { + name string + value any + dst any + expectedDst any + expectedError error +} + +func TestCoerceClaim(t *testing.T) { + tests := []coerceClaimTableInput{ + { + name: "coerces a string to a string", + value: "some_string", + dst: ptr.To(""), + expectedDst: ptr.To("some_string"), + }, + { + name: "coerces a slice to a string slice", + value: []any{"a", "b"}, + dst: ptr.To([]string{}), + expectedDst: ptr.To([]string{"a", "b"}), + }, + { + name: "coerces a bool to a bool", + value: true, + dst: ptr.To(false), + expectedDst: ptr.To(true), + }, + { + name: "coerces a string to a bool", + value: "true", + dst: ptr.To(false), + expectedDst: ptr.To(true), + }, + { + name: "coerces a map to a string", + value: map[string]any{ + "foo": []any{"bar", "baz"}, + }, + dst: ptr.To(""), + expectedDst: ptr.To("{\"foo\":[\"bar\",\"baz\"]}"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := CoerceClaim(tt.value, tt.dst) + if tt.expectedError != nil { + if err == nil || err.Error() != tt.expectedError.Error() { + t.Errorf("expected error %v, got %v", tt.expectedError, err) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !reflect.DeepEqual(tt.dst, tt.expectedDst) { + gotJSON, _ := json.Marshal(tt.dst) + wantJSON, _ := json.Marshal(tt.expectedDst) + t.Errorf("expected dst to be %+v, got %+v", string(wantJSON), string(gotJSON)) + } + }) + } +} diff --git a/providers/provider_data.go b/providers/provider_data.go index 95de5c50..8f9d1e36 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -45,11 +45,26 @@ type ProviderData struct { SupportedCodeChallengeMethods []string `json:"code_challenge_methods_supported,omitempty"` // Common OIDC options for any OIDC-based providers to consume - AllowUnverifiedEmail bool - UserClaim string - EmailClaim string - GroupsClaim string - Verifier internaloidc.IDTokenVerifier + AllowUnverifiedEmail bool + + // UserClaim is the claim to use for populating the SessionState.User field. Defaults to "sub" if not set. + UserClaim string + + // EmailClaim is the claim to use for populating the SessionState.Email field. + EmailClaim string + + // GroupsClaim is the claim to use for populating the SessionState.Groups field. + // If not set, groups will not be extracted from the ID Token or userinfo response. + GroupsClaim string + + // Verifier is the OIDC ID Token Verifier to be used by any OIDC-based providers to verify ID Tokens returned by the provider. + // It must be set up by the provider implementation and is not expected to be configured directly by users. + Verifier internaloidc.IDTokenVerifier + + // Additional claims to be obtained from the upstream IDP, either from the id_token or from the userinfo endpoint if configured. + AdditionalClaims []string `json:"additionalClaims,omitempty"` + + // SkipClaimsFromProfileURL indicates that claims should not be fetched from the ProfileURL, even if it is set. SkipClaimsFromProfileURL bool // Universal Group authorization data structure @@ -268,6 +283,10 @@ func (p *ProviderData) buildSessionFromClaims(rawIDToken, accessToken string) (* } } + if p.AdditionalClaims != nil { + p.extractAdditionalClaims(extractor, ss) + } + // `email_verified` must be present and explicitly set to `false` to be // considered unverified. verifyEmail := (p.EmailClaim == options.OIDCEmailClaim) && !p.AllowUnverifiedEmail @@ -301,6 +320,22 @@ func (p *ProviderData) getClaimExtractor(rawIDToken, accessToken string) (util.C return extractor, nil } +func (p *ProviderData) extractAdditionalClaims(extractor util.ClaimExtractor, ss *sessions.SessionState) { + if ss.AdditionalClaims == nil { + ss.AdditionalClaims = make(map[string]any) + } + for _, claim := range p.AdditionalClaims { + value, exists, err := extractor.GetClaim(claim) + if err != nil { + logger.Printf("error extracting additional claim %q: %v", claim, err) + continue + } + if exists { + ss.AdditionalClaims[claim] = value + } + } +} + // checkNonce compares the session's nonce with the IDToken's nonce claim func (p *ProviderData) checkNonce(s *sessions.SessionState) error { extractor, err := p.getClaimExtractor(s.IDToken, "") diff --git a/providers/provider_data_test.go b/providers/provider_data_test.go index 044a77b1..9801d20c 100644 --- a/providers/provider_data_test.go +++ b/providers/provider_data_test.go @@ -237,6 +237,7 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) { ExpectedError error ExpectedSession *sessions.SessionState ExpectProfileURLCalled bool + AdditionalClaims []string }{ "Standard": { IDToken: defaultIDToken, @@ -417,6 +418,27 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) { SkipClaimsFromProfileURL: true, ExpectedSession: &sessions.SessionState{}, }, + "Additional claims": { + IDToken: defaultIDToken, + AdditionalClaims: []string{"phone_number", "picture"}, + ExpectedSession: &sessions.SessionState{ + PreferredUsername: "Jane Dobbs", + AdditionalClaims: map[string]interface{}{ + "phone_number": "+4798765432", + "picture": "http://mugbook.com/janed/me.jpg", + }, + }, + }, + "Additional claims with missing claim": { + IDToken: defaultIDToken, + AdditionalClaims: []string{"phone_number", "picture1"}, + ExpectedSession: &sessions.SessionState{ + PreferredUsername: "Jane Dobbs", + AdditionalClaims: map[string]interface{}{ + "phone_number": "+4798765432", + }, + }, + }, } for testName, tc := range testCases { t.Run(testName, func(t *testing.T) { @@ -453,6 +475,7 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) { provider.EmailClaim = tc.EmailClaim provider.GroupsClaim = tc.GroupsClaim provider.SkipClaimsFromProfileURL = tc.SkipClaimsFromProfileURL + provider.AdditionalClaims = tc.AdditionalClaims rawIDToken, err := newSignedTestIDToken(tc.IDToken) g.Expect(err).ToNot(HaveOccurred()) diff --git a/providers/providers.go b/providers/providers.go index 6af51ecf..85c45ac5 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -84,6 +84,7 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData, ClientSecret: providerConfig.ClientSecret, ClientSecretFile: providerConfig.ClientSecretFile, AuthRequestResponseMode: providerConfig.AuthRequestResponseMode, + AdditionalClaims: providerConfig.AdditionalClaims, } needsVerifier, err := providerRequiresOIDCProviderVerifier(providerConfig.Type)