From 4fc18325a0fdc356a2615b333e1cbc84231f5ae2 Mon Sep 17 00:00:00 2001 From: Stavros Date: Fri, 16 Jan 2026 17:06:22 +0200 Subject: [PATCH] refactor: rework ldap group fetching logic --- .zed/debug.json | 13 ++++++ Makefile | 5 +++ frontend/src/pages/login-page.tsx | 6 ++- .../migrations/000005_ldap_groups.down.sql | 1 - .../migrations/000005_ldap_groups.up.sql | 1 - internal/bootstrap/app_bootstrap.go | 14 +++++-- internal/config/config.go | 6 +++ internal/controller/context_controller.go | 2 - .../controller/context_controller_test.go | 5 ++- internal/controller/oauth_controller.go | 2 +- internal/controller/proxy_controller.go | 22 +++++++--- internal/controller/proxy_controller_test.go | 1 + internal/controller/user_controller.go | 19 ++------- internal/controller/user_controller_test.go | 6 +-- internal/middleware/context_middleware.go | 40 +++++++++++++------ internal/repository/models.go | 1 - internal/repository/queries.sql.go | 21 +++------- internal/service/auth_service.go | 29 +++++++++++--- sql/queries.sql | 8 ++-- sql/schema.sql | 3 +- 20 files changed, 129 insertions(+), 76 deletions(-) create mode 100644 .zed/debug.json delete mode 100644 internal/assets/migrations/000005_ldap_groups.down.sql delete mode 100644 internal/assets/migrations/000005_ldap_groups.up.sql diff --git a/.zed/debug.json b/.zed/debug.json new file mode 100644 index 0000000..767b3a0 --- /dev/null +++ b/.zed/debug.json @@ -0,0 +1,13 @@ +[ + { + "label": "Attach to remote Delve", + "adapter": "Delve", + "mode": "remote", + "remotePath": "/tinyauth", + "request": "attach", + "tcp_connection": { + "host": "127.0.0.1", + "port": 4000, + }, + }, +] diff --git a/Makefile b/Makefile index a7d526e..ccf9735 100644 --- a/Makefile +++ b/Makefile @@ -62,3 +62,8 @@ develop: # Production prod: docker compose -f $(PROD_COMPOSE) up --force-recreate --pull=always --remove-orphans + +# SQL +.PHONY: sql +sql: + sqlc generate diff --git a/frontend/src/pages/login-page.tsx b/frontend/src/pages/login-page.tsx index e2efdd6..a4e939a 100644 --- a/frontend/src/pages/login-page.tsx +++ b/frontend/src/pages/login-page.tsx @@ -50,10 +50,12 @@ export const LoginPage = () => { const redirectUri = searchParams.get("redirect_uri"); const oauthProviders = providers.filter( - (provider) => provider.id !== "username", + (provider) => provider.id !== "local" && provider.id != "ldap", ); const userAuthConfigured = - providers.find((provider) => provider.id === "username") !== undefined; + providers.find( + (provider) => provider.id === "local" || provider.id === "ldap", + ) !== undefined; const oauthMutation = useMutation({ mutationFn: (provider: string) => diff --git a/internal/assets/migrations/000005_ldap_groups.down.sql b/internal/assets/migrations/000005_ldap_groups.down.sql deleted file mode 100644 index 047c05c..0000000 --- a/internal/assets/migrations/000005_ldap_groups.down.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE "sessions" DROP COLUMN "ldap_groups"; diff --git a/internal/assets/migrations/000005_ldap_groups.up.sql b/internal/assets/migrations/000005_ldap_groups.up.sql deleted file mode 100644 index b75f36f..0000000 --- a/internal/assets/migrations/000005_ldap_groups.up.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE "sessions" ADD COLUMN "ldap_groups" TEXT; diff --git a/internal/bootstrap/app_bootstrap.go b/internal/bootstrap/app_bootstrap.go index 20355fc..f1c4b0b 100644 --- a/internal/bootstrap/app_bootstrap.go +++ b/internal/bootstrap/app_bootstrap.go @@ -144,10 +144,18 @@ func (app *BootstrapApp) Setup() error { return configuredProviders[i].Name < configuredProviders[j].Name }) - if services.authService.UserAuthConfigured() { + if services.authService.LocalAuthConfigured() { configuredProviders = append(configuredProviders, controller.Provider{ - Name: "Username", - ID: "username", + Name: "Local", + ID: "local", + OAuth: false, + }) + } + + if services.authService.LdapAuthConfigured() { + configuredProviders = append(configuredProviders, controller.Provider{ + Name: "LDAP", + ID: "ldap", OAuth: false, }) } diff --git a/internal/config/config.go b/internal/config/config.go index 6f30f60..9ab4a00 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -153,6 +153,7 @@ type UserContext struct { Name string Email string IsLoggedIn bool + IsBasicAuth bool OAuth bool Provider string TotpPending bool @@ -189,6 +190,7 @@ type App struct { IP AppIP `description:"IP access configuration." yaml:"ip"` Response AppResponse `description:"Response customization." yaml:"response"` Path AppPath `description:"Path access configuration." yaml:"path"` + LDAP AppLDAP `description:"LDAP access configuration." yaml:"ldap"` } type AppConfig struct { @@ -205,6 +207,10 @@ type AppOAuth struct { Groups string `description:"Comma-separated list of required OAuth groups." yaml:"groups"` } +type AppLDAP struct { + Groups string `description:"Comma-separated list of required LDAP groups." yaml:"groups"` +} + type AppIP struct { Allow []string `description:"List of allowed IPs or CIDR ranges." yaml:"allow"` Block []string `description:"List of blocked IPs or CIDR ranges." yaml:"block"` diff --git a/internal/controller/context_controller.go b/internal/controller/context_controller.go index 3c6f008..58651a1 100644 --- a/internal/controller/context_controller.go +++ b/internal/controller/context_controller.go @@ -21,7 +21,6 @@ type UserContextResponse struct { OAuth bool `json:"oauth"` TotpPending bool `json:"totpPending"` OAuthName string `json:"oauthName"` - OAuthSub string `json:"oauthSub"` } type AppContextResponse struct { @@ -90,7 +89,6 @@ func (controller *ContextController) userContextHandler(c *gin.Context) { OAuth: context.OAuth, TotpPending: context.TotpPending, OAuthName: context.OAuthName, - OAuthSub: context.OAuthSub, } if err != nil { diff --git a/internal/controller/context_controller_test.go b/internal/controller/context_controller_test.go index 1a28e54..30e89f0 100644 --- a/internal/controller/context_controller_test.go +++ b/internal/controller/context_controller_test.go @@ -16,8 +16,8 @@ import ( var controllerCfg = controller.ContextControllerConfig{ Providers: []controller.Provider{ { - Name: "Username", - ID: "username", + Name: "Local", + ID: "local", OAuth: false, }, { @@ -40,6 +40,7 @@ var userContext = config.UserContext{ Name: "testuser", Email: "test@example.com", IsLoggedIn: true, + IsBasicAuth: false, OAuth: false, Provider: "username", TotpPending: false, diff --git a/internal/controller/oauth_controller.go b/internal/controller/oauth_controller.go index b5c1d00..019bae7 100644 --- a/internal/controller/oauth_controller.go +++ b/internal/controller/oauth_controller.go @@ -189,7 +189,7 @@ func (controller *OAuthController) oauthCallbackHandler(c *gin.Context) { username = user.PreferredUsername } else { tlog.App.Debug().Msg("No preferred username from OAuth provider, using pseudo username") - username = strings.Replace(user.Email, "@", "_", -1) + username = strings.Replace(user.Email, "@", "_", 1) } sessionCookie := repository.Session{ diff --git a/internal/controller/proxy_controller.go b/internal/controller/proxy_controller.go index bb62712..bbb1946 100644 --- a/internal/controller/proxy_controller.go +++ b/internal/controller/proxy_controller.go @@ -173,7 +173,7 @@ func (controller *ProxyController) proxyHandler(c *gin.Context) { tlog.App.Trace().Interface("context", userContext).Msg("User context from request") - if userContext.Provider == "basic" && userContext.TotpEnabled { + if userContext.IsBasicAuth && userContext.TotpEnabled { tlog.App.Debug().Msg("User has TOTP enabled, denying basic auth access") userContext.IsLoggedIn = false } @@ -212,11 +212,17 @@ func (controller *ProxyController) proxyHandler(c *gin.Context) { return } - if userContext.OAuth { - groupOK := controller.auth.IsInOAuthGroup(c, userContext, acls.OAuth.Groups) + if userContext.OAuth || userContext.Provider == "ldap" { + var groupOK bool + + if userContext.OAuth { + groupOK = controller.auth.IsInOAuthGroup(c, userContext, acls.OAuth.Groups) + } else { + groupOK = controller.auth.IsInLdapGroup(c, userContext, acls.LDAP.Groups) + } if !groupOK { - tlog.App.Warn().Str("user", userContext.Username).Str("resource", strings.Split(host, ".")[0]).Msg("User OAuth groups do not match resource requirements") + tlog.App.Warn().Str("user", userContext.Username).Str("resource", strings.Split(host, ".")[0]).Msg("User groups do not match resource requirements") if req.Proxy == "nginx" || !isBrowser { c.JSON(403, gin.H{ @@ -251,7 +257,13 @@ func (controller *ProxyController) proxyHandler(c *gin.Context) { c.Header("Remote-User", utils.SanitizeHeader(userContext.Username)) c.Header("Remote-Name", utils.SanitizeHeader(userContext.Name)) c.Header("Remote-Email", utils.SanitizeHeader(userContext.Email)) - c.Header("Remote-Groups", utils.SanitizeHeader(userContext.OAuthGroups)) + + if userContext.Provider == "ldap" { + c.Header("Remote-Groups", utils.SanitizeHeader(userContext.LdapGroups)) + } else if userContext.Provider != "local" { + c.Header("Remote-Groups", utils.SanitizeHeader(userContext.OAuthGroups)) + } + c.Header("Remote-Sub", utils.SanitizeHeader(userContext.OAuthSub)) controller.setHeaders(c, acls) diff --git a/internal/controller/proxy_controller_test.go b/internal/controller/proxy_controller_test.go index e0ddaf4..9e83c49 100644 --- a/internal/controller/proxy_controller_test.go +++ b/internal/controller/proxy_controller_test.go @@ -192,6 +192,7 @@ func TestProxyHandler(t *testing.T) { Name: "testuser", Email: "testuser@example.com", IsLoggedIn: true, + IsBasicAuth: true, OAuth: false, Provider: "basic", TotpPending: false, diff --git a/internal/controller/user_controller.go b/internal/controller/user_controller.go index d4f0f44..35d60d3 100644 --- a/internal/controller/user_controller.go +++ b/internal/controller/user_controller.go @@ -116,7 +116,7 @@ func (controller *UserController) loginHandler(c *gin.Context) { Username: user.Username, Name: utils.Capitalize(req.Username), Email: fmt.Sprintf("%s@%s", strings.ToLower(req.Username), controller.config.CookieDomain), - Provider: "username", + Provider: "local", TotpPending: true, }) @@ -142,22 +142,11 @@ func (controller *UserController) loginHandler(c *gin.Context) { Username: req.Username, Name: utils.Capitalize(req.Username), Email: fmt.Sprintf("%s@%s", strings.ToLower(req.Username), controller.config.CookieDomain), - Provider: "username", + Provider: "local", } if userSearch.Type == "ldap" { - ldapUser, err := controller.auth.GetLdapUser(userSearch.Username) - - if err != nil { - tlog.App.Error().Err(err).Str("username", req.Username).Msg("Failed to get LDAP user details") - c.JSON(500, gin.H{ - "status": 500, - "message": "Internal Server Error", - }) - return - } - - sessionCookie.LdapGroups = strings.Join(ldapUser.Groups, ",") + sessionCookie.Provider = "ldap" } tlog.App.Trace().Interface("session_cookie", sessionCookie).Msg("Creating session cookie") @@ -267,7 +256,7 @@ func (controller *UserController) totpHandler(c *gin.Context) { Username: user.Username, Name: utils.Capitalize(user.Username), Email: fmt.Sprintf("%s@%s", strings.ToLower(user.Username), controller.config.CookieDomain), - Provider: "username", + Provider: "local", } tlog.App.Trace().Interface("session_cookie", sessionCookie).Msg("Creating session cookie") diff --git a/internal/controller/user_controller_test.go b/internal/controller/user_controller_test.go index ef10487..fedc98c 100644 --- a/internal/controller/user_controller_test.go +++ b/internal/controller/user_controller_test.go @@ -204,7 +204,7 @@ func TestTotpHandler(t *testing.T) { Email: "totpuser@example.com", IsLoggedIn: false, OAuth: false, - Provider: "username", + Provider: "local", TotpPending: true, OAuthGroups: "", TotpEnabled: true, @@ -267,7 +267,7 @@ func TestTotpHandler(t *testing.T) { Email: "totpuser@example.com", IsLoggedIn: false, OAuth: false, - Provider: "username", + Provider: "local", TotpPending: true, OAuthGroups: "", TotpEnabled: true, @@ -290,7 +290,7 @@ func TestTotpHandler(t *testing.T) { Email: "totpuser@example.com", IsLoggedIn: false, OAuth: false, - Provider: "username", + Provider: "local", TotpPending: false, OAuthGroups: "", TotpEnabled: false, diff --git a/internal/middleware/context_middleware.go b/internal/middleware/context_middleware.go index 06469d2..eb56498 100644 --- a/internal/middleware/context_middleware.go +++ b/internal/middleware/context_middleware.go @@ -49,7 +49,7 @@ func (m *ContextMiddleware) Middleware() gin.HandlerFunc { Username: cookie.Username, Name: cookie.Name, Email: cookie.Email, - Provider: "username", + Provider: "local", TotpPending: true, TotpEnabled: true, }) @@ -58,23 +58,37 @@ func (m *ContextMiddleware) Middleware() gin.HandlerFunc { } switch cookie.Provider { - case "username": + case "local", "ldap": userSearch := m.auth.SearchUser(cookie.Username) - if userSearch.Type == "unknown" || userSearch.Type == "error" { + if userSearch.Type == "unknown" { tlog.App.Debug().Msg("User from session cookie not found") m.auth.DeleteSessionCookie(c) goto basic } + var ldapGroups []string + + if userSearch.Type == "ldap" { + ldapUser, err := m.auth.GetLdapUser(userSearch.Username) + + if err != nil { + tlog.App.Error().Err(err).Msg("Error retrieving LDAP user details") + c.Next() + return + } + + ldapGroups = ldapUser.Groups + } + m.auth.RefreshSessionCookie(c) c.Set("context", &config.UserContext{ Username: cookie.Username, Name: cookie.Name, Email: cookie.Email, - Provider: "username", + Provider: userSearch.Type, IsLoggedIn: true, - LdapGroups: cookie.LdapGroups, + LdapGroups: strings.Join(ldapGroups, ","), }) c.Next() return @@ -156,9 +170,10 @@ func (m *ContextMiddleware) Middleware() gin.HandlerFunc { Username: user.Username, Name: utils.Capitalize(user.Username), Email: fmt.Sprintf("%s@%s", strings.ToLower(user.Username), m.config.CookieDomain), - Provider: "username", + Provider: "local", IsLoggedIn: true, TotpEnabled: user.TotpSecret != "", + IsBasicAuth: true, }) c.Next() return @@ -174,12 +189,13 @@ func (m *ContextMiddleware) Middleware() gin.HandlerFunc { } c.Set("context", &config.UserContext{ - Username: basic.Username, - Name: utils.Capitalize(basic.Username), - Email: fmt.Sprintf("%s@%s", strings.ToLower(basic.Username), m.config.CookieDomain), - Provider: "ldap", - IsLoggedIn: true, - LdapGroups: strings.Join(ldapUser.Groups, ","), + Username: basic.Username, + Name: utils.Capitalize(basic.Username), + Email: fmt.Sprintf("%s@%s", strings.ToLower(basic.Username), m.config.CookieDomain), + Provider: "ldap", + IsLoggedIn: true, + LdapGroups: strings.Join(ldapUser.Groups, ","), + IsBasicAuth: true, }) c.Next() return diff --git a/internal/repository/models.go b/internal/repository/models.go index b1879c0..61f7f80 100644 --- a/internal/repository/models.go +++ b/internal/repository/models.go @@ -16,5 +16,4 @@ type Session struct { CreatedAt int64 OAuthName string OAuthSub string - LdapGroups string } diff --git a/internal/repository/queries.sql.go b/internal/repository/queries.sql.go index 78057e5..e171b7a 100644 --- a/internal/repository/queries.sql.go +++ b/internal/repository/queries.sql.go @@ -21,12 +21,11 @@ INSERT INTO sessions ( "expiry", "created_at", "oauth_name", - "oauth_sub", - "ldap_groups" + "oauth_sub" ) VALUES ( - ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? + ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ) -RETURNING uuid, username, email, name, provider, totp_pending, oauth_groups, expiry, created_at, oauth_name, oauth_sub, ldap_groups +RETURNING uuid, username, email, name, provider, totp_pending, oauth_groups, expiry, created_at, oauth_name, oauth_sub ` type CreateSessionParams struct { @@ -41,7 +40,6 @@ type CreateSessionParams struct { CreatedAt int64 OAuthName string OAuthSub string - LdapGroups string } func (q *Queries) CreateSession(ctx context.Context, arg CreateSessionParams) (Session, error) { @@ -57,7 +55,6 @@ func (q *Queries) CreateSession(ctx context.Context, arg CreateSessionParams) (S arg.CreatedAt, arg.OAuthName, arg.OAuthSub, - arg.LdapGroups, ) var i Session err := row.Scan( @@ -72,7 +69,6 @@ func (q *Queries) CreateSession(ctx context.Context, arg CreateSessionParams) (S &i.CreatedAt, &i.OAuthName, &i.OAuthSub, - &i.LdapGroups, ) return i, err } @@ -98,7 +94,7 @@ func (q *Queries) DeleteSession(ctx context.Context, uuid string) error { } const getSession = `-- name: GetSession :one -SELECT uuid, username, email, name, provider, totp_pending, oauth_groups, expiry, created_at, oauth_name, oauth_sub, ldap_groups FROM "sessions" +SELECT uuid, username, email, name, provider, totp_pending, oauth_groups, expiry, created_at, oauth_name, oauth_sub FROM "sessions" WHERE "uuid" = ? ` @@ -117,7 +113,6 @@ func (q *Queries) GetSession(ctx context.Context, uuid string) (Session, error) &i.CreatedAt, &i.OAuthName, &i.OAuthSub, - &i.LdapGroups, ) return i, err } @@ -132,10 +127,9 @@ UPDATE "sessions" SET "oauth_groups" = ?, "expiry" = ?, "oauth_name" = ?, - "oauth_sub" = ?, - "ldap_groups" = ? + "oauth_sub" = ? WHERE "uuid" = ? -RETURNING uuid, username, email, name, provider, totp_pending, oauth_groups, expiry, created_at, oauth_name, oauth_sub, ldap_groups +RETURNING uuid, username, email, name, provider, totp_pending, oauth_groups, expiry, created_at, oauth_name, oauth_sub ` type UpdateSessionParams struct { @@ -148,7 +142,6 @@ type UpdateSessionParams struct { Expiry int64 OAuthName string OAuthSub string - LdapGroups string UUID string } @@ -163,7 +156,6 @@ func (q *Queries) UpdateSession(ctx context.Context, arg UpdateSessionParams) (S arg.Expiry, arg.OAuthName, arg.OAuthSub, - arg.LdapGroups, arg.UUID, ) var i Session @@ -179,7 +171,6 @@ func (q *Queries) UpdateSession(ctx context.Context, arg UpdateSessionParams) (S &i.CreatedAt, &i.OAuthName, &i.OAuthSub, - &i.LdapGroups, ) return i, err } diff --git a/internal/service/auth_service.go b/internal/service/auth_service.go index c0f9cd4..54de520 100644 --- a/internal/service/auth_service.go +++ b/internal/service/auth_service.go @@ -75,7 +75,7 @@ func (auth *AuthService) SearchUser(username string) config.UserSearch { if err != nil { tlog.App.Warn().Err(err).Str("username", username).Msg("Failed to search for user in LDAP") return config.UserSearch{ - Type: "error", + Type: "unknown", } } @@ -230,7 +230,6 @@ func (auth *AuthService) CreateSessionCookie(c *gin.Context, data *repository.Se CreatedAt: time.Now().Unix(), OAuthName: data.OAuthName, OAuthSub: data.OAuthSub, - LdapGroups: data.LdapGroups, } _, err = auth.queries.CreateSession(c, session) @@ -284,7 +283,6 @@ func (auth *AuthService) RefreshSessionCookie(c *gin.Context) error { OAuthName: session.OAuthName, OAuthSub: session.OAuthSub, UUID: session.UUID, - LdapGroups: session.LdapGroups, }) if err != nil { @@ -361,12 +359,15 @@ func (auth *AuthService) GetSessionCookie(c *gin.Context) (repository.Session, e OAuthGroups: session.OAuthGroups, OAuthName: session.OAuthName, OAuthSub: session.OAuthSub, - LdapGroups: session.LdapGroups, }, nil } -func (auth *AuthService) UserAuthConfigured() bool { - return len(auth.config.Users) > 0 || auth.ldap != nil +func (auth *AuthService) LocalAuthConfigured() bool { + return len(auth.config.Users) > 0 +} + +func (auth *AuthService) LdapAuthConfigured() bool { + return auth.ldap != nil } func (auth *AuthService) IsUserAllowed(c *gin.Context, context config.UserContext, acls config.App) bool { @@ -409,6 +410,22 @@ func (auth *AuthService) IsInOAuthGroup(c *gin.Context, context config.UserConte return false } +func (auth *AuthService) IsInLdapGroup(c *gin.Context, context config.UserContext, requiredGroups string) bool { + if requiredGroups == "" { + return true + } + + for userGroup := range strings.SplitSeq(context.LdapGroups, ",") { + if utils.CheckFilter(requiredGroups, strings.TrimSpace(userGroup)) { + tlog.App.Trace().Str("group", userGroup).Str("required", requiredGroups).Msg("User group matched") + return true + } + } + + tlog.App.Debug().Msg("No groups matched") + return false +} + func (auth *AuthService) IsAuthEnabled(uri string, path config.AppPath) (bool, error) { // Check for block list if path.Block != "" { diff --git a/sql/queries.sql b/sql/queries.sql index 9004edd..9fde4e2 100644 --- a/sql/queries.sql +++ b/sql/queries.sql @@ -10,10 +10,9 @@ INSERT INTO sessions ( "expiry", "created_at", "oauth_name", - "oauth_sub", - "ldap_groups" + "oauth_sub" ) VALUES ( - ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? + ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ) RETURNING *; @@ -35,8 +34,7 @@ UPDATE "sessions" SET "oauth_groups" = ?, "expiry" = ?, "oauth_name" = ?, - "oauth_sub" = ?, - "ldap_groups" = ? + "oauth_sub" = ? WHERE "uuid" = ? RETURNING *; diff --git a/sql/schema.sql b/sql/schema.sql index bd5abdf..a7f37eb 100644 --- a/sql/schema.sql +++ b/sql/schema.sql @@ -9,6 +9,5 @@ CREATE TABLE IF NOT EXISTS "sessions" ( "expiry" INTEGER NOT NULL, "created_at" INTEGER NOT NULL, "oauth_name" TEXT NULL, - "oauth_sub" TEXT NULL, - "ldap_groups" TEXT NULL + "oauth_sub" TEXT NULL );