From b74366d50b126581d086b47dc620589375874805 Mon Sep 17 00:00:00 2001
From: Nikita Kotikov <25552941+onidoru@users.noreply.github.com>
Date: Thu, 15 Feb 2024 22:55:49 +0200
Subject: [PATCH] fix: LDAP credentials files are now required, add more tests

Signed-off-by: onidoru <25552941+onidoru@users.noreply.github.com>
Signed-off-by: Nikita Kotikov <25552941+onidoru@users.noreply.github.com>
---
 .github/workflows/verify-config.yaml |  1 -
 pkg/cli/server/root.go               | 24 ++++++++-
 pkg/cli/server/root_test.go          | 77 ++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/verify-config.yaml b/.github/workflows/verify-config.yaml
index 9f4f0261..eb4e9655 100644
--- a/.github/workflows/verify-config.yaml
+++ b/.github/workflows/verify-config.yaml
@@ -39,7 +39,6 @@ jobs:
         run: |
           cd $GITHUB_WORKSPACE
           go mod download
-      - uses: ./.github/actions/setup-localstack
       - name: run verify-config
         run: |
           cd $GITHUB_WORKSPACE
diff --git a/pkg/cli/server/root.go b/pkg/cli/server/root.go
index b8922863..022af65d 100644
--- a/pkg/cli/server/root.go
+++ b/pkg/cli/server/root.go
@@ -861,12 +861,34 @@ func readLDAPCredentials(ldapConfigPath string) (config.LDAPCredentials, error)
 
 	var ldapCredentials config.LDAPCredentials
 
-	if err := viperInstance.UnmarshalExact(&ldapCredentials); err != nil {
+	metaData := &mapstructure.Metadata{}
+	if err := viperInstance.UnmarshalExact(&ldapCredentials, metadataConfig(metaData)); err != nil {
 		log.Error().Err(err).Msg("failed to unmarshal ldap credentials config")
 
 		return config.LDAPCredentials{}, err
 	}
 
+	if len(metaData.Keys) == 0 {
+		log.Error().Err(zerr.ErrBadConfig).
+			Msg("failed to load ldap credentials config due to the absence of any key:value pair")
+
+		return config.LDAPCredentials{}, zerr.ErrBadConfig
+	}
+
+	if len(metaData.Unused) > 0 {
+		log.Error().Err(zerr.ErrBadConfig).Strs("keys", metaData.Unused).
+			Msg("failed to load ldap credentials config due to unknown keys")
+
+		return config.LDAPCredentials{}, zerr.ErrBadConfig
+	}
+
+	if len(metaData.Unset) > 0 {
+		log.Error().Err(zerr.ErrBadConfig).Strs("keys", metaData.Unset).
+			Msg("failed to load ldap credentials config due to unset keys")
+
+		return config.LDAPCredentials{}, zerr.ErrBadConfig
+	}
+
 	return ldapCredentials, nil
 }
 
diff --git a/pkg/cli/server/root_test.go b/pkg/cli/server/root_test.go
index 5a3bacab..73bab520 100644
--- a/pkg/cli/server/root_test.go
+++ b/pkg/cli/server/root_test.go
@@ -1236,6 +1236,83 @@ storage:
 		err = cli.NewServerRootCmd().Execute()
 		So(err, ShouldBeNil)
 	})
+
+	Convey("Test verify good ldap config", t, func(c C) {
+		tmpFile, err := os.CreateTemp("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpFile.Name())
+
+		tmpCredsFile, err := os.CreateTemp("", "zot-cred*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpCredsFile.Name())
+
+		content := []byte(`{
+    		"bindDN":"cn=ldap-searcher,ou=Users,dc=example,dc=org",
+    		"bindPassword":"ldap-searcher-password"
+		}`)
+
+		_, err = tmpCredsFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpCredsFile.Close()
+		So(err, ShouldBeNil)
+
+		content = []byte(fmt.Sprintf(`{ "distSpecVersion": "1.1.0-dev", 
+			"storage": { "rootDirectory": "/tmp/zot" }, "http": { "address": "127.0.0.1", "port": "8080", 
+			"auth": { "ldap": { "credentialsFile": "%v", "address": "ldap.example.org", "port": 389, 
+			"startTLS": false, "baseDN": "ou=Users,dc=example,dc=org", 
+			"userAttribute": "uid", "userGroupAttribute": "memberOf", "skipVerify": true, "subtreeSearch": true }, 
+			"failDelay": 5 } }, "log": { "level": "debug" } }`,
+			tmpCredsFile.Name()),
+		)
+
+		_, err = tmpFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpFile.Close()
+		So(err, ShouldBeNil)
+
+		os.Args = []string{"cli_test", "verify", tmpFile.Name()}
+		err = cli.NewServerRootCmd().Execute()
+		So(err, ShouldBeNil)
+	})
+
+	Convey("Test verify bad ldap config", t, func(c C) {
+		tmpFile, err := os.CreateTemp("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpFile.Name())
+
+		tmpCredsFile, err := os.CreateTemp("", "zot-cred*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpCredsFile.Name())
+
+		// `bindDN` key is missing
+		content := []byte(`{
+    		"bindPassword":"ldap-searcher-password"
+		}`)
+
+		_, err = tmpCredsFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpCredsFile.Close()
+		So(err, ShouldBeNil)
+
+		content = []byte(fmt.Sprintf(`{ "distSpecVersion": "1.1.0-dev", 
+			"storage": { "rootDirectory": "/tmp/zot" }, "http": { "address": "127.0.0.1", "port": "8080", 
+			"auth": { "ldap": { "credentialsFile": "%v", "address": "ldap.example.org", "port": 389, 
+			"startTLS": false, "baseDN": "ou=Users,dc=example,dc=org", 
+			"userAttribute": "uid", "userGroupAttribute": "memberOf", "skipVerify": true, "subtreeSearch": true }, 
+			"failDelay": 5 } }, "log": { "level": "debug" } }`,
+			tmpCredsFile.Name()),
+		)
+
+		_, err = tmpFile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpFile.Close()
+		So(err, ShouldBeNil)
+
+		os.Args = []string{"cli_test", "verify", tmpFile.Name()}
+		err = cli.NewServerRootCmd().Execute()
+		So(err, ShouldNotBeNil)
+		So(err.Error(), ShouldContainSubstring, "invalid server config")
+	})
 }
 
 func TestApiKeyConfig(t *testing.T) {