From 369a876ebe357d9cd3b50bc8baf4a117fe606f0d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 14 Jul 2020 10:26:47 -0700 Subject: [PATCH] fix: handle array policies in JWT claim (#10041) PR #10014 was not complete as only handled policy claims partially. --- cmd/auth-handler.go | 7 +++-- docs/sts/keycloak.md | 28 +++++++++---------- pkg/bucket/policy/condition/jwt.go | 4 +++ pkg/iam/policy/policy.go | 33 +++++++++++++++------- pkg/iam/policy/policy_test.go | 45 ++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 27 deletions(-) diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index 8b133e427..d28c1097a 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -213,15 +213,15 @@ func getClaimsFromToken(r *http.Request, token string) (map[string]interface{}, if globalPolicyOPA == nil { // If OPA is not set and if ldap claim key is set, allow the claim. - if _, ok := claims.Lookup(ldapUser); ok { + if _, ok := claims.MapClaims[ldapUser]; ok { return claims.Map(), nil } // If OPA is not set, session token should // have a policy and its mandatory, reject // requests without policy claim. - _, pokOpenID := claims.Lookup(iamPolicyClaimNameOpenID()) - _, pokSA := claims.Lookup(iamPolicyClaimNameSA()) + _, pokOpenID := claims.MapClaims[iamPolicyClaimNameOpenID()] + _, pokSA := claims.MapClaims[iamPolicyClaimNameSA()] if !pokOpenID && !pokSA { return nil, errAuthentication } @@ -360,6 +360,7 @@ func checkRequestAuthTypeToAccessKey(ctx context.Context, r *http.Request, actio // Request is allowed return the appropriate access key. return cred.AccessKey, owner, ErrNone } + return cred.AccessKey, owner, ErrAccessDenied } diff --git a/docs/sts/keycloak.md b/docs/sts/keycloak.md index ff03cbf74..52fd6e534 100644 --- a/docs/sts/keycloak.md +++ b/docs/sts/keycloak.md @@ -8,31 +8,31 @@ Configure and install keycloak server by following [Keycloak Installation Guide] ### Configure Keycloak UI - Go to Clients - -> Click on account - -> Settings - -> Enable `Implicit Flow` - -> Save + - Click on account + - Settings + - Enable `Implicit Flow` + - Save - Go to Users - -> Click on the user - -> Attribute, add a new attribute `Key` is `policy`, `Value` is name of the `policy` on MinIO (ex: `readwrite`) - -> Add and Save + - Click on the user + - Attribute, add a new attribute `Key` is `policy`, `Value` is name of the `policy` on MinIO (ex: `readwrite`) + - Add and Save - Go to Clients - -> Click on `account` - -> Settings, set `Valid Redirect URIs` to `*`, expand `Advanced Settings` and set `Access Token Lifespan` to `1 Hours` - -> Save + - Click on `account` + - Settings, set `Valid Redirect URIs` to `*`, expand `Advanced Settings` and set `Access Token Lifespan` to `1 Hours` + - Save - Go to Clients - -> Client on `account` - -> Mappers - -> Create + - Client on `account` + - Mappers + - Create - `Name` with any text - `Mapper Type` is `User Attribute` - `User Attribute` is `policy` - `Token Claim Name` is `policy` - `Claim JSON Type` is `string` - -> Save + - Save - Open http://localhost:8080/auth/realms/demo/.well-known/openid-configuration to verify OpenID discovery document, verify it has `authorization_endpoint` and `jwks_uri` diff --git a/pkg/bucket/policy/condition/jwt.go b/pkg/bucket/policy/condition/jwt.go index 422fff550..360a24c81 100644 --- a/pkg/bucket/policy/condition/jwt.go +++ b/pkg/bucket/policy/condition/jwt.go @@ -31,7 +31,9 @@ const ( // JWTJti JWT unique identifier claim substitution. JWTJti Key = "jwt:jti" + JWTUpn Key = "jwt:upn" JWTName Key = "jwt:name" + JWTGroups Key = "jwt:groups" JWTGivenName Key = "jwt:given_name" JWTFamilyName Key = "jwt:family_name" JWTMiddleName Key = "jwt:middle_name" @@ -57,6 +59,8 @@ var JWTKeys = []Key{ JWTAud, JWTJti, JWTName, + JWTUpn, + JWTGroups, JWTGivenName, JWTFamilyName, JWTMiddleName, diff --git a/pkg/iam/policy/policy.go b/pkg/iam/policy/policy.go index fb0d7bcb1..84c3f3390 100644 --- a/pkg/iam/policy/policy.go +++ b/pkg/iam/policy/policy.go @@ -47,23 +47,36 @@ func GetPoliciesFromClaims(claims map[string]interface{}, policyClaimName string if !ok { return s, false } - pnames, ok := pname.([]string) + pnames, ok := pname.([]interface{}) if !ok { pnameStr, ok := pname.(string) if ok { - pnames = strings.Split(pnameStr, ",") - } else { - return s, false + for _, pname := range strings.Split(pnameStr, ",") { + pname = strings.TrimSpace(pname) + if pname == "" { + // ignore any empty strings, considerate + // towards some user errors. + continue + } + s.Add(pname) + } + return s, true } + return s, false } for _, pname := range pnames { - pname = strings.TrimSpace(pname) - if pname == "" { - // ignore any empty strings, considerate - // towards some user errors. - continue + pnameStr, ok := pname.(string) + if ok { + for _, pnameStr := range strings.Split(pnameStr, ",") { + pnameStr = strings.TrimSpace(pnameStr) + if pnameStr == "" { + // ignore any empty strings, considerate + // towards some user errors. + continue + } + s.Add(pnameStr) + } } - s.Add(pname) } return s, true } diff --git a/pkg/iam/policy/policy_test.go b/pkg/iam/policy/policy_test.go index b082f0972..1b814c1e2 100644 --- a/pkg/iam/policy/policy_test.go +++ b/pkg/iam/policy/policy_test.go @@ -22,10 +22,55 @@ import ( "reflect" "testing" + "github.com/minio/minio-go/v6/pkg/set" "github.com/minio/minio/pkg/bucket/policy" "github.com/minio/minio/pkg/bucket/policy/condition" ) +func TestGetPoliciesFromClaims(t *testing.T) { + attributesArray := `{ + "exp": 1594690452, + "iat": 1594689552, + "auth_time": 1594689552, + "jti": "18ed05c9-2c69-45d5-a33f-8c94aca99ad5", + "iss": "http://localhost:8080/auth/realms/minio", + "aud": "account", + "sub": "7e5e2f30-1c97-4616-8623-2eae14dee9b1", + "typ": "ID", + "azp": "account", + "nonce": "66ZoLzwJbjdkiedI", + "session_state": "3df7b526-5310-4038-9f35-50ecd295a31d", + "acr": "1", + "upn": "harsha", + "address": {}, + "email_verified": false, + "groups": [ + "offline_access" + ], + "preferred_username": "harsha", + "policy": [ + "readwrite", + "readwrite,readonly", + " readonly", + "" + ]}` + var m = make(map[string]interface{}) + if err := json.Unmarshal([]byte(attributesArray), &m); err != nil { + t.Fatal(err) + } + var expectedSet = set.CreateStringSet("readwrite", "readonly") + gotSet, ok := GetPoliciesFromClaims(m, "policy") + if !ok { + t.Fatal("no policy claim was found") + } + if gotSet.IsEmpty() { + t.Fatal("no policies were found in policy claim") + } + if !gotSet.Equals(expectedSet) { + t.Fatalf("Expected %v got %v", expectedSet, gotSet) + } +} + func TestPolicyIsAllowed(t *testing.T) { case1Policy := Policy{ Version: DefaultVersion,