Infrastructure/zot
0
Fork 0
mirror of https://github.com/project-zot/zot.git synced 2025-01-06 22:40:28 -05:00
Code Issues Activity

fix: improvements after code review; try to fix tests

Browse source 
Signed-off-by: onidoru <onidoru@yahoo.com>
This commit is contained in:
onidoru 2024-01-31 09:56:29 +02:00 committed by Nikita K
parent 8696688a84
commit 7460a25564
11 changed files with 36 additions and 124 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
errors/errors.go
View file

@ -169,13 +169,11 @@ var (
ErrURLNotFound = errors.New("url not found")
ErrInvalidSearchQuery = errors.New("invalid search query")
// ErrUserIsNotFound returned if the user is not found.
ErrUserIsNotFound = errors.New("user is not found")
// ErrPasswordsDoNotMatch returned if given password does not match existing user's password.
ErrPasswordsDoNotMatch = errors.New("passwords do not match")
// ErrOldPasswordIsWrong returned if provided old password for user verification
// during the password change is wrong.
ErrOldPasswordIsWrong = errors.New("old password is wrong")
// ErrPasswordIsEmpty returned if user's new password is empty
// ErrPasswordIsEmpty returned if user's new password is empty.
ErrPasswordIsEmpty = errors.New("password can not be empty")
)

16
pkg/api/controller.go
View file

