Auth: Fix SAML user IsExternallySynced not being set correctly (#98487)
This commit is contained in:
@@ -95,6 +95,10 @@ type SSOClientConfig interface {
|
||||
IsAutoLoginEnabled() bool
|
||||
// IsSingleLogoutEnabled returns true if the client has single logout enabled
|
||||
IsSingleLogoutEnabled() bool
|
||||
// IsSkipOrgRoleSyncEnabled returns true if the client has enabled skipping org role sync
|
||||
IsSkipOrgRoleSyncEnabled() bool
|
||||
// IsAllowAssignGrafanaAdminEnabled returns true if the client has enabled assigning grafana admin
|
||||
IsAllowAssignGrafanaAdminEnabled() bool
|
||||
}
|
||||
|
||||
type Service interface {
|
||||
|
||||
@@ -11,9 +11,11 @@ import (
|
||||
var _ authn.SSOClientConfig = new(FakeSSOClientConfig)
|
||||
|
||||
type FakeSSOClientConfig struct {
|
||||
ExpectedName string
|
||||
ExpectedIsAutoLoginEnabled bool
|
||||
ExpectedIsSingleLogoutEnabled bool
|
||||
ExpectedName string
|
||||
ExpectedIsAutoLoginEnabled bool
|
||||
ExpectedIsSingleLogoutEnabled bool
|
||||
ExpectedIsSkipOrgRoleSyncEnabled bool
|
||||
ExpectedIsAllowAssignGrafanaAdminEnabled bool
|
||||
}
|
||||
|
||||
func (f *FakeSSOClientConfig) GetDisplayName() string {
|
||||
@@ -28,6 +30,14 @@ func (f *FakeSSOClientConfig) IsSingleLogoutEnabled() bool {
|
||||
return f.ExpectedIsSingleLogoutEnabled
|
||||
}
|
||||
|
||||
func (f *FakeSSOClientConfig) IsSkipOrgRoleSyncEnabled() bool {
|
||||
return f.ExpectedIsSkipOrgRoleSyncEnabled
|
||||
}
|
||||
|
||||
func (f *FakeSSOClientConfig) IsAllowAssignGrafanaAdminEnabled() bool {
|
||||
return f.ExpectedIsAllowAssignGrafanaAdminEnabled
|
||||
}
|
||||
|
||||
var (
|
||||
_ authn.Service = new(FakeService)
|
||||
_ authn.IdentitySynchronizer = new(FakeService)
|
||||
@@ -41,6 +51,7 @@ type FakeService struct {
|
||||
ExpectedErrs []error
|
||||
ExpectedIdentities []*authn.Identity
|
||||
CurrentIndex int
|
||||
EnabledClients []string
|
||||
}
|
||||
|
||||
func (f *FakeService) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identity, error) {
|
||||
@@ -64,7 +75,17 @@ func (f *FakeService) Authenticate(ctx context.Context, r *authn.Request) (*auth
|
||||
}
|
||||
|
||||
func (f *FakeService) IsClientEnabled(name string) bool {
|
||||
return true
|
||||
// Consider all clients as enabled if EnabledClients is not explicitly set
|
||||
if f.EnabledClients == nil {
|
||||
return true
|
||||
}
|
||||
// Check if client is in the list of enabled clients
|
||||
for _, s := range f.EnabledClients {
|
||||
if s == name {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func (f *FakeService) GetClientConfig(name string) (authn.SSOClientConfig, bool) {
|
||||
|
||||
@@ -2,9 +2,6 @@ package login
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/grafana/grafana/pkg/login/social"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
|
||||
type AuthInfoService interface {
|
||||
@@ -59,73 +56,6 @@ const (
|
||||
OktaLabel = "Okta"
|
||||
)
|
||||
|
||||
// IsExternnalySynced is used to tell if the user roles are externally synced
|
||||
// true means that the org role sync is handled by Grafana
|
||||
// Note: currently the users authinfo is overridden each time the user logs in
|
||||
// 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, oauthInfo *social.OAuthInfo) bool {
|
||||
// provider enabled in config
|
||||
if !IsProviderEnabled(cfg, authModule, oauthInfo) {
|
||||
return false
|
||||
}
|
||||
// first check SAML, LDAP and JWT
|
||||
switch authModule {
|
||||
case SAMLAuthModule:
|
||||
return !cfg.SAMLSkipOrgRoleSync
|
||||
case LDAPAuthModule:
|
||||
return !cfg.LDAPSkipOrgRoleSync
|
||||
case JWTModule:
|
||||
return !cfg.JWTAuth.SkipOrgRoleSync
|
||||
}
|
||||
switch authModule {
|
||||
case GoogleAuthModule, OktaAuthModule, AzureADAuthModule, GitLabAuthModule, GithubAuthModule, GrafanaComAuthModule, GenericOAuthModule:
|
||||
if oauthInfo == nil {
|
||||
return false
|
||||
}
|
||||
return !oauthInfo.SkipOrgRoleSync
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// 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, oauthInfo *social.OAuthInfo, authModule string) bool {
|
||||
if !IsExternallySynced(cfg, authModule, oauthInfo) {
|
||||
return false
|
||||
}
|
||||
|
||||
switch authModule {
|
||||
case JWTModule:
|
||||
return cfg.JWTAuth.AllowAssignGrafanaAdmin
|
||||
case SAMLAuthModule:
|
||||
return cfg.SAMLRoleValuesGrafanaAdmin != ""
|
||||
case LDAPAuthModule:
|
||||
return true
|
||||
default:
|
||||
return oauthInfo != nil && oauthInfo.AllowAssignGrafanaAdmin
|
||||
}
|
||||
}
|
||||
|
||||
func IsProviderEnabled(cfg *setting.Cfg, authModule string, oauthInfo *social.OAuthInfo) bool {
|
||||
switch authModule {
|
||||
case SAMLAuthModule:
|
||||
return cfg.SAMLAuthEnabled
|
||||
case LDAPAuthModule:
|
||||
return cfg.LDAPAuthEnabled
|
||||
case JWTModule:
|
||||
return cfg.JWTAuth.Enabled
|
||||
case GoogleAuthModule, OktaAuthModule, AzureADAuthModule, GitLabAuthModule, GithubAuthModule, GrafanaComAuthModule, GenericOAuthModule:
|
||||
if oauthInfo == nil {
|
||||
return false
|
||||
}
|
||||
return oauthInfo.Enabled
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// used for frontend to display a more user friendly label
|
||||
func GetAuthProviderLabel(authModule string) string {
|
||||
switch authModule {
|
||||
|
||||
@@ -1,131 +0,0 @@
|
||||
package login
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/grafana/grafana/pkg/login/social"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
)
|
||||
|
||||
func TestIsExternallySynced(t *testing.T) {
|
||||
testcases := []struct {
|
||||
name string
|
||||
cfg *setting.Cfg
|
||||
oauthInfo *social.OAuthInfo
|
||||
provider string
|
||||
expected bool
|
||||
}{
|
||||
// Same for all of the OAuth providers
|
||||
{
|
||||
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 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,
|
||||
},
|
||||
{
|
||||
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: "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
|
||||
{
|
||||
name: "SAML synced user should return that it is externally synced",
|
||||
cfg: &setting.Cfg{SAMLAuthEnabled: true, SAMLSkipOrgRoleSync: false},
|
||||
provider: SAMLAuthModule,
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "SAML synced user should return that it is not externally synced when org role sync is set",
|
||||
cfg: &setting.Cfg{SAMLAuthEnabled: true, SAMLSkipOrgRoleSync: true},
|
||||
provider: SAMLAuthModule,
|
||||
expected: false,
|
||||
},
|
||||
// ldap
|
||||
{
|
||||
name: "LDAP synced user should return that it is externally synced",
|
||||
cfg: &setting.Cfg{LDAPAuthEnabled: true, LDAPSkipOrgRoleSync: false},
|
||||
provider: LDAPAuthModule,
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "LDAP synced user should return that it is not externally synced when org role sync is set",
|
||||
cfg: &setting.Cfg{LDAPAuthEnabled: true, LDAPSkipOrgRoleSync: true},
|
||||
provider: LDAPAuthModule,
|
||||
expected: false,
|
||||
},
|
||||
// jwt
|
||||
{
|
||||
name: "JWT synced user should return that it is externally synced",
|
||||
cfg: &setting.Cfg{JWTAuth: setting.AuthJWTSettings{Enabled: true, SkipOrgRoleSync: false}},
|
||||
provider: JWTModule,
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "JWT synced user should return that it is not externally synced when org role sync is set",
|
||||
cfg: &setting.Cfg{JWTAuth: setting.AuthJWTSettings{Enabled: true, SkipOrgRoleSync: true}},
|
||||
provider: JWTModule,
|
||||
expected: false,
|
||||
},
|
||||
// IsProvider test
|
||||
{
|
||||
name: "If no provider enabled should return false",
|
||||
cfg: &setting.Cfg{JWTAuth: setting.AuthJWTSettings{Enabled: false, SkipOrgRoleSync: true}},
|
||||
provider: JWTModule,
|
||||
expected: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testcases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
assert.Equal(t, tc.expected, IsExternallySynced(tc.cfg, tc.provider, tc.oauthInfo))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsProviderEnabled(t *testing.T) {
|
||||
testcases := []struct {
|
||||
name string
|
||||
oauthInfo *social.OAuthInfo
|
||||
provider string
|
||||
expected bool
|
||||
}{
|
||||
// github
|
||||
{
|
||||
name: "Github should return true if enabled",
|
||||
oauthInfo: &social.OAuthInfo{Enabled: true},
|
||||
provider: GithubAuthModule,
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
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(setting.NewCfg(), tc.provider, tc.oauthInfo))
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user