From d16cbb0b101f39bed5a914c14cbc8092a956ecf1 Mon Sep 17 00:00:00 2001 From: Tanmay Naik Date: Tue, 9 Jun 2020 17:18:30 -0400 Subject: [PATCH 1/4] .gitignore: add .vscode/ --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 1a214e55..79b8cffc 100644 --- a/.gitignore +++ b/.gitignore @@ -19,4 +19,5 @@ test/data/ .idea/ coverage.html tags -vendor/ \ No newline at end of file +vendor/ +.vscode/ \ No newline at end of file From 3cfb2b30a68dfc265f4157f049cdf1f0e4b42046 Mon Sep 17 00:00:00 2001 From: Tanmay Naik Date: Tue, 9 Jun 2020 17:19:01 -0400 Subject: [PATCH 2/4] fix: the bug when htpasswd has multiple creds earlier, when you had more than one creds in htpasswd file separated by newline, it used to only read the first cred in the file and ignore the rest. --- pkg/api/auth.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/api/auth.go b/pkg/api/auth.go index 2efaa4fe..402cbe86 100644 --- a/pkg/api/auth.go +++ b/pkg/api/auth.go @@ -141,15 +141,16 @@ 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 + scanner := bufio.NewScanner(f) + + for scanner.Scan() { + line := scanner.Text() + if strings.Contains(line, ":") { + tokens := strings.Split(scanner.Text(), ":") + credMap[tokens[0]] = tokens[1] } - tokens := strings.Split(line, ":") - credMap[tokens[0]] = tokens[1] } } } From 904ae763d77fbb64bc5001da5725c755a4400db0 Mon Sep 17 00:00:00 2001 From: Tanmay Naik Date: Tue, 9 Jun 2020 19:18:33 -0400 Subject: [PATCH 3/4] tests: add unit tests for fix 3cfb2b3 --- pkg/api/controller_test.go | 139 +++++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 4cf4bd22..0accf368 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -71,6 +71,20 @@ 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 TestNew(t *testing.T) { Convey("Make a new controller", t, func() { config := api.NewConfig() @@ -79,6 +93,131 @@ func TestNew(t *testing.T) { }) } +func TestHtpasswdSingleCred(t *testing.T) { + Convey("Single cred", t, func() { + singleCredtests := + []string{"alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma", + "alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma\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("alice", "alice").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{"alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma\n" + + "bob:$2y$12$MZ9Wh44eZC4ck6TIftfM5.0QAQgpgvCTGDHQHgKCeqtxIx/OzJt6m", + + "alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma\n" + + "bob:$2y$12$MZ9Wh44eZC4ck6TIftfM5.0QAQgpgvCTGDHQHgKCeqtxIx/OzJt6m\n", + + "alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma\n\n" + + "bob:$2y$12$MZ9Wh44eZC4ck6TIftfM5.0QAQgpgvCTGDHQHgKCeqtxIx/OzJt6m\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("alice", "alice").Get(BaseURL1 + "/v2/") + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, 200) + + resp, _ = resty.R().SetBasicAuth("bob", "bob").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() From 3f3f7e3f8c96a58afe7bf166bb16198327ca9dac Mon Sep 17 00:00:00 2001 From: Tanmay Naik Date: Wed, 17 Jun 2020 20:17:49 -0400 Subject: [PATCH 4/4] tests: add better tests for 3cfb2b3 --- pkg/api/controller_test.go | 113 +++++++++++++++++++++++++++++++------ 1 file changed, 97 insertions(+), 16 deletions(-) diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 0accf368..95643a15 100644 --- a/pkg/api/controller_test.go +++ b/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" @@ -85,6 +87,17 @@ func makeHtpasswdFileFromString(fileContent string) string { 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() @@ -95,10 +108,11 @@ func TestNew(t *testing.T) { func TestHtpasswdSingleCred(t *testing.T) { Convey("Single cred", t, func() { - singleCredtests := - []string{"alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma", - "alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma\n", - } + singleCredtests := []string{} + user := "alice" + password := "alice" + singleCredtests = append(singleCredtests, getCredString(user, password)) + singleCredtests = append(singleCredtests, getCredString(user, password)+"\n") for _, testString := range singleCredtests { func() { @@ -138,7 +152,7 @@ func TestHtpasswdSingleCred(t *testing.T) { _ = controller.Server.Shutdown(ctx) }(c) // with creds, should get expected status code - resp, _ := resty.R().SetBasicAuth("alice", "alice").Get(BaseURL1 + "/v2/") + resp, _ := resty.R().SetBasicAuth(user, password).Get(BaseURL1 + "/v2/") So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, 200) @@ -153,18 +167,21 @@ func TestHtpasswdSingleCred(t *testing.T) { func TestHtpasswdTwoCreds(t *testing.T) { Convey("Two creds", t, func() { - twoCredtests := - []string{"alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma\n" + - "bob:$2y$12$MZ9Wh44eZC4ck6TIftfM5.0QAQgpgvCTGDHQHgKCeqtxIx/OzJt6m", + twoCredTests := []string{} + user1 := "alicia" + password1 := "aliciapassword" + user2 := "bob" + password2 := "robert" + twoCredTests = append(twoCredTests, getCredString(user1, password1)+"\n"+ + getCredString(user2, password2)) - "alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma\n" + - "bob:$2y$12$MZ9Wh44eZC4ck6TIftfM5.0QAQgpgvCTGDHQHgKCeqtxIx/OzJt6m\n", + twoCredTests = append(twoCredTests, getCredString(user1, password1)+"\n"+ + getCredString(user2, password2)+"\n") - "alice:$2y$12$wc6M7d7vTyFD3m9J2hTWPuM3q1gd1lM.8Epnpd8YheZCh6hx/vxma\n\n" + - "bob:$2y$12$MZ9Wh44eZC4ck6TIftfM5.0QAQgpgvCTGDHQHgKCeqtxIx/OzJt6m\n\n", - } + twoCredTests = append(twoCredTests, getCredString(user1, password1)+"\n\n"+ + getCredString(user2, password2)+"\n\n") - for _, testString := range twoCredtests { + for _, testString := range twoCredTests { func() { config := api.NewConfig() config.HTTP.Port = SecurePort1 @@ -202,11 +219,11 @@ func TestHtpasswdTwoCreds(t *testing.T) { _ = controller.Server.Shutdown(ctx) }(c) // with creds, should get expected status code - resp, _ := resty.R().SetBasicAuth("alice", "alice").Get(BaseURL1 + "/v2/") + resp, _ := resty.R().SetBasicAuth(user1, password1).Get(BaseURL1 + "/v2/") So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, 200) - resp, _ = resty.R().SetBasicAuth("bob", "bob").Get(BaseURL1 + "/v2/") + resp, _ = resty.R().SetBasicAuth(user2, password2).Get(BaseURL1 + "/v2/") So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, 200) @@ -218,6 +235,70 @@ func TestHtpasswdTwoCreds(t *testing.T) { } }) } +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()