Merge pull request #9896 from heww/owner-check-for-project-member-robot-account

fix(robot,project-member): check owner of member, robot when update, …
This commit is contained in:
Wang Yan 2019-11-15 16:53:22 +08:00 committed by GitHub
commit 6e03c8a54e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 166 additions and 68 deletions

View File

@ -48,6 +48,11 @@ func (b *BaseAPI) GetInt64FromPath(key string) (int64, error) {
return strconv.ParseInt(value, 10, 64)
}
// ParamExistsInPath returns true when param exists in the path
func (b *BaseAPI) ParamExistsInPath(key string) bool {
return b.GetStringFromPath(key) != ""
}
// Render returns nil as it won't render template
func (b *BaseAPI) Render() error {
return nil

View File

@ -26,6 +26,7 @@ import (
"path/filepath"
"runtime"
"strconv"
"strings"
"github.com/astaxie/beego"
"github.com/dghubble/sling"
@ -247,19 +248,25 @@ func init() {
defer mockServer.Close()
}
func request(_sling *sling.Sling, acceptHeader string, authInfo ...usrInfo) (int, []byte, error) {
func request0(_sling *sling.Sling, acceptHeader string, authInfo ...usrInfo) (int, http.Header, []byte, error) {
_sling = _sling.Set("Accept", acceptHeader)
req, err := _sling.Request()
if err != nil {
return 400, nil, err
return 400, nil, nil, err
}
if len(authInfo) > 0 {
req.SetBasicAuth(authInfo[0].Name, authInfo[0].Passwd)
}
w := httptest.NewRecorder()
beego.BeeApp.Handlers.ServeHTTP(w, req)
body, err := ioutil.ReadAll(w.Body)
return w.Code, body, err
return w.Code, w.Header(), body, err
}
func request(_sling *sling.Sling, acceptHeader string, authInfo ...usrInfo) (int, []byte, error) {
code, _, body, err := request0(_sling, acceptHeader, authInfo...)
return code, body, err
}
// Search for projects and repositories
@ -543,23 +550,35 @@ func (a testapi) GetProjectMembersByProID(prjUsr usrInfo, projectID string) (int
}
// Add project role member accompany with projectID
// func (a testapi) AddProjectMember(prjUsr usrInfo, projectID string, roles apilib.RoleParam) (int, error) {
func (a testapi) AddProjectMember(prjUsr usrInfo, projectID string, member *models.MemberReq) (int, error) {
// func (a testapi) AddProjectMember(prjUsr usrInfo, projectID string, roles apilib.RoleParam) (int, int, error) {
func (a testapi) AddProjectMember(prjUsr usrInfo, projectID string, member *models.MemberReq) (int, int, error) {
_sling := sling.New().Post(a.basePath)
path := "/api/projects/" + projectID + "/members/"
_sling = _sling.Path(path)
_sling = _sling.BodyJSON(member)
httpStatusCode, _, err := request(_sling, jsonAcceptHeader, prjUsr)
return httpStatusCode, err
httpStatusCode, header, _, err := request0(_sling, jsonAcceptHeader, prjUsr)
var memberID int
location := header.Get("Location")
if location != "" {
parts := strings.Split(location, "/")
if len(parts) > 0 {
i, err := strconv.Atoi(parts[len(parts)-1])
if err == nil {
memberID = i
}
}
}
return httpStatusCode, memberID, err
}
// Delete project role member accompany with projectID
func (a testapi) DeleteProjectMember(authInfo usrInfo, projectID string, userID string) (int, error) {
func (a testapi) DeleteProjectMember(authInfo usrInfo, projectID string, memberID string) (int, error) {
_sling := sling.New().Delete(a.basePath)
path := "/api/projects/" + projectID + "/members/" + userID
path := "/api/projects/" + projectID + "/members/" + memberID
_sling = _sling.Path(path)
httpStatusCode, _, err := request(_sling, jsonAcceptHeader, authInfo)
return httpStatusCode, err

View File

@ -214,7 +214,8 @@ func TestListProjects(t *testing.T) {
},
}
projectID := strconv.Itoa(addPID)
httpStatusCode, err = apiTest.AddProjectMember(*admin, projectID, member)
var memberID int
httpStatusCode, memberID, err = apiTest.AddProjectMember(*admin, projectID, member)
if err != nil {
t.Error("Error whihle add project role member", err.Error())
t.Log(err)
@ -234,8 +235,7 @@ func TestListProjects(t *testing.T) {
assert.Equal("true", result[0].Metadata[models.ProMetaPublic], "Public is wrong")
assert.Equal(int32(2), result[0].CurrentUserRoleId, "User project role is wrong")
}
id := strconv.Itoa(CommonGetUserID())
httpStatusCode, err = apiTest.DeleteProjectMember(*admin, projectID, id)
httpStatusCode, err = apiTest.DeleteProjectMember(*admin, projectID, strconv.Itoa(memberID))
if err != nil {
t.Error("Error whihle add project role member", err.Error())
t.Log(err)

View File

@ -27,7 +27,6 @@ import (
"github.com/goharbor/harbor/src/common/dao/project"
"github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/rbac"
"github.com/goharbor/harbor/src/common/utils/log"
"github.com/goharbor/harbor/src/core/auth"
"github.com/goharbor/harbor/src/core/config"
)
@ -36,9 +35,8 @@ import (
type ProjectMemberAPI struct {
BaseController
id int
entityID int
entityType string
project *models.Project
member *models.Member
groupType int
}
@ -67,26 +65,38 @@ func (pma *ProjectMemberAPI) Prepare() {
pma.SendBadRequestError(errors.New(text))
return
}
project, err := pma.ProjectMgr.Get(pid)
pro, err := pma.ProjectMgr.Get(pid)
if err != nil {
pma.ParseAndHandleError(fmt.Sprintf("failed to get project %d", pid), err)
return
}
if project == nil {
if pro == nil {
pma.SendNotFoundError(fmt.Errorf("project %d not found", pid))
return
}
pma.project = project
pma.project = pro
if pma.ParamExistsInPath(":pmid") {
pmid, err := pma.GetInt64FromPath(":pmid")
if err != nil {
log.Warningf("Failed to get pmid from path, error %v", err)
}
if pmid <= 0 && (pma.Ctx.Input.IsPut() || pma.Ctx.Input.IsDelete()) {
if err != nil || pmid <= 0 {
pma.SendBadRequestError(fmt.Errorf("The project member id is invalid, pmid:%s", pma.GetStringFromPath(":pmid")))
return
}
pma.id = int(pmid)
members, err := project.GetProjectMember(models.Member{ProjectID: pid, ID: pma.id})
if err != nil {
pma.SendInternalServerError(err)
return
}
if len(members) == 0 {
pma.SendNotFoundError(fmt.Errorf("project member %d not found in project %d", pmid, pid))
return
}
pma.member = members[0]
}
authMode, err := config.AuthMode()
if err != nil {
pma.SendInternalServerError(fmt.Errorf("failed to get authentication mode"))
@ -123,22 +133,10 @@ func (pma *ProjectMemberAPI) Get() {
}
} else {
// return a specific member
queryMember.ID = pma.id
memberList, err := project.GetProjectMember(queryMember)
if err != nil {
pma.SendInternalServerError(fmt.Errorf("Failed to query database for member list, error: %v", err))
return
}
if len(memberList) == 0 {
pma.SendNotFoundError(fmt.Errorf("The project member does not exist, pmid:%v", pma.id))
return
}
if !pma.requireAccess(rbac.ActionRead) {
return
}
pma.Data["json"] = memberList[0]
pma.Data["json"] = pma.member
}
pma.ServeJSON()
}

View File

@ -268,6 +268,23 @@ func TestProjectMemberAPI_PutAndDelete(t *testing.T) {
if err != nil {
t.Errorf("Error occurred when add project member: %v", err)
}
projectID, err := dao.AddProject(models.Project{Name: "memberputanddelete", OwnerID: 1})
if err != nil {
t.Errorf("Error occurred when add project: %v", err)
}
defer dao.DeleteProject(projectID)
memberID, err := project.AddProjectMember(models.Member{
ProjectID: projectID,
Role: 1,
EntityID: int(userID),
EntityType: "u",
})
if err != nil {
t.Errorf("Error occurred when add project member: %v", err)
}
URL := fmt.Sprintf("/api/projects/1/members/%v", ID)
badURL := fmt.Sprintf("/api/projects/1/members/%v", 0)
cases := []*codeCheckingCase{
@ -330,6 +347,18 @@ func TestProjectMemberAPI_PutAndDelete(t *testing.T) {
},
code: http.StatusBadRequest,
},
// 404
{
request: &testingRequest{
method: http.MethodPut,
url: fmt.Sprintf("/api/projects/1/members/%v", memberID),
bodyJSON: &models.Member{
Role: 2,
},
credential: admin,
},
code: http.StatusNotFound,
},
// 200
{
request: &testingRequest{
@ -339,6 +368,18 @@ func TestProjectMemberAPI_PutAndDelete(t *testing.T) {
},
code: http.StatusOK,
},
// 404
{
request: &testingRequest{
method: http.MethodDelete,
url: fmt.Sprintf("/api/projects/1/members/%v", memberID),
bodyJSON: &models.Member{
Role: 2,
},
credential: admin,
},
code: http.StatusNotFound,
},
}
runCodeCheckingCases(t, cases...)

View File

@ -16,6 +16,9 @@ package api
import (
"fmt"
"net/http"
"strconv"
"github.com/goharbor/harbor/src/common/dao"
"github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/rbac"
@ -24,8 +27,6 @@ import (
"github.com/goharbor/harbor/src/pkg/robot"
"github.com/goharbor/harbor/src/pkg/robot/model"
"github.com/pkg/errors"
"net/http"
"strconv"
)
// RobotAPI ...
@ -40,7 +41,6 @@ type RobotAPI struct {
func (r *RobotAPI) Prepare() {
r.BaseController.Prepare()
method := r.Ctx.Request.Method
if !r.SecurityCtx.IsAuthenticated() {
r.SendUnAuthorizedError(errors.New("UnAuthorized"))
@ -70,10 +70,10 @@ func (r *RobotAPI) Prepare() {
r.project = project
r.ctr = robot.RobotCtr
if method == http.MethodPut || method == http.MethodDelete {
if r.ParamExistsInPath(":id") {
id, err := r.GetInt64FromPath(":id")
if err != nil || id <= 0 {
r.SendBadRequestError(errors.New("invalid robot ID"))
r.SendBadRequestError(fmt.Errorf("invalid robot ID %s", r.GetStringFromPath(":id")))
return
}
robot, err := r.ctr.GetRobotAccount(id)
@ -87,6 +87,11 @@ func (r *RobotAPI) Prepare() {
return
}
if robot.ProjectID != pid {
r.SendNotFoundError(fmt.Errorf("robot %d not found in project %d", id, pid))
return
}
r.robot = robot
}
}
@ -173,27 +178,7 @@ func (r *RobotAPI) Get() {
return
}
id, err := r.GetInt64FromPath(":id")
if err != nil || id <= 0 {
r.SendBadRequestError(fmt.Errorf("invalid robot ID: %s", r.GetStringFromPath(":id")))
return
}
robot, err := r.ctr.GetRobotAccount(id)
if err != nil {
r.SendInternalServerError(errors.Wrap(err, "robot API: get robot"))
return
}
if robot == nil {
r.SendNotFoundError(fmt.Errorf("robot API: robot %d not found", id))
return
}
if !robot.Visible {
r.SendForbiddenError(fmt.Errorf("robot API: robot %d is invisible", id))
return
}
r.Data["json"] = robot
r.Data["json"] = r.robot
r.ServeJSON()
}

View File

@ -19,6 +19,8 @@ import (
"net/http"
"testing"
"github.com/goharbor/harbor/src/common/dao"
"github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/rbac"
"github.com/goharbor/harbor/src/pkg/robot/model"
)
@ -163,6 +165,12 @@ func TestRobotAPIPost(t *testing.T) {
}
func TestRobotAPIGet(t *testing.T) {
projectID, err := dao.AddProject(models.Project{Name: "robotget", OwnerID: 1})
if err != nil {
t.Errorf("Error occurred when add project: %v", err)
}
defer dao.DeleteProject(projectID)
cases := []*codeCheckingCase{
// 400
{
@ -183,6 +191,16 @@ func TestRobotAPIGet(t *testing.T) {
code: http.StatusNotFound,
},
// 404 robot 1 not belong to the project
{
request: &testingRequest{
method: http.MethodGet,
url: fmt.Sprintf("/api/projects/%d/robots/1", projectID),
credential: sysAdmin,
},
code: http.StatusNotFound,
},
// 200
{
request: &testingRequest{
@ -251,6 +269,12 @@ func TestRobotAPIList(t *testing.T) {
}
func TestRobotAPIPut(t *testing.T) {
projectID, err := dao.AddProject(models.Project{Name: "robotput", OwnerID: 1})
if err != nil {
t.Errorf("Error occurred when add project: %v", err)
}
defer dao.DeleteProject(projectID)
cases := []*codeCheckingCase{
// 401
{
@ -281,6 +305,16 @@ func TestRobotAPIPut(t *testing.T) {
code: http.StatusNotFound,
},
// 404 robot 1 not belong to the project
{
request: &testingRequest{
method: http.MethodPut,
url: fmt.Sprintf("/api/projects/%d/robots/1", projectID),
credential: sysAdmin,
},
code: http.StatusNotFound,
},
// 403 non-member user
{
request: &testingRequest{
@ -319,6 +353,12 @@ func TestRobotAPIPut(t *testing.T) {
}
func TestRobotAPIDelete(t *testing.T) {
projectID, err := dao.AddProject(models.Project{Name: "robotdelete", OwnerID: 1})
if err != nil {
t.Errorf("Error occurred when add project: %v", err)
}
defer dao.DeleteProject(projectID)
cases := []*codeCheckingCase{
// 401
{
@ -349,6 +389,16 @@ func TestRobotAPIDelete(t *testing.T) {
code: http.StatusNotFound,
},
// 404 robot 1 not belong to the project
{
request: &testingRequest{
method: http.MethodDelete,
url: fmt.Sprintf("/api/projects/%d/robots/1", projectID),
credential: sysAdmin,
},
code: http.StatusNotFound,
},
// 403 non-member user
{
request: &testingRequest{