artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 18:51:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

own: add API for removing an assigned owner. (#52691)

Browse Source 
Test plan:
GraphQL tests added.
This commit is contained in:
Alex Ostrikov 2023-06-01 16:58:17 +04:00 committed by GitHub
parent 4e2b7c1d14
commit 5f8711684a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 185 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cmd/frontend/graphqlbackend/own.go
View File

@ -61,6 +61,7 @@ type OwnResolver interface {
// Assigned ownership mutations.
AssignOwner(context.Context, *AssignOwnerArgs) (*EmptyResponse, error)
RemoveAssignedOwner(context.Context, *AssignOwnerArgs) (*EmptyResponse, error)
// Config.
OwnSignalConfigurations(ctx context.Context) ([]SignalConfigurationResolver, error)

6
cmd/frontend/graphqlbackend/own.graphql
View File

@ -249,6 +249,10 @@ extend type Mutation {
assignOwner creates a new assigned owner.
"""
assignOwner(input: AssignOwnerInput!): EmptyResponse
"""
removeAssignedOwner removes an assigned owner.
"""
removeAssignedOwner(input: AssignOwnerInput!): EmptyResponse
}
"""
@ -347,7 +351,7 @@ input UpdateSignalConfigurationsInput {
}
"""
AssignOwnerInput represents the input for assigning an owner.
AssignOwnerInput represents the input for assigning or deleting an owner.
"""
input AssignOwnerInput {
"""

36
enterprise/cmd/frontend/internal/own/resolvers/resolvers.go
View File

@ -777,6 +777,25 @@ func (r *ownResolver) AssignOwner(ctx context.Context, args *graphqlbackend.Assi
if actor.FromContext(ctx).IsInternal() {
return nil, nil
}
user, err := r.checkAssignedOwnershipPermission(ctx)
if err != nil {
return nil, err
}
u, err := unmarshalAssignOwnerArgs(args.Input)
if err != nil {
return nil, err
}
whoAssignedUserID := user.ID
err = r.db.AssignedOwners().Insert(ctx, u.AssignedOwnerID, u.RepoID, u.AbsolutePath, whoAssignedUserID)
if err != nil {
return nil, errors.Wrap(err, "creating assigned owner")
}
return &graphqlbackend.EmptyResponse{}, nil
}
// checkAssignedOwnershipPermission checks that the user from the context has
// `rbac.OwnershipAssignPermission` and returns the user if so.
func (r *ownResolver) checkAssignedOwnershipPermission(ctx context.Context) (*types.User, error) {
// Extracting the user to run an RBAC check and then use their ID as an
// `whoAssignedUserID`.
user, err := auth.CurrentUser(ctx, r.db)
@ -790,14 +809,25 @@ func (r *ownResolver) AssignOwner(ctx context.Context, args *graphqlbackend.Assi
if err := rbac.CheckGivenUserHasPermission(ctx, r.db, user, rbac.OwnershipAssignPermission); err != nil {
return nil, err
}
return user, nil
}
func (r *ownResolver) RemoveAssignedOwner(ctx context.Context, args *graphqlbackend.AssignOwnerArgs) (*graphqlbackend.EmptyResponse, error) {
// Internal actor is a no-op, only a user can remove an assigned owner.
if actor.FromContext(ctx).IsInternal() {
return nil, nil
}
_, err := r.checkAssignedOwnershipPermission(ctx)
if err != nil {
return nil, err
}
u, err := unmarshalAssignOwnerArgs(args.Input)
if err != nil {
return nil, err
}
whoAssignedUserID := user.ID
err = r.db.AssignedOwners().Insert(ctx, u.AssignedOwnerID, u.RepoID, u.AbsolutePath, whoAssignedUserID)
err = r.db.AssignedOwners().DeleteOwner(ctx, u.AssignedOwnerID, u.RepoID, u.AbsolutePath)
if err != nil {
return nil, errors.Wrap(err, "creating assigned owner")
return nil, errors.Wrap(err, "deleting assigned owner")
}
return &graphqlbackend.EmptyResponse{}, nil
}

155
enterprise/cmd/frontend/internal/own/resolvers/resolvers_test.go
View File

@ -1411,10 +1411,10 @@ func TestAssignOwner(t *testing.T) {
repo := types.Repo{Name: "test-repo-1", ID: 101}
err := db.Repos().Create(ctx, &repo)
require.NoError(t, err)
// Creating admin and non-admin users, only admin has rights to assign owners.
admin, err := db.Users().Create(context.Background(), database.NewUser{Username: "admin"})
// Creating 2 users, only "hasPermission" user has rights to assign owners.
hasPermission, err := db.Users().Create(context.Background(), database.NewUser{Username: "has-permission"})
require.NoError(t, err)
user, err := db.Users().Create(context.Background(), database.NewUser{Username: "non-admin"})
noPermission, err := db.Users().Create(context.Background(), database.NewUser{Username: "no-permission"})
require.NoError(t, err)
// RBAC stuff below.
permission, err := db.Permissions().Create(ctx, database.CreatePermissionOpts{
@ -1430,7 +1430,7 @@ func TestAssignOwner(t *testing.T) {
})
require.NoError(t, err)
err = db.UserRoles().Assign(ctx, database.AssignUserRoleOpts{
UserID: admin.ID,
UserID: hasPermission.ID,
RoleID: role.ID,
})
require.NoError(t, err)
@ -1440,8 +1440,8 @@ func TestAssignOwner(t *testing.T) {
t.Fatal(err)
}
adminCtx := actor.WithActor(ctx, actor.FromUser(admin.ID))
userCtx := actor.WithActor(ctx, actor.FromUser(user.ID))
adminCtx := actor.WithActor(ctx, actor.FromUser(hasPermission.ID))
userCtx := actor.WithActor(ctx, actor.FromUser(noPermission.ID))
getBaseTest := func() *graphqlbackend.Test {
return &graphqlbackend.Test{
@ -1454,7 +1454,7 @@ func TestAssignOwner(t *testing.T) {
}
}`,
Variables: map[string]any{"input": map[string]any{
"assignedOwnerID": string(graphqlbackend.MarshalUserID(user.ID)),
"assignedOwnerID": string(graphqlbackend.MarshalUserID(noPermission.ID)),
"repoID": string(graphqlbackend.MarshalRepositoryID(repo.ID)),
"absolutePath": "",
}},
@ -1512,7 +1512,7 @@ func TestAssignOwner(t *testing.T) {
"assignedOwnerID": string(graphqlbackend.MarshalUserID(0)),
"repoID": string(graphqlbackend.MarshalRepositoryID(repo.ID)),
"absolutePath": "",
"whoAssignedUserID": string(graphqlbackend.MarshalUserID(admin.ID)),
"whoAssignedUserID": string(graphqlbackend.MarshalUserID(hasPermission.ID)),
}}
graphqlbackend.RunTest(t, baseTest)
assertNoAssignedOwners(t, repo.ID)
@ -1524,6 +1524,143 @@ func TestAssignOwner(t *testing.T) {
baseTest.Context = adminCtx
baseTest.ExpectedResult = `{"assignOwner":{"alwaysNil": null}}`
graphqlbackend.RunTest(t, baseTest)
assertAssignedOwner(t, user.ID, admin.ID, repo.ID, "")
assertAssignedOwner(t, noPermission.ID, hasPermission.ID, repo.ID, "")
})
}
func TestDeleteAssignedOwner(t *testing.T) {
logger := logtest.Scoped(t)
testDB := dbtest.NewDB(logger, t)
db := database.NewDB(logger, testDB)
git := fakeGitserver{}
own := fakeOwnService{}
ctx := context.Background()
repo := types.Repo{Name: "test-repo-1", ID: 101}
err := db.Repos().Create(ctx, &repo)
require.NoError(t, err)
// Creating 2 users, only "hasPermission" user has rights to assign owners.
hasPermission, err := db.Users().Create(context.Background(), database.NewUser{Username: "has-permission"})
require.NoError(t, err)
noPermission, err := db.Users().Create(context.Background(), database.NewUser{Username: "non-permission"})
require.NoError(t, err)
// Creating an existing assigned owner.
require.NoError(t, db.AssignedOwners().Insert(ctx, noPermission.ID, repo.ID, "", hasPermission.ID))
// RBAC stuff below.
permission, err := db.Permissions().Create(ctx, database.CreatePermissionOpts{
Namespace: rbactypes.OwnershipNamespace,
Action: rbactypes.OwnershipAssignAction,
})
require.NoError(t, err)
role, err := db.Roles().Create(ctx, "Can assign owners", false)
require.NoError(t, err)
err = db.RolePermissions().Assign(ctx, database.AssignRolePermissionOpts{
RoleID: role.ID,
PermissionID: permission.ID,
})
require.NoError(t, err)
err = db.UserRoles().Assign(ctx, database.AssignUserRoleOpts{
UserID: hasPermission.ID,
RoleID: role.ID,
})
require.NoError(t, err)
// RBAC stuff finished. Creating a GraphQL schema.
schema, err := graphqlbackend.NewSchema(db, git, nil, graphqlbackend.OptionalResolver{OwnResolver: resolvers.NewWithService(db, git, own, logger)})
if err != nil {
t.Fatal(err)
}
adminCtx := actor.WithActor(ctx, actor.FromUser(hasPermission.ID))
userCtx := actor.WithActor(ctx, actor.FromUser(noPermission.ID))
getBaseTest := func() *graphqlbackend.Test {
return &graphqlbackend.Test{
Context: userCtx,
Schema: schema,
Query: `
mutation removeAssignedOwner($input:AssignOwnerInput!) {
removeAssignedOwner(input:$input) {
alwaysNil
}
}`,
Variables: map[string]any{"input": map[string]any{
"assignedOwnerID": string(graphqlbackend.MarshalUserID(noPermission.ID)),
"repoID": string(graphqlbackend.MarshalRepositoryID(repo.ID)),
"absolutePath": "",
}},
}
}
assertOwnerExists := func(t *testing.T) {
t.Helper()
owners, err := db.AssignedOwners().ListAssignedOwnersForRepo(ctx, repo.ID)
require.NoError(t, err)
require.Len(t, owners, 1)
owner := owners[0]
assert.Equal(t, noPermission.ID, owner.OwnerUserID)
assert.Equal(t, hasPermission.ID, owner.WhoAssignedUserID)
assert.Equal(t, "", owner.FilePath)
}
assertNoAssignedOwners := func(t *testing.T) {
t.Helper()
owners, err := db.AssignedOwners().ListAssignedOwnersForRepo(ctx, repo.ID)
require.NoError(t, err)
require.Empty(t, owners)
}
t.Run("cannot delete assigned owner without permission", func(t *testing.T) {
baseTest := getBaseTest()
expectedErrs := []*gqlerrors.QueryError{{
Message: "user is missing permission OWNERSHIP#ASSIGN",
Path: []any{"removeAssignedOwner"},
}}
baseTest.ExpectedErrors = expectedErrs
baseTest.ExpectedResult = `{"removeAssignedOwner":null}`
graphqlbackend.RunTest(t, baseTest)
assertOwnerExists(t)
})
t.Run("bad request", func(t *testing.T) {
baseTest := getBaseTest()
baseTest.Context = adminCtx
expectedErrs := []*gqlerrors.QueryError{{
Message: "assigned user ID should not be 0",
Path: []any{"removeAssignedOwner"},
}}
baseTest.ExpectedErrors = expectedErrs
baseTest.ExpectedResult = `{"removeAssignedOwner":null}`
baseTest.Variables = map[string]any{"input": map[string]any{
"assignedOwnerID": string(graphqlbackend.MarshalUserID(0)),
"repoID": string(graphqlbackend.MarshalRepositoryID(repo.ID)),
"absolutePath": "",
}}
graphqlbackend.RunTest(t, baseTest)
assertOwnerExists(t)
})
t.Run("assigned owner not found", func(t *testing.T) {
baseTest := getBaseTest()
baseTest.Context = adminCtx
expectedErrs := []*gqlerrors.QueryError{{
Message: `deleting assigned owner: cannot delete assigned owner with ID=1337 for "" path for repo with ID=1`,
Path: []any{"removeAssignedOwner"},
}}
baseTest.ExpectedErrors = expectedErrs
baseTest.ExpectedResult = `{"removeAssignedOwner":null}`
baseTest.Variables = map[string]any{"input": map[string]any{
"assignedOwnerID": string(graphqlbackend.MarshalUserID(1337)),
"repoID": string(graphqlbackend.MarshalRepositoryID(repo.ID)),
"absolutePath": "",
}}
graphqlbackend.RunTest(t, baseTest)
assertOwnerExists(t)
})
t.Run("assigned owner successfully deleted", func(t *testing.T) {
baseTest := getBaseTest()
baseTest.Context = adminCtx
baseTest.ExpectedResult = `{"removeAssignedOwner":{"alwaysNil": null}}`
graphqlbackend.RunTest(t, baseTest)
assertNoAssignedOwners(t)
})
}