IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Split Create User (#50502)

Browse Source 
* Split Create User

* Use new create user and User from package user

* Add service to wire

* Making create user work

* Replace user from user pkg

* One more

* Move Insert to orguser Service/Store

* Remove unnecessary conversion

* Cleaunp

* Fix Get User and add fakes

* Fixing get org id for user logic, adding fakes and other adjustments

* Add some tests for ourguser service and store

* Fix insert org logic

* Add comment about deprecation

* Fix after merge with main

* Move orguser service/store to org service/store

* Remove orguser from wire

* Unimplement new Create user and use User from pkg user

* Fix wire generation

* Fix lint

* Fix lint - use only User and CrateUserCommand from user pkg

* Remove User and CreateUserCommand from models

* Fix lint 2
This commit is contained in:
idafurjes
2022-06-28 14:32:25 +02:00
committed by GitHub
parent 2429fe1c70
commit 6c43eb0b4d
105 changed files with 1524 additions and 701 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
pkg/services/serviceaccounts/api/api_test.go
View File

@@ -185,7 +185,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
serviceAccountRequestScenario(t, http.MethodDelete, serviceAccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, saStore)
actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, fmt.Sprint(createduser.Id))).Code
actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, fmt.Sprint(createduser.ID))).Code
require.Equal(t, testcase.expectedCode, actual)
})
})
@@ -209,7 +209,7 @@ func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
serviceAccountRequestScenario(t, http.MethodDelete, serviceAccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock, store, saStore)
actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, createduser.Id)).Code
actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, createduser.ID)).Code
require.Equal(t, testcase.expectedCode, actual)
})
})
@@ -314,7 +314,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) {
scopeID := tc.Id
if tc.user != nil {
createdUser := tests.SetupUserServiceAccount(t, store, *tc.user)
scopeID = int(createdUser.Id)
scopeID = int(createdUser.ID)
}
server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, saStore)
@@ -440,7 +440,7 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
scopeID := tc.Id
if tc.user != nil {
createdUser := tests.SetupUserServiceAccount(t, store, *tc.user)
scopeID = int(createdUser.Id)
scopeID = int(createdUser.ID)
}
var rawBody io.Reader = http.NoBody

16
pkg/services/serviceaccounts/api/token_test.go
View File

