From 01b9ff54d9138b033ebc32963b350dda130e6898 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Thu, 4 Nov 2021 08:16:30 -0700 Subject: [PATCH] Add LDAP STS tests and workflow for CI (#13576) Runs LDAP tests with openldap container on GH Actions --- .github/workflows/ldap-integration.yaml | 45 ++++++ Makefile | 6 + cmd/admin-handlers-users_test.go | 16 ++- cmd/iam.go | 36 +++-- cmd/server_test.go | 22 +++ cmd/sts-handlers_test.go | 178 ++++++++++++++++++++++++ cmd/test-utils_test.go | 23 +-- 7 files changed, 306 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/ldap-integration.yaml diff --git a/.github/workflows/ldap-integration.yaml b/.github/workflows/ldap-integration.yaml new file mode 100644 index 000000000..b88bd26c3 --- /dev/null +++ b/.github/workflows/ldap-integration.yaml @@ -0,0 +1,45 @@ +name: External IDP Integration Tests + +on: + pull_request: + branches: + - master + +# This ensures that previous jobs for the PR are cancelled when the PR is +# updated. +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref }} + cancel-in-progress: true + +jobs: + ldap-test: + name: LDAP Tests with Go ${{ matrix.go-version }} + runs-on: ubuntu-latest + + services: + openldap: + image: quay.io/minio/openldap + ports: + - 389:389 + - 636:636 + env: + LDAP_ORGANISATION: "MinIO Inc" + LDAP_DOMAIN: "min.io" + LDAP_ADMIN_PASSWORD: "admin" + + + strategy: + matrix: + go-version: [1.16.x, 1.17.x] + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-go@v2 + with: + go-version: ${{ matrix.go-version }} + - name: Test LDAP + env: + LDAP_TEST_SERVER: "localhost:389" + run: | + sudo sysctl net.ipv6.conf.all.disable_ipv6=0 + sudo sysctl net.ipv6.conf.default.disable_ipv6=0 + make test-ldap diff --git a/Makefile b/Makefile index 6db6c8241..acc78a4fa 100644 --- a/Makefile +++ b/Makefile @@ -46,6 +46,12 @@ test-race: verifiers build @echo "Running unit tests under -race" @(env bash $(PWD)/buildscripts/race.sh) +test-ldap: build + @echo "Running tests for LDAP integration" + @CGO_ENABLED=0 go test -tags kqueue -v -run TestIAMWithLDAPServerSuite ./cmd + @echo "Running tests for LDAP integration with -race" + @GOGC=25 CGO_ENABLED=1 go test -race -tags kqueue -v -run TestIAMWithLDAPServerSuite ./cmd + verify: ## verify minio various setups @echo "Verifying build with race" @GO111MODULE=on CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null diff --git a/cmd/admin-handlers-users_test.go b/cmd/admin-handlers-users_test.go index 400bfedc5..d7699f155 100644 --- a/cmd/admin-handlers-users_test.go +++ b/cmd/admin-handlers-users_test.go @@ -48,9 +48,7 @@ func newTestSuiteIAM(c TestSuiteCommon) *TestSuiteIAM { return &TestSuiteIAM{TestSuiteCommon: c} } -func (s *TestSuiteIAM) SetUpSuite(c *check) { - s.TestSuiteCommon.SetUpSuite(c) - +func (s *TestSuiteIAM) iamSetup(c *check) { var err error // strip url scheme from endpoint s.endpoint = strings.TrimPrefix(s.endPoint, "http://") @@ -75,6 +73,18 @@ func (s *TestSuiteIAM) SetUpSuite(c *check) { } } +func (s *TestSuiteIAM) SetUpSuite(c *check) { + s.TestSuiteCommon.SetUpSuite(c) + + s.iamSetup(c) +} + +func (s *TestSuiteIAM) RestartIAMSuite(c *check) { + s.TestSuiteCommon.RestartTestServer(c) + + s.iamSetup(c) +} + func (s *TestSuiteIAM) getUserClient(c *check, accessKey, secretKey, sessionToken string) *minio.Client { client, err := minio.New(s.endpoint, &minio.Options{ Creds: credentials.NewStaticV4(accessKey, secretKey, sessionToken), diff --git a/cmd/iam.go b/cmd/iam.go index 5aaf82c19..69351c1c9 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -275,17 +275,29 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc switch { case globalOpenIDConfig.ProviderEnabled(): go func() { + ticker := time.NewTicker(sys.iamRefreshInterval) + defer ticker.Stop() for { - time.Sleep(sys.iamRefreshInterval) - sys.purgeExpiredCredentialsForExternalSSO(ctx) + select { + case <-ticker.C: + sys.purgeExpiredCredentialsForExternalSSO(ctx) + case <-ctx.Done(): + return + } } }() case globalLDAPConfig.EnabledWithLookupBind(): go func() { + ticker := time.NewTicker(sys.iamRefreshInterval) + defer ticker.Stop() for { - time.Sleep(sys.iamRefreshInterval) - sys.purgeExpiredCredentialsForLDAP(ctx) - sys.updateGroupMembershipsForLDAP(ctx) + select { + case <-ticker.C: + sys.purgeExpiredCredentialsForLDAP(ctx) + sys.updateGroupMembershipsForLDAP(ctx) + case <-ctx.Done(): + return + } } }() } @@ -308,14 +320,20 @@ func (sys *IAMSys) watch(ctx context.Context) { err := sys.loadWatchedEvent(ctx, event) logger.LogIf(ctx, err) } + return + } - } else { - // Fall back to loading all items - for { - time.Sleep(sys.iamRefreshInterval) + // Fall back to loading all items periodically + ticker := time.NewTicker(sys.iamRefreshInterval) + defer ticker.Stop() + for { + select { + case <-ticker.C: if err := sys.Load(ctx, sys.store); err != nil { logger.LogIf(ctx, err) } + case <-ctx.Done(): + return } } } diff --git a/cmd/server_test.go b/cmd/server_test.go index a6771d7a3..a635c49cc 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -19,6 +19,7 @@ package cmd import ( "bytes" + "context" "encoding/xml" "fmt" "io" @@ -158,6 +159,27 @@ func (s *TestSuiteCommon) SetUpSuite(c *check) { s.secretKey = s.testServer.SecretKey } +func (s *TestSuiteCommon) RestartTestServer(c *check) { + // Shutdown. + s.testServer.cancel() + s.testServer.Server.Close() + s.testServer.Obj.Shutdown(context.Background()) + + // Restart. + ctx, cancel := context.WithCancel(context.Background()) + + s.testServer.cancel = cancel + s.testServer = initTestServerWithBackend(ctx, c, s.testServer, s.testServer.Obj, s.testServer.rawDiskPaths) + if s.secure { + s.testServer.Server.StartTLS() + } else { + s.testServer.Server.Start() + } + + s.client = s.testServer.Server.Client() + s.endPoint = s.testServer.Server.URL +} + func (s *TestSuiteCommon) TearDownSuite(c *check) { s.testServer.Stop() } diff --git a/cmd/sts-handlers_test.go b/cmd/sts-handlers_test.go index 90c75dfcb..cdfbc3559 100644 --- a/cmd/sts-handlers_test.go +++ b/cmd/sts-handlers_test.go @@ -20,6 +20,8 @@ package cmd import ( "context" "fmt" + "os" + "strings" "testing" "github.com/minio/madmin-go" @@ -132,3 +134,179 @@ func (s *TestSuiteIAM) TestSTS(c *check) { c.Fatalf("unexpected non-access-denied err: %v", err) } } + +const ( + EnvTestLDAPServer = "LDAP_TEST_SERVER" +) + +func (s *TestSuiteIAM) GetLDAPServer(c *check) string { + return os.Getenv(EnvTestLDAPServer) +} + +// SetUpLDAP - expects to setup an LDAP test server using the test LDAP +// container and canned data from https://github.com/minio/minio-ldap-testing +func (s *TestSuiteIAM) SetUpLDAP(c *check) { + ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) + defer cancel() + + serverAddr := s.GetLDAPServer(c) + configCmds := []string{ + "identity_ldap", + fmt.Sprintf("server_addr=%s", serverAddr), + "server_insecure=on", + "lookup_bind_dn=cn=admin,dc=min,dc=io", + "lookup_bind_password=admin", + "user_dn_search_base_dn=dc=min,dc=io", + "user_dn_search_filter=(uid=%s)", + "group_search_base_dn=ou=swengg,dc=min,dc=io", + "group_search_filter=(&(objectclass=groupofnames)(member=%d))", + } + _, err := s.adm.SetConfigKV(ctx, strings.Join(configCmds, " ")) + if err != nil { + c.Fatalf("unable to setup LDAP for tests: %v", err) + } + + s.RestartIAMSuite(c) +} + +func TestIAMWithLDAPServerSuite(t *testing.T) { + testCases := []*TestSuiteIAM{ + // Init and run test on FS backend with signature v4. + newTestSuiteIAM(TestSuiteCommon{serverType: "FS", signer: signerV4}), + // Init and run test on FS backend, with tls enabled. + newTestSuiteIAM(TestSuiteCommon{serverType: "FS", signer: signerV4, secure: true}), + // Init and run test on Erasure backend. + newTestSuiteIAM(TestSuiteCommon{serverType: "Erasure", signer: signerV4}), + // Init and run test on ErasureSet backend. + newTestSuiteIAM(TestSuiteCommon{serverType: "ErasureSet", signer: signerV4}), + } + for i, testCase := range testCases { + t.Run(fmt.Sprintf("Test: %d, ServerType: %s", i+1, testCase.serverType), func(t *testing.T) { + c := &check{t, testCase.serverType} + suite := testCase + + if suite.GetLDAPServer(c) == "" { + return + } + + suite.SetUpSuite(c) + suite.SetUpLDAP(c) + suite.TestLDAPSTS(c) + suite.TearDownSuite(c) + }) + } +} + +func (s *TestSuiteIAM) TestLDAPSTS(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 create error: %v", err) + } + + // Create policy, user and associate policy + policy := "mypolicy" + 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) + } + + ldapID := cr.LDAPIdentity{ + Client: s.TestSuiteCommon.client, + STSEndpoint: s.endPoint, + LDAPUsername: "dillon", + LDAPPassword: "dillon", + } + + _, err = ldapID.Retrieve() + if err == nil { + c.Fatalf("Expected to fail to create a user with no associated policy!") + } + + userDN := "uid=dillon,ou=people,ou=swengg,dc=min,dc=io" + err = s.adm.SetPolicy(ctx, policy, userDN, false) + if err != nil { + c.Fatalf("Unable to set policy: %v", err) + } + + value, err := ldapID.Retrieve() + if err != nil { + c.Fatalf("Expected to generate STS creds, got err: %#v", err) + } + + minioClient, err := minio.New(s.endpoint, &minio.Options{ + Creds: cr.NewStaticV4(value.AccessKeyID, value.SecretAccessKey, value.SessionToken), + Secure: s.secure, + Transport: s.TestSuiteCommon.client.Transport, + }) + if err != nil { + c.Fatalf("Error initializing client: %v", err) + } + + // Validate that the client from sts creds can access the bucket. + c.mustListObjects(ctx, minioClient, bucket) + + // Validate that the client cannot remove any objects + err = minioClient.RemoveObject(ctx, bucket, "someobject", minio.RemoveObjectOptions{}) + if err.Error() != "Access Denied." { + c.Fatalf("unexpected non-access-denied err: %v", err) + } + + // Remove the policy assignment on the user DN: + err = s.adm.SetPolicy(ctx, "", userDN, false) + if err != nil { + c.Fatalf("Unable to remove policy setting: %v", err) + } + + _, err = ldapID.Retrieve() + if err == nil { + c.Fatalf("Expected to fail to create a user with no associated policy!") + } + + // Set policy via group and validate policy assignment. + groupDN := "cn=projectb,ou=groups,ou=swengg,dc=min,dc=io" + err = s.adm.SetPolicy(ctx, policy, groupDN, true) + if err != nil { + c.Fatalf("Unable to set group policy: %v", err) + } + + value, err = ldapID.Retrieve() + if err != nil { + c.Fatalf("Expected to generate STS creds, got err: %#v", err) + } + + minioClient, err = minio.New(s.endpoint, &minio.Options{ + Creds: cr.NewStaticV4(value.AccessKeyID, value.SecretAccessKey, value.SessionToken), + Secure: s.secure, + Transport: s.TestSuiteCommon.client.Transport, + }) + if err != nil { + c.Fatalf("Error initializing client: %v", err) + } + + // Validate that the client from sts creds can access the bucket. + c.mustListObjects(ctx, minioClient, bucket) + + // Validate that the client cannot remove any objects + err = minioClient.RemoveObject(ctx, bucket, "someobject", minio.RemoveObjectOptions{}) + c.Assert(err.Error(), "Access Denied.") +} diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index a347a75a0..76b0dd09e 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -292,13 +292,14 @@ func isSameType(obj1, obj2 interface{}) bool { // s := StartTestServer(t,"Erasure") // defer s.Stop() type TestServer struct { - Root string - Disks EndpointServerPools - AccessKey string - SecretKey string - Server *httptest.Server - Obj ObjectLayer - cancel context.CancelFunc + Root string + Disks EndpointServerPools + AccessKey string + SecretKey string + Server *httptest.Server + Obj ObjectLayer + cancel context.CancelFunc + rawDiskPaths []string } // UnstartedTestServer - Configures a temp FS/Erasure backend, @@ -314,16 +315,22 @@ func UnstartedTestServer(t TestErrHandler, instanceType string) TestServer { t.Fatal(err) } - // set the server configuration. + // set new server configuration. if err = newTestConfig(globalMinioDefaultRegion, objLayer); err != nil { t.Fatalf("%s", err) } + return initTestServerWithBackend(ctx, t, testServer, objLayer, disks) +} + +// initializes a test server with the given object layer and disks. +func initTestServerWithBackend(ctx context.Context, t TestErrHandler, testServer TestServer, objLayer ObjectLayer, disks []string) TestServer { // Test Server needs to start before formatting of disks. // Get credential. credentials := globalActiveCred testServer.Obj = objLayer + testServer.rawDiskPaths = disks testServer.Disks = mustGetPoolEndpoints(disks...) testServer.AccessKey = credentials.AccessKey testServer.SecretKey = credentials.SecretKey