Merge pull request #12016 from reasonerjt/project-name-401-1.10

Change to repositories api

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
 	}
-
+	project, err := b.ProjectMgr.Get(projectIDOrName)
-	if projectName != "" {
+	if err != nil {
-		project, err := b.ProjectMgr.Get(projectName)
+		return false, err
-		if err != nil {
+	}
-			return false, err
+	if project == nil {
-		}
+		return false, errNotFound
-		if project == nil {
-			return false, errNotFound
-		}
-
-		projectID = project.ProjectID
 	}
 
-	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.SendPermissionError()
-			b.SendUnAuthorizedError(errors.New("UnAuthorized"))
-		} else {
-			b.SendForbiddenError(errors.New(b.SecurityCtx.GetUsername()))
-		}
-
 		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

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
 	}
 

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)

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
 		}
 

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
 	}

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------------------------//
+	// -------------------case 1 : response code = 404---------------------//
-	fmt.Println("case 1 : response code = 404,repo not found")
+	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 {

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 {