@@ -125,7 +125,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
endpoint := fmt.Sprintf(serviceaccountIDTokensPath, sa.Id)
endpoint := fmt.Sprintf(serviceaccountIDTokensPath, sa.ID)
bodyString := ""
if tc.body != nil {
b, err := json.Marshal(tc.body)
@@ -146,12 +146,12 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) {
if actualCode == http.StatusOK {
assert.Equal(t, tc.body["name"], actualBody["name"])
query := models.GetApiKeyByNameQuery{KeyName: tc.body["name"].(string), OrgId: sa.OrgId}
query := models.GetApiKeyByNameQuery{KeyName: tc.body["name"].(string), OrgId: sa.OrgID}
err = store.GetApiKeyByName(context.Background(), &query)
require.NoError(t, err)
assert.Equal(t, sa.Id, *query.Result.ServiceAccountId)
assert.Equal(t, sa.OrgId, query.Result.OrgId)
assert.Equal(t, sa.ID, *query.Result.ServiceAccountId)
assert.Equal(t, sa.OrgID, query.Result.OrgId)
assert.True(t, strings.HasPrefix(actualBody["key"].(string), "glsa"))
keyInfo, err := apikeygenprefix.Decode(actualBody["key"].(string))
@@ -229,9 +229,9 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
token := createTokenforSA(t, saStore, tc.keyName, sa.OrgId, sa.Id, 1)
token := createTokenforSA(t, saStore, tc.keyName, sa.OrgID, sa.ID, 1)
endpoint := fmt.Sprintf(serviceaccountIDTokensDetailPath, sa.Id, token.Id)
endpoint := fmt.Sprintf(serviceaccountIDTokensDetailPath, sa.ID, token.Id)
bodyString := ""
server, _ := setupTestServer(t, svcMock, routing.NewRouteRegister(), tc.acmock, store, saStore)
actual := requestResponse(server, http.MethodDelete, endpoint, strings.NewReader(bodyString))
@@ -242,7 +242,7 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
_ = json.Unmarshal(actual.Body.Bytes(), &actualBody)
require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody)
query := models.GetApiKeyByNameQuery{KeyName: tc.keyName, OrgId: sa.OrgId}
query := models.GetApiKeyByNameQuery{KeyName: tc.keyName, OrgId: sa.OrgID}
err := store.GetApiKeyByName(context.Background(), &query)
if actualCode == http.StatusOK {
require.Error(t, err)
@@ -354,7 +354,7 @@ func TestServiceAccountsAPI_ListTokens(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
endpoint := fmt.Sprintf(serviceAccountIDPath+"/tokens", sa.Id)
endpoint := fmt.Sprintf(serviceAccountIDPath+"/tokens", sa.ID)
server, _ := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store, &saStoreMockTokens{saAPIKeys: tc.tokens})
actual := requestResponse(server, http.MethodGet, endpoint, http.NoBody)

25
pkg/services/serviceaccounts/database/database.go
View File

@@ -14,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
)
type ServiceAccountsStoreImpl struct {
@@ -34,9 +35,9 @@ func NewServiceAccountsStore(store *sqlstore.SQLStore, kvStore kvstore.KVStore)
func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, orgId int64, name string) (saDTO *serviceaccounts.ServiceAccountDTO, err error) {
generatedLogin := "sa-" + strings.ToLower(name)
generatedLogin = strings.ReplaceAll(generatedLogin, " ", "-")
cmd := models.CreateUserCommand{
cmd := user.CreateUserCommand{
Login: generatedLogin,
OrgId: orgId,
OrgID: orgId,
Name: name,
IsServiceAccount: true,
}
@@ -50,10 +51,10 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, org
}
return &serviceaccounts.ServiceAccountDTO{
Id: newuser.Id,
Id: newuser.ID,
Name: newuser.Name,
Login: newuser.Login,
OrgId: newuser.OrgId,
OrgId: newuser.OrgID,
Tokens: 0,
}, nil
}
@@ -89,7 +90,7 @@ func (s *ServiceAccountsStoreImpl) UpdateServiceAccount(ctx context.Context,
}
if saForm.Name != nil || saForm.IsDisabled != nil {
user := models.User{
user := user.User{
Updated: updateTime,
}
@@ -131,7 +132,7 @@ func (s *ServiceAccountsStoreImpl) DeleteServiceAccount(ctx context.Context, org
}
func (s *ServiceAccountsStoreImpl) deleteServiceAccount(sess *sqlstore.DBSession, orgId, serviceAccountId int64) error {
user := models.User{}
user := user.User{}
has, err := sess.Where(`org_id = ? and id = ? and is_service_account = ?`,
orgId, serviceAccountId, s.sqlStore.Dialect.BooleanStr(true)).Get(&user)
if err != nil {
@@ -141,7 +142,7 @@ func (s *ServiceAccountsStoreImpl) deleteServiceAccount(sess *sqlstore.DBSession
return serviceaccounts.ErrServiceAccountNotFound
}
for _, sql := range ServiceAccountDeletions() {
_, err := sess.Exec(sql, user.Id)
_, err := sess.Exec(sql, user.ID)
if err != nil {
return err
}
@@ -405,10 +406,10 @@ func (s *ServiceAccountsStoreImpl) MigrateApiKey(ctx context.Context, orgId int6
func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Context, key *models.ApiKey) error {
prefix := "sa-autogen"
cmd := models.CreateUserCommand{
cmd := user.CreateUserCommand{
Login: fmt.Sprintf("%v-%v-%v", prefix, key.OrgId, key.Name),
Name: fmt.Sprintf("%v-%v", prefix, key.Name),
OrgId: key.OrgId,
OrgID: key.OrgId,
DefaultOrgRole: string(key.Role),
IsServiceAccount: true,
}
@@ -419,8 +420,8 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Co
return fmt.Errorf("failed to create service account: %w", errCreateSA)
}
if err := s.assignApiKeyToServiceAccount(sess, key.Id, newSA.Id); err != nil {
if err := s.sqlStore.DeleteUser(ctx, &models.DeleteUserCommand{UserId: newSA.Id}); err != nil {
if err := s.assignApiKeyToServiceAccount(sess, key.Id, newSA.ID); err != nil {
if err := s.sqlStore.DeleteUser(ctx, &models.DeleteUserCommand{UserId: newSA.ID}); err != nil {
s.log.Error("Error deleting service account", "error", err)
}
return fmt.Errorf("failed to migrate API key to service account token: %w", err)
@@ -451,7 +452,7 @@ func (s *ServiceAccountsStoreImpl) RevertApiKey(ctx context.Context, keyId int64
}
err = s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
user := models.User{}
user := user.User{}
has, err := sess.Where(`org_id = ? and id = ? and is_service_account = ?`,
key.OrgId, *key.ServiceAccountId, s.sqlStore.Dialect.BooleanStr(true)).Get(&user)
if err != nil {

4
pkg/services/serviceaccounts/database/database_test.go
View File

@@ -59,7 +59,7 @@ func TestStore_DeleteServiceAccount(t *testing.T) {
t.Run(c.desc, func(t *testing.T) {
db, store := setupTestDatabase(t)
user := tests.SetupUserServiceAccount(t, db, c.user)
err := store.DeleteServiceAccount(context.Background(), user.OrgId, user.Id)
err := store.DeleteServiceAccount(context.Background(), user.OrgID, user.ID)
if c.expectedErr != nil {
require.ErrorIs(t, err, c.expectedErr)
} else {
@@ -98,7 +98,7 @@ func TestStore_RetrieveServiceAccount(t *testing.T) {
t.Run(c.desc, func(t *testing.T) {
db, store := setupTestDatabase(t)
user := tests.SetupUserServiceAccount(t, db, c.user)
dto, err := store.RetrieveServiceAccount(context.Background(), user.OrgId, user.Id)
dto, err := store.RetrieveServiceAccount(context.Background(), user.OrgID, user.ID)
if c.expectedErr != nil {
require.ErrorIs(t, err, c.expectedErr)
} else {

6
pkg/services/serviceaccounts/database/stats_test.go
View File

@@ -18,18 +18,18 @@ func TestStore_UsageStats(t *testing.T) {
sa := tests.SetupUserServiceAccount(t, db, saToCreate)
keyName := t.Name()
key, err := apikeygen.New(sa.OrgId, keyName)
key, err := apikeygen.New(sa.OrgID, keyName)
require.NoError(t, err)
cmd := serviceaccounts.AddServiceAccountTokenCommand{
Name: keyName,
OrgId: sa.OrgId,
OrgId: sa.OrgID,
Key: key.HashedKey,
SecondsToLive: 0,
Result: &models.ApiKey{},
}
err = store.AddServiceAccountToken(context.Background(), sa.Id, &cmd)
err = store.AddServiceAccountToken(context.Background(), sa.ID, &cmd)
require.NoError(t, err)
stats, err := store.GetUsageMetrics(context.Background())

28
pkg/services/serviceaccounts/database/token_store_test.go
View File

@@ -26,18 +26,18 @@ func TestStore_AddServiceAccountToken(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
keyName := t.Name()
key, err := apikeygen.New(user.OrgId, keyName)
key, err := apikeygen.New(user.OrgID, keyName)
require.NoError(t, err)
cmd := serviceaccounts.AddServiceAccountTokenCommand{
Name: keyName,
OrgId: user.OrgId,
OrgId: user.OrgID,
Key: key.HashedKey,
SecondsToLive: tc.secondsToLive,
Result: &models.ApiKey{},
}
err = store.AddServiceAccountToken(context.Background(), user.Id, &cmd)
err = store.AddServiceAccountToken(context.Background(), user.ID, &cmd)
if tc.secondsToLive < 0 {
require.Error(t, err)
return
@@ -48,7 +48,7 @@ func TestStore_AddServiceAccountToken(t *testing.T) {
require.Equal(t, t.Name(), newKey.Name)
// Verify against DB
keys, errT := store.ListTokens(context.Background(), user.OrgId, user.Id)
keys, errT := store.ListTokens(context.Background(), user.OrgID, user.ID)
require.NoError(t, errT)
@@ -76,18 +76,18 @@ func TestStore_AddServiceAccountToken_WrongServiceAccount(t *testing.T) {
sa := tests.SetupUserServiceAccount(t, db, saToCreate)
keyName := t.Name()
key, err := apikeygen.New(sa.OrgId, keyName)
key, err := apikeygen.New(sa.OrgID, keyName)
require.NoError(t, err)
cmd := serviceaccounts.AddServiceAccountTokenCommand{
Name: keyName,
OrgId: sa.OrgId,
OrgId: sa.OrgID,
Key: key.HashedKey,
SecondsToLive: 0,
Result: &models.ApiKey{},
}
err = store.AddServiceAccountToken(context.Background(), sa.Id+1, &cmd)
err = store.AddServiceAccountToken(context.Background(), sa.ID+1, &cmd)
require.Error(t, err, "It should not be possible to add token to non-existing service account")
}
@@ -97,34 +97,34 @@ func TestStore_DeleteServiceAccountToken(t *testing.T) {
sa := tests.SetupUserServiceAccount(t, db, userToCreate)
keyName := t.Name()
key, err := apikeygen.New(sa.OrgId, keyName)
key, err := apikeygen.New(sa.OrgID, keyName)
require.NoError(t, err)
cmd := serviceaccounts.AddServiceAccountTokenCommand{
Name: keyName,
OrgId: sa.OrgId,
OrgId: sa.OrgID,
Key: key.HashedKey,
SecondsToLive: 0,
Result: &models.ApiKey{},
}
err = store.AddServiceAccountToken(context.Background(), sa.Id, &cmd)
err = store.AddServiceAccountToken(context.Background(), sa.ID, &cmd)
require.NoError(t, err)
newKey := cmd.Result
// Delete key from wrong service account
err = store.DeleteServiceAccountToken(context.Background(), sa.OrgId, sa.Id+2, newKey.Id)
err = store.DeleteServiceAccountToken(context.Background(), sa.OrgID, sa.ID+2, newKey.Id)
require.Error(t, err)
// Delete key from wrong org
err = store.DeleteServiceAccountToken(context.Background(), sa.OrgId+2, sa.Id, newKey.Id)
err = store.DeleteServiceAccountToken(context.Background(), sa.OrgID+2, sa.ID, newKey.Id)
require.Error(t, err)
err = store.DeleteServiceAccountToken(context.Background(), sa.OrgId, sa.Id, newKey.Id)
err = store.DeleteServiceAccountToken(context.Background(), sa.OrgID, sa.ID, newKey.Id)
require.NoError(t, err)
// Verify against DB
keys, errT := store.ListTokens(context.Background(), sa.OrgId, sa.Id)
keys, errT := store.ListTokens(context.Background(), sa.OrgID, sa.ID)
require.NoError(t, errT)
for _, k := range keys {

5
pkg/services/serviceaccounts/tests/common.go
View File

@@ -9,6 +9,7 @@ import (
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/stretchr/testify/require"
)
@@ -27,13 +28,13 @@ type TestApiKey struct {
IsExpired bool
}
func SetupUserServiceAccount(t *testing.T, sqlStore *sqlstore.SQLStore, testUser TestUser) *models.User {
func SetupUserServiceAccount(t *testing.T, sqlStore *sqlstore.SQLStore, testUser TestUser) *user.User {
role := string(models.ROLE_VIEWER)
if testUser.Role != "" {
role = testUser.Role
}
u1, err := sqlStore.CreateUser(context.Background(), models.CreateUserCommand{
u1, err := sqlStore.CreateUser(context.Background(), user.CreateUserCommand{
Login: testUser.Login,
IsServiceAccount: testUser.IsServiceAccount,
DefaultOrgRole: role,