Skip to content

Commit

Permalink
OAuth: Make sub claim required for generic oauth behind feature toggle (
Browse files Browse the repository at this point in the history
#85065)

* Add feature toggle for sub claims requirement

* OAuth: require valid auth id

* Fix feature toggle description
  • Loading branch information
kalleep committed Mar 25, 2024
1 parent e2f155f commit e586119
Show file tree
Hide file tree
Showing 9 changed files with 219 additions and 111 deletions.
1 change: 1 addition & 0 deletions packages/grafana-data/src/types/featureToggles.gen.ts
Expand Up @@ -176,4 +176,5 @@ export interface FeatureToggles {
scopeFilters?: boolean;
ssoSettingsSAML?: boolean;
usePrometheusFrontendPackage?: boolean;
oauthRequireSubClaim?: boolean;
}
115 changes: 115 additions & 0 deletions pkg/services/authn/authnimpl/registration.go
@@ -0,0 +1,115 @@
package authnimpl

import (
"github.com/grafana/grafana/pkg/infra/remotecache"
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/apikey"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/authn/authnimpl/sync"
"github.com/grafana/grafana/pkg/services/authn/clients"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/ldap/service"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/loginattempt"
"github.com/grafana/grafana/pkg/services/oauthtoken"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/quota"
"github.com/grafana/grafana/pkg/services/rendering"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)

func ProvideRegistration(
cfg *setting.Cfg, authnSvc authn.Service,
orgService org.Service, sessionService auth.UserTokenService,
accessControlService accesscontrol.Service,
apikeyService apikey.Service, userService user.Service,
jwtService auth.JWTVerifierService, userProtectionService login.UserProtectionService,
loginAttempts loginattempt.Service, quotaService quota.Service,
authInfoService login.AuthInfoService, renderService rendering.Service,
features *featuremgmt.FeatureManager, oauthTokenService oauthtoken.OAuthTokenService,
socialService social.Service, cache *remotecache.RemoteCache,
ldapService service.LDAP, settingsProviderService setting.Provider,
) Registration {
authnSvc.RegisterClient(clients.ProvideRender(renderService))
authnSvc.RegisterClient(clients.ProvideAPIKey(apikeyService))

if cfg.LoginCookieName != "" {
authnSvc.RegisterClient(clients.ProvideSession(cfg, sessionService))
}

var proxyClients []authn.ProxyClient
var passwordClients []authn.PasswordClient
if cfg.LDAPAuthEnabled {
ldap := clients.ProvideLDAP(cfg, ldapService, userService, authInfoService)
proxyClients = append(proxyClients, ldap)
passwordClients = append(passwordClients, ldap)
}

if !cfg.DisableLogin {
grafana := clients.ProvideGrafana(cfg, userService)
proxyClients = append(proxyClients, grafana)
passwordClients = append(passwordClients, grafana)
}

// if we have password clients configure check if basic auth or form auth is enabled
if len(passwordClients) > 0 {
passwordClient := clients.ProvidePassword(loginAttempts, passwordClients...)
if cfg.BasicAuthEnabled {
authnSvc.RegisterClient(clients.ProvideBasic(passwordClient))
}

if !cfg.DisableLoginForm {
authnSvc.RegisterClient(clients.ProvideForm(passwordClient))
}
}

if cfg.AuthProxy.Enabled && len(proxyClients) > 0 {
proxy, err := clients.ProvideProxy(cfg, cache, proxyClients...)
if err != nil {
// TODO: Fix logging
// s.log.Error("Failed to configure auth proxy", "err", err)
} else {
authnSvc.RegisterClient(proxy)
}
}

if cfg.JWTAuth.Enabled {
authnSvc.RegisterClient(clients.ProvideJWT(jwtService, cfg))
}

// FIXME (gamab): Commenting that out for now as we want to re-use the client for external service auth
// if cfg.ExtendedJWTAuthEnabled && features.IsEnabledGlobally(featuremgmt.FlagExternalServiceAuth) {
// authnSvc.RegisterClient(clients.ProvideExtendedJWT(userService, cfg, signingKeysService, oauthServer))
// }

for name := range socialService.GetOAuthProviders() {
clientName := authn.ClientWithPrefix(name)
authnSvc.RegisterClient(clients.ProvideOAuth(clientName, cfg, oauthTokenService, socialService, settingsProviderService, features))
}

// FIXME (jguer): move to User package
userSync := sync.ProvideUserSync(userService, userProtectionService, authInfoService, quotaService)
orgSync := sync.ProvideOrgSync(userService, orgService, accessControlService, cfg)
authnSvc.RegisterPostAuthHook(userSync.SyncUserHook, 10)
authnSvc.RegisterPostAuthHook(userSync.EnableUserHook, 20)
authnSvc.RegisterPostAuthHook(orgSync.SyncOrgRolesHook, 30)
authnSvc.RegisterPostAuthHook(userSync.SyncLastSeenHook, 130)
authnSvc.RegisterPostAuthHook(sync.ProvideOAuthTokenSync(oauthTokenService, sessionService, socialService).SyncOauthTokenHook, 60)
authnSvc.RegisterPostAuthHook(userSync.FetchSyncedUserHook, 100)

rbacSync := sync.ProvideRBACSync(accessControlService)
if features.IsEnabledGlobally(featuremgmt.FlagCloudRBACRoles) {
authnSvc.RegisterPostAuthHook(rbacSync.SyncCloudRoles, 110)
}

authnSvc.RegisterPostAuthHook(rbacSync.SyncPermissionsHook, 120)
authnSvc.RegisterPostAuthHook(orgSync.SetDefaultOrgHook, 130)

return Registration{}
}

type Registration struct {
}
104 changes: 2 additions & 102 deletions pkg/services/authn/authnimpl/service.go
Expand Up @@ -13,26 +13,13 @@ import (

"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/network"
"github.com/grafana/grafana/pkg/infra/remotecache"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/infra/usagestats"
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/apikey"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/authn/authnimpl/sync"
"github.com/grafana/grafana/pkg/services/authn/clients"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/ldap/service"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/loginattempt"
"github.com/grafana/grafana/pkg/services/oauthtoken"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/quota"
"github.com/grafana/grafana/pkg/services/rendering"
"github.com/grafana/grafana/pkg/services/signingkeys"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util/errutil"
Expand Down Expand Up @@ -60,19 +47,8 @@ func ProvideIdentitySynchronizer(s *Service) authn.IdentitySynchronizer {

func ProvideService(
cfg *setting.Cfg, tracer tracing.Tracer,
orgService org.Service, sessionService auth.UserTokenService,
accessControlService accesscontrol.Service,
apikeyService apikey.Service, userService user.Service,
jwtService auth.JWTVerifierService,
usageStats usagestats.Service,
userProtectionService login.UserProtectionService,
loginAttempts loginattempt.Service, quotaService quota.Service,
authInfoService login.AuthInfoService, renderService rendering.Service,
features *featuremgmt.FeatureManager, oauthTokenService oauthtoken.OAuthTokenService,
socialService social.Service, cache *remotecache.RemoteCache,
ldapService service.LDAP, registerer prometheus.Registerer,
signingKeysService signingkeys.Service,
settingsProviderService setting.Provider,
sessionService auth.UserTokenService, usageStats usagestats.Service,
authInfoService login.AuthInfoService, registerer prometheus.Registerer,
) *Service {
s := &Service{
log: log.New("authn.service"),
Expand All @@ -88,82 +64,6 @@ func ProvideService(
}

usageStats.RegisterMetricsFunc(s.getUsageStats)

s.RegisterClient(clients.ProvideRender(renderService))
s.RegisterClient(clients.ProvideAPIKey(apikeyService))

if cfg.LoginCookieName != "" {
s.RegisterClient(clients.ProvideSession(cfg, sessionService))
}

var proxyClients []authn.ProxyClient
var passwordClients []authn.PasswordClient
if s.cfg.LDAPAuthEnabled {
ldap := clients.ProvideLDAP(cfg, ldapService, userService, authInfoService)
proxyClients = append(proxyClients, ldap)
passwordClients = append(passwordClients, ldap)
}

if !s.cfg.DisableLogin {
grafana := clients.ProvideGrafana(cfg, userService)
proxyClients = append(proxyClients, grafana)
passwordClients = append(passwordClients, grafana)
}

// if we have password clients configure check if basic auth or form auth is enabled
if len(passwordClients) > 0 {
passwordClient := clients.ProvidePassword(loginAttempts, passwordClients...)
if s.cfg.BasicAuthEnabled {
s.RegisterClient(clients.ProvideBasic(passwordClient))
}

if !s.cfg.DisableLoginForm {
s.RegisterClient(clients.ProvideForm(passwordClient))
}
}

if s.cfg.AuthProxy.Enabled && len(proxyClients) > 0 {
proxy, err := clients.ProvideProxy(cfg, cache, proxyClients...)
if err != nil {
s.log.Error("Failed to configure auth proxy", "err", err)
} else {
s.RegisterClient(proxy)
}
}

if s.cfg.JWTAuth.Enabled {
s.RegisterClient(clients.ProvideJWT(jwtService, cfg))
}

// FIXME (gamab): Commenting that out for now as we want to re-use the client for external service auth
// if s.cfg.ExtendedJWTAuthEnabled && features.IsEnabledGlobally(featuremgmt.FlagExternalServiceAuth) {
// s.RegisterClient(clients.ProvideExtendedJWT(userService, cfg, signingKeysService, oauthServer))
// }

for name := range socialService.GetOAuthProviders() {
clientName := authn.ClientWithPrefix(name)
s.RegisterClient(clients.ProvideOAuth(clientName, cfg, oauthTokenService, socialService, settingsProviderService))
}

// FIXME (jguer): move to User package
userSyncService := sync.ProvideUserSync(userService, userProtectionService, authInfoService, quotaService)
orgUserSyncService := sync.ProvideOrgSync(userService, orgService, accessControlService, cfg)
s.RegisterPostAuthHook(userSyncService.SyncUserHook, 10)
s.RegisterPostAuthHook(userSyncService.EnableUserHook, 20)
s.RegisterPostAuthHook(orgUserSyncService.SyncOrgRolesHook, 30)
s.RegisterPostAuthHook(userSyncService.SyncLastSeenHook, 130)
s.RegisterPostAuthHook(sync.ProvideOAuthTokenSync(oauthTokenService, sessionService, socialService).SyncOauthTokenHook, 60)
s.RegisterPostAuthHook(userSyncService.FetchSyncedUserHook, 100)

rbacSync := sync.ProvideRBACSync(accessControlService)
if features.IsEnabledGlobally(featuremgmt.FlagCloudRBACRoles) {
s.RegisterPostAuthHook(rbacSync.SyncCloudRoles, 110)
}

s.RegisterPostAuthHook(rbacSync.SyncPermissionsHook, 120)

s.RegisterPostAuthHook(orgUserSyncService.SetDefaultOrgHook, 130)

return s
}

Expand Down
17 changes: 11 additions & 6 deletions pkg/services/authn/clients/oauth.go
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/grafana/grafana/pkg/login/social/connectors"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/oauthtoken"
"github.com/grafana/grafana/pkg/services/org"
Expand Down Expand Up @@ -65,12 +66,12 @@ var _ authn.RedirectClient = new(OAuth)

func ProvideOAuth(
name string, cfg *setting.Cfg, oauthService oauthtoken.OAuthTokenService,
socialService social.Service, settingsProviderService setting.Provider,
socialService social.Service, settingsProviderService setting.Provider, features featuremgmt.FeatureToggles,
) *OAuth {
providerName := strings.TrimPrefix(name, "auth.client.")
return &OAuth{
name, fmt.Sprintf("oauth_%s", providerName), providerName,
log.New(name), cfg, settingsProviderService, oauthService, socialService,
log.New(name), cfg, settingsProviderService, oauthService, socialService, features,
}
}

Expand All @@ -84,6 +85,7 @@ type OAuth struct {
settingsProviderSvc setting.Provider
oauthService oauthtoken.OAuthTokenService
socialService social.Service
features featuremgmt.FeatureToggles
}

func (c *OAuth) Name() string {
Expand Down Expand Up @@ -148,10 +150,13 @@ func (c *OAuth) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden
return nil, errOAuthUserInfo.Errorf("failed to get user info: %w", err)
}

// Implement in Grafana 11
// if userInfo.Id == "" {
// return nil, errors.New("idP did not return a user id")
// }
if userInfo.Id == "" {
if c.features.IsEnabledGlobally(featuremgmt.FlagOauthRequireSubClaim) {
return nil, errOAuthUserInfo.Errorf("missing required sub claims")
} else {
c.log.FromContext(ctx).Warn("Missing sub claim, oauth authentication without a sub claim is deprecated and will be rejected in future versions.")
}
}

if userInfo.Email == "" {
return nil, errOAuthMissingRequiredEmail.Errorf("required attribute email was not provided")
Expand Down

0 comments on commit e586119

Please sign in to comment.