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

Chore: Configure SkipOrgRoleSync from OAuthInfo for OAuth connectors (#79443)

Browse Source 
* Configure SkipOrgRoleSync from OAuthInfo

* Remove skipOrgRoleSync from socialbase and connectors

* Add test to socialimpl.ProvideService

* Deprecate AuthSettings' fields

* clean up misleading init of frontendsettings.Auth
This commit is contained in:
Misi
2023-12-15 10:58:08 +01:00
committed by GitHub
parent d50ce18357
commit ce1450d4d3
30 changed files with 295 additions and 455 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/login/authinfo.go
+17 -34
View File
@@ -3,6 +3,7 @@ package login
import (
"context"
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/setting"
)
@@ -63,9 +64,9 @@ const (
// https://github.com/grafana/grafana/blob/4181acec72f76df7ad02badce13769bae4a1f840/pkg/services/login/authinfoservice/database/database.go#L61
// this means that if the user has multiple auth providers and one of them is set to sync org roles
// then IsExternallySynced will be true for this one provider and false for the others
func IsExternallySynced(cfg *setting.Cfg, authModule string) bool {
func IsExternallySynced(cfg *setting.Cfg, authModule string, oauthInfo *social.OAuthInfo) bool {
// provider enabled in config
if !IsProviderEnabled(cfg, authModule) {
if !IsProviderEnabled(cfg, authModule, oauthInfo) {
return false
}
// first check SAML, LDAP and JWT
@@ -84,20 +85,11 @@ func IsExternallySynced(cfg *setting.Cfg, authModule string) bool {
return false
}
switch authModule {
case GoogleAuthModule:
return !cfg.GoogleSkipOrgRoleSync
case OktaAuthModule:
return !cfg.OktaSkipOrgRoleSync
case AzureADAuthModule:
return !cfg.AzureADSkipOrgRoleSync
case GitLabAuthModule:
return !cfg.GitLabSkipOrgRoleSync
case GithubAuthModule:
return !cfg.GitHubSkipOrgRoleSync
case GrafanaComAuthModule:
return !cfg.GrafanaComSkipOrgRoleSync
case GenericOAuthModule:
return !cfg.GenericOAuthSkipOrgRoleSync
case GoogleAuthModule, OktaAuthModule, AzureADAuthModule, GitLabAuthModule, GithubAuthModule, GrafanaComAuthModule, GenericOAuthModule:
if oauthInfo == nil {
return false
}
return !oauthInfo.SkipOrgRoleSync
}
return true
}
@@ -105,8 +97,8 @@ func IsExternallySynced(cfg *setting.Cfg, authModule string) bool {
// IsGrafanaAdminExternallySynced returns true if Grafana server admin role is being managed by an external auth provider, and false otherwise.
// Grafana admin role sync is available for JWT, OAuth providers and LDAP.
// For JWT and OAuth providers there is an additional config option `allow_assign_grafana_admin` that has to be enabled for Grafana Admin role to be synced.
func IsGrafanaAdminExternallySynced(cfg *setting.Cfg, authModule string, oAuthAndAllowAssignGrafanaAdmin bool) bool {
if !IsExternallySynced(cfg, authModule) {
func IsGrafanaAdminExternallySynced(cfg *setting.Cfg, oauthInfo *social.OAuthInfo, authModule string) bool {
if !IsExternallySynced(cfg, authModule, oauthInfo) {
return false
}
@@ -118,11 +110,11 @@ func IsGrafanaAdminExternallySynced(cfg *setting.Cfg, authModule string, oAuthAn
case LDAPAuthModule:
return true
default:
return oAuthAndAllowAssignGrafanaAdmin
return oauthInfo != nil && oauthInfo.AllowAssignGrafanaAdmin
}
}
func IsProviderEnabled(cfg *setting.Cfg, authModule string) bool {
func IsProviderEnabled(cfg *setting.Cfg, authModule string, oauthInfo *social.OAuthInfo) bool {
switch authModule {
case SAMLAuthModule:
return cfg.SAMLAuthEnabled
@@ -130,20 +122,11 @@ func IsProviderEnabled(cfg *setting.Cfg, authModule string) bool {
return cfg.LDAPAuthEnabled
case JWTModule:
return cfg.JWTAuthEnabled
case GoogleAuthModule:
return cfg.GoogleAuthEnabled
case OktaAuthModule:
return cfg.OktaAuthEnabled
case AzureADAuthModule:
return cfg.AzureADEnabled
case GitLabAuthModule:
return cfg.GitLabAuthEnabled
case GithubAuthModule:
return cfg.GitHubAuthEnabled
case GrafanaComAuthModule:
return cfg.GrafanaComAuthEnabled || cfg.GrafanaNetAuthEnabled
case GenericOAuthModule:
return cfg.GenericOAuthAuthEnabled
case GoogleAuthModule, OktaAuthModule, AzureADAuthModule, GitLabAuthModule, GithubAuthModule, GrafanaComAuthModule, GenericOAuthModule:
if oauthInfo == nil {
return false
}
return oauthInfo.Enabled
}
return false
}
pkg/services/login/authinfo_test.go
+46 -156
View File
@@ -3,165 +3,55 @@ package login
import (
"testing"
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
)
func TestIsExternallySynced(t *testing.T) {
testcases := []struct {
name string
cfg *setting.Cfg
provider string
expected bool
name string
cfg *setting.Cfg
oauthInfo *social.OAuthInfo
provider string
expected bool
}{
// azure
// Same for all of the OAuth providers
{
name: "AzureAD synced user should return that it is externally synced",
cfg: &setting.Cfg{AzureADEnabled: true, AzureADSkipOrgRoleSync: false},
provider: AzureADAuthModule,
expected: true,
name: "AzureAD external user should return that it is externally synced",
cfg: &setting.Cfg{},
oauthInfo: &social.OAuthInfo{Enabled: true, SkipOrgRoleSync: false},
provider: AzureADAuthModule,
expected: true,
},
{
name: "AzureAD synced user should return that it is not externally synced when org role sync is set",
cfg: &setting.Cfg{AzureADEnabled: true, AzureADSkipOrgRoleSync: true},
provider: AzureADAuthModule,
expected: false,
name: "AzureAD external user should return that it is not externally synced when org role sync is set",
cfg: &setting.Cfg{},
oauthInfo: &social.OAuthInfo{Enabled: true, SkipOrgRoleSync: true},
provider: AzureADAuthModule,
expected: false,
},
// FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers
{
name: "azuread external user should return that it is not externally synced when oauth org role sync is set",
cfg: &setting.Cfg{AzureADEnabled: true, AzureADSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true},
provider: AzureADAuthModule,
expected: false,
},
// google
{
name: "Google synced user should return that it is externally synced",
cfg: &setting.Cfg{GoogleAuthEnabled: true, GoogleSkipOrgRoleSync: false},
provider: GoogleAuthModule,
expected: true,
name: "AzureAD external user should return that it is not externally synced when oauth org role sync is set",
cfg: &setting.Cfg{OAuthSkipOrgRoleUpdateSync: true},
oauthInfo: &social.OAuthInfo{Enabled: true, SkipOrgRoleSync: false},
provider: AzureADAuthModule,
expected: false,
},
{
name: "Google synced user should return that it is not externally synced when org role sync is set",
cfg: &setting.Cfg{GoogleAuthEnabled: true, GoogleSkipOrgRoleSync: true},
provider: GoogleAuthModule,
expected: false,
},
// FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers
{
name: "google external user should return that it is not externally synced when oauth org role sync is set",
cfg: &setting.Cfg{GoogleAuthEnabled: true, GoogleSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true},
provider: GoogleAuthModule,
expected: false,
name: "AzureAD external user should return that it is not externally synced when the provider is not enabled",
cfg: &setting.Cfg{},
oauthInfo: &social.OAuthInfo{Enabled: false, SkipOrgRoleSync: false},
provider: AzureADAuthModule,
expected: false,
},
{
name: "external user should return that it is not externally synced when oauth org role sync is set and google skip org role sync set",
cfg: &setting.Cfg{GoogleAuthEnabled: true, GoogleSkipOrgRoleSync: true, OAuthSkipOrgRoleUpdateSync: true},
provider: GoogleAuthModule,
expected: false,
},
// okta
{
name: "Okta synced user should return that it is externally synced",
cfg: &setting.Cfg{OktaAuthEnabled: true, OktaSkipOrgRoleSync: false},
provider: OktaAuthModule,
expected: true,
},
{
name: "Okta synced user should return that it is not externally synced when org role sync is set",
cfg: &setting.Cfg{OktaAuthEnabled: true, OktaSkipOrgRoleSync: true},
provider: OktaAuthModule,
expected: false,
},
// FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers
{
name: "okta external user should return that it is not externally synced when oauth org role sync is set",
cfg: &setting.Cfg{OktaAuthEnabled: true, OktaSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true},
provider: OktaAuthModule,
expected: false,
},
// github
{
name: "Github synced user should return that it is externally synced",
cfg: &setting.Cfg{GitHubAuthEnabled: true, GitHubSkipOrgRoleSync: false},
provider: GithubAuthModule,
expected: true,
},
{
name: "Github synced user should return that it is not externally synced when org role sync is set",
cfg: &setting.Cfg{GitHubAuthEnabled: true, GitHubSkipOrgRoleSync: true},
provider: GithubAuthModule,
expected: false,
},
// FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers
{
name: "github external user should return that it is not externally synced when oauth org role sync is set",
cfg: &setting.Cfg{GitHubAuthEnabled: true, GitHubSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true},
provider: GithubAuthModule,
expected: false,
},
// gitlab
{
name: "Gitlab synced user should return that it is externally synced",
cfg: &setting.Cfg{GitLabAuthEnabled: true, GitLabSkipOrgRoleSync: false},
provider: GitLabAuthModule,
expected: true,
},
{
name: "Gitlab synced user should return that it is not externally synced when org role sync is set",
cfg: &setting.Cfg{GitLabAuthEnabled: true, GitLabSkipOrgRoleSync: true},
provider: GitLabAuthModule,
expected: false,
},
// FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers
{
name: "gitlab external user should return that it is not externally synced when oauth org role sync is set",
cfg: &setting.Cfg{GitLabAuthEnabled: true, GitLabSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true},
provider: GitLabAuthModule,
expected: false,
},
// grafana.com
{
name: "Grafana.com synced user should return that it is externally synced",
cfg: &setting.Cfg{GrafanaComAuthEnabled: true, GrafanaComSkipOrgRoleSync: false},
provider: GrafanaComAuthModule,
expected: true,
},
{
name: "Grafana.com synced user should return that it is not externally synced when org role sync is set",
cfg: &setting.Cfg{GrafanaComAuthEnabled: true, GrafanaComSkipOrgRoleSync: true},
provider: GrafanaComAuthModule,
expected: false,
},
// FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers
{
name: "grafanacom external user should return that it is not externally synced when oauth org role sync is set",
cfg: &setting.Cfg{GrafanaComAuthEnabled: true, GrafanaComSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true},
provider: GrafanaComAuthModule,
expected: false,
},
// generic oauth
{
name: "OAuth synced user should return that it is externally synced",
cfg: &setting.Cfg{GenericOAuthAuthEnabled: true, OAuthSkipOrgRoleUpdateSync: false},
// this could be any of the external oauth providers
provider: GenericOAuthModule,
expected: true,
},
{
name: "OAuth synced user should return that it is not externally synced when org role sync is set",
cfg: &setting.Cfg{GenericOAuthAuthEnabled: true, OAuthSkipOrgRoleUpdateSync: true},
// this could be any of the external oauth providers
provider: GenericOAuthModule,
expected: false,
},
// FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers
{
name: "generic oauth external user should return that it is not externally synced when oauth org role sync is set",
cfg: &setting.Cfg{GenericOAuthAuthEnabled: true, GenericOAuthSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true},
provider: GenericOAuthModule,
expected: false,
name: "AzureAD synced user should return that it is not externally synced when the provider is not enabled and nil",
cfg: &setting.Cfg{},
oauthInfo: nil,
provider: AzureADAuthModule,
expected: false,
},
// saml
{
@@ -213,36 +103,36 @@ func TestIsExternallySynced(t *testing.T) {
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, IsExternallySynced(tc.cfg, tc.provider))
assert.Equal(t, tc.expected, IsExternallySynced(tc.cfg, tc.provider, tc.oauthInfo))
})
}
}
func TestIsProviderEnabled(t *testing.T) {
testcases := []struct {
name string
cfg *setting.Cfg
provider string
expected bool
name string
oauthInfo *social.OAuthInfo
provider string
expected bool
}{
// github
{
name: "Github should return true if enabled",
cfg: &setting.Cfg{GitHubAuthEnabled: true},
provider: GithubAuthModule,
expected: true,
name: "Github should return true if enabled",
oauthInfo: &social.OAuthInfo{Enabled: true},
provider: GithubAuthModule,
expected: true,
},
{
name: "Github should return false if not enabled",
cfg: &setting.Cfg{},
provider: GithubAuthModule,
expected: false,
name: "Github should return false if not enabled",
oauthInfo: &social.OAuthInfo{Enabled: false},
provider: GithubAuthModule,
expected: false,
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, IsProviderEnabled(tc.cfg, tc.provider))
assert.Equal(t, tc.expected, IsProviderEnabled(setting.NewCfg(), tc.provider, tc.oauthInfo))
})
}
}
pkg/services/ssosettings/strategies/oauth_strategy_test.go
+1 -1
View File
@@ -4,11 +4,11 @@ import (
"context"
"testing"
"github.com/stretchr/testify/require"
"gopkg.in/ini.v1"
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/require"
)
var (