SAML: Do not SAML SLO if user is not SAML authenticated (#53418)

* Only SLO user if the user is using SAML

* only one source of truth for auth module info

* ensure SAML is also enabled and not only SLO

* move auth module naming to auth module login package

* use constants in other previously unused spots

pkg/api/api.go

@@ -21,11 +21,14 @@
 //
 // SecurityDefinitions:
 // basic:
-//  type: basic
+//
+//	type: basic
+//
 // api_key:
-//  type: apiKey
+//
-//  name: Authorization
+//	type: apiKey
-//  in: header
+//	name: Authorization
+//	in: header
 //
 // swagger:meta
 package api

pkg/api/ldap_debug.go

@@ -12,6 +12,7 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/ldap"
+	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/multildap"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/services/user"
@@ -220,7 +221,7 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon
 		return response.Error(500, "Failed to get user", err)
 	}
 
-	authModuleQuery := &models.GetAuthInfoQuery{UserId: usr.ID, AuthModule: models.AuthModuleLDAP}
+	authModuleQuery := &models.GetAuthInfoQuery{UserId: usr.ID, AuthModule: login.LDAPAuthModule}
 	if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), authModuleQuery); err != nil { // validate the userId comes from LDAP
 		if errors.Is(err, user.ErrUserNotFound) {
 			return response.Error(404, user.ErrUserNotFound.Error(), nil)

pkg/api/login.go

@@ -16,6 +16,7 @@ import (
 	"github.com/grafana/grafana/pkg/login"
 	"github.com/grafana/grafana/pkg/middleware/cookies"
 	"github.com/grafana/grafana/pkg/models"
+	loginService "github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/secrets"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
@@ -287,9 +288,15 @@ func (hs *HTTPServer) loginUserWithUser(user *user.User, c *models.ReqContext) e
 }
 
 func (hs *HTTPServer) Logout(c *models.ReqContext) {
+	// If SAML is enabled and this is a SAML user use saml logout
 	if hs.samlSingleLogoutEnabled() {
-		c.Redirect(hs.Cfg.AppSubURL + "/logout/saml")
+		getAuthQuery := models.GetAuthInfoQuery{UserId: c.UserId}
-		return
+		if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &getAuthQuery); err == nil {
+			if getAuthQuery.Result.AuthModule == loginService.SAMLAuthModule {
+				c.Redirect(hs.Cfg.AppSubURL + "/logout/saml")
+				return
+			}
+		}
 	}
 
 	err := hs.AuthTokenService.RevokeToken(c.Req.Context(), c.UserToken, false)
@@ -360,7 +367,7 @@ func (hs *HTTPServer) samlName() string {
 }
 
 func (hs *HTTPServer) samlSingleLogoutEnabled() bool {
-	return hs.SettingsProvider.KeyValue("auth.saml", "single_logout").MustBool(false) && hs.samlEnabled()
+	return hs.samlEnabled() && hs.SettingsProvider.KeyValue("auth.saml", "single_logout").MustBool(false) && hs.samlEnabled()
 }
 
 func getLoginExternalError(err error) string {

pkg/api/login_test.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 	"testing"
 
+	loginservice "github.com/grafana/grafana/pkg/services/login"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -658,9 +659,9 @@ func TestLoginPostRunLokingHook(t *testing.T) {
 		{
 			desc:       "valid LDAP user",
 			authUser:   testUser,
-			authModule: "ldap",
+			authModule: loginservice.LDAPAuthModule,
 			info: models.LoginInfo{
-				AuthModule: "ldap",
+				AuthModule: loginservice.LDAPAuthModule,
 				User:       testUser,
 				HTTPStatus: 200,
 			},

pkg/api/password.go

@@ -8,6 +8,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
@@ -39,7 +40,7 @@ func (hs *HTTPServer) SendResetPasswordEmail(c *models.ReqContext) response.Resp
 	getAuthQuery := models.GetAuthInfoQuery{UserId: usr.ID}
 	if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &getAuthQuery); err == nil {
 		authModule := getAuthQuery.Result.AuthModule
-		if authModule == models.AuthModuleLDAP || authModule == models.AuthModuleProxy {
+		if authModule == login.LDAPAuthModule || authModule == login.AuthProxyAuthModule {
 			return response.Error(401, "Not allowed to reset password for LDAP or Auth Proxy user", nil)
 		}
 	}

pkg/api/team_members.go

@@ -11,6 +11,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/web"
 )
@@ -55,7 +56,7 @@ func (hs *HTTPServer) GetTeamMembers(c *models.ReqContext) response.Response {
 		member.Labels = []string{}
 
 		if hs.License.FeatureEnabled("teamgroupsync") && member.External {
-			authProvider := GetAuthProviderLabel(member.AuthModule)
+			authProvider := login.GetAuthProviderLabel(member.AuthModule)
 			member.Labels = append(member.Labels, authProvider)
 		}
 

pkg/api/user.go

@@ -9,6 +9,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
@@ -60,7 +61,7 @@ func (hs *HTTPServer) getUserUserProfile(c *models.ReqContext, userID int64) res
 	getAuthQuery := models.GetAuthInfoQuery{UserId: userID}
 	query.Result.AuthLabels = []string{}
 	if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &getAuthQuery); err == nil {
-		authLabel := GetAuthProviderLabel(getAuthQuery.Result.AuthModule)
+		authLabel := login.GetAuthProviderLabel(getAuthQuery.Result.AuthModule)
 		query.Result.AuthLabels = append(query.Result.AuthLabels, authLabel)
 		query.Result.IsExternal = true
 	}
@@ -394,7 +395,7 @@ func (hs *HTTPServer) ChangeUserPassword(c *models.ReqContext) response.Response
 	getAuthQuery := models.GetAuthInfoQuery{UserId: user.ID}
 	if err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &getAuthQuery); err == nil {
 		authModule := getAuthQuery.Result.AuthModule
-		if authModule == models.AuthModuleLDAP || authModule == models.AuthModuleProxy {
+		if authModule == login.LDAPAuthModule || authModule == login.AuthProxyAuthModule {
 			return response.Error(400, "Not allowed to reset password for LDAP or Auth Proxy user", nil)
 		}
 	}
@@ -482,29 +483,6 @@ func (hs *HTTPServer) ClearHelpFlags(c *models.ReqContext) response.Response {
 	return response.JSON(http.StatusOK, &util.DynMap{"message": "Help flag set", "helpFlags1": cmd.HelpFlags1})
 }
 
-func GetAuthProviderLabel(authModule string) string {
-	switch authModule {
-	case "oauth_github":
-		return "GitHub"
-	case "oauth_google":
-		return "Google"
-	case "oauth_azuread":
-		return "AzureAD"
-	case "oauth_gitlab":
-		return "GitLab"
-	case "oauth_grafana_com", "oauth_grafananet":
-		return "grafana.com"
-	case "auth.saml":
-		return "SAML"
-	case "authproxy":
-		return "Auth Proxy"
-	case "ldap", "":
-		return "LDAP"
-	default:
-		return "OAuth"
-	}
-}
-
 // swagger:parameters searchUsers
 type SearchUsersParams struct {
 	// Limit the maximum number of users to return per page

pkg/login/auth.go

@@ -65,7 +65,7 @@ func (a *AuthenticatorService) AuthenticateUser(ctx context.Context, query *mode
 
 	ldapEnabled, ldapErr := loginUsingLDAP(ctx, query, a.loginService)
 	if ldapEnabled {
-		query.AuthModule = models.AuthModuleLDAP
+		query.AuthModule = login.LDAPAuthModule
 		if ldapErr == nil || !errors.Is(ldapErr, ldap.ErrInvalidCredentials) {
 			return ldapErr
 		}

pkg/login/auth_test.go

@@ -118,7 +118,7 @@ func TestAuthenticateUser(t *testing.T) {
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.True(t, sc.ldapLoginWasCalled)
 		assert.True(t, sc.saveInvalidLoginAttemptWasCalled)
-		assert.Equal(t, "ldap", sc.loginUserQuery.AuthModule)
+		assert.Equal(t, login.LDAPAuthModule, sc.loginUserQuery.AuthModule)
 	})
 
 	authScenario(t, "When a non-existing grafana user authenticate and valid ldap credentials", func(sc *authScenarioContext) {
@@ -135,7 +135,7 @@ func TestAuthenticateUser(t *testing.T) {
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.True(t, sc.ldapLoginWasCalled)
 		assert.False(t, sc.saveInvalidLoginAttemptWasCalled)
-		assert.Equal(t, "ldap", sc.loginUserQuery.AuthModule)
+		assert.Equal(t, login.LDAPAuthModule, sc.loginUserQuery.AuthModule)
 	})
 
 	authScenario(t, "When a non-existing grafana user authenticate and ldap returns unexpected error", func(sc *authScenarioContext) {
@@ -153,7 +153,7 @@ func TestAuthenticateUser(t *testing.T) {
 		assert.True(t, sc.grafanaLoginWasCalled)
 		assert.True(t, sc.ldapLoginWasCalled)
 		assert.False(t, sc.saveInvalidLoginAttemptWasCalled)
-		assert.Equal(t, "ldap", sc.loginUserQuery.AuthModule)
+		assert.Equal(t, login.LDAPAuthModule, sc.loginUserQuery.AuthModule)
 	})
 
 	authScenario(t, "When grafana user authenticate with invalid credentials and invalid ldap credentials", func(sc *authScenarioContext) {

pkg/models/user_auth.go

@@ -9,11 +9,6 @@ import (
 	"golang.org/x/oauth2"
 )
 
-const (
-	AuthModuleLDAP  = "ldap"
-	AuthModuleProxy = "authproxy"
-)
-
 type UserAuth struct {
 	Id                int64
 	UserId            int64

pkg/services/contexthandler/authproxy/authproxy.go

@@ -258,7 +258,7 @@ func (auth *AuthProxy) LoginViaLDAP(reqCtx *models.ReqContext) (int64, error) {
 func (auth *AuthProxy) loginViaHeader(reqCtx *models.ReqContext) (int64, error) {
 	header := auth.getDecodedHeader(reqCtx, auth.cfg.AuthProxyHeaderName)
 	extUser := &models.ExternalUserInfo{
-		AuthModule: "authproxy",
+		AuthModule: login.AuthProxyAuthModule,
 		AuthId:     header,
 	}
 

pkg/services/ldap/ldap.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/login"
 )
 
 // IConnection is interface for LDAP connection manipulation
@@ -429,7 +430,7 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*models.ExternalUserIn
 
 	attrs := server.Config.Attr
 	extUser := &models.ExternalUserInfo{
-		AuthModule: models.AuthModuleLDAP,
+		AuthModule: login.LDAPAuthModule,
 		AuthId:     user.DN,
 		Name: strings.TrimSpace(
 			fmt.Sprintf(

pkg/services/login/authinfo.go

@@ -14,3 +14,34 @@ type AuthInfoService interface {
 	SetAuthInfo(ctx context.Context, cmd *models.SetAuthInfoCommand) error
 	UpdateAuthInfo(ctx context.Context, cmd *models.UpdateAuthInfoCommand) error
 }
+
+const (
+	SAMLAuthModule      = "auth.saml"
+	LDAPAuthModule      = "ldap"
+	AuthProxyAuthModule = "authproxy"
+)
+
+func GetAuthProviderLabel(authModule string) string {
+	switch authModule {
+	case "oauth_github":
+		return "GitHub"
+	case "oauth_google":
+		return "Google"
+	case "oauth_azuread":
+		return "AzureAD"
+	case "oauth_gitlab":
+		return "GitLab"
+	case "oauth_grafana_com", "oauth_grafananet":
+		return "grafana.com"
+	case SAMLAuthModule:
+		return "SAML"
+	case LDAPAuthModule, "": // FIXME: verify this situation doesn't exist anymore
+		return "LDAP"
+	case "jwt":
+		return "JWT"
+	case AuthProxyAuthModule:
+		return "Auth Proxy"
+	default:
+		return "OAuth" // FIXME: replace with "Unknown" and handle generic oauth as a case
+	}
+}

pkg/services/login/loginservice/loginservice.go

@@ -124,7 +124,7 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUser
 			}
 		}
 
-		if extUser.AuthModule == models.AuthModuleLDAP && usr.IsDisabled {
+		if extUser.AuthModule == login.LDAPAuthModule && usr.IsDisabled {
 			// Re-enable user when it found in LDAP
 			if errDisableUser := ls.SQLStore.DisableUser(ctx,
 				&models.DisableUserCommand{

pkg/services/login/loginservice/loginservice_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/login/logintest"
 	"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
 	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
@@ -144,7 +145,7 @@ func createUserOrgDTO() []*models.UserOrgDTO {
 
 func createSimpleExternalUser() models.ExternalUserInfo {
 	externalUser := models.ExternalUserInfo{
-		AuthModule: "ldap",
+		AuthModule: login.LDAPAuthModule,
 		OrgRoles: map[int64]models.RoleType{
 			1: models.ROLE_VIEWER,
 		},

pkg/services/searchusers/searchusers.go

@@ -6,6 +6,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 )
 
@@ -99,7 +100,7 @@ func (s *OSSService) SearchUser(c *models.ReqContext) (*models.SearchUsersQuery,
 		user.AuthLabels = make([]string, 0)
 		if user.AuthModule != nil && len(user.AuthModule) > 0 {
 			for _, authModule := range user.AuthModule {
-				user.AuthLabels = append(user.AuthLabels, GetAuthProviderLabel(authModule))
+				user.AuthLabels = append(user.AuthLabels, login.GetAuthProviderLabel(authModule))
 			}
 		}
 	}
@@ -109,26 +110,3 @@ func (s *OSSService) SearchUser(c *models.ReqContext) (*models.SearchUsersQuery,
 
 	return query, nil
 }
-
-func GetAuthProviderLabel(authModule string) string {
-	switch authModule {
-	case "oauth_github":
-		return "GitHub"
-	case "oauth_google":
-		return "Google"
-	case "oauth_azuread":
-		return "AzureAD"
-	case "oauth_gitlab":
-		return "GitLab"
-	case "oauth_grafana_com", "oauth_grafananet":
-		return "grafana.com"
-	case "auth.saml":
-		return "SAML"
-	case "ldap", "":
-		return "LDAP"
-	case "jwt":
-		return "JWT"
-	default:
-		return "OAuth"
-	}
-}