Skip to content

Commit

Permalink
samlsettings: api integration (#84300)
Browse files Browse the repository at this point in the history
* 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

(cherry picked from commit fc205db)
  • Loading branch information
linoman authored and grafana-delivery-bot[bot] committed Apr 15, 2024
1 parent de27d86 commit 0f906d3
Show file tree
Hide file tree
Showing 8 changed files with 192 additions and 63 deletions.
3 changes: 2 additions & 1 deletion pkg/api/login.go
Expand Up @@ -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"
Expand Down Expand Up @@ -334,7 +335,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 {
Expand Down
1 change: 1 addition & 0 deletions pkg/login/social/social.go
Expand Up @@ -23,6 +23,7 @@ const (
// legacy/old settings for the provider
GrafanaNetProviderName = "grafananet"
OktaProviderName = "okta"
SAMLProviderName = "saml"
)

var (
Expand Down
16 changes: 14 additions & 2 deletions pkg/login/social/socialimpl/service_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion pkg/services/navtree/navtreeimpl/admin.go
@@ -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"
Expand All @@ -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
Expand Down
43 changes: 28 additions & 15 deletions pkg/services/ssosettings/ssosettingsimpl/service.go
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0f906d3

Please sign in to comment.