From 403117838827ad07098f375f96a55754af2e26c6 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Tue, 12 Sep 2023 15:05:50 +0200 Subject: [PATCH 01/15] introduce app_tid and deprecade user_uuid --- auth/middleware_test.go | 20 +++++++++++++++-- auth/token.go | 22 ++++++++++++++++--- auth/validator.go | 2 +- env/iasConfig.go | 18 +++++++++++++-- env/iasConfig_test.go | 3 ++- .../service-instance/credentials | 3 ++- .../single-instance/service-instance/app_tid | 1 + mocks/mockServer.go | 18 ++++++++++----- mocks/oidcClaims.go | 1 + mocks/oidcTokenBuilder.go | 6 +++++ oidcclient/jwk.go | 6 ++--- 11 files changed, 81 insertions(+), 19 deletions(-) create mode 100644 env/testdata/k8s/single-instance/service-instance/app_tid diff --git a/auth/middleware_test.go b/auth/middleware_test.go index 26d10a8..882191e 100644 --- a/auth/middleware_test.go +++ b/auth/middleware_test.go @@ -213,14 +213,14 @@ func TestEnd2End(t *testing.T) { name: "jwks rejects zone", header: oidcMockServer.DefaultHeaders(), claims: mocks.NewOIDCClaimsBuilder(oidcMockServer.DefaultClaims()). - ZoneID(mocks.InvalidZoneID). + ZoneID(mocks.InvalidAppTID). Build(), wantErr: true, }, { name: "lib rejects unaccepted zone again", header: oidcMockServer.DefaultHeaders(), claims: mocks.NewOIDCClaimsBuilder(oidcMockServer.DefaultClaims()). - ZoneID(mocks.InvalidZoneID). + ZoneID(mocks.InvalidAppTID). Build(), wantErr: true, }, { @@ -231,6 +231,22 @@ func TestEnd2End(t *testing.T) { Build(), wantErr: false, }, + { + name: "lib accepts any app_tid", + header: oidcMockServer.DefaultHeaders(), + claims: mocks.NewOIDCClaimsBuilder(oidcMockServer.DefaultClaims()). + AppTID(uuid.New().String()). + Build(), + wantErr: false, + }, + { + name: "lib rejects unaccepted app_tid", + header: oidcMockServer.DefaultHeaders(), + claims: mocks.NewOIDCClaimsBuilder(oidcMockServer.DefaultClaims()). + AppTID(mocks.InvalidAppTID). + Build(), + wantErr: true, + }, } for _, tt := range tests { diff --git a/auth/token.go b/auth/token.go index 126db33..2baa748 100644 --- a/auth/token.go +++ b/auth/token.go @@ -22,6 +22,7 @@ const ( claimEmail = "email" claimSapGlobalUserID = "user_uuid" claimSapGlobalZoneID = "zone_uuid" // tenant GUID + claimSapGlobalAppTID = "app_tid" claimIasIssuer = "ias_iss" ) @@ -115,10 +116,25 @@ func (t Token) Email() string { return v } -// ZoneID returns "zone_uuid" claim, if it doesn't exist empty string is returned +// ZoneID returns "app_tid" claim, if it doesn't exist empty string is returned +// Deprecated: will be replaced by AppTID and removed in future func (t Token) ZoneID() string { - v, _ := t.GetClaimAsString(claimSapGlobalZoneID) - return v + appTID, err := t.GetClaimAsString(claimSapGlobalAppTID) + if errors.Is(err, ErrClaimNotExists) { + zoneUUID, _ := t.GetClaimAsString(claimSapGlobalZoneID) + return zoneUUID + } + return appTID +} + +// AppTID returns "app_tid" claim, if it doesn't exist empty string is returned +func (t Token) AppTID() string { + appTID, err := t.GetClaimAsString(claimSapGlobalAppTID) + if errors.Is(err, ErrClaimNotExists) { + zoneUUID, _ := t.GetClaimAsString(claimSapGlobalZoneID) + return zoneUUID + } + return appTID } // UserUUID returns "user_uuid" claim, if it doesn't exist empty string is returned diff --git a/auth/validator.go b/auth/validator.go index 54bacd6..dfe9e11 100644 --- a/auth/validator.go +++ b/auth/validator.go @@ -56,7 +56,7 @@ func (m *Middleware) verifySignature(t Token, keySet *oidcclient.OIDCTenant) (er } // parse and verify signature - jwks, err := keySet.GetJWKs(t.ZoneID()) + jwks, err := keySet.GetJWKs(t.AppTID()) if err != nil { return err } diff --git a/env/iasConfig.go b/env/iasConfig.go index 4b292df..d889f10 100644 --- a/env/iasConfig.go +++ b/env/iasConfig.go @@ -33,7 +33,8 @@ type Identity interface { GetClientSecret() string // Returns the client secret. Optional GetURL() string // Returns the url to the DefaultIdentity tenant. E.g. https://abcdefgh.accounts.ondemand.com GetDomains() []string // Returns the domains of the DefaultIdentity service. E.g. ["accounts.ondemand.com"] - GetZoneUUID() uuid.UUID // Returns the zone uuid. Optional + GetZoneUUID() uuid.UUID // Deprecated: Returns the zone uuid, will be replaced by GetAppTID Optional + GetAppTID() uuid.UUID // Returns the app tid uuid and replaces zone uuid in future Optional GetProofTokenURL() string // Returns the proof token url. Optional GetCertificate() string // Returns the client certificate. Optional GetKey() string // Returns the client certificate key. Optional @@ -47,7 +48,8 @@ type DefaultIdentity struct { ClientSecret string `json:"clientsecret"` Domains []string `json:"domains"` URL string `json:"url"` - ZoneUUID uuid.UUID `json:"zone_uuid"` + ZoneUUID uuid.UUID `json:"zone_uuid"` // Deprecated: will be replaced by AppTID + AppTID uuid.UUID `json:"app_tid"` // replaces ZoneUUID ProofTokenURL string `json:"prooftoken_url"` OsbURL string `json:"osb_url"` Certificate string `json:"certificate"` @@ -184,7 +186,19 @@ func (c DefaultIdentity) GetDomains() []string { } // GetZoneUUID implements the env.Identity interface. +// Deprecated: will be replaced by GetAppTID and removed in future func (c DefaultIdentity) GetZoneUUID() uuid.UUID { + if c.AppTID != uuid.Nil { + return c.AppTID + } + return c.ZoneUUID +} + +// GetAppTID implements the env.Identity interface and replaces GetZoneUUID in future +func (c DefaultIdentity) GetAppTID() uuid.UUID { + if c.AppTID != uuid.Nil { + return c.AppTID + } return c.ZoneUUID } diff --git a/env/iasConfig_test.go b/env/iasConfig_test.go index d9615a7..df85803 100644 --- a/env/iasConfig_test.go +++ b/env/iasConfig_test.go @@ -21,6 +21,7 @@ var testConfig = &DefaultIdentity{ Domains: []string{"accounts400.ondemand.com", "my.arbitrary.domain"}, URL: "https://mytenant.accounts400.ondemand.com", ZoneUUID: uuid.MustParse("bef12345-de57-480f-be92-1d8c1c7abf16"), + AppTID: uuid.MustParse("70cd0de3-528a-4655-b56a-5862591def5c"), } func TestParseIdentityConfig(t *testing.T) { @@ -33,7 +34,7 @@ func TestParseIdentityConfig(t *testing.T) { }{ { name: "[CF] single identity service instance bound", - env: "{\"identity\":[{\"binding_name\":null,\"credentials\":{\"clientid\":\"cef76757-de57-480f-be92-1d8c1c7abf16\",\"clientsecret\":\"[the_CLIENT.secret:3[/abc\",\"domains\":[\"accounts400.ondemand.com\",\"my.arbitrary.domain\"],\"token_url\":\"https://mytenant.accounts400.ondemand.com/oauth2/token\",\"url\":\"https://mytenant.accounts400.ondemand.com\",\"zone_uuid\":\"bef12345-de57-480f-be92-1d8c1c7abf16\"},\"instance_name\":\"my-ams-instance\",\"label\":\"identity\",\"name\":\"my-ams-instance\",\"plan\":\"application\",\"provider\":null,\"syslog_drain_url\":null,\"tags\":[\"ias\"],\"volume_mounts\":[]}]}", + env: "{\"identity\":[{\"binding_name\":null,\"credentials\":{\"clientid\":\"cef76757-de57-480f-be92-1d8c1c7abf16\",\"clientsecret\":\"[the_CLIENT.secret:3[/abc\",\"domains\":[\"accounts400.ondemand.com\",\"my.arbitrary.domain\"],\"token_url\":\"https://mytenant.accounts400.ondemand.com/oauth2/token\",\"url\":\"https://mytenant.accounts400.ondemand.com\",\"zone_uuid\":\"bef12345-de57-480f-be92-1d8c1c7abf16\", \"app_tid\":\"70cd0de3-528a-4655-b56a-5862591def5c\"},\"instance_name\":\"my-ams-instance\",\"label\":\"identity\",\"name\":\"my-ams-instance\",\"plan\":\"application\",\"provider\":null,\"syslog_drain_url\":null,\"tags\":[\"ias\"],\"volume_mounts\":[]}]}", want: testConfig, wantErr: false, }, diff --git a/env/testdata/k8s/single-instance-onecredentialsfile/service-instance/credentials b/env/testdata/k8s/single-instance-onecredentialsfile/service-instance/credentials index 6236300..79884ab 100644 --- a/env/testdata/k8s/single-instance-onecredentialsfile/service-instance/credentials +++ b/env/testdata/k8s/single-instance-onecredentialsfile/service-instance/credentials @@ -5,5 +5,6 @@ "accounts400.ondemand.com", "my.arbitrary.domain" ], "url": "https://mytenant.accounts400.ondemand.com", - "zone_uuid": "bef12345-de57-480f-be92-1d8c1c7abf16" + "zone_uuid": "bef12345-de57-480f-be92-1d8c1c7abf16", + "app_tid": "70cd0de3-528a-4655-b56a-5862591def5c" } \ No newline at end of file diff --git a/env/testdata/k8s/single-instance/service-instance/app_tid b/env/testdata/k8s/single-instance/service-instance/app_tid new file mode 100644 index 0000000..cc6d798 --- /dev/null +++ b/env/testdata/k8s/single-instance/service-instance/app_tid @@ -0,0 +1 @@ +70cd0de3-528a-4655-b56a-5862591def5c \ No newline at end of file diff --git a/mocks/mockServer.go b/mocks/mockServer.go index ca84cb8..4093fe1 100644 --- a/mocks/mockServer.go +++ b/mocks/mockServer.go @@ -51,8 +51,8 @@ type MockServer struct { CustomIssuer string // CustomIssuer holds a custom domain returned by the discovery endpoint } -// InvalidZoneID represents a zone guid which is rejected by mock server on behalf of IAS tenant -const InvalidZoneID string = "dff69954-a259-4104-9074-193bc9a366ce" +// InvalidAppTID represents a zone guid which is rejected by mock server on behalf of IAS tenant +const InvalidAppTID string = "dff69954-a259-4104-9074-193bc9a366ce" // NewOIDCMockServer instantiates a new MockServer. func NewOIDCMockServer() (*MockServer, error) { @@ -93,7 +93,7 @@ func newOIDCMockServer(customIssuer string) (*MockServer, error) { } r.HandleFunc("/.well-known/openid-configuration", mockServer.WellKnownHandler).Methods(http.MethodGet) - r.HandleFunc("/oauth2/certs", mockServer.JWKsHandlerInvalidZone).Methods(http.MethodGet).Headers("x-zone_uuid", InvalidZoneID) + r.HandleFunc("/oauth2/certs", mockServer.JWKsHandlerInvalidAppTID).Methods(http.MethodGet).Headers("x-zone_uuid", InvalidAppTID) r.HandleFunc("/oauth2/certs", mockServer.JWKsHandler).Methods(http.MethodGet) r.HandleFunc("/oauth2/token", mockServer.tokenHandler).Methods(http.MethodPost).Headers("Content-Type", "application/x-www-form-urlencoded") @@ -150,9 +150,9 @@ func (m *MockServer) JWKsHandler(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write(payload) } -// JWKsHandlerInvalidZone is the http handler which answers invalid requests to the JWKS endpoint. -// in reality it returns "{ \"msg\":\"Invalid zone_uuid provided\" }" -func (m *MockServer) JWKsHandlerInvalidZone(w http.ResponseWriter, _ *http.Request) { +// JWKsHandlerInvalidAppTID is the http handler which answers invalid requests to the JWKS endpoint. +// in reality, it returns "{ \"msg\":\"Invalid app_tid provided\" }" +func (m *MockServer) JWKsHandlerInvalidAppTID(w http.ResponseWriter, _ *http.Request) { m.JWKsHitCounter++ w.WriteHeader(http.StatusBadRequest) } @@ -295,6 +295,7 @@ type MockConfig struct { URL string Domains []string ZoneUUID uuid.UUID + AppTID uuid.UUID ProofTokenURL string OsbURL string Certificate string @@ -327,6 +328,11 @@ func (c MockConfig) GetZoneUUID() uuid.UUID { return c.ZoneUUID } +// GetAppTID implements the env.Identity interface. +func (c MockConfig) GetAppTID() uuid.UUID { + return c.AppTID +} + // GetProofTokenURL implements the env.Identity interface. func (c MockConfig) GetProofTokenURL() string { return c.ProofTokenURL diff --git a/mocks/oidcClaims.go b/mocks/oidcClaims.go index fbc2bad..4ae8242 100644 --- a/mocks/oidcClaims.go +++ b/mocks/oidcClaims.go @@ -24,5 +24,6 @@ type OIDCClaims struct { FamilyName string `json:"family_name,omitempty"` Email string `json:"email,omitempty"` ZoneID string `json:"zone_uuid,omitempty"` + AppTID string `json:"app_tid,omitempty"` UserUUID string `json:"user_uuid,omitempty"` } diff --git a/mocks/oidcTokenBuilder.go b/mocks/oidcTokenBuilder.go index 08de26b..6a79b92 100644 --- a/mocks/oidcTokenBuilder.go +++ b/mocks/oidcTokenBuilder.go @@ -140,6 +140,12 @@ func (b *OIDCClaimsBuilder) ZoneID(zoneID string) *OIDCClaimsBuilder { return b } +// AppTID sets the app_tid field +func (b *OIDCClaimsBuilder) AppTID(appTID string) *OIDCClaimsBuilder { + b.claims.AppTID = appTID + return b +} + // WithoutAudience removes the aud claim func (b *OIDCClaimsBuilder) WithoutAudience() *OIDCClaimsBuilder { b.claims.Audience = nil diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index 9bdf02e..e689475 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -53,13 +53,13 @@ func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, er } // GetJWKs returns the validation keys either cached or updated ones -func (ks *OIDCTenant) GetJWKs(zoneID string) (jwk.Set, error) { - keys, err := ks.readJWKsFromMemory(zoneID) +func (ks *OIDCTenant) GetJWKs(appTID string) (jwk.Set, error) { + keys, err := ks.readJWKsFromMemory(appTID) if keys == nil { if err != nil { return nil, err } - return ks.updateJWKsMemory(zoneID) + return ks.updateJWKsMemory(appTID) } return keys, nil } From 17f21ec636f611f0dfc13e125e96004dc1feb611 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Wed, 13 Sep 2023 11:50:16 +0200 Subject: [PATCH 02/15] support app_tid and client_id in oids_client --- auth/middleware_test.go | 22 +++------------ auth/token.go | 6 ++--- auth/validator.go | 2 +- mocks/mockServer.go | 4 +-- oidcclient/jwk.go | 44 +++++++++++++++++------------- oidcclient/jwk_test.go | 60 ++++++++++++++++++++++++----------------- 6 files changed, 70 insertions(+), 68 deletions(-) diff --git a/auth/middleware_test.go b/auth/middleware_test.go index 882191e..8a6aaf6 100644 --- a/auth/middleware_test.go +++ b/auth/middleware_test.go @@ -210,36 +210,20 @@ func TestEnd2End(t *testing.T) { claims: oidcMockServer.DefaultClaims(), wantErr: true, }, { - name: "jwks rejects zone", + name: "jwks prioritize app_tid", header: oidcMockServer.DefaultHeaders(), claims: mocks.NewOIDCClaimsBuilder(oidcMockServer.DefaultClaims()). ZoneID(mocks.InvalidAppTID). Build(), - wantErr: true, - }, { - name: "lib rejects unaccepted zone again", - header: oidcMockServer.DefaultHeaders(), - claims: mocks.NewOIDCClaimsBuilder(oidcMockServer.DefaultClaims()). - ZoneID(mocks.InvalidAppTID). - Build(), - wantErr: true, - }, { - name: "lib accepts any zone", - header: oidcMockServer.DefaultHeaders(), - claims: mocks.NewOIDCClaimsBuilder(oidcMockServer.DefaultClaims()). - ZoneID(uuid.New().String()). - Build(), wantErr: false, - }, - { + }, { name: "lib accepts any app_tid", header: oidcMockServer.DefaultHeaders(), claims: mocks.NewOIDCClaimsBuilder(oidcMockServer.DefaultClaims()). AppTID(uuid.New().String()). Build(), wantErr: false, - }, - { + }, { name: "lib rejects unaccepted app_tid", header: oidcMockServer.DefaultHeaders(), claims: mocks.NewOIDCClaimsBuilder(oidcMockServer.DefaultClaims()). diff --git a/auth/token.go b/auth/token.go index 2baa748..da41af5 100644 --- a/auth/token.go +++ b/auth/token.go @@ -22,7 +22,7 @@ const ( claimEmail = "email" claimSapGlobalUserID = "user_uuid" claimSapGlobalZoneID = "zone_uuid" // tenant GUID - claimSapGlobalAppTID = "app_tid" + claimAppTID = "app_tid" claimIasIssuer = "ias_iss" ) @@ -119,7 +119,7 @@ func (t Token) Email() string { // ZoneID returns "app_tid" claim, if it doesn't exist empty string is returned // Deprecated: will be replaced by AppTID and removed in future func (t Token) ZoneID() string { - appTID, err := t.GetClaimAsString(claimSapGlobalAppTID) + appTID, err := t.GetClaimAsString(claimAppTID) if errors.Is(err, ErrClaimNotExists) { zoneUUID, _ := t.GetClaimAsString(claimSapGlobalZoneID) return zoneUUID @@ -129,7 +129,7 @@ func (t Token) ZoneID() string { // AppTID returns "app_tid" claim, if it doesn't exist empty string is returned func (t Token) AppTID() string { - appTID, err := t.GetClaimAsString(claimSapGlobalAppTID) + appTID, err := t.GetClaimAsString(claimAppTID) if errors.Is(err, ErrClaimNotExists) { zoneUUID, _ := t.GetClaimAsString(claimSapGlobalZoneID) return zoneUUID diff --git a/auth/validator.go b/auth/validator.go index dfe9e11..da5e504 100644 --- a/auth/validator.go +++ b/auth/validator.go @@ -56,7 +56,7 @@ func (m *Middleware) verifySignature(t Token, keySet *oidcclient.OIDCTenant) (er } // parse and verify signature - jwks, err := keySet.GetJWKs(t.AppTID()) + jwks, err := keySet.GetJWKs(t.AppTID(), m.identity.GetClientID()) if err != nil { return err } diff --git a/mocks/mockServer.go b/mocks/mockServer.go index 4093fe1..8486a0c 100644 --- a/mocks/mockServer.go +++ b/mocks/mockServer.go @@ -93,7 +93,7 @@ func newOIDCMockServer(customIssuer string) (*MockServer, error) { } r.HandleFunc("/.well-known/openid-configuration", mockServer.WellKnownHandler).Methods(http.MethodGet) - r.HandleFunc("/oauth2/certs", mockServer.JWKsHandlerInvalidAppTID).Methods(http.MethodGet).Headers("x-zone_uuid", InvalidAppTID) + r.HandleFunc("/oauth2/certs", mockServer.JWKsHandlerInvalidAppTID).Methods(http.MethodGet).Headers("x-app_tid", InvalidAppTID) r.HandleFunc("/oauth2/certs", mockServer.JWKsHandler).Methods(http.MethodGet) r.HandleFunc("/oauth2/token", mockServer.tokenHandler).Methods(http.MethodPost).Headers("Content-Type", "application/x-www-form-urlencoded") @@ -271,7 +271,7 @@ func (m *MockServer) DefaultClaims() OIDCClaims { GivenName: "Foo", FamilyName: "Bar", Email: "foo@bar.org", - ZoneID: "11111111-2222-3333-4444-888888888888", + AppTID: "11111111-2222-3333-4444-888888888888", UserUUID: "22222222-3333-4444-5555-666666666666", } return claims diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index e689475..5344b32 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -21,12 +21,13 @@ import ( ) const defaultJwkExpiration = 15 * time.Minute -const zoneIDHeader = "x-zone_uuid" +const appTIDHeader = "x-app_tid" +const clientIDHeader = "x-client_id" // OIDCTenant represents one IAS tenant correlating with one zone with it's OIDC discovery results and cached JWKs type OIDCTenant struct { ProviderJSON ProviderJSON - acceptedZoneIds map[string]bool + acceptedTenants map[tenantKey]bool httpClient *http.Client // A set of cached keys and their expiry. jwks jwk.Set @@ -34,6 +35,11 @@ type OIDCTenant struct { mu sync.RWMutex } +type tenantKey struct { + appTID string + clientID string +} + type updateKeysResult struct { keys jwk.Set expiry time.Time @@ -43,7 +49,7 @@ type updateKeysResult struct { func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, error) { ks := new(OIDCTenant) ks.httpClient = httpClient - ks.acceptedZoneIds = make(map[string]bool) + ks.acceptedTenants = make(map[tenantKey]bool) err := ks.performDiscovery(targetIss.Host) if err != nil { return nil, err @@ -53,39 +59,39 @@ func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, er } // GetJWKs returns the validation keys either cached or updated ones -func (ks *OIDCTenant) GetJWKs(appTID string) (jwk.Set, error) { - keys, err := ks.readJWKsFromMemory(appTID) +func (ks *OIDCTenant) GetJWKs(appTID, clientID string) (jwk.Set, error) { + keys, err := ks.readJWKsFromMemory(appTID, clientID) if keys == nil { if err != nil { return nil, err } - return ks.updateJWKsMemory(appTID) + return ks.updateJWKsMemory(appTID, clientID) } return keys, nil } // readJWKsFromMemory returns the validation keys from memory, or error in case of invalid zone or nil, in case nothing found in memory -func (ks *OIDCTenant) readJWKsFromMemory(zoneID string) (jwk.Set, error) { +func (ks *OIDCTenant) readJWKsFromMemory(appTID, clientID string) (jwk.Set, error) { ks.mu.RLock() defer ks.mu.RUnlock() - isZoneAccepted, isZoneKnown := ks.acceptedZoneIds[zoneID] + isTenantAccepted, isTenantKnown := ks.acceptedTenants[tenantKey{appTID, clientID}] - if time.Now().Before(ks.jwksExpiry) && isZoneKnown { - if isZoneAccepted { + if time.Now().Before(ks.jwksExpiry) && isTenantKnown { + if isTenantAccepted { return ks.jwks, nil } - return nil, fmt.Errorf("zone_uuid %v is not accepted by the identity service tenant", zoneID) + return nil, fmt.Errorf("combination of app_tid: %s and client_id: %s is not accepted by the identity service", appTID, clientID) } return nil, nil } // updateJWKsMemory updates and returns the validation keys from memory, or error in case of invalid zone or nil, in case nothing found in memory -func (ks *OIDCTenant) updateJWKsMemory(zoneID string) (jwk.Set, error) { +func (ks *OIDCTenant) updateJWKsMemory(appTID, clientID string) (jwk.Set, error) { ks.mu.Lock() defer ks.mu.Unlock() - updatedKeys, err := ks.getJWKsFromServer(zoneID) + updatedKeys, err := ks.getJWKsFromServer(appTID, clientID) if err != nil { return nil, fmt.Errorf("error updating JWKs: %v", err) } @@ -96,13 +102,14 @@ func (ks *OIDCTenant) updateJWKsMemory(zoneID string) (jwk.Set, error) { return ks.jwks, nil } -func (ks *OIDCTenant) getJWKsFromServer(zoneID string) (r interface{}, err error) { +func (ks *OIDCTenant) getJWKsFromServer(appTID, clientID string) (r interface{}, err error) { result := updateKeysResult{} req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, ks.ProviderJSON.JWKsURL, http.NoBody) if err != nil { return result, fmt.Errorf("can't create request to fetch jwk: %v", err) } - req.Header.Add(zoneIDHeader, zoneID) + req.Header.Add(appTIDHeader, appTID) + req.Header.Add(clientIDHeader, clientID) resp, err := ks.httpClient.Do(req) if err != nil { @@ -111,10 +118,11 @@ func (ks *OIDCTenant) getJWKsFromServer(zoneID string) (r interface{}, err error defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - ks.acceptedZoneIds[zoneID] = false - return result, fmt.Errorf("failed to fetch jwks from remote for x-zone_uuid %s: %v (%s)", zoneID, err, resp.Body) + ks.acceptedTenants[tenantKey{appTID, clientID}] = false + return result, fmt.Errorf( + "failed to fetch jwks from remote for appTID %s and clientID %s: %v (%s)", appTID, clientID, err, resp.Body) } - ks.acceptedZoneIds[zoneID] = true + ks.acceptedTenants[tenantKey{appTID, clientID}] = true jwks, err := jwk.ParseReader(resp.Body) if err != nil { return nil, fmt.Errorf("failed to parse JWK set: %w", err) diff --git a/oidcclient/jwk_test.go b/oidcclient/jwk_test.go index 2756966..64b7ade 100644 --- a/oidcclient/jwk_test.go +++ b/oidcclient/jwk_test.go @@ -66,7 +66,8 @@ func TestProviderJSON_assertMandatoryFieldsPresent(t *testing.T) { func TestOIDCTenant_ReadJWKs(t *testing.T) { type fields struct { Duration time.Duration - ZoneID string + AppTID string + ClientID string ExpectedErrorMsg string } tests := []struct { @@ -76,19 +77,21 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { wantProviderJSON bool }{ { - name: "read from cache with accepted zone", + name: "read from cache with accepted app_tid and client_id", fields: fields{ Duration: 2 * time.Second, - ZoneID: "zone-id", + AppTID: "app-tid", + ClientID: "client-id", }, wantErr: false, wantProviderJSON: false, }, { - name: "read from cache with denied zone", + name: "read from cache with unknown app-tid", fields: fields{ Duration: 2 * time.Second, - ZoneID: "unknown-zone-id", - ExpectedErrorMsg: "zone_uuid unknown-zone-id is not accepted by the identity service tenant", + AppTID: "unknown-app-tid", + ClientID: "unknown-client-id", + ExpectedErrorMsg: "combination of app_tid: unknown-app-tid and client_id: unknown-client-id is not accepted", }, wantErr: true, wantProviderJSON: false, @@ -97,7 +100,8 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with accepted zone", fields: fields{ Duration: 0, - ZoneID: "zone-id", + AppTID: "app-tid", + ClientID: "client-id", }, wantErr: false, wantProviderJSON: true, @@ -105,8 +109,9 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with denied zone", fields: fields{ Duration: 0, - ZoneID: "unknown-zone-id", - ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for x-zone_uuid unknown-zone-id", + AppTID: "unknown-app-tid", + ClientID: "unknown-client-id", + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for appTID unknown-app-tid", }, wantErr: true, wantProviderJSON: true, @@ -114,7 +119,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with accepted zone but no jwks response", fields: fields{ Duration: 0, - ZoneID: "provide-invalidJWKS", + AppTID: "provide-invalidJWKS", ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote: ", }, wantErr: true, // as providerJSON is nil @@ -123,7 +128,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with accepted zone provoking parsing error", fields: fields{ Duration: 0, - ZoneID: "provide-invalidJWKS", + AppTID: "provide-invalidJWKS", ExpectedErrorMsg: "error updating JWKs: failed to parse JWK set: failed to unmarshal JWK set", }, wantErr: true, // as jwks endpoint returns no JSON @@ -132,8 +137,9 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with deleted zone", fields: fields{ Duration: 0, - ZoneID: "deleted-zone-id", - ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for x-zone_uuid deleted-zone-id", + AppTID: "deleted-app-tid", + ClientID: "deleted-client-id", + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for appTID deleted-app-tid", }, wantErr: true, wantProviderJSON: true, @@ -153,22 +159,26 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { } jwksJSON, _ := jwk.ParseString(jwksJSONString) tenant := OIDCTenant{ - jwksExpiry: time.Now().Add(tt.fields.Duration), - acceptedZoneIds: map[string]bool{"zone-id": true, "deleted-zone-id": true, "unknown-zone-id": false}, - httpClient: http.DefaultClient, - jwks: jwksJSON, - ProviderJSON: providerJSON, + jwksExpiry: time.Now().Add(tt.fields.Duration), + acceptedTenants: map[tenantKey]bool{ + {"app-tid", "client-id"}: true, + {"deleted-app-tid", "deleted-client-id"}: true, + {"unknown-app-tid", "unknown-client-id"}: false, + }, + httpClient: http.DefaultClient, + jwks: jwksJSON, + ProviderJSON: providerJSON, } - jwks, err := tenant.GetJWKs(tt.fields.ZoneID) + jwks, err := tenant.GetJWKs(tt.fields.AppTID, tt.fields.ClientID) if tt.wantErr { if err == nil { - t.Errorf("GetJWKs() does not provide error = %v, zoneID %v", err, tt.fields.ZoneID) + t.Errorf("GetJWKs() does not provide error = %v, appTID %v clienID %v", err, tt.fields.AppTID, tt.fields.ClientID) } if !strings.HasPrefix(err.Error(), tt.fields.ExpectedErrorMsg) { t.Errorf("GetJWKs() does not provide expected error message = %v", err.Error()) } } else if jwks == nil { - t.Errorf("GetJWKs() returns nil = %v, zoneID %v", err, tt.fields.ZoneID) + t.Errorf("GetJWKs() returns nil = %v, ppTID %v clienID %v", err, tt.fields.AppTID, tt.fields.ClientID) } }) } @@ -176,10 +186,10 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { func NewRouter() (r *mux.Router) { r = mux.NewRouter() - r.HandleFunc("/oauth2/certs", ReturnJWKS).Methods(http.MethodGet).Headers("x-zone_uuid", "zone-id") - r.HandleFunc("/oauth2/certs", ReturnInvalidZone).Methods(http.MethodGet).Headers("x-zone_uuid", "unknown-zone-id") - r.HandleFunc("/oauth2/certs", ReturnInvalidZone).Methods(http.MethodGet).Headers("x-zone_uuid", "deleted-zone-id") - r.HandleFunc("/oauth2/certs", ReturnInvalidJWKS).Methods(http.MethodGet).Headers("x-zone_uuid", "provide-invalidJWKS") + r.HandleFunc("/oauth2/certs", ReturnJWKS).Methods(http.MethodGet).Headers(appTIDHeader, "app-tid", clientIDHeader, "client-id") + r.HandleFunc("/oauth2/certs", ReturnInvalidZone).Methods(http.MethodGet).Headers(appTIDHeader, "unknown-app-tid", clientIDHeader, "unknown-client-id") + r.HandleFunc("/oauth2/certs", ReturnInvalidZone).Methods(http.MethodGet).Headers(appTIDHeader, "deleted-app-tid", clientIDHeader, "deleted-client-id") + r.HandleFunc("/oauth2/certs", ReturnInvalidJWKS).Methods(http.MethodGet).Headers(appTIDHeader, "provide-invalidJWKS") return r } From 0777788be33d251f8af600453a0a481025484cfe Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Wed, 13 Sep 2023 11:53:24 +0200 Subject: [PATCH 03/15] sort imports in sample app --- samples/middleware.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/samples/middleware.go b/samples/middleware.go index 38fa310..fb89fc8 100644 --- a/samples/middleware.go +++ b/samples/middleware.go @@ -6,12 +6,13 @@ package main import ( "fmt" - "github.com/gorilla/handlers" "log" "net/http" "os" "time" + "github.com/gorilla/handlers" + "github.com/gorilla/mux" "github.com/sap/cloud-security-client-go/auth" From 30ed551fb714d56f00fc197e9f9b10be4af2ace9 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Mon, 25 Sep 2023 15:17:45 +0200 Subject: [PATCH 04/15] change appTID type from uuid to string --- env/iasConfig.go | 15 ++++++++------- env/iasConfig_test.go | 2 +- mocks/mockServer.go | 4 ++-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/env/iasConfig.go b/env/iasConfig.go index d889f10..6326869 100644 --- a/env/iasConfig.go +++ b/env/iasConfig.go @@ -34,7 +34,7 @@ type Identity interface { GetURL() string // Returns the url to the DefaultIdentity tenant. E.g. https://abcdefgh.accounts.ondemand.com GetDomains() []string // Returns the domains of the DefaultIdentity service. E.g. ["accounts.ondemand.com"] GetZoneUUID() uuid.UUID // Deprecated: Returns the zone uuid, will be replaced by GetAppTID Optional - GetAppTID() uuid.UUID // Returns the app tid uuid and replaces zone uuid in future Optional + GetAppTID() string // Returns the app tid uuid and replaces zone uuid in future Optional GetProofTokenURL() string // Returns the proof token url. Optional GetCertificate() string // Returns the client certificate. Optional GetKey() string // Returns the client certificate key. Optional @@ -49,7 +49,7 @@ type DefaultIdentity struct { Domains []string `json:"domains"` URL string `json:"url"` ZoneUUID uuid.UUID `json:"zone_uuid"` // Deprecated: will be replaced by AppTID - AppTID uuid.UUID `json:"app_tid"` // replaces ZoneUUID + AppTID string `json:"app_tid"` // replaces ZoneUUID ProofTokenURL string `json:"prooftoken_url"` OsbURL string `json:"osb_url"` Certificate string `json:"certificate"` @@ -188,18 +188,19 @@ func (c DefaultIdentity) GetDomains() []string { // GetZoneUUID implements the env.Identity interface. // Deprecated: will be replaced by GetAppTID and removed in future func (c DefaultIdentity) GetZoneUUID() uuid.UUID { - if c.AppTID != uuid.Nil { - return c.AppTID + appTid, err := uuid.Parse(c.AppTID) + if err != nil { + return appTid } return c.ZoneUUID } // GetAppTID implements the env.Identity interface and replaces GetZoneUUID in future -func (c DefaultIdentity) GetAppTID() uuid.UUID { - if c.AppTID != uuid.Nil { +func (c DefaultIdentity) GetAppTID() string { + if c.AppTID != "" { return c.AppTID } - return c.ZoneUUID + return c.ZoneUUID.String() } // GetProofTokenURL implements the env.Identity interface. diff --git a/env/iasConfig_test.go b/env/iasConfig_test.go index df85803..f99f456 100644 --- a/env/iasConfig_test.go +++ b/env/iasConfig_test.go @@ -21,7 +21,7 @@ var testConfig = &DefaultIdentity{ Domains: []string{"accounts400.ondemand.com", "my.arbitrary.domain"}, URL: "https://mytenant.accounts400.ondemand.com", ZoneUUID: uuid.MustParse("bef12345-de57-480f-be92-1d8c1c7abf16"), - AppTID: uuid.MustParse("70cd0de3-528a-4655-b56a-5862591def5c"), + AppTID: "70cd0de3-528a-4655-b56a-5862591def5c", } func TestParseIdentityConfig(t *testing.T) { diff --git a/mocks/mockServer.go b/mocks/mockServer.go index 8486a0c..aa96e25 100644 --- a/mocks/mockServer.go +++ b/mocks/mockServer.go @@ -295,7 +295,7 @@ type MockConfig struct { URL string Domains []string ZoneUUID uuid.UUID - AppTID uuid.UUID + AppTID string ProofTokenURL string OsbURL string Certificate string @@ -329,7 +329,7 @@ func (c MockConfig) GetZoneUUID() uuid.UUID { } // GetAppTID implements the env.Identity interface. -func (c MockConfig) GetAppTID() uuid.UUID { +func (c MockConfig) GetAppTID() string { return c.AppTID } From 757e1c246443b716fba91b793da335e60fb036a6 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Mon, 25 Sep 2023 15:35:02 +0200 Subject: [PATCH 05/15] remove zone from strings in tests --- mocks/mockServer.go | 2 +- oidcclient/jwk.go | 6 +++--- oidcclient/jwk_test.go | 16 ++++++++-------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/mocks/mockServer.go b/mocks/mockServer.go index aa96e25..cbc3e20 100644 --- a/mocks/mockServer.go +++ b/mocks/mockServer.go @@ -51,7 +51,7 @@ type MockServer struct { CustomIssuer string // CustomIssuer holds a custom domain returned by the discovery endpoint } -// InvalidAppTID represents a zone guid which is rejected by mock server on behalf of IAS tenant +// InvalidAppTID represents a guid which is rejected by mock server on behalf of IAS tenant const InvalidAppTID string = "dff69954-a259-4104-9074-193bc9a366ce" // NewOIDCMockServer instantiates a new MockServer. diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index 5344b32..e88da53 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -24,7 +24,7 @@ const defaultJwkExpiration = 15 * time.Minute const appTIDHeader = "x-app_tid" const clientIDHeader = "x-client_id" -// OIDCTenant represents one IAS tenant correlating with one zone with it's OIDC discovery results and cached JWKs +// OIDCTenant represents one IAS tenant correlating with one app_tid and client_id with it's OIDC discovery results and cached JWKs type OIDCTenant struct { ProviderJSON ProviderJSON acceptedTenants map[tenantKey]bool @@ -70,7 +70,7 @@ func (ks *OIDCTenant) GetJWKs(appTID, clientID string) (jwk.Set, error) { return keys, nil } -// readJWKsFromMemory returns the validation keys from memory, or error in case of invalid zone or nil, in case nothing found in memory +// readJWKsFromMemory returns the validation keys from memory, or error in case of invalid header combination or nil, in case nothing found in memory func (ks *OIDCTenant) readJWKsFromMemory(appTID, clientID string) (jwk.Set, error) { ks.mu.RLock() defer ks.mu.RUnlock() @@ -86,7 +86,7 @@ func (ks *OIDCTenant) readJWKsFromMemory(appTID, clientID string) (jwk.Set, erro return nil, nil } -// updateJWKsMemory updates and returns the validation keys from memory, or error in case of invalid zone or nil, in case nothing found in memory +// updateJWKsMemory updates and returns the validation keys from memory, or error in case of invalid header combination nil, in case nothing found in memory func (ks *OIDCTenant) updateJWKsMemory(appTID, clientID string) (jwk.Set, error) { ks.mu.Lock() defer ks.mu.Unlock() diff --git a/oidcclient/jwk_test.go b/oidcclient/jwk_test.go index 64b7ade..fafea32 100644 --- a/oidcclient/jwk_test.go +++ b/oidcclient/jwk_test.go @@ -97,7 +97,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { wantProviderJSON: false, }, { - name: "read from token keys endpoint with accepted zone", + name: "read from token keys endpoint with accepted headers (appTID, ClientID...)", fields: fields{ Duration: 0, AppTID: "app-tid", @@ -106,7 +106,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { wantErr: false, wantProviderJSON: true, }, { - name: "read from token keys endpoint with denied zone", + name: "read from token keys endpoint with denied headers (appTID, ClientID...)", fields: fields{ Duration: 0, AppTID: "unknown-app-tid", @@ -116,7 +116,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { wantErr: true, wantProviderJSON: true, }, { - name: "read from token keys endpoint with accepted zone but no jwks response", + name: "read from token keys endpoint with accepted headers (appTID, ClientID...) but no jwks response", fields: fields{ Duration: 0, AppTID: "provide-invalidJWKS", @@ -125,7 +125,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { wantErr: true, // as providerJSON is nil wantProviderJSON: false, }, { - name: "read from token keys endpoint with accepted zone provoking parsing error", + name: "read from token keys endpoint with accepted headers (appTID, ClientID...) provoking parsing error", fields: fields{ Duration: 0, AppTID: "provide-invalidJWKS", @@ -134,7 +134,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { wantErr: true, // as jwks endpoint returns no JSON wantProviderJSON: true, }, { - name: "read from token keys endpoint with deleted zone", + name: "read from token keys endpoint with deleted headers (appTID, ClientID...)", fields: fields{ Duration: 0, AppTID: "deleted-app-tid", @@ -187,8 +187,8 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { func NewRouter() (r *mux.Router) { r = mux.NewRouter() r.HandleFunc("/oauth2/certs", ReturnJWKS).Methods(http.MethodGet).Headers(appTIDHeader, "app-tid", clientIDHeader, "client-id") - r.HandleFunc("/oauth2/certs", ReturnInvalidZone).Methods(http.MethodGet).Headers(appTIDHeader, "unknown-app-tid", clientIDHeader, "unknown-client-id") - r.HandleFunc("/oauth2/certs", ReturnInvalidZone).Methods(http.MethodGet).Headers(appTIDHeader, "deleted-app-tid", clientIDHeader, "deleted-client-id") + r.HandleFunc("/oauth2/certs", ReturnInvalidHeaders).Methods(http.MethodGet).Headers(appTIDHeader, "unknown-app-tid", clientIDHeader, "unknown-client-id") + r.HandleFunc("/oauth2/certs", ReturnInvalidHeaders).Methods(http.MethodGet).Headers(appTIDHeader, "deleted-app-tid", clientIDHeader, "deleted-client-id") r.HandleFunc("/oauth2/certs", ReturnInvalidJWKS).Methods(http.MethodGet).Headers(appTIDHeader, "provide-invalidJWKS") return r } @@ -201,6 +201,6 @@ func ReturnInvalidJWKS(writer http.ResponseWriter, _ *http.Request) { _, _ = writer.Write([]byte("\"kid\":\"default-kid-ias\"")) } -func ReturnInvalidZone(writer http.ResponseWriter, _ *http.Request) { +func ReturnInvalidHeaders(writer http.ResponseWriter, _ *http.Request) { writer.WriteHeader(400) } From 1bf9020f127c57d58bed0498b724796c4f5e1aa3 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Tue, 26 Sep 2023 10:45:31 +0200 Subject: [PATCH 06/15] remove app_tid fallback to zone_id --- auth/token.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/auth/token.go b/auth/token.go index da41af5..52714d2 100644 --- a/auth/token.go +++ b/auth/token.go @@ -129,11 +129,7 @@ func (t Token) ZoneID() string { // AppTID returns "app_tid" claim, if it doesn't exist empty string is returned func (t Token) AppTID() string { - appTID, err := t.GetClaimAsString(claimAppTID) - if errors.Is(err, ErrClaimNotExists) { - zoneUUID, _ := t.GetClaimAsString(claimSapGlobalZoneID) - return zoneUUID - } + appTID, _ := t.GetClaimAsString(claimAppTID) return appTID } From 46ff3d788afce3063e704edffb3705f42b97012d Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Thu, 28 Sep 2023 10:57:19 +0200 Subject: [PATCH 07/15] code cleanup --- auth/token.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/auth/token.go b/auth/token.go index 52714d2..a65212f 100644 --- a/auth/token.go +++ b/auth/token.go @@ -22,7 +22,7 @@ const ( claimEmail = "email" claimSapGlobalUserID = "user_uuid" claimSapGlobalZoneID = "zone_uuid" // tenant GUID - claimAppTID = "app_tid" + claimSapGlobalAppTID = "app_tid" claimIasIssuer = "ias_iss" ) @@ -117,10 +117,10 @@ func (t Token) Email() string { } // ZoneID returns "app_tid" claim, if it doesn't exist empty string is returned -// Deprecated: will be replaced by AppTID and removed in future +// Deprecated: is replaced by AppTID and will be removed with the next major release func (t Token) ZoneID() string { - appTID, err := t.GetClaimAsString(claimAppTID) - if errors.Is(err, ErrClaimNotExists) { + appTID := t.AppTID() + if appTID == "" { zoneUUID, _ := t.GetClaimAsString(claimSapGlobalZoneID) return zoneUUID } @@ -129,7 +129,7 @@ func (t Token) ZoneID() string { // AppTID returns "app_tid" claim, if it doesn't exist empty string is returned func (t Token) AppTID() string { - appTID, _ := t.GetClaimAsString(claimAppTID) + appTID, _ := t.GetClaimAsString(claimSapGlobalAppTID) return appTID } From dd5da31dde52c1d1b42a3f0f068dc0aeee238417 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Thu, 28 Sep 2023 11:07:07 +0200 Subject: [PATCH 08/15] adjust deprecation comment --- env/iasConfig.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env/iasConfig.go b/env/iasConfig.go index 6326869..4ad4dce 100644 --- a/env/iasConfig.go +++ b/env/iasConfig.go @@ -186,7 +186,7 @@ func (c DefaultIdentity) GetDomains() []string { } // GetZoneUUID implements the env.Identity interface. -// Deprecated: will be replaced by GetAppTID and removed in future +// Deprecated: is replaced by GetAppTID and will be removed with the next major release func (c DefaultIdentity) GetZoneUUID() uuid.UUID { appTid, err := uuid.Parse(c.AppTID) if err != nil { From c46e12e80f45527b0cb2075e41810411cd5229c8 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Wed, 4 Oct 2023 13:33:19 +0200 Subject: [PATCH 09/15] add azp token claim --- auth/token.go | 7 ++++ auth/validator.go | 7 +++- oidcclient/jwk.go | 41 ++++++++++++---------- oidcclient/jwk_test.go | 78 ++++++++++++++++++------------------------ 4 files changed, 68 insertions(+), 65 deletions(-) diff --git a/auth/token.go b/auth/token.go index a65212f..5e3fae7 100644 --- a/auth/token.go +++ b/auth/token.go @@ -24,6 +24,7 @@ const ( claimSapGlobalZoneID = "zone_uuid" // tenant GUID claimSapGlobalAppTID = "app_tid" claimIasIssuer = "ias_iss" + claimAzp = "azp" ) type Token struct { @@ -133,6 +134,12 @@ func (t Token) AppTID() string { return appTID } +// Azp returns "azp" claim, if it doesn't exist empty string is returned +func (t Token) Azp() string { + appTID, _ := t.GetClaimAsString(claimAzp) + return appTID +} + // UserUUID returns "user_uuid" claim, if it doesn't exist empty string is returned func (t Token) UserUUID() string { v, _ := t.GetClaimAsString(claimSapGlobalUserID) diff --git a/auth/validator.go b/auth/validator.go index da5e504..d3ecb69 100644 --- a/auth/validator.go +++ b/auth/validator.go @@ -56,7 +56,12 @@ func (m *Middleware) verifySignature(t Token, keySet *oidcclient.OIDCTenant) (er } // parse and verify signature - jwks, err := keySet.GetJWKs(t.AppTID(), m.identity.GetClientID()) + tenantOpts := oidcclient.TenantInfo{ + ClientID: m.identity.GetClientID(), + AppTID: t.AppTID(), + Azp: t.Azp(), + } + jwks, err := keySet.GetJWKs(tenantOpts) if err != nil { return err } diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index e88da53..739ba4c 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -23,11 +23,12 @@ import ( const defaultJwkExpiration = 15 * time.Minute const appTIDHeader = "x-app_tid" const clientIDHeader = "x-client_id" +const azpHeader = "x-azp" // OIDCTenant represents one IAS tenant correlating with one app_tid and client_id with it's OIDC discovery results and cached JWKs type OIDCTenant struct { ProviderJSON ProviderJSON - acceptedTenants map[tenantKey]bool + acceptedTenants map[TenantInfo]bool httpClient *http.Client // A set of cached keys and their expiry. jwks jwk.Set @@ -35,9 +36,10 @@ type OIDCTenant struct { mu sync.RWMutex } -type tenantKey struct { - appTID string - clientID string +type TenantInfo struct { + ClientID string + AppTID string + Azp string } type updateKeysResult struct { @@ -49,7 +51,7 @@ type updateKeysResult struct { func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, error) { ks := new(OIDCTenant) ks.httpClient = httpClient - ks.acceptedTenants = make(map[tenantKey]bool) + ks.acceptedTenants = make(map[TenantInfo]bool) err := ks.performDiscovery(targetIss.Host) if err != nil { return nil, err @@ -59,39 +61,39 @@ func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, er } // GetJWKs returns the validation keys either cached or updated ones -func (ks *OIDCTenant) GetJWKs(appTID, clientID string) (jwk.Set, error) { - keys, err := ks.readJWKsFromMemory(appTID, clientID) +func (ks *OIDCTenant) GetJWKs(tenant TenantInfo) (jwk.Set, error) { + keys, err := ks.readJWKsFromMemory(tenant) if keys == nil { if err != nil { return nil, err } - return ks.updateJWKsMemory(appTID, clientID) + return ks.updateJWKsMemory(tenant) } return keys, nil } // readJWKsFromMemory returns the validation keys from memory, or error in case of invalid header combination or nil, in case nothing found in memory -func (ks *OIDCTenant) readJWKsFromMemory(appTID, clientID string) (jwk.Set, error) { +func (ks *OIDCTenant) readJWKsFromMemory(tenant TenantInfo) (jwk.Set, error) { ks.mu.RLock() defer ks.mu.RUnlock() - isTenantAccepted, isTenantKnown := ks.acceptedTenants[tenantKey{appTID, clientID}] + isTenantAccepted, isTenantKnown := ks.acceptedTenants[tenant] if time.Now().Before(ks.jwksExpiry) && isTenantKnown { if isTenantAccepted { return ks.jwks, nil } - return nil, fmt.Errorf("combination of app_tid: %s and client_id: %s is not accepted by the identity service", appTID, clientID) + return nil, fmt.Errorf("tenant credentials: %+v are not accepted by the identity service", tenant) } return nil, nil } // updateJWKsMemory updates and returns the validation keys from memory, or error in case of invalid header combination nil, in case nothing found in memory -func (ks *OIDCTenant) updateJWKsMemory(appTID, clientID string) (jwk.Set, error) { +func (ks *OIDCTenant) updateJWKsMemory(tenant TenantInfo) (jwk.Set, error) { ks.mu.Lock() defer ks.mu.Unlock() - updatedKeys, err := ks.getJWKsFromServer(appTID, clientID) + updatedKeys, err := ks.getJWKsFromServer(tenant) if err != nil { return nil, fmt.Errorf("error updating JWKs: %v", err) } @@ -102,14 +104,15 @@ func (ks *OIDCTenant) updateJWKsMemory(appTID, clientID string) (jwk.Set, error) return ks.jwks, nil } -func (ks *OIDCTenant) getJWKsFromServer(appTID, clientID string) (r interface{}, err error) { +func (ks *OIDCTenant) getJWKsFromServer(tenant TenantInfo) (r interface{}, err error) { result := updateKeysResult{} req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, ks.ProviderJSON.JWKsURL, http.NoBody) if err != nil { return result, fmt.Errorf("can't create request to fetch jwk: %v", err) } - req.Header.Add(appTIDHeader, appTID) - req.Header.Add(clientIDHeader, clientID) + req.Header.Add(clientIDHeader, tenant.ClientID) + req.Header.Add(appTIDHeader, tenant.AppTID) + req.Header.Add(azpHeader, tenant.Azp) resp, err := ks.httpClient.Do(req) if err != nil { @@ -118,11 +121,11 @@ func (ks *OIDCTenant) getJWKsFromServer(appTID, clientID string) (r interface{}, defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - ks.acceptedTenants[tenantKey{appTID, clientID}] = false + ks.acceptedTenants[tenant] = false return result, fmt.Errorf( - "failed to fetch jwks from remote for appTID %s and clientID %s: %v (%s)", appTID, clientID, err, resp.Body) + "failed to fetch jwks from remote for tenant credentials %+v: %v (%s)", tenant, err, resp.Body) } - ks.acceptedTenants[tenantKey{appTID, clientID}] = true + ks.acceptedTenants[tenant] = true jwks, err := jwk.ParseReader(resp.Body) if err != nil { return nil, fmt.Errorf("failed to parse JWK set: %w", err) diff --git a/oidcclient/jwk_test.go b/oidcclient/jwk_test.go index fafea32..04b8ae6 100644 --- a/oidcclient/jwk_test.go +++ b/oidcclient/jwk_test.go @@ -66,8 +66,7 @@ func TestProviderJSON_assertMandatoryFieldsPresent(t *testing.T) { func TestOIDCTenant_ReadJWKs(t *testing.T) { type fields struct { Duration time.Duration - AppTID string - ClientID string + Tenant TenantInfo ExpectedErrorMsg string } tests := []struct { @@ -77,69 +76,58 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { wantProviderJSON bool }{ { - name: "read from cache with accepted app_tid and client_id", + name: "read from cache with accepted tenant credentials", fields: fields{ Duration: 2 * time.Second, - AppTID: "app-tid", - ClientID: "client-id", + Tenant: TenantInfo{"client-id", "app-tid", "azp"}, }, wantErr: false, wantProviderJSON: false, }, { - name: "read from cache with unknown app-tid", + name: "read from cache with unknown tenant credentials", fields: fields{ - Duration: 2 * time.Second, - AppTID: "unknown-app-tid", - ClientID: "unknown-client-id", - ExpectedErrorMsg: "combination of app_tid: unknown-app-tid and client_id: unknown-client-id is not accepted", + Duration: 2 * time.Second, + Tenant: TenantInfo{"unknown-client-id", "unknown-app-tid", "unknown-azp"}, + ExpectedErrorMsg: "tenant credentials: {ClientID:unknown-client-id AppTID:unknown-app-tid Azp:unknown-azp} " + + "are not accepted by the identity service", }, wantErr: true, wantProviderJSON: false, }, { - name: "read from token keys endpoint with accepted headers (appTID, ClientID...)", + name: "read from token keys endpoint with accepted tenant credentials", fields: fields{ Duration: 0, - AppTID: "app-tid", - ClientID: "client-id", + Tenant: TenantInfo{"client-id", "app-tid", "azp"}, }, wantErr: false, wantProviderJSON: true, }, { - name: "read from token keys endpoint with denied headers (appTID, ClientID...)", + name: "read from token keys endpoint with denied tenant credentials", fields: fields{ - Duration: 0, - AppTID: "unknown-app-tid", - ClientID: "unknown-client-id", - ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for appTID unknown-app-tid", + Duration: 0, + Tenant: TenantInfo{"unknown-client-id", "unknown-app-tid", "unknown-azp"}, + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote " + + "for tenant credentials {ClientID:unknown-client-id AppTID:unknown-app-tid Azp:unknown-azp}", }, wantErr: true, wantProviderJSON: true, }, { - name: "read from token keys endpoint with accepted headers (appTID, ClientID...) but no jwks response", - fields: fields{ - Duration: 0, - AppTID: "provide-invalidJWKS", - ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote: ", - }, - wantErr: true, // as providerJSON is nil - wantProviderJSON: false, - }, { - name: "read from token keys endpoint with accepted headers (appTID, ClientID...) provoking parsing error", + name: "read from token keys endpoint with accepted tenant credentials provoking parsing error", fields: fields{ Duration: 0, - AppTID: "provide-invalidJWKS", + Tenant: TenantInfo{ClientID: "provide-invalidJWKS"}, ExpectedErrorMsg: "error updating JWKs: failed to parse JWK set: failed to unmarshal JWK set", }, wantErr: true, // as jwks endpoint returns no JSON wantProviderJSON: true, }, { - name: "read from token keys endpoint with deleted headers (appTID, ClientID...)", + name: "read from token keys endpoint with deleted tenant credentials", fields: fields{ - Duration: 0, - AppTID: "deleted-app-tid", - ClientID: "deleted-client-id", - ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for appTID deleted-app-tid", + Duration: 0, + Tenant: TenantInfo{"deleted-client-id", "deleted-app-tid", "deleted-azp"}, + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for " + + "tenant credentials {ClientID:deleted-client-id AppTID:deleted-app-tid Azp:deleted-azp}", }, wantErr: true, wantProviderJSON: true, @@ -160,25 +148,25 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { jwksJSON, _ := jwk.ParseString(jwksJSONString) tenant := OIDCTenant{ jwksExpiry: time.Now().Add(tt.fields.Duration), - acceptedTenants: map[tenantKey]bool{ - {"app-tid", "client-id"}: true, - {"deleted-app-tid", "deleted-client-id"}: true, - {"unknown-app-tid", "unknown-client-id"}: false, + acceptedTenants: map[TenantInfo]bool{ + {ClientID: "client-id", AppTID: "app-tid", Azp: "azp"}: true, + {ClientID: "deleted-client-id", AppTID: "deleted-app-tid", Azp: "deleted-azp"}: true, + {ClientID: "unknown-client-id", AppTID: "unknown-app-tid", Azp: "unknown-azp"}: false, }, httpClient: http.DefaultClient, jwks: jwksJSON, ProviderJSON: providerJSON, } - jwks, err := tenant.GetJWKs(tt.fields.AppTID, tt.fields.ClientID) + jwks, err := tenant.GetJWKs(tt.fields.Tenant) if tt.wantErr { if err == nil { - t.Errorf("GetJWKs() does not provide error = %v, appTID %v clienID %v", err, tt.fields.AppTID, tt.fields.ClientID) + t.Errorf("GetJWKs() does not provide error = %v, tenantCredentials %+v", err, tt.fields.Tenant) } if !strings.HasPrefix(err.Error(), tt.fields.ExpectedErrorMsg) { t.Errorf("GetJWKs() does not provide expected error message = %v", err.Error()) } } else if jwks == nil { - t.Errorf("GetJWKs() returns nil = %v, ppTID %v clienID %v", err, tt.fields.AppTID, tt.fields.ClientID) + t.Errorf("GetJWKs() returns nil = %v, tenantCredentials %+v", err, tt.fields.Tenant) } }) } @@ -186,10 +174,10 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { func NewRouter() (r *mux.Router) { r = mux.NewRouter() - r.HandleFunc("/oauth2/certs", ReturnJWKS).Methods(http.MethodGet).Headers(appTIDHeader, "app-tid", clientIDHeader, "client-id") - r.HandleFunc("/oauth2/certs", ReturnInvalidHeaders).Methods(http.MethodGet).Headers(appTIDHeader, "unknown-app-tid", clientIDHeader, "unknown-client-id") - r.HandleFunc("/oauth2/certs", ReturnInvalidHeaders).Methods(http.MethodGet).Headers(appTIDHeader, "deleted-app-tid", clientIDHeader, "deleted-client-id") - r.HandleFunc("/oauth2/certs", ReturnInvalidJWKS).Methods(http.MethodGet).Headers(appTIDHeader, "provide-invalidJWKS") + r.HandleFunc("/oauth2/certs", ReturnJWKS).Methods(http.MethodGet).Headers(clientIDHeader, "client-id", appTIDHeader, "app-tid", azpHeader, "azp") + r.HandleFunc("/oauth2/certs", ReturnInvalidHeaders).Methods(http.MethodGet).Headers(clientIDHeader, "unknown-client-id", appTIDHeader, "unknown-app-tid", azpHeader, "unknown-azp") + r.HandleFunc("/oauth2/certs", ReturnInvalidHeaders).Methods(http.MethodGet).Headers(clientIDHeader, "deleted-client-id", appTIDHeader, "deleted-app-tid", azpHeader, "deleted-azp") + r.HandleFunc("/oauth2/certs", ReturnInvalidJWKS).Methods(http.MethodGet).Headers(clientIDHeader, "provide-invalidJWKS") return r } From 751fd871305ac8b3d8ff63ff5117dee9a8d1e06e Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Wed, 4 Oct 2023 14:22:24 +0200 Subject: [PATCH 10/15] improve test cases --- oidcclient/jwk.go | 8 ++++- oidcclient/jwk_test.go | 71 +++++++++++++++++++++++++++++++++--------- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index 739ba4c..225cd67 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -110,6 +110,7 @@ func (ks *OIDCTenant) getJWKsFromServer(tenant TenantInfo) (r interface{}, err e if err != nil { return result, fmt.Errorf("can't create request to fetch jwk: %v", err) } + // at least client-id is necessary, all further headers only refine the validation req.Header.Add(clientIDHeader, tenant.ClientID) req.Header.Add(appTIDHeader, tenant.AppTID) req.Header.Add(azpHeader, tenant.Azp) @@ -122,8 +123,13 @@ func (ks *OIDCTenant) getJWKsFromServer(tenant TenantInfo) (r interface{}, err e if resp.StatusCode != http.StatusOK { ks.acceptedTenants[tenant] = false + resp, err := io.ReadAll(resp.Body) + if err != nil { + return result, fmt.Errorf( + "failed to fetch jwks from remote for tenant credentials %+v: %v", tenant, err) + } return result, fmt.Errorf( - "failed to fetch jwks from remote for tenant credentials %+v: %v (%s)", tenant, err, resp.Body) + "failed to fetch jwks from remote for tenant credentials %+v: (%s)", tenant, resp) } ks.acceptedTenants[tenant] = true jwks, err := jwk.ParseReader(resp.Body) diff --git a/oidcclient/jwk_test.go b/oidcclient/jwk_test.go index 04b8ae6..9721a03 100644 --- a/oidcclient/jwk_test.go +++ b/oidcclient/jwk_test.go @@ -84,17 +84,46 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { wantErr: false, wantProviderJSON: false, }, { - name: "read from cache with unknown tenant credentials", + name: "read from cache with invalid tenant credentials", fields: fields{ Duration: 2 * time.Second, - Tenant: TenantInfo{"unknown-client-id", "unknown-app-tid", "unknown-azp"}, - ExpectedErrorMsg: "tenant credentials: {ClientID:unknown-client-id AppTID:unknown-app-tid Azp:unknown-azp} " + + Tenant: TenantInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, + ExpectedErrorMsg: "tenant credentials: {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp} " + "are not accepted by the identity service", }, wantErr: true, wantProviderJSON: false, - }, - { + }, { + name: "read token endpoint with invalid client_id", + fields: fields{ + Duration: 2 * time.Second, + Tenant: TenantInfo{"invalid-client-id", "app-tid", "azp"}, + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + + "{ClientID:invalid-client-id AppTID:app-tid Azp:azp}: ({\"msg\":\"Invalid x-client_id or x-app_tid provided\"})", + }, + wantErr: true, + wantProviderJSON: true, + }, { + name: "read token endpoint with invalid app_tid", + fields: fields{ + Duration: 2 * time.Second, + Tenant: TenantInfo{"client-id", "invalid-app-tid", "azp"}, + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + + "{ClientID:client-id AppTID:invalid-app-tid Azp:azp}: ({\"msg\":\"Invalid x-client_id or x-app_tid provided\"})", + }, + wantErr: true, + wantProviderJSON: true, + }, { + name: "read token endpoint with invalid azp", + fields: fields{ + Duration: 2 * time.Second, + Tenant: TenantInfo{"client-id", "app-tid", "invalid-azp"}, + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + + "{ClientID:client-id AppTID:app-tid Azp:invalid-azp}: ({\"msg\":\"Invalid x-azp provided\"})", + }, + wantErr: true, + wantProviderJSON: true, + }, { name: "read from token keys endpoint with accepted tenant credentials", fields: fields{ Duration: 0, @@ -106,9 +135,9 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with denied tenant credentials", fields: fields{ Duration: 0, - Tenant: TenantInfo{"unknown-client-id", "unknown-app-tid", "unknown-azp"}, + Tenant: TenantInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote " + - "for tenant credentials {ClientID:unknown-client-id AppTID:unknown-app-tid Azp:unknown-azp}", + "for tenant credentials {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp}", }, wantErr: true, wantProviderJSON: true, @@ -151,7 +180,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { acceptedTenants: map[TenantInfo]bool{ {ClientID: "client-id", AppTID: "app-tid", Azp: "azp"}: true, {ClientID: "deleted-client-id", AppTID: "deleted-app-tid", Azp: "deleted-azp"}: true, - {ClientID: "unknown-client-id", AppTID: "unknown-app-tid", Azp: "unknown-azp"}: false, + {ClientID: "invalid-client-id", AppTID: "invalid-app-tid", Azp: "invalid-azp"}: false, }, httpClient: http.DefaultClient, jwks: jwksJSON, @@ -175,20 +204,32 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { func NewRouter() (r *mux.Router) { r = mux.NewRouter() r.HandleFunc("/oauth2/certs", ReturnJWKS).Methods(http.MethodGet).Headers(clientIDHeader, "client-id", appTIDHeader, "app-tid", azpHeader, "azp") - r.HandleFunc("/oauth2/certs", ReturnInvalidHeaders).Methods(http.MethodGet).Headers(clientIDHeader, "unknown-client-id", appTIDHeader, "unknown-app-tid", azpHeader, "unknown-azp") + r.HandleFunc("/oauth2/certs", ReturnInvalidTenant).Methods(http.MethodGet).Headers(clientIDHeader, "invalid-client-id") + r.HandleFunc("/oauth2/certs", ReturnInvalidTenant).Methods(http.MethodGet).Headers(appTIDHeader, "invalid-app-tid") + r.HandleFunc("/oauth2/certs", ReturnInvalidTenant).Methods(http.MethodGet).Headers(azpHeader, "invalid-azp") r.HandleFunc("/oauth2/certs", ReturnInvalidHeaders).Methods(http.MethodGet).Headers(clientIDHeader, "deleted-client-id", appTIDHeader, "deleted-app-tid", azpHeader, "deleted-azp") r.HandleFunc("/oauth2/certs", ReturnInvalidJWKS).Methods(http.MethodGet).Headers(clientIDHeader, "provide-invalidJWKS") return r } -func ReturnJWKS(writer http.ResponseWriter, _ *http.Request) { - _, _ = writer.Write([]byte(jwksJSONString)) +func ReturnJWKS(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(jwksJSONString)) } -func ReturnInvalidJWKS(writer http.ResponseWriter, _ *http.Request) { - _, _ = writer.Write([]byte("\"kid\":\"default-kid-ias\"")) +func ReturnInvalidJWKS(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("\"kid\":\"default-kid-ias\"")) } -func ReturnInvalidHeaders(writer http.ResponseWriter, _ *http.Request) { - writer.WriteHeader(400) +func ReturnInvalidHeaders(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(400) +} + +func ReturnInvalidTenant(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(400) + w.Header().Set("Content-Type", "application/json") + if r.Header.Get(azpHeader) == "invalid-azp" { + _, _ = w.Write([]byte(`{"msg":"Invalid x-azp provided"}`)) + } else { + _, _ = w.Write([]byte(`{"msg":"Invalid x-client_id or x-app_tid provided"}`)) + } } From 900b66995894585388a6d824ff507facde54cf5d Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 6 Oct 2023 17:28:35 +0200 Subject: [PATCH 11/15] rename TenantInfo to just Info --- auth/validator.go | 2 +- oidcclient/jwk.go | 14 +++++++------- oidcclient/jwk_test.go | 22 +++++++++++----------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/auth/validator.go b/auth/validator.go index d3ecb69..75b6915 100644 --- a/auth/validator.go +++ b/auth/validator.go @@ -56,7 +56,7 @@ func (m *Middleware) verifySignature(t Token, keySet *oidcclient.OIDCTenant) (er } // parse and verify signature - tenantOpts := oidcclient.TenantInfo{ + tenantOpts := oidcclient.Info{ ClientID: m.identity.GetClientID(), AppTID: t.AppTID(), Azp: t.Azp(), diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index 225cd67..5e26c8a 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -28,7 +28,7 @@ const azpHeader = "x-azp" // OIDCTenant represents one IAS tenant correlating with one app_tid and client_id with it's OIDC discovery results and cached JWKs type OIDCTenant struct { ProviderJSON ProviderJSON - acceptedTenants map[TenantInfo]bool + acceptedTenants map[Info]bool httpClient *http.Client // A set of cached keys and their expiry. jwks jwk.Set @@ -36,7 +36,7 @@ type OIDCTenant struct { mu sync.RWMutex } -type TenantInfo struct { +type Info struct { ClientID string AppTID string Azp string @@ -51,7 +51,7 @@ type updateKeysResult struct { func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, error) { ks := new(OIDCTenant) ks.httpClient = httpClient - ks.acceptedTenants = make(map[TenantInfo]bool) + ks.acceptedTenants = make(map[Info]bool) err := ks.performDiscovery(targetIss.Host) if err != nil { return nil, err @@ -61,7 +61,7 @@ func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, er } // GetJWKs returns the validation keys either cached or updated ones -func (ks *OIDCTenant) GetJWKs(tenant TenantInfo) (jwk.Set, error) { +func (ks *OIDCTenant) GetJWKs(tenant Info) (jwk.Set, error) { keys, err := ks.readJWKsFromMemory(tenant) if keys == nil { if err != nil { @@ -73,7 +73,7 @@ func (ks *OIDCTenant) GetJWKs(tenant TenantInfo) (jwk.Set, error) { } // readJWKsFromMemory returns the validation keys from memory, or error in case of invalid header combination or nil, in case nothing found in memory -func (ks *OIDCTenant) readJWKsFromMemory(tenant TenantInfo) (jwk.Set, error) { +func (ks *OIDCTenant) readJWKsFromMemory(tenant Info) (jwk.Set, error) { ks.mu.RLock() defer ks.mu.RUnlock() @@ -89,7 +89,7 @@ func (ks *OIDCTenant) readJWKsFromMemory(tenant TenantInfo) (jwk.Set, error) { } // updateJWKsMemory updates and returns the validation keys from memory, or error in case of invalid header combination nil, in case nothing found in memory -func (ks *OIDCTenant) updateJWKsMemory(tenant TenantInfo) (jwk.Set, error) { +func (ks *OIDCTenant) updateJWKsMemory(tenant Info) (jwk.Set, error) { ks.mu.Lock() defer ks.mu.Unlock() @@ -104,7 +104,7 @@ func (ks *OIDCTenant) updateJWKsMemory(tenant TenantInfo) (jwk.Set, error) { return ks.jwks, nil } -func (ks *OIDCTenant) getJWKsFromServer(tenant TenantInfo) (r interface{}, err error) { +func (ks *OIDCTenant) getJWKsFromServer(tenant Info) (r interface{}, err error) { result := updateKeysResult{} req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, ks.ProviderJSON.JWKsURL, http.NoBody) if err != nil { diff --git a/oidcclient/jwk_test.go b/oidcclient/jwk_test.go index 9721a03..11f0f61 100644 --- a/oidcclient/jwk_test.go +++ b/oidcclient/jwk_test.go @@ -66,7 +66,7 @@ func TestProviderJSON_assertMandatoryFieldsPresent(t *testing.T) { func TestOIDCTenant_ReadJWKs(t *testing.T) { type fields struct { Duration time.Duration - Tenant TenantInfo + Tenant Info ExpectedErrorMsg string } tests := []struct { @@ -79,7 +79,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from cache with accepted tenant credentials", fields: fields{ Duration: 2 * time.Second, - Tenant: TenantInfo{"client-id", "app-tid", "azp"}, + Tenant: Info{"client-id", "app-tid", "azp"}, }, wantErr: false, wantProviderJSON: false, @@ -87,7 +87,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from cache with invalid tenant credentials", fields: fields{ Duration: 2 * time.Second, - Tenant: TenantInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, + Tenant: Info{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, ExpectedErrorMsg: "tenant credentials: {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp} " + "are not accepted by the identity service", }, @@ -97,7 +97,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read token endpoint with invalid client_id", fields: fields{ Duration: 2 * time.Second, - Tenant: TenantInfo{"invalid-client-id", "app-tid", "azp"}, + Tenant: Info{"invalid-client-id", "app-tid", "azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + "{ClientID:invalid-client-id AppTID:app-tid Azp:azp}: ({\"msg\":\"Invalid x-client_id or x-app_tid provided\"})", }, @@ -107,7 +107,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read token endpoint with invalid app_tid", fields: fields{ Duration: 2 * time.Second, - Tenant: TenantInfo{"client-id", "invalid-app-tid", "azp"}, + Tenant: Info{"client-id", "invalid-app-tid", "azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + "{ClientID:client-id AppTID:invalid-app-tid Azp:azp}: ({\"msg\":\"Invalid x-client_id or x-app_tid provided\"})", }, @@ -117,7 +117,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read token endpoint with invalid azp", fields: fields{ Duration: 2 * time.Second, - Tenant: TenantInfo{"client-id", "app-tid", "invalid-azp"}, + Tenant: Info{"client-id", "app-tid", "invalid-azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + "{ClientID:client-id AppTID:app-tid Azp:invalid-azp}: ({\"msg\":\"Invalid x-azp provided\"})", }, @@ -127,7 +127,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with accepted tenant credentials", fields: fields{ Duration: 0, - Tenant: TenantInfo{"client-id", "app-tid", "azp"}, + Tenant: Info{"client-id", "app-tid", "azp"}, }, wantErr: false, wantProviderJSON: true, @@ -135,7 +135,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with denied tenant credentials", fields: fields{ Duration: 0, - Tenant: TenantInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, + Tenant: Info{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote " + "for tenant credentials {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp}", }, @@ -145,7 +145,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with accepted tenant credentials provoking parsing error", fields: fields{ Duration: 0, - Tenant: TenantInfo{ClientID: "provide-invalidJWKS"}, + Tenant: Info{ClientID: "provide-invalidJWKS"}, ExpectedErrorMsg: "error updating JWKs: failed to parse JWK set: failed to unmarshal JWK set", }, wantErr: true, // as jwks endpoint returns no JSON @@ -154,7 +154,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with deleted tenant credentials", fields: fields{ Duration: 0, - Tenant: TenantInfo{"deleted-client-id", "deleted-app-tid", "deleted-azp"}, + Tenant: Info{"deleted-client-id", "deleted-app-tid", "deleted-azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for " + "tenant credentials {ClientID:deleted-client-id AppTID:deleted-app-tid Azp:deleted-azp}", }, @@ -177,7 +177,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { jwksJSON, _ := jwk.ParseString(jwksJSONString) tenant := OIDCTenant{ jwksExpiry: time.Now().Add(tt.fields.Duration), - acceptedTenants: map[TenantInfo]bool{ + acceptedTenants: map[Info]bool{ {ClientID: "client-id", AppTID: "app-tid", Azp: "azp"}: true, {ClientID: "deleted-client-id", AppTID: "deleted-app-tid", Azp: "deleted-azp"}: true, {ClientID: "invalid-client-id", AppTID: "invalid-app-tid", Azp: "invalid-azp"}: false, From 50980fcd0daa069cd89e4258c8c4a2df4babb1d6 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 6 Oct 2023 17:35:56 +0200 Subject: [PATCH 12/15] rename TenantInfo to ClientInfo --- auth/validator.go | 2 +- oidcclient/jwk.go | 38 +++++++++++++++++++------------------- oidcclient/jwk_test.go | 22 +++++++++++----------- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/auth/validator.go b/auth/validator.go index 75b6915..ab2f2b7 100644 --- a/auth/validator.go +++ b/auth/validator.go @@ -56,7 +56,7 @@ func (m *Middleware) verifySignature(t Token, keySet *oidcclient.OIDCTenant) (er } // parse and verify signature - tenantOpts := oidcclient.Info{ + tenantOpts := oidcclient.ClientInfo{ ClientID: m.identity.GetClientID(), AppTID: t.AppTID(), Azp: t.Azp(), diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index 5e26c8a..4993346 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -28,7 +28,7 @@ const azpHeader = "x-azp" // OIDCTenant represents one IAS tenant correlating with one app_tid and client_id with it's OIDC discovery results and cached JWKs type OIDCTenant struct { ProviderJSON ProviderJSON - acceptedTenants map[Info]bool + acceptedTenants map[ClientInfo]bool httpClient *http.Client // A set of cached keys and their expiry. jwks jwk.Set @@ -36,7 +36,7 @@ type OIDCTenant struct { mu sync.RWMutex } -type Info struct { +type ClientInfo struct { ClientID string AppTID string Azp string @@ -51,7 +51,7 @@ type updateKeysResult struct { func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, error) { ks := new(OIDCTenant) ks.httpClient = httpClient - ks.acceptedTenants = make(map[Info]bool) + ks.acceptedTenants = make(map[ClientInfo]bool) err := ks.performDiscovery(targetIss.Host) if err != nil { return nil, err @@ -61,39 +61,39 @@ func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, er } // GetJWKs returns the validation keys either cached or updated ones -func (ks *OIDCTenant) GetJWKs(tenant Info) (jwk.Set, error) { - keys, err := ks.readJWKsFromMemory(tenant) +func (ks *OIDCTenant) GetJWKs(clientInfo ClientInfo) (jwk.Set, error) { + keys, err := ks.readJWKsFromMemory(clientInfo) if keys == nil { if err != nil { return nil, err } - return ks.updateJWKsMemory(tenant) + return ks.updateJWKsMemory(clientInfo) } return keys, nil } // readJWKsFromMemory returns the validation keys from memory, or error in case of invalid header combination or nil, in case nothing found in memory -func (ks *OIDCTenant) readJWKsFromMemory(tenant Info) (jwk.Set, error) { +func (ks *OIDCTenant) readJWKsFromMemory(clientInfo ClientInfo) (jwk.Set, error) { ks.mu.RLock() defer ks.mu.RUnlock() - isTenantAccepted, isTenantKnown := ks.acceptedTenants[tenant] + isTenantAccepted, isTenantKnown := ks.acceptedTenants[clientInfo] if time.Now().Before(ks.jwksExpiry) && isTenantKnown { if isTenantAccepted { return ks.jwks, nil } - return nil, fmt.Errorf("tenant credentials: %+v are not accepted by the identity service", tenant) + return nil, fmt.Errorf("tenant credentials: %+v are not accepted by the identity service", clientInfo) } return nil, nil } // updateJWKsMemory updates and returns the validation keys from memory, or error in case of invalid header combination nil, in case nothing found in memory -func (ks *OIDCTenant) updateJWKsMemory(tenant Info) (jwk.Set, error) { +func (ks *OIDCTenant) updateJWKsMemory(clientInfo ClientInfo) (jwk.Set, error) { ks.mu.Lock() defer ks.mu.Unlock() - updatedKeys, err := ks.getJWKsFromServer(tenant) + updatedKeys, err := ks.getJWKsFromServer(clientInfo) if err != nil { return nil, fmt.Errorf("error updating JWKs: %v", err) } @@ -104,16 +104,16 @@ func (ks *OIDCTenant) updateJWKsMemory(tenant Info) (jwk.Set, error) { return ks.jwks, nil } -func (ks *OIDCTenant) getJWKsFromServer(tenant Info) (r interface{}, err error) { +func (ks *OIDCTenant) getJWKsFromServer(clientInfo ClientInfo) (r interface{}, err error) { result := updateKeysResult{} req, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, ks.ProviderJSON.JWKsURL, http.NoBody) if err != nil { return result, fmt.Errorf("can't create request to fetch jwk: %v", err) } // at least client-id is necessary, all further headers only refine the validation - req.Header.Add(clientIDHeader, tenant.ClientID) - req.Header.Add(appTIDHeader, tenant.AppTID) - req.Header.Add(azpHeader, tenant.Azp) + req.Header.Add(clientIDHeader, clientInfo.ClientID) + req.Header.Add(appTIDHeader, clientInfo.AppTID) + req.Header.Add(azpHeader, clientInfo.Azp) resp, err := ks.httpClient.Do(req) if err != nil { @@ -122,16 +122,16 @@ func (ks *OIDCTenant) getJWKsFromServer(tenant Info) (r interface{}, err error) defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - ks.acceptedTenants[tenant] = false + ks.acceptedTenants[clientInfo] = false resp, err := io.ReadAll(resp.Body) if err != nil { return result, fmt.Errorf( - "failed to fetch jwks from remote for tenant credentials %+v: %v", tenant, err) + "failed to fetch jwks from remote for tenant credentials %+v: %v", clientInfo, err) } return result, fmt.Errorf( - "failed to fetch jwks from remote for tenant credentials %+v: (%s)", tenant, resp) + "failed to fetch jwks from remote for tenant credentials %+v: (%s)", clientInfo, resp) } - ks.acceptedTenants[tenant] = true + ks.acceptedTenants[clientInfo] = true jwks, err := jwk.ParseReader(resp.Body) if err != nil { return nil, fmt.Errorf("failed to parse JWK set: %w", err) diff --git a/oidcclient/jwk_test.go b/oidcclient/jwk_test.go index 11f0f61..008e0a3 100644 --- a/oidcclient/jwk_test.go +++ b/oidcclient/jwk_test.go @@ -66,7 +66,7 @@ func TestProviderJSON_assertMandatoryFieldsPresent(t *testing.T) { func TestOIDCTenant_ReadJWKs(t *testing.T) { type fields struct { Duration time.Duration - Tenant Info + Tenant ClientInfo ExpectedErrorMsg string } tests := []struct { @@ -79,7 +79,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from cache with accepted tenant credentials", fields: fields{ Duration: 2 * time.Second, - Tenant: Info{"client-id", "app-tid", "azp"}, + Tenant: ClientInfo{"client-id", "app-tid", "azp"}, }, wantErr: false, wantProviderJSON: false, @@ -87,7 +87,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from cache with invalid tenant credentials", fields: fields{ Duration: 2 * time.Second, - Tenant: Info{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, + Tenant: ClientInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, ExpectedErrorMsg: "tenant credentials: {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp} " + "are not accepted by the identity service", }, @@ -97,7 +97,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read token endpoint with invalid client_id", fields: fields{ Duration: 2 * time.Second, - Tenant: Info{"invalid-client-id", "app-tid", "azp"}, + Tenant: ClientInfo{"invalid-client-id", "app-tid", "azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + "{ClientID:invalid-client-id AppTID:app-tid Azp:azp}: ({\"msg\":\"Invalid x-client_id or x-app_tid provided\"})", }, @@ -107,7 +107,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read token endpoint with invalid app_tid", fields: fields{ Duration: 2 * time.Second, - Tenant: Info{"client-id", "invalid-app-tid", "azp"}, + Tenant: ClientInfo{"client-id", "invalid-app-tid", "azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + "{ClientID:client-id AppTID:invalid-app-tid Azp:azp}: ({\"msg\":\"Invalid x-client_id or x-app_tid provided\"})", }, @@ -117,7 +117,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read token endpoint with invalid azp", fields: fields{ Duration: 2 * time.Second, - Tenant: Info{"client-id", "app-tid", "invalid-azp"}, + Tenant: ClientInfo{"client-id", "app-tid", "invalid-azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + "{ClientID:client-id AppTID:app-tid Azp:invalid-azp}: ({\"msg\":\"Invalid x-azp provided\"})", }, @@ -127,7 +127,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with accepted tenant credentials", fields: fields{ Duration: 0, - Tenant: Info{"client-id", "app-tid", "azp"}, + Tenant: ClientInfo{"client-id", "app-tid", "azp"}, }, wantErr: false, wantProviderJSON: true, @@ -135,7 +135,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with denied tenant credentials", fields: fields{ Duration: 0, - Tenant: Info{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, + Tenant: ClientInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote " + "for tenant credentials {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp}", }, @@ -145,7 +145,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with accepted tenant credentials provoking parsing error", fields: fields{ Duration: 0, - Tenant: Info{ClientID: "provide-invalidJWKS"}, + Tenant: ClientInfo{ClientID: "provide-invalidJWKS"}, ExpectedErrorMsg: "error updating JWKs: failed to parse JWK set: failed to unmarshal JWK set", }, wantErr: true, // as jwks endpoint returns no JSON @@ -154,7 +154,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read from token keys endpoint with deleted tenant credentials", fields: fields{ Duration: 0, - Tenant: Info{"deleted-client-id", "deleted-app-tid", "deleted-azp"}, + Tenant: ClientInfo{"deleted-client-id", "deleted-app-tid", "deleted-azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for " + "tenant credentials {ClientID:deleted-client-id AppTID:deleted-app-tid Azp:deleted-azp}", }, @@ -177,7 +177,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { jwksJSON, _ := jwk.ParseString(jwksJSONString) tenant := OIDCTenant{ jwksExpiry: time.Now().Add(tt.fields.Duration), - acceptedTenants: map[Info]bool{ + acceptedTenants: map[ClientInfo]bool{ {ClientID: "client-id", AppTID: "app-tid", Azp: "azp"}: true, {ClientID: "deleted-client-id", AppTID: "deleted-app-tid", Azp: "deleted-azp"}: true, {ClientID: "invalid-client-id", AppTID: "invalid-app-tid", Azp: "invalid-azp"}: false, From 68961263b22abd6ad981ae085d2270141a51fae3 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 6 Oct 2023 17:52:15 +0200 Subject: [PATCH 13/15] prevent caching ias flaps in jwks backend call --- oidcclient/jwk.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index 4993346..46b52ef 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -122,7 +122,10 @@ func (ks *OIDCTenant) getJWKsFromServer(clientInfo ClientInfo) (r interface{}, e defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - ks.acceptedTenants[clientInfo] = false + // prevent caching ias backend flaps like 503 -> only cache 400 + if resp.StatusCode == http.StatusBadRequest { + ks.acceptedTenants[clientInfo] = false + } resp, err := io.ReadAll(resp.Body) if err != nil { return result, fmt.Errorf( From 2fbc4daa304b6f725d8222621c2524fea3398ae8 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 6 Oct 2023 17:54:35 +0200 Subject: [PATCH 14/15] rename acceptedTenants to acceptedClients --- oidcclient/jwk.go | 10 +++++----- oidcclient/jwk_test.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index 46b52ef..0ef7f8d 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -28,7 +28,7 @@ const azpHeader = "x-azp" // OIDCTenant represents one IAS tenant correlating with one app_tid and client_id with it's OIDC discovery results and cached JWKs type OIDCTenant struct { ProviderJSON ProviderJSON - acceptedTenants map[ClientInfo]bool + acceptedClients map[ClientInfo]bool httpClient *http.Client // A set of cached keys and their expiry. jwks jwk.Set @@ -51,7 +51,7 @@ type updateKeysResult struct { func NewOIDCTenant(httpClient *http.Client, targetIss *url.URL) (*OIDCTenant, error) { ks := new(OIDCTenant) ks.httpClient = httpClient - ks.acceptedTenants = make(map[ClientInfo]bool) + ks.acceptedClients = make(map[ClientInfo]bool) err := ks.performDiscovery(targetIss.Host) if err != nil { return nil, err @@ -77,7 +77,7 @@ func (ks *OIDCTenant) readJWKsFromMemory(clientInfo ClientInfo) (jwk.Set, error) ks.mu.RLock() defer ks.mu.RUnlock() - isTenantAccepted, isTenantKnown := ks.acceptedTenants[clientInfo] + isTenantAccepted, isTenantKnown := ks.acceptedClients[clientInfo] if time.Now().Before(ks.jwksExpiry) && isTenantKnown { if isTenantAccepted { @@ -124,7 +124,7 @@ func (ks *OIDCTenant) getJWKsFromServer(clientInfo ClientInfo) (r interface{}, e if resp.StatusCode != http.StatusOK { // prevent caching ias backend flaps like 503 -> only cache 400 if resp.StatusCode == http.StatusBadRequest { - ks.acceptedTenants[clientInfo] = false + ks.acceptedClients[clientInfo] = false } resp, err := io.ReadAll(resp.Body) if err != nil { @@ -134,7 +134,7 @@ func (ks *OIDCTenant) getJWKsFromServer(clientInfo ClientInfo) (r interface{}, e return result, fmt.Errorf( "failed to fetch jwks from remote for tenant credentials %+v: (%s)", clientInfo, resp) } - ks.acceptedTenants[clientInfo] = true + ks.acceptedClients[clientInfo] = true jwks, err := jwk.ParseReader(resp.Body) if err != nil { return nil, fmt.Errorf("failed to parse JWK set: %w", err) diff --git a/oidcclient/jwk_test.go b/oidcclient/jwk_test.go index 008e0a3..1e8b6a9 100644 --- a/oidcclient/jwk_test.go +++ b/oidcclient/jwk_test.go @@ -177,7 +177,7 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { jwksJSON, _ := jwk.ParseString(jwksJSONString) tenant := OIDCTenant{ jwksExpiry: time.Now().Add(tt.fields.Duration), - acceptedTenants: map[ClientInfo]bool{ + acceptedClients: map[ClientInfo]bool{ {ClientID: "client-id", AppTID: "app-tid", Azp: "azp"}: true, {ClientID: "deleted-client-id", AppTID: "deleted-app-tid", Azp: "deleted-azp"}: true, {ClientID: "invalid-client-id", AppTID: "invalid-app-tid", Azp: "invalid-azp"}: false, From 21fbd3d5cd0eb40d095bb6b3f25664585cf1b4e4 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 6 Oct 2023 18:00:44 +0200 Subject: [PATCH 15/15] more renaming from tenant to client --- oidcclient/jwk.go | 12 ++++----- oidcclient/jwk_test.go | 58 +++++++++++++++++++++--------------------- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/oidcclient/jwk.go b/oidcclient/jwk.go index 0ef7f8d..5e83a34 100644 --- a/oidcclient/jwk.go +++ b/oidcclient/jwk.go @@ -77,13 +77,13 @@ func (ks *OIDCTenant) readJWKsFromMemory(clientInfo ClientInfo) (jwk.Set, error) ks.mu.RLock() defer ks.mu.RUnlock() - isTenantAccepted, isTenantKnown := ks.acceptedClients[clientInfo] + isClientAccepted, isClientKnown := ks.acceptedClients[clientInfo] - if time.Now().Before(ks.jwksExpiry) && isTenantKnown { - if isTenantAccepted { + if time.Now().Before(ks.jwksExpiry) && isClientKnown { + if isClientAccepted { return ks.jwks, nil } - return nil, fmt.Errorf("tenant credentials: %+v are not accepted by the identity service", clientInfo) + return nil, fmt.Errorf("client credentials: %+v are not accepted by the identity service", clientInfo) } return nil, nil } @@ -129,10 +129,10 @@ func (ks *OIDCTenant) getJWKsFromServer(clientInfo ClientInfo) (r interface{}, e resp, err := io.ReadAll(resp.Body) if err != nil { return result, fmt.Errorf( - "failed to fetch jwks from remote for tenant credentials %+v: %v", clientInfo, err) + "failed to fetch jwks from remote for client credentials %+v: %v", clientInfo, err) } return result, fmt.Errorf( - "failed to fetch jwks from remote for tenant credentials %+v: (%s)", clientInfo, resp) + "failed to fetch jwks from remote for client credentials %+v: (%s)", clientInfo, resp) } ks.acceptedClients[clientInfo] = true jwks, err := jwk.ParseReader(resp.Body) diff --git a/oidcclient/jwk_test.go b/oidcclient/jwk_test.go index 1e8b6a9..0cf2041 100644 --- a/oidcclient/jwk_test.go +++ b/oidcclient/jwk_test.go @@ -66,7 +66,7 @@ func TestProviderJSON_assertMandatoryFieldsPresent(t *testing.T) { func TestOIDCTenant_ReadJWKs(t *testing.T) { type fields struct { Duration time.Duration - Tenant ClientInfo + Client ClientInfo ExpectedErrorMsg string } tests := []struct { @@ -76,19 +76,19 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { wantProviderJSON bool }{ { - name: "read from cache with accepted tenant credentials", + name: "read from cache with accepted client credentials", fields: fields{ Duration: 2 * time.Second, - Tenant: ClientInfo{"client-id", "app-tid", "azp"}, + Client: ClientInfo{"client-id", "app-tid", "azp"}, }, wantErr: false, wantProviderJSON: false, }, { - name: "read from cache with invalid tenant credentials", + name: "read from cache with invalid client credentials", fields: fields{ Duration: 2 * time.Second, - Tenant: ClientInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, - ExpectedErrorMsg: "tenant credentials: {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp} " + + Client: ClientInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, + ExpectedErrorMsg: "client credentials: {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp} " + "are not accepted by the identity service", }, wantErr: true, @@ -97,8 +97,8 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read token endpoint with invalid client_id", fields: fields{ Duration: 2 * time.Second, - Tenant: ClientInfo{"invalid-client-id", "app-tid", "azp"}, - ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + + Client: ClientInfo{"invalid-client-id", "app-tid", "azp"}, + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for client credentials " + "{ClientID:invalid-client-id AppTID:app-tid Azp:azp}: ({\"msg\":\"Invalid x-client_id or x-app_tid provided\"})", }, wantErr: true, @@ -107,8 +107,8 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read token endpoint with invalid app_tid", fields: fields{ Duration: 2 * time.Second, - Tenant: ClientInfo{"client-id", "invalid-app-tid", "azp"}, - ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + + Client: ClientInfo{"client-id", "invalid-app-tid", "azp"}, + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for client credentials " + "{ClientID:client-id AppTID:invalid-app-tid Azp:azp}: ({\"msg\":\"Invalid x-client_id or x-app_tid provided\"})", }, wantErr: true, @@ -117,46 +117,46 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { name: "read token endpoint with invalid azp", fields: fields{ Duration: 2 * time.Second, - Tenant: ClientInfo{"client-id", "app-tid", "invalid-azp"}, - ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for tenant credentials " + + Client: ClientInfo{"client-id", "app-tid", "invalid-azp"}, + ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for client credentials " + "{ClientID:client-id AppTID:app-tid Azp:invalid-azp}: ({\"msg\":\"Invalid x-azp provided\"})", }, wantErr: true, wantProviderJSON: true, }, { - name: "read from token keys endpoint with accepted tenant credentials", + name: "read from token keys endpoint with accepted client credentials", fields: fields{ Duration: 0, - Tenant: ClientInfo{"client-id", "app-tid", "azp"}, + Client: ClientInfo{"client-id", "app-tid", "azp"}, }, wantErr: false, wantProviderJSON: true, }, { - name: "read from token keys endpoint with denied tenant credentials", + name: "read from token keys endpoint with denied client credentials", fields: fields{ Duration: 0, - Tenant: ClientInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, + Client: ClientInfo{"invalid-client-id", "invalid-app-tid", "invalid-azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote " + - "for tenant credentials {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp}", + "for client credentials {ClientID:invalid-client-id AppTID:invalid-app-tid Azp:invalid-azp}", }, wantErr: true, wantProviderJSON: true, }, { - name: "read from token keys endpoint with accepted tenant credentials provoking parsing error", + name: "read from token keys endpoint with accepted client credentials provoking parsing error", fields: fields{ Duration: 0, - Tenant: ClientInfo{ClientID: "provide-invalidJWKS"}, + Client: ClientInfo{ClientID: "provide-invalidJWKS"}, ExpectedErrorMsg: "error updating JWKs: failed to parse JWK set: failed to unmarshal JWK set", }, wantErr: true, // as jwks endpoint returns no JSON wantProviderJSON: true, }, { - name: "read from token keys endpoint with deleted tenant credentials", + name: "read from token keys endpoint with deleted client credentials", fields: fields{ Duration: 0, - Tenant: ClientInfo{"deleted-client-id", "deleted-app-tid", "deleted-azp"}, + Client: ClientInfo{"deleted-client-id", "deleted-app-tid", "deleted-azp"}, ExpectedErrorMsg: "error updating JWKs: failed to fetch jwks from remote for " + - "tenant credentials {ClientID:deleted-client-id AppTID:deleted-app-tid Azp:deleted-azp}", + "client credentials {ClientID:deleted-client-id AppTID:deleted-app-tid Azp:deleted-azp}", }, wantErr: true, wantProviderJSON: true, @@ -186,16 +186,16 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { jwks: jwksJSON, ProviderJSON: providerJSON, } - jwks, err := tenant.GetJWKs(tt.fields.Tenant) + jwks, err := tenant.GetJWKs(tt.fields.Client) if tt.wantErr { if err == nil { - t.Errorf("GetJWKs() does not provide error = %v, tenantCredentials %+v", err, tt.fields.Tenant) + t.Errorf("GetJWKs() does not provide error = %v, tenantCredentials %+v", err, tt.fields.Client) } if !strings.HasPrefix(err.Error(), tt.fields.ExpectedErrorMsg) { t.Errorf("GetJWKs() does not provide expected error message = %v", err.Error()) } } else if jwks == nil { - t.Errorf("GetJWKs() returns nil = %v, tenantCredentials %+v", err, tt.fields.Tenant) + t.Errorf("GetJWKs() returns nil = %v, tenantCredentials %+v", err, tt.fields.Client) } }) } @@ -204,9 +204,9 @@ func TestOIDCTenant_ReadJWKs(t *testing.T) { func NewRouter() (r *mux.Router) { r = mux.NewRouter() r.HandleFunc("/oauth2/certs", ReturnJWKS).Methods(http.MethodGet).Headers(clientIDHeader, "client-id", appTIDHeader, "app-tid", azpHeader, "azp") - r.HandleFunc("/oauth2/certs", ReturnInvalidTenant).Methods(http.MethodGet).Headers(clientIDHeader, "invalid-client-id") - r.HandleFunc("/oauth2/certs", ReturnInvalidTenant).Methods(http.MethodGet).Headers(appTIDHeader, "invalid-app-tid") - r.HandleFunc("/oauth2/certs", ReturnInvalidTenant).Methods(http.MethodGet).Headers(azpHeader, "invalid-azp") + r.HandleFunc("/oauth2/certs", ReturnInvalidClient).Methods(http.MethodGet).Headers(clientIDHeader, "invalid-client-id") + r.HandleFunc("/oauth2/certs", ReturnInvalidClient).Methods(http.MethodGet).Headers(appTIDHeader, "invalid-app-tid") + r.HandleFunc("/oauth2/certs", ReturnInvalidClient).Methods(http.MethodGet).Headers(azpHeader, "invalid-azp") r.HandleFunc("/oauth2/certs", ReturnInvalidHeaders).Methods(http.MethodGet).Headers(clientIDHeader, "deleted-client-id", appTIDHeader, "deleted-app-tid", azpHeader, "deleted-azp") r.HandleFunc("/oauth2/certs", ReturnInvalidJWKS).Methods(http.MethodGet).Headers(clientIDHeader, "provide-invalidJWKS") return r @@ -224,7 +224,7 @@ func ReturnInvalidHeaders(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(400) } -func ReturnInvalidTenant(w http.ResponseWriter, r *http.Request) { +func ReturnInvalidClient(w http.ResponseWriter, r *http.Request) { w.WriteHeader(400) w.Header().Set("Content-Type", "application/json") if r.Header.Get(azpHeader) == "invalid-azp" {