Revoke access token on logout (#66)

Adding functionality to revoke access token on logout. This will use the
newly added OAUTH2_PROXY_BACKEND_REVOKE_ACCESS_TOKEN_URL environment
variable.

AB#1624642

## Motivation and Context

Adding the functionality to revoke an access token on logout prevents an
attacker from continuing to use a stolen access token until the
expiration of the TTL.

## How Has This Been Tested?

Running it locally integrated with Pics.
This commit is contained in:
Anderson Valério 2025-05-09 06:51:10 -03:00 committed by GitHub
commit b5da4ecc31
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 54 additions and 9 deletions

View File

@ -35,12 +35,12 @@ jobs:
- uses: actions/setup-go@v5 - uses: actions/setup-go@v5
with: with:
go-version: 1.22.4 go-version-file: go.mod
- name: golangci-lint - name: golangci-lint
uses: golangci/golangci-lint-action@v6 uses: golangci/golangci-lint-action@v6
with: with:
version: v1.61.0 version: v1.64.8
Tests: Tests:
name: Tests - Executing unit tests name: Tests - Executing unit tests

View File

@ -1,5 +1,3 @@
run:
deadline: 120s
linters: linters:
enable: enable:
- govet - govet

View File

@ -446,6 +446,7 @@ Provider holds all configuration for a single provider
| `code_challenge_method` | _string_ | The code challenge method | | `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 | | `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 | | `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 ### ProviderType
#### (`string` alias) #### (`string` alias)

View File

@ -798,7 +798,6 @@ func (p *OAuthProxy) backendLogout(rw http.ResponseWriter, req *http.Request, si
} }
providerData := p.provider.Data() providerData := p.provider.Data()
var resp *http.Response
if signOutAllSessions { if signOutAllSessions {
if providerData.BackendLogoutAllSessionsURL == "" { if providerData.BackendLogoutAllSessionsURL == "" {
return 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")) p.picsAuditClient.CreateSuccessfulLogoutAuditEntry(session, req.RequestURI, req.Header.Get("edisp-org-id"))
} else { } 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.CreateSuccessfulRevokeAccessTokenAuditEntry(session, req.RequestURI, req.Header.Get("edisp-org-id"))
}
if providerData.BackendLogoutURL == "" { if providerData.BackendLogoutURL == "" {
return 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) backendLogoutURL := strings.ReplaceAll(providerData.BackendLogoutURL, "{id_token}", session.IDToken)
// security exception because URL is dynamic ({id_token} replacement) but // security exception because URL is dynamic ({id_token} replacement) but
// base is not end-user provided but comes from configuration somewhat secure // 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 { if err != nil {
logger.Errorf("error while calling backend logout: %v", err) logger.Errorf("error while calling backend logout: %v", err)
return return

View File

@ -29,12 +29,32 @@ func PicsSignOutAllSessions(backendLogoutAllSessionsURL string, introspectClaims
Do() Do()
if resp.Error() != nil { 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 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) { func getUserID(introspectClaims string) (string, error) {
decodedClaims, err := base64.StdEncoding.DecodeString(introspectClaims) decodedClaims, err := base64.StdEncoding.DecodeString(introspectClaims)
if err != nil { if err != nil {

View File

@ -546,6 +546,7 @@ type LegacyProvider struct {
BackendLogoutURL string `flag:"backend-logout-url" cfg:"backend_logout_url"` BackendLogoutURL string `flag:"backend-logout-url" cfg:"backend_logout_url"`
BackendLogoutAllSessionsURL string `flag:"backend-logout-all-sessions-url" cfg:"backend_logout_all_sessions_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"` AcrValues string `flag:"acr-values" cfg:"acr_values"`
JWTKey string `flag:"jwt-key" cfg:"jwt_key"` 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.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-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-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 return flagSet
} }
@ -698,6 +700,7 @@ func (l *LegacyProvider) convert() (Providers, error) {
BackendLogoutURL: l.BackendLogoutURL, BackendLogoutURL: l.BackendLogoutURL,
BackendLogoutAllSessionsURL: l.BackendLogoutAllSessionsURL, BackendLogoutAllSessionsURL: l.BackendLogoutAllSessionsURL,
BackendRevokeAccessTokenURL: l.BackendRevokeAccessTokenURL,
} }
// This part is out of the switch section for all providers that support OIDC // This part is out of the switch section for all providers that support OIDC

View File

@ -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 // 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"` 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 // ProviderType is used to enumerate the different provider type options

View File

@ -70,6 +70,12 @@ func (c *Client) CreateSuccessfulLogoutAuditEntry(ss *sessions.SessionState, app
c.createAuditEntry(ss, appURL, tenantID, "0", "Success", &coding) 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) { func (c *Client) createAuditEntry(ss *sessions.SessionState, appURL string, tenantID string, outcomeCode string, outcomeDesc string, coding *Coding) {
if !c.enabled { if !c.enabled {
return return

View File

@ -38,8 +38,8 @@ var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
func randSeq(n int) string { func randSeq(n int) string {
b := make([]rune, n) b := make([]rune, n)
for i := range b { for i := range b {
max := big.NewInt(int64(len(letters))) maxInt := big.NewInt(int64(len(letters)))
bigN, err := rand.Int(rand.Reader, max) bigN, err := rand.Int(rand.Reader, maxInt)
if err != nil { if err != nil {
// This should never happen // This should never happen
panic(err) panic(err)

View File

@ -62,6 +62,7 @@ type ProviderData struct {
BackendLogoutURL string BackendLogoutURL string
BackendLogoutAllSessionsURL string BackendLogoutAllSessionsURL string
BackendRevokeAccessTokenURL string
} }
// Data returns the ProviderData // Data returns the ProviderData

View File

@ -164,6 +164,7 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,
p.BackendLogoutURL = providerConfig.BackendLogoutURL p.BackendLogoutURL = providerConfig.BackendLogoutURL
p.BackendLogoutAllSessionsURL = providerConfig.BackendLogoutAllSessionsURL p.BackendLogoutAllSessionsURL = providerConfig.BackendLogoutAllSessionsURL
p.BackendRevokeAccessTokenURL = providerConfig.BackendRevokeAccessTokenURL
return p, nil return p, nil
} }