From e902d8fd1021c594ae267166a7c92a79062a6d44 Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Thu, 5 Oct 2023 18:13:06 +0200 Subject: [PATCH] AuthN: New service to support multiple authentication providers for plugins (#75979) * OnGoing * Continue migrating structure * Comment * Add intermediary service * Remove unused error so far * no need for fmt use errors * use RoleNone * Docs * Fix test * Accounting for review feedback * Rename oauthserver.ExternalService to OAuthClient * Revert as the interface looks weird * Update pluginintegration * Rename oauthserver.ExternalService * closer to what it was before --- pkg/server/wire.go | 6 +- pkg/services/extsvcauth/errors.go | 7 ++ pkg/services/extsvcauth/extsvcauth.go | 1 - pkg/services/extsvcauth/models.go | 91 ++++++++++++++++ .../oauthserver/external_service.go | 70 ++++++------ .../oauthserver/external_service_test.go | 47 ++++---- pkg/services/extsvcauth/oauthserver/models.go | 52 ++------- .../oasimpl/aggregate_store_test.go | 4 +- .../extsvcauth/oauthserver/oasimpl/service.go | 32 +++--- .../oauthserver/oasimpl/service_test.go | 103 +++++++++--------- .../extsvcauth/oauthserver/oasimpl/token.go | 4 +- .../oauthserver/oasimpl/token_test.go | 30 ++--- .../extsvcauth/oauthserver/oastest/fakes.go | 9 +- .../oauthserver/oastest/store_mock.go | 28 ++--- .../extsvcauth/oauthserver/store/database.go | 26 ++--- .../oauthserver/store/database_test.go | 42 +++---- pkg/services/extsvcauth/registry/service.go | 43 ++++++++ .../serviceregistration.go | 30 +++-- 18 files changed, 369 insertions(+), 256 deletions(-) create mode 100644 pkg/services/extsvcauth/errors.go delete mode 100644 pkg/services/extsvcauth/extsvcauth.go create mode 100644 pkg/services/extsvcauth/models.go create mode 100644 pkg/services/extsvcauth/registry/service.go diff --git a/pkg/server/wire.go b/pkg/server/wire.go index f38ae1d8467..930647c0079 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -63,8 +63,10 @@ import ( datasourceservice "github.com/grafana/grafana/pkg/services/datasources/service" "github.com/grafana/grafana/pkg/services/encryption" encryptionservice "github.com/grafana/grafana/pkg/services/encryption/service" + "github.com/grafana/grafana/pkg/services/extsvcauth" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/oasimpl" + extsvcreg "github.com/grafana/grafana/pkg/services/extsvcauth/registry" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder/folderimpl" @@ -358,9 +360,11 @@ var wireBasicSet = wire.NewSet( authnimpl.ProvideAuthnService, supportbundlesimpl.ProvideService, oasimpl.ProvideService, + wire.Bind(new(oauthserver.OAuth2Server), new(*oasimpl.OAuth2ServiceImpl)), + extsvcreg.ProvideExtSvcRegistry, + wire.Bind(new(extsvcauth.ExternalServiceRegistry), new(*extsvcreg.Registry)), anonstore.ProvideAnonDBStore, wire.Bind(new(anonstore.AnonStore), new(*anonstore.AnonDBStore)), - wire.Bind(new(oauthserver.OAuth2Server), new(*oasimpl.OAuth2ServiceImpl)), loggermw.Provide, signingkeysimpl.ProvideEmbeddedSigningKeysService, wire.Bind(new(signingkeys.Service), new(*signingkeysimpl.Service)), diff --git a/pkg/services/extsvcauth/errors.go b/pkg/services/extsvcauth/errors.go new file mode 100644 index 00000000000..f1af61f4f0c --- /dev/null +++ b/pkg/services/extsvcauth/errors.go @@ -0,0 +1,7 @@ +package extsvcauth + +import "github.com/grafana/grafana/pkg/util/errutil" + +var ( + ErrUnknownProvider = errutil.BadRequest("extsvcauth.unknown-provider") +) diff --git a/pkg/services/extsvcauth/extsvcauth.go b/pkg/services/extsvcauth/extsvcauth.go deleted file mode 100644 index 745c80dbd4d..00000000000 --- a/pkg/services/extsvcauth/extsvcauth.go +++ /dev/null @@ -1 +0,0 @@ -package extsvcauth diff --git a/pkg/services/extsvcauth/models.go b/pkg/services/extsvcauth/models.go new file mode 100644 index 00000000000..efa9b4b8649 --- /dev/null +++ b/pkg/services/extsvcauth/models.go @@ -0,0 +1,91 @@ +package extsvcauth + +import ( + "context" + + "github.com/grafana/grafana/pkg/services/accesscontrol" +) + +const ( + OAuth2Server AuthProvider = "OAuth2Server" +) + +type AuthProvider string + +type ExternalServiceRegistry interface { + // SaveExternalService creates or updates an external service in the database. Based on the requested auth provider, + // it generates client_id, secrets and any additional provider specificities (ex: rsa keys). It also ensures that the + // associated service account has the correct permissions. + SaveExternalService(ctx context.Context, cmd *ExternalServiceRegistration) (*ExternalService, error) +} + +type SelfCfg struct { + // Enabled allows the service to request access tokens for itself + Enabled bool + // Permissions are the permissions that the external service needs its associated service account to have. + Permissions []accesscontrol.Permission +} + +type ImpersonationCfg struct { + // Enabled allows the service to request access tokens to impersonate users + Enabled bool + // Groups allows the service to list the impersonated user's teams + Groups bool + // Permissions are the permissions that the external service needs when impersonating a user. + // The intersection of this set with the impersonated user's permission guarantees that the client will not + // gain more privileges than the impersonated user has and vice versa. + Permissions []accesscontrol.Permission +} + +// ExternalServiceRegistration represents the registration form to save new client. +type ExternalServiceRegistration struct { + Name string + // Impersonation access configuration + // (this is not available on all auth providers) + Impersonation ImpersonationCfg + // Self access configuration + Self SelfCfg + // Auth Provider that the client will use to connect to Grafana + AuthProvider AuthProvider + // Auth Provider specific config + OAuthProviderCfg *OAuthProviderCfg +} + +// ExternalService represents the credentials that the ExternalService can use to connect to Grafana. +type ExternalService struct { + Name string + ID string + Secret string + OAuthExtra *OAuthExtra // Auth Provider specificities (ex: ecdsa key pair) +} + +type KeyOption struct { + // URL string `json:"url,omitempty"` // TODO allow specifying a URL (to a .jwks file) to fetch the key from + // PublicPEM contains the Base64 encoded public key in PEM format + PublicPEM string + Generate bool +} + +// ProviderCfg represents the registration form specificities needed to register OAuth2 clients. +type OAuthProviderCfg struct { + // RedirectURI is the URI that is used in the code flow. + // Note that this is not used yet. + RedirectURI *string + // Key is the option to specify a public key or ask the server to generate a crypto key pair. + Key *KeyOption +} + +type KeyResult struct { + URL string + PrivatePem string + PublicPem string + Generated bool +} + +// OAuthExtra represents the specificities of an OAuth2 client. +type OAuthExtra struct { + Audiences string + GrantTypes string + KeyResult *KeyResult + RedirectURI string +} diff --git a/pkg/services/extsvcauth/oauthserver/external_service.go b/pkg/services/extsvcauth/oauthserver/external_service.go index 8d0be07372d..bff2bf93466 100644 --- a/pkg/services/extsvcauth/oauthserver/external_service.go +++ b/pkg/services/extsvcauth/oauthserver/external_service.go @@ -8,27 +8,11 @@ import ( "github.com/ory/fosite" ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/extsvcauth" "github.com/grafana/grafana/pkg/services/user" ) -type KeyResult struct { - URL string `json:"url,omitempty"` - PrivatePem string `json:"private,omitempty"` - PublicPem string `json:"public,omitempty"` - Generated bool `json:"generated,omitempty"` -} - -type ExternalServiceDTO struct { - Name string `json:"name"` - ID string `json:"clientId"` - Secret string `json:"clientSecret"` - RedirectURI string `json:"redirectUri,omitempty"` // Not used yet (code flow) - GrantTypes string `json:"grantTypes"` // CSV value - Audiences string `json:"audiences"` // CSV value - KeyResult *KeyResult `json:"key,omitempty"` -} - -type ExternalService struct { +type OAuthExternalService struct { ID int64 `xorm:"id pk autoincr"` Name string `xorm:"name"` ClientID string `xorm:"client_id"` @@ -49,53 +33,61 @@ type ExternalService struct { ImpersonateScopes []string } -func (c *ExternalService) ToDTO() *ExternalServiceDTO { - c2 := ExternalServiceDTO{ - Name: c.Name, - ID: c.ClientID, - Secret: c.Secret, - GrantTypes: c.GrantTypes, - Audiences: c.Audiences, - RedirectURI: c.RedirectURI, +// ToExternalService converts the ExternalService (used internally by the oauthserver) to extsvcauth.ExternalService (used outside the package) +// If object must contain Key pairs, pass them as parameters, otherwise only the client PublicPem will be added. +func (c *OAuthExternalService) ToExternalService(keys *extsvcauth.KeyResult) *extsvcauth.ExternalService { + c2 := &extsvcauth.ExternalService{ + ID: c.ClientID, + Name: c.Name, + Secret: c.Secret, + OAuthExtra: &extsvcauth.OAuthExtra{ + GrantTypes: c.GrantTypes, + Audiences: c.Audiences, + RedirectURI: c.RedirectURI, + KeyResult: keys, + }, } - if len(c.PublicPem) > 0 { - c2.KeyResult = &KeyResult{PublicPem: string(c.PublicPem)} + + // Fallback to only display the public pem + if keys == nil && len(c.PublicPem) > 0 { + c2.OAuthExtra.KeyResult = &extsvcauth.KeyResult{PublicPem: string(c.PublicPem)} } - return &c2 + + return c2 } -func (c *ExternalService) LogID() string { +func (c *OAuthExternalService) LogID() string { return "{name: " + c.Name + ", clientID: " + c.ClientID + "}" } // GetID returns the client ID. -func (c *ExternalService) GetID() string { return c.ClientID } +func (c *OAuthExternalService) GetID() string { return c.ClientID } // GetHashedSecret returns the hashed secret as it is stored in the store. -func (c *ExternalService) GetHashedSecret() []byte { +func (c *OAuthExternalService) GetHashedSecret() []byte { // Hashed version is stored in the secret field return []byte(c.Secret) } // GetRedirectURIs returns the client's allowed redirect URIs. -func (c *ExternalService) GetRedirectURIs() []string { +func (c *OAuthExternalService) GetRedirectURIs() []string { return []string{c.RedirectURI} } // GetGrantTypes returns the client's allowed grant types. -func (c *ExternalService) GetGrantTypes() fosite.Arguments { +func (c *OAuthExternalService) GetGrantTypes() fosite.Arguments { return strings.Split(c.GrantTypes, ",") } // GetResponseTypes returns the client's allowed response types. // All allowed combinations of response types have to be listed, each combination having // response types of the combination separated by a space. -func (c *ExternalService) GetResponseTypes() fosite.Arguments { +func (c *OAuthExternalService) GetResponseTypes() fosite.Arguments { return fosite.Arguments{"code"} } // GetScopes returns the scopes this client is allowed to request on its own behalf. -func (c *ExternalService) GetScopes() fosite.Arguments { +func (c *OAuthExternalService) GetScopes() fosite.Arguments { if c.Scopes != nil { return c.Scopes } @@ -114,7 +106,7 @@ func (c *ExternalService) GetScopes() fosite.Arguments { } // GetScopes returns the scopes this client is allowed to request on a specific user. -func (c *ExternalService) GetScopesOnUser(ctx context.Context, accessControl ac.AccessControl, userID int64) []string { +func (c *OAuthExternalService) GetScopesOnUser(ctx context.Context, accessControl ac.AccessControl, userID int64) []string { ev := ac.EvalPermission(ac.ActionUsersImpersonate, ac.Scope("users", "id", strconv.FormatInt(userID, 10))) hasAccess, errAccess := accessControl.Evaluate(ctx, c.SignedInUser, ev) if errAccess != nil || !hasAccess { @@ -151,11 +143,11 @@ func (c *ExternalService) GetScopesOnUser(ctx context.Context, accessControl ac. } // IsPublic returns true, if this client is marked as public. -func (c *ExternalService) IsPublic() bool { +func (c *OAuthExternalService) IsPublic() bool { return false } // GetAudience returns the allowed audience(s) for this client. -func (c *ExternalService) GetAudience() fosite.Arguments { +func (c *OAuthExternalService) GetAudience() fosite.Arguments { return strings.Split(c.Audiences, ",") } diff --git a/pkg/services/extsvcauth/oauthserver/external_service_test.go b/pkg/services/extsvcauth/oauthserver/external_service_test.go index ab5e562b59e..67e40117387 100644 --- a/pkg/services/extsvcauth/oauthserver/external_service_test.go +++ b/pkg/services/extsvcauth/oauthserver/external_service_test.go @@ -13,10 +13,10 @@ import ( "github.com/grafana/grafana/pkg/setting" ) -func setupTestEnv(t *testing.T) *ExternalService { +func setupTestEnv(t *testing.T) *OAuthExternalService { t.Helper() - client := &ExternalService{ + client := &OAuthExternalService{ Name: "my-ext-service", ClientID: "RANDOMID", Secret: "RANDOMSECRET", @@ -37,7 +37,7 @@ func TestExternalService_GetScopesOnUser(t *testing.T) { testCases := []struct { name string impersonatePermissions []ac.Permission - initTestEnv func(*ExternalService) + initTestEnv func(*OAuthExternalService) expectedScopes []string }{ { @@ -46,7 +46,7 @@ func TestExternalService_GetScopesOnUser(t *testing.T) { }, { name: "should return the 'profile', 'email' and associated RBAC action", - initTestEnv: func(c *ExternalService) { + initTestEnv: func(c *OAuthExternalService) { c.SignedInUser.Permissions = map[int64]map[string][]string{ 1: { ac.ActionUsersImpersonate: {ac.ScopeUsersAll}, @@ -60,7 +60,7 @@ func TestExternalService_GetScopesOnUser(t *testing.T) { }, { name: "should return 'entitlements' and associated RBAC action scopes", - initTestEnv: func(c *ExternalService) { + initTestEnv: func(c *OAuthExternalService) { c.SignedInUser.Permissions = map[int64]map[string][]string{ 1: { ac.ActionUsersImpersonate: {ac.ScopeUsersAll}, @@ -74,7 +74,7 @@ func TestExternalService_GetScopesOnUser(t *testing.T) { }, { name: "should return 'groups' and associated RBAC action scopes", - initTestEnv: func(c *ExternalService) { + initTestEnv: func(c *OAuthExternalService) { c.SignedInUser.Permissions = map[int64]map[string][]string{ 1: { ac.ActionUsersImpersonate: {ac.ScopeUsersAll}, @@ -88,7 +88,7 @@ func TestExternalService_GetScopesOnUser(t *testing.T) { }, { name: "should return all scopes", - initTestEnv: func(c *ExternalService) { + initTestEnv: func(c *OAuthExternalService) { c.SignedInUser.Permissions = map[int64]map[string][]string{ 1: { ac.ActionUsersImpersonate: {ac.ScopeUsersAll}, @@ -108,7 +108,7 @@ func TestExternalService_GetScopesOnUser(t *testing.T) { }, { name: "should return stored scopes when the client's impersonate scopes has already been set", - initTestEnv: func(c *ExternalService) { + initTestEnv: func(c *OAuthExternalService) { c.SignedInUser.Permissions = map[int64]map[string][]string{ 1: { ac.ActionUsersImpersonate: {ac.ScopeUsersAll}, @@ -135,26 +135,26 @@ func TestExternalService_GetScopes(t *testing.T) { testCases := []struct { name string impersonatePermissions []ac.Permission - initTestEnv func(*ExternalService) + initTestEnv func(*OAuthExternalService) expectedScopes []string }{ { name: "should return default scopes when the signed in user is nil", - initTestEnv: func(c *ExternalService) { + initTestEnv: func(c *OAuthExternalService) { c.SignedInUser = nil }, expectedScopes: []string{"profile", "email", "entitlements", "groups"}, }, { name: "should return default scopes when the signed in user has no permissions", - initTestEnv: func(c *ExternalService) { + initTestEnv: func(c *OAuthExternalService) { c.SignedInUser.Permissions = map[int64]map[string][]string{} }, expectedScopes: []string{"profile", "email", "entitlements", "groups"}, }, { name: "should return additional scopes from signed in user's permissions", - initTestEnv: func(c *ExternalService) { + initTestEnv: func(c *OAuthExternalService) { c.SignedInUser.Permissions = map[int64]map[string][]string{ 1: { dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}, @@ -165,7 +165,7 @@ func TestExternalService_GetScopes(t *testing.T) { }, { name: "should return stored scopes when the client's scopes has already been set", - initTestEnv: func(c *ExternalService) { + initTestEnv: func(c *OAuthExternalService) { c.Scopes = []string{"profile", "email", "entitlements", "groups"} }, expectedScopes: []string{"profile", "email", "entitlements", "groups"}, @@ -184,7 +184,7 @@ func TestExternalService_GetScopes(t *testing.T) { } func TestExternalService_ToDTO(t *testing.T) { - client := &ExternalService{ + client := &OAuthExternalService{ ID: 1, Name: "my-ext-service", ClientID: "test", @@ -195,16 +195,19 @@ func TestExternalService_ToDTO(t *testing.T) { PublicPem: []byte("pem_encoded_public_key"), } - dto := client.ToDTO() + dto := client.ToExternalService(nil) require.Equal(t, client.ClientID, dto.ID) require.Equal(t, client.Name, dto.Name) - require.Equal(t, client.RedirectURI, dto.RedirectURI) - require.Equal(t, client.GrantTypes, dto.GrantTypes) - require.Equal(t, client.Audiences, dto.Audiences) - require.Equal(t, client.PublicPem, []byte(dto.KeyResult.PublicPem)) - require.Empty(t, dto.KeyResult.PrivatePem) - require.Empty(t, dto.KeyResult.URL) - require.False(t, dto.KeyResult.Generated) require.Equal(t, client.Secret, dto.Secret) + + require.NotNil(t, dto.OAuthExtra) + + require.Equal(t, client.RedirectURI, dto.OAuthExtra.RedirectURI) + require.Equal(t, client.GrantTypes, dto.OAuthExtra.GrantTypes) + require.Equal(t, client.Audiences, dto.OAuthExtra.Audiences) + require.Equal(t, client.PublicPem, []byte(dto.OAuthExtra.KeyResult.PublicPem)) + require.Empty(t, dto.OAuthExtra.KeyResult.PrivatePem) + require.Empty(t, dto.OAuthExtra.KeyResult.URL) + require.False(t, dto.OAuthExtra.KeyResult.Generated) } diff --git a/pkg/services/extsvcauth/oauthserver/models.go b/pkg/services/extsvcauth/oauthserver/models.go index 7eee4c57920..606086895db 100644 --- a/pkg/services/extsvcauth/oauthserver/models.go +++ b/pkg/services/extsvcauth/oauthserver/models.go @@ -4,7 +4,7 @@ import ( "context" "net/http" - "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/extsvcauth" "gopkg.in/square/go-jose.v2" ) @@ -29,10 +29,10 @@ const ( type OAuth2Server interface { // SaveExternalService creates or updates an external service in the database, it generates client_id and secrets and // it ensures that the associated service account has the correct permissions. - SaveExternalService(ctx context.Context, cmd *ExternalServiceRegistration) (*ExternalServiceDTO, error) + SaveExternalService(ctx context.Context, cmd *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) // GetExternalService retrieves an external service from store by client_id. It populates the SelfPermissions and // SignedInUser from the associated service account. - GetExternalService(ctx context.Context, id string) (*ExternalService, error) + GetExternalService(ctx context.Context, id string) (*OAuthExternalService, error) // HandleTokenRequest handles the client's OAuth2 query to obtain an access_token by presenting its authorization // grant (ex: client_credentials, jwtbearer). @@ -45,47 +45,9 @@ type OAuth2Server interface { //go:generate mockery --name Store --structname MockStore --outpkg oauthtest --filename store_mock.go --output ./oauthtest/ type Store interface { - RegisterExternalService(ctx context.Context, client *ExternalService) error - SaveExternalService(ctx context.Context, client *ExternalService) error - GetExternalService(ctx context.Context, id string) (*ExternalService, error) - GetExternalServiceByName(ctx context.Context, name string) (*ExternalService, error) + RegisterExternalService(ctx context.Context, client *OAuthExternalService) error + SaveExternalService(ctx context.Context, client *OAuthExternalService) error + GetExternalService(ctx context.Context, id string) (*OAuthExternalService, error) + GetExternalServiceByName(ctx context.Context, name string) (*OAuthExternalService, error) GetExternalServicePublicKey(ctx context.Context, clientID string) (*jose.JSONWebKey, error) } - -type KeyOption struct { - // URL string `json:"url,omitempty"` // TODO allow specifying a URL (to a .jwks file) to fetch the key from - // PublicPEM contains the Base64 encoded public key in PEM format - PublicPEM string `json:"public_pem,omitempty"` - Generate bool `json:"generate,omitempty"` -} - -type SelfCfg struct { - // Enabled allows the service to request access tokens for itself using the client_credentials grant - Enabled bool `json:"enabled"` - // Permissions are the permissions that the external service needs its associated service account to have. - Permissions []accesscontrol.Permission `json:"permissions,omitempty"` -} -type ImpersonationCfg struct { - // Enabled allows the service to request access tokens to impersonate users using the jwtbearer grant - Enabled bool `json:"enabled"` - // Groups allows the service to list the impersonated user's teams - Groups bool `json:"groups"` - // Permissions are the permissions that the external service needs when impersonating a user. - // The intersection of this set with the impersonated user's permission guarantees that the client will not - // gain more privileges than the impersonated user has. - Permissions []accesscontrol.Permission `json:"permissions,omitempty"` -} - -// ExternalServiceRegistration represents the registration form to save new OAuth2 client. -type ExternalServiceRegistration struct { - Name string `json:"name"` - // RedirectURI is the URI that is used in the code flow. - // Note that this is not used yet. - RedirectURI *string `json:"redirectUri,omitempty"` - // Impersonation access configuration - Impersonation ImpersonationCfg `json:"impersonation"` - // Self access configuration - Self SelfCfg `json:"self"` - // Key is the option to specify a public key or ask the server to generate a crypto key pair. - Key *KeyOption `json:"key,omitempty"` -} diff --git a/pkg/services/extsvcauth/oauthserver/oasimpl/aggregate_store_test.go b/pkg/services/extsvcauth/oauthserver/oasimpl/aggregate_store_test.go index bc49297b9d0..b5899c87601 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/aggregate_store_test.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/aggregate_store_test.go @@ -13,8 +13,8 @@ import ( "github.com/grafana/grafana/pkg/services/user" ) -var cachedExternalService = func() *oauthserver.ExternalService { - return &oauthserver.ExternalService{ +var cachedExternalService = func() *oauthserver.OAuthExternalService { + return &oauthserver.OAuthExternalService{ Name: "my-ext-service", ClientID: "RANDOMID", Secret: "RANDOMSECRET", diff --git a/pkg/services/extsvcauth/oauthserver/oasimpl/service.go b/pkg/services/extsvcauth/oauthserver/oasimpl/service.go index 440cddb13ee..25e53cbeaa4 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/service.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/service.go @@ -28,6 +28,7 @@ import ( "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/models/roletype" ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/extsvcauth" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/api" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/store" @@ -120,10 +121,10 @@ func newProvider(config *fosite.Config, storage any, signingKeyService signingke // GetExternalService retrieves an external service from store by client_id. It populates the SelfPermissions and // SignedInUser from the associated service account. // For performance reason, the service uses caching. -func (s *OAuth2ServiceImpl) GetExternalService(ctx context.Context, id string) (*oauthserver.ExternalService, error) { +func (s *OAuth2ServiceImpl) GetExternalService(ctx context.Context, id string) (*oauthserver.OAuthExternalService, error) { entry, ok := s.cache.Get(id) if ok { - client, ok := entry.(oauthserver.ExternalService) + client, ok := entry.(oauthserver.OAuthExternalService) if ok { s.logger.Debug("GetExternalService: cache hit", "id", id) return &client, nil @@ -178,7 +179,7 @@ func (s *OAuth2ServiceImpl) GetExternalService(ctx context.Context, id string) ( // SaveExternalService creates or updates an external service in the database, it generates client_id and secrets and // it ensures that the associated service account has the correct permissions. // Database consistency is not guaranteed, consider changing this in the future. -func (s *OAuth2ServiceImpl) SaveExternalService(ctx context.Context, registration *oauthserver.ExternalServiceRegistration) (*oauthserver.ExternalServiceDTO, error) { +func (s *OAuth2ServiceImpl) SaveExternalService(ctx context.Context, registration *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) { if registration == nil { s.logger.Warn("RegisterExternalService called without registration") return nil, nil @@ -199,7 +200,7 @@ func (s *OAuth2ServiceImpl) SaveExternalService(ctx context.Context, registratio // Otherwise, create a new client if client == nil { s.logger.Debug("External service does not yet exist", "external service name", registration.Name) - client = &oauthserver.ExternalService{ + client = &oauthserver.OAuthExternalService{ Name: registration.Name, ServiceAccountID: oauthserver.NoServiceAccountID, Audiences: s.cfg.AppURL, @@ -209,8 +210,12 @@ func (s *OAuth2ServiceImpl) SaveExternalService(ctx context.Context, registratio // Parse registration form to compute required permissions for the client client.SelfPermissions, client.ImpersonatePermissions = s.handleRegistrationPermissions(registration) - if registration.RedirectURI != nil { - client.RedirectURI = *registration.RedirectURI + if registration.OAuthProviderCfg == nil { + return nil, errors.New("missing oauth provider configuration") + } + + if registration.OAuthProviderCfg.RedirectURI != nil { + client.RedirectURI = *registration.OAuthProviderCfg.RedirectURI } var errGenCred error @@ -232,7 +237,7 @@ func (s *OAuth2ServiceImpl) SaveExternalService(ctx context.Context, registratio // Handle key options s.logger.Debug("Handle key options") - keys, err := s.handleKeyOptions(ctx, registration.Key) + keys, err := s.handleKeyOptions(ctx, registration.OAuthProviderCfg.Key) if err != nil { s.logger.Error("Error handling key options", "client", client.LogID(), "error", err) return nil, err @@ -240,8 +245,7 @@ func (s *OAuth2ServiceImpl) SaveExternalService(ctx context.Context, registratio if keys != nil { client.PublicPem = []byte(keys.PublicPem) } - dto := client.ToDTO() - dto.KeyResult = keys + dto := client.ToExternalService(keys) hashedSecret, err := bcrypt.GenerateFromPassword([]byte(client.Secret), bcrypt.DefaultCost) if err != nil { @@ -295,7 +299,7 @@ func (s *OAuth2ServiceImpl) computeGrantTypes(selfAccessEnabled, impersonationEn return grantTypes } -func (s *OAuth2ServiceImpl) handleKeyOptions(ctx context.Context, keyOption *oauthserver.KeyOption) (*oauthserver.KeyResult, error) { +func (s *OAuth2ServiceImpl) handleKeyOptions(ctx context.Context, keyOption *extsvcauth.KeyOption) (*extsvcauth.KeyResult, error) { if keyOption == nil { return nil, fmt.Errorf("keyOption is nil") } @@ -344,7 +348,7 @@ func (s *OAuth2ServiceImpl) handleKeyOptions(ctx context.Context, keyOption *oau s.logger.Debug("ECDSA key has been generated") } - return &oauthserver.KeyResult{ + return &extsvcauth.KeyResult{ PrivatePem: privatePem, PublicPem: publicPem, Generated: true, @@ -368,7 +372,7 @@ func (s *OAuth2ServiceImpl) handleKeyOptions(ctx context.Context, keyOption *oau s.logger.Error("Cannot parse PEM encoded string", "error", err) return nil, err } - return &oauthserver.KeyResult{ + return &extsvcauth.KeyResult{ PublicPem: string(pemEncoded), }, nil } @@ -444,7 +448,7 @@ func (s *OAuth2ServiceImpl) createServiceAccount(ctx context.Context, extSvcName s.logger.Debug("Generate service account", "external service name", extSvcName, "orgID", oauthserver.TmpOrgID, "name", slug) sa, err := s.saService.CreateServiceAccount(ctx, oauthserver.TmpOrgID, &serviceaccounts.CreateServiceAccountForm{ Name: slug, - Role: newRole(roletype.RoleViewer), // FIXME: Use empty role + Role: newRole(roletype.RoleNone), IsDisabled: newBool(false), }) if err != nil { @@ -467,7 +471,7 @@ func (s *OAuth2ServiceImpl) createServiceAccount(ctx context.Context, extSvcName // handleRegistrationPermissions parses the registration form to retrieve requested permissions and adds default // permissions when impersonation is requested -func (*OAuth2ServiceImpl) handleRegistrationPermissions(registration *oauthserver.ExternalServiceRegistration) ([]ac.Permission, []ac.Permission) { +func (*OAuth2ServiceImpl) handleRegistrationPermissions(registration *extsvcauth.ExternalServiceRegistration) ([]ac.Permission, []ac.Permission) { selfPermissions := []ac.Permission{} impersonatePermissions := []ac.Permission{} diff --git a/pkg/services/extsvcauth/oauthserver/oasimpl/service_test.go b/pkg/services/extsvcauth/oauthserver/oasimpl/service_test.go index 1ff9fdce4f7..6f57f581cbb 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/service_test.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/service_test.go @@ -22,6 +22,7 @@ import ( ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" + "github.com/grafana/grafana/pkg/services/extsvcauth" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver/oastest" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -105,12 +106,12 @@ func setupTestEnv(t *testing.T) *TestEnv { func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { const serviceName = "my-ext-service" - sa1 := sa.ServiceAccountDTO{Id: 1, Name: serviceName, Login: serviceName, OrgId: oauthserver.TmpOrgID, IsDisabled: false, Role: "Viewer"} - sa1Profile := sa.ServiceAccountProfileDTO{Id: 1, Name: serviceName, Login: serviceName, OrgId: oauthserver.TmpOrgID, IsDisabled: false, Role: "Viewer"} + sa1 := sa.ServiceAccountDTO{Id: 1, Name: serviceName, Login: serviceName, OrgId: oauthserver.TmpOrgID, IsDisabled: false, Role: "None"} + sa1Profile := sa.ServiceAccountProfileDTO{Id: 1, Name: serviceName, Login: serviceName, OrgId: oauthserver.TmpOrgID, IsDisabled: false, Role: "None"} prevSaID := int64(3) // Using a function to prevent modifying the same object in the tests - client1 := func() *oauthserver.ExternalService { - return &oauthserver.ExternalService{ + client1 := func() *oauthserver.OAuthExternalService { + return &oauthserver.OAuthExternalService{ Name: serviceName, ClientID: "RANDOMID", Secret: "RANDOMSECRET", @@ -124,7 +125,7 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { tests := []struct { name string init func(*TestEnv) - cmd *oauthserver.ExternalServiceRegistration + cmd *extsvcauth.ExternalServiceRegistration mockChecks func(*testing.T, *TestEnv) wantErr bool }{ @@ -135,15 +136,15 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { env.OAuthStore.On("GetExternalServiceByName", mock.Anything, mock.Anything).Return(nil, oauthserver.ErrClientNotFound(serviceName)) env.OAuthStore.On("SaveExternalService", mock.Anything, mock.Anything).Return(nil) }, - cmd: &oauthserver.ExternalServiceRegistration{ - Name: serviceName, - Key: &oauthserver.KeyOption{Generate: true}, + cmd: &extsvcauth.ExternalServiceRegistration{ + Name: serviceName, + OAuthProviderCfg: &extsvcauth.OAuthProviderCfg{Key: &extsvcauth.KeyOption{Generate: true}}, }, mockChecks: func(t *testing.T, env *TestEnv) { env.OAuthStore.AssertCalled(t, "GetExternalServiceByName", mock.Anything, mock.MatchedBy(func(name string) bool { return name == serviceName })) - env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.ExternalService) bool { + env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.OAuthExternalService) bool { return client.Name == serviceName && client.ClientID != "" && client.Secret != "" && len(client.GrantTypes) == 0 && len(client.PublicPem) > 0 && client.ServiceAccountID == 0 && len(client.ImpersonatePermissions) == 0 @@ -160,17 +161,17 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { env.SAService.On("CreateServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(&sa1, nil) env.AcStore.On("SaveExternalServiceRole", mock.Anything, mock.Anything).Return(nil) }, - cmd: &oauthserver.ExternalServiceRegistration{ - Name: serviceName, - Key: &oauthserver.KeyOption{Generate: true}, - Self: oauthserver.SelfCfg{ + cmd: &extsvcauth.ExternalServiceRegistration{ + Name: serviceName, + OAuthProviderCfg: &extsvcauth.OAuthProviderCfg{Key: &extsvcauth.KeyOption{Generate: true}}, + Self: extsvcauth.SelfCfg{ Enabled: true, Permissions: []ac.Permission{{Action: "users:read", Scope: "users:*"}}, }, }, mockChecks: func(t *testing.T, env *TestEnv) { // Check that the client has a service account and the correct grant type - env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.ExternalService) bool { + env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.OAuthExternalService) bool { return client.Name == serviceName && client.GrantTypes == "client_credentials" && client.ServiceAccountID == sa1.Id })) @@ -178,7 +179,7 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { env.SAService.AssertCalled(t, "CreateServiceAccount", mock.Anything, mock.MatchedBy(func(orgID int64) bool { return orgID == oauthserver.TmpOrgID }), mock.MatchedBy(func(cmd *sa.CreateServiceAccountForm) bool { - return cmd.Name == serviceName && *cmd.Role == roletype.RoleViewer + return cmd.Name == serviceName && *cmd.Role == roletype.RoleNone }), ) }, @@ -194,16 +195,16 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { env.SAService.On("DeleteServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(nil) env.AcStore.On("DeleteExternalServiceRole", mock.Anything, mock.Anything).Return(nil) }, - cmd: &oauthserver.ExternalServiceRegistration{ - Name: serviceName, - Key: &oauthserver.KeyOption{Generate: true}, - Self: oauthserver.SelfCfg{ + cmd: &extsvcauth.ExternalServiceRegistration{ + Name: serviceName, + OAuthProviderCfg: &extsvcauth.OAuthProviderCfg{Key: &extsvcauth.KeyOption{Generate: true}}, + Self: extsvcauth.SelfCfg{ Enabled: false, }, }, mockChecks: func(t *testing.T, env *TestEnv) { // Check that the service has no service account anymore - env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.ExternalService) bool { + env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.OAuthExternalService) bool { return client.Name == serviceName && client.ServiceAccountID == oauthserver.NoServiceAccountID })) // Check that the service account is retrieved with the correct ID @@ -229,10 +230,10 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { // Update the service account permissions env.AcStore.On("SaveExternalServiceRole", mock.Anything, mock.Anything).Return(nil) }, - cmd: &oauthserver.ExternalServiceRegistration{ - Name: serviceName, - Key: &oauthserver.KeyOption{Generate: true}, - Self: oauthserver.SelfCfg{ + cmd: &extsvcauth.ExternalServiceRegistration{ + Name: serviceName, + OAuthProviderCfg: &extsvcauth.OAuthProviderCfg{Key: &extsvcauth.KeyOption{Generate: true}}, + Self: extsvcauth.SelfCfg{ Enabled: true, Permissions: []ac.Permission{{Action: "dashboards:create", Scope: "folders:uid:general"}}, }, @@ -257,10 +258,10 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { env.SAService.On("CreateServiceAccount", mock.Anything, mock.Anything, mock.Anything).Return(&sa1, nil) env.AcStore.On("SaveExternalServiceRole", mock.Anything, mock.Anything).Return(nil) }, - cmd: &oauthserver.ExternalServiceRegistration{ - Name: serviceName, - Key: &oauthserver.KeyOption{Generate: true}, - Impersonation: oauthserver.ImpersonationCfg{ + cmd: &extsvcauth.ExternalServiceRegistration{ + Name: serviceName, + OAuthProviderCfg: &extsvcauth.OAuthProviderCfg{Key: &extsvcauth.KeyOption{Generate: true}}, + Impersonation: extsvcauth.ImpersonationCfg{ Enabled: true, Groups: true, Permissions: []ac.Permission{{Action: "dashboards:read", Scope: "dashboards:*"}}, @@ -268,7 +269,7 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { }, mockChecks: func(t *testing.T, env *TestEnv) { // Check that the external service impersonate permissions contains the default permissions required to populate the access token - env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.ExternalService) bool { + env.OAuthStore.AssertCalled(t, "SaveExternalService", mock.Anything, mock.MatchedBy(func(client *oauthserver.OAuthExternalService) bool { impPerm := client.ImpersonatePermissions return slices.Contains(impPerm, ac.Permission{Action: "dashboards:read", Scope: "dashboards:*"}) && slices.Contains(impPerm, ac.Permission{Action: ac.ActionUsersRead, Scope: oauthserver.ScopeGlobalUsersSelf}) && @@ -302,26 +303,26 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { require.NotEmpty(t, dto.Secret) // Check that we have generated keys and that we correctly return them - if tt.cmd.Key != nil && tt.cmd.Key.Generate { - require.NotNil(t, dto.KeyResult) - require.True(t, dto.KeyResult.Generated) - require.NotEmpty(t, dto.KeyResult.PublicPem) - require.NotEmpty(t, dto.KeyResult.PrivatePem) + if tt.cmd.OAuthProviderCfg.Key != nil && tt.cmd.OAuthProviderCfg.Key.Generate { + require.NotNil(t, dto.OAuthExtra.KeyResult) + require.True(t, dto.OAuthExtra.KeyResult.Generated) + require.NotEmpty(t, dto.OAuthExtra.KeyResult.PublicPem) + require.NotEmpty(t, dto.OAuthExtra.KeyResult.PrivatePem) } // Check that we computed grant types and created or updated the service account if tt.cmd.Self.Enabled { - require.NotNil(t, dto.GrantTypes) - require.Contains(t, dto.GrantTypes, fosite.GrantTypeClientCredentials, "grant types should contain client_credentials") + require.NotNil(t, dto.OAuthExtra.GrantTypes) + require.Contains(t, dto.OAuthExtra.GrantTypes, fosite.GrantTypeClientCredentials, "grant types should contain client_credentials") } else { - require.NotContains(t, dto.GrantTypes, fosite.GrantTypeClientCredentials, "grant types should not contain client_credentials") + require.NotContains(t, dto.OAuthExtra.GrantTypes, fosite.GrantTypeClientCredentials, "grant types should not contain client_credentials") } // Check that we updated grant types if tt.cmd.Impersonation.Enabled { - require.NotNil(t, dto.GrantTypes) - require.Contains(t, dto.GrantTypes, fosite.GrantTypeJWTBearer, "grant types should contain JWT Bearer grant") + require.NotNil(t, dto.OAuthExtra.GrantTypes) + require.Contains(t, dto.OAuthExtra.GrantTypes, fosite.GrantTypeJWTBearer, "grant types should contain JWT Bearer grant") } else { - require.NotContains(t, dto.GrantTypes, fosite.GrantTypeJWTBearer, "grant types should not contain JWT Bearer grant") + require.NotContains(t, dto.OAuthExtra.GrantTypes, fosite.GrantTypeJWTBearer, "grant types should not contain JWT Bearer grant") } // Check that mocks were called as expected @@ -340,8 +341,8 @@ func TestOAuth2ServiceImpl_SaveExternalService(t *testing.T) { func TestOAuth2ServiceImpl_GetExternalService(t *testing.T) { const serviceName = "my-ext-service" - dummyClient := func() *oauthserver.ExternalService { - return &oauthserver.ExternalService{ + dummyClient := func() *oauthserver.OAuthExternalService { + return &oauthserver.OAuthExternalService{ Name: serviceName, ClientID: "RANDOMID", Secret: "RANDOMSECRET", @@ -350,7 +351,7 @@ func TestOAuth2ServiceImpl_GetExternalService(t *testing.T) { ServiceAccountID: 1, } } - cachedClient := &oauthserver.ExternalService{ + cachedClient := &oauthserver.OAuthExternalService{ Name: serviceName, ClientID: "RANDOMID", Secret: "RANDOMSECRET", @@ -432,7 +433,7 @@ func TestOAuth2ServiceImpl_GetExternalService(t *testing.T) { { name: "should return correctly when the client has no service account", init: func(env *TestEnv) { - client := &oauthserver.ExternalService{ + client := &oauthserver.OAuthExternalService{ Name: serviceName, ClientID: "RANDOMID", Secret: "RANDOMSECRET", @@ -486,8 +487,8 @@ func assertArrayInMap[K comparable, V string](t *testing.T, m1 map[K][]V, m2 map func TestTestOAuth2ServiceImpl_handleKeyOptions(t *testing.T) { testCases := []struct { name string - keyOption *oauthserver.KeyOption - expectedResult *oauthserver.KeyResult + keyOption *extsvcauth.KeyOption + expectedResult *extsvcauth.KeyResult wantErr bool }{ { @@ -496,19 +497,19 @@ func TestTestOAuth2ServiceImpl_handleKeyOptions(t *testing.T) { }, { name: "should return error when the key option is empty", - keyOption: &oauthserver.KeyOption{}, + keyOption: &extsvcauth.KeyOption{}, wantErr: true, }, { name: "should return successfully when PublicPEM is specified", - keyOption: &oauthserver.KeyOption{ + keyOption: &extsvcauth.KeyOption{ PublicPEM: base64.StdEncoding.EncodeToString([]byte(`-----BEGIN PUBLIC KEY----- MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEbsGtoGJTopAIbhqy49/vyCJuDot+ mgGaC8vUIigFQVsVB+v/HZ4yG1Rcvysig+tyNk1dZQpozpFc2dGmzHlGhw== -----END PUBLIC KEY-----`)), }, wantErr: false, - expectedResult: &oauthserver.KeyResult{ + expectedResult: &extsvcauth.KeyResult{ PublicPem: `-----BEGIN PUBLIC KEY----- MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEbsGtoGJTopAIbhqy49/vyCJuDot+ mgGaC8vUIigFQVsVB+v/HZ4yG1Rcvysig+tyNk1dZQpozpFc2dGmzHlGhw== @@ -533,7 +534,7 @@ mgGaC8vUIigFQVsVB+v/HZ4yG1Rcvysig+tyNk1dZQpozpFc2dGmzHlGhw== } t.Run("should generate an ECDSA key pair (default) when generate key option is specified", func(t *testing.T) { - result, err := env.S.handleKeyOptions(context.Background(), &oauthserver.KeyOption{Generate: true}) + result, err := env.S.handleKeyOptions(context.Background(), &extsvcauth.KeyOption{Generate: true}) require.NoError(t, err) require.NotNil(t, result.PrivatePem) @@ -543,7 +544,7 @@ mgGaC8vUIigFQVsVB+v/HZ4yG1Rcvysig+tyNk1dZQpozpFc2dGmzHlGhw== t.Run("should generate an RSA key pair when generate key option is specified", func(t *testing.T) { env.S.cfg.OAuth2ServerGeneratedKeyTypeForClient = "RSA" - result, err := env.S.handleKeyOptions(context.Background(), &oauthserver.KeyOption{Generate: true}) + result, err := env.S.handleKeyOptions(context.Background(), &extsvcauth.KeyOption{Generate: true}) require.NoError(t, err) require.NotNil(t, result.PrivatePem) diff --git a/pkg/services/extsvcauth/oauthserver/oasimpl/token.go b/pkg/services/extsvcauth/oauthserver/oasimpl/token.go index 561e4e6089c..2dc18d6370d 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/token.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/token.go @@ -98,7 +98,7 @@ func splitOAuthScopes(requestedScopes fosite.Arguments) (map[string]bool, map[st // handleJWTBearer populates the "impersonation" access_token generated by fosite to match the rfc9068 specifications (entitlements, groups). // It ensures that the user can be impersonated, that the generated token audiences only contain Grafana's AppURL (and token endpoint) // and that entitlements solely contain the user's permissions that the client is allowed to have. -func (s *OAuth2ServiceImpl) handleJWTBearer(ctx context.Context, accessRequest fosite.AccessRequester, oauthSession *oauth2.JWTSession, client *oauthserver.ExternalService) error { +func (s *OAuth2ServiceImpl) handleJWTBearer(ctx context.Context, accessRequest fosite.AccessRequester, oauthSession *oauth2.JWTSession, client *oauthserver.OAuthExternalService) error { if !accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeJWTBearer)) { return nil } @@ -292,7 +292,7 @@ func (*OAuth2ServiceImpl) filteredImpersonatePermissions(impersonatePermissions } // handleClientCredentials populates the client's access_token generated by fosite to match the rfc9068 specifications (entitlements, groups) -func (s *OAuth2ServiceImpl) handleClientCredentials(ctx context.Context, accessRequest fosite.AccessRequester, oauthSession *oauth2.JWTSession, client *oauthserver.ExternalService) error { +func (s *OAuth2ServiceImpl) handleClientCredentials(ctx context.Context, accessRequest fosite.AccessRequester, oauthSession *oauth2.JWTSession, client *oauthserver.OAuthExternalService) error { if !accessRequest.GetGrantTypes().ExactOne("client_credentials") { return nil } diff --git a/pkg/services/extsvcauth/oauthserver/oasimpl/token_test.go b/pkg/services/extsvcauth/oauthserver/oasimpl/token_test.go index 6fb4a42903b..f8f4eb25cce 100644 --- a/pkg/services/extsvcauth/oauthserver/oasimpl/token_test.go +++ b/pkg/services/extsvcauth/oauthserver/oasimpl/token_test.go @@ -29,7 +29,7 @@ import ( ) func TestOAuth2ServiceImpl_handleClientCredentials(t *testing.T) { - client1 := &oauthserver.ExternalService{ + client1 := &oauthserver.OAuthExternalService{ Name: "testapp", ClientID: "RANDOMID", GrantTypes: string(fosite.GrantTypeClientCredentials), @@ -51,13 +51,13 @@ func TestOAuth2ServiceImpl_handleClientCredentials(t *testing.T) { tests := []struct { name string scopes []string - client *oauthserver.ExternalService + client *oauthserver.OAuthExternalService expectedClaims map[string]any wantErr bool }{ { name: "no claim without client_credentials grant type", - client: &oauthserver.ExternalService{ + client: &oauthserver.OAuthExternalService{ Name: "testapp", ClientID: "RANDOMID", GrantTypes: string(fosite.GrantTypeJWTBearer), @@ -146,7 +146,7 @@ func TestOAuth2ServiceImpl_handleClientCredentials(t *testing.T) { func TestOAuth2ServiceImpl_handleJWTBearer(t *testing.T) { now := time.Now() - client1 := &oauthserver.ExternalService{ + client1 := &oauthserver.OAuthExternalService{ Name: "testapp", ClientID: "RANDOMID", GrantTypes: string(fosite.GrantTypeJWTBearer), @@ -175,7 +175,7 @@ func TestOAuth2ServiceImpl_handleJWTBearer(t *testing.T) { {ID: 1, Name: "Team 1", OrgID: 1}, {ID: 2, Name: "Team 2", OrgID: 1}, } - client1WithPerm := func(perms []ac.Permission) *oauthserver.ExternalService { + client1WithPerm := func(perms []ac.Permission) *oauthserver.OAuthExternalService { client := *client1 client.ImpersonatePermissions = perms return &client @@ -185,14 +185,14 @@ func TestOAuth2ServiceImpl_handleJWTBearer(t *testing.T) { name string initEnv func(*TestEnv) scopes []string - client *oauthserver.ExternalService + client *oauthserver.OAuthExternalService subject string expectedClaims map[string]any wantErr bool }{ { name: "no claim without jwtbearer grant type", - client: &oauthserver.ExternalService{ + client: &oauthserver.OAuthExternalService{ Name: "testapp", ClientID: "RANDOMID", GrantTypes: string(fosite.GrantTypeClientCredentials), @@ -207,7 +207,7 @@ func TestOAuth2ServiceImpl_handleJWTBearer(t *testing.T) { }, { name: "err client is not allowed to impersonate", - client: &oauthserver.ExternalService{ + client: &oauthserver.OAuthExternalService{ Name: "testapp", ClientID: "RANDOMID", GrantTypes: string(fosite.GrantTypeJWTBearer), @@ -461,7 +461,7 @@ type claims struct { func TestOAuth2ServiceImpl_HandleTokenRequest(t *testing.T) { tests := []struct { name string - tweakTestClient func(*oauthserver.ExternalService) + tweakTestClient func(*oauthserver.OAuthExternalService) reqParams url.Values wantCode int wantScope []string @@ -547,7 +547,7 @@ func TestOAuth2ServiceImpl_HandleTokenRequest(t *testing.T) { }, "scope": {"profile email groups entitlements"}, }, - tweakTestClient: func(es *oauthserver.ExternalService) { + tweakTestClient: func(es *oauthserver.OAuthExternalService) { es.GrantTypes = string(fosite.GrantTypeClientCredentials) }, wantCode: http.StatusBadRequest, @@ -561,7 +561,7 @@ func TestOAuth2ServiceImpl_HandleTokenRequest(t *testing.T) { "scope": {"profile email groups entitlements"}, "audience": {AppURL}, }, - tweakTestClient: func(es *oauthserver.ExternalService) { + tweakTestClient: func(es *oauthserver.OAuthExternalService) { es.GrantTypes = string(fosite.GrantTypeJWTBearer) }, wantCode: http.StatusBadRequest, @@ -575,7 +575,7 @@ func TestOAuth2ServiceImpl_HandleTokenRequest(t *testing.T) { "scope": {"profile email groups entitlements"}, "audience": {AppURL}, }, - tweakTestClient: func(es *oauthserver.ExternalService) { + tweakTestClient: func(es *oauthserver.OAuthExternalService) { es.GrantTypes = string(fosite.GrantTypeClientCredentials) }, wantCode: http.StatusUnauthorized, @@ -591,7 +591,7 @@ func TestOAuth2ServiceImpl_HandleTokenRequest(t *testing.T) { }, "scope": {"profile email groups entitlements"}, }, - tweakTestClient: func(es *oauthserver.ExternalService) { + tweakTestClient: func(es *oauthserver.OAuthExternalService) { es.GrantTypes = string(fosite.GrantTypeJWTBearer) }, wantCode: http.StatusUnauthorized, @@ -673,11 +673,11 @@ func genAssertion(t *testing.T, signKey *rsa.PrivateKey, clientID, sub string, a } // setupHandleTokenRequestEnv creates a client and a user and sets all Mocks call for the handleTokenRequest test cases -func setupHandleTokenRequestEnv(t *testing.T, env *TestEnv, opt func(*oauthserver.ExternalService)) { +func setupHandleTokenRequestEnv(t *testing.T, env *TestEnv, opt func(*oauthserver.OAuthExternalService)) { now := time.Now() hashedSecret, err := bcrypt.GenerateFromPassword([]byte("CLIENT1SECRET"), bcrypt.DefaultCost) require.NoError(t, err) - client1 := &oauthserver.ExternalService{ + client1 := &oauthserver.OAuthExternalService{ Name: "client-1", ClientID: "CLIENT1ID", Secret: string(hashedSecret), diff --git a/pkg/services/extsvcauth/oauthserver/oastest/fakes.go b/pkg/services/extsvcauth/oauthserver/oastest/fakes.go index 21b7598d01e..71aab89ab68 100644 --- a/pkg/services/extsvcauth/oauthserver/oastest/fakes.go +++ b/pkg/services/extsvcauth/oauthserver/oastest/fakes.go @@ -4,23 +4,24 @@ import ( "context" "net/http" + "github.com/grafana/grafana/pkg/services/extsvcauth" "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" "gopkg.in/square/go-jose.v2" ) type FakeService struct { - ExpectedClient *oauthserver.ExternalService + ExpectedClient *oauthserver.OAuthExternalService ExpectedKey *jose.JSONWebKey ExpectedErr error } var _ oauthserver.OAuth2Server = &FakeService{} -func (s *FakeService) SaveExternalService(ctx context.Context, cmd *oauthserver.ExternalServiceRegistration) (*oauthserver.ExternalServiceDTO, error) { - return s.ExpectedClient.ToDTO(), s.ExpectedErr +func (s *FakeService) SaveExternalService(ctx context.Context, cmd *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) { + return s.ExpectedClient.ToExternalService(nil), s.ExpectedErr } -func (s *FakeService) GetExternalService(ctx context.Context, id string) (*oauthserver.ExternalService, error) { +func (s *FakeService) GetExternalService(ctx context.Context, id string) (*oauthserver.OAuthExternalService, error) { return s.ExpectedClient, s.ExpectedErr } diff --git a/pkg/services/extsvcauth/oauthserver/oastest/store_mock.go b/pkg/services/extsvcauth/oauthserver/oastest/store_mock.go index b4afc43d6ee..30cf07882b9 100644 --- a/pkg/services/extsvcauth/oauthserver/oastest/store_mock.go +++ b/pkg/services/extsvcauth/oauthserver/oastest/store_mock.go @@ -17,19 +17,19 @@ type MockStore struct { } // GetExternalService provides a mock function with given fields: ctx, id -func (_m *MockStore) GetExternalService(ctx context.Context, id string) (*oauthserver.ExternalService, error) { +func (_m *MockStore) GetExternalService(ctx context.Context, id string) (*oauthserver.OAuthExternalService, error) { ret := _m.Called(ctx, id) - var r0 *oauthserver.ExternalService + var r0 *oauthserver.OAuthExternalService var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (*oauthserver.ExternalService, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) (*oauthserver.OAuthExternalService, error)); ok { return rf(ctx, id) } - if rf, ok := ret.Get(0).(func(context.Context, string) *oauthserver.ExternalService); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) *oauthserver.OAuthExternalService); ok { r0 = rf(ctx, id) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*oauthserver.ExternalService) + r0 = ret.Get(0).(*oauthserver.OAuthExternalService) } } @@ -43,19 +43,19 @@ func (_m *MockStore) GetExternalService(ctx context.Context, id string) (*oauths } // GetExternalServiceByName provides a mock function with given fields: ctx, name -func (_m *MockStore) GetExternalServiceByName(ctx context.Context, name string) (*oauthserver.ExternalService, error) { +func (_m *MockStore) GetExternalServiceByName(ctx context.Context, name string) (*oauthserver.OAuthExternalService, error) { ret := _m.Called(ctx, name) - var r0 *oauthserver.ExternalService + var r0 *oauthserver.OAuthExternalService var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (*oauthserver.ExternalService, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) (*oauthserver.OAuthExternalService, error)); ok { return rf(ctx, name) } - if rf, ok := ret.Get(0).(func(context.Context, string) *oauthserver.ExternalService); ok { + if rf, ok := ret.Get(0).(func(context.Context, string) *oauthserver.OAuthExternalService); ok { r0 = rf(ctx, name) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*oauthserver.ExternalService) + r0 = ret.Get(0).(*oauthserver.OAuthExternalService) } } @@ -95,11 +95,11 @@ func (_m *MockStore) GetExternalServicePublicKey(ctx context.Context, clientID s } // RegisterExternalService provides a mock function with given fields: ctx, client -func (_m *MockStore) RegisterExternalService(ctx context.Context, client *oauthserver.ExternalService) error { +func (_m *MockStore) RegisterExternalService(ctx context.Context, client *oauthserver.OAuthExternalService) error { ret := _m.Called(ctx, client) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, *oauthserver.ExternalService) error); ok { + if rf, ok := ret.Get(0).(func(context.Context, *oauthserver.OAuthExternalService) error); ok { r0 = rf(ctx, client) } else { r0 = ret.Error(0) @@ -109,11 +109,11 @@ func (_m *MockStore) RegisterExternalService(ctx context.Context, client *oauths } // SaveExternalService provides a mock function with given fields: ctx, client -func (_m *MockStore) SaveExternalService(ctx context.Context, client *oauthserver.ExternalService) error { +func (_m *MockStore) SaveExternalService(ctx context.Context, client *oauthserver.OAuthExternalService) error { ret := _m.Called(ctx, client) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, *oauthserver.ExternalService) error); ok { + if rf, ok := ret.Get(0).(func(context.Context, *oauthserver.OAuthExternalService) error); ok { r0 = rf(ctx, client) } else { r0 = ret.Error(0) diff --git a/pkg/services/extsvcauth/oauthserver/store/database.go b/pkg/services/extsvcauth/oauthserver/store/database.go index 7d73cca2508..124fc5888a6 100644 --- a/pkg/services/extsvcauth/oauthserver/store/database.go +++ b/pkg/services/extsvcauth/oauthserver/store/database.go @@ -22,7 +22,7 @@ func NewStore(db db.DB) oauthserver.Store { return &store{db: db} } -func createImpersonatePermissions(sess *db.Session, client *oauthserver.ExternalService) error { +func createImpersonatePermissions(sess *db.Session, client *oauthserver.OAuthExternalService) error { if len(client.ImpersonatePermissions) == 0 { return nil } @@ -38,7 +38,7 @@ func createImpersonatePermissions(sess *db.Session, client *oauthserver.External return err } -func registerExternalService(sess *db.Session, client *oauthserver.ExternalService) error { +func registerExternalService(sess *db.Session, client *oauthserver.OAuthExternalService) error { insertQuery := []any{ `INSERT INTO oauth_client (name, client_id, secret, grant_types, audiences, service_account_id, public_pem, redirect_uri) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, client.Name, @@ -57,13 +57,13 @@ func registerExternalService(sess *db.Session, client *oauthserver.ExternalServi return createImpersonatePermissions(sess, client) } -func (s *store) RegisterExternalService(ctx context.Context, client *oauthserver.ExternalService) error { +func (s *store) RegisterExternalService(ctx context.Context, client *oauthserver.OAuthExternalService) error { return s.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { return registerExternalService(sess, client) }) } -func recreateImpersonatePermissions(sess *db.Session, client *oauthserver.ExternalService, prevClientID string) error { +func recreateImpersonatePermissions(sess *db.Session, client *oauthserver.OAuthExternalService, prevClientID string) error { deletePermQuery := `DELETE FROM oauth_impersonate_permission WHERE client_id = ?` if _, errDelPerm := sess.Exec(deletePermQuery, prevClientID); errDelPerm != nil { return errDelPerm @@ -76,7 +76,7 @@ func recreateImpersonatePermissions(sess *db.Session, client *oauthserver.Extern return createImpersonatePermissions(sess, client) } -func updateExternalService(sess *db.Session, client *oauthserver.ExternalService, prevClientID string) error { +func updateExternalService(sess *db.Session, client *oauthserver.OAuthExternalService, prevClientID string) error { updateQuery := []any{ `UPDATE oauth_client SET client_id = ?, secret = ?, grant_types = ?, audiences = ?, service_account_id = ?, public_pem = ?, redirect_uri = ? WHERE name = ?`, client.ClientID, @@ -95,7 +95,7 @@ func updateExternalService(sess *db.Session, client *oauthserver.ExternalService return recreateImpersonatePermissions(sess, client, prevClientID) } -func (s *store) SaveExternalService(ctx context.Context, client *oauthserver.ExternalService) error { +func (s *store) SaveExternalService(ctx context.Context, client *oauthserver.OAuthExternalService) error { if client.Name == "" { return oauthserver.ErrClientRequiredName } @@ -116,8 +116,8 @@ func (s *store) SaveExternalService(ctx context.Context, client *oauthserver.Ext }) } -func (s *store) GetExternalService(ctx context.Context, id string) (*oauthserver.ExternalService, error) { - res := &oauthserver.ExternalService{} +func (s *store) GetExternalService(ctx context.Context, id string) (*oauthserver.OAuthExternalService, error) { + res := &oauthserver.OAuthExternalService{} if id == "" { return nil, oauthserver.ErrClientRequiredID } @@ -145,7 +145,7 @@ func (s *store) GetExternalService(ctx context.Context, id string) (*oauthserver // GetPublicKey returns public key, issued by 'issuer', and assigned for subject. Public key is used to check // signature of jwt assertion in authorization grants. func (s *store) GetExternalServicePublicKey(ctx context.Context, clientID string) (*jose.JSONWebKey, error) { - res := &oauthserver.ExternalService{} + res := &oauthserver.OAuthExternalService{} if clientID == "" { return nil, oauthserver.ErrClientRequiredID } @@ -183,8 +183,8 @@ func (s *store) GetExternalServicePublicKey(ctx context.Context, clientID string }, nil } -func (s *store) GetExternalServiceByName(ctx context.Context, name string) (*oauthserver.ExternalService, error) { - res := &oauthserver.ExternalService{} +func (s *store) GetExternalServiceByName(ctx context.Context, name string) (*oauthserver.OAuthExternalService, error) { + res := &oauthserver.OAuthExternalService{} if name == "" { return nil, oauthserver.ErrClientRequiredName } @@ -198,8 +198,8 @@ func (s *store) GetExternalServiceByName(ctx context.Context, name string) (*oau return res, err } -func getExternalServiceByName(sess *db.Session, name string) (*oauthserver.ExternalService, error) { - res := &oauthserver.ExternalService{} +func getExternalServiceByName(sess *db.Session, name string) (*oauthserver.OAuthExternalService, error) { + res := &oauthserver.OAuthExternalService{} getClientQuery := `SELECT id, name, client_id, secret, grant_types, audiences, service_account_id, public_pem, redirect_uri FROM oauth_client diff --git a/pkg/services/extsvcauth/oauthserver/store/database_test.go b/pkg/services/extsvcauth/oauthserver/store/database_test.go index 558277b1388..5748ac2d91c 100644 --- a/pkg/services/extsvcauth/oauthserver/store/database_test.go +++ b/pkg/services/extsvcauth/oauthserver/store/database_test.go @@ -17,12 +17,12 @@ func TestStore_RegisterAndGetClient(t *testing.T) { s := &store{db: db.InitTestDB(t, db.InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagExternalServiceAuth}})} tests := []struct { name string - client oauthserver.ExternalService + client oauthserver.OAuthExternalService wantErr bool }{ { name: "register and get", - client: oauthserver.ExternalService{ + client: oauthserver.OAuthExternalService{ Name: "The Worst App Ever", ClientID: "ANonRandomClientID", Secret: "ICouldKeepSecrets", @@ -59,7 +59,7 @@ dCBBIFJlZ3VsYXIgQmFzZTY0IEVuY29kZWQgU3RyaW5nLi4uCg== }, { name: "register with impersonate permissions and get", - client: oauthserver.ExternalService{ + client: oauthserver.OAuthExternalService{ Name: "The Best App Ever", ClientID: "AnAlmostRandomClientID", Secret: "ICannotKeepSecrets", @@ -80,7 +80,7 @@ dCBBIFJlZ3VsYXIgQmFzZTY0IEVuY29kZWQgU3RyaW5nLi4uCg== }, { name: "register with audiences and get", - client: oauthserver.ExternalService{ + client: oauthserver.OAuthExternalService{ Name: "The Most Normal App Ever", ClientID: "AnAlmostRandomClientIDAgain", Secret: "ICanKeepSecretsEventually", @@ -111,7 +111,7 @@ dCBBIFJlZ3VsYXIgQmFzZTY0IEVuY29kZWQgU3RyaW5nLi4uCg== } func TestStore_SaveExternalService(t *testing.T) { - client1 := oauthserver.ExternalService{ + client1 := oauthserver.OAuthExternalService{ Name: "my-external-service", ClientID: "ClientID", Secret: "Secret", @@ -136,42 +136,42 @@ func TestStore_SaveExternalService(t *testing.T) { tests := []struct { name string - runs []oauthserver.ExternalService + runs []oauthserver.OAuthExternalService wantErr bool }{ { name: "error no name", - runs: []oauthserver.ExternalService{{}}, + runs: []oauthserver.OAuthExternalService{{}}, wantErr: true, }, { name: "simple register", - runs: []oauthserver.ExternalService{client1}, + runs: []oauthserver.OAuthExternalService{client1}, wantErr: false, }, { name: "no update", - runs: []oauthserver.ExternalService{client1, client1}, + runs: []oauthserver.OAuthExternalService{client1, client1}, wantErr: false, }, { name: "add permissions", - runs: []oauthserver.ExternalService{client1, client1WithPerm}, + runs: []oauthserver.OAuthExternalService{client1, client1WithPerm}, wantErr: false, }, { name: "remove permissions", - runs: []oauthserver.ExternalService{client1WithPerm, client1}, + runs: []oauthserver.OAuthExternalService{client1WithPerm, client1}, wantErr: false, }, { name: "update id and secrets", - runs: []oauthserver.ExternalService{client1, client1WithNewSecrets}, + runs: []oauthserver.OAuthExternalService{client1, client1WithNewSecrets}, wantErr: false, }, { name: "update audience", - runs: []oauthserver.ExternalService{client1, client1WithAud}, + runs: []oauthserver.OAuthExternalService{client1, client1WithAud}, wantErr: false, }, } @@ -193,7 +193,7 @@ func TestStore_SaveExternalService(t *testing.T) { } func TestStore_GetExternalServiceByName(t *testing.T) { - client1 := oauthserver.ExternalService{ + client1 := oauthserver.OAuthExternalService{ Name: "my-external-service", ClientID: "ClientID", Secret: "Secret", @@ -203,7 +203,7 @@ func TestStore_GetExternalServiceByName(t *testing.T) { ImpersonatePermissions: []accesscontrol.Permission{}, RedirectURI: "/whereto", } - client2 := oauthserver.ExternalService{ + client2 := oauthserver.OAuthExternalService{ Name: "my-external-service-2", ClientID: "ClientID2", Secret: "Secret2", @@ -224,7 +224,7 @@ func TestStore_GetExternalServiceByName(t *testing.T) { tests := []struct { name string search string - want *oauthserver.ExternalService + want *oauthserver.OAuthExternalService wantErr bool }{ { @@ -268,8 +268,8 @@ func TestStore_GetExternalServiceByName(t *testing.T) { func TestStore_GetExternalServicePublicKey(t *testing.T) { clientID := "ClientID" - createClient := func(clientID string, publicPem string) *oauthserver.ExternalService { - return &oauthserver.ExternalService{ + createClient := func(clientID string, publicPem string) *oauthserver.OAuthExternalService { + return &oauthserver.OAuthExternalService{ Name: "my-external-service", ClientID: clientID, Secret: "Secret", @@ -283,7 +283,7 @@ func TestStore_GetExternalServicePublicKey(t *testing.T) { testCases := []struct { name string - client *oauthserver.ExternalService + client *oauthserver.OAuthExternalService clientID string want *jose.JSONWebKey wantKeyType string @@ -352,7 +352,7 @@ vuO8AU0bVoUmYMKhozkcCYHudkeS08hEjQIDAQAB } } -func compareClientToStored(t *testing.T, s *store, wanted *oauthserver.ExternalService) { +func compareClientToStored(t *testing.T, s *store, wanted *oauthserver.OAuthExternalService) { ctx := context.Background() stored, err := s.GetExternalService(ctx, wanted.ClientID) require.NoError(t, err) @@ -361,7 +361,7 @@ func compareClientToStored(t *testing.T, s *store, wanted *oauthserver.ExternalS compareClients(t, stored, wanted) } -func compareClients(t *testing.T, stored *oauthserver.ExternalService, wanted *oauthserver.ExternalService) { +func compareClients(t *testing.T, stored *oauthserver.OAuthExternalService, wanted *oauthserver.OAuthExternalService) { // Reset ID so we can compare require.NotZero(t, stored.ID) stored.ID = 0 diff --git a/pkg/services/extsvcauth/registry/service.go b/pkg/services/extsvcauth/registry/service.go new file mode 100644 index 00000000000..d8cf77a37b6 --- /dev/null +++ b/pkg/services/extsvcauth/registry/service.go @@ -0,0 +1,43 @@ +package registry + +import ( + "context" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/extsvcauth" + "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" + "github.com/grafana/grafana/pkg/services/featuremgmt" +) + +var _ extsvcauth.ExternalServiceRegistry = &Registry{} + +type Registry struct { + features featuremgmt.FeatureToggles + logger log.Logger + oauthServer oauthserver.OAuth2Server +} + +func ProvideExtSvcRegistry(oauthServer oauthserver.OAuth2Server, features featuremgmt.FeatureToggles) *Registry { + return &Registry{ + features: features, + logger: log.New("extsvcauth.registry"), + oauthServer: oauthServer, + } +} + +// SaveExternalService creates or updates an external service in the database. Based on the requested auth provider, +// it generates client_id, secrets and any additional provider specificities (ex: rsa keys). It also ensures that the +// associated service account has the correct permissions. +func (r *Registry) SaveExternalService(ctx context.Context, cmd *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) { + switch cmd.AuthProvider { + case extsvcauth.OAuth2Server: + if !r.features.IsEnabled(featuremgmt.FlagExternalServiceAuth) { + r.logger.Warn("Skipping external service authentication, flag disabled", "service", cmd.Name, "flag", featuremgmt.FlagExternalServiceAuth) + return nil, nil + } + r.logger.Debug("Routing the External Service registration to the OAuth2Server", "service", cmd.Name) + return r.oauthServer.SaveExternalService(ctx, cmd) + default: + return nil, extsvcauth.ErrUnknownProvider.Errorf("unknow provider '%v'", cmd.AuthProvider) + } +} diff --git a/pkg/services/pluginsintegration/serviceregistration/serviceregistration.go b/pkg/services/pluginsintegration/serviceregistration/serviceregistration.go index 8c82c2dc0b8..f2054f8ca3b 100644 --- a/pkg/services/pluginsintegration/serviceregistration/serviceregistration.go +++ b/pkg/services/pluginsintegration/serviceregistration/serviceregistration.go @@ -6,14 +6,14 @@ import ( "github.com/grafana/grafana/pkg/plugins/auth" "github.com/grafana/grafana/pkg/plugins/plugindef" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/extsvcauth/oauthserver" + "github.com/grafana/grafana/pkg/services/extsvcauth" ) type Service struct { - os oauthserver.OAuth2Server + os extsvcauth.ExternalServiceRegistry } -func ProvideService(os oauthserver.OAuth2Server) *Service { +func ProvideService(os extsvcauth.ExternalServiceRegistry) *Service { s := &Service{ os: os, } @@ -22,7 +22,7 @@ func ProvideService(os oauthserver.OAuth2Server) *Service { // RegisterExternalService is a simplified wrapper around SaveExternalService for the plugin use case. func (s *Service) RegisterExternalService(ctx context.Context, svcName string, svc *plugindef.ExternalServiceRegistration) (*auth.ExternalService, error) { - impersonation := oauthserver.ImpersonationCfg{} + impersonation := extsvcauth.ImpersonationCfg{} if svc.Impersonation != nil { impersonation.Permissions = toAccessControlPermissions(svc.Impersonation.Permissions) if svc.Impersonation.Enabled != nil { @@ -37,7 +37,7 @@ func (s *Service) RegisterExternalService(ctx context.Context, svcName string, s } } - self := oauthserver.SelfCfg{} + self := extsvcauth.SelfCfg{} if svc.Self != nil { self.Permissions = toAccessControlPermissions(svc.Self.Permissions) if svc.Self.Enabled != nil { @@ -46,21 +46,27 @@ func (s *Service) RegisterExternalService(ctx context.Context, svcName string, s self.Enabled = true } } - extSvc, err := s.os.SaveExternalService(ctx, &oauthserver.ExternalServiceRegistration{ - Name: svcName, - Impersonation: impersonation, - Self: self, - Key: &oauthserver.KeyOption{Generate: true}, + + extSvc, err := s.os.SaveExternalService(ctx, &extsvcauth.ExternalServiceRegistration{ + Name: svcName, + Impersonation: impersonation, + Self: self, + AuthProvider: extsvcauth.OAuth2Server, + OAuthProviderCfg: &extsvcauth.OAuthProviderCfg{Key: &extsvcauth.KeyOption{Generate: true}}, }) if err != nil { return nil, err } + privateKey := "" + if extSvc.OAuthExtra != nil { + privateKey = extSvc.OAuthExtra.KeyResult.PrivatePem + } + return &auth.ExternalService{ ClientID: extSvc.ID, ClientSecret: extSvc.Secret, - PrivateKey: extSvc.KeyResult.PrivatePem, - }, nil + PrivateKey: privateKey}, nil } func toAccessControlPermissions(ps []plugindef.Permission) []accesscontrol.Permission {