samlsettings: api integration (#84300)

* add strategy and tests

* use settings provider service and remove multiple providers strategy

* Move SAML strategy to ssosettings service

* Update codeowners file

* reload from settings provider

* add saml as configurable provider

* Add new SAML strategy

* rename old saml settings interface

* update saml string references

* use OSS license

* validate saml provider depends on license for List

* add tests for list rendering including saml

* change the licensing validation to service init

* replace service struct for provider
This commit is contained in:
linoman 2024-03-25 03:54:45 -06:00 committed by GitHub
parent c33bc819bc
commit fc205db466
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 193 additions and 64 deletions

View File

@ -12,6 +12,7 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/infra/network"
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/middleware/cookies"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/auth/identity"
@ -333,7 +334,7 @@ func (hs *HTTPServer) redirectURLWithErrorCookie(c *contextmodel.ReqContext, err
}
func (hs *HTTPServer) samlEnabled() bool {
return hs.SettingsProvider.KeyValue("auth.saml", "enabled").MustBool(false) && hs.License.FeatureEnabled("saml")
return hs.SettingsProvider.KeyValue("auth.saml", "enabled").MustBool(false) && hs.License.FeatureEnabled(social.SAMLProviderName)
}
func (hs *HTTPServer) samlName() string {

View File

@ -23,6 +23,7 @@ const (
// legacy/old settings for the provider
GrafanaNetProviderName = "grafananet"
OktaProviderName = "okta"
SAMLProviderName = "saml"
)
var (

View File

@ -14,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/login/social/connectors"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/licensing"
secretsfake "github.com/grafana/grafana/pkg/services/secrets/fakes"
"github.com/grafana/grafana/pkg/services/ssosettings/ssosettingsimpl"
"github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest"
@ -69,7 +70,18 @@ func TestSocialService_ProvideService(t *testing.T) {
accessControl := acimpl.ProvideAccessControl(cfg)
sqlStore := db.InitTestDB(t)
ssoSettingsSvc := ssosettingsimpl.ProvideService(cfg, sqlStore, accessControl, routing.NewRouteRegister(), featuremgmt.WithFeatures(), secrets, &usagestats.UsageStatsMock{}, nil)
ssoSettingsSvc := ssosettingsimpl.ProvideService(
cfg,
sqlStore,
accessControl,
routing.NewRouteRegister(),
featuremgmt.WithFeatures(),
secrets,
&usagestats.UsageStatsMock{},
nil,
&setting.OSSImpl{Cfg: cfg},
&licensing.OSSLicensingService{},
)
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
@ -170,7 +182,7 @@ func TestSocialService_ProvideService_GrafanaComGrafanaNet(t *testing.T) {
accessControl := acimpl.ProvideAccessControl(cfg)
sqlStore := db.InitTestDB(t)
ssoSettingsSvc := ssosettingsimpl.ProvideService(cfg, sqlStore, accessControl, routing.NewRouteRegister(), featuremgmt.WithFeatures(), secrets, &usagestats.UsageStatsMock{}, nil)
ssoSettingsSvc := ssosettingsimpl.ProvideService(cfg, sqlStore, accessControl, routing.NewRouteRegister(), featuremgmt.WithFeatures(), secrets, &usagestats.UsageStatsMock{}, nil, nil, &licensing.OSSLicensingService{})
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {

View File

@ -1,6 +1,7 @@
package navtreeimpl
import (
"github.com/grafana/grafana/pkg/login/social"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/ssoutils"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
@ -18,7 +19,7 @@ func (s *ServiceImpl) getAdminNode(c *contextmodel.ReqContext) (*navtree.NavLink
hasAccess := ac.HasAccess(s.accessControl, c)
hasGlobalAccess := ac.HasGlobalAccess(s.accessControl, s.accesscontrolService, c)
orgsAccessEvaluator := ac.EvalPermission(ac.ActionOrgsRead)
authConfigUIAvailable := s.license.FeatureEnabled("saml") || s.cfg.LDAPAuthEnabled
authConfigUIAvailable := s.license.FeatureEnabled(social.SAMLProviderName) || s.cfg.LDAPAuthEnabled
// FIXME: If plugin admin is disabled or externally managed, server admins still need to access the page, this is why
// while we don't have a permissions for listing plugins the legacy check has to stay as a default

View File

@ -12,9 +12,11 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/usagestats"
"github.com/grafana/grafana/pkg/login/social"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/licensing"
"github.com/grafana/grafana/pkg/services/secrets"
"github.com/grafana/grafana/pkg/services/ssosettings"
"github.com/grafana/grafana/pkg/services/ssosettings/api"
@ -35,29 +37,40 @@ type Service struct {
secrets secrets.Service
metrics *metrics
fbStrategies []ssosettings.FallbackStrategy
reloadables map[string]ssosettings.Reloadable
fbStrategies []ssosettings.FallbackStrategy
providersList []string
reloadables map[string]ssosettings.Reloadable
}
func ProvideService(cfg *setting.Cfg, sqlStore db.DB, ac ac.AccessControl,
routeRegister routing.RouteRegister, features featuremgmt.FeatureToggles,
secrets secrets.Service, usageStats usagestats.Service, registerer prometheus.Registerer) *Service {
strategies := []ssosettings.FallbackStrategy{
secrets secrets.Service, usageStats usagestats.Service, registerer prometheus.Registerer,
settingsProvider setting.Provider, licensing licensing.Licensing) *Service {
fbStrategies := []ssosettings.FallbackStrategy{
strategies.NewOAuthStrategy(cfg),
// register other strategies here, for example SAML
}
providersList := ssosettings.AllOAuthProviders
if licensing.FeatureEnabled(social.SAMLProviderName) {
fbStrategies = append(fbStrategies, strategies.NewSAMLStrategy(settingsProvider))
if cfg.SSOSettingsConfigurableProviders[social.SAMLProviderName] {
providersList = append(providersList, social.SAMLProviderName)
}
}
store := database.ProvideStore(sqlStore)
svc := &Service{
logger: log.New("ssosettings.service"),
cfg: cfg,
store: store,
ac: ac,
fbStrategies: strategies,
secrets: secrets,
metrics: newMetrics(registerer),
reloadables: make(map[string]ssosettings.Reloadable),
logger: log.New("ssosettings.service"),
cfg: cfg,
store: store,
ac: ac,
fbStrategies: fbStrategies,
secrets: secrets,
metrics: newMetrics(registerer),
providersList: providersList,
reloadables: make(map[string]ssosettings.Reloadable),
}
usageStats.RegisterMetricsFunc(svc.getUsageStats)
@ -114,14 +127,14 @@ func (s *Service) GetForProviderWithRedactedSecrets(ctx context.Context, provide
}
func (s *Service) List(ctx context.Context) ([]*models.SSOSettings, error) {
result := make([]*models.SSOSettings, 0, len(ssosettings.AllOAuthProviders))
result := make([]*models.SSOSettings, 0, len(s.providersList))
storedSettings, err := s.store.List(ctx)
if err != nil {
return nil, err
}
for _, provider := range ssosettings.AllOAuthProviders {
for _, provider := range s.providersList {
dbSettings := getSettingByProvider(provider, storedSettings)
if dbSettings != nil {
// Settings are coming from the database thus secrets are encrypted

View File

@ -13,19 +13,29 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"gopkg.in/ini.v1"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/infra/db/dbtest"
"github.com/grafana/grafana/pkg/infra/usagestats"
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/licensing/licensingtest"
secretsFakes "github.com/grafana/grafana/pkg/services/secrets/fakes"
"github.com/grafana/grafana/pkg/services/ssosettings"
"github.com/grafana/grafana/pkg/services/ssosettings/models"
"github.com/grafana/grafana/pkg/services/ssosettings/ssosettingstests"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/tests/testsuite"
)
func TestMain(m *testing.M) {
testsuite.Run(m)
}
func TestService_GetForProvider(t *testing.T) {
t.Parallel()
@ -231,7 +241,7 @@ func TestService_GetForProvider(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
if tc.setup != nil {
tc.setup(env)
}
@ -340,7 +350,7 @@ func TestService_GetForProviderWithRedactedSecrets(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
if tc.setup != nil {
tc.setup(env)
}
@ -368,7 +378,7 @@ func TestService_List(t *testing.T) {
wantErr bool
}{
{
name: "should return successfully",
name: "should return all oauth providers successfully without saml",
setup: func(env testEnv) {
env.store.ExpectedSSOSettings = []*models.SSOSettings{
{
@ -491,7 +501,7 @@ func TestService_List(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
if tc.setup != nil {
tc.setup(env)
}
@ -793,7 +803,7 @@ func TestService_ListWithRedactedSecrets(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
if tc.setup != nil {
tc.setup(env)
}
@ -817,7 +827,7 @@ func TestService_Upsert(t *testing.T) {
t.Run("successfully upsert SSO settings", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
settings := models.SSOSettings{
@ -880,7 +890,7 @@ func TestService_Upsert(t *testing.T) {
t.Run("returns error if provider is not configurable", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.GrafanaComProviderName
settings := &models.SSOSettings{
@ -903,7 +913,7 @@ func TestService_Upsert(t *testing.T) {
t.Run("returns error if provider was not found in reloadables", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
settings := &models.SSOSettings{
@ -927,7 +937,7 @@ func TestService_Upsert(t *testing.T) {
t.Run("returns error if validation fails", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
settings := models.SSOSettings{
@ -951,7 +961,7 @@ func TestService_Upsert(t *testing.T) {
t.Run("returns error if a fallback strategy is not available for the provider", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
settings := &models.SSOSettings{
Provider: social.AzureADProviderName,
@ -972,7 +982,7 @@ func TestService_Upsert(t *testing.T) {
t.Run("returns error if secrets encryption failed", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.OktaProviderName
settings := models.SSOSettings{
@ -997,7 +1007,7 @@ func TestService_Upsert(t *testing.T) {
t.Run("should not update the current secret if the secret has not been updated", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
settings := models.SSOSettings{
@ -1034,7 +1044,7 @@ func TestService_Upsert(t *testing.T) {
t.Run("returns error if store failed to upsert settings", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
settings := models.SSOSettings{
@ -1066,7 +1076,7 @@ func TestService_Upsert(t *testing.T) {
t.Run("successfully upsert SSO settings if reload fails", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
settings := models.SSOSettings{
@ -1099,7 +1109,7 @@ func TestService_Delete(t *testing.T) {
t.Run("successfully delete SSO settings", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
var wg sync.WaitGroup
wg.Add(1)
@ -1137,7 +1147,7 @@ func TestService_Delete(t *testing.T) {
t.Run("return error if SSO setting was not found for the specified provider", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
reloadable := ssosettingstests.NewMockReloadable(t)
@ -1153,7 +1163,7 @@ func TestService_Delete(t *testing.T) {
t.Run("should not delete the SSO settings if the provider is not configurable", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
env.cfg.SSOSettingsConfigurableProviders = map[string]bool{social.AzureADProviderName: true}
provider := social.GrafanaComProviderName
@ -1166,7 +1176,7 @@ func TestService_Delete(t *testing.T) {
t.Run("return error when store fails to delete the SSO settings for the specified provider", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
env.store.ExpectedError = errors.New("delete sso settings failed")
@ -1179,7 +1189,7 @@ func TestService_Delete(t *testing.T) {
t.Run("return successfully when the deletion was successful but reloading the settings fail", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := social.AzureADProviderName
reloadable := ssosettingstests.NewMockReloadable(t)
@ -1201,7 +1211,7 @@ func TestService_DoReload(t *testing.T) {
t.Run("successfully reload settings", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
settingsList := []*models.SSOSettings{
{
@ -1241,7 +1251,7 @@ func TestService_DoReload(t *testing.T) {
t.Run("failed fetching the SSO settings", func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
provider := "github"
@ -1336,7 +1346,7 @@ func TestService_decryptSecrets(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t)
env := setupTestEnv(t, false, false, nil)
if tc.setup != nil {
tc.setup(env)
@ -1357,7 +1367,73 @@ func TestService_decryptSecrets(t *testing.T) {
}
}
func setupTestEnv(t *testing.T) testEnv {
func Test_ProviderService(t *testing.T) {
tests := []struct {
name string
isLicenseEnabled bool
configurableProviders map[string]bool
expectedProvidersList []string
strategiesLength int
}{
{
name: "should return all OAuth providers but not saml because the licensing feature is not enabled",
isLicenseEnabled: false,
expectedProvidersList: []string{
"github",
"gitlab",
"google",
"generic_oauth",
"grafana_com",
"azuread",
"okta",
},
strategiesLength: 1,
},
{
name: "should return all fallback strategies and it should return all OAuth providers but not saml because the licensing feature is enabled but the configurable provider is not setup",
isLicenseEnabled: true,
expectedProvidersList: []string{
"github",
"gitlab",
"google",
"generic_oauth",
"grafana_com",
"azuread",
"okta",
},
strategiesLength: 2,
},
{
name: "should return all fallback strategies and it should return all OAuth providers and saml because the licensing feature is enabled and the provider is setup",
isLicenseEnabled: true,
configurableProviders: map[string]bool{"saml": true},
expectedProvidersList: []string{
"github",
"gitlab",
"google",
"generic_oauth",
"grafana_com",
"azuread",
"okta",
"saml",
},
strategiesLength: 2,
},
}
for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
env := setupTestEnv(t, tc.isLicenseEnabled, true, tc.configurableProviders)
require.Equal(t, tc.expectedProvidersList, env.service.providersList)
require.Equal(t, tc.strategiesLength, len(env.service.fbStrategies))
})
}
}
func setupTestEnv(t *testing.T, isLicensingEnabled, keepFallbackStratergies bool, extraConfigurableProviders map[string]bool) testEnv {
t.Helper()
store := ssosettingstests.NewFakeStore()
@ -1368,28 +1444,52 @@ func setupTestEnv(t *testing.T) testEnv {
fallbackStrategy.ExpectedIsMatch = true
cfg := &setting.Cfg{
SSOSettingsConfigurableProviders: map[string]bool{
"github": true,
"okta": true,
"azuread": true,
"google": true,
"generic_oauth": true,
"gitlab": true,
},
iniFile, _ := ini.Load([]byte(""))
configurableProviders := map[string]bool{
"github": true,
"okta": true,
"azuread": true,
"google": true,
"generic_oauth": true,
"gitlab": true,
}
svc := &Service{
logger: log.NewNopLogger(),
cfg: cfg,
store: store,
ac: accessControl,
fbStrategies: []ssosettings.FallbackStrategy{fallbackStrategy},
reloadables: reloadables,
metrics: newMetrics(prometheus.NewRegistry()),
secrets: secrets,
for k, v := range extraConfigurableProviders {
configurableProviders[k] = v
}
cfg := &setting.Cfg{
SSOSettingsConfigurableProviders: configurableProviders,
Raw: iniFile,
}
licensing := licensingtest.NewFakeLicensing()
licensing.On("FeatureEnabled", "saml").Return(isLicensingEnabled)
svc := ProvideService(
cfg,
&dbtest.FakeDB{},
accessControl,
routing.NewRouteRegister(),
featuremgmt.WithManager(nil),
secretsFakes.NewMockService(t),
&usagestats.UsageStatsMock{},
prometheus.NewRegistry(),
&setting.OSSImpl{Cfg: cfg},
licensing,
)
// overriding values for exposed fields
svc.store = store
if !keepFallbackStratergies {
svc.fbStrategies = []ssosettings.FallbackStrategy{
fallbackStrategy,
}
}
svc.secrets = secrets
svc.reloadables = reloadables
return testEnv{
cfg: cfg,
service: svc,

View File

@ -4,6 +4,7 @@ import (
"context"
"time"
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/services/ssosettings"
"github.com/grafana/grafana/pkg/setting"
)
@ -21,7 +22,7 @@ func NewSAMLStrategy(settingsProvider setting.Provider) *SAMLStrategy {
}
func (s *SAMLStrategy) IsMatch(provider string) bool {
return provider == "saml"
return provider == social.SAMLProviderName
}
func (s *SAMLStrategy) GetProviderConfig(_ context.Context, provider string) (map[string]any, error) {

View File

@ -85,11 +85,11 @@ type KeyValue interface {
// service that have support for configuration reloads.
type ReloadHandler interface {
// Reload handles reloading of configuration changes.
Reload(section Section) error
ReloadSection(section Section) error
// Validate validates the configuration, if the validation
// fails the configuration will not be updated neither reloaded.
Validate(section Section) error
ValidateSection(section Section) error
}
type SettingsBag map[string]map[string]string