mirror of
https://github.com/sourcegraph/sourcegraph.git
synced 2026-02-06 19:51:50 +00:00
rbac: create users and assign role in single transaction (#47700)
This is a follow-up to [this comment](https://github.com/sourcegraph/sourcegraph/pull/47406#discussion_r1106606664) on [#47406](https://github.com/sourcegraph/sourcegraph/pull/47406). This ensures that we don't have `zombie` users (users without any role assigned to them) on a Sourcegraph instance. ## Test plan <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> * Manually tested * Updated unit tests
This commit is contained in:
parent
4078dcc092
commit
c45b4763f3
@ -58,52 +58,32 @@ func (r *schemaResolver) CreateUser(ctx context.Context, args *struct {
|
||||
}
|
||||
}
|
||||
|
||||
var user *types.User
|
||||
err := r.db.WithTransact(ctx, func(tx database.DB) (err error) {
|
||||
user, err = tx.Users().Create(ctx, database.NewUser{
|
||||
Username: args.Username,
|
||||
Password: backend.MakeRandomHardToGuessPassword(),
|
||||
user, err := r.db.Users().Create(ctx, database.NewUser{
|
||||
Username: args.Username,
|
||||
Password: backend.MakeRandomHardToGuessPassword(),
|
||||
|
||||
Email: email,
|
||||
Email: email,
|
||||
|
||||
// In order to mark an email as unverified, we must generate a verification code.
|
||||
EmailIsVerified: !needsEmailVerification,
|
||||
EmailVerificationCode: emailVerificationCode,
|
||||
})
|
||||
if err != nil {
|
||||
msg := "failed to create user"
|
||||
logger.Error(msg, log.Error(err))
|
||||
return errors.Wrap(err, msg)
|
||||
}
|
||||
|
||||
logger = logger.With(log.Int32("userID", user.ID))
|
||||
logger.Debug("user created")
|
||||
|
||||
roles := []types.SystemRole{types.UserSystemRole}
|
||||
if user.SiteAdmin {
|
||||
roles = append(roles, types.SiteAdministratorSystemRole)
|
||||
}
|
||||
opts := database.BulkAssignSystemRolesToUserOpts{UserID: user.ID, Roles: roles}
|
||||
if err = tx.UserRoles().BulkAssignSystemRolesToUser(ctx, opts); err != nil {
|
||||
r.logger.Error("failed to assign system roles to user",
|
||||
log.Error(err))
|
||||
return errors.Wrap(err, "failed to assign system roles to user")
|
||||
}
|
||||
|
||||
if err = tx.Authz().GrantPendingPermissions(ctx, &database.GrantPendingPermissionsArgs{
|
||||
UserID: user.ID,
|
||||
Perm: authz.Read,
|
||||
Type: authz.PermRepos,
|
||||
}); err != nil {
|
||||
r.logger.Error("failed to grant user pending permissions",
|
||||
log.Error(err))
|
||||
}
|
||||
|
||||
return nil
|
||||
// In order to mark an email as unverified, we must generate a verification code.
|
||||
EmailIsVerified: !needsEmailVerification,
|
||||
EmailVerificationCode: emailVerificationCode,
|
||||
})
|
||||
|
||||
if err != nil {
|
||||
return nil, err
|
||||
msg := "failed to create user"
|
||||
logger.Error(msg, log.Error(err))
|
||||
return nil, errors.Wrap(err, msg)
|
||||
}
|
||||
|
||||
logger = logger.With(log.Int32("userID", user.ID))
|
||||
logger.Debug("user created")
|
||||
|
||||
if err = r.db.Authz().GrantPendingPermissions(ctx, &database.GrantPendingPermissionsArgs{
|
||||
UserID: user.ID,
|
||||
Perm: authz.Read,
|
||||
Type: authz.PermRepos,
|
||||
}); err != nil {
|
||||
r.logger.Error("failed to grant user pending permissions",
|
||||
log.Error(err))
|
||||
}
|
||||
|
||||
return &createUserResult{
|
||||
|
||||
@ -20,7 +20,7 @@ import (
|
||||
type mockFuncs struct {
|
||||
dB *database.MockDB
|
||||
authzStore *database.MockAuthzStore
|
||||
userRoleStore *database.MockUserRoleStore
|
||||
usersStore *database.MockUserStore
|
||||
userEmailStore *database.MockUserEmailsStore
|
||||
}
|
||||
|
||||
@ -34,24 +34,17 @@ func makeUsersCreateTestDB(t *testing.T) mockFuncs {
|
||||
authz := database.NewMockAuthzStore()
|
||||
authz.GrantPendingPermissionsFunc.SetDefaultReturn(nil)
|
||||
|
||||
userRoles := database.NewMockUserRoleStore()
|
||||
userRoles.BulkAssignSystemRolesToUserFunc.SetDefaultReturn(nil)
|
||||
|
||||
userEmails := database.NewMockUserEmailsStore()
|
||||
|
||||
db := database.NewMockDB()
|
||||
db.WithTransactFunc.SetDefaultHook(func(ctx context.Context, f func(database.DB) error) error {
|
||||
return f(db)
|
||||
})
|
||||
db.UsersFunc.SetDefaultReturn(users)
|
||||
db.AuthzFunc.SetDefaultReturn(authz)
|
||||
db.UserRolesFunc.SetDefaultReturn(userRoles)
|
||||
db.UserEmailsFunc.SetDefaultReturn(userEmails)
|
||||
|
||||
return mockFuncs{
|
||||
dB: db,
|
||||
usersStore: users,
|
||||
authzStore: authz,
|
||||
userRoleStore: userRoles,
|
||||
userEmailStore: userEmails,
|
||||
}
|
||||
}
|
||||
@ -84,7 +77,7 @@ func TestCreateUser(t *testing.T) {
|
||||
})
|
||||
|
||||
mockrequire.CalledOnce(t, mocks.authzStore.GrantPendingPermissionsFunc)
|
||||
mockrequire.CalledOnce(t, mocks.userRoleStore.BulkAssignSystemRolesToUserFunc)
|
||||
mockrequire.CalledOnce(t, mocks.usersStore.CreateFunc)
|
||||
}
|
||||
|
||||
func TestCreateUserResetPasswordURL(t *testing.T) {
|
||||
|
||||
@ -165,71 +165,46 @@ func handleSignUp(logger log.Logger, db database.DB, w http.ResponseWriter, r *h
|
||||
}
|
||||
}
|
||||
|
||||
var usr *types.User
|
||||
err := db.WithTransact(r.Context(), func(tx database.DB) (err error) {
|
||||
usr, err = tx.Users().Create(r.Context(), newUserData)
|
||||
if err != nil {
|
||||
var (
|
||||
message string
|
||||
statusCode int
|
||||
)
|
||||
switch {
|
||||
case database.IsUsernameExists(err):
|
||||
message = "Username is already in use. Try a different username."
|
||||
statusCode = http.StatusConflict
|
||||
case database.IsEmailExists(err):
|
||||
message = "Email address is already in use. Try signing into that account instead, or use a different email address."
|
||||
statusCode = http.StatusConflict
|
||||
case errcode.PresentationMessage(err) != "":
|
||||
message = errcode.PresentationMessage(err)
|
||||
statusCode = http.StatusConflict
|
||||
default:
|
||||
// Do not show non-allowed error messages to user, in case they contain sensitive or confusing
|
||||
// information.
|
||||
message = defaultErrorMessage
|
||||
statusCode = http.StatusInternalServerError
|
||||
}
|
||||
logger.Error("Error in user signup.", log.String("email", creds.Email), log.String("username", creds.Username), log.Error(err))
|
||||
http.Error(w, message, statusCode)
|
||||
|
||||
if err = usagestats.LogBackendEvent(db, sgactor.FromContext(r.Context()).UID, deviceid.FromContext(r.Context()), "SignUpFailed", nil, nil, featureflag.GetEvaluatedFlagSet(r.Context()), nil); err != nil {
|
||||
logger.Warn("Failed to log event SignUpFailed", log.Error(err))
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
roles := []types.SystemRole{types.UserSystemRole}
|
||||
if usr.SiteAdmin {
|
||||
roles = append(roles, types.SiteAdministratorSystemRole)
|
||||
}
|
||||
|
||||
if err = tx.UserRoles().BulkAssignSystemRolesToUser(r.Context(), database.BulkAssignSystemRolesToUserOpts{
|
||||
UserID: usr.ID,
|
||||
Roles: roles,
|
||||
}); err != nil {
|
||||
logger.Error("Error assigning role to user", log.String("email", creds.Email), log.String("username", creds.Username), log.Error(err))
|
||||
http.Error(w, "Unable to assign system role to user", http.StatusBadRequest)
|
||||
return err
|
||||
}
|
||||
|
||||
if err = tx.Authz().GrantPendingPermissions(r.Context(), &database.GrantPendingPermissionsArgs{
|
||||
UserID: usr.ID,
|
||||
Perm: authz.Read,
|
||||
Type: authz.PermRepos,
|
||||
}); err != nil {
|
||||
logger.Error("Failed to grant user pending permissions", log.Int32("userID", usr.ID), log.Error(err))
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
|
||||
usr, err := db.Users().Create(r.Context(), newUserData)
|
||||
if err != nil {
|
||||
// It's okay to simply return here because any error that occur in the transaction above,
|
||||
// will result in `http.Error` been called to respond to the request early. This will also
|
||||
// result in the transaction being rolled back.
|
||||
var (
|
||||
message string
|
||||
statusCode int
|
||||
)
|
||||
switch {
|
||||
case database.IsUsernameExists(err):
|
||||
message = "Username is already in use. Try a different username."
|
||||
statusCode = http.StatusConflict
|
||||
case database.IsEmailExists(err):
|
||||
message = "Email address is already in use. Try signing into that account instead, or use a different email address."
|
||||
statusCode = http.StatusConflict
|
||||
case errcode.PresentationMessage(err) != "":
|
||||
message = errcode.PresentationMessage(err)
|
||||
statusCode = http.StatusConflict
|
||||
default:
|
||||
// Do not show non-allowed error messages to user, in case they contain sensitive or confusing
|
||||
// information.
|
||||
message = defaultErrorMessage
|
||||
statusCode = http.StatusInternalServerError
|
||||
}
|
||||
logger.Error("Error in user signup.", log.String("email", creds.Email), log.String("username", creds.Username), log.Error(err))
|
||||
http.Error(w, message, statusCode)
|
||||
|
||||
if err = usagestats.LogBackendEvent(db, sgactor.FromContext(r.Context()).UID, deviceid.FromContext(r.Context()), "SignUpFailed", nil, nil, featureflag.GetEvaluatedFlagSet(r.Context()), nil); err != nil {
|
||||
logger.Warn("Failed to log event SignUpFailed", log.Error(err))
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if err = db.Authz().GrantPendingPermissions(r.Context(), &database.GrantPendingPermissionsArgs{
|
||||
UserID: usr.ID,
|
||||
Perm: authz.Read,
|
||||
Type: authz.PermRepos,
|
||||
}); err != nil {
|
||||
logger.Error("Failed to grant user pending permissions", log.Int32("userID", usr.ID), log.Error(err))
|
||||
}
|
||||
|
||||
if conf.EmailVerificationRequired() && !newUserData.EmailIsVerified {
|
||||
if err := backend.SendUserEmailVerificationEmail(r.Context(), usr.Username, creds.Email, newUserData.EmailVerificationCode); err != nil {
|
||||
logger.Error("failed to send email verification (continuing, user's email will be unverified)", log.String("email", creds.Email), log.Error(err))
|
||||
|
||||
@ -10,7 +10,6 @@ import (
|
||||
"time"
|
||||
|
||||
mockrequire "github.com/derision-test/go-mockgen/testutil/require"
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
@ -425,19 +424,6 @@ func TestHandleSignUp(t *testing.T) {
|
||||
return &types.User{ID: 1, SiteAdmin: false, CreatedAt: time.Now()}, nil
|
||||
})
|
||||
|
||||
userRoles := database.NewMockUserRoleStore()
|
||||
userRoles.BulkAssignSystemRolesToUserFunc.SetDefaultHook(func(ctx context.Context, basrtuo database.BulkAssignSystemRolesToUserOpts) error {
|
||||
if len(basrtuo.Roles) != 1 {
|
||||
t.Fatalf("expected UserRoles().BulkAssignSystemRolesToUser to be called with one role, got %d", len(basrtuo.Roles))
|
||||
}
|
||||
|
||||
if basrtuo.Roles[0] != types.UserSystemRole {
|
||||
t.Fatalf("expected UserRoles().BulkAssignSystemRolesToUser to be called with %s role, got %s", types.UserSystemRole, basrtuo.Roles[0])
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
|
||||
authz := database.NewMockAuthzStore()
|
||||
authz.GrantPendingPermissionsFunc.SetDefaultReturn(nil)
|
||||
|
||||
@ -449,7 +435,6 @@ func TestHandleSignUp(t *testing.T) {
|
||||
return f(db)
|
||||
})
|
||||
db.UsersFunc.SetDefaultReturn(users)
|
||||
db.UserRolesFunc.SetDefaultReturn(userRoles)
|
||||
db.AuthzFunc.SetDefaultReturn(authz)
|
||||
db.EventLogsFunc.SetDefaultReturn(eventLogs)
|
||||
|
||||
@ -477,7 +462,6 @@ func TestHandleSignUp(t *testing.T) {
|
||||
|
||||
mockrequire.CalledOnce(t, authz.GrantPendingPermissionsFunc)
|
||||
mockrequire.CalledOnce(t, users.CreateFunc)
|
||||
mockrequire.CalledOnce(t, userRoles.BulkAssignSystemRolesToUserFunc)
|
||||
})
|
||||
}
|
||||
|
||||
@ -516,21 +500,6 @@ func TestHandleSiteInit(t *testing.T) {
|
||||
return &types.User{ID: 1, SiteAdmin: true, CreatedAt: time.Now()}, nil
|
||||
})
|
||||
|
||||
userRoles := database.NewMockUserRoleStore()
|
||||
userRoles.BulkAssignSystemRolesToUserFunc.SetDefaultHook(func(ctx context.Context, opts database.BulkAssignSystemRolesToUserOpts) error {
|
||||
if len(opts.Roles) != 2 {
|
||||
t.Fatalf("expected UserRoles().BulkAssignSystemRolesToUser to be called with two system roles, got %d", len(opts.Roles))
|
||||
}
|
||||
|
||||
want := []types.SystemRole{types.UserSystemRole, types.SiteAdministratorSystemRole}
|
||||
have := opts.Roles
|
||||
if diff := cmp.Diff(want, have); diff != "" {
|
||||
t.Fatalf("Mismatch (-want +got):\n%s", diff)
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
|
||||
authz := database.NewMockAuthzStore()
|
||||
authz.GrantPendingPermissionsFunc.SetDefaultReturn(nil)
|
||||
|
||||
@ -542,7 +511,6 @@ func TestHandleSiteInit(t *testing.T) {
|
||||
return f(db)
|
||||
})
|
||||
db.UsersFunc.SetDefaultReturn(users)
|
||||
db.UserRolesFunc.SetDefaultReturn(userRoles)
|
||||
db.AuthzFunc.SetDefaultReturn(authz)
|
||||
db.EventLogsFunc.SetDefaultReturn(eventLogs)
|
||||
|
||||
@ -570,7 +538,6 @@ func TestHandleSiteInit(t *testing.T) {
|
||||
|
||||
mockrequire.CalledOnce(t, authz.GrantPendingPermissionsFunc)
|
||||
mockrequire.CalledOnce(t, users.CreateFunc)
|
||||
mockrequire.CalledOnce(t, userRoles.BulkAssignSystemRolesToUserFunc)
|
||||
mockrequire.CalledOnce(t, eventLogs.BulkInsertFunc)
|
||||
})
|
||||
}
|
||||
|
||||
@ -193,15 +193,6 @@ func dbAddUserAction(cmd *cli.Context) error {
|
||||
return err
|
||||
}
|
||||
|
||||
// tx.Users().SetIsSiteAdmin assigns the `SITE_ADMINISTRATOR` role to the created user, we also need to
|
||||
// assign the `USER` role to the created user.
|
||||
if err = tx.UserRoles().AssignSystemRole(ctx, database.AssignSystemRoleOpts{
|
||||
UserID: user.ID,
|
||||
Role: types.UserSystemRole,
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Report back the new user information.
|
||||
std.Out.WriteSuccessf(
|
||||
// the space after the last %s is so the user can select the password easily in the shell to copy it.
|
||||
|
||||
@ -74,7 +74,7 @@ func TestAddSourcegraphOperatorExternalAccount(t *testing.T) {
|
||||
assert func(t *testing.T, uid int32, db database.DB)
|
||||
}{
|
||||
{
|
||||
name: "user is not a side admin",
|
||||
name: "user is not a site admin",
|
||||
setup: func(t *testing.T) (int32, database.DB) {
|
||||
providers.MockProviders = []providers.Provider{soap}
|
||||
t.Cleanup(func() { providers.MockProviders = nil })
|
||||
@ -127,6 +127,12 @@ func TestAddSourcegraphOperatorExternalAccount(t *testing.T) {
|
||||
|
||||
logger := logtest.NoOp(t)
|
||||
db := database.NewDB(logger, dbtest.NewDB(logger, t))
|
||||
|
||||
// We ensure the GlobalState is initialized so that the first user isn't
|
||||
// a site administrator.
|
||||
_, err := db.GlobalState().EnsureInitialized(ctx)
|
||||
require.NoError(t, err)
|
||||
|
||||
u, err := db.Users().Create(
|
||||
ctx,
|
||||
database.NewUser{
|
||||
@ -134,8 +140,10 @@ func TestAddSourcegraphOperatorExternalAccount(t *testing.T) {
|
||||
},
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
err = db.Users().SetIsSiteAdmin(ctx, u.ID, true)
|
||||
require.NoError(t, err)
|
||||
|
||||
return u.ID, db
|
||||
},
|
||||
accountDetails: &accountDetailsBody{
|
||||
@ -174,6 +182,12 @@ func TestAddSourcegraphOperatorExternalAccount(t *testing.T) {
|
||||
|
||||
logger := logtest.NoOp(t)
|
||||
db := database.NewDB(logger, dbtest.NewDB(logger, t))
|
||||
|
||||
// We ensure the GlobalState is initialized so that the first user isn't
|
||||
// a site administrator.
|
||||
_, err := db.GlobalState().EnsureInitialized(ctx)
|
||||
require.NoError(t, err)
|
||||
|
||||
u, err := db.Users().Create(
|
||||
ctx,
|
||||
database.NewUser{
|
||||
|
||||
@ -262,22 +262,6 @@ func (s *userExternalAccountsStore) CreateUserAndSave(ctx context.Context, newUs
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Every user on a Sourcegraph instance is assigned the `USER` role.
|
||||
roles := []types.SystemRole{types.UserSystemRole}
|
||||
if createdUser.SiteAdmin {
|
||||
// if the created user is a site admin, assign them the SITE_ADMINISTRATOR role.
|
||||
roles = append(roles, types.SiteAdministratorSystemRole)
|
||||
}
|
||||
|
||||
// We use the BulkAssignSystemRolesToUser method here because in cases where the created
|
||||
// user is also a site admin, we want to assign them both USER and SITE_ADMINISTRATOR roles.
|
||||
if err := UserRolesWith(tx).BulkAssignSystemRolesToUser(ctx, BulkAssignSystemRolesToUserOpts{
|
||||
UserID: createdUser.ID,
|
||||
Roles: roles,
|
||||
}); err != nil {
|
||||
s.logger.Error("failed to assign system role to user", log.Error(err))
|
||||
}
|
||||
|
||||
err = tx.Insert(ctx, createdUser.ID, spec, data)
|
||||
if err == nil {
|
||||
logAccountCreatedEvent(ctx, NewDBWith(s.logger, s), createdUser, spec.ServiceType)
|
||||
|
||||
@ -346,7 +346,7 @@ func seedPermissionDataForList(ctx context.Context, t *testing.T, store Permissi
|
||||
t.Helper()
|
||||
|
||||
perms, totalPerms := createTestPermissions(ctx, t, store)
|
||||
user := createTestUserForUserRole(ctx, "test@test.com", "test-user-1", t, db)
|
||||
user := createTestUserWithoutRoles(t, db, "test-user-1", false)
|
||||
role, err := createTestRole(ctx, "TEST-ROLE", false, t, db.Roles())
|
||||
require.NoError(t, err)
|
||||
|
||||
|
||||
@ -62,7 +62,7 @@ func TestRoleList(t *testing.T) {
|
||||
store := db.Roles()
|
||||
|
||||
roles, total := createTestRoles(ctx, t, store)
|
||||
user := createTestUserForUserRole(ctx, "test@test.com", "test-user-1", t, db)
|
||||
user := createTestUserWithoutRoles(t, db, "test-user-1", false)
|
||||
|
||||
err := db.UserRoles().Assign(ctx, AssignUserRoleOpts{
|
||||
RoleID: roles[0].ID,
|
||||
@ -115,6 +115,7 @@ func TestRoleList(t *testing.T) {
|
||||
UserID: user.ID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Len(t, userRoles, 1)
|
||||
require.Equal(t, userRoles[0].ID, roles[0].ID)
|
||||
})
|
||||
@ -137,7 +138,7 @@ func TestRoleCount(t *testing.T) {
|
||||
db := NewDB(logger, dbtest.NewDB(logger, t))
|
||||
store := db.Roles()
|
||||
|
||||
user := createTestUserForUserRole(ctx, "test@test.com", "test-user-1", t, db)
|
||||
user := createTestUserWithoutRoles(t, db, "test-user-1", false)
|
||||
roles, total := createTestRoles(ctx, t, store)
|
||||
|
||||
err := db.UserRoles().Assign(ctx, AssignUserRoleOpts{
|
||||
|
||||
@ -5,6 +5,7 @@ import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/keegancsmith/sqlf"
|
||||
"github.com/sourcegraph/log/logtest"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
@ -286,7 +287,7 @@ func TestUserRoleGetByRoleID(t *testing.T) {
|
||||
totalUsersWithRole := 10
|
||||
for i := 1; i <= totalUsersWithRole; i++ {
|
||||
username := fmt.Sprintf("ANOTHERTESTUSER%d", i)
|
||||
user := createTestUserForUserRole(ctx, fmt.Sprintf("testa%d@example.com", i), username, t, db)
|
||||
user := createTestUserWithoutRoles(t, db, username, false)
|
||||
|
||||
err := store.Assign(ctx, AssignUserRoleOpts{
|
||||
RoleID: role.ID,
|
||||
@ -324,7 +325,7 @@ func TestUserRoleGetByUserID(t *testing.T) {
|
||||
db := NewDB(logger, dbtest.NewDB(logger, t))
|
||||
store := db.UserRoles()
|
||||
|
||||
user := createTestUserForUserRole(ctx, "testuser@example.com", "ANOTHERTESTUSER", t, db)
|
||||
user := createTestUserWithoutRoles(t, db, "ANOTHERTESTUSER", false)
|
||||
|
||||
totalRoles := 3
|
||||
for i := 1; i <= totalRoles; i++ {
|
||||
@ -408,7 +409,7 @@ func TestUserRoleGetByRoleIDAndUserID(t *testing.T) {
|
||||
|
||||
func createUserAndRole(ctx context.Context, t *testing.T, db DB) (*types.User, *types.Role) {
|
||||
t.Helper()
|
||||
user := createTestUserForUserRole(ctx, "a1@example.com", "u1", t, db)
|
||||
user := createTestUserWithoutRoles(t, db, "u1", false)
|
||||
role := createTestRoleForUserRole(ctx, "ANOTHERTESTROLE - 1", t, db)
|
||||
return user, role
|
||||
}
|
||||
@ -422,16 +423,28 @@ func createTestRoleForUserRole(ctx context.Context, name string, t *testing.T, d
|
||||
return role
|
||||
}
|
||||
|
||||
func createTestUserForUserRole(ctx context.Context, email, username string, t *testing.T, db DB) *types.User {
|
||||
func createTestUserWithoutRoles(t *testing.T, db DB, username string, siteAdmin bool) *types.User {
|
||||
t.Helper()
|
||||
user, err := db.Users().Create(ctx, NewUser{
|
||||
Email: email,
|
||||
Username: username,
|
||||
Password: "p1",
|
||||
EmailVerificationCode: email + username,
|
||||
})
|
||||
|
||||
user := &types.User{
|
||||
Username: username,
|
||||
DisplayName: "testuser",
|
||||
}
|
||||
|
||||
q := sqlf.Sprintf("INSERT INTO users (username, site_admin) VALUES (%s, %t) RETURNING id, site_admin", user.Username, siteAdmin)
|
||||
err := db.QueryRowContext(context.Background(), q.Query(sqlf.PostgresBindVar), q.Args()...).Scan(&user.ID, &user.SiteAdmin)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if user.SiteAdmin != siteAdmin {
|
||||
t.Fatalf("user.SiteAdmin=%t, but expected is %t", user.SiteAdmin, siteAdmin)
|
||||
}
|
||||
|
||||
_, err = db.ExecContext(context.Background(), "INSERT INTO names(name, user_id) VALUES($1, $2)", user.Username, user.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create name: %s", err)
|
||||
}
|
||||
|
||||
return user
|
||||
}
|
||||
|
||||
@ -376,6 +376,26 @@ func (u *userStore) CreateInTransaction(ctx context.Context, info NewUser, spec
|
||||
InvalidatedSessionsAt: invalidatedSessionsAt,
|
||||
Searchable: searchable,
|
||||
}
|
||||
|
||||
{
|
||||
// Assign roles to the created user. We do this in here to ensure role assign occurs in the same transaction as user creation occurs.
|
||||
// This ensures we don't have "zombie" users (users with no role assigned to them).
|
||||
// All users on a Sourcegraph instance must have the `USER` role, however depending on the value of user.SiteAdmin,
|
||||
// we assign them the `SITE_ADMINISTRATOR` role.
|
||||
roles := []types.SystemRole{types.UserSystemRole}
|
||||
if user.SiteAdmin {
|
||||
roles = append(roles, types.SiteAdministratorSystemRole)
|
||||
}
|
||||
|
||||
db := NewDBWith(u.logger, u)
|
||||
if err := db.UserRoles().BulkAssignSystemRolesToUser(ctx, BulkAssignSystemRolesToUserOpts{
|
||||
UserID: user.ID,
|
||||
Roles: roles,
|
||||
}); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
// Run hooks.
|
||||
//
|
||||
|
||||
@ -125,6 +125,13 @@ func TestUsers_Create_SiteAdmin(t *testing.T) {
|
||||
if !user.SiteAdmin {
|
||||
t.Fatal("!user.SiteAdmin")
|
||||
}
|
||||
ur, err := getUserRoles(ctx, db, user.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(ur) != 2 {
|
||||
t.Fatalf("expected user to be assigned two roles (USER and SITE_ADMINISTRATOR), got %d", len(ur))
|
||||
}
|
||||
|
||||
// Creating a non-site-admin now that the site has already been initialized.
|
||||
u2, err := db.Users().Create(ctx, NewUser{
|
||||
@ -139,6 +146,14 @@ func TestUsers_Create_SiteAdmin(t *testing.T) {
|
||||
if u2.SiteAdmin {
|
||||
t.Fatal("want u2 not site admin because site is already initialized")
|
||||
}
|
||||
ur, err = getUserRoles(ctx, db, u2.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(ur) != 1 {
|
||||
t.Fatalf("expected user to be assigned one role, got %d", len(ur))
|
||||
}
|
||||
|
||||
// Similar to the above, but expect an error because we pass FailIfNotInitialUser: true.
|
||||
_, err = db.Users().Create(ctx, NewUser{
|
||||
Email: "a3@a3.com",
|
||||
@ -172,6 +187,14 @@ func TestUsers_Create_SiteAdmin(t *testing.T) {
|
||||
if u4.SiteAdmin {
|
||||
t.Fatal("want u4 not site admin because site is already initialized")
|
||||
}
|
||||
ur, err = getUserRoles(ctx, db, u4.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(ur) != 1 {
|
||||
t.Fatalf("expected user to be assigned one role, got %d", len(ur))
|
||||
}
|
||||
|
||||
// Similar to the above, but expect an error because we pass FailIfNotInitialUser: true.
|
||||
if _, err := db.ExecContext(ctx, "UPDATE site_config SET initialized=false"); err != nil {
|
||||
t.Fatal(err)
|
||||
@ -1168,30 +1191,61 @@ func TestUsers_SetIsSiteAdmin(t *testing.T) {
|
||||
db := NewDB(logger, dbtest.NewDB(logger, t))
|
||||
ctx := context.Background()
|
||||
|
||||
// Create user.
|
||||
u, err := db.Users().Create(ctx, NewUser{Username: "u"})
|
||||
adminUser, err := db.Users().Create(ctx, NewUser{Username: "u"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Create user. This user will have a `SiteAdmin` value of false because
|
||||
// Global state hasn't been initialized at this point, so technically this is the
|
||||
// first user.
|
||||
if !adminUser.SiteAdmin {
|
||||
t.Fatalf("expected site admin to be created")
|
||||
}
|
||||
|
||||
regularUser, err := db.Users().Create(ctx, NewUser{Username: "u2"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
t.Run("promoting to site admin", func(t *testing.T) {
|
||||
err := db.Users().SetIsSiteAdmin(ctx, u.ID, true)
|
||||
t.Run("revoking site admin role for a site admin", func(t *testing.T) {
|
||||
// Confirm that the user has only two roles assigned to them.
|
||||
ur, err := db.UserRoles().GetByUserID(ctx, GetUserRoleOpts{UserID: adminUser.ID})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, ur, 2)
|
||||
|
||||
err = db.Users().SetIsSiteAdmin(ctx, adminUser.ID, false)
|
||||
require.NoError(t, err)
|
||||
|
||||
// check that site admin role has been assigned to user
|
||||
ur, err := db.UserRoles().GetByUserID(ctx, GetUserRoleOpts{UserID: u.ID})
|
||||
// check that site admin role has been revoked for user
|
||||
ur, err = db.UserRoles().GetByUserID(ctx, GetUserRoleOpts{UserID: adminUser.ID})
|
||||
require.NoError(t, err)
|
||||
// Since we've revoked the SITE_ADMINISTRATOR role, the user should still have the
|
||||
// USER role assigned to them.
|
||||
require.Len(t, ur, 1)
|
||||
|
||||
u, err := db.Users().GetByID(ctx, regularUser.ID)
|
||||
require.NoError(t, err)
|
||||
require.False(t, u.SiteAdmin)
|
||||
})
|
||||
|
||||
t.Run("revoking site admin", func(t *testing.T) {
|
||||
err := db.Users().SetIsSiteAdmin(ctx, u.ID, false)
|
||||
t.Run("promoting a regular user to site admin", func(t *testing.T) {
|
||||
// Confirm that the user has only one role assigned to them.
|
||||
ur, err := db.UserRoles().GetByUserID(ctx, GetUserRoleOpts{UserID: regularUser.ID})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, ur, 1)
|
||||
|
||||
err = db.Users().SetIsSiteAdmin(ctx, regularUser.ID, true)
|
||||
require.NoError(t, err)
|
||||
|
||||
// check that site admin role has been assigned to user
|
||||
ur, err := db.UserRoles().GetByUserID(ctx, GetUserRoleOpts{UserID: u.ID})
|
||||
ur, err = db.UserRoles().GetByUserID(ctx, GetUserRoleOpts{UserID: regularUser.ID})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, ur, 0)
|
||||
// The user should have both USER role and SITE_ADMINISTRATOR role assigned to them.
|
||||
require.Len(t, ur, 2)
|
||||
|
||||
u, err := db.Users().GetByID(ctx, regularUser.ID)
|
||||
require.NoError(t, err)
|
||||
require.True(t, u.SiteAdmin)
|
||||
})
|
||||
}
|
||||
|
||||
@ -1203,3 +1257,7 @@ func normalizeUsers(users []*types.User) []*types.User {
|
||||
}
|
||||
return users
|
||||
}
|
||||
|
||||
func getUserRoles(ctx context.Context, db DB, userID int32) ([]*types.UserRole, error) {
|
||||
return db.UserRoles().GetByUserID(ctx, GetUserRoleOpts{UserID: userID})
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user