public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

AuthN: Remove embedded oauth server (#83146)

Browse Source 
* AuthN: Remove embedded oauth server

* Restore main

* go mod tidy

* Fix problem

* Remove permission intersection

* Fix test and lint

* Fix TestData test

* Revert to origin/main

* Update go.mod

* Update go.mod

* Update go.sum
This commit is contained in:
Gabriel MABILLE
2024-02-26 11:29:09 +01:00
committed by GitHub
parent d0679f0993
commit 80d6bf6da0
55 changed files with 46 additions and 5631 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/extsvcauth/registry/service.go
+5 -33
View File
@@ -1,5 +1,7 @@
package registry
// FIXME (gamab): we can eventually remove this package
import (
"context"
"sync"
@@ -9,7 +11,6 @@ import (
"github.com/grafana/grafana/pkg/infra/serverlock"
"github.com/grafana/grafana/pkg/infra/slugify"
"github.com/grafana/grafana/pkg/services/extsvcauth"
"github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/oasimpl"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/serviceaccounts/extsvcaccounts"
)
@@ -29,21 +30,20 @@ type serverLocker interface {
type Registry struct {
features featuremgmt.FeatureToggles
logger log.Logger
oauthReg extsvcauth.ExternalServiceRegistry
saReg extsvcauth.ExternalServiceRegistry
// FIXME (gamab): we can remove this field and use the saReg.GetExternalServiceNames directly
extSvcProviders map[string]extsvcauth.AuthProvider
lock sync.Mutex
serverLock serverLocker
}
func ProvideExtSvcRegistry(oauthServer *oasimpl.OAuth2ServiceImpl, saSvc *extsvcaccounts.ExtSvcAccountsService, serverLock *serverlock.ServerLockService, features featuremgmt.FeatureToggles) *Registry {
func ProvideExtSvcRegistry(saSvc *extsvcaccounts.ExtSvcAccountsService, serverLock *serverlock.ServerLockService, features featuremgmt.FeatureToggles) *Registry {
return &Registry{
extSvcProviders: map[string]extsvcauth.AuthProvider{},
features: features,
lock: sync.Mutex{},
logger: log.New("extsvcauth.registry"),
oauthReg: oauthServer,
saReg: saSvc,
serverLock: serverLock,
}
@@ -70,11 +70,6 @@ func (r *Registry) CleanUpOrphanedExternalServices(ctx context.Context) error {
errCleanUp = err
return
}
case extsvcauth.OAuth2Server:
if err := r.oauthReg.RemoveExternalService(ctx, name); err != nil {
errCleanUp = err
return
}
}
}
}
@@ -121,13 +116,6 @@ func (r *Registry) RemoveExternalService(ctx context.Context, name string) error
}
r.logger.Debug("Routing External Service removal to the External Service Account service", "service", name)
return r.saReg.RemoveExternalService(ctx, name)
case extsvcauth.OAuth2Server:
if !r.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAuth) {
r.logger.Debug("Skipping External Service removal, flag disabled", "service", name, "flag", featuremgmt.FlagExternalServiceAccounts)
return nil
}
r.logger.Debug("Routing External Service removal to the OAuth2Server", "service", name)
return r.oauthReg.RemoveExternalService(ctx, name)
default:
return extsvcauth.ErrUnknownProvider.Errorf("unknown provider '%v'", provider)
}
@@ -157,13 +145,6 @@ func (r *Registry) SaveExternalService(ctx context.Context, cmd *extsvcauth.Exte
}
r.logger.Debug("Routing the External Service registration to the External Service Account service", "service", cmd.Name)
extSvc, errSave = r.saReg.SaveExternalService(ctx, cmd)
case extsvcauth.OAuth2Server:
if !r.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAuth) {
r.logger.Warn("Skipping External Service authentication, flag disabled", "service", cmd.Name, "flag", featuremgmt.FlagExternalServiceAuth)
return
}
r.logger.Debug("Routing the External Service registration to the OAuth2Server", "service", cmd.Name)
extSvc, errSave = r.oauthReg.SaveExternalService(ctx, cmd)
default:
errSave = extsvcauth.ErrUnknownProvider.Errorf("unknown provider '%v'", cmd.AuthProvider)
}
@@ -187,16 +168,7 @@ func (r *Registry) retrieveExtSvcProviders(ctx context.Context) (map[string]exts
extsvcs[names[i]] = extsvcauth.ServiceAccounts
}
}
// Important to run this second as the OAuth server uses External Service Accounts as well.
if r.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAuth) {
names, err := r.oauthReg.GetExternalServiceNames(ctx)
if err != nil {
return nil, err
}
for i := range names {
extsvcs[names[i]] = extsvcauth.OAuth2Server
}
}
return extsvcs, nil
}
pkg/services/extsvcauth/registry/service_test.go
+7 -56
View File
@@ -14,9 +14,8 @@ import (
)
type TestEnv struct {
r *Registry
oauthReg *tests.ExternalServiceRegistryMock
saReg *tests.ExternalServiceRegistryMock
r *Registry
saReg *tests.ExternalServiceRegistryMock
}
// Never lock in tests
@@ -29,12 +28,10 @@ func (f *fakeServerLock) LockExecuteAndReleaseWithRetries(ctx context.Context, a
func setupTestEnv(t *testing.T) *TestEnv {
env := TestEnv{}
env.oauthReg = tests.NewExternalServiceRegistryMock(t)
env.saReg = tests.NewExternalServiceRegistryMock(t)
env.r = &Registry{
features: featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAuth, featuremgmt.FlagExternalServiceAccounts),
features: featuremgmt.WithFeatures(featuremgmt.FlagExternalServiceAccounts),
logger: log.New("extsvcauth.registry.test"),
oauthReg: env.oauthReg,
saReg: env.saReg,
extSvcProviders: map[string]extsvcauth.AuthProvider{},
serverLock: &fakeServerLock{},
@@ -51,39 +48,24 @@ func TestRegistry_CleanUpOrphanedExternalServices(t *testing.T) {
name: "should not clean up when every service registered",
init: func(te *TestEnv) {
// Have registered two services one requested a service account, the other requested to be an oauth client
te.r.extSvcProviders = map[string]extsvcauth.AuthProvider{"sa-svc": extsvcauth.ServiceAccounts, "oauth-svc": extsvcauth.OAuth2Server}
te.r.extSvcProviders = map[string]extsvcauth.AuthProvider{"sa-svc": extsvcauth.ServiceAccounts}
te.oauthReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"oauth-svc"}, nil)
// Also return the external service account attached to the OAuth Server
te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc", "oauth-svc"}, nil)
te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc"}, nil)
},
},
{
name: "should clean up an orphaned service account",
init: func(te *TestEnv) {
// Have registered two services one requested a service account, the other requested to be an oauth client
te.r.extSvcProviders = map[string]extsvcauth.AuthProvider{"sa-svc": extsvcauth.ServiceAccounts, "oauth-svc": extsvcauth.OAuth2Server}
te.r.extSvcProviders = map[string]extsvcauth.AuthProvider{"sa-svc": extsvcauth.ServiceAccounts}
te.oauthReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"oauth-svc"}, nil)
// Also return the external service account attached to the OAuth Server
te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc", "orphaned-sa-svc", "oauth-svc"}, nil)
te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc", "orphaned-sa-svc"}, nil)
te.saReg.On("RemoveExternalService", mock.Anything, "orphaned-sa-svc").Return(nil)
},
},
{
name: "should clean up an orphaned OAuth Client",
init: func(te *TestEnv) {
// Have registered two services one requested a service account, the other requested to be an oauth client
te.r.extSvcProviders = map[string]extsvcauth.AuthProvider{"sa-svc": extsvcauth.ServiceAccounts, "oauth-svc": extsvcauth.OAuth2Server}
te.oauthReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"oauth-svc", "orphaned-oauth-svc"}, nil)
// Also return the external service account attached to the OAuth Server
te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc", "orphaned-oauth-svc", "oauth-svc"}, nil)
te.oauthReg.On("RemoveExternalService", mock.Anything, "orphaned-oauth-svc").Return(nil)
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -93,37 +75,6 @@ func TestRegistry_CleanUpOrphanedExternalServices(t *testing.T) {
err := env.r.CleanUpOrphanedExternalServices(context.Background())
require.NoError(t, err)
env.oauthReg.AssertExpectations(t)
env.saReg.AssertExpectations(t)
})
}
}
func TestRegistry_GetExternalServiceNames(t *testing.T) {
tests := []struct {
name string
init func(*TestEnv)
want []string
}{
{
name: "should deduplicate names",
init: func(te *TestEnv) {
te.saReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"sa-svc", "oauth-svc"}, nil)
te.oauthReg.On("GetExternalServiceNames", mock.Anything).Return([]string{"oauth-svc"}, nil)
},
want: []string{"sa-svc", "oauth-svc"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
env := setupTestEnv(t)
tt.init(env)
names, err := env.r.GetExternalServiceNames(context.Background())
require.NoError(t, err)
require.ElementsMatch(t, tt.want, names)
env.oauthReg.AssertExpectations(t)
env.saReg.AssertExpectations(t)
})
}