Users: Fix org user always getting org id = 1 on auto assign false (#63708)

* fix org user always getting org id = 1 on auto assign false

* make tests explicit

* use correct cfg in service accounts

* fix api tests

* fix database test of ac

* fix InsertOrgUser returning affected rows as orgID
This commit is contained in:
Jo
2023-02-24 17:08:44 +00:00
committed by GitHub
parent 823aaaeb7c
commit c8db771939
8 changed files with 65 additions and 31 deletions

View File

@@ -58,7 +58,7 @@ func (s *Service) GetIDForNewUser(ctx context.Context, cmd org.GetOrgIDForNewUse
return -1, nil
}
if setting.AutoAssignOrg && cmd.OrgID != 0 {
if s.cfg.AutoAssignOrg && cmd.OrgID != 0 {
_, err := s.store.Get(ctx, cmd.OrgID)
if err != nil {
return -1, err
@@ -73,7 +73,7 @@ func (s *Service) GetIDForNewUser(ctx context.Context, cmd org.GetOrgIDForNewUse
}
orga.Name = orgName
if setting.AutoAssignOrg {
if s.cfg.AutoAssignOrg {
orga, err := s.store.Get(ctx, int64(s.cfg.AutoAssignOrgId))
if err != nil {
return 0, err
@@ -81,14 +81,14 @@ func (s *Service) GetIDForNewUser(ctx context.Context, cmd org.GetOrgIDForNewUse
if orga.ID != 0 {
return orga.ID, nil
}
if setting.AutoAssignOrgId != 1 {
if s.cfg.AutoAssignOrgId != 1 {
s.log.Error("Could not create user: organization ID does not exist", "orgID",
setting.AutoAssignOrgId)
s.cfg.AutoAssignOrgId)
return 0, fmt.Errorf("could not create user: organization ID %d does not exist",
setting.AutoAssignOrgId)
s.cfg.AutoAssignOrgId)
}
orga.Name = MainOrgName
orga.ID = int64(setting.AutoAssignOrgId)
orga.ID = int64(s.cfg.AutoAssignOrgId)
} else {
orga.Name = orgName
}

View File

@@ -4,6 +4,7 @@ import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/services/org"
@@ -19,35 +20,47 @@ func TestOrgService(t *testing.T) {
}
t.Run("create org", func(t *testing.T) {
_, err := orgService.GetIDForNewUser(context.Background(), org.GetOrgIDForNewUserCommand{})
orgService.cfg.AutoAssignOrg = false
orgService.cfg.AutoAssignOrgId = 1
orgStore.ExpectedOrgID = 3
id, err := orgService.GetIDForNewUser(context.Background(), org.GetOrgIDForNewUserCommand{})
require.NoError(t, err)
assert.Equal(t, int64(3), id)
})
t.Run("create org", func(t *testing.T) {
_, err := orgService.GetIDForNewUser(context.Background(), org.GetOrgIDForNewUserCommand{})
// Should return a new org instead of the org defined in the command
t.Run("no autoassign - org defined", func(t *testing.T) {
orgService.cfg.AutoAssignOrg = false
orgService.cfg.AutoAssignOrgId = 1
orgStore.ExpectedOrgID = 3
orgStore.ExpectedOrg = &org.Org{ID: 1}
id, err := orgService.GetIDForNewUser(context.Background(), org.GetOrgIDForNewUserCommand{OrgID: 1})
require.NoError(t, err)
assert.Equal(t, int64(3), id)
})
t.Run("create org with auto assign org ID", func(t *testing.T) {
setting.AutoAssignOrg = true
setting.AutoAssignOrgId = 1
orgService.cfg.AutoAssignOrg = true
orgService.cfg.AutoAssignOrgId = 1
orgStore.ExpectedOrgID = 1
orgStore.ExpectedOrg = &org.Org{}
_, err := orgService.GetIDForNewUser(context.Background(), org.GetOrgIDForNewUserCommand{})
orgStore.ExpectedOrg = &org.Org{ID: 1}
id, err := orgService.GetIDForNewUser(context.Background(), org.GetOrgIDForNewUserCommand{})
require.NoError(t, err)
assert.Equal(t, int64(1), id)
})
t.Run("create org with auto assign org ID and orgID", func(t *testing.T) {
setting.AutoAssignOrg = true
setting.AutoAssignOrgId = 1
orgService.cfg.AutoAssignOrg = true
orgService.cfg.AutoAssignOrgId = 1
orgStore.ExpectedOrgID = 1
orgStore.ExpectedOrg = &org.Org{}
_, err := orgService.GetIDForNewUser(context.Background(), org.GetOrgIDForNewUserCommand{OrgID: 1})
orgStore.ExpectedOrg = &org.Org{ID: 1}
id, err := orgService.GetIDForNewUser(context.Background(), org.GetOrgIDForNewUserCommand{OrgID: 1})
require.NoError(t, err)
assert.Equal(t, int64(1), id)
})
setting.AutoAssignOrg = false
setting.AutoAssignOrgId = 0
orgService.cfg.AutoAssignOrg = false
orgService.cfg.AutoAssignOrgId = 0
t.Run("delete user from all orgs", func(t *testing.T) {
err := orgService.DeleteUserFromAll(context.Background(), 1)

View File

@@ -25,7 +25,9 @@ const MainOrgName = "Main Org."
type store interface {
Get(context.Context, int64) (*org.Org, error)
// Insert adds a new organization. returns organization id
Insert(context.Context, *org.Org) (int64, error)
// InsertOrgUser adds a new membership record for a user in an organization. returns membership id
InsertOrgUser(context.Context, *org.OrgUser) (int64, error)
DeleteUserFromAll(context.Context, int64) error
Update(ctx context.Context, cmd *org.UpdateOrgCommand) error
@@ -76,9 +78,12 @@ func (ss *sqlStore) Insert(ctx context.Context, org *org.Org) (int64, error) {
var orgID int64
var err error
err = ss.db.WithDbSession(ctx, func(sess *db.Session) error {
if orgID, err = sess.InsertOne(org); err != nil {
if _, err = sess.Insert(org); err != nil {
return err
}
orgID = org.ID
if org.ID != 0 {
// it sets the setval in the sequence
if err := ss.dialect.PostInsertId("org", sess.Session); err != nil {
@@ -98,11 +103,11 @@ func (ss *sqlStore) Insert(ctx context.Context, org *org.Org) (int64, error) {
return orgID, nil
}
// InsertOrgUser adds a new membership record for a user in an organization.
func (ss *sqlStore) InsertOrgUser(ctx context.Context, cmd *org.OrgUser) (int64, error) {
var orgID int64
var err error
err = ss.db.WithDbSession(ctx, func(sess *db.Session) error {
if orgID, err = sess.Insert(cmd); err != nil {
if _, err = sess.Insert(cmd); err != nil {
return err
}
return nil
@@ -110,7 +115,7 @@ func (ss *sqlStore) InsertOrgUser(ctx context.Context, cmd *org.OrgUser) (int64,
if err != nil {
return 0, err
}
return orgID, nil
return cmd.ID, nil
}
func (ss *sqlStore) DeleteUserFromAll(ctx context.Context, userID int64) error {

View File

@@ -415,6 +415,10 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) {
t.Run("Given single org and 2 users inserted", func(t *testing.T) {
ss = db.InitTestDB(t)
ss.Cfg.AutoAssignOrg = true
ss.Cfg.AutoAssignOrgId = 1
ss.Cfg.AutoAssignOrgRole = "Viewer"
_, usrSvc := createOrgAndUserSvc(t, ss, ss.Cfg)
testUser := &user.SignedInUser{
@@ -422,17 +426,17 @@ func TestIntegrationOrgUserDataAccess(t *testing.T) {
1: {accesscontrol.ActionOrgUsersRead: []string{accesscontrol.ScopeUsersAll}},
},
}
ss.Cfg.AutoAssignOrg = true
ss.Cfg.AutoAssignOrgId = 1
ss.Cfg.AutoAssignOrgRole = "Viewer"
ac1cmd := &user.CreateUserCommand{Login: "ac1", Email: "ac1@test.com", Name: "ac1 name"}
ac2cmd := &user.CreateUserCommand{Login: "ac2", Email: "ac2@test.com", Name: "ac2 name"}
ac1, err := usrSvc.CreateUserForTests(context.Background(), ac1cmd)
testUser.OrgID = ac1.OrgID
require.Equal(t, int64(1), ac1.OrgID)
require.NoError(t, err)
_, err = usrSvc.Create(context.Background(), ac2cmd)
ac2, err := usrSvc.Create(context.Background(), ac2cmd)
require.Equal(t, int64(1), ac2.OrgID)
require.NoError(t, err)
t.Run("Can get organization users paginated with query", func(t *testing.T) {