Convert service account add/update expiration to cond values (#19024)

In order to force some users allowed to create or update a service
account to provide an expiration satifying the user policy conditions.

cmd/admin-handlers-users.go

@@ -27,6 +27,7 @@ import (
 	"net/http"
 	"os"
 	"sort"
+	"strconv"
 	"time"
 
 	"github.com/klauspost/compress/zip"
@@ -774,28 +775,6 @@ func (a adminAPIHandlers) UpdateServiceAccount(w http.ResponseWriter, r *http.Re
 		return
 	}
 
-	// Permission checks:
-	//
-	// 1. Any type of account (i.e. access keys (previously/still called service
-	// accounts), STS accounts, internal IDP accounts, etc) with the
-	// policy.UpdateServiceAccountAdminAction permission can update any service
-	// account.
-	//
-	// 2. We would like to let a user update their own access keys, however it
-	// is currently blocked pending a re-design. Users are still able to delete
-	// and re-create them.
-	if !globalIAMSys.IsAllowed(policy.Args{
-		AccountName:     cred.AccessKey,
-		Groups:          cred.Groups,
-		Action:          policy.UpdateServiceAccountAdminAction,
-		ConditionValues: getConditionValues(r, "", cred),
-		IsOwner:         owner,
-		Claims:          cred.Claims,
-	}) {
-		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
-		return
-	}
-
 	password := cred.SecretKey
 	reqBytes, err := madmin.DecryptData(password, io.LimitReader(r.Body, r.ContentLength))
 	if err != nil {
@@ -816,6 +795,31 @@ func (a adminAPIHandlers) UpdateServiceAccount(w http.ResponseWriter, r *http.Re
 		return
 	}
 
+	condValues := getConditionValues(r, "", cred)
+	addExpirationToCondValues(updateReq.NewExpiration, condValues)
+
+	// Permission checks:
+	//
+	// 1. Any type of account (i.e. access keys (previously/still called service
+	// accounts), STS accounts, internal IDP accounts, etc) with the
+	// policy.UpdateServiceAccountAdminAction permission can update any service
+	// account.
+	//
+	// 2. We would like to let a user update their own access keys, however it
+	// is currently blocked pending a re-design. Users are still able to delete
+	// and re-create them.
+	if !globalIAMSys.IsAllowed(policy.Args{
+		AccountName:     cred.AccessKey,
+		Groups:          cred.Groups,
+		Action:          policy.UpdateServiceAccountAdminAction,
+		ConditionValues: condValues,
+		IsOwner:         owner,
+		Claims:          cred.Claims,
+	}) {
+		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
+		return
+	}
+
 	var sp *policy.Policy
 	if len(updateReq.NewPolicy) > 0 {
 		sp, err = policy.ParseConfig(bytes.NewReader(updateReq.NewPolicy))
@@ -2417,6 +2421,13 @@ func (a adminAPIHandlers) ImportIAM(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+func addExpirationToCondValues(exp *time.Time, condValues map[string][]string) {
+	if exp == nil {
+		return
+	}
+	condValues["DurationSeconds"] = []string{strconv.FormatInt(int64(exp.Sub(time.Now()).Seconds()), 10)}
+}
+
 func commonAddServiceAccount(r *http.Request) (context.Context, auth.Credentials, newServiceAccountOpts, madmin.AddServiceAccountReq, string, APIError) {
 	ctx := r.Context()
 
@@ -2472,13 +2483,16 @@ func commonAddServiceAccount(r *http.Request) (context.Context, auth.Credentials
 		claims:      make(map[string]interface{}),
 	}
 
+	condValues := getConditionValues(r, "", cred)
+	addExpirationToCondValues(createReq.Expiration, condValues)
+
 	// Check if action is allowed if creating access key for another user
 	// Check if action is explicitly denied if for self
 	if !globalIAMSys.IsAllowed(policy.Args{
 		AccountName:     cred.AccessKey,
 		Groups:          cred.Groups,
 		Action:          policy.CreateServiceAccountAdminAction,
-		ConditionValues: getConditionValues(r, "", cred),
+		ConditionValues: condValues,
 		IsOwner:         owner,
 		Claims:          cred.Claims,
 		DenyOnly:        (targetUser == cred.AccessKey || targetUser == cred.ParentUser),

cmd/admin-handlers-users_test.go

@@ -208,6 +208,7 @@ func TestIAMInternalIDPServerSuite(t *testing.T) {
 				suite.TestServiceAccountOpsByAdmin(c)
 				suite.TestServiceAccountPrivilegeEscalationBug(c)
 				suite.TestServiceAccountOpsByUser(c)
+				suite.TestServiceAccountDurationSecondsCondition(c)
 				suite.TestAddServiceAccountPerms(c)
 				suite.TearDownSuite(c)
 			},
@@ -904,6 +905,93 @@ func (s *TestSuiteIAM) TestServiceAccountOpsByUser(c *check) {
 	c.mustNotCreateSvcAccount(ctx, globalActiveCred.AccessKey, userAdmClient)
 }
 
+func (s *TestSuiteIAM) TestServiceAccountDurationSecondsCondition(c *check) {
+	ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout)
+	defer cancel()
+
+	bucket := getRandomBucketName()
+	err := s.client.MakeBucket(ctx, bucket, minio.MakeBucketOptions{})
+	if err != nil {
+		c.Fatalf("bucket creat error: %v", err)
+	}
+
+	// Create policy, user and associate policy
+	policy := "mypolicy"
+	policyBytes := []byte(fmt.Sprintf(`{
+ "Version": "2012-10-17",
+ "Statement": [
+  {
+   "Effect": "Deny",
+   "Action": [
+     "admin:CreateServiceAccount",
+     "admin:UpdateServiceAccount"
+   ],
+   "Condition": {"NumericGreaterThan": {"svc:DurationSeconds": "3600"}}
+  },
+  {
+   "Effect": "Allow",
+   "Action": [
+    "s3:PutObject",
+    "s3:GetObject",
+    "s3:ListBucket"
+   ],
+   "Resource": [
+    "arn:aws:s3:::%s/*"
+   ]
+  }
+ ]
+}`, bucket))
+	err = s.adm.AddCannedPolicy(ctx, policy, policyBytes)
+	if err != nil {
+		c.Fatalf("policy add error: %v", err)
+	}
+
+	accessKey, secretKey := mustGenerateCredentials(c)
+	err = s.adm.SetUser(ctx, accessKey, secretKey, madmin.AccountEnabled)
+	if err != nil {
+		c.Fatalf("Unable to set user: %v", err)
+	}
+
+	err = s.adm.SetPolicy(ctx, policy, accessKey, false)
+	if err != nil {
+		c.Fatalf("Unable to set policy: %v", err)
+	}
+
+	// Create an madmin client with user creds
+	userAdmClient, err := madmin.NewWithOptions(s.endpoint, &madmin.Options{
+		Creds:  credentials.NewStaticV4(accessKey, secretKey, ""),
+		Secure: s.secure,
+	})
+	if err != nil {
+		c.Fatalf("Err creating user admin client: %v", err)
+	}
+	userAdmClient.SetCustomTransport(s.TestSuiteCommon.client.Transport)
+
+	distantExpiration := time.Now().Add(30 * time.Minute)
+	cr, err := userAdmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{
+		TargetUser: accessKey,
+		AccessKey:  "svc-accesskey",
+		SecretKey:  "svc-secretkey",
+		Expiration: &distantExpiration,
+	})
+	if err != nil {
+		c.Fatalf("Unable to create svc acc: %v", err)
+	}
+
+	c.assertSvcAccS3Access(ctx, s, cr, bucket)
+
+	closeExpiration := time.Now().Add(2 * time.Hour)
+	_, err = userAdmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{
+		TargetUser: accessKey,
+		AccessKey:  "svc-accesskey",
+		SecretKey:  "svc-secretkey",
+		Expiration: &closeExpiration,
+	})
+	if err == nil {
+		c.Fatalf("Creating a svc acc with distant expiration should fail")
+	}
+}
+
 func (s *TestSuiteIAM) TestServiceAccountOpsByAdmin(c *check) {
 	ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout)
 	defer cancel()