fix: review comments batch 3

.github/workflows/scorecard.yml

@@ -38,6 +38,6 @@ jobs:
           retention-days: 5
 
       - name: Upload to code-scanning
-        uses: github/codeql-action/upload-sarif@e46ed2cbd01164d986452f91f178727624ae40d7 # v4
+        uses: github/codeql-action/upload-sarif@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4
         with:
           sarif_file: results.sarif

internal/bootstrap/app_bootstrap.go

@@ -104,13 +104,7 @@ func (app *BootstrapApp) Setup() error {
 	}
 
 	// Get cookie domain
-	cookieDomainResolver := utils.GetCookieDomain
-	if !app.config.Auth.SubdomainsEnabled {
-		tlog.App.Info().Msg("Subdomains disabled, automatic authentication for proxied apps will not work")
-		cookieDomainResolver = utils.GetStandaloneCookieDomain
-	}
-
-	cookieDomain, err := cookieDomainResolver(app.context.appUrl)
+	cookieDomain, err := utils.GetCookieDomain(app.context.appUrl)
 
 	if err != nil {
 		return err

internal/bootstrap/router_bootstrap.go

@@ -84,7 +84,6 @@ func (app *BootstrapApp) setupRouter() (*gin.Engine, error) {
 		RedirectCookieName:     app.context.redirectCookieName,
 		CookieDomain:           app.context.cookieDomain,
 		OAuthSessionCookieName: app.context.oauthSessionCookieName,
-		SubdomainsEnabled:      app.config.Auth.SubdomainsEnabled,
 	}, apiRouter, app.services.authService)
 
 	oauthController.SetupRoutes()

internal/bootstrap/service_bootstrap.go

@@ -100,7 +100,6 @@ func (app *BootstrapApp) initServices(queries *repository.Queries) (Services, er
 		SessionCookieName:  app.context.sessionCookieName,
 		IP:                 app.config.Auth.IP,
 		LDAPGroupsCacheTTL: app.config.LDAP.GroupCacheTTL,
-		SubdomainsEnabled:  app.config.Auth.SubdomainsEnabled,
 	}, services.ldapService, queries, services.oauthBrokerService)
 
 	err = authService.Init()

internal/controller/context_controller.go

@@ -95,7 +95,7 @@ func (controller *ContextController) userContextHandler(c *gin.Context) {
 		Username:    context.GetUsername(),
 		Name:        context.GetName(),
 		Email:       context.GetEmail(),
-		Provider:    context.GetProviderID(),
+		Provider:    context.ProviderName(),
 		OAuth:       context.IsOAuth(),
 		TOTPPending: context.TOTPPending(),
 		OAuthName:   context.OAuthName(),

internal/controller/oauth_controller.go

@@ -26,7 +26,6 @@ type OAuthControllerConfig struct {
 	SecureCookie           bool
 	AppURL                 string
 	CookieDomain           string
-	SubdomainsEnabled      bool
 }
 
 type OAuthController struct {
@@ -106,7 +105,7 @@ func (controller *OAuthController) oauthURLHandler(c *gin.Context) {
 		return
 	}
 
-	c.SetCookie(controller.config.OAuthSessionCookieName, sessionId, int(time.Hour.Seconds()), "/", controller.getCookieDomain(), controller.config.SecureCookie, true)
+	c.SetCookie(controller.config.OAuthSessionCookieName, sessionId, int(time.Hour.Seconds()), "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
 
 	c.JSON(200, gin.H{
 		"status":  200,
@@ -136,7 +135,7 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) {
 		return
 	}
 
-	c.SetCookie(controller.config.OAuthSessionCookieName, "", -1, "/", controller.getCookieDomain(), controller.config.SecureCookie, true)
+	c.SetCookie(controller.config.OAuthSessionCookieName, "", -1, "/", fmt.Sprintf(".%s", controller.config.CookieDomain), controller.config.SecureCookie, true)
 
 	oauthPendingSession, err := controller.auth.GetOAuthPendingSession(sessionIdCookie)
 
@@ -284,10 +283,3 @@ func (controller *OAuthController) isOidcRequest(params service.OAuthURLParams)
 		params.ClientID != "" &&
 		params.RedirectURI != ""
 }
-
-func (controller *OAuthController) getCookieDomain() string {
-	if controller.config.SubdomainsEnabled {
-		return "." + controller.config.CookieDomain
-	}
-	return controller.config.CookieDomain
-}

internal/controller/proxy_controller_test.go

@@ -98,6 +98,7 @@ func TestProxyController(t *testing.T) {
 					Name:     "Totpuser",
 					Email:    "totpuser@example.com",
 				},
+				TOTPEnabled: true,
 			},
 		})
 		c.Next()

internal/controller/user_controller.go

@@ -102,7 +102,7 @@ func (controller *UserController) loginHandler(c *gin.Context) {
 	}
 
 	if err := controller.auth.CheckUserPassword(*search, req.Password); err != nil {
-		tlog.App.Warn().Err(err).Str("username", req.Username).Msg("Failed to verify password")
+		tlog.App.Warn().Str("username", req.Username).Msg("Invalid password")
 		controller.auth.RecordLoginAttempt(req.Username, false)
 		tlog.AuditLoginFailure(c, req.Username, "username", "invalid password")
 		c.JSON(401, gin.H{
@@ -112,20 +112,16 @@ func (controller *UserController) loginHandler(c *gin.Context) {
 		return
 	}
 
+	tlog.App.Info().Str("username", req.Username).Msg("Login successful")
+	tlog.AuditLoginSuccess(c, req.Username, "username")
+
+	controller.auth.RecordLoginAttempt(req.Username, true)
+
 	var localUser *model.LocalUser
 
 	if search.Type == model.UserLocal {
 		localUser = controller.auth.GetLocalUser(req.Username)
 
-		if localUser == nil {
-			tlog.App.Warn().Str("username", req.Username).Msg("User disappeared during login")
-			c.JSON(401, gin.H{
-				"status":  401,
-				"message": "Unauthorized",
-			})
-			return
-		}
-
 		if localUser.TOTPSecret != "" {
 			tlog.App.Debug().Str("username", req.Username).Msg("User has TOTP enabled, requiring TOTP verification")
 
@@ -202,11 +198,6 @@ func (controller *UserController) loginHandler(c *gin.Context) {
 
 	http.SetCookie(c.Writer, cookie)
 
-	tlog.App.Info().Str("username", req.Username).Msg("Login successful")
-	tlog.AuditLoginSuccess(c, req.Username, "username")
-
-	controller.auth.RecordLoginAttempt(req.Username, true)
-
 	c.JSON(200, gin.H{
 		"status":  200,
 		"message": "Login successful",
@@ -249,7 +240,7 @@ func (controller *UserController) logoutHandler(c *gin.Context) {
 	context, err := new(model.UserContext).NewFromGin(c)
 
 	if err == nil {
-		tlog.AuditLogout(c, context.GetUsername(), context.GetProviderID())
+		tlog.AuditLogout(c, context.GetUsername(), context.ProviderName())
 	} else {
 		tlog.App.Warn().Err(err).Msg("Failed to get user context for logout audit, proceeding without username")
 		tlog.AuditLogout(c, "unknown", "unknown")
@@ -335,16 +326,8 @@ func (controller *UserController) totpHandler(c *gin.Context) {
 		return
 	}
 
-	uuid, err := c.Cookie(controller.config.SessionCookieName)
-
-	if err == nil {
-		_, err = controller.auth.DeleteSession(c, uuid)
-		if err != nil {
-			tlog.App.Warn().Err(err).Msg("Failed to delete pending TOTP session")
-		}
-	} else {
-		tlog.App.Warn().Err(err).Msg("Failed to retrieve session cookie for pending TOTP session, proceeding without deleting it")
-	}
+	tlog.App.Info().Str("username", context.GetUsername()).Msg("TOTP verification successful")
+	tlog.AuditLoginSuccess(c, context.GetUsername(), "totp")
 
 	controller.auth.RecordLoginAttempt(context.GetUsername(), true)
 
@@ -377,9 +360,6 @@ func (controller *UserController) totpHandler(c *gin.Context) {
 
 	http.SetCookie(c.Writer, cookie)
 
-	tlog.App.Info().Str("username", context.GetUsername()).Msg("TOTP verification successful")
-	tlog.AuditLoginSuccess(c, context.GetUsername(), "totp")
-
 	c.JSON(200, gin.H{
 		"status":  200,
 		"message": "Login successful",

internal/controller/user_controller_test.go

@@ -1,9 +1,7 @@
 package controller_test
 
 import (
-	"context"
 	"encoding/json"
-	"net/http"
 	"net/http/httptest"
 	"path"
 	"strings"
@@ -78,6 +76,7 @@ func TestUserController(t *testing.T) {
 					Email:    "totpuser@example.com",
 				},
 				TOTPPending: true,
+				TOTPEnabled: true,
 			},
 		})
 	}
@@ -93,6 +92,7 @@ func TestUserController(t *testing.T) {
 					Email:    "bob@example.com",
 				},
 				TOTPPending: true,
+				TOTPEnabled: true,
 			},
 		})
 	}
@@ -111,15 +111,6 @@ func TestUserController(t *testing.T) {
 		})
 	}
 
-	oauthBrokerCfgs := make(map[string]model.OAuthServiceConfig)
-
-	app := bootstrap.NewBootstrapApp(model.Config{})
-
-	db, err := app.SetupDatabase(path.Join(tempDir, "tinyauth.db"))
-	require.NoError(t, err)
-
-	queries := repository.New(db)
-
 	type testCase struct {
 		description string
 		middlewares []gin.HandlerFunc
@@ -150,9 +141,7 @@ func TestUserController(t *testing.T) {
 				assert.Equal(t, "tinyauth-session", cookie.Name)
 				assert.True(t, cookie.HttpOnly)
 				assert.Equal(t, "example.com", cookie.Domain)
-				// 3 seconds should be more than enough for even slow test environments
-				assert.GreaterOrEqual(t, cookie.MaxAge, 7)
-				assert.LessOrEqual(t, cookie.MaxAge, 10)
+				assert.Equal(t, 9, cookie.MaxAge)
 			},
 		},
 		{
@@ -241,8 +230,7 @@ func TestUserController(t *testing.T) {
 				assert.Equal(t, "tinyauth-session", cookie.Name)
 				assert.True(t, cookie.HttpOnly)
 				assert.Equal(t, "example.com", cookie.Domain)
-				assert.GreaterOrEqual(t, cookie.MaxAge, 3597)
-				assert.LessOrEqual(t, cookie.MaxAge, 3600)
+				assert.Equal(t, 3599, cookie.MaxAge) // 1 hour, default for totp pending sessions
 			},
 		},
 		{
@@ -294,18 +282,6 @@ func TestUserController(t *testing.T) {
 				totpCtx,
 			},
 			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				_, err := queries.CreateSession(context.TODO(), repository.CreateSessionParams{
-					UUID:        "test-totp-login-uuid",
-					Username:    "test",
-					Email:       "test@example.com",
-					Name:        "Test",
-					Provider:    "local",
-					TotpPending: true,
-					Expiry:      time.Now().Add(1 * time.Hour).Unix(),
-					CreatedAt:   time.Now().Unix(),
-				})
-				require.NoError(t, err)
-
 				code, err := totp.GenerateCode("JPIEBDKJH6UGWJMX66RR3S55UFP2SGKK", time.Now())
 				assert.NoError(t, err)
 
@@ -319,13 +295,7 @@ func TestUserController(t *testing.T) {
 				recorder = httptest.NewRecorder()
 				req := httptest.NewRequest("POST", "/api/user/totp", strings.NewReader(string(totpReqBody)))
 				req.Header.Set("Content-Type", "application/json")
-				req.AddCookie(&http.Cookie{
-					Name:     "tinyauth-session",
-					Value:    "test-totp-login-uuid",
-					HttpOnly: true,
-					MaxAge:   3600,
-					Expires:  time.Now().Add(1 * time.Hour),
-				})
+
 				router.ServeHTTP(recorder, req)
 
 				assert.Equal(t, 200, recorder.Code)
@@ -336,8 +306,7 @@ func TestUserController(t *testing.T) {
 				assert.Equal(t, "tinyauth-session", totpCookie.Name)
 				assert.True(t, totpCookie.HttpOnly)
 				assert.Equal(t, "example.com", totpCookie.Domain)
-				assert.GreaterOrEqual(t, totpCookie.MaxAge, 7)
-				assert.LessOrEqual(t, totpCookie.MaxAge, 10)
+				assert.Equal(t, 9, totpCookie.MaxAge) // should use the regular session expiry time
 			},
 		},
 		{
@@ -418,18 +387,6 @@ func TestUserController(t *testing.T) {
 				totpAttrCtx,
 			},
 			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
-				_, err := queries.CreateSession(context.TODO(), repository.CreateSessionParams{
-					UUID:        "test-totp-login-attributes-uuid",
-					Username:    "test",
-					Email:       "test@example.com",
-					Name:        "Test",
-					Provider:    "local",
-					TotpPending: true,
-					Expiry:      time.Now().Add(1 * time.Hour).Unix(),
-					CreatedAt:   time.Now().Unix(),
-				})
-				require.NoError(t, err)
-
 				code, err := totp.GenerateCode("JPIEBDKJH6UGWJMX66RR3S55UFP2SGKK", time.Now())
 				require.NoError(t, err)
 
@@ -439,13 +396,6 @@ func TestUserController(t *testing.T) {
 
 				req := httptest.NewRequest("POST", "/api/user/totp", strings.NewReader(string(body)))
 				req.Header.Set("Content-Type", "application/json")
-				req.AddCookie(&http.Cookie{
-					Name:     "tinyauth-session",
-					Value:    "test-totp-login-attributes-uuid",
-					HttpOnly: true,
-					MaxAge:   3600,
-					Expires:  time.Now().Add(1 * time.Hour),
-				})
 				router.ServeHTTP(recorder, req)
 
 				require.Equal(t, 200, recorder.Code)
@@ -456,6 +406,15 @@ func TestUserController(t *testing.T) {
 		},
 	}
 
+	oauthBrokerCfgs := make(map[string]model.OAuthServiceConfig)
+
+	app := bootstrap.NewBootstrapApp(model.Config{})
+
+	db, err := app.SetupDatabase(path.Join(tempDir, "tinyauth.db"))
+	require.NoError(t, err)
+
+	queries := repository.New(db)
+
 	docker := service.NewDockerService()
 	err = docker.Init()
 	require.NoError(t, err)

internal/middleware/context_middleware.go

@@ -123,6 +123,7 @@ func (m *ContextMiddleware) cookieAuth(ctx context.Context, uuid string) (*model
 
 	if userContext.Provider == model.ProviderLocal &&
 		userContext.Local.TOTPPending {
+		userContext.Local.TOTPEnabled = true
 		return userContext, nil, nil
 	}
 

internal/middleware/context_middleware_test.go

@@ -109,6 +109,7 @@ func TestContextMiddleware(t *testing.T) {
 				assert.Equal(t, "testuser", userCtx.GetUsername())
 				assert.True(t, userCtx.Authenticated)
 				require.NotNil(t, userCtx.Local)
+				assert.False(t, userCtx.Local.TOTPEnabled)
 			},
 		},
 		{
@@ -133,6 +134,7 @@ func TestContextMiddleware(t *testing.T) {
 				assert.False(t, userCtx.Authenticated)
 				require.NotNil(t, userCtx.Local)
 				assert.True(t, userCtx.Local.TOTPPending)
+				assert.True(t, userCtx.Local.TOTPEnabled)
 			},
 		},
 		{

internal/model/config.go

@@ -18,7 +18,6 @@ func NewDefaultConfiguration() *Config {
 			Address: "0.0.0.0",
 		},
 		Auth: AuthConfig{
-			SubdomainsEnabled:  true,
 			SessionExpiry:      86400, // 1 day
 			SessionMaxLifetime: 0,     // disabled
 			LoginTimeout:       300,   // 5 minutes
@@ -103,7 +102,6 @@ type ServerConfig struct {
 type AuthConfig struct {
 	IP                 IPConfig                  `description:"IP whitelisting config options." yaml:"ip"`
 	Users              []string                  `description:"Comma-separated list of users (username:hashed_password)." yaml:"users"`
-	SubdomainsEnabled  bool                      `description:"Enable subdomains support." yaml:"subdomainsEnabled"`
 	UserAttributes     map[string]UserAttributes `description:"Map of per-user OIDC attributes (username -> attributes)." yaml:"userAttributes"`
 	UsersFile          string                    `description:"Path to the users file." yaml:"usersFile"`
 	SecureCookie       bool                      `description:"Enable secure cookies." yaml:"secureCookie"`

internal/model/context.go

@@ -34,6 +34,7 @@ type BaseContext struct {
 type LocalContext struct {
 	BaseContext
 	TOTPPending bool
+	TOTPEnabled bool
 	Attributes  UserAttributes
 }
 
@@ -222,14 +223,14 @@ func (c *UserContext) GetName() string {
 	}
 }
 
-func (c *UserContext) GetProviderID() string {
+func (c *UserContext) ProviderName() string {
 	switch c.Provider {
 	case ProviderBasicAuth, ProviderLocal:
 		return "local"
 	case ProviderLDAP:
 		return "ldap"
 	case ProviderOAuth:
-		return c.OAuth.ID
+		return c.OAuth.DisplayName // compatability
 	default:
 		return "unknown"
 	}

internal/model/context_test.go

@@ -153,29 +153,29 @@ func TestContext(t *testing.T) {
 		{
 			description: "ProviderName returns 'local' for ProviderLocal",
 			context:     &model.UserContext{Provider: model.ProviderLocal},
-			run:         func(t *testing.T, c *model.UserContext) any { return c.GetProviderID() },
+			run:         func(t *testing.T, c *model.UserContext) any { return c.ProviderName() },
 			expected:    "local",
 		},
 		{
 			description: "ProviderName returns 'local' for ProviderBasicAuth",
 			context:     &model.UserContext{Provider: model.ProviderBasicAuth},
-			run:         func(t *testing.T, c *model.UserContext) any { return c.GetProviderID() },
+			run:         func(t *testing.T, c *model.UserContext) any { return c.ProviderName() },
 			expected:    "local",
 		},
 		{
 			description: "ProviderName returns 'ldap' for ProviderLDAP",
 			context:     &model.UserContext{Provider: model.ProviderLDAP},
-			run:         func(t *testing.T, c *model.UserContext) any { return c.GetProviderID() },
+			run:         func(t *testing.T, c *model.UserContext) any { return c.ProviderName() },
 			expected:    "ldap",
 		},
 		{
-			description: "ProviderName returns OAuth provider ID for ProviderOAuth",
+			description: "ProviderName returns OAuth DisplayName for ProviderOAuth",
 			context: &model.UserContext{
 				Provider: model.ProviderOAuth,
-				OAuth:    &model.OAuthContext{ID: "github"},
+				OAuth:    &model.OAuthContext{DisplayName: "GitHub"},
 			},
-			run:      func(t *testing.T, c *model.UserContext) any { return c.GetProviderID() },
-			expected: "github",
+			run:      func(t *testing.T, c *model.UserContext) any { return c.ProviderName() },
+			expected: "GitHub",
 		},
 		{
 			description: "TOTPPending returns true when local context is pending",

internal/service/auth_service.go

@@ -84,7 +84,6 @@ type AuthServiceConfig struct {
 	SessionCookieName  string
 	IP                 model.IPConfig
 	LDAPGroupsCacheTTL int
-	SubdomainsEnabled  bool
 }
 
 type AuthService struct {
@@ -398,12 +397,6 @@ func (auth *AuthService) DeleteSession(ctx context.Context, uuid string) (*http.
 		tlog.App.Warn().Err(err).Msg("Failed to delete session from database, proceeding to clear cookie anyway")
 	}
 
-	err = auth.queries.DeleteSession(ctx, uuid)
-
-	if err != nil {
-		return nil, err
-	}
-
 	return &http.Cookie{
 		Name:     auth.config.SessionCookieName,
 		Value:    "",
@@ -845,10 +838,3 @@ func (auth *AuthService) ClearRateLimitsTestingOnly() {
 	}
 	auth.loginMutex.Unlock()
 }
-
-func (auth *AuthService) getCookieDomain() string {
-	if auth.config.SubdomainsEnabled {
-		return "." + auth.config.CookieDomain
-	}
-	return auth.config.CookieDomain
-}

internal/service/oauth_extractors.go

@@ -27,7 +27,7 @@ func defaultExtractor(client *http.Client, url string) (*model.Claims, error) {
 	return simpleReq[model.Claims](client, url, nil)
 }
 
-func githubExtractor(client *http.Client, _ string) (*model.Claims, error) {
+func githubExtractor(client *http.Client, url string) (*model.Claims, error) {
 	var user model.Claims
 
 	userInfo, err := simpleReq[GithubUserInfoResponse](client, "https://api.github.com/user", map[string]string{
@@ -49,7 +49,7 @@ func githubExtractor(client *http.Client, _ string) (*model.Claims, error) {
 	}
 
 	for _, email := range *userEmails {
-		if email.Primary && email.Verified {
+		if email.Primary {
 			user.Email = email.Email
 			break
 		}

internal/utils/app_utils.go

@@ -47,15 +47,6 @@ func GetCookieDomain(u string) (string, error) {
 	return domain, nil
 }
 
-func GetStandaloneCookieDomain(u string) (string, error) {
-	parsed, err := url.Parse(u)
-	if err != nil {
-		return "", err
-	}
-
-	return parsed.Hostname(), nil
-}
-
 func ParseFileToLine(content string) string {
 	lines := strings.Split(content, "\n")
 	users := make([]string, 0)