@ -50,7 +50,7 @@ type Controller struct {
CookieStore *CookieStore
LDAPClient *LDAPClient
taskScheduler *scheduler.Scheduler
htpasswdClient *HtpasswdClient
HtpasswdClient *HtpasswdClient
// runtime params
chosenPort int // kernel-chosen port
}
@ -100,9 +100,7 @@ func (c *Controller) Run() error {
return err
}
if err := c.initHtpasswdClient(); err != nil {
return err
}
c.StartBackgroundTasks()
// setup HTTP API router
engine := mux.NewRouter()
@ -246,6 +244,12 @@ func (c *Controller) Init() error {
c.InitCVEInfo()
if c.Config.IsHtpasswdAuthEnabled() {
if err := c.initHtpasswdClient(); err != nil {
return err
}
}
return nil
}
@ -285,9 +289,9 @@ func (c *Controller) initCookieStore() error {
func (c *Controller) initHtpasswdClient() error {
if c.Config.IsHtpasswdAuthEnabled() {
c.htpasswdClient = NewHtpasswdClient(c.Config.HTTP.Auth.HTPasswd.Path)
c.HtpasswdClient = NewHtpasswdClient(c.Config.HTTP.Auth.HTPasswd.Path)
return c.htpasswdClient.Init()
return c.HtpasswdClient.Init()
}
return nil

54
pkg/api/controller_test.go
View file

@ -4629,60 +4629,6 @@ func TestAuthorization(t *testing.T) {
})
}
func TestChangePassword(t *testing.T) {
Convey("Make a new controller", t, func() {
port := test.GetFreePort()
baseURL := test.GetBaseURL(port)
conf := config.New()
conf.HTTP.Port = port
username, seedUser := test.GenerateRandomString()
password, seedPass := test.GenerateRandomString()
htpasswdPath := test.MakeHtpasswdFileFromString(test.GetCredString(username, password))
defer os.Remove(htpasswdPath)
conf.HTTP.Auth = &config.AuthConfig{
HTPasswd: config.AuthHTPasswd{
Path: htpasswdPath,
},
}
conf.HTTP.AccessControl = &config.AccessControlConfig{
Repositories: config.Repositories{
test.AuthorizationAllRepos: config.PolicyGroup{
Policies: []config.Policy{
{
Users: []string{},
Actions: []string{},
},
},
DefaultPolicy: []string{},
},
},
AdminPolicy: config.Policy{
Users: []string{},
Actions: []string{},
},
}
Convey("with basic auth", func() {
ctlr := api.NewController(conf)
ctlr.Config.Storage.RootDirectory = t.TempDir()
err := WriteImageToFileSystem(CreateDefaultImage(), "zot-test", "0.0.1",
ociutils.GetDefaultStoreController(ctlr.Config.Storage.RootDirectory, ctlr.Log))
So(err, ShouldBeNil)
cm := test.NewControllerManager(ctlr)
cm.StartAndWait(port)
defer cm.StopServer()
client := resty.New()
client.SetBasicAuth(username, password)
RunAuthorizationTests(t, client, baseURL, username, conf)
})
})
}
func TestGetUsername(t *testing.T) {
Convey("Make a new controller", t, func() {
port := test.GetFreePort()

4
pkg/api/htpasswd.go
View file

@ -92,7 +92,7 @@ func (hc *HtpasswdClient) Set(login, password string) error {
func (hc *HtpasswdClient) CheckPassword(login, password string) error {
passwordHash, ok := hc.Get(login)
if !ok {
return zerr.ErrUserIsNotFound
return zerr.ErrBadUser
}
err := bcrypt.CompareHashAndPassword([]byte(passwordHash), []byte(password))
@ -115,7 +115,7 @@ func (hc *HtpasswdClient) ChangePassword(login, supposedOldPassword, newPassword
hc.credMap.rw.RUnlock()
if !ok {
return zerr.ErrUserIsNotFound
return zerr.ErrBadUser
}
// given old password must match actual old password

39
pkg/api/htpasswd_test.go
View file

@ -27,7 +27,7 @@ func TestHtpasswdClient_ChangePassword(t *testing.T) {
Convey("change for non-existing login", func() {
err := client.ChangePassword("non-existing", "old_password", "new_password")
So(err, ShouldEqual, zerr.ErrUserIsNotFound)
So(err, ShouldEqual, zerr.ErrBadUser)
})
Convey("change with wrong old oldPassword", func() {
@ -96,7 +96,7 @@ func TestHtpasswdClient_CheckPassword(t *testing.T) {
Convey("check for non-existing login", func() {
err := client.CheckPassword("non-existing", "password")
So(err, ShouldEqual, zerr.ErrUserIsNotFound)
So(err, ShouldEqual, zerr.ErrBadUser)
})
Convey("check with wrong password", func() {
@ -146,38 +146,3 @@ func TestHtpasswdClient_Init(t *testing.T) {
})
})
}
//
// func Test_credMap_Get(t *testing.T) {
// credsMap := credMap{
// m: map[string]string{"testuser": "testpassword"},
// rw: &sync.RWMutex{},
// }
//
// Convey("test credMap Get", t, func() {
// Convey("should get existing password", func() {
// passhprase, ok := credsMap.Get("testuser")
// So(ok, ShouldBeTrue)
// So(passhprase, ShouldEqual, "testpassword")
// })
//
// Convey("should not get unexisting password", func() {
// passhprase, ok := credsMap.Get("non-existing")
// So(ok, ShouldBeFalse)
// So(passhprase, ShouldBeBlank)
// })
// })
// }
//
// func Test_credMap_Set(t *testing.T) {
// credsMap := credMap{
// m: make(map[string]string),
// rw: &sync.RWMutex{},
// }
//
// Convey("should set password", t, func() {
// err := credsMap.Set("testuser", "testpassword")
// So(err, ShouldBeNil)
// So(credsMap.m["testuser"], ShouldNotBeEmpty)
// })
// }

17
pkg/api/routes.go
View file

@ -2234,7 +2234,6 @@ func (rh *RouteHandler) ChangePassword(resp http.ResponseWriter, req *http.Reque
if err != nil {
rh.c.Log.Error().Msg("failed to read req body")
resp.WriteHeader(http.StatusInternalServerError)
_, _ = resp.Write([]byte("internal server error"))
return
}
@ -2243,41 +2242,41 @@ func (rh *RouteHandler) ChangePassword(resp http.ResponseWriter, req *http.Reque
if err := json.Unmarshal(body, &reqBody); err != nil {
rh.c.Log.Error().Msg("failed to unmarshal req body")
resp.WriteHeader(http.StatusBadRequest)
_, _ = resp.Write([]byte("bad req"))
return
}
userAc, err := reqCtx.UserAcFromContext(req.Context())
if err != nil {
resp.WriteHeader(http.StatusNotFound)
return
}
username := userAc.GetUsername()
if err := rh.c.htpasswdClient.ChangePassword(username, reqBody.OldPassword, reqBody.NewPassword); err != nil {
if username == "" {
resp.WriteHeader(http.StatusNotFound)
}
if err := rh.c.HtpasswdClient.ChangePassword(username, reqBody.OldPassword, reqBody.NewPassword); err != nil {
rh.c.Log.Error().Err(err).Str("identity", username).Msg("failed to change user password")
status := http.StatusInternalServerError
msg := err.Error()
switch {
case errors.Is(err, zerr.ErrUserIsNotFound):
case errors.Is(err, zerr.ErrBadUser):
status = http.StatusNotFound
case errors.Is(err, zerr.ErrOldPasswordIsWrong):
status = http.StatusUnauthorized
case errors.Is(err, zerr.ErrPasswordIsEmpty):
status = http.StatusBadRequest
default:
msg = "internal server error"
}
resp.WriteHeader(status)
_, _ = resp.Write([]byte(msg))
return
}
resp.WriteHeader(http.StatusOK)
_, _ = resp.Write([]byte("password changed"))
}
type ChangePasswordRequest struct {

2
pkg/api/routes_test.go
View file

@ -1562,7 +1562,7 @@ func TestRoutes(t *testing.T) {
NewPassword: "new_password",
},
wantCode: http.StatusNotFound,
wantBody: []byte(zerr.ErrUserIsNotFound.Error()),
wantBody: []byte(zerr.ErrBadUser.Error()),
}))
Convey("old password is wrong", testFn(testCase{

18
pkg/test/image-utils/upload_test.go
View file

@ -524,28 +524,28 @@ func TestInjectUploadImageWithBasicAuth(t *testing.T) {
}
Convey("first marshal", func() {
injected := inject.InjectFailure(0)
if injected {
err := UploadImageWithBasicAuth(img, baseURL, "test", img.DigestStr(), "user", "password")
So(err, ShouldNotBeNil)
}
})
Convey("CreateBlobUpload POST call", func() {
injected := inject.InjectFailure(1)
if injected {
err := UploadImageWithBasicAuth(img, baseURL, "test", img.DigestStr(), "user", "password")
So(err, ShouldNotBeNil)
}
})
Convey("CreateBlobUpload POST call", func() {
injected := inject.InjectFailure(2)
if injected {
err := UploadImageWithBasicAuth(img, baseURL, "test", img.DigestStr(), "user", "password")
So(err, ShouldNotBeNil)
}
})
Convey("UpdateBlobUpload PUT call", func() {
injected := inject.InjectFailure(3)
injected := inject.InjectFailure(4)
if injected {
err := UploadImageWithBasicAuth(img, baseURL, "test", img.DigestStr(), "user", "password")
So(err, ShouldNotBeNil)
}
})
Convey("second marshal", func() {
injected := inject.InjectFailure(5)
injected := inject.InjectFailure(6)
if injected {
err := UploadImageWithBasicAuth(img, baseURL, "test", img.DigestStr(), "user", "password")
So(err, ShouldNotBeNil)

2
swagger/docs.go
View file

@ -1187,7 +1187,7 @@ const docTemplate = `{
}
},
"403": {
"description": "old password is incorrect",
"description": "old password is incorrect\".",
"schema": {
"type": "string"
}

2
swagger/swagger.json
View file

@ -1179,7 +1179,7 @@
}
},
"403": {
"description": "old password is incorrect",
"description": "old password is incorrect\".",
"schema": {
"type": "string"
}

2
swagger/swagger.yaml
View file

@ -1027,7 +1027,7 @@ paths:
schema:
type: string
"403":
description: old password is incorrect
description: old password is incorrect".
schema:
type: string
"500":