diff --git a/cmd/admin-handlers-users_test.go b/cmd/admin-handlers-users_test.go index 287833e45..b674c2811 100644 --- a/cmd/admin-handlers-users_test.go +++ b/cmd/admin-handlers-users_test.go @@ -740,144 +740,27 @@ func (s *TestSuiteIAM) TestServiceAccountOpsByUser(c *check) { userAdmClient.SetCustomTransport(s.TestSuiteCommon.client.Transport) // Create svc acc - cr, err := userAdmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ - TargetUser: accessKey, - }) - if err != nil { - c.Fatalf("Err creating svc acc: %v", err) - } + cr := c.mustCreateSvcAccount(ctx, accessKey, userAdmClient) // 1. Check that svc account appears in listing - listResp, err := userAdmClient.ListServiceAccounts(ctx, accessKey) - if err != nil { - c.Fatalf("unable to list svc accounts: %v", err) - } - if !set.CreateStringSet(listResp.Accounts...).Contains(cr.AccessKey) { - c.Fatalf("created service account did not appear in listing!") - } + c.assertSvcAccAppearsInListing(ctx, userAdmClient, accessKey, cr.AccessKey) // 2. Check that svc account info can be queried - infoResp, err := userAdmClient.InfoServiceAccount(ctx, cr.AccessKey) - if err != nil { - c.Fatalf("unable to get svc acc info: %v", err) - } - c.Assert(infoResp.ParentUser, accessKey) - c.Assert(infoResp.AccountStatus, "on") - c.Assert(infoResp.ImpliedPolicy, true) + c.assertSvcAccInfoQueryable(ctx, userAdmClient, accessKey, cr.AccessKey, false) + + // 3. Check S3 access + c.assertSvcAccS3Access(ctx, s, cr, bucket) // 4. Check that svc account can restrict the policy, and that the // session policy can be updated. - { - svcAK, svcSK := mustGenerateCredentials(c) - policyBytes := []byte(fmt.Sprintf(`{ - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:PutObject", - "s3:GetObject" - ], - "Resource": [ - "arn:aws:s3:::%s/*" - ] - } - ] -}`, bucket)) - cr, err := userAdmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ - Policy: policyBytes, - TargetUser: accessKey, - AccessKey: svcAK, - SecretKey: svcSK, - }) - if err != nil { - c.Fatalf("Unable to create svc acc: %v", err) - } - svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") - c.mustNotListObjects(ctx, svcClient, bucket) - - newPolicyBytes := []byte(fmt.Sprintf(`{ - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:ListBucket" - ], - "Resource": [ - "arn:aws:s3:::%s/*" - ] - } - ] -}`, bucket)) - err = userAdmClient.UpdateServiceAccount(ctx, svcAK, madmin.UpdateServiceAccountReq{ - NewPolicy: newPolicyBytes, - }) - if err != nil { - c.Fatalf("unable to update session policy for svc acc: %v", err) - } - c.mustListObjects(ctx, svcClient, bucket) - } + c.assertSvcAccSessionPolicyUpdate(ctx, s, userAdmClient, accessKey, bucket) // 4. Check that service account's secret key and account status can be // updated. - { - svcAK, svcSK := mustGenerateCredentials(c) - cr, err := userAdmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ - TargetUser: accessKey, - AccessKey: svcAK, - SecretKey: svcSK, - }) - if err != nil { - c.Fatalf("Unable to create svc acc: %v", err) - } - svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") - c.mustListObjects(ctx, svcClient, bucket) - - _, svcSK2 := mustGenerateCredentials(c) - err = userAdmClient.UpdateServiceAccount(ctx, svcAK, madmin.UpdateServiceAccountReq{ - NewSecretKey: svcSK2, - }) - if err != nil { - c.Fatalf("unable to update secret key for svc acc: %v", err) - } - // old creds should not work: - c.mustNotListObjects(ctx, svcClient, bucket) - // new creds work: - svcClient2 := s.getUserClient(c, cr.AccessKey, svcSK2, "") - c.mustListObjects(ctx, svcClient2, bucket) - - // update status to disabled - err = userAdmClient.UpdateServiceAccount(ctx, svcAK, madmin.UpdateServiceAccountReq{ - NewStatus: "off", - }) - if err != nil { - c.Fatalf("unable to update secret key for svc acc: %v", err) - } - c.mustNotListObjects(ctx, svcClient2, bucket) - } + c.assertSvcAccSecretKeyAndStatusUpdate(ctx, s, userAdmClient, accessKey, bucket) // 5. Check that service account can be deleted. - { - svcAK, svcSK := mustGenerateCredentials(c) - cr, err := userAdmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ - TargetUser: accessKey, - AccessKey: svcAK, - SecretKey: svcSK, - }) - if err != nil { - c.Fatalf("Unable to create svc acc: %v", err) - } - svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") - c.mustListObjects(ctx, svcClient, bucket) - - err = userAdmClient.DeleteServiceAccount(ctx, svcAK) - if err != nil { - c.Fatalf("unable to delete svc acc: %v", err) - } - c.mustNotListObjects(ctx, svcClient, bucket) - } - + c.assertSvcAccDeletion(ctx, s, userAdmClient, accessKey, bucket) } func (s *TestSuiteIAM) TestServiceAccountOpsByAdmin(c *check) { @@ -925,159 +808,37 @@ func (s *TestSuiteIAM) TestServiceAccountOpsByAdmin(c *check) { } // 1. Create a service account for the user - svcAK, svcSK := mustGenerateCredentials(c) - cr, err := s.adm.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ - TargetUser: accessKey, - AccessKey: svcAK, - SecretKey: svcSK, - }) - if err != nil { - c.Fatalf("Unable to create svc acc: %v", err) - } + cr := c.mustCreateSvcAccount(ctx, accessKey, s.adm) + // 1.2 Check that svc account appears in listing - listResp, err := s.adm.ListServiceAccounts(ctx, accessKey) - if err != nil { - c.Fatalf("unable to list svc accounts: %v", err) - } - if !set.CreateStringSet(listResp.Accounts...).Contains(svcAK) { - c.Fatalf("created service account did not appear in listing!") - } + c.assertSvcAccAppearsInListing(ctx, s.adm, accessKey, cr.AccessKey) + // 1.3 Check that svc account info can be queried - infoResp, err := s.adm.InfoServiceAccount(ctx, svcAK) - if err != nil { - c.Fatalf("unable to get svc acc info: %v", err) - } - c.Assert(infoResp.ParentUser, accessKey) - c.Assert(infoResp.AccountStatus, "on") - c.Assert(infoResp.ImpliedPolicy, true) + c.assertSvcAccInfoQueryable(ctx, s.adm, accessKey, cr.AccessKey, false) // 2. Check that svc account can access the bucket - { - svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") - c.mustListObjects(ctx, svcClient, bucket) - } + c.assertSvcAccS3Access(ctx, s, cr, bucket) // 3. Check that svc account can restrict the policy, and that the // session policy can be updated. - { - svcAK, svcSK := mustGenerateCredentials(c) - policyBytes := []byte(fmt.Sprintf(`{ - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:PutObject", - "s3:GetObject" - ], - "Resource": [ - "arn:aws:s3:::%s/*" - ] - } - ] -}`, bucket)) - cr, err := s.adm.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ - Policy: policyBytes, - TargetUser: accessKey, - AccessKey: svcAK, - SecretKey: svcSK, - }) - if err != nil { - c.Fatalf("Unable to create svc acc: %v", err) - } - svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") - c.mustNotListObjects(ctx, svcClient, bucket) - - newPolicyBytes := []byte(fmt.Sprintf(`{ - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "s3:ListBucket" - ], - "Resource": [ - "arn:aws:s3:::%s/*" - ] - } - ] -}`, bucket)) - err = s.adm.UpdateServiceAccount(ctx, svcAK, madmin.UpdateServiceAccountReq{ - NewPolicy: newPolicyBytes, - }) - if err != nil { - c.Fatalf("unable to update session policy for svc acc: %v", err) - } - c.mustListObjects(ctx, svcClient, bucket) - } + c.assertSvcAccSessionPolicyUpdate(ctx, s, s.adm, accessKey, bucket) // 4. Check that service account's secret key and account status can be // updated. - { - svcAK, svcSK := mustGenerateCredentials(c) - cr, err := s.adm.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ - TargetUser: accessKey, - AccessKey: svcAK, - SecretKey: svcSK, - }) - if err != nil { - c.Fatalf("Unable to create svc acc: %v", err) - } - svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") - c.mustListObjects(ctx, svcClient, bucket) - - _, svcSK2 := mustGenerateCredentials(c) - err = s.adm.UpdateServiceAccount(ctx, svcAK, madmin.UpdateServiceAccountReq{ - NewSecretKey: svcSK2, - }) - if err != nil { - c.Fatalf("unable to update secret key for svc acc: %v", err) - } - // old creds should not work: - c.mustNotListObjects(ctx, svcClient, bucket) - // new creds work: - svcClient2 := s.getUserClient(c, cr.AccessKey, svcSK2, "") - c.mustListObjects(ctx, svcClient2, bucket) - - // update status to disabled - err = s.adm.UpdateServiceAccount(ctx, svcAK, madmin.UpdateServiceAccountReq{ - NewStatus: "off", - }) - if err != nil { - c.Fatalf("unable to update secret key for svc acc: %v", err) - } - c.mustNotListObjects(ctx, svcClient2, bucket) - } + c.assertSvcAccSecretKeyAndStatusUpdate(ctx, s, s.adm, accessKey, bucket) // 5. Check that service account can be deleted. - { - svcAK, svcSK := mustGenerateCredentials(c) - cr, err := s.adm.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ - TargetUser: accessKey, - AccessKey: svcAK, - SecretKey: svcSK, - }) - if err != nil { - c.Fatalf("Unable to create svc acc: %v", err) - } - svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") - c.mustListObjects(ctx, svcClient, bucket) - - err = s.adm.DeleteServiceAccount(ctx, svcAK) - if err != nil { - c.Fatalf("unable to delete svc acc: %v", err) - } - c.mustNotListObjects(ctx, svcClient, bucket) - } + c.assertSvcAccDeletion(ctx, s, s.adm, accessKey, bucket) } -func (c *check) mustCreateSvcAccount(ctx context.Context, tgtUser string, admClnt *madmin.AdminClient) { - _, err := admClnt.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ +func (c *check) mustCreateSvcAccount(ctx context.Context, tgtUser string, admClnt *madmin.AdminClient) madmin.Credentials { + cr, err := admClnt.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ TargetUser: tgtUser, }) if err != nil { c.Fatalf("user should be able to create service accounts %s", err) } + return cr } func (c *check) mustNotCreateSvcAccount(ctx context.Context, tgtUser string, admClnt *madmin.AdminClient) { @@ -1106,6 +867,147 @@ func (c *check) mustListObjects(ctx context.Context, client *minio.Client, bucke } } +func (c *check) assertSvcAccS3Access(ctx context.Context, s *TestSuiteIAM, cr madmin.Credentials, bucket string) { + svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") + c.mustListObjects(ctx, svcClient, bucket) +} + +func (c *check) assertSvcAccAppearsInListing(ctx context.Context, madmClient *madmin.AdminClient, parentAK, svcAK string) { + listResp, err := madmClient.ListServiceAccounts(ctx, parentAK) + if err != nil { + c.Fatalf("unable to list svc accounts: %v", err) + } + if !set.CreateStringSet(listResp.Accounts...).Contains(svcAK) { + c.Fatalf("service account did not appear in listing!") + } +} + +func (c *check) assertSvcAccInfoQueryable(ctx context.Context, madmClient *madmin.AdminClient, parentAK, svcAK string, skipParentUserCheck bool) { + infoResp, err := madmClient.InfoServiceAccount(ctx, svcAK) + if err != nil { + c.Fatalf("unable to get svc acc info: %v", err) + } + if !skipParentUserCheck { + c.Assert(infoResp.ParentUser, parentAK) + } + c.Assert(infoResp.AccountStatus, "on") + c.Assert(infoResp.ImpliedPolicy, true) +} + +// This test assumes that the policy for `accessKey` allows listing on the given +// bucket. It creates a session policy that restricts listing on the bucket and +// then enables it again in a session policy update call. +func (c *check) assertSvcAccSessionPolicyUpdate(ctx context.Context, s *TestSuiteIAM, madmClient *madmin.AdminClient, accessKey, bucket string) { + svcAK, svcSK := mustGenerateCredentials(c) + + // This policy does not allow listing objects. + policyBytes := []byte(fmt.Sprintf(`{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:GetObject" + ], + "Resource": [ + "arn:aws:s3:::%s/*" + ] + } + ] +}`, bucket)) + cr, err := madmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ + Policy: policyBytes, + TargetUser: accessKey, + AccessKey: svcAK, + SecretKey: svcSK, + }) + if err != nil { + c.Fatalf("Unable to create svc acc: %v", err) + } + svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") + c.mustNotListObjects(ctx, svcClient, bucket) + + // This policy allows listing objects. + newPolicyBytes := []byte(fmt.Sprintf(`{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:ListBucket" + ], + "Resource": [ + "arn:aws:s3:::%s/*" + ] + } + ] +}`, bucket)) + err = madmClient.UpdateServiceAccount(ctx, svcAK, madmin.UpdateServiceAccountReq{ + NewPolicy: newPolicyBytes, + }) + if err != nil { + c.Fatalf("unable to update session policy for svc acc: %v", err) + } + c.mustListObjects(ctx, svcClient, bucket) +} + +func (c *check) assertSvcAccSecretKeyAndStatusUpdate(ctx context.Context, s *TestSuiteIAM, madmClient *madmin.AdminClient, accessKey, bucket string) { + svcAK, svcSK := mustGenerateCredentials(c) + cr, err := madmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ + TargetUser: accessKey, + AccessKey: svcAK, + SecretKey: svcSK, + }) + if err != nil { + c.Fatalf("Unable to create svc acc: %v", err) + } + svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") + c.mustListObjects(ctx, svcClient, bucket) + + _, svcSK2 := mustGenerateCredentials(c) + err = madmClient.UpdateServiceAccount(ctx, svcAK, madmin.UpdateServiceAccountReq{ + NewSecretKey: svcSK2, + }) + if err != nil { + c.Fatalf("unable to update secret key for svc acc: %v", err) + } + // old creds should not work: + c.mustNotListObjects(ctx, svcClient, bucket) + // new creds work: + svcClient2 := s.getUserClient(c, cr.AccessKey, svcSK2, "") + c.mustListObjects(ctx, svcClient2, bucket) + + // update status to disabled + err = madmClient.UpdateServiceAccount(ctx, svcAK, madmin.UpdateServiceAccountReq{ + NewStatus: "off", + }) + if err != nil { + c.Fatalf("unable to update secret key for svc acc: %v", err) + } + c.mustNotListObjects(ctx, svcClient2, bucket) +} + +func (c *check) assertSvcAccDeletion(ctx context.Context, s *TestSuiteIAM, madmClient *madmin.AdminClient, accessKey, bucket string) { + svcAK, svcSK := mustGenerateCredentials(c) + cr, err := madmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ + TargetUser: accessKey, + AccessKey: svcAK, + SecretKey: svcSK, + }) + if err != nil { + c.Fatalf("Unable to create svc acc: %v", err) + } + svcClient := s.getUserClient(c, cr.AccessKey, cr.SecretKey, "") + c.mustListObjects(ctx, svcClient, bucket) + + err = madmClient.DeleteServiceAccount(ctx, svcAK) + if err != nil { + c.Fatalf("unable to delete svc acc: %v", err) + } + c.mustNotListObjects(ctx, svcClient, bucket) +} + func mustGenerateCredentials(c *check) (string, string) { ak, sk, err := auth.GenerateCredentials() if err != nil { diff --git a/cmd/iam-store.go b/cmd/iam-store.go index b34e75730..ccc1b9118 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -1464,24 +1464,8 @@ func (store *IAMStoreSys) AddServiceAccount(ctx context.Context, cred auth.Crede return errIAMActionNotAllowed } - // Check that at least one policy is available. - policies, err := cache.policyDBGet(store.getUsersSysType(), parentUser, false) - if err != nil { - return err - } - for _, group := range cred.Groups { - gp, err := cache.policyDBGet(store.getUsersSysType(), group, true) - if err != nil && err != errNoSuchGroup { - return err - } - policies = append(policies, gp...) - } - if len(policies) == 0 { - return errNoSuchUser - } - u := newUserIdentity(cred) - err = store.saveUserIdentity(ctx, u.Credentials.AccessKey, svcUser, u) + err := store.saveUserIdentity(ctx, u.Credentials.AccessKey, svcUser, u) if err != nil { return err } @@ -1519,8 +1503,12 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st } if opts.sessionPolicy != nil { - m := make(map[string]interface{}) - err := opts.sessionPolicy.Validate() + m, err := getClaimsFromToken(cr.SessionToken) + if err != nil { + return fmt.Errorf("unable to get svc acc claims: %v", err) + } + + err = opts.sessionPolicy.Validate() if err != nil { return err } @@ -1532,9 +1520,9 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st return fmt.Errorf("Session policy should not exceed 16 KiB characters") } + // Overwrite session policy claims. m[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString(policyBuf) m[iamPolicyClaimNameSA()] = "embedded-policy" - m[parentClaim] = cr.ParentUser cr.SessionToken, err = auth.JWTSignWithAccessKey(accessKey, m, globalActiveCred.SecretKey) if err != nil { return err diff --git a/cmd/sts-handlers_test.go b/cmd/sts-handlers_test.go index 492cf259c..e6a76566a 100644 --- a/cmd/sts-handlers_test.go +++ b/cmd/sts-handlers_test.go @@ -419,6 +419,95 @@ func (s *TestSuiteIAM) TestOpenIDSTS(c *check) { } } +func (s *TestSuiteIAM) TestOpenIDServiceAcc(c *check) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + bucket := getRandomBucketName() + err := s.client.MakeBucket(ctx, bucket, minio.MakeBucketOptions{}) + if err != nil { + c.Fatalf("bucket create error: %v", err) + } + + // Generate web identity STS token by interacting with OpenID IDP. + token, err := mockTestUserInteraction(ctx, testProvider, "dillon@example.io", "dillon") + if err != nil { + c.Fatalf("mock user err: %v", err) + } + + webID := cr.STSWebIdentity{ + Client: s.TestSuiteCommon.client, + STSEndpoint: s.endPoint, + GetWebIDTokenExpiry: func() (*cr.WebIdentityToken, error) { + return &cr.WebIdentityToken{ + Token: token, + }, nil + }, + } + + // Create policy - with name as one of the groups in OpenID the user is + // a member of. + policy := "projecta" + policyBytes := []byte(fmt.Sprintf(`{ + "Version": "2012-10-17", + "Statement": [ + { + "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) + } + + value, err := webID.Retrieve() + if err != nil { + c.Fatalf("Expected to generate STS creds, got err: %#v", err) + } + + // Create an madmin client with user creds + userAdmClient, err := madmin.NewWithOptions(s.endpoint, &madmin.Options{ + Creds: cr.NewStaticV4(value.AccessKeyID, value.SecretAccessKey, value.SessionToken), + Secure: s.secure, + }) + if err != nil { + c.Fatalf("Err creating user admin client: %v", err) + } + userAdmClient.SetCustomTransport(s.TestSuiteCommon.client.Transport) + + // Create svc acc + cr := c.mustCreateSvcAccount(ctx, value.AccessKeyID, userAdmClient) + + // 1. Check that svc account appears in listing + c.assertSvcAccAppearsInListing(ctx, userAdmClient, value.AccessKeyID, cr.AccessKey) + + // 2. Check that svc account info can be queried + c.assertSvcAccInfoQueryable(ctx, userAdmClient, value.AccessKeyID, cr.AccessKey, true) + + // 3. Check S3 access + c.assertSvcAccS3Access(ctx, s, cr, bucket) + + // 4. Check that svc account can restrict the policy, and that the + // session policy can be updated. + c.assertSvcAccSessionPolicyUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket) + + // 4. Check that service account's secret key and account status can be + // updated. + c.assertSvcAccSecretKeyAndStatusUpdate(ctx, s, userAdmClient, value.AccessKeyID, bucket) + + // 5. Check that service account can be deleted. + c.assertSvcAccDeletion(ctx, s, userAdmClient, value.AccessKeyID, bucket) +} + type providerParams struct { clientID, clientSecret, providerURL, redirectURL string } @@ -611,6 +700,7 @@ func TestIAMWithOpenIDServerSuite(t *testing.T) { suite.SetUpSuite(c) suite.SetUpOpenID(c, openIDServer) suite.TestOpenIDSTS(c) + suite.TestOpenIDServiceAcc(c) suite.TearDownSuite(c) }, )