Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

samlsettings: api integration #84300

Merged
merged 39 commits into from Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
3ab4b7f
add strategy and tests
linoman Mar 11, 2024
e48eacc
use settings provider service and remove multiple providers strategy
linoman Mar 11, 2024
669b6c8
update codeowners file
linoman Mar 11, 2024
b84b8bc
Move SAML strategy to ssosettings service
linoman Mar 11, 2024
35446a1
Update codeowners file
linoman Mar 11, 2024
c3580af
reload from settings provider
linoman Mar 12, 2024
5358938
adjust tests for duration values
linoman Mar 12, 2024
010502c
reload settings from provider
linoman Mar 12, 2024
cf54e29
add saml as configurable provider
linoman Mar 12, 2024
8005bf2
wip
linoman Mar 12, 2024
4484fe7
WIP
linoman Mar 12, 2024
c7f971e
Merge branch 'main' into linoman/samlsettings_api_integration
linoman Mar 13, 2024
8edd0ac
Add new SAML strategy
linoman Mar 13, 2024
562b8a5
remove unrelated import
linoman Mar 13, 2024
3d34542
deprecate ReloadHandler
linoman Mar 13, 2024
d2e1df0
rename old saml settings interface
linoman Mar 13, 2024
3126b6a
wip
linoman Mar 13, 2024
e2c2721
include saml with the list of providers to evaluate
linoman Mar 13, 2024
abea8d7
update saml string references
linoman Mar 13, 2024
61a0a14
fix tests
linoman Mar 13, 2024
ed290f1
fix tests
linoman Mar 13, 2024
8794197
Merge branch 'main' into linoman/samlsettings_api_integration
linoman Mar 14, 2024
d43ef86
request license for saml envs
linoman Mar 14, 2024
33b7adf
add licensing dependecy
linoman Mar 14, 2024
a49125f
use OSS license
linoman Mar 14, 2024
cea102a
Merge remote-tracking branch 'origin/main' into linoman/samlsettings_…
linoman Mar 14, 2024
6a8c392
validate saml provider depends on license for List
linoman Mar 14, 2024
e59122c
add tests for list rendering including saml
linoman Mar 15, 2024
b0bb381
fix tests
linoman Mar 15, 2024
94a4469
Merge remote-tracking branch 'origin/main' into linoman/samlsettings_…
linoman Mar 15, 2024
ac6834c
review licensing
linoman Mar 15, 2024
8c610aa
Merge remote-tracking branch 'origin/main' into linoman/samlsettings_…
linoman Mar 15, 2024
f7c89d4
change the licensing validation to service init
linoman Mar 18, 2024
11f96b3
fix tests
linoman Mar 18, 2024
fc2ad1e
fix typo
linoman Mar 18, 2024
1ec2cba
replace service struct for provider
linoman Mar 20, 2024
5579158
Merge branch 'main' into linoman/samlsettings_api_integration
linoman Mar 20, 2024
dcc6660
address pr comments
linoman Mar 25, 2024
fdb7de4
Merge remote-tracking branch 'origin/main' into linoman/samlsettings_…
linoman Mar 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -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 {
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 @@ -17,7 +18,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
1 change: 1 addition & 0 deletions pkg/services/ssosettings/ssosettings.go
Expand Up @@ -10,6 +10,7 @@ import (

var (
AllOAuthProviders = []string{social.GitHubProviderName, social.GitlabProviderName, social.GoogleProviderName, social.GenericOAuthProviderName, social.GrafanaComProviderName, social.AzureADProviderName, social.OktaProviderName}
AllProviders = append(AllOAuthProviders, social.SAMLProviderName)
)

// Service is a SSO settings service
Expand Down
47 changes: 32 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 = ssosettings.AllProviders
linoman marked this conversation as resolved.
Show resolved Hide resolved
}
}

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 @@ -99,6 +112,10 @@ func (s *Service) GetForProviderWithRedactedSecrets(ctx context.Context, provide
return nil, ssosettings.ErrNotConfigurable
}

if provider == social.SAMLProviderName {
linoman marked this conversation as resolved.
Show resolved Hide resolved
return nil, ssosettings.ErrNotConfigurable
}

storeSettings, err := s.GetForProvider(ctx, provider)
if err != nil {
return nil, err
Expand All @@ -114,14 +131,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