From aee5c6dea0b7f011e0b29c38d0dab4b1226f26bc Mon Sep 17 00:00:00 2001 From: Jo Date: Wed, 7 Jun 2023 08:57:41 +0200 Subject: [PATCH] Auth: Use auth broker by default (#69620) remove authnservice toggle --- .../configure-grafana/feature-toggles/index.md | 1 - packages/grafana-data/src/types/featureToggles.gen.ts | 1 - pkg/api/http_server.go | 2 +- pkg/api/login.go | 2 +- pkg/api/login_oauth.go | 7 +++---- pkg/services/contexthandler/contexthandler.go | 4 ++-- pkg/services/featuremgmt/registry.go | 6 ------ pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 ---- pkg/setting/setting.go | 9 +++++++-- pkg/tests/testinfra/testinfra.go | 10 ++++++++++ pkg/tests/web/index_view_test.go | 9 +++------ 12 files changed, 27 insertions(+), 29 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index dd24f98b9aa..293fb0ee999 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -90,7 +90,6 @@ Alpha features might be changed or removed without prior notice. | `showDashboardValidationWarnings` | Show warnings when dashboards do not validate against the schema | | `mysqlAnsiQuotes` | Use double quotes to escape keyword in a MySQL query | | `showTraceId` | Show trace ids for requests | -| `authnService` | Use new auth service to perform authentication | | `alertingBacktesting` | Rule backtesting API for alerting | | `editPanelCSVDragAndDrop` | Enables drag and drop for CSV and Excel files | | `lokiQuerySplitting` | Split large interval queries into subqueries with smaller time intervals | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 64f007ad613..ffe63a8b450 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -59,7 +59,6 @@ export interface FeatureToggles { accessTokenExpirationCheck?: boolean; showTraceId?: boolean; emptyDashboardPage?: boolean; - authnService?: boolean; disablePrometheusExemplarSampling?: boolean; alertingBacktesting?: boolean; editPanelCSVDragAndDrop?: boolean; diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 4f8e75eef86..7e407001953 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -617,7 +617,7 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() { m.UseMiddleware(hs.ContextHandler.Middleware) m.Use(middleware.OrgRedirect(hs.Cfg, hs.userService)) - if !hs.Features.IsEnabled(featuremgmt.FlagAuthnService) { + if !hs.Cfg.AuthBrokerEnabled { m.Use(accesscontrol.LoadPermissionsMiddleware(hs.accesscontrolService)) } diff --git a/pkg/api/login.go b/pkg/api/login.go index 442b2d27521..61726000d78 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -194,7 +194,7 @@ func (hs *HTTPServer) LoginAPIPing(c *contextmodel.ReqContext) response.Response } func (hs *HTTPServer) LoginPost(c *contextmodel.ReqContext) response.Response { - if hs.Features.IsEnabled(featuremgmt.FlagAuthnService) { + if hs.Cfg.AuthBrokerEnabled { identity, err := hs.authnService.Login(c.Req.Context(), authn.ClientForm, &authn.Request{HTTPRequest: c.Req, Resp: c.Resp}) if err != nil { tokenErr := &auth.CreateTokenErr{} diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 4edbe195186..30a1e787ab0 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -19,7 +19,6 @@ import ( "github.com/grafana/grafana/pkg/middleware/cookies" "github.com/grafana/grafana/pkg/services/authn" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" - "github.com/grafana/grafana/pkg/services/featuremgmt" loginservice "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" @@ -84,7 +83,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *contextmodel.ReqContext) { code := ctx.Query("code") - if hs.Features.IsEnabled(featuremgmt.FlagAuthnService) { + if hs.Cfg.AuthBrokerEnabled { req := &authn.Request{HTTPRequest: ctx.Req, Resp: ctx.Resp} if code == "" { redirect, err := hs.authnService.RedirectURL(ctx.Req.Context(), authn.ClientWithPrefix(name), req) @@ -381,7 +380,7 @@ func (hs *HTTPServer) handleOAuthLoginError(ctx *contextmodel.ReqContext, info l ctx.Handle(hs.Cfg, err.HttpStatus, err.PublicMessage, err.Err) // login hooks is handled by authn.Service - if !hs.Features.IsEnabled(featuremgmt.FlagAuthnService) { + if !hs.Cfg.AuthBrokerEnabled { info.Error = err.Err if info.Error == nil { info.Error = errors.New(err.PublicMessage) @@ -396,7 +395,7 @@ func (hs *HTTPServer) handleOAuthLoginErrorWithRedirect(ctx *contextmodel.ReqCon hs.redirectWithError(ctx, err, v...) // login hooks is handled by authn.Service - if !hs.Features.IsEnabled(featuremgmt.FlagAuthnService) { + if !hs.Cfg.AuthBrokerEnabled { info.Error = err hs.HooksService.RunLoginHook(&info, ctx) } diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 9ceecdf049b..1ea23f6357c 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -139,7 +139,7 @@ func (h *ContextHandler) Middleware(next http.Handler) http.Handler { reqContext.Logger = reqContext.Logger.New("traceID", traceID) } - if h.features.IsEnabled(featuremgmt.FlagAuthnService) { + if h.Cfg.AuthBrokerEnabled { identity, err := h.authnService.Authenticate(ctx, &authn.Request{HTTPRequest: reqContext.Req, Resp: reqContext.Resp}) if err != nil { if errors.Is(err, auth.ErrInvalidSessionToken) { @@ -207,7 +207,7 @@ func (h *ContextHandler) Middleware(next http.Handler) http.Handler { ) // when using authn service this is implemented as a post auth hook - if !h.features.IsEnabled(featuremgmt.FlagAuthnService) { + if !h.Cfg.AuthBrokerEnabled { // update last seen every 5min if reqContext.ShouldUpdateLastSeenAt() { reqContext.Logger.Debug("Updating last user_seen_at", "user_id", reqContext.UserID) diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index f7353b18e92..f89fea580c1 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -279,12 +279,6 @@ var ( Expression: "true", // enabled by default Owner: grafanaDashboardsSquad, }, - { - Name: "authnService", - Description: "Use new auth service to perform authentication", - State: FeatureStateAlpha, - Owner: grafanaAuthnzSquad, - }, { Name: "disablePrometheusExemplarSampling", Description: "Disable Prometheus exemplar sampling", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index ea99bbb62b5..b34dca2ff8f 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -40,7 +40,6 @@ nestedFolders,beta,@grafana/backend-platform,false,false,false,false accessTokenExpirationCheck,stable,@grafana/grafana-authnz-team,false,false,false,false showTraceId,alpha,@grafana/observability-logs,false,false,false,false emptyDashboardPage,stable,@grafana/dashboards-squad,false,false,false,true -authnService,alpha,@grafana/grafana-authnz-team,false,false,false,false disablePrometheusExemplarSampling,stable,@grafana/observability-metrics,false,false,false,false alertingBacktesting,alpha,@grafana/alerting-squad,false,false,false,false editPanelCSVDragAndDrop,alpha,@grafana/grafana-bi-squad,false,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 555a4c376f1..795ffef0377 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -171,10 +171,6 @@ const ( // Enable the redesigned user interface of a dashboard page that includes no panels FlagEmptyDashboardPage = "emptyDashboardPage" - // FlagAuthnService - // Use new auth service to perform authentication - FlagAuthnService = "authnService" - // FlagDisablePrometheusExemplarSampling // Disable Prometheus exemplar sampling FlagDisablePrometheusExemplarSampling = "disablePrometheusExemplarSampling" diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 642e3295a93..498d0f89df7 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -277,6 +277,8 @@ type Cfg struct { // Not documented & not supported // stand in until a more complete solution is implemented AuthConfigUIAdminAccess bool + // TO REMOVE: Not documented & not supported. Remove with legacy handlers in 10.2 + AuthBrokerEnabled bool // AWS Plugin Auth AWSAllowedAuthProviders []string @@ -553,7 +555,7 @@ type CommandLineArgs struct { Args []string } -func (cfg Cfg) parseAppUrlAndSubUrl(section *ini.Section) (string, string, error) { +func (cfg *Cfg) parseAppUrlAndSubUrl(section *ini.Section) (string, string, error) { appUrl := valueAsString(section, "root_url", "http://localhost:3000/") if appUrl[len(appUrl)-1] != '/' { @@ -776,7 +778,7 @@ func applyCommandLineProperties(props map[string]string, file *ini.File) { } } -func (cfg Cfg) getCommandLineProperties(args []string) map[string]string { +func (cfg *Cfg) getCommandLineProperties(args []string) map[string]string { props := make(map[string]string) for _, arg := range args { @@ -1492,8 +1494,11 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { // Debug setting unlocking frontend auth sync lock. Users will still be reset on their next login. cfg.DisableSyncLock = auth.Key("disable_sync_lock").MustBool(false) + // Do not use cfg.AuthConfigUIAdminAccess = auth.Key("config_ui_admin_access").MustBool(false) + cfg.AuthBrokerEnabled = auth.Key("broker").MustBool(true) + cfg.DisableLoginForm = auth.Key("disable_login_form").MustBool(false) DisableSignoutMenu = auth.Key("disable_signout_menu").MustBool(false) diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index 535cd1ac89e..195d3f6c773 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -199,6 +199,15 @@ func CreateGrafDir(t *testing.T, opts ...GrafanaOpts) (string, string) { _, err = serverSect.NewKey("static_root_path", publicDir) require.NoError(t, err) + authSect, err := cfg.NewSection("auth") + require.NoError(t, err) + authBrokerState := "false" + if len(opts) > 0 && opts[0].AuthBrokerEnabled { + authBrokerState = "true" + } + _, err = authSect.NewKey("broker", authBrokerState) + require.NoError(t, err) + anonSect, err := cfg.NewSection("auth.anonymous") require.NoError(t, err) _, err = anonSect.NewKey("enabled", "true") @@ -384,6 +393,7 @@ type GrafanaOpts struct { EnableLog bool GRPCServerAddress string QueryRetries int64 + AuthBrokerEnabled bool } func CreateUser(t *testing.T, store *sqlstore.SQLStore, cmd user.CreateUserCommand) *user.User { diff --git a/pkg/tests/web/index_view_test.go b/pkg/tests/web/index_view_test.go index fee29a2664a..041487886a6 100644 --- a/pkg/tests/web/index_view_test.go +++ b/pkg/tests/web/index_view_test.go @@ -13,7 +13,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/login" databaseAuthInfo "github.com/grafana/grafana/pkg/services/login/authinfoservice/database" "github.com/grafana/grafana/pkg/services/secrets/database" @@ -119,14 +118,12 @@ func TestIntegrationIndexViewAnalytics(t *testing.T) { } // can be removed once ff is removed - testCaseFeatures := map[string][]string{"none": {}, "authnService": {featuremgmt.FlagAuthnService}} + authBrokerStates := map[string]bool{"none": false, "authnService": true} - for k, tcFeatures := range testCaseFeatures { + for k, enabled := range authBrokerStates { for _, tc := range testCases { t.Run(tc.name+"-"+k, func(t *testing.T) { - grafDir, cfgPath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ - EnableFeatureToggles: tcFeatures, - }) + grafDir, cfgPath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{AuthBrokerEnabled: enabled}) addr, store := testinfra.StartGrafana(t, grafDir, cfgPath) createdUser := testinfra.CreateUser(t, store, user.CreateUserCommand{ Login: "admin",