From 1102ad256c8d356e1e0815ed751b9d61ad5fd392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20Val=C3=A9rio?= Date: Wed, 7 May 2025 08:09:50 -0300 Subject: [PATCH 1/9] revoke access token on logout --- oauthproxy.go | 16 ++++++++++++++-- pics_oauthproxy.go | 22 +++++++++++++++++++++- pkg/apis/options/legacy_options.go | 3 +++ pkg/apis/options/providers.go | 3 +++ providers/provider_data.go | 1 + providers/providers.go | 1 + 6 files changed, 43 insertions(+), 3 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index 5efbd4cf..1b76b18a 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -798,7 +798,6 @@ func (p *OAuthProxy) backendLogout(rw http.ResponseWriter, req *http.Request, si } providerData := p.provider.Data() - var resp *http.Response if signOutAllSessions { if providerData.BackendLogoutAllSessionsURL == "" { return @@ -815,6 +814,19 @@ func (p *OAuthProxy) backendLogout(rw http.ResponseWriter, req *http.Request, si } p.picsAuditClient.CreateSuccessfulLogoutAuditEntry(session, req.RequestURI, req.Header.Get("edisp-org-id")) } else { + if providerData.BackendRevokeAccessTokenURL != "" { + resp, err := PicsRevokeAcessToken(providerData.BackendRevokeAccessTokenURL, session.AccessToken, providerData.ClientID, providerData.ClientSecret) + if err != nil { + logger.Errorf("error while calling backend revoke access token: %v", err) + return + } + + if resp.StatusCode() != 200 { + logger.Errorf("error while calling backend revoke acess token url, returned error code %v", resp.StatusCode()) + } + p.picsAuditClient.CreateSuccessfulLogoutAuditEntry(session, req.RequestURI, req.Header.Get("edisp-org-id")) + } + if providerData.BackendLogoutURL == "" { return } @@ -822,7 +834,7 @@ func (p *OAuthProxy) backendLogout(rw http.ResponseWriter, req *http.Request, si backendLogoutURL := strings.ReplaceAll(providerData.BackendLogoutURL, "{id_token}", session.IDToken) // security exception because URL is dynamic ({id_token} replacement) but // base is not end-user provided but comes from configuration somewhat secure - resp, err = http.Get(backendLogoutURL) // #nosec G107 + resp, err := http.Get(backendLogoutURL) // #nosec G107 if err != nil { logger.Errorf("error while calling backend logout: %v", err) return diff --git a/pics_oauthproxy.go b/pics_oauthproxy.go index 2b39e834..9d9d9b78 100644 --- a/pics_oauthproxy.go +++ b/pics_oauthproxy.go @@ -29,12 +29,32 @@ func PicsSignOutAllSessions(backendLogoutAllSessionsURL string, introspectClaims Do() if resp.Error() != nil { - return nil, fmt.Errorf("error logging out from IAM: %v", err) + return nil, fmt.Errorf("error logging out from IAM: %v", resp.Error()) } return resp, err } +func PicsRevokeAcessToken(backendRevokeURL string, accessToken string, clientID string, clientSecret string) (resp requests.Result, err error) { + authHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(clientID+":"+clientSecret)) + body := "token=" + accessToken + + resp = requests.New(backendRevokeURL). + WithMethod("POST"). + SetHeader("Authorization", authHeader). + SetHeader("api-version", "2"). + SetHeader("Content-Type", "application/x-www-form-urlencoded"). + SetHeader("Accept", "application/json"). + WithBody(strings.NewReader(body)). + Do() + + if resp.Error() != nil { + return nil, fmt.Errorf("error revoking access token: %v", resp.Error()) + } + + return resp, nil +} + func getUserID(introspectClaims string) (string, error) { decodedClaims, err := base64.StdEncoding.DecodeString(introspectClaims) if err != nil { diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index 8fc5110e..854bda8a 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -546,6 +546,7 @@ type LegacyProvider struct { BackendLogoutURL string `flag:"backend-logout-url" cfg:"backend_logout_url"` BackendLogoutAllSessionsURL string `flag:"backend-logout-all-sessions-url" cfg:"backend_logout_all_sessions_url"` + BackendRevokeAccessTokenURL string `flag:"backend-revoke-access-token-url" cfg:"backend_revoke_access_token_url"` AcrValues string `flag:"acr-values" cfg:"acr_values"` JWTKey string `flag:"jwt-key" cfg:"jwt_key"` @@ -616,6 +617,7 @@ func legacyProviderFlagSet() *pflag.FlagSet { flagSet.StringSlice("allowed-role", []string{}, "(keycloak-oidc) restrict logins to members of these roles (may be given multiple times)") flagSet.String("backend-logout-url", "", "url to perform a backend logout, {id_token} can be used as placeholder for the id_token") flagSet.String("backend-logout-all-sessions-url", "", "url to perform a backend logout, {user_id} can be used as placeholder for the user_id") + flagSet.String("backend-revoke-access-token-url", "", "url to perform a backend revoke access token") return flagSet } @@ -698,6 +700,7 @@ func (l *LegacyProvider) convert() (Providers, error) { BackendLogoutURL: l.BackendLogoutURL, BackendLogoutAllSessionsURL: l.BackendLogoutAllSessionsURL, + BackendRevokeAccessTokenURL: l.BackendRevokeAccessTokenURL, } // This part is out of the switch section for all providers that support OIDC diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index aefd3cc2..7fddf28c 100644 --- a/pkg/apis/options/providers.go +++ b/pkg/apis/options/providers.go @@ -91,6 +91,9 @@ type Provider struct { // URL to call to perform backend logout, `{user_id}` would be replaced by the actual `user_id` if available in the session IntrospectClaims BackendLogoutAllSessionsURL string `json:"backendLogoutAllSessionsURL"` + + // URL to call to perform backend revoke token + BackendRevokeAccessTokenURL string `json:"backendRevokeAccessTokenURL"` } // ProviderType is used to enumerate the different provider type options diff --git a/providers/provider_data.go b/providers/provider_data.go index 4744f674..68eb9027 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -62,6 +62,7 @@ type ProviderData struct { BackendLogoutURL string BackendLogoutAllSessionsURL string + BackendRevokeAccessTokenURL string } // Data returns the ProviderData diff --git a/providers/providers.go b/providers/providers.go index 1c950aaf..12fcfed6 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -164,6 +164,7 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData, p.BackendLogoutURL = providerConfig.BackendLogoutURL p.BackendLogoutAllSessionsURL = providerConfig.BackendLogoutAllSessionsURL + p.BackendRevokeAccessTokenURL = providerConfig.BackendRevokeAccessTokenURL return p, nil } From 13549c6092bf8e098d73c74920f63717c8c8b89e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20Val=C3=A9rio?= Date: Wed, 7 May 2025 08:17:17 -0300 Subject: [PATCH 2/9] update docs --- docs/docs/configuration/alpha_config.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 5def6d14..949a5ae5 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -446,6 +446,7 @@ Provider holds all configuration for a single provider | `code_challenge_method` | _string_ | The code challenge method | | `backendLogoutURL` | _string_ | URL to call to perform backend logout, `{id_token}` would be replaced by the actual `id_token` if available in the session | | `backendLogoutAllSessionsURL` | _string_ | URL to call to perform backend logout, `{user_id}` would be replaced by the actual `user_id` if available in the session IntrospectClaims | +| `backendRevokeAccessTokenURL` | _string_ | URL to call to perform backend revoke token | ### ProviderType #### (`string` alias) From 7ad27949b2ec13eb8bfe3337133b72ce1fc65bd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20Val=C3=A9rio?= Date: Wed, 7 May 2025 08:43:01 -0300 Subject: [PATCH 3/9] remove deprecated option --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 9c56aa9c..f19aabc4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,5 @@ run: - deadline: 120s + linters: enable: - govet From 64be9956edea255f5c29098997e296e8cc325456 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20Val=C3=A9rio?= Date: Wed, 7 May 2025 09:17:35 -0300 Subject: [PATCH 4/9] anderson/fix golint --- .github/workflows/ci.yaml | 4 ++-- .golangci.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2a9db00b..007f5b9f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,12 +35,12 @@ jobs: - uses: actions/setup-go@v5 with: - go-version: 1.22.4 + go-version-file: go.mod - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.61.0 + version: v1.64.8 Tests: name: Tests - Executing unit tests diff --git a/.golangci.yml b/.golangci.yml index f19aabc4..9c56aa9c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,5 @@ run: - + deadline: 120s linters: enable: - govet From 1c4797a0341079e8b5602238f2ca6f6616aad3c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20Val=C3=A9rio?= Date: Wed, 7 May 2025 09:22:38 -0300 Subject: [PATCH 5/9] fix lint --- .golangci.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9c56aa9c..ef36f8e4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,3 @@ -run: - deadline: 120s linters: enable: - govet From 2e210d1cfdca42b4ff2ef28f2ee2b7175221bf5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20Val=C3=A9rio?= Date: Wed, 7 May 2025 09:36:44 -0300 Subject: [PATCH 6/9] fix linter --- providers/logingov.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/providers/logingov.go b/providers/logingov.go index 30b144a0..eb848218 100644 --- a/providers/logingov.go +++ b/providers/logingov.go @@ -38,8 +38,8 @@ var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") func randSeq(n int) string { b := make([]rune, n) for i := range b { - max := big.NewInt(int64(len(letters))) - bigN, err := rand.Int(rand.Reader, max) + maxInt := big.NewInt(int64(len(letters))) + bigN, err := rand.Int(rand.Reader, maxInt) if err != nil { // This should never happen panic(err) From 67e218c6b464a0c11d26bc21368d4dbf0651a125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20Val=C3=A9rio?= Date: Fri, 9 May 2025 05:57:22 -0300 Subject: [PATCH 7/9] try to remove linter change --- .github/workflows/ci.yaml | 4 ++-- .golangci.yml | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 007f5b9f..2a9db00b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,12 +35,12 @@ jobs: - uses: actions/setup-go@v5 with: - go-version-file: go.mod + go-version: 1.22.4 - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.64.8 + version: v1.61.0 Tests: name: Tests - Executing unit tests diff --git a/.golangci.yml b/.golangci.yml index ef36f8e4..90d4d6f3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,3 +1,6 @@ +run: + deadline: 120s + linters: enable: - govet From 5e57984cb11ed85e6f1cea384ea63834f565254e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20Val=C3=A9rio?= Date: Fri, 9 May 2025 06:00:11 -0300 Subject: [PATCH 8/9] try to remove linter change --- .github/workflows/ci.yaml | 4 ++-- .golangci.yml | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2a9db00b..007f5b9f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,12 +35,12 @@ jobs: - uses: actions/setup-go@v5 with: - go-version: 1.22.4 + go-version-file: go.mod - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.61.0 + version: v1.64.8 Tests: name: Tests - Executing unit tests diff --git a/.golangci.yml b/.golangci.yml index 90d4d6f3..ef36f8e4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,3 @@ -run: - deadline: 120s - linters: enable: - govet From c922ebecb04ada62cf1f6bf5a058c0e0878984a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20Val=C3=A9rio?= Date: Fri, 9 May 2025 06:05:01 -0300 Subject: [PATCH 9/9] fix audit log --- oauthproxy.go | 2 +- pkg/pics/audit/audit_client.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/oauthproxy.go b/oauthproxy.go index 1b76b18a..09fd2e2a 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -824,7 +824,7 @@ func (p *OAuthProxy) backendLogout(rw http.ResponseWriter, req *http.Request, si if resp.StatusCode() != 200 { logger.Errorf("error while calling backend revoke acess token url, returned error code %v", resp.StatusCode()) } - p.picsAuditClient.CreateSuccessfulLogoutAuditEntry(session, req.RequestURI, req.Header.Get("edisp-org-id")) + p.picsAuditClient.CreateSuccessfulRevokeAccessTokenAuditEntry(session, req.RequestURI, req.Header.Get("edisp-org-id")) } if providerData.BackendLogoutURL == "" { diff --git a/pkg/pics/audit/audit_client.go b/pkg/pics/audit/audit_client.go index 2f3d981d..3cc2c9d5 100644 --- a/pkg/pics/audit/audit_client.go +++ b/pkg/pics/audit/audit_client.go @@ -70,6 +70,12 @@ func (c *Client) CreateSuccessfulLogoutAuditEntry(ss *sessions.SessionState, app c.createAuditEntry(ss, appURL, tenantID, "0", "Success", &coding) } +func (c *Client) CreateSuccessfulRevokeAccessTokenAuditEntry(ss *sessions.SessionState, appURL string, tenantID string) { + coding := Coding{ + System: "http://hl7.org/fhir/ValueSet/audit-event-type", Version: "1", Code: "110123", Display: "User revoked access token"} + c.createAuditEntry(ss, appURL, tenantID, "0", "Success", &coding) +} + func (c *Client) createAuditEntry(ss *sessions.SessionState, appURL string, tenantID string, outcomeCode string, outcomeDesc string, coding *Coding) { if !c.enabled { return