From 62e724532a15d0fb2c9b13501a16691a67dbad9e Mon Sep 17 00:00:00 2001
From: Petu Eusebiu <peusebiu@cisco.com>
Date: Wed, 1 Sep 2021 12:15:00 +0300
Subject: [PATCH] Check if auth config is provided when using access control

---
 pkg/api/authn.go     | 21 +++++++++++++++++++++
 pkg/api/authz.go     | 12 ------------
 pkg/api/routes.go    |  5 +++--
 pkg/cli/root.go      |  9 +++++++++
 pkg/cli/root_test.go | 33 +++++++++++++++++++++++++++++++++
 5 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/pkg/api/authn.go b/pkg/api/authn.go
index d8581c4a..f9c12c75 100644
--- a/pkg/api/authn.go
+++ b/pkg/api/authn.go
@@ -237,6 +237,27 @@ func basicAuthHandler(c *Controller) mux.MiddlewareFunc {
 	}
 }
 
+func isAuthnEnabled(config *Config) bool {
+	if config.HTTP.Auth != nil &&
+		(config.HTTP.Auth.HTPasswd.Path != "" || config.HTTP.Auth.LDAP != nil) {
+		return true
+	}
+
+	return false
+}
+
+func isBearerAuthEnabled(config *Config) bool {
+	if config.HTTP.Auth != nil &&
+		config.HTTP.Auth.Bearer != nil &&
+		config.HTTP.Auth.Bearer.Cert != "" &&
+		config.HTTP.Auth.Bearer.Realm != "" &&
+		config.HTTP.Auth.Bearer.Service != "" {
+		return true
+	}
+
+	return false
+}
+
 func authFail(w http.ResponseWriter, realm string, delay int) {
 	time.Sleep(time.Duration(delay) * time.Second)
 	w.Header().Set("WWW-Authenticate", realm)
diff --git a/pkg/api/authz.go b/pkg/api/authz.go
index b0550797..6e5729a5 100755
--- a/pkg/api/authz.go
+++ b/pkg/api/authz.go
@@ -216,18 +216,6 @@ func getUsername(r *http.Request) string {
 	return pair[0]
 }
 
-func isBearerAuthEnabled(config *Config) bool {
-	if config.HTTP.Auth != nil &&
-		config.HTTP.Auth.Bearer != nil &&
-		config.HTTP.Auth.Bearer.Cert != "" &&
-		config.HTTP.Auth.Bearer.Realm != "" &&
-		config.HTTP.Auth.Bearer.Service != "" {
-		return true
-	}
-
-	return false
-}
-
 func authzFail(w http.ResponseWriter, realm string, delay int) {
 	time.Sleep(time.Duration(delay) * time.Second)
 	w.Header().Set("WWW-Authenticate", realm)
diff --git a/pkg/api/routes.go b/pkg/api/routes.go
index 4902b6fe..a87bc4d8 100644
--- a/pkg/api/routes.go
+++ b/pkg/api/routes.go
@@ -54,8 +54,9 @@ func NewRouteHandler(c *Controller) *RouteHandler {
 
 func (rh *RouteHandler) SetupRoutes() {
 	rh.c.Router.Use(AuthHandler(rh.c))
-
-	if !isBearerAuthEnabled(rh.c.Config) && rh.c.Config.AccessControl != nil {
+	// authz is being enabled because authn is found
+	if rh.c.Config.AccessControl != nil && !isBearerAuthEnabled(rh.c.Config) && isAuthnEnabled(rh.c.Config) {
+		rh.c.Log.Info().Msg("access control is being enabled")
 		rh.c.Router.Use(AuthzHandler(rh.c))
 	}
 
diff --git a/pkg/cli/root.go b/pkg/cli/root.go
index f25bacad..0e5c8000 100644
--- a/pkg/cli/root.go
+++ b/pkg/cli/root.go
@@ -164,6 +164,15 @@ func LoadConfiguration(config *api.Config, configPath string) {
 		panic(errors.ErrBadConfig)
 	}
 
+	// check authorization config, it should have basic auth enabled or ldap
+	if config.HTTP.RawAccessControl != nil {
+		if config.HTTP.Auth == nil || (config.HTTP.Auth.HTPasswd.Path == "" && config.HTTP.Auth.LDAP == nil) {
+			log.Error().Err(errors.ErrBadConfig).
+				Msg("access control config requires httpasswd or ldap authentication to be enabled")
+			panic(errors.ErrBadConfig)
+		}
+	}
+
 	err := config.LoadAccessControlConfig()
 	if err != nil {
 		log.Error().Err(errors.ErrBadConfig).Msg("Unable to unmarshal http.accessControl.key.policies")
diff --git a/pkg/cli/root_test.go b/pkg/cli/root_test.go
index 1b5f0638..66cd0656 100644
--- a/pkg/cli/root_test.go
+++ b/pkg/cli/root_test.go
@@ -85,6 +85,39 @@ func TestVerify(t *testing.T) {
 		So(func() { _ = cli.NewRootCmd().Execute() }, ShouldPanic)
 	})
 
+	Convey("Test verify w/ authorization and w/o authentication", t, func(c C) {
+		tmpfile, err := ioutil.TempFile("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpfile.Name()) // clean up
+		content := []byte(`{"storage":{"rootDirectory":"/tmp/zot"},
+		 					"http":{"address":"127.0.0.1","port":"8080","realm":"zot",
+							 "accessControl":{"adminPolicy":{"users":["admin"],
+							 "actions":["read","create","update","delete"]}}}}`)
+		_, err = tmpfile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpfile.Close()
+		So(err, ShouldBeNil)
+		os.Args = []string{"cli_test", "verify", tmpfile.Name()}
+		So(func() { _ = cli.NewRootCmd().Execute() }, ShouldPanic)
+	})
+
+	Convey("Test verify w/ authorization and w/ authentication", t, func(c C) {
+		tmpfile, err := ioutil.TempFile("", "zot-test*.json")
+		So(err, ShouldBeNil)
+		defer os.Remove(tmpfile.Name()) // clean up
+		content := []byte(`{"storage":{"rootDirectory":"/tmp/zot"},
+							"http":{"address":"127.0.0.1","port":"8080","realm":"zot",
+							"auth":{"htpasswd":{"path":"test/data/htpasswd"},"failDelay":1},
+							"accessControl":{"adminPolicy":{"users":["admin"],
+							"actions":["read","create","update","delete"]}}}}`)
+		_, err = tmpfile.Write(content)
+		So(err, ShouldBeNil)
+		err = tmpfile.Close()
+		So(err, ShouldBeNil)
+		os.Args = []string{"cli_test", "verify", tmpfile.Name()}
+		So(func() { _ = cli.NewRootCmd().Execute() }, ShouldNotPanic)
+	})
+
 	Convey("Test verify good config", t, func(c C) {
 		tmpfile, err := ioutil.TempFile("", "zot-test*.json")
 		So(err, ShouldBeNil)