From 87cbd4126599ae825903230fdb32197204e42c22 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Tue, 29 Nov 2022 15:40:49 -0800 Subject: [PATCH] feat: Allow at most one claim based OpenID IDP (#16145) --- cmd/admin-handlers-users.go | 6 +++- cmd/sts-handlers.go | 14 ++++++--- cmd/sts-handlers_test.go | 36 +++++++++++++++++++++-- docs/sts/web-identity.md | 10 +++---- internal/config/identity/openid/openid.go | 16 ++-------- 5 files changed, 57 insertions(+), 25 deletions(-) diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index df6d2243e..263a2e7a7 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -1187,6 +1187,7 @@ func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Requ } roleArn := iampolicy.Args{Claims: claims}.GetRoleArn() + policySetFromClaims, hasPolicyClaim := iampolicy.GetPoliciesFromClaims(claims, iamPolicyClaimNameOpenID()) var effectivePolicy iampolicy.Policy var buf []byte @@ -1198,16 +1199,19 @@ func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Requ break } } + case roleArn != "": _, policy, err := globalIAMSys.GetRolePolicy(roleArn) if err != nil { - logger.LogIf(ctx, err) writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return } policySlice := newMappedPolicy(policy).toSlice() effectivePolicy = globalIAMSys.GetCombinedPolicy(policySlice...) + case hasPolicyClaim: + effectivePolicy = globalIAMSys.GetCombinedPolicy(policySetFromClaims.ToSlice()...) + default: policies, err := globalIAMSys.PolicyDBGet(accountName, false, cred.Groups...) if err != nil { diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index c9de8ab99..313e1b020 100644 --- a/cmd/sts-handlers.go +++ b/cmd/sts-handlers.go @@ -343,17 +343,23 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ accessToken := r.Form.Get(stsWebIdentityAccessToken) + // RoleARN parameter processing: If a role ARN is given in the request, we + // use that and validate the authentication request. If not, we assume this + // is an STS request for a claim based IDP (if one is present) and set + // roleArn = openid.DummyRoleARN. + // + // Currently, we do not support multiple claim based IDPs, as there is no + // defined parameter to disambiguate the intended IDP in this STS request. roleArn := openid.DummyRoleARN - if globalIAMSys.HasRolePolicy() { + roleArnStr := r.Form.Get(stsRoleArn) + if roleArnStr != "" { var err error - roleArnStr := r.Form.Get(stsRoleArn) roleArn, _, err = globalIAMSys.GetRolePolicy(roleArnStr) if err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, fmt.Errorf("Error processing %s parameter: %v", stsRoleArn, err)) return } - } // Validate JWT; check clientID in claims matches the one associated with the roleArn @@ -376,7 +382,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ } var policyName string - if globalIAMSys.HasRolePolicy() { + if roleArnStr != "" && globalIAMSys.HasRolePolicy() { // If roleArn is used, we set it as a claim, and use the // associated policy when credentials are used. claims[roleArnClaim] = roleArn.String() diff --git a/cmd/sts-handlers_test.go b/cmd/sts-handlers_test.go index 899edc2e6..8dfc6445f 100644 --- a/cmd/sts-handlers_test.go +++ b/cmd/sts-handlers_test.go @@ -1563,7 +1563,7 @@ func (s *TestSuiteIAM) TestOpenIDSTSWithRolePolicyWithPolVar(c *check, roleARN s c.mustNotListObjects(ctx, lisaClient, "other") } -func TestIAMWithOpenIDMultipleConfigsValidation(t *testing.T) { +func TestIAMWithOpenIDMultipleConfigsValidation1(t *testing.T) { openIDServer := os.Getenv(EnvTestOpenIDServer) openIDServer2 := os.Getenv(EnvTestOpenIDServer2) if openIDServer == "" || openIDServer2 == "" { @@ -1576,6 +1576,38 @@ func TestIAMWithOpenIDMultipleConfigsValidation(t *testing.T) { "readwrite", } + for i, testCase := range iamTestSuites { + t.Run( + fmt.Sprintf("Test: %d, ServerType: %s", i+1, testCase.ServerTypeDescription), + func(t *testing.T) { + c := &check{t, testCase.serverType} + suite := testCase + + suite.SetUpSuite(c) + defer suite.TearDownSuite(c) + + err := suite.SetUpOpenIDs(c, testApps, rolePolicies) + if err != nil { + c.Fatalf("config with 1 claim based and 1 role based provider should pass but got: %v", err) + } + }, + ) + } +} + +func TestIAMWithOpenIDMultipleConfigsValidation2(t *testing.T) { + openIDServer := os.Getenv(EnvTestOpenIDServer) + openIDServer2 := os.Getenv(EnvTestOpenIDServer2) + if openIDServer == "" || openIDServer2 == "" { + t.Skip("Skipping OpenID test as enough OpenID servers are not provided.") + } + testApps := testClientApps + + rolePolicies := []string{ + "", // Treated as claim-based provider as no role policy is given. + "", // Treated as claim-based provider as no role policy is given. + } + for i, testCase := range iamTestSuites { t.Run( fmt.Sprintf("Test: %d, ServerType: %s", i+1, testCase.ServerTypeDescription), @@ -1588,7 +1620,7 @@ func TestIAMWithOpenIDMultipleConfigsValidation(t *testing.T) { err := suite.SetUpOpenIDs(c, testApps, rolePolicies) if err == nil { - c.Fatal("config with both claim based and role policy based providers should fail") + c.Fatalf("config with 2 claim based provider should fail") } }, ) diff --git a/docs/sts/web-identity.md b/docs/sts/web-identity.md index f1ce91937..0e0ebfdce 100644 --- a/docs/sts/web-identity.md +++ b/docs/sts/web-identity.md @@ -37,7 +37,7 @@ MINIO_IDENTITY_OPENID_REDIRECT_URI (string) [DEPRECATED use env 'MIN Either `MINIO_IDENTITY_OPENID_ROLE_POLICY` (recommended) or `MINIO_IDENTITY_OPENID_CLAIM_NAME` must be specified but not both. See the section Access Control Policies to understand the differences between the two. -With role policies, it is possible to specify multiple OpenID provider configurations - this is useful to integrate multiple OpenID client applications to interact with object storage. +**NOTE**: When configuring multiple OpenID based authentication providers on a MinIO cluster, any number of Role Policy based providers may be configured, and at most one JWT Claim based provider may be configured.
Example 1: Two role policy providers @@ -80,13 +80,13 @@ MINIO_IDENTITY_OPENID_CLAIM_NAME="groups"
-## Access Control Policies +## Specifying Access Control with IAM Policies -MinIO's AssumeRoleWithWebIdentity supports specifying access control policies in two ways: +The STS API authenticates the user by verifying the JWT provided in the request. However access to object storage resources are controlled via named IAM policies defined in the MinIO instance. Once authenticated via the STS API, the MinIO server applies one or more IAM policies to the generated credentials. MinIO's AssumeRoleWithWebIdentity implementation supports specifying IAM policies in two ways: -1. Role Policy (Recommended): When specified, all users authenticating via this API are authorized to (only) use the specified role policy. The policy to associate with such users is specified when configuring OpenID provider in the server, via the `role_policy` configuration parameter or the `MINIO_IDENTITY_OPENID_ROLE_POLICY` environment variable. The value is a comma-separated list of IAM access policy names already defined in the server. In this situation, the server prints a role ARN at startup that must be specified as a `RoleARN` API request parameter in the STS AssumeRoleWithWebIdentity API call. When using Role Policies, multiple OpenID providers and/or client applications (with unique client IDs) may be configured with independent role policies. Each configuration is assigned a unique RoleARN by the MinIO server and this is used to select the policies to apply to temporary credentials generated in the AssumeRoleWithWebIdentity call. +1. Role Policy (Recommended): When specified as part of the OpenID provider configuration, all users authenticating via this provider are authorized to (only) use the specified role policy. The policy to associate with such users is specified via the `role_policy` configuration parameter or the `MINIO_IDENTITY_OPENID_ROLE_POLICY` environment variable. The value is a comma-separated list of IAM access policy names already defined in the server. In this situation, the server prints a role ARN at startup that must be specified as a `RoleARN` API request parameter in the STS AssumeRoleWithWebIdentity API call. When using Role Policies, multiple OpenID providers and/or client applications (with unique client IDs) may be configured with independent role policies. Each configuration is assigned a unique RoleARN by the MinIO server and this is used to select the policies to apply to temporary credentials generated in the AssumeRoleWithWebIdentity call. -2. `id_token` claims: When the role policy is not configured, MinIO looks for a specific claim in the `id_token` (JWT) returned by the OpenID provider. The default claim is `policy` and can be overridden by the `claim_name` configuration parameter or the `MINIO_IDENTITY_OPENID_CLAIM_NAME` environment variable. The claim value can be a string (comma-separated list) or an array of IAM access policy names defined in the server. A `RoleARN` API request parameter *must not* be specified in the STS AssumeRoleWithWebIdentity API call. +2. `id_token` claims: When the role policy is not configured, MinIO looks for a specific claim in the `id_token` (JWT) returned by the OpenID provider in the STS request. The default claim is `policy` and can be overridden by the `claim_name` configuration parameter or the `MINIO_IDENTITY_OPENID_CLAIM_NAME` environment variable. The claim value can be a string (comma-separated list) or an array of IAM access policy names defined in the server. A `RoleARN` API request parameter *must not* be specified in the STS AssumeRoleWithWebIdentity API call. ## API Request Parameters diff --git a/internal/config/identity/openid/openid.go b/internal/config/identity/openid/openid.go index c48c01c9a..1d24b7623 100644 --- a/internal/config/identity/openid/openid.go +++ b/internal/config/identity/openid/openid.go @@ -204,10 +204,7 @@ func LookupConfig(s config.Config, transport http.RoundTripper, closeRespFn func closeRespFn: closeRespFn, } - var ( - hasLegacyPolicyMapping = false - seenClientIDs = set.NewStringSet() - ) + seenClientIDs := set.NewStringSet() deprecatedKeys := []string{JwksURL} @@ -376,9 +373,8 @@ func LookupConfig(s config.Config, transport http.RoundTripper, closeRespFn func arnKey := p.roleArn if p.RolePolicy == "" { arnKey = DummyRoleARN - hasLegacyPolicyMapping = true - // Ensure that when a JWT policy claim based provider - // exists, it is the only one. + // Ensure that at most one JWT policy claim based provider may be + // defined. if _, ok := c.arnProviderCfgsMap[DummyRoleARN]; ok { return c, errSingleProvider } @@ -392,12 +388,6 @@ func LookupConfig(s config.Config, transport http.RoundTripper, closeRespFn func } } - // Ensure that when a JWT policy claim based provider - // exists, it is the only one. - if hasLegacyPolicyMapping && len(c.ProviderCfgs) > 1 { - return c, errSingleProvider - } - c.Enabled = true return c, nil