From 75dc5c9c1d599253863954bbb0c961b6ef314b3c Mon Sep 17 00:00:00 2001 From: Laurentiu Niculae Date: Wed, 30 Aug 2023 14:31:51 +0300 Subject: [PATCH] feat(ldap): add option to load ldap from file Signed-off-by: Laurentiu Niculae --- pkg/api/config/config.go | 10 +- pkg/api/controller_test.go | 372 ++++++++++++++++++++++++++++++++++++ pkg/cli/server/root.go | 41 ++++ pkg/cli/server/root_test.go | 52 +++++ 4 files changed, 473 insertions(+), 2 deletions(-) diff --git a/pkg/api/config/config.go b/pkg/api/config/config.go index c6ed53da26..819b8d9de6 100644 --- a/pkg/api/config/config.go +++ b/pkg/api/config/config.go @@ -101,16 +101,22 @@ type SchedulerConfig struct { NumWorkers int } +type LDAPCredentials struct { + BindDN string + BindPassword string +} + type LDAPConfig struct { + CredentialsFile string Port int Insecure bool StartTLS bool // if !Insecure, then StartTLS or LDAPs SkipVerify bool SubtreeSearch bool Address string - BindDN string + BindDN string `json:"-"` + BindPassword string `json:"-"` UserGroupAttribute string - BindPassword string BaseDN string UserAttribute string CACert string diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index e3396a046e..3c949ddcee 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -53,6 +53,7 @@ import ( "zotregistry.io/zot/pkg/api/config" "zotregistry.io/zot/pkg/api/constants" apiErr "zotregistry.io/zot/pkg/api/errors" + "zotregistry.io/zot/pkg/cli/server" "zotregistry.io/zot/pkg/common" extconf "zotregistry.io/zot/pkg/extensions/config" "zotregistry.io/zot/pkg/log" @@ -2077,6 +2078,282 @@ func TestBasicAuthWithLDAP(t *testing.T) { }) } +func TestBasicAuthWithLDAPFromFile(t *testing.T) { + Convey("Make a new controller", t, func() { + l := newTestLDAPServer() + port := test.GetFreePort() + ldapPort, err := strconv.Atoi(port) + So(err, ShouldBeNil) + l.Start(ldapPort) + defer l.Stop() + + port = test.GetFreePort() + baseURL := test.GetBaseURL(port) + tempDir := t.TempDir() + + ldapConfigContent := fmt.Sprintf(` + { + "BindDN": "%v", + "BindPassword": "%v" + }`, LDAPBindDN, LDAPBindPassword) + + ldapConfigPath := filepath.Join(tempDir, "ldap.json") + + err = os.WriteFile(ldapConfigPath, []byte(ldapConfigContent), 0o600) + So(err, ShouldBeNil) + + configStr := fmt.Sprintf(` + { + "Storage": { + "RootDirectory": "%s" + }, + "HTTP": { + "Address": "%s", + "Port": "%s", + "Auth": { + "LDAP": { + "CredentialsFile": "%s", + "BaseDN": "%v", + "UserAttribute": "uid", + "UserGroupAttribute": "memberOf", + "Insecure": true, + "Address": "%v", + "Port": %v + } + } + } + }`, tempDir, "127.0.0.1", port, ldapConfigPath, LDAPBaseDN, LDAPAddress, ldapPort) + + configPath := filepath.Join(tempDir, "config.json") + + err = os.WriteFile(configPath, []byte(configStr), 0o0600) + So(err, ShouldBeNil) + + server := server.NewServerRootCmd() + server.SetArgs([]string{"serve", configPath}) + go func() { + err := server.Execute() + if err != nil { + panic(err) + } + }() + + test.WaitTillServerReady(baseURL) + + // without creds, should get access error + resp, err := resty.R().Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + var e apiErr.Error + err = json.Unmarshal(resp.Body(), &e) + So(err, ShouldBeNil) + + // with creds, should get expected status code + resp, _ = resty.R().SetBasicAuth(username, password).Get(baseURL) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + + resp, _ = resty.R().SetBasicAuth(username, password).Get(baseURL + "/v2/") + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // missing password + resp, _ = resty.R().SetBasicAuth(username, "").Get(baseURL + "/v2/") + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + }) + + Convey("Fail because of missing credential file field from config", t, func() { + l := newTestLDAPServer() + port := test.GetFreePort() + ldapPort, err := strconv.Atoi(port) + So(err, ShouldBeNil) + l.Start(ldapPort) + defer l.Stop() + + port = test.GetFreePort() + baseURL := test.GetBaseURL(port) + tempDir := t.TempDir() + + configStr := fmt.Sprintf(` + { + "Storage": { + "RootDirectory": "%s" + }, + "HTTP": { + "Address": "%s", + "Port": "%s", + "Auth": { + "LDAP": { + "BaseDN": "%v", + "UserAttribute": "uid", + "UserGroupAttribute": "memberOf", + "Insecure": true, + "Address": "%v", + "Port": %v, + "BindDN": "%v", + "BindPassword": "%v" + } + } + } + }`, tempDir, "127.0.0.1", port, LDAPBaseDN, LDAPAddress, ldapPort, LDAPBindDN, LDAPBindPassword) + + configPath := filepath.Join(tempDir, "config.json") + + err = os.WriteFile(configPath, []byte(configStr), 0o0600) + So(err, ShouldBeNil) + + server := server.NewServerRootCmd() + server.SetArgs([]string{"serve", configPath}) + + errChan := make(chan error, 1) + timeout := time.After(300 * time.Second) + + go func() { + defer func() { + if r := recover(); r != nil { + err, ok := r.(error) + if !ok { + errChan <- fmt.Errorf("returned object from panic is not type 'error', it's type '%T'", r) //nolint: goerr113 + } + + errChan <- err + } + }() + + err := server.Execute() + if err != nil { + errChan <- err + } + }() + + WaitErrorLoop: + for { + select { + case <-timeout: + t.Log("Server is stuck") + t.FailNow() + case err := <-errChan: + So(err.Error(), ShouldContainSubstring, "missing CredentialsFile field") + + break WaitErrorLoop + default: + _, err := resty.R().Get(baseURL) + if err != nil { + time.Sleep(500 * time.Millisecond) + + break + } + + t.Log("Server successfully loaded when it should have failed") + t.FailNow() + } + } + }) +} + +func TestLDAPConfigErrors(t *testing.T) { + const configTemplate = ` + { + "Storage": { + "RootDirectory": "%s" + }, + "HTTP": { + "Address": "%s", + "Port": "%s", + "Auth": { + "LDAP": { + "CredentialsFile": "%s", + "BaseDN": "%v", + "UserAttribute": "%v", + "UserGroupAttribute": "memberOf", + "Insecure": true, + "Address": "%v", + "Port": %v, + "BindDN": "%v", + "BindPassword": "%v" + } + } + } + }` + + Convey("UserAttribute is empty", t, func() { + conf := config.New() + tempDir := t.TempDir() + ldapPort := 9000 + userAttribute := "" + + ldapConfigContent := fmt.Sprintf(` + { + "BindDN": "%v", + "BindPassword": "%v" + }`, LDAPBindDN, LDAPBindPassword) + ldapConfigPath := filepath.Join(tempDir, "ldap.json") + err := os.WriteFile(ldapConfigPath, []byte(ldapConfigContent), 0o600) + So(err, ShouldBeNil) + + configStr := fmt.Sprintf(configTemplate, + tempDir, "127.0.0.1", "8000", ldapConfigPath, LDAPBaseDN, userAttribute, LDAPAddress, ldapPort, LDAPBindDN, LDAPBindPassword) + configPath := filepath.Join(tempDir, "config.json") + err = os.WriteFile(configPath, []byte(configStr), 0o0600) + So(err, ShouldBeNil) + + err = server.LoadConfiguration(conf, configPath) + So(err, ShouldNotBeNil) + }) + + Convey("address is empty", t, func() { + conf := config.New() + tempDir := t.TempDir() + ldapPort := 9000 + userAttribute := "uid" + + ldapConfigContent := fmt.Sprintf(` + { + "BindDN": "%v", + "BindPassword": "%v" + }`, LDAPBindDN, LDAPBindPassword) + ldapConfigPath := filepath.Join(tempDir, "ldap.json") + err := os.WriteFile(ldapConfigPath, []byte(ldapConfigContent), 0o600) + So(err, ShouldBeNil) + + configStr := fmt.Sprintf(configTemplate, + tempDir, "127.0.0.1", "8000", ldapConfigPath, LDAPBaseDN, userAttribute, "", ldapPort, LDAPBindDN, LDAPBindPassword) + configPath := filepath.Join(tempDir, "config.json") + err = os.WriteFile(configPath, []byte(configStr), 0o0600) + So(err, ShouldBeNil) + + err = server.LoadConfiguration(conf, configPath) + So(err, ShouldNotBeNil) + }) + + Convey("BaseDN is empty", t, func() { + conf := config.New() + tempDir := t.TempDir() + ldapPort := 9000 + userAttribute := "uid" + + ldapConfigContent := fmt.Sprintf(` + { + "BindDN": "%v", + "BindPassword": "%v" + }`, LDAPBindDN, LDAPBindPassword) + ldapConfigPath := filepath.Join(tempDir, "ldap.json") + err := os.WriteFile(ldapConfigPath, []byte(ldapConfigContent), 0o600) + So(err, ShouldBeNil) + + configStr := fmt.Sprintf(configTemplate, + tempDir, "127.0.0.1", "8000", ldapConfigPath, "", userAttribute, LDAPAddress, ldapPort, LDAPBindDN, LDAPBindPassword) + configPath := filepath.Join(tempDir, "config.json") + err = os.WriteFile(configPath, []byte(configStr), 0o0600) + So(err, ShouldBeNil) + + err = server.LoadConfiguration(conf, configPath) + So(err, ShouldNotBeNil) + }) +} + func TestGroupsPermissionsForLDAP(t *testing.T) { Convey("Make a new controller", t, func() { l := newTestLDAPServer() @@ -2145,6 +2422,101 @@ func TestGroupsPermissionsForLDAP(t *testing.T) { }) } +func TestLDAPConfigFromFile(t *testing.T) { + Convey("Make a new controller", t, func() { + l := newTestLDAPServer() + port := test.GetFreePort() + ldapPort, err := strconv.Atoi(port) + So(err, ShouldBeNil) + l.Start(ldapPort) + defer l.Stop() + + port = test.GetFreePort() + baseURL := test.GetBaseURL(port) + tempDir := t.TempDir() + + ldapConfigContent := fmt.Sprintf(` + { + "BindDN": "%v", + "BindPassword": "%v" + }`, LDAPBindDN, LDAPBindPassword) + + ldapConfigPath := filepath.Join(tempDir, "ldap.json") + + err = os.WriteFile(ldapConfigPath, []byte(ldapConfigContent), 0o600) + So(err, ShouldBeNil) + + configStr := fmt.Sprintf(` + { + "Storage": { + "RootDirectory": "%s" + }, + "HTTP": { + "Address": "%s", + "Port": "%s", + "Auth": { + "LDAP": { + "CredentialsFile": "%s", + "BaseDN": "%v", + "UserAttribute": "uid", + "UserGroupAttribute": "memberOf", + "Insecure": true, + "Address": "%v", + "Port": %v + } + }, + "AccessControl": { + "repositories": { + "test-ldap": { + "Policies": [ + { + "Users": null, + "Actions": [ + "read", + "create" + ], + "Groups": [ + "test" + ] + } + ] + } + }, + "Groups": { + "test": { + "Users": [ + "test" + ] + } + } + } + } + }`, tempDir, "127.0.0.1", port, ldapConfigPath, LDAPBaseDN, LDAPAddress, ldapPort) + + configPath := filepath.Join(tempDir, "config.json") + + err = os.WriteFile(configPath, []byte(configStr), 0o0600) + So(err, ShouldBeNil) + + server := server.NewServerRootCmd() + server.SetArgs([]string{"serve", configPath}) + go func() { + err := server.Execute() + if err != nil { + panic(err) + } + }() + + test.WaitTillServerReady(baseURL) + + repo := "test-ldap" + img := CreateDefaultImage() + + err = UploadImageWithBasicAuth(img, baseURL, repo, img.DigestStr(), username, password) + So(err, ShouldBeNil) + }) +} + func TestLDAPFailures(t *testing.T) { Convey("Make a LDAP conn", t, func() { l := newTestLDAPServer() diff --git a/pkg/cli/server/root.go b/pkg/cli/server/root.go index c4088fa806..764f75cd74 100644 --- a/pkg/cli/server/root.go +++ b/pkg/cli/server/root.go @@ -2,6 +2,7 @@ package server import ( "context" + "encoding/json" "fmt" "net" "net/http" @@ -727,6 +728,10 @@ func LoadConfiguration(config *config.Config, configPath string) error { return zerr.ErrBadConfig } + if err := updateLDAPConfig(config); err != nil { + return err + } + // defaults applyDefaultValues(config, viperInstance, log) @@ -741,6 +746,42 @@ func LoadConfiguration(config *config.Config, configPath string) error { return nil } +func updateLDAPConfig(conf *config.Config) error { + if conf.HTTP.Auth == nil || conf.HTTP.Auth.LDAP == nil { + return nil + } + + if conf.HTTP.Auth.LDAP.CredentialsFile == "" { + return fmt.Errorf("%w: missing CredentialsFile field", zerr.ErrLDAPConfig) + } + + newLDAPCredentials, err := readLDAPCredentials(conf.HTTP.Auth.LDAP.CredentialsFile) + if err != nil { + return err + } + + conf.HTTP.Auth.LDAP.BindDN = newLDAPCredentials.BindDN + conf.HTTP.Auth.LDAP.BindPassword = newLDAPCredentials.BindPassword + + return nil +} + +func readLDAPCredentials(ldapConfigPath string) (config.LDAPCredentials, error) { + ldapConfigBlob, err := os.ReadFile(ldapConfigPath) + if err != nil { + return config.LDAPCredentials{}, err + } + + var ldapCredentials config.LDAPCredentials + + err = json.Unmarshal(ldapConfigBlob, &ldapCredentials) + if err != nil { + return config.LDAPCredentials{}, err + } + + return ldapCredentials, nil +} + func authzContainsOnlyAnonymousPolicy(cfg *config.Config) bool { adminPolicy := cfg.HTTP.AccessControl.AdminPolicy anonymousPolicyPresent := false diff --git a/pkg/cli/server/root_test.go b/pkg/cli/server/root_test.go index c97c0277d4..1cb019056f 100644 --- a/pkg/cli/server/root_test.go +++ b/pkg/cli/server/root_test.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path" + "path/filepath" "testing" "time" @@ -1359,6 +1360,57 @@ func TestScrub(t *testing.T) { }) } +func TestUpdateLDAPConfig(t *testing.T) { + Convey("updateLDAPConfig", t, func() { + tempDir := t.TempDir() + + ldapConfigContent := "bad-json" + + ldapConfigPath := filepath.Join(tempDir, "ldap.json") + + err := os.WriteFile(ldapConfigPath, []byte(ldapConfigContent), 0o000) + So(err, ShouldBeNil) + + configStr := fmt.Sprintf(` + { + "Storage": { + "RootDirectory": "%s" + }, + "HTTP": { + "Address": "%s", + "Port": "%s", + "Auth": { + "LDAP": { + "CredentialsFile": "%s", + "BaseDN": "%v", + "UserAttribute": "uid", + "UserGroupAttribute": "memberOf", + "Insecure": true, + "Address": "%v", + "Port": %v + } + } + } + }`, tempDir, "127.0.0.1", "8000", ldapConfigPath, "LDAPBaseDN", "LDAPAddress", 1000) + + configPath := filepath.Join(tempDir, "config.json") + + err = os.WriteFile(configPath, []byte(configStr), 0o0600) + So(err, ShouldBeNil) + + server := cli.NewServerRootCmd() + server.SetArgs([]string{"serve", configPath}) + So(func() { err = server.Execute() }, ShouldPanic) + + err = os.Chmod(ldapConfigPath, 0o600) + So(err, ShouldBeNil) + + server = cli.NewServerRootCmd() + server.SetArgs([]string{"serve", configPath}) + So(func() { err = server.Execute() }, ShouldPanic) + }) +} + // run cli and return output. func runCLIWithConfig(tempDir string, config string) (string, error) { port := GetFreePort()