diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 662ed9c8ad5..4ece9a1021d 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -27,7 +27,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/infra/tracing" - loginpkg "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/middleware/csrf" @@ -125,7 +124,6 @@ type HTTPServer struct { QuotaService quota.Service RemoteCacheService *remotecache.RemoteCache ProvisioningService provisioning.ProvisioningService - Login login.Service License licensing.Licensing AccessControl accesscontrol.AccessControl DataProxy *datasourceproxy.DataSourceProxyService @@ -170,7 +168,6 @@ type HTTPServer struct { queryDataService query.Service serviceAccountsService serviceaccounts.Service authInfoService login.AuthInfoService - authenticator loginpkg.Authenticator teamPermissionsService accesscontrol.TeamPermissionsService NotificationService *notifications.NotificationService DashboardService dashboards.DashboardService @@ -219,9 +216,9 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi pluginDashboardService plugindashboards.Service, pluginStore plugins.Store, pluginClient plugins.Client, pluginErrorResolver plugins.ErrorResolver, pluginInstaller plugins.Installer, settingsProvider setting.Provider, dataSourceCache datasources.CacheService, userTokenService auth.UserTokenService, - cleanUpService *cleanup.CleanUpService, shortURLService shorturls.Service, queryHistoryService queryhistory.Service, correlationsService correlations.Service, remoteCache *remotecache.RemoteCache, provisioningService provisioning.ProvisioningService, - loginService login.Service, authenticator loginpkg.Authenticator, accessControl accesscontrol.AccessControl, - dataSourceProxy *datasourceproxy.DataSourceProxyService, searchService *search.SearchService, + cleanUpService *cleanup.CleanUpService, shortURLService shorturls.Service, queryHistoryService queryhistory.Service, + correlationsService correlations.Service, remoteCache *remotecache.RemoteCache, provisioningService provisioning.ProvisioningService, + accessControl accesscontrol.AccessControl, dataSourceProxy *datasourceproxy.DataSourceProxyService, searchService *search.SearchService, live *live.GrafanaLive, livePushGateway *pushhttp.Gateway, plugCtxProvider *plugincontext.Provider, contextHandler *contexthandler.ContextHandler, loggerMiddleware loggermw.Logger, features *featuremgmt.FeatureManager, alertNG *ngalert.AlertNG, libraryPanelService librarypanels.Service, libraryElementService libraryelements.Service, @@ -284,7 +281,6 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi StorageService: storageService, RemoteCacheService: remoteCache, ProvisioningService: provisioningService, - Login: loginService, AccessControl: accessControl, DataProxy: dataSourceProxy, SearchV2HTTPService: searchv2HTTPService, @@ -315,7 +311,6 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi queryDataService: queryDataService, serviceAccountsService: serviceaccountsService, authInfoService: authInfoService, - authenticator: authenticator, NotificationService: notificationService, DashboardService: dashboardService, dashboardProvisioningService: dashboardProvisioningService, @@ -617,10 +612,6 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() { m.UseMiddleware(hs.ContextHandler.Middleware) m.Use(middleware.OrgRedirect(hs.Cfg, hs.userService)) - if !hs.Cfg.AuthBrokerEnabled { - m.Use(accesscontrol.LoadPermissionsMiddleware(hs.accesscontrolService)) - } - // needs to be after context handler if hs.Cfg.EnforceDomain { m.Use(middleware.ValidateHostHeader(hs.Cfg)) diff --git a/pkg/api/login.go b/pkg/api/login.go index db8e7f73f5c..9ca800f5b0f 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -9,7 +9,6 @@ import ( "net/url" "strings" - "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/network" @@ -26,7 +25,6 @@ import ( "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util/errutil" - "github.com/grafana/grafana/pkg/web" ) const ( @@ -195,99 +193,17 @@ func (hs *HTTPServer) LoginAPIPing(c *contextmodel.ReqContext) response.Response } func (hs *HTTPServer) LoginPost(c *contextmodel.ReqContext) response.Response { - 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{} - if errors.As(err, &tokenErr) { - return response.Error(tokenErr.StatusCode, tokenErr.ExternalErr, tokenErr.InternalErr) - } - return response.Err(err) - } - - metrics.MApiLoginPost.Inc() - return authn.HandleLoginResponse(c.Req, c.Resp, hs.Cfg, identity, hs.ValidateRedirectTo) - } - - cmd := dtos.LoginCommand{} - if err := web.Bind(c.Req, &cmd); err != nil { - return response.Error(http.StatusBadRequest, "bad login data", err) - } - authModule := "" - var usr *user.User - var resp *response.NormalResponse - - defer func() { - err := resp.Err() - if err == nil && resp.ErrMessage() != "" { - err = errors.New(resp.ErrMessage()) - } - hs.HooksService.RunLoginHook(&loginservice.LoginInfo{ - AuthModule: authModule, - User: usr, - LoginUsername: cmd.User, - HTTPStatus: resp.Status(), - Error: err, - }, c) - }() - - if hs.Cfg.DisableLoginForm { - resp = response.Error(http.StatusUnauthorized, "Login is disabled", nil) - return resp - } - - authQuery := &loginservice.LoginUserQuery{ - ReqContext: c, - Username: cmd.User, - Password: cmd.Password, - IpAddress: c.RemoteAddr(), - Cfg: hs.Cfg, - } - - err := hs.authenticator.AuthenticateUser(c.Req.Context(), authQuery) - authModule = authQuery.AuthModule + identity, err := hs.authnService.Login(c.Req.Context(), authn.ClientForm, &authn.Request{HTTPRequest: c.Req, Resp: c.Resp}) if err != nil { - resp = response.Error(401, "Invalid username or password", err) - if errors.Is(err, login.ErrInvalidCredentials) || errors.Is(err, login.ErrTooManyLoginAttempts) || errors.Is(err, - user.ErrUserNotFound) { - return resp + tokenErr := &auth.CreateTokenErr{} + if errors.As(err, &tokenErr) { + return response.Error(tokenErr.StatusCode, tokenErr.ExternalErr, tokenErr.InternalErr) } - - if errors.Is(err, login.ErrNoAuthProvider) { - resp = response.Error(http.StatusInternalServerError, "No authorization providers enabled", err) - return resp - } - - // Do not expose disabled status, - // just show incorrect user credentials error (see #17947) - if errors.Is(err, login.ErrUserDisabled) { - hs.log.Warn("User is disabled", "user", cmd.User) - return resp - } - - resp = response.Error(500, "Error while trying to authenticate user", err) - return resp - } - - usr = authQuery.User - - err = hs.loginUserWithUser(usr, c) - if err != nil { - var createTokenErr *auth.CreateTokenErr - if errors.As(err, &createTokenErr) { - resp = response.Error(createTokenErr.StatusCode, createTokenErr.ExternalErr, createTokenErr.InternalErr) - } else { - resp = response.Error(http.StatusInternalServerError, "Error while signing in user", err) - } - return resp + return response.Err(err) } metrics.MApiLoginPost.Inc() - resp = response.JSON(http.StatusOK, map[string]any{ - "message": "Logged in", - "redirectUrl": hs.GetRedirectURL(c), - }) - return resp + return authn.HandleLoginResponse(c.Req, c.Resp, hs.Cfg, identity, hs.ValidateRedirectTo) } func (hs *HTTPServer) loginUserWithUser(user *user.User, c *contextmodel.ReqContext) error { diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 5e2eefa4156..77a0a251824 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -1,405 +1,60 @@ package api import ( - "context" - "crypto/rand" - "crypto/sha256" - "encoding/base64" - "encoding/hex" - "errors" - "fmt" - "net/http" - - "golang.org/x/oauth2" - - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/login" - "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/middleware/cookies" "github.com/grafana/grafana/pkg/services/authn" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" - loginservice "github.com/grafana/grafana/pkg/services/login" - "github.com/grafana/grafana/pkg/services/org" - "github.com/grafana/grafana/pkg/services/user" - "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" ) -var ( - oauthLogger = log.New("oauth") -) - const ( OauthStateCookieName = "oauth_state" OauthPKCECookieName = "oauth_code_verifier" ) -func GenStateString() (string, error) { - rnd := make([]byte, 32) - if _, err := rand.Read(rnd); err != nil { - oauthLogger.Error("failed to generate state string", "err", err) - return "", err - } - return base64.URLEncoding.EncodeToString(rnd), nil -} - -// genPKCECode returns a random URL-friendly string and it's base64 URL encoded SHA256 digest. -func genPKCECode() (string, string, error) { - // IETF RFC 7636 specifies that the code verifier should be 43-128 - // characters from a set of unreserved URI characters which is - // almost the same as the set of characters in base64url. - // https://datatracker.ietf.org/doc/html/rfc7636#section-4.1 - // - // It doesn't hurt to generate a few more bytes here, we generate - // 96 bytes which we then encode using base64url to make sure - // they're within the set of unreserved characters. - // - // 96 is chosen because 96*8/6 = 128, which means that we'll have - // 128 characters after it has been base64 encoded. - raw := make([]byte, 96) - _, err := rand.Read(raw) - if err != nil { - return "", "", err - } - ascii := make([]byte, 128) - base64.RawURLEncoding.Encode(ascii, raw) - - shasum := sha256.Sum256(ascii) - pkce := base64.RawURLEncoding.EncodeToString(shasum[:]) - return string(ascii), pkce, nil -} - func (hs *HTTPServer) OAuthLogin(reqCtx *contextmodel.ReqContext) { name := web.Params(reqCtx.Req)[":name"] - loginInfo := loginservice.LoginInfo{AuthModule: name} if errorParam := reqCtx.Query("error"); errorParam != "" { errorDesc := reqCtx.Query("error_description") - oauthLogger.Error("failed to login ", "error", errorParam, "errorDesc", errorDesc) - hs.handleOAuthLoginErrorWithRedirect(reqCtx, loginInfo, login.ErrProviderDeniedRequest, "error", errorParam, "errorDesc", errorDesc) + hs.log.Error("failed to login ", "error", errorParam, "errorDesc", errorDesc) + + hs.redirectWithError(reqCtx, login.ErrProviderDeniedRequest, "error", errorParam, "errorDesc", errorDesc) return } code := reqCtx.Query("code") - if hs.Cfg.AuthBrokerEnabled { - req := &authn.Request{HTTPRequest: reqCtx.Req, Resp: reqCtx.Resp} - if code == "" { - redirect, err := hs.authnService.RedirectURL(reqCtx.Req.Context(), authn.ClientWithPrefix(name), req) - if err != nil { - reqCtx.Redirect(hs.redirectURLWithErrorCookie(reqCtx, err)) - return - } - - cookies.WriteCookie(reqCtx.Resp, OauthStateCookieName, redirect.Extra[authn.KeyOAuthState], hs.Cfg.OAuthCookieMaxAge, hs.CookieOptionsFromCfg) - - if pkce := redirect.Extra[authn.KeyOAuthPKCE]; pkce != "" { - cookies.WriteCookie(reqCtx.Resp, OauthPKCECookieName, pkce, hs.Cfg.OAuthCookieMaxAge, hs.CookieOptionsFromCfg) - } - - reqCtx.Redirect(redirect.URL) - return - } - - identity, err := hs.authnService.Login(reqCtx.Req.Context(), authn.ClientWithPrefix(name), req) - // NOTE: always delete these cookies, even if login failed - cookies.DeleteCookie(reqCtx.Resp, OauthStateCookieName, hs.CookieOptionsFromCfg) - cookies.DeleteCookie(reqCtx.Resp, OauthPKCECookieName, hs.CookieOptionsFromCfg) - + req := &authn.Request{HTTPRequest: reqCtx.Req, Resp: reqCtx.Resp} + if code == "" { + redirect, err := hs.authnService.RedirectURL(reqCtx.Req.Context(), authn.ClientWithPrefix(name), req) if err != nil { reqCtx.Redirect(hs.redirectURLWithErrorCookie(reqCtx, err)) return } - metrics.MApiLoginOAuth.Inc() - authn.HandleLoginRedirect(reqCtx.Req, reqCtx.Resp, hs.Cfg, identity, hs.ValidateRedirectTo) - return - } + cookies.WriteCookie(reqCtx.Resp, OauthStateCookieName, redirect.Extra[authn.KeyOAuthState], hs.Cfg.OAuthCookieMaxAge, hs.CookieOptionsFromCfg) - provider := hs.SocialService.GetOAuthInfoProvider(name) - if provider == nil { - hs.handleOAuthLoginErrorWithRedirect(reqCtx, loginInfo, errors.New("OAuth not enabled")) - return - } - - connect, err := hs.SocialService.GetConnector(name) - if err != nil { - hs.handleOAuthLoginErrorWithRedirect(reqCtx, loginInfo, fmt.Errorf("no OAuth with name %s configured", name)) - return - } - - if code == "" { - var opts []oauth2.AuthCodeOption - if provider.UsePKCE { - ascii, pkce, err := genPKCECode() - if err != nil { - reqCtx.Logger.Error("Generating PKCE failed", "error", err) - hs.handleOAuthLoginError(reqCtx, loginInfo, LoginError{ - HttpStatus: http.StatusInternalServerError, - PublicMessage: "An internal error occurred", - }) - return - } - - cookies.WriteCookie(reqCtx.Resp, OauthPKCECookieName, ascii, hs.Cfg.OAuthCookieMaxAge, hs.CookieOptionsFromCfg) - - opts = append(opts, - oauth2.SetAuthURLParam("code_challenge", pkce), - oauth2.SetAuthURLParam("code_challenge_method", "S256"), - ) + if pkce := redirect.Extra[authn.KeyOAuthPKCE]; pkce != "" { + cookies.WriteCookie(reqCtx.Resp, OauthPKCECookieName, pkce, hs.Cfg.OAuthCookieMaxAge, hs.CookieOptionsFromCfg) } - state, err := GenStateString() - if err != nil { - reqCtx.Logger.Error("Generating state string failed", "err", err) - hs.handleOAuthLoginError(reqCtx, loginInfo, LoginError{ - HttpStatus: http.StatusInternalServerError, - PublicMessage: "An internal error occurred", - }) - return - } - - hashedState := hs.hashStatecode(state, provider.ClientSecret) - cookies.WriteCookie(reqCtx.Resp, OauthStateCookieName, hashedState, hs.Cfg.OAuthCookieMaxAge, hs.CookieOptionsFromCfg) - if provider.HostedDomain != "" { - opts = append(opts, oauth2.SetAuthURLParam("hd", provider.HostedDomain)) - } - - reqCtx.Redirect(connect.AuthCodeURL(state, opts...)) + reqCtx.Redirect(redirect.URL) return } - cookieState := reqCtx.GetCookie(OauthStateCookieName) - - // delete cookie + identity, err := hs.authnService.Login(reqCtx.Req.Context(), authn.ClientWithPrefix(name), req) + // NOTE: always delete these cookies, even if login failed cookies.DeleteCookie(reqCtx.Resp, OauthStateCookieName, hs.CookieOptionsFromCfg) - - if cookieState == "" { - hs.handleOAuthLoginError(reqCtx, loginInfo, LoginError{ - HttpStatus: http.StatusInternalServerError, - PublicMessage: "login.OAuthLogin(missing saved state)", - }) - return - } - - queryState := hs.hashStatecode(reqCtx.Query("state"), provider.ClientSecret) - oauthLogger.Info("state check", "queryState", queryState, "cookieState", cookieState) - if cookieState != queryState { - hs.handleOAuthLoginError(reqCtx, loginInfo, LoginError{ - HttpStatus: http.StatusInternalServerError, - PublicMessage: "login.OAuthLogin(state mismatch)", - }) - return - } - - oauthClient, err := hs.SocialService.GetOAuthHttpClient(name) - if err != nil { - reqCtx.Logger.Error("Failed to create OAuth http client", "error", err) - hs.handleOAuthLoginError(reqCtx, loginInfo, LoginError{ - HttpStatus: http.StatusInternalServerError, - PublicMessage: "login.OAuthLogin(" + err.Error() + ")", - }) - return - } - - ctx := reqCtx.Req.Context() - oauthCtx := context.WithValue(ctx, oauth2.HTTPClient, oauthClient) - opts := []oauth2.AuthCodeOption{} - - codeVerifier := reqCtx.GetCookie(OauthPKCECookieName) cookies.DeleteCookie(reqCtx.Resp, OauthPKCECookieName, hs.CookieOptionsFromCfg) - if codeVerifier != "" { - opts = append(opts, - oauth2.SetAuthURLParam("code_verifier", codeVerifier), - ) - } - // get token from provider - token, err := connect.Exchange(oauthCtx, code, opts...) if err != nil { - hs.handleOAuthLoginError(reqCtx, loginInfo, LoginError{ - HttpStatus: http.StatusInternalServerError, - PublicMessage: "login.OAuthLogin(NewTransportWithCode)", - Err: err, - }) - return - } - // token.TokenType was defaulting to "bearer", which is out of spec, so we explicitly set to "Bearer" - token.TokenType = "Bearer" - - if hs.Cfg.Env != setting.Dev { - oauthLogger.Debug("OAuthLogin: got token", - "expiry", fmt.Sprintf("%v", token.Expiry), - "type", token.TokenType, - "has_refresh_token", token.RefreshToken != "", - ) - } else { - oauthLogger.Debug("OAuthLogin: got token", - "expiry", fmt.Sprintf("%v", token.Expiry), - "type", token.TokenType, - "access_token", fmt.Sprintf("%v", token.AccessToken), - "refresh_token", fmt.Sprintf("%v", token.RefreshToken), - ) - } - - // set up oauth2 client - client := connect.Client(oauthCtx, token) - - // get user info - userInfo, err := connect.UserInfo(ctx, client, token) - if err != nil { - var sErr *social.Error - if errors.As(err, &sErr) { - hs.handleOAuthLoginErrorWithRedirect(reqCtx, loginInfo, sErr) - } else { - hs.handleOAuthLoginError(reqCtx, loginInfo, LoginError{ - HttpStatus: http.StatusInternalServerError, - PublicMessage: fmt.Sprintf("login.OAuthLogin(get info from %s)", name), - Err: err, - }) - } + reqCtx.Redirect(hs.redirectURLWithErrorCookie(reqCtx, err)) return } - oauthLogger.Debug("OAuthLogin got user info", "userInfo", fmt.Sprintf("%v", userInfo)) - - // validate that we got at least an email address - if userInfo.Email == "" { - hs.handleOAuthLoginErrorWithRedirect(reqCtx, loginInfo, login.ErrNoEmail) - return - } - - // validate that the email is allowed to login to grafana - if !connect.IsEmailAllowed(userInfo.Email) { - hs.handleOAuthLoginErrorWithRedirect(reqCtx, loginInfo, login.ErrEmailNotAllowed) - return - } - - loginInfo.ExternalUser = *hs.buildExternalUserInfo(token, userInfo, name) - loginInfo.User, err = hs.SyncUser(reqCtx, &loginInfo.ExternalUser, connect) - if err != nil { - hs.handleOAuthLoginErrorWithRedirect(reqCtx, loginInfo, err) - return - } - - // login - if err := hs.loginUserWithUser(loginInfo.User, reqCtx); err != nil { - hs.handleOAuthLoginErrorWithRedirect(reqCtx, loginInfo, err) - return - } - - loginInfo.HTTPStatus = http.StatusOK - hs.HooksService.RunLoginHook(&loginInfo, reqCtx) metrics.MApiLoginOAuth.Inc() - - reqCtx.Redirect(hs.GetRedirectURL(reqCtx)) -} - -// buildExternalUserInfo returns a ExternalUserInfo struct from OAuth user profile -func (hs *HTTPServer) buildExternalUserInfo(token *oauth2.Token, userInfo *social.BasicUserInfo, name string) *loginservice.ExternalUserInfo { - oauthLogger.Debug("Building external user info from OAuth user info") - - extUser := &loginservice.ExternalUserInfo{ - AuthModule: fmt.Sprintf("oauth_%s", name), - OAuthToken: token, - AuthId: userInfo.Id, - Name: userInfo.Name, - Login: userInfo.Login, - Email: userInfo.Email, - OrgRoles: map[int64]org.RoleType{}, - Groups: userInfo.Groups, - IsGrafanaAdmin: userInfo.IsGrafanaAdmin, - } - - if userInfo.Role != "" && !hs.Cfg.OAuthSkipOrgRoleUpdateSync { - rt := userInfo.Role - if rt.IsValid() { - // The user will be assigned a role in either the auto-assigned organization or in the default one - var orgID int64 - if hs.Cfg.AutoAssignOrg && hs.Cfg.AutoAssignOrgId > 0 { - orgID = int64(hs.Cfg.AutoAssignOrgId) - plog.Debug("The user has a role assignment and organization membership is auto-assigned", - "role", userInfo.Role, "orgId", orgID) - } else { - orgID = int64(1) - plog.Debug("The user has a role assignment and organization membership is not auto-assigned", - "role", userInfo.Role, "orgId", orgID) - } - extUser.OrgRoles[orgID] = rt - } - } - - return extUser -} - -// SyncUser syncs a Grafana user profile with the corresponding OAuth profile. -func (hs *HTTPServer) SyncUser( - ctx *contextmodel.ReqContext, - extUser *loginservice.ExternalUserInfo, - connect social.SocialConnector, -) (*user.User, error) { - oauthLogger.Debug("Syncing Grafana user with corresponding OAuth profile") - lookupParams := loginservice.UserLookupParams{} - if hs.Cfg.OAuthAllowInsecureEmailLookup { - lookupParams.Email = &extUser.Email - } - - // add/update user in Grafana - cmd := &loginservice.UpsertUserCommand{ - ReqContext: ctx, - ExternalUser: extUser, - SignupAllowed: connect.IsSignupAllowed(), - UserLookupParams: lookupParams, - } - - upsertedUser, err := hs.Login.UpsertUser(ctx.Req.Context(), cmd) - if err != nil { - return nil, err - } - - // Do not expose disabled status, - // just show incorrect user credentials error (see #17947) - if upsertedUser.IsDisabled { - oauthLogger.Warn("User is disabled", "user", upsertedUser.Login) - return nil, login.ErrInvalidCredentials - } - - return upsertedUser, nil -} - -func (hs *HTTPServer) hashStatecode(code, seed string) string { - hashBytes := sha256.Sum256([]byte(code + hs.Cfg.SecretKey + seed)) - return hex.EncodeToString(hashBytes[:]) -} - -type LoginError struct { - HttpStatus int - PublicMessage string - Err error -} - -func (hs *HTTPServer) handleOAuthLoginError(ctx *contextmodel.ReqContext, info loginservice.LoginInfo, err LoginError) { - ctx.Handle(hs.Cfg, err.HttpStatus, err.PublicMessage, err.Err) - - // login hooks is handled by authn.Service - if !hs.Cfg.AuthBrokerEnabled { - info.Error = err.Err - if info.Error == nil { - info.Error = errors.New(err.PublicMessage) - } - info.HTTPStatus = err.HttpStatus - - hs.HooksService.RunLoginHook(&info, ctx) - } -} - -func (hs *HTTPServer) handleOAuthLoginErrorWithRedirect(ctx *contextmodel.ReqContext, info loginservice.LoginInfo, err error, v ...interface{}) { - hs.redirectWithError(ctx, err, v...) - - // login hooks is handled by authn.Service - if !hs.Cfg.AuthBrokerEnabled { - info.Error = err - hs.HooksService.RunLoginHook(&info, ctx) - } + authn.HandleLoginRedirect(reqCtx.Req, reqCtx.Resp, hs.Cfg, identity, hs.ValidateRedirectTo) } diff --git a/pkg/api/login_oauth_test.go b/pkg/api/login_oauth_test.go index 5627c0bed12..fefff77b260 100644 --- a/pkg/api/login_oauth_test.go +++ b/pkg/api/login_oauth_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models/usertoken" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn/authntest" @@ -194,6 +195,7 @@ func TestOAuthLogin_AuthorizationCode(t *testing.T) { func TestOAuthLogin_Error(t *testing.T) { server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = setting.NewCfg() + hs.log = log.NewNopLogger() hs.SecretsService = fakes.NewFakeSecretsService() }) diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 5c3049c658a..1e3ac64183d 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -280,8 +280,6 @@ 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 // TO REMOVE: Not documented & not supported. Remove in 10.3 TagAuthedDevices bool @@ -968,12 +966,11 @@ var skipStaticRootValidation = false func NewCfg() *Cfg { return &Cfg{ - Target: []string{}, - Logger: log.New("settings"), - Raw: ini.Empty(), - Azure: &azsettings.AzureSettings{}, - RBACEnabled: true, - AuthBrokerEnabled: true, + Target: []string{}, + Logger: log.New("settings"), + Raw: ini.Empty(), + Azure: &azsettings.AzureSettings{}, + RBACEnabled: true, } } @@ -1541,7 +1538,6 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { // Do not use cfg.AuthConfigUIAdminAccess = auth.Key("config_ui_admin_access").MustBool(false) - cfg.AuthBrokerEnabled = auth.Key("broker").MustBool(true) cfg.TagAuthedDevices = auth.Key("tag_authed_devices").MustBool(true) cfg.DisableLoginForm = auth.Key("disable_login_form").MustBool(false) diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index b34c95fbfbe..535cd1ac89e 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -384,7 +384,6 @@ 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 0af4042dc03..59240f765ce 100644 --- a/pkg/tests/web/index_view_test.go +++ b/pkg/tests/web/index_view_test.go @@ -119,9 +119,7 @@ func TestIntegrationIndexViewAnalytics(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - grafDir, cfgPath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ - AuthBrokerEnabled: true, - }) + grafDir, cfgPath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{}) addr, store := testinfra.StartGrafana(t, grafDir, cfgPath) createdUser := testinfra.CreateUser(t, store, user.CreateUserCommand{ Login: "admin",