Skip to content

Commit fef1e1d

Browse files
kalleepkalleep
authored
Auth: Refactor auth package (grafana#58920)
* Auth: move interface to its own file * Auth: move to test package * Auth: move quota consts to auth file * Auth: move service to impl package * Auth: move interfaces and related models to auth package * Auth: Create sub package and type alias to avoid circular dependency
1 parent ea27eca commit fef1e1d

35 files changed

+245
-221
lines changed

‎pkg/api/admin_users.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/grafana/grafana/pkg/infra/metrics"
1515
"github.com/grafana/grafana/pkg/models"
1616
"github.com/grafana/grafana/pkg/services/accesscontrol"
17+
"github.com/grafana/grafana/pkg/services/auth"
1718
"github.com/grafana/grafana/pkg/services/user"
1819
"github.com/grafana/grafana/pkg/util"
1920
"github.com/grafana/grafana/pkg/web"
@@ -416,7 +417,7 @@ func (hs *HTTPServer) AdminGetUserAuthTokens(c *models.ReqContext) response.Resp
416417
// 404: notFoundError
417418
// 500: internalServerError
418419
func (hs *HTTPServer) AdminRevokeUserAuthToken(c *models.ReqContext) response.Response {
419-
cmd := models.RevokeAuthTokenCmd{}
420+
cmd := auth.RevokeAuthTokenCmd{}
420421
if err := web.Bind(c.Req, &cmd); err != nil {
421422
return response.Error(http.StatusBadRequest, "bad request data", err)
422423
}
@@ -476,7 +477,7 @@ type AdminLogoutUserParams struct {
476477
type AdminRevokeUserAuthTokenParams struct {
477478
// in:body
478479
// required:true
479-
Body models.RevokeAuthTokenCmd `json:"body"`
480+
Body auth.RevokeAuthTokenCmd `json:"body"`
480481
// in:path
481482
// required:true
482483
UserID int64 `json:"user_id"`
@@ -508,5 +509,5 @@ type AdminCreateUserResponseResponse struct {
508509
// swagger:response adminGetUserAuthTokensResponse
509510
type AdminGetUserAuthTokensResponse struct {
510511
// in:body
511-
Body []*models.UserToken `json:"body"`
512+
Body []*auth.UserToken `json:"body"`
512513
}

‎pkg/api/admin_users_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/grafana/grafana/pkg/components/simplejson"
1414
"github.com/grafana/grafana/pkg/models"
1515
"github.com/grafana/grafana/pkg/services/auth"
16+
"github.com/grafana/grafana/pkg/services/auth/authtest"
1617
"github.com/grafana/grafana/pkg/services/login/loginservice"
1718
"github.com/grafana/grafana/pkg/services/login/logintest"
1819
"github.com/grafana/grafana/pkg/services/org"
@@ -65,7 +66,7 @@ func TestAdminAPIEndpoint(t *testing.T) {
6566
})
6667

6768
t.Run("When a server admin attempts to revoke an auth token for a non-existing user", func(t *testing.T) {
68-
cmd := models.RevokeAuthTokenCmd{AuthTokenId: 2}
69+
cmd := auth.RevokeAuthTokenCmd{AuthTokenId: 2}
6970
mockUser := usertest.NewUserServiceFake()
7071
mockUser.ExpectedError = user.ErrUserNotFound
7172
adminRevokeUserAuthTokenScenario(t, "Should return not found when calling POST on",
@@ -263,7 +264,7 @@ func putAdminScenario(t *testing.T, desc string, url string, routePattern string
263264
func adminLogoutUserScenario(t *testing.T, desc string, url string, routePattern string, fn scenarioFunc, userService *usertest.FakeUserService) {
264265
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
265266
hs := HTTPServer{
266-
AuthTokenService: auth.NewFakeUserAuthTokenService(),
267+
AuthTokenService: authtest.NewFakeUserAuthTokenService(),
267268
userService: userService,
268269
}
269270

@@ -285,9 +286,9 @@ func adminLogoutUserScenario(t *testing.T, desc string, url string, routePattern
285286
})
286287
}
287288

288-
func adminRevokeUserAuthTokenScenario(t *testing.T, desc string, url string, routePattern string, cmd models.RevokeAuthTokenCmd, fn scenarioFunc, userService user.Service) {
289+
func adminRevokeUserAuthTokenScenario(t *testing.T, desc string, url string, routePattern string, cmd auth.RevokeAuthTokenCmd, fn scenarioFunc, userService user.Service) {
289290
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
290-
fakeAuthTokenService := auth.NewFakeUserAuthTokenService()
291+
fakeAuthTokenService := authtest.NewFakeUserAuthTokenService()
291292

292293
hs := HTTPServer{
293294
AuthTokenService: fakeAuthTokenService,
@@ -315,7 +316,7 @@ func adminRevokeUserAuthTokenScenario(t *testing.T, desc string, url string, rou
315316

316317
func adminGetUserAuthTokensScenario(t *testing.T, desc string, url string, routePattern string, fn scenarioFunc, userService *usertest.FakeUserService) {
317318
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
318-
fakeAuthTokenService := auth.NewFakeUserAuthTokenService()
319+
fakeAuthTokenService := authtest.NewFakeUserAuthTokenService()
319320

320321
hs := HTTPServer{
321322
AuthTokenService: fakeAuthTokenService,
@@ -341,7 +342,7 @@ func adminGetUserAuthTokensScenario(t *testing.T, desc string, url string, route
341342

342343
func adminDisableUserScenario(t *testing.T, desc string, action string, url string, routePattern string, fn scenarioFunc) {
343344
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
344-
fakeAuthTokenService := auth.NewFakeUserAuthTokenService()
345+
fakeAuthTokenService := authtest.NewFakeUserAuthTokenService()
345346

346347
authInfoService := &logintest.AuthInfoServiceFake{}
347348

‎pkg/api/common_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import (
2828
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
2929
"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
3030
"github.com/grafana/grafana/pkg/services/annotations/annotationstest"
31-
"github.com/grafana/grafana/pkg/services/auth"
31+
"github.com/grafana/grafana/pkg/services/auth/authtest"
3232
"github.com/grafana/grafana/pkg/services/contexthandler"
3333
"github.com/grafana/grafana/pkg/services/contexthandler/authproxy"
3434
"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
@@ -181,7 +181,7 @@ type scenarioContext struct {
181181
defaultHandler web.Handler
182182
req *http.Request
183183
url string
184-
userAuthTokenService *auth.FakeUserAuthTokenService
184+
userAuthTokenService *authtest.FakeUserAuthTokenService
185185
sqlStore sqlstore.Store
186186
authInfoService *logintest.AuthInfoServiceFake
187187
dashboardVersionService dashver.Service
@@ -207,7 +207,7 @@ func getContextHandler(t *testing.T, cfg *setting.Cfg) *contexthandler.ContextHa
207207
cfg.RemoteCacheOptions = &setting.RemoteCacheOptions{
208208
Name: "database",
209209
}
210-
userAuthTokenSvc := auth.NewFakeUserAuthTokenService()
210+
userAuthTokenSvc := authtest.NewFakeUserAuthTokenService()
211211
renderSvc := &fakeRenderService{}
212212
authJWTSvc := models.NewFakeJWTService()
213213
tracer := tracing.InitializeTracerForTest()

‎pkg/api/http_server.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"github.com/grafana/grafana/pkg/bus"
1717
"github.com/grafana/grafana/pkg/middleware/csrf"
18+
"github.com/grafana/grafana/pkg/services/auth"
1819
"github.com/grafana/grafana/pkg/services/folder"
1920
"github.com/grafana/grafana/pkg/services/oauthtoken"
2021
"github.com/grafana/grafana/pkg/services/querylibrary"
@@ -120,7 +121,7 @@ type HTTPServer struct {
120121
navTreeService navtree.Service
121122
CacheService *localcache.CacheService
122123
DataSourceCache datasources.CacheService
123-
AuthTokenService models.UserTokenService
124+
AuthTokenService auth.UserTokenService
124125
QuotaService quota.Service
125126
RemoteCacheService *remotecache.RemoteCache
126127
ProvisioningService provisioning.ProvisioningService
@@ -220,7 +221,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
220221
pluginRequestValidator models.PluginRequestValidator, pluginStaticRouteResolver plugins.StaticRouteResolver,
221222
pluginDashboardService plugindashboards.Service, pluginStore plugins.Store, pluginClient plugins.Client,
222223
pluginErrorResolver plugins.ErrorResolver, pluginInstaller plugins.Installer, settingsProvider setting.Provider,
223-
dataSourceCache datasources.CacheService, userTokenService models.UserTokenService,
224+
dataSourceCache datasources.CacheService, userTokenService auth.UserTokenService,
224225
cleanUpService *cleanup.CleanUpService, shortURLService shorturls.Service, queryHistoryService queryhistory.Service, correlationsService correlations.Service,
225226
thumbService thumbs.Service, remoteCache *remotecache.RemoteCache, provisioningService provisioning.ProvisioningService,
226227
loginService login.Service, authenticator loginpkg.Authenticator, accessControl accesscontrol.AccessControl,

‎pkg/api/ldap_debug_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
"github.com/grafana/grafana/pkg/api/routing"
1616
"github.com/grafana/grafana/pkg/models"
1717
"github.com/grafana/grafana/pkg/services/accesscontrol"
18-
"github.com/grafana/grafana/pkg/services/auth"
18+
"github.com/grafana/grafana/pkg/services/auth/authtest"
1919
"github.com/grafana/grafana/pkg/services/ldap"
2020
"github.com/grafana/grafana/pkg/services/login/loginservice"
2121
"github.com/grafana/grafana/pkg/services/login/logintest"
@@ -379,7 +379,7 @@ func postSyncUserWithLDAPContext(t *testing.T, requestURL string, preHook func(*
379379

380380
hs := &HTTPServer{
381381
Cfg: sc.cfg,
382-
AuthTokenService: auth.NewFakeUserAuthTokenService(),
382+
AuthTokenService: authtest.NewFakeUserAuthTokenService(),
383383
Login: loginservice.LoginServiceMock{},
384384
authInfoService: sc.authInfoService,
385385
userService: userService,

‎pkg/api/login.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/grafana/grafana/pkg/login"
1717
"github.com/grafana/grafana/pkg/middleware/cookies"
1818
"github.com/grafana/grafana/pkg/models"
19+
"github.com/grafana/grafana/pkg/services/auth"
1920
loginService "github.com/grafana/grafana/pkg/services/login"
2021
"github.com/grafana/grafana/pkg/services/secrets"
2122
"github.com/grafana/grafana/pkg/services/user"
@@ -227,7 +228,7 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext) response.Response {
227228

228229
err = hs.loginUserWithUser(usr, c)
229230
if err != nil {
230-
var createTokenErr *models.CreateTokenErr
231+
var createTokenErr *auth.CreateTokenErr
231232
if errors.As(err, &createTokenErr) {
232233
resp = response.Error(createTokenErr.StatusCode, createTokenErr.ExternalErr, createTokenErr.InternalErr)
233234
} else {
@@ -299,7 +300,7 @@ func (hs *HTTPServer) Logout(c *models.ReqContext) {
299300
}
300301

301302
err := hs.AuthTokenService.RevokeToken(c.Req.Context(), c.UserToken, false)
302-
if err != nil && !errors.Is(err, models.ErrUserTokenNotFound) {
303+
if err != nil && !errors.Is(err, auth.ErrUserTokenNotFound) {
303304
hs.log.Error("failed to revoke auth token", "error", err)
304305
}
305306

@@ -370,7 +371,7 @@ func (hs *HTTPServer) samlSingleLogoutEnabled() bool {
370371
}
371372

372373
func getLoginExternalError(err error) string {
373-
var createTokenErr *models.CreateTokenErr
374+
var createTokenErr *auth.CreateTokenErr
374375
if errors.As(err, &createTokenErr) {
375376
return createTokenErr.ExternalErr
376377
}

‎pkg/api/login_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ import (
1212
"strings"
1313
"testing"
1414

15-
loginservice "github.com/grafana/grafana/pkg/services/login"
16-
"github.com/grafana/grafana/pkg/services/navtree"
1715
"github.com/stretchr/testify/assert"
1816
"github.com/stretchr/testify/require"
1917

@@ -25,9 +23,11 @@ import (
2523
"github.com/grafana/grafana/pkg/login"
2624
"github.com/grafana/grafana/pkg/login/social"
2725
"github.com/grafana/grafana/pkg/models"
28-
"github.com/grafana/grafana/pkg/services/auth"
26+
"github.com/grafana/grafana/pkg/services/auth/authtest"
2927
"github.com/grafana/grafana/pkg/services/hooks"
3028
"github.com/grafana/grafana/pkg/services/licensing"
29+
loginservice "github.com/grafana/grafana/pkg/services/login"
30+
"github.com/grafana/grafana/pkg/services/navtree"
3131
"github.com/grafana/grafana/pkg/services/secrets"
3232
"github.com/grafana/grafana/pkg/services/secrets/fakes"
3333
secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
@@ -323,7 +323,7 @@ func TestLoginPostRedirect(t *testing.T) {
323323
Cfg: setting.NewCfg(),
324324
HooksService: &hooks.HooksService{},
325325
License: &licensing.OSSLicensingService{},
326-
AuthTokenService: auth.NewFakeUserAuthTokenService(),
326+
AuthTokenService: authtest.NewFakeUserAuthTokenService(),
327327
}
328328
hs.Cfg.CookieSecure = true
329329

@@ -564,7 +564,7 @@ func setupAuthProxyLoginTest(t *testing.T, enableLoginToken bool) *scenarioConte
564564
Cfg: sc.cfg,
565565
SettingsProvider: &setting.OSSImpl{Cfg: sc.cfg},
566566
License: &licensing.OSSLicensingService{},
567-
AuthTokenService: auth.NewFakeUserAuthTokenService(),
567+
AuthTokenService: authtest.NewFakeUserAuthTokenService(),
568568
log: log.New("hello"),
569569
SocialService: &mockSocialService{},
570570
}
@@ -602,7 +602,7 @@ func TestLoginPostRunLokingHook(t *testing.T) {
602602
log: log.New("test"),
603603
Cfg: setting.NewCfg(),
604604
License: &licensing.OSSLicensingService{},
605-
AuthTokenService: auth.NewFakeUserAuthTokenService(),
605+
AuthTokenService: authtest.NewFakeUserAuthTokenService(),
606606
HooksService: hookService,
607607
}
608608

‎pkg/api/user_token.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/grafana/grafana/pkg/api/dtos"
1010
"github.com/grafana/grafana/pkg/api/response"
1111
"github.com/grafana/grafana/pkg/models"
12+
"github.com/grafana/grafana/pkg/services/auth"
1213
"github.com/grafana/grafana/pkg/services/user"
1314
"github.com/grafana/grafana/pkg/util"
1415
"github.com/grafana/grafana/pkg/web"
@@ -43,7 +44,7 @@ func (hs *HTTPServer) GetUserAuthTokens(c *models.ReqContext) response.Response
4344
// 403: forbiddenError
4445
// 500: internalServerError
4546
func (hs *HTTPServer) RevokeUserAuthToken(c *models.ReqContext) response.Response {
46-
cmd := models.RevokeAuthTokenCmd{}
47+
cmd := auth.RevokeAuthTokenCmd{}
4748
if err := web.Bind(c.Req, &cmd); err != nil {
4849
return response.Error(http.StatusBadRequest, "bad request data", err)
4950
}
@@ -143,7 +144,7 @@ func (hs *HTTPServer) getUserAuthTokensInternal(c *models.ReqContext, userID int
143144
return response.JSON(http.StatusOK, result)
144145
}
145146

146-
func (hs *HTTPServer) revokeUserAuthTokenInternal(c *models.ReqContext, userID int64, cmd models.RevokeAuthTokenCmd) response.Response {
147+
func (hs *HTTPServer) revokeUserAuthTokenInternal(c *models.ReqContext, userID int64, cmd auth.RevokeAuthTokenCmd) response.Response {
147148
userQuery := user.GetUserByIDQuery{ID: userID}
148149
_, err := hs.userService.GetByID(c.Req.Context(), &userQuery)
149150
if err != nil {
@@ -155,7 +156,7 @@ func (hs *HTTPServer) revokeUserAuthTokenInternal(c *models.ReqContext, userID i
155156

156157
token, err := hs.AuthTokenService.GetUserToken(c.Req.Context(), userID, cmd.AuthTokenId)
157158
if err != nil {
158-
if errors.Is(err, models.ErrUserTokenNotFound) {
159+
if errors.Is(err, auth.ErrUserTokenNotFound) {
159160
return response.Error(404, "User auth token not found", err)
160161
}
161162
return response.Error(500, "Failed to get user auth token", err)
@@ -167,7 +168,7 @@ func (hs *HTTPServer) revokeUserAuthTokenInternal(c *models.ReqContext, userID i
167168

168169
err = hs.AuthTokenService.RevokeToken(c.Req.Context(), token, false)
169170
if err != nil {
170-
if errors.Is(err, models.ErrUserTokenNotFound) {
171+
if errors.Is(err, auth.ErrUserTokenNotFound) {
171172
return response.Error(404, "User auth token not found", err)
172173
}
173174
return response.Error(500, "Failed to revoke user auth token", err)
@@ -182,11 +183,11 @@ func (hs *HTTPServer) revokeUserAuthTokenInternal(c *models.ReqContext, userID i
182183
type RevokeUserAuthTokenParams struct {
183184
// in:body
184185
// required:true
185-
Body models.RevokeAuthTokenCmd `json:"body"`
186+
Body auth.RevokeAuthTokenCmd `json:"body"`
186187
}
187188

188189
// swagger:response getUserAuthTokensResponse
189190
type GetUserAuthTokensResponse struct {
190191
// in:body
191-
Body []*models.UserToken `json:"body"`
192+
Body []*auth.UserToken `json:"body"`
192193
}

0 commit comments

Comments
 (0)