Change to repositories api

Update to repositories API so it will not differentiate if a project
does not exist or the user doesn't have permission to access it.

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
This commit is contained in:
Daniel Jiang 2020-05-22 18:29:51 +08:00
parent d6cab4fb8e
commit d8336c4549
7 changed files with 53 additions and 83 deletions

View File

@ -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

View File

@ -69,7 +69,7 @@ func (m *MetadataAPI) Prepare() {
}
if project == nil {
m.SendNotFoundError(fmt.Errorf("project %d not found", id))
m.handleProjectNotFound(id)
return
}

View File

@ -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)

View File

@ -76,7 +76,7 @@ func (p *ProjectAPI) Prepare() {
}
if project == nil {
p.SendNotFoundError(fmt.Errorf("project %d not found", id))
p.handleProjectNotFound(id)
return
}

View File

@ -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
}

View File

@ -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 {

View File

@ -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 {