From 30d0094c431025abf3ff6e965db76494120db39a Mon Sep 17 00:00:00 2001 From: Wilfred Asomani Date: Mon, 26 Jun 2023 16:59:14 +0000 Subject: [PATCH] satellite/console: prevent unauthorized project mutation This change further restricts projects members from modifying project details by restricting the project edit graphql mutation; making it check if the user performing the operation is the owner of the project. Change-Id: Iaf10d16269ddc29437d3d5629db06e20cea3004e --- satellite/console/consoleweb/endpoints_test.go | 2 +- satellite/console/service.go | 3 +-- satellite/console/service_test.go | 13 +++++++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/satellite/console/consoleweb/endpoints_test.go b/satellite/console/consoleweb/endpoints_test.go index 00daf76d6..9d73e7596 100644 --- a/satellite/console/consoleweb/endpoints_test.go +++ b/satellite/console/consoleweb/endpoints_test.go @@ -870,7 +870,7 @@ func TestWrongUser(t *testing.T) { }`})) require.Contains(t, body, "not authorized") // TODO: wrong error code - require.Equal(t, http.StatusInternalServerError, resp.StatusCode) + require.Equal(t, http.StatusUnauthorized, resp.StatusCode) } { // get bucket usages diff --git a/satellite/console/service.go b/satellite/console/service.go index a76c779b0..adf25bbf8 100644 --- a/satellite/console/service.go +++ b/satellite/console/service.go @@ -1813,12 +1813,11 @@ func (s *Service) UpdateProject(ctx context.Context, projectID uuid.UUID, update return nil, Error.Wrap(err) } - isMember, err := s.isProjectMember(ctx, user.ID, projectID) + _, project, err := s.isProjectOwner(ctx, user.ID, projectID) if err != nil { return nil, Error.Wrap(err) } - project := isMember.project if updatedProject.Name != project.Name { passesNameCheck, err := s.checkProjectName(ctx, updatedProject, user.ID) if err != nil || !passesNameCheck { diff --git a/satellite/console/service_test.go b/satellite/console/service_test.go index 2de8a3bbb..6e71f2806 100644 --- a/satellite/console/service_test.go +++ b/satellite/console/service_test.go @@ -270,6 +270,19 @@ func TestService(t *testing.T) { }) require.Error(t, err) require.Nil(t, updatedProject) + + user2, userCtx2 := getOwnerAndCtx(ctx, up2Proj) + _, err = service.AddProjectMembers(userCtx1, up1Proj.ID, []string{user2.Email}) + require.NoError(t, err) + // Members should not be able to update project. + _, err = service.UpdateProject(userCtx2, up1Proj.ID, console.ProjectInfo{ + Name: updatedName, + }) + require.Error(t, err) + require.True(t, console.ErrUnauthorized.Has(err)) + // remove user2. + err = service.DeleteProjectMembersAndInvitations(userCtx1, up1Proj.ID, []string{user2.Email}) + require.NoError(t, err) }) t.Run("AddProjectMembers", func(t *testing.T) {