diff --git a/src/core/api/base.go b/src/core/api/base.go index 5139edfc2..bdc4c40e6 100644 --- a/src/core/api/base.go +++ b/src/core/api/base.go @@ -102,24 +102,19 @@ func (b *BaseController) RequireAuthenticated() bool { // HasProjectPermission returns true when the request has action permission on project subresource func (b *BaseController) HasProjectPermission(projectIDOrName interface{}, action rbac.Action, subresource ...rbac.Resource) (bool, error) { - projectID, projectName, err := utils.ParseProjectIDOrName(projectIDOrName) + _, _, err := utils.ParseProjectIDOrName(projectIDOrName) if err != nil { return false, err } - - if projectName != "" { - project, err := b.ProjectMgr.Get(projectName) - if err != nil { - return false, err - } - if project == nil { - return false, errNotFound - } - - projectID = project.ProjectID + project, err := b.ProjectMgr.Get(projectIDOrName) + if err != nil { + return false, err + } + if project == nil { + return false, errNotFound } - resource := rbac.NewProjectNamespace(projectID).Resource(subresource...) + resource := rbac.NewProjectNamespace(project.ProjectID).Resource(subresource...) if !b.SecurityCtx.Can(action, resource) { return false, nil } @@ -133,27 +128,39 @@ func (b *BaseController) RequireProjectAccess(projectIDOrName interface{}, actio hasPermission, err := b.HasProjectPermission(projectIDOrName, action, subresource...) if err != nil { if err == errNotFound { - b.SendNotFoundError(fmt.Errorf("project %v not found", projectIDOrName)) + b.handleProjectNotFound(projectIDOrName) } else { b.SendInternalServerError(err) } - return false } - if !hasPermission { - if !b.SecurityCtx.IsAuthenticated() { - b.SendUnAuthorizedError(errors.New("UnAuthorized")) - } else { - b.SendForbiddenError(errors.New(b.SecurityCtx.GetUsername())) - } - + b.SendPermissionError() return false } - return true } +// This should be called when a project is not found, if the caller is a system admin it returns 404. +// If it's regular user, it will render permission error +func (b *BaseController) handleProjectNotFound(projectIDOrName interface{}) { + if b.SecurityCtx.IsSysAdmin() { + b.SendNotFoundError(fmt.Errorf("project %v not found", projectIDOrName)) + } else { + b.SendPermissionError() + } +} + +// SendPermissionError is a shortcut for sending different http error based on authentication status. +func (b *BaseController) SendPermissionError() { + if !b.SecurityCtx.IsAuthenticated() { + b.SendUnAuthorizedError(errors.New("UnAuthorized")) + } else { + b.SendForbiddenError(errors.New(b.SecurityCtx.GetUsername())) + } + +} + // WriteJSONData writes the JSON data to the client. func (b *BaseController) WriteJSONData(object interface{}) { b.Data["json"] = object diff --git a/src/core/api/metadata.go b/src/core/api/metadata.go index 490e5be74..62f53858d 100644 --- a/src/core/api/metadata.go +++ b/src/core/api/metadata.go @@ -69,7 +69,7 @@ func (m *MetadataAPI) Prepare() { } if project == nil { - m.SendNotFoundError(fmt.Errorf("project %d not found", id)) + m.handleProjectNotFound(id) return } diff --git a/src/core/api/metadata_test.go b/src/core/api/metadata_test.go index 97dd59181..2fd6d823a 100644 --- a/src/core/api/metadata_test.go +++ b/src/core/api/metadata_test.go @@ -65,10 +65,10 @@ func TestValidateProjectMetadata(t *testing.T) { func TestMetaAPI(t *testing.T) { client := newHarborAPI() - // non-exist project + // non-exist project, it should return 401 if user is not logged in. code, _, err := client.PostMeta(*unknownUsr, int64(1000), nil) require.Nil(t, err) - assert.Equal(t, http.StatusNotFound, code) + assert.Equal(t, http.StatusUnauthorized, code) // non-login code, _, err = client.PostMeta(*unknownUsr, int64(1), nil) diff --git a/src/core/api/project.go b/src/core/api/project.go index 791d0834d..0fee84112 100644 --- a/src/core/api/project.go +++ b/src/core/api/project.go @@ -76,7 +76,7 @@ func (p *ProjectAPI) Prepare() { } if project == nil { - p.SendNotFoundError(fmt.Errorf("project %d not found", id)) + p.handleProjectNotFound(id) return } diff --git a/src/core/api/repository.go b/src/core/api/repository.go index 9764a286a..4f6e8bce1 100755 --- a/src/core/api/repository.go +++ b/src/core/api/repository.go @@ -103,18 +103,6 @@ func (ra *RepositoryAPI) Get() { return } - exist, err := ra.ProjectMgr.Exists(projectID) - if err != nil { - ra.ParseAndHandleError(fmt.Sprintf("failed to check the existence of project %d", - projectID), err) - return - } - - if !exist { - ra.SendNotFoundError(fmt.Errorf("project %d not found", projectID)) - return - } - if !ra.RequireProjectAccess(projectID, rbac.ActionList, rbac.ResourceRepository) { return } @@ -411,6 +399,10 @@ func (ra *RepositoryAPI) Delete() { func (ra *RepositoryAPI) GetTag() { repository := ra.GetString(":splat") tag := ra.GetString(":tag") + projectName, _ := utils.ParseRepository(repository) + if !ra.RequireProjectAccess(projectName, rbac.ActionRead, rbac.ResourceRepositoryTag) { + return + } exist, _, err := ra.checkExistence(repository, tag) if err != nil { ra.SendInternalServerError(fmt.Errorf("failed to check the existence of resource, error: %v", err)) @@ -421,11 +413,6 @@ func (ra *RepositoryAPI) GetTag() { return } - projectName, _ := utils.ParseRepository(repository) - if !ra.RequireProjectAccess(projectName, rbac.ActionRead, rbac.ResourceRepositoryTag) { - return - } - project, err := ra.ProjectMgr.Get(projectName) if err != nil { ra.ParseAndHandleError(fmt.Sprintf("failed to get the project %s", @@ -559,6 +546,9 @@ func (ra *RepositoryAPI) GetTags() { } projectName, _ := utils.ParseRepository(repoName) + if !ra.RequireProjectAccess(projectName, rbac.ActionList, rbac.ResourceRepositoryTag) { + return + } project, err := ra.ProjectMgr.Get(projectName) if err != nil { ra.ParseAndHandleError(fmt.Sprintf("failed to get the project %s", @@ -571,10 +561,6 @@ func (ra *RepositoryAPI) GetTags() { return } - if !ra.RequireProjectAccess(projectName, rbac.ActionList, rbac.ResourceRepositoryTag) { - return - } - client, err := coreutils.NewRepositoryClientForUI(ra.SecurityCtx.GetUsername(), repoName) if err != nil { log.Errorf("error occurred while initializing repository client for %s: %v", repoName, err) @@ -853,18 +839,6 @@ func (ra *RepositoryAPI) GetManifests() { } projectName, _ := utils.ParseRepository(repoName) - exist, err := ra.ProjectMgr.Exists(projectName) - if err != nil { - ra.ParseAndHandleError(fmt.Sprintf("failed to check the existence of project %s", - projectName), err) - return - } - - if !exist { - ra.SendNotFoundError(fmt.Errorf("project %s not found", projectName)) - return - } - if !ra.RequireProjectAccess(projectName, rbac.ActionRead, rbac.ResourceRepositoryTagManifest) { return } @@ -1011,18 +985,6 @@ func (ra *RepositoryAPI) GetSignatures() { repoName := ra.GetString(":splat") projectName, _ := utils.ParseRepository(repoName) - exist, err := ra.ProjectMgr.Exists(projectName) - if err != nil { - ra.ParseAndHandleError(fmt.Sprintf("failed to check the existence of project %s", - projectName), err) - return - } - - if !exist { - ra.SendNotFoundError(fmt.Errorf("project %s not found", projectName)) - return - } - if !ra.RequireProjectAccess(projectName, rbac.ActionRead, rbac.ResourceRepository) { return } diff --git a/src/core/api/repository_test.go b/src/core/api/repository_test.go index f52d5758a..6acc27b35 100644 --- a/src/core/api/repository_test.go +++ b/src/core/api/repository_test.go @@ -48,11 +48,11 @@ func TestGetRepos(t *testing.T) { } // -------------------case 2 : response code = 404------------------------// - fmt.Println("case 2 : response code = 404:project not found") + fmt.Println("case 2 : response code = 404: project not found") projectID = "111" httpStatusCode, _, err := apiTest.GetRepos(*admin, projectID, keyword) if err != nil { - t.Error("Error whihle get repos by projectID", err.Error()) + t.Error("Error while get repos by projectID", err.Error()) t.Log(err) } else { assert.Equal(int(404), httpStatusCode, "httpStatusCode should be 404") @@ -77,8 +77,8 @@ func TestGetReposTags(t *testing.T) { assert := assert.New(t) apiTest := newHarborAPI() - // -------------------case 1 : response code = 404------------------------// - fmt.Println("case 1 : response code = 404,repo not found") + // -------------------case 1 : response code = 404---------------------// + fmt.Println("case 1 : response code = 404") repository := "errorRepos" code, _, err := apiTest.GetReposTags(*admin, repository) if err != nil { @@ -155,7 +155,7 @@ func TestGetReposManifests(t *testing.T) { } // -------------------case 3 : response code = 404------------------------// - fmt.Println("case 3 : response code = 404,repo not found") + fmt.Println("case 3 : response code = 404") repoName = "111" httpStatusCode, err = apiTest.GetReposManifests(*admin, repoName, tag) if err != nil { diff --git a/src/core/api/scan.go b/src/core/api/scan.go index 0a9a26eab..e6c1352b4 100644 --- a/src/core/api/scan.go +++ b/src/core/api/scan.go @@ -21,6 +21,7 @@ import ( "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils" + "github.com/goharbor/harbor/src/common/utils/log" coreutils "github.com/goharbor/harbor/src/core/utils" "github.com/goharbor/harbor/src/jobservice/logger" "github.com/goharbor/harbor/src/pkg/scan/api/scan" @@ -47,6 +48,10 @@ func (sa *ScanAPI) Prepare() { // Call super prepare method sa.BaseController.Prepare() + // Check authentication + if !sa.RequireAuthenticated() { + return + } // Parse parameters repoName := sa.GetString(":splat") tag := sa.GetString(":tag") @@ -58,16 +63,12 @@ func (sa *ScanAPI) Prepare() { return } if pro == nil { - sa.SendNotFoundError(errors.Errorf("project %s not found", projectName)) + log.Errorf("project %s not found", projectName) + sa.SendForbiddenError(errors.New(sa.SecurityCtx.GetUsername())) return } sa.pro = pro - // Check authentication - if !sa.RequireAuthenticated() { - return - } - // Assemble artifact object digest, err := digestFunc(repoName, tag, sa.SecurityCtx.GetUsername()) if err != nil {