Added iam import tests for openid (#20432)

Tests if imported service accounts have 
required access to buckets and objects.

Signed-off-by: Shubhendu Ram Tripathi <shubhendu@minio.io>

Co-authored-by: Harshavardhana <harsha@minio.io>

.github/workflows/iam-integrations.yaml

@@ -142,3 +142,20 @@ jobs:
       - name: Test import of IAM artifacts when in fresh cluster there are missing groups etc
         run: |
           make test-iam-import-with-missing-entities
+  iam-import-with-openid:
+    name: Test IAM import in new cluster with opendid configurations
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: ${{ matrix.go-version }}
+          check-latest: true
+      - name: Checkout minio-iam-testing
+        uses: actions/checkout@v4
+        with:
+          repository: minio/minio-iam-testing
+          path: minio-iam-testing
+      - name: Test import of IAM artifacts when in fresh cluster with openid configurations
+        run: |
+          make test-iam-import-with-openid

Makefile

@@ -101,6 +101,10 @@ test-iam-import-with-missing-entities: install-race ## test import of external i
 	@echo "Test IAM import configurations with missing entities"
 	@env bash $(PWD)/docs/distributed/iam-import-with-missing-entities.sh
 
+test-iam-import-with-openid: install-race
+	@echo "Test IAM import configurations with openid"
+	@env bash $(PWD)/docs/distributed/iam-import-with-openid.sh
+
 test-sio-error:
 	@(env bash $(PWD)/docs/bucket/replication/sio-error.sh)
 

cmd/auth-handler.go

@@ -222,7 +222,7 @@ func mustGetClaimsFromToken(r *http.Request) map[string]interface{} {
 	return claims
 }
 
-func getClaimsFromTokenWithSecret(token, secret string) (map[string]interface{}, error) {
+func getClaimsFromTokenWithSecret(token, secret string) (*xjwt.MapClaims, error) {
 	// JWT token for x-amz-security-token is signed with admin
 	// secret key, temporary credentials become invalid if
 	// server admin credentials change. This is done to ensure
@@ -244,7 +244,7 @@ func getClaimsFromTokenWithSecret(token, secret string) (map[string]interface{},
 
 	// If AuthZPlugin is set, return without any further checks.
 	if newGlobalAuthZPluginFn() != nil {
-		return claims.Map(), nil
+		return claims, nil
 	}
 
 	// Check if a session policy is set. If so, decode it here.
@@ -263,12 +263,16 @@ func getClaimsFromTokenWithSecret(token, secret string) (map[string]interface{},
 		claims.MapClaims[sessionPolicyNameExtracted] = string(spBytes)
 	}
 
-	return claims.Map(), nil
+	return claims, nil
 }
 
 // Fetch claims in the security token returned by the client.
 func getClaimsFromToken(token string) (map[string]interface{}, error) {
-	return getClaimsFromTokenWithSecret(token, globalActiveCred.SecretKey)
+	jwtClaims, err := getClaimsFromTokenWithSecret(token, globalActiveCred.SecretKey)
+	if err != nil {
+		return nil, err
+	}
+	return jwtClaims.Map(), nil
 }
 
 // Fetch claims in the security token returned by the client and validate the token.
@@ -319,7 +323,7 @@ func checkClaimsFromToken(r *http.Request, cred auth.Credentials) (map[string]in
 		if err != nil {
 			return nil, toAPIErrorCode(r.Context(), err)
 		}
-		return claims, ErrNone
+		return claims.Map(), ErrNone
 	}
 
 	claims := xjwt.NewMapClaims()

cmd/iam-store.go

@@ -2031,7 +2031,7 @@ func (store *IAMStoreSys) getParentUsers(cache *iamCache) map[string]ParentUserI
 
 		var (
 			err    error
-			claims map[string]interface{} = cred.Claims
+			claims *jwt.MapClaims
 		)
 
 		if cred.IsServiceAccount() {
@@ -2053,24 +2053,17 @@ func (store *IAMStoreSys) getParentUsers(cache *iamCache) map[string]ParentUserI
 		}
 
 		subClaimValue := cred.ParentUser
-		if v, ok := claims[subClaim]; ok {
-			subFromToken, ok := v.(string)
-			if ok {
-				subClaimValue = subFromToken
-			}
-		}
-		if v, ok := claims[ldapActualUser]; ok {
-			subFromToken, ok := v.(string)
-			if ok {
-				subClaimValue = subFromToken
+		if v, ok := claims.Lookup(subClaim); ok {
+			subClaimValue = v
 		}
+		if v, ok := claims.Lookup(ldapActualUser); ok {
+			subClaimValue = v
 		}
 
 		roleArn := openid.DummyRoleARN.String()
-		s, ok := claims[roleArnClaim]
-		val, ok2 := s.(string)
-		if ok && ok2 {
-			roleArn = val
+		s, ok := claims.Lookup(roleArnClaim)
+		if ok {
+			roleArn = s
 		}
 		v, ok := res[cred.ParentUser]
 		if ok {
@@ -2537,13 +2530,15 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st
 
 	// Extracted session policy name string can be removed as its not useful
 	// at this point.
-	delete(m, sessionPolicyNameExtracted)
+	m.Delete(sessionPolicyNameExtracted)
+
+	nosp := opts.sessionPolicy == nil || opts.sessionPolicy.Version == "" && len(opts.sessionPolicy.Statements) == 0
 
 	// sessionPolicy is nil and there is embedded policy attached we remove
 	// embedded policy at that point.
-	if _, ok := m[policy.SessionPolicyName]; ok && opts.sessionPolicy == nil {
-		delete(m, policy.SessionPolicyName)
-		m[iamPolicyClaimNameSA()] = inheritedPolicyType
+	if _, ok := m.Lookup(policy.SessionPolicyName); ok && nosp {
+		m.Delete(policy.SessionPolicyName)
+		m.Set(iamPolicyClaimNameSA(), inheritedPolicyType)
 	}
 
 	if opts.sessionPolicy != nil { // session policies is being updated
@@ -2551,6 +2546,7 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st
 			return updatedAt, err
 		}
 
+		if opts.sessionPolicy.Version != "" && len(opts.sessionPolicy.Statements) > 0 {
 			policyBuf, err := json.Marshal(opts.sessionPolicy)
 			if err != nil {
 				return updatedAt, err
@@ -2561,11 +2557,12 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st
 			}
 
 			// Overwrite session policy claims.
-		m[policy.SessionPolicyName] = base64.StdEncoding.EncodeToString(policyBuf)
-		m[iamPolicyClaimNameSA()] = embeddedPolicyType
+			m.Set(policy.SessionPolicyName, base64.StdEncoding.EncodeToString(policyBuf))
+			m.Set(iamPolicyClaimNameSA(), embeddedPolicyType)
+		}
 	}
 
-	cr.SessionToken, err = auth.JWTSignWithAccessKey(accessKey, m, cr.SecretKey)
+	cr.SessionToken, err = auth.JWTSignWithAccessKey(accessKey, m.Map(), cr.SecretKey)
 	if err != nil {
 		return updatedAt, err
 	}
@@ -2892,22 +2889,22 @@ func (store *IAMStoreSys) LoadUser(ctx context.Context, accessKey string) error
 
 func extractJWTClaims(u UserIdentity) (jwtClaims *jwt.MapClaims, err error) {
 	keys := make([]string, 0, 3)
+
 	// Append credentials secret key itself
 	keys = append(keys, u.Credentials.SecretKey)
+
 	// Use site-replication credentials if found
 	if globalSiteReplicationSys.isEnabled() {
-		siteReplSecretKey, e := globalSiteReplicatorCred.Get(GlobalContext)
-		if e != nil {
-			return nil, e
+		secretKey, err := getTokenSigningKey()
+		if err != nil {
+			return nil, err
 		}
-		keys = append(keys, siteReplSecretKey)
+		keys = append(keys, secretKey)
 	}
-	// Use root credentials for credentials created with older deployments
-	keys = append(keys, globalActiveCred.SecretKey)
 
 	// Iterate over all keys and return with the first successful claim extraction
 	for _, key := range keys {
-		jwtClaims, err = auth.ExtractClaims(u.Credentials.SessionToken, key)
+		jwtClaims, err = getClaimsFromTokenWithSecret(u.Credentials.SessionToken, key)
 		if err == nil {
 			break
 		}

cmd/iam.go

@@ -1294,10 +1294,6 @@ func (sys *IAMSys) GetClaimsForSvcAcc(ctx context.Context, accessKey string) (ma
 		return nil, errServerNotInitialized
 	}
 
-	if sys.usersSysType != LDAPUsersSysType {
-		return nil, nil
-	}
-
 	sa, ok := sys.store.GetUser(accessKey)
 	if !ok || !sa.Credentials.IsServiceAccount() {
 		return nil, errNoSuchServiceAccount
@@ -2179,7 +2175,6 @@ func (sys *IAMSys) IsAllowedServiceAccount(args policy.Args, parentUser string)
 			return false
 		}
 		svcPolicies = newMappedPolicy(sys.rolesMap[arn]).toSlice()
-
 	default:
 		// Check policy for parent user of service account.
 		svcPolicies, err = sys.PolicyDBGet(parentUser, args.Groups...)

cmd/sts-handlers_test.go

@@ -1997,7 +1997,7 @@ func (s *TestSuiteIAM) TestLDAPCyrillicUser(c *check) {
 		}
 
 		// Validate claims.
-		dnClaim := claims[ldapActualUser].(string)
+		dnClaim := claims.MapClaims[ldapActualUser].(string)
 		if dnClaim != testCase.dn {
 			c.Fatalf("Test %d: unexpected dn claim: %s", i+1, dnClaim)
 		}
@@ -2079,11 +2079,11 @@ func (s *TestSuiteIAM) TestLDAPAttributesLookup(c *check) {
 		}
 
 		// Validate claims. Check if the sshPublicKey claim is present.
-		dnClaim := claims[ldapActualUser].(string)
+		dnClaim := claims.MapClaims[ldapActualUser].(string)
 		if dnClaim != testCase.dn {
 			c.Fatalf("Test %d: unexpected dn claim: %s", i+1, dnClaim)
 		}
-		sshPublicKeyClaim := claims[ldapAttribPrefix+"sshPublicKey"].([]interface{})[0].(string)
+		sshPublicKeyClaim := claims.MapClaims[ldapAttribPrefix+"sshPublicKey"].([]interface{})[0].(string)
 		if sshPublicKeyClaim == "" {
 			c.Fatalf("Test %d: expected sshPublicKey claim to be present", i+1)
 		}

docs/distributed/iam-import-with-openid.sh

@@ -0,0 +1,82 @@
+#!/bin/bash
+
+if [ -n "$TEST_DEBUG" ]; then
+	set -x
+fi
+
+pkill minio
+docker rm -f $(docker ps -aq)
+rm -rf /tmp/openid{1..4}
+
+export MC_HOST_myminio="http://minioadmin:minioadmin@localhost:22000"
+# The service account used below is already present in iam configuration getting imported
+export MC_HOST_myminio1="http://dillon-service-2:dillon-service-2@localhost:22000"
+
+# Start MinIO instance
+export CI=true
+
+if [ ! -f ./mc ]; then
+	wget --quiet -O mc https://dl.minio.io/client/mc/release/linux-amd64/mc &&
+		chmod +x mc
+fi
+
+mc -v
+
+# Start openid server
+(
+	cd ./minio-iam-testing
+	make docker-images
+	make docker-run
+	cd -
+)
+
+(minio server --address :22000 --console-address :10000 http://localhost:22000/tmp/openid{1...4} 2>&1 >/tmp/server.log) &
+./mc ready myminio
+./mc mb myminio/test-bucket
+./mc cp /etc/hosts myminio/test-bucket
+
+./mc idp openid add myminio \
+	config_url="http://localhost:5556/dex/.well-known/openid-configuration" \
+	client_id="minio-client-app" \
+	client_secret="minio-client-app-secret" \
+	scopes="openid,groups,email,profile" \
+	redirect_uri="http://127.0.0.1:10000/oauth_callback" \
+	display_name="Login via dex1" \
+	role_policy="consoleAdmin"
+
+./mc admin service restart myminio --json
+./mc ready myminio
+./mc admin cluster iam import myminio docs/distributed/samples/myminio-iam-info-openid.zip
+
+# Verify if buckets / objects accessible using service account
+echo "Verifying buckets and objects access for the imported service account"
+
+./mc ls myminio1/ --json
+BKT_COUNT=$(./mc ls myminio1/ --json | jq '.key' | wc -l)
+if [ "${BKT_COUNT}" -ne 1 ]; then
+	echo "BUG: Expected no of bucket: 1, Found: ${BKT_COUNT}"
+	exit 1
+fi
+
+BKT_NAME=$(./mc ls myminio1/ --json | jq '.key' | sed 's/"//g' | sed 's\/\\g')
+if [[ ${BKT_NAME} != "test-bucket" ]]; then
+	echo "BUG: Expected bucket: test-bucket, Found: ${BKT_NAME}"
+	exit 1
+fi
+
+./mc ls myminio1/test-bucket
+OBJ_COUNT=$(./mc ls myminio1/test-bucket --json | jq '.key' | wc -l)
+if [ "${OBJ_COUNT}" -ne 1 ]; then
+	echo "BUG: Expected no of objects: 1, Found: ${OBJ_COUNT}"
+	exit 1
+fi
+
+OBJ_NAME=$(./mc ls myminio1/test-bucket --json | jq '.key' | sed 's/"//g')
+if [[ ${OBJ_NAME} != "hosts" ]]; then
+	echo "BUG: Expected object: hosts, Found: ${BKT_NAME}"
+	exit 1
+fi
+
+# Finally kill running processes
+pkill minio
+docker rm -f $(docker ps -aq)

internal/jwt/parser.go

@@ -245,6 +245,22 @@ func NewMapClaims() *MapClaims {
 	return &MapClaims{MapClaims: jwtgo.MapClaims{}}
 }
 
+// Set Adds new arbitrary claim keys and values.
+func (c *MapClaims) Set(key string, val interface{}) {
+	if c == nil {
+		return
+	}
+	c.MapClaims[key] = val
+}
+
+// Delete deletes a key named key.
+func (c *MapClaims) Delete(key string) {
+	if c == nil {
+		return
+	}
+	delete(c.MapClaims, key)
+}
+
 // Lookup returns the value and if the key is found.
 func (c *MapClaims) Lookup(key string) (value string, ok bool) {
 	if c == nil {