Auth: prevent auto_login redirect if user is already authenticated (#72477)

* Auth: prevent auto_login redirect if user is already authenticated

Before attempting an auto-login for OAuth, verifies if current context has already been
authenticated.

Fixes: #72476

Co-authored-by: Karl Persson <kalle.persson92@gmail.com>

pkg/api/login.go

@@ -115,7 +115,8 @@ func (hs *HTTPServer) LoginView(c *contextmodel.ReqContext) {
 		return
 	}
 
-	if hs.tryAutoLogin(c) {
+	// If user is not authenticated try auto-login
+	if !c.IsSignedIn && hs.tryAutoLogin(c) {
 		return
 	}
 

pkg/api/login_test.go

@@ -36,6 +36,8 @@ import (
 	"github.com/grafana/grafana/pkg/setting"
 )
 
+const loginCookieName = "grafana_session"
+
 func fakeSetIndexViewData(t *testing.T) {
 	origSetIndexViewData := setIndexViewData
 	t.Cleanup(func() {
@@ -110,7 +112,7 @@ func TestLoginErrorCookieAPIEndpoint(t *testing.T) {
 		return response.Empty(http.StatusOK)
 	})
 
-	cfg.LoginCookieName = "grafana_session"
+	cfg.LoginCookieName = loginCookieName
 	setting.SecretKey = "login_testing"
 
 	cfg.OAuthAutoLogin = true
@@ -551,14 +553,65 @@ func TestAuthProxyLoginWithEnableLoginToken(t *testing.T) {
 	assert.Equal(t, "/", location[0])
 	setCookie := sc.resp.Header()["Set-Cookie"]
 	require.NotNil(t, setCookie, "Set-Cookie should exist")
-	assert.Equal(t, "grafana_session=; Path=/; Max-Age=0; HttpOnly", setCookie[0])
+	assert.Equal(t, fmt.Sprintf("%s=; Path=/; Max-Age=0; HttpOnly", loginCookieName), setCookie[0])
+}
+
+func TestAuthProxyLoginWithEnableLoginTokenAndEnabledOauthAutoLogin(t *testing.T) {
+	fakeSetIndexViewData(t)
+
+	mock := &mockSocialService{
+		oAuthInfo: &social.OAuthInfo{
+			ClientId:     "fake",
+			ClientSecret: "fakefake",
+			Enabled:      true,
+			AllowSignup:  true,
+			Name:         "github",
+		},
+		oAuthInfos: oAuthInfos,
+	}
+
+	sc := setupScenarioContext(t, "/login")
+	sc.cfg.LoginCookieName = loginCookieName
+	sc.cfg.OAuthAutoLogin = true
+	hs := &HTTPServer{
+		Cfg:              sc.cfg,
+		SettingsProvider: &setting.OSSImpl{Cfg: sc.cfg},
+		License:          &licensing.OSSLicensingService{},
+		AuthTokenService: authtest.NewFakeUserAuthTokenService(),
+		log:              log.New("hello"),
+		SocialService:    mock,
+		Features:         featuremgmt.WithFeatures(),
+	}
+
+	sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response {
+		c.IsSignedIn = true
+		c.SignedInUser = &user.SignedInUser{
+			UserID: 10,
+		}
+		hs.LoginView(c)
+		return response.Empty(http.StatusOK)
+	})
+
+	sc.cfg.AuthProxyEnabled = true
+	sc.cfg.AuthProxyEnableLoginToken = true
+
+	sc.m.Get(sc.url, sc.defaultHandler)
+	sc.fakeReqNoAssertions("GET", sc.url).exec()
+	require.Equal(t, 302, sc.resp.Code)
+
+	location, ok := sc.resp.Header()["Location"]
+	assert.True(t, ok)
+	assert.Equal(t, "/", location[0])
+	setCookie := sc.resp.Header()["Set-Cookie"]
+	require.NotNil(t, setCookie, "Set-Cookie should exist")
+	assert.Equal(t, fmt.Sprintf("%s=; Path=/; Max-Age=0; HttpOnly", loginCookieName), setCookie[0])
 }
 
 func setupAuthProxyLoginTest(t *testing.T, enableLoginToken bool) *scenarioContext {
 	fakeSetIndexViewData(t)
 
 	sc := setupScenarioContext(t, "/login")
-	sc.cfg.LoginCookieName = "grafana_session"
+	sc.cfg.LoginCookieName = loginCookieName
 	hs := &HTTPServer{
 		Cfg:              sc.cfg,
 		SettingsProvider: &setting.OSSImpl{Cfg: sc.cfg},