From 46ccc7b074e16197ad553afcebab1a0f316b4bc3 Mon Sep 17 00:00:00 2001 From: Yury Kovalev Date: Wed, 15 May 2024 15:41:23 +0200 Subject: [PATCH] Support the audience as list and add more test cases --- .../dinosaur/pkg/environments/integration.go | 1 - pkg/auth/acs_claims.go | 11 +++-- pkg/auth/fleetshard_authz_middleware.go | 42 ++++++++++++++++--- pkg/auth/fleetshard_authz_middleware_test.go | 36 +++++++++++++++- 4 files changed, 80 insertions(+), 10 deletions(-) diff --git a/internal/dinosaur/pkg/environments/integration.go b/internal/dinosaur/pkg/environments/integration.go index ca3b9f20f5..530ac8ac17 100644 --- a/internal/dinosaur/pkg/environments/integration.go +++ b/internal/dinosaur/pkg/environments/integration.go @@ -43,7 +43,6 @@ func (b IntegrationEnvLoader) Defaults() map[string]string { "quota-type": "quota-management-list", "enable-deletion-of-expired-central": "true", "dataplane-cluster-scaling-type": "auto", // need to set this to 'auto' for integration environment as some tests rely on this - "fleetshard-authz-config-file": "config/fleetshard-authz-development.yaml", } } diff --git a/pkg/auth/acs_claims.go b/pkg/auth/acs_claims.go index a708ab11b2..be61a781f9 100644 --- a/pkg/auth/acs_claims.go +++ b/pkg/auth/acs_claims.go @@ -16,6 +16,11 @@ func (c *ACSClaims) VerifyIssuer(cmp string, req bool) bool { return jwt.MapClaims(*c).VerifyIssuer(cmp, req) } +// VerifyAudience wraps jwt.VerifyAudience +func (c *ACSClaims) VerifyAudience(cmp string, req bool) bool { + return jwt.MapClaims(*c).VerifyAudience(cmp, req) +} + // GetUsername ... func (c *ACSClaims) GetUsername() (string, error) { if idx, val := arrays.FindFirst(func(x interface{}) bool { return x != nil }, @@ -80,9 +85,9 @@ func (c *ACSClaims) GetSubject() (string, error) { } // GetAudience returns the audience claim of the token. It identifies the token consumer. -func (c *ACSClaims) GetAudience() (string, error) { - if sub, ok := (*c)[audienceClaim].(string); ok { - return sub, nil +func (c *ACSClaims) GetAudience() (interface{}, error) { + if aud, ok := (*c)[audienceClaim]; ok { + return aud, nil } return "", fmt.Errorf("can't find %q attribute in claims", audienceClaim) diff --git a/pkg/auth/fleetshard_authz_middleware.go b/pkg/auth/fleetshard_authz_middleware.go index 1cc1412aef..2c08a38e60 100644 --- a/pkg/auth/fleetshard_authz_middleware.go +++ b/pkg/auth/fleetshard_authz_middleware.go @@ -45,15 +45,47 @@ func fleetShardAuthorizationMiddleware(iamConfig *iam.IAMConfig, fleetShardAuthZ } func checkAllowedOrgIDs(allowedOrgIDs []string) mux.MiddlewareFunc { - return checkClaim(alternateTenantIDClaim, (*ACSClaims).GetOrgID, allowedOrgIDs) + return checkClaim(tenantIDClaim, (*ACSClaims).GetOrgID, allowedOrgIDs) } -func checkSubject(subjects []string) mux.MiddlewareFunc { - return checkClaim(tenantSubClaim, (*ACSClaims).GetSubject, subjects) +func checkSubject(allowedSubjects []string) mux.MiddlewareFunc { + return checkClaim(tenantSubClaim, (*ACSClaims).GetSubject, allowedSubjects) } -func checkAudience(audiences []string) mux.MiddlewareFunc { - return checkClaim(audienceClaim, (*ACSClaims).GetAudience, audiences) +func checkAudience(allowedAudiences []string) mux.MiddlewareFunc { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + ctx := request.Context() + claims, err := GetClaimsFromContext(ctx) + if err != nil { + // Deliberately return 404 here so that it will appear as the endpoint doesn't exist if requests are + // not authorised. Otherwise, we would leak information about existing cluster IDs, since the path + // of the request is /agent-clusters/. + shared.HandleError(request, writer, errors.NotFound("")) + return + } + + audienceAccepted := false + for _, audience := range allowedAudiences { + if claims.VerifyAudience(audience, true) { + audienceAccepted = true + break + } + } + + if !audienceAccepted { + audience, _ := claims.GetAudience() + glog.Infof("none of the audiences (%q) in the access token is not in the list of allowed values [%s]", + audience, strings.Join(allowedAudiences, ",")) + + shared.HandleError(request, writer, errors.NotFound("")) + return + } + + next.ServeHTTP(writer, request) + + }) + } } type claimsGetter func(*ACSClaims) (string, error) diff --git a/pkg/auth/fleetshard_authz_middleware_test.go b/pkg/auth/fleetshard_authz_middleware_test.go index dbc36f2f7b..9a42709392 100644 --- a/pkg/auth/fleetshard_authz_middleware_test.go +++ b/pkg/auth/fleetshard_authz_middleware_test.go @@ -126,6 +126,7 @@ func TestUseFleetShardAuthorizationMiddleware_NoTokenSet(t *testing.T) { func TestUseFleetShardAuthorizationMiddleware_DataPlaneOIDCIssuers(t *testing.T) { const validIssuer = "http://localhost" + validAudience := []string{"acs-fleet-manager-private-api"} tests := map[string]struct { token *jwt.Token @@ -136,6 +137,7 @@ func TestUseFleetShardAuthorizationMiddleware_DataPlaneOIDCIssuers(t *testing.T) Claims: jwt.MapClaims{ "iss": validIssuer, "sub": "fleetshard-sync", + "aud": validAudience, }, }, expectedStatusCode: http.StatusOK, @@ -145,6 +147,7 @@ func TestUseFleetShardAuthorizationMiddleware_DataPlaneOIDCIssuers(t *testing.T) Claims: jwt.MapClaims{ "iss": validIssuer, "sub": "third-party-service", + "aud": "acs-fleet-manager-private-api", }, }, expectedStatusCode: http.StatusNotFound, @@ -168,10 +171,40 @@ func TestUseFleetShardAuthorizationMiddleware_DataPlaneOIDCIssuers(t *testing.T) token: &jwt.Token{ Claims: jwt.MapClaims{ "iss": validIssuer, + "aud": validAudience, }, }, expectedStatusCode: http.StatusNotFound, }, + "should fail when audience is not set": { + token: &jwt.Token{ + Claims: jwt.MapClaims{ + "iss": validIssuer, + "sub": "fleetshard-sync", + }, + }, + expectedStatusCode: http.StatusNotFound, + }, + "should fail when audience is not allowed": { + token: &jwt.Token{ + Claims: jwt.MapClaims{ + "iss": validIssuer, + "sub": "fleetshard-sync", + "aud": []string{"https://kubernetes.default.svc"}, + }, + }, + expectedStatusCode: http.StatusNotFound, + }, + "should succeed when at least one audience is allowed": { + token: &jwt.Token{ + Claims: jwt.MapClaims{ + "iss": validIssuer, + "sub": "fleetshard-sync", + "aud": []string{"other-api", "acs-fleet-manager-private-api"}, + }, + }, + expectedStatusCode: http.StatusOK, + }, } for name, tt := range tests { @@ -192,7 +225,8 @@ func TestUseFleetShardAuthorizationMiddleware_DataPlaneOIDCIssuers(t *testing.T) DataPlaneOIDCIssuers: &iam.OIDCIssuers{URIs: []string{validIssuer}}, }, &FleetShardAuthZConfig{ - AllowedSubjects: []string{"fleetshard-sync"}, + AllowedSubjects: []string{"fleetshard-sync"}, + AllowedAudiences: validAudience, }) req := httptest.NewRequest("GET", "http://example.com/agent-clusters/1234", nil)