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

[v9.4.x] Auth: Fix orgrole picker disabled if isSynced user (#65553)

Browse Source 
Auth: Fix orgrole picker disabled if isSynced user (#64033)

* fix: disable orgrolepicker if externaluser is synced

* add disable to role picker

* just took me 2 hours to center the icon

* wip

* fix: check externallySyncedUser for API call

* remove check from store

* add: tests

* refactor authproxy and made tests run

* add: feature toggle

* set feature toggle for tests

* add: IsProviderEnabled

* refactor: featuretoggle name

* IsProviderEnabled tests

* add specific tests for isProviderEnabled

* fix: org_user tests

* add: owner to featuretoggle

* add missing authlabels

* remove fmt

* feature toggle

* change config

* add test for a different authmodule

* test refactor

* gen feature toggle again

* fix basic auth user able to change the org role

* test for basic auth role

* make err.base to error

* lowered lvl of log and input mesg

(cherry picked from commit 3cd952b8ba)
This commit is contained in:
Eric Leijonmarck
2023-04-05 09:55:43 +01:00
committed by GitHub
parent f9f0dd8b1b
commit d37fa06f05
27 changed files with 618 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/authn/authnimpl/service.go
+1 -1
View File
@@ -85,7 +85,7 @@ func ProvideService(
var proxyClients []authn.ProxyClient
var passwordClients []authn.PasswordClient
if s.cfg.LDAPEnabled {
if s.cfg.LDAPAuthEnabled {
ldap := clients.ProvideLDAP(cfg)
proxyClients = append(proxyClients, ldap)
passwordClients = append(passwordClients, ldap)
pkg/services/authn/authnimpl/usage_stats.go
+1 -1
View File
@@ -13,7 +13,7 @@ func (s *Service) getUsageStats(ctx context.Context) (map[string]interface{}, er
// Add stats about auth configuration
authTypes := map[string]bool{}
authTypes["basic_auth"] = s.cfg.BasicAuthEnabled
authTypes["ldap"] = s.cfg.LDAPEnabled
authTypes["ldap"] = s.cfg.LDAPAuthEnabled
authTypes["auth_proxy"] = s.cfg.AuthProxyEnabled
authTypes["anonymous"] = s.cfg.AnonymousEnabled
pkg/services/contexthandler/authproxy/authproxy.go
+2 -2
View File
@@ -38,10 +38,10 @@ var getLDAPConfig = ldap.GetConfig
// isLDAPEnabled checks if LDAP is enabled
var isLDAPEnabled = func(cfg *setting.Cfg) bool {
if cfg != nil {
return cfg.LDAPEnabled
return cfg.LDAPAuthEnabled
}
return setting.LDAPEnabled
return setting.LDAPAuthEnabled
}
// newLDAP creates multiple LDAP instance
pkg/services/featuremgmt/registry.go
+5
View File
@@ -423,6 +423,11 @@ var (
State: FeatureStateAlpha,
FrontendOnly: true,
},
{
Name: "onlyExternalOrgRoleSync",
Description: "Prohibits a user from changing organization roles synced with external auth providers",
State: FeatureStateAlpha,
},
{
Name: "prometheusMetricEncyclopedia",
Description: "Replaces the Prometheus query builder metric select option with a paginated and filterable component",
pkg/services/featuremgmt/toggles_gen.go
+4
View File
@@ -307,6 +307,10 @@ const (
// Allow datasource to provide custom UI for context view
FlagLogsContextDatasourceUi = "logsContextDatasourceUi"
// FlagOnlyExternalOrgRoleSync
// Prohibits a user from changing organization roles synced with external auth providers
FlagOnlyExternalOrgRoleSync = "onlyExternalOrgRoleSync"
// FlagPrometheusMetricEncyclopedia
// Replaces the Prometheus query builder metric select option with a paginated and filterable component
FlagPrometheusMetricEncyclopedia = "prometheusMetricEncyclopedia"
pkg/services/ldap/ldap_private_test.go
+2 -2
View File
@@ -154,7 +154,7 @@ func TestServer_validateGrafanaUser(t *testing.T) {
Config: &ServerConfig{
Groups: []*GroupToOrgRole{},
},
log: logger.New("test"),
log: log.New("test"),
}
user := &login.ExternalUserInfo{
@@ -174,7 +174,7 @@ func TestServer_validateGrafanaUser(t *testing.T) {
},
},
},
log: logger.New("test"),
log: log.New("test"),
}
user := &login.ExternalUserInfo{
pkg/services/ldap/settings.go
+2 -2
View File
@@ -73,7 +73,7 @@ var loadingMutex = &sync.Mutex{}
// IsEnabled checks if ldap is enabled
func IsEnabled() bool {
return setting.LDAPEnabled
return setting.LDAPAuthEnabled
}
func SkipOrgRoleSync() bool {
@@ -102,7 +102,7 @@ var config *Config
// the config or it reads it and caches it first.
func GetConfig(cfg *setting.Cfg) (*Config, error) {
if cfg != nil {
if !cfg.LDAPEnabled {
if !cfg.LDAPAuthEnabled {
return nil, nil
}
} else if !IsEnabled() {
pkg/services/login/authinfo.go
+99 -1
View File
@@ -4,6 +4,7 @@ import (
"context"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)
type AuthInfoService interface {
@@ -22,8 +23,103 @@ const (
AuthProxyAuthModule = "authproxy"
JWTModule = "jwt"
RenderModule = "render"
// OAuth provider modules
AzureADAuthModule = "oauth_azuread"
GoogleAuthModule = "oauth_google"
GitLabAuthModule = "oauth_gitlab"
GithubAuthModule = "oauth_github"
GenericOAuthModule = "oauth_generic_oauth"
GrafanaComAuthModule = "oauth_grafana_com"
GrafanaNetAuthModule = "oauth_grafananet"
OktaAuthModule = "oauth_okta"
// labels
SAMLLabel = "SAML"
LDAPLabel = "LDAP"
JWTLabel = "JWT"
// OAuth provider labels
AuthProxyLabel = "Auth Proxy"
AzureADLabel = "AzureAD"
GoogleLabel = "Google"
GenericOAuthLabel = "Generic OAuth"
GitLabLabel = "GitLab"
GithubLabel = "GitHub"
GrafanaComLabel = "grafana.com"
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) bool {
// provider enabled in config
if !IsProviderEnabled(cfg, authModule) {
return false
}
// first check SAML, LDAP and JWT
switch authModule {
case SAMLAuthModule:
return !cfg.SAMLSkipOrgRoleSync
case LDAPAuthModule:
return !cfg.LDAPSkipOrgRoleSync
case JWTModule:
return !cfg.JWTAuthSkipOrgRoleSync
}
// then check the rest of the oauth providers
// FIXME: remove this once we remove the setting
// is a deprecated setting that is used to skip org role sync for all external oauth providers
if cfg.OAuthSkipOrgRoleUpdateSync {
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
}
return true
}
func IsProviderEnabled(cfg *setting.Cfg, authModule string) bool {
switch authModule {
case SAMLAuthModule:
return cfg.SAMLAuthEnabled
case LDAPAuthModule:
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
case GenericOAuthModule:
return cfg.GenericOAuthAuthEnabled
}
return false
}
// used for frontend to display a more user friendly label
func GetAuthProviderLabel(authModule string) string {
switch authModule {
case "oauth_github":
@@ -45,7 +141,9 @@ func GetAuthProviderLabel(authModule string) string {
case JWTModule:
return "JWT"
case AuthProxyAuthModule:
return "Auth Proxy"
return AuthProxyLabel
case GenericOAuthModule:
return GenericOAuthLabel
default:
return "OAuth" // FIXME: replace with "Unknown" and handle generic oauth as a case
}
pkg/services/login/authinfo_test.go
+248
View File
@@ -0,0 +1,248 @@
package login
import (
"testing"
"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
}{
// azure
{
name: "AzureAD synced user should return that it is externally synced",
cfg: &setting.Cfg{AzureADEnabled: true, AzureADSkipOrgRoleSync: 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,
},
// 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: "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: "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,
},
// 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{JWTAuthEnabled: true, JWTAuthSkipOrgRoleSync: 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{JWTAuthEnabled: true, JWTAuthSkipOrgRoleSync: true},
provider: JWTModule,
expected: false,
},
// IsProvider test
{
name: "If no provider enabled should return false",
cfg: &setting.Cfg{JWTAuthSkipOrgRoleSync: 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))
})
}
}
func TestIsProviderEnabled(t *testing.T) {
testcases := []struct {
name string
cfg *setting.Cfg
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 false if not enabled",
cfg: &setting.Cfg{},
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))
})
}
}
pkg/services/navtree/navtreeimpl/admin.go
+1 -1
View File
@@ -182,7 +182,7 @@ func (s *ServiceImpl) getServerAdminNode(c *contextmodel.ReqContext) *navtree.Na
}
}
if s.cfg.LDAPEnabled && hasAccess(ac.ReqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPStatusRead)) {
if s.cfg.LDAPAuthEnabled && hasAccess(ac.ReqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPStatusRead)) {
adminNavLinks = append(adminNavLinks, &navtree.NavLink{
Text: "LDAP", Id: "ldap", Url: s.cfg.AppSubURL + "/admin/ldap", Icon: "book",
})
pkg/services/org/model.go
+22 -19
View File
@@ -7,15 +7,17 @@ import (
"github.com/grafana/grafana/pkg/models/roletype"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util/errutil"
)
// Typed errors
var (
ErrOrgNotFound = errors.New("organization not found")
ErrOrgNameTaken = errors.New("organization name is taken")
ErrLastOrgAdmin = errors.New("cannot remove last organization admin")
ErrOrgUserNotFound = errors.New("cannot find the organization user")
ErrOrgUserAlreadyAdded = errors.New("user is already added to organization")
ErrOrgNameTaken = errors.New("organization name is taken")
ErrLastOrgAdmin = errors.New("cannot remove last organization admin")
ErrOrgUserNotFound = errors.New("cannot find the organization user")
ErrOrgUserAlreadyAdded = errors.New("user is already added to organization")
ErrOrgNotFound = errors.New("organization not found")
ErrCannotChangeRoleForExternallySyncedUser = errutil.NewBase(errutil.StatusForbidden, "org.externallySynced", errutil.WithPublicMessage("cannot change role for externally synced user"))
)
type Org struct {
@@ -138,20 +140,21 @@ type UpdateOrgUserCommand struct {
}
type OrgUserDTO struct {
OrgID int64 `json:"orgId" xorm:"org_id"`
UserID int64 `json:"userId" xorm:"user_id"`
Email string `json:"email"`
Name string `json:"name"`
AvatarURL string `json:"avatarUrl" xorm:"avatar_url"`
Login string `json:"login"`
Role string `json:"role"`
LastSeenAt time.Time `json:"lastSeenAt"`
Updated time.Time `json:"-"`
Created time.Time `json:"-"`
LastSeenAtAge string `json:"lastSeenAtAge"`
AccessControl map[string]bool `json:"accessControl,omitempty"`
IsDisabled bool `json:"isDisabled"`
AuthLabels []string `json:"authLabels" xorm:"-"`
OrgID int64 `json:"orgId" xorm:"org_id"`
UserID int64 `json:"userId" xorm:"user_id"`
Email string `json:"email"`
Name string `json:"name"`
AvatarURL string `json:"avatarUrl" xorm:"avatar_url"`
Login string `json:"login"`
Role string `json:"role"`
LastSeenAt time.Time `json:"lastSeenAt"`
Updated time.Time `json:"-"`
Created time.Time `json:"-"`
LastSeenAtAge string `json:"lastSeenAtAge"`
AccessControl map[string]bool `json:"accessControl,omitempty"`
IsDisabled bool `json:"isDisabled"`
AuthLabels []string `json:"authLabels" xorm:"-"`
IsExternallySynced bool `json:"isExternallySynced"`
}
type RemoveOrgUserCommand struct {
pkg/services/user/model.go
+15 -14
View File
@@ -140,20 +140,21 @@ type GetUserProfileQuery struct {
}
type UserProfileDTO struct {
ID int64 `json:"id"`
Email string `json:"email"`
Name string `json:"name"`
Login string `json:"login"`
Theme string `json:"theme"`
OrgID int64 `json:"orgId,omitempty"`
IsGrafanaAdmin bool `json:"isGrafanaAdmin"`
IsDisabled bool `json:"isDisabled"`
IsExternal bool `json:"isExternal"`
AuthLabels []string `json:"authLabels"`
UpdatedAt time.Time `json:"updatedAt"`
CreatedAt time.Time `json:"createdAt"`
AvatarUrl string `json:"avatarUrl"`
AccessControl map[string]bool `json:"accessControl,omitempty"`
ID int64 `json:"id"`
Email string `json:"email"`
Name string `json:"name"`
Login string `json:"login"`
Theme string `json:"theme"`
OrgID int64 `json:"orgId,omitempty"`
IsGrafanaAdmin bool `json:"isGrafanaAdmin"`
IsDisabled bool `json:"isDisabled"`
IsExternal bool `json:"isExternal"`
IsExternallySynced bool `json:"isExternallySynced"`
AuthLabels []string `json:"authLabels"`
UpdatedAt time.Time `json:"updatedAt"`
CreatedAt time.Time `json:"createdAt"`
AvatarUrl string `json:"avatarUrl"`
AccessControl map[string]bool `json:"accessControl,omitempty"`
}
// implement Conversion interface to define custom field mapping (xorm feature)