Merge pull request #98 from tsnaik/fix-htpasswd

fix: the bug when htpasswd has multiple creds

.gitignore

@@ -20,3 +20,4 @@ test/data/
 coverage.html
 tags
 vendor/
+.vscode/

pkg/api/auth.go

@@ -141,18 +141,19 @@ func basicAuthHandler(c *Controller) mux.MiddlewareFunc {
 			if err != nil {
 				panic(err)
 			}
+			defer f.Close()
 
-			for {
-				r := bufio.NewReader(f)
-				line, err := r.ReadString('\n')
-				if err != nil {
-					break
-				}
-				tokens := strings.Split(line, ":")
+			scanner := bufio.NewScanner(f)
+
+			for scanner.Scan() {
+				line := scanner.Text()
+				if strings.Contains(line, ":") {
+					tokens := strings.Split(scanner.Text(), ":")
 					credMap[tokens[0]] = tokens[1]
 				}
 			}
 		}
+	}
 
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

pkg/api/controller_test.go

@@ -18,6 +18,8 @@ import (
 	"testing"
 	"time"
 
+	"golang.org/x/crypto/bcrypt"
+
 	"github.com/anuvu/zot/pkg/api"
 	"github.com/chartmuseum/auth"
 	"github.com/mitchellh/mapstructure"
@@ -71,6 +73,31 @@ func makeHtpasswdFile() string {
 	return f.Name()
 }
 
+func makeHtpasswdFileFromString(fileContent string) string {
+	f, err := ioutil.TempFile("", "htpasswd-")
+	if err != nil {
+		panic(err)
+	}
+
+	// bcrypt(username="test", passwd="test")
+	content := []byte(fileContent)
+	if err := ioutil.WriteFile(f.Name(), content, 0644); err != nil {
+		panic(err)
+	}
+
+	return f.Name()
+}
+
+func getCredString(username, password string) string {
+	hash, err := bcrypt.GenerateFromPassword([]byte(password), 10)
+	if err != nil {
+		panic(err)
+	}
+
+	usernameAndHash := fmt.Sprintf("%s:%s", username, string(hash))
+
+	return usernameAndHash
+}
 func TestNew(t *testing.T) {
 	Convey("Make a new controller", t, func() {
 		config := api.NewConfig()
@@ -79,6 +106,199 @@ func TestNew(t *testing.T) {
 	})
 }
 
+func TestHtpasswdSingleCred(t *testing.T) {
+	Convey("Single cred", t, func() {
+		singleCredtests := []string{}
+		user := "alice"
+		password := "alice"
+		singleCredtests = append(singleCredtests, getCredString(user, password))
+		singleCredtests = append(singleCredtests, getCredString(user, password)+"\n")
+
+		for _, testString := range singleCredtests {
+			func() {
+				config := api.NewConfig()
+				config.HTTP.Port = SecurePort1
+
+				htpasswdPath := makeHtpasswdFileFromString(testString)
+				defer os.Remove(htpasswdPath)
+				config.HTTP.Auth = &api.AuthConfig{
+					HTPasswd: api.AuthHTPasswd{
+						Path: htpasswdPath,
+					},
+				}
+				c := api.NewController(config)
+				dir, err := ioutil.TempDir("", "oci-repo-test")
+				if err != nil {
+					panic(err)
+				}
+				defer os.RemoveAll(dir)
+				c.Config.Storage.RootDirectory = dir
+				go func(controller *api.Controller) {
+					// this blocks
+					if err := controller.Run(); err != nil {
+						return
+					}
+				}(c)
+				// wait till ready
+				for {
+					_, err := resty.R().Get(BaseURL1)
+					if err == nil {
+						break
+					}
+					time.Sleep(100 * time.Millisecond)
+				}
+				defer func(controller *api.Controller) {
+					ctx := context.Background()
+					_ = controller.Server.Shutdown(ctx)
+				}(c)
+				// with creds, should get expected status code
+				resp, _ := resty.R().SetBasicAuth(user, password).Get(BaseURL1 + "/v2/")
+				So(resp, ShouldNotBeNil)
+				So(resp.StatusCode(), ShouldEqual, 200)
+
+				//with invalid creds, it should fail
+				resp, _ = resty.R().SetBasicAuth("chuck", "chuck").Get(BaseURL1 + "/v2/")
+				So(resp, ShouldNotBeNil)
+				So(resp.StatusCode(), ShouldEqual, 401)
+			}()
+		}
+	})
+}
+
+func TestHtpasswdTwoCreds(t *testing.T) {
+	Convey("Two creds", t, func() {
+		twoCredTests := []string{}
+		user1 := "alicia"
+		password1 := "aliciapassword"
+		user2 := "bob"
+		password2 := "robert"
+		twoCredTests = append(twoCredTests, getCredString(user1, password1)+"\n"+
+			getCredString(user2, password2))
+
+		twoCredTests = append(twoCredTests, getCredString(user1, password1)+"\n"+
+			getCredString(user2, password2)+"\n")
+
+		twoCredTests = append(twoCredTests, getCredString(user1, password1)+"\n\n"+
+			getCredString(user2, password2)+"\n\n")
+
+		for _, testString := range twoCredTests {
+			func() {
+				config := api.NewConfig()
+				config.HTTP.Port = SecurePort1
+				htpasswdPath := makeHtpasswdFileFromString(testString)
+				defer os.Remove(htpasswdPath)
+				config.HTTP.Auth = &api.AuthConfig{
+					HTPasswd: api.AuthHTPasswd{
+						Path: htpasswdPath,
+					},
+				}
+				c := api.NewController(config)
+				dir, err := ioutil.TempDir("", "oci-repo-test")
+				if err != nil {
+					panic(err)
+				}
+				defer os.RemoveAll(dir)
+				c.Config.Storage.RootDirectory = dir
+				go func(controller *api.Controller) {
+					// this blocks
+					if err := controller.Run(); err != nil {
+						return
+					}
+				}(c)
+				// wait till ready
+				for {
+					_, err := resty.R().Get(BaseURL1)
+					if err == nil {
+						break
+					}
+					time.Sleep(100 * time.Millisecond)
+				}
+
+				defer func(controller *api.Controller) {
+					ctx := context.Background()
+					_ = controller.Server.Shutdown(ctx)
+				}(c)
+				// with creds, should get expected status code
+				resp, _ := resty.R().SetBasicAuth(user1, password1).Get(BaseURL1 + "/v2/")
+				So(resp, ShouldNotBeNil)
+				So(resp.StatusCode(), ShouldEqual, 200)
+
+				resp, _ = resty.R().SetBasicAuth(user2, password2).Get(BaseURL1 + "/v2/")
+				So(resp, ShouldNotBeNil)
+				So(resp.StatusCode(), ShouldEqual, 200)
+
+				//with invalid creds, it should fail
+				resp, _ = resty.R().SetBasicAuth("chuck", "chuck").Get(BaseURL1 + "/v2/")
+				So(resp, ShouldNotBeNil)
+				So(resp.StatusCode(), ShouldEqual, 401)
+			}()
+		}
+	})
+}
+func TestHtpasswdFiveCreds(t *testing.T) {
+	Convey("Five creds", t, func() {
+		tests := map[string]string{
+			"michael": "scott",
+			"jim":     "halpert",
+			"dwight":  "shrute",
+			"pam":     "bessley",
+			"creed":   "bratton",
+		}
+		credString := strings.Builder{}
+		for key, val := range tests {
+			credString.WriteString(getCredString(key, val) + "\n")
+		}
+
+		func() {
+			config := api.NewConfig()
+			config.HTTP.Port = SecurePort1
+			htpasswdPath := makeHtpasswdFileFromString(credString.String())
+			defer os.Remove(htpasswdPath)
+			config.HTTP.Auth = &api.AuthConfig{
+				HTPasswd: api.AuthHTPasswd{
+					Path: htpasswdPath,
+				},
+			}
+			c := api.NewController(config)
+			dir, err := ioutil.TempDir("", "oci-repo-test")
+			if err != nil {
+				panic(err)
+			}
+			defer os.RemoveAll(dir)
+			c.Config.Storage.RootDirectory = dir
+			go func(controller *api.Controller) {
+				// this blocks
+				if err := controller.Run(); err != nil {
+					return
+				}
+			}(c)
+			// wait till ready
+			for {
+				_, err := resty.R().Get(BaseURL1)
+				if err == nil {
+					break
+				}
+				time.Sleep(100 * time.Millisecond)
+			}
+
+			defer func(controller *api.Controller) {
+				ctx := context.Background()
+				_ = controller.Server.Shutdown(ctx)
+			}(c)
+			// with creds, should get expected status code
+			for key, val := range tests {
+				resp, _ := resty.R().SetBasicAuth(key, val).Get(BaseURL1 + "/v2/")
+				So(resp, ShouldNotBeNil)
+				So(resp.StatusCode(), ShouldEqual, 200)
+			}
+
+			//with invalid creds, it should fail
+			resp, _ := resty.R().SetBasicAuth("chuck", "chuck").Get(BaseURL1 + "/v2/")
+			So(resp, ShouldNotBeNil)
+			So(resp.StatusCode(), ShouldEqual, 401)
+		}()
+	})
+}
 func TestBasicAuth(t *testing.T) {
 	Convey("Make a new controller", t, func() {
 		config := api.NewConfig()