From 2b01b5749572ab6f296ecdcb6493f2ae2e6c9f79 Mon Sep 17 00:00:00 2001 From: Jan Eitzinger Date: Wed, 17 Jun 2026 07:54:26 +0200 Subject: [PATCH] feat: replace gorilla/sessions with alexedwards/scs/v2 Browser sessions are now server-side, stored in the SQLite database via scs/sqlite3store (new `sessions` table, DB migration to version 12) instead of gorilla/sessions client-side cookie storage. Only an opaque random token is kept in the cookie; session data lives server-side and survives restarts. Session middleware is wired as a hybrid to avoid buffering large responses: scs.LoadAndSave on the login/logout write paths, and a non-buffering read-only LoadSession middleware on the secured/config/frontend read paths so the large GraphQL /query responses stream unbuffered. JWT-only APIs (/api, /userapi, /api/metricstore) and static files are left unwrapped. The session cookie Secure flag is now derived from the server config (set when cc-backend terminates TLS itself); previously it was effectively never set. The SESSION_KEY env var is removed as server-side tokens need no signing secret. The dormant Bearer-JWT branch in the frontend urql client is removed; the web UI authenticates GraphQL via the session cookie. Closes #558 Co-Authored-By: Claude Opus 4.8 (1M context) Entire-Checkpoint: b51075f43cc7 --- Makefile | 2 +- README.md | 15 ++ ReleaseNotes.md | 46 ++++- cmd/cc-backend/init.go | 3 - cmd/cc-backend/server.go | 17 +- configs/env-template.txt | 3 - go.mod | 4 +- go.sum | 11 +- internal/api/api_test.go | 1 - internal/api/nats_test.go | 1 - internal/auth/auth.go | 176 +++++++++--------- internal/repository/migration.go | 3 +- .../sqlite3/12_add-sessions-table.down.sql | 1 + .../sqlite3/12_add-sessions-table.up.sql | 7 + web/frontend/src/generic/utils.js | 11 +- 15 files changed, 183 insertions(+), 118 deletions(-) create mode 100644 internal/repository/migrations/sqlite3/12_add-sessions-table.down.sql create mode 100644 internal/repository/migrations/sqlite3/12_add-sessions-table.up.sql diff --git a/Makefile b/Makefile index 04ad954c..1cdb2283 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ TARGET = ./cc-backend FRONTEND = ./web/frontend -VERSION = 1.5.4 +VERSION = 1.6.0 GIT_HASH := $(shell git rev-parse --short HEAD || echo 'development') CURRENT_TIME = $(shell date +"%Y-%m-%d:T%H:%M:%S") LD_FLAGS = '-s -X main.date=${CURRENT_TIME} -X main.version=${VERSION} -X main.commit=${GIT_HASH}' diff --git a/README.md b/README.md index 86b12034..899651ad 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,21 @@ ln -s ./var/job-archive ./cc-backend -help ``` +### Authentication and sessions + +Browser sessions are stored server-side in the SQLite database (the `sessions` +table) using [`alexedwards/scs`](https://github.com/alexedwards/scs); only an +opaque random token is kept in the session cookie. No cookie-signing secret is +required, so the former `SESSION_KEY` environment variable is no longer used and +can be removed from your `.env`. + +The session cookie's `Secure` flag is set automatically when cc-backend serves +HTTPS itself (i.e. `https-cert-file` and `https-key-file` are configured in +`config.json`); otherwise it is left unset so that plain-HTTP development works. +For production deployments, serve cc-backend over HTTPS so the session cookie is +marked `Secure`. If you terminate TLS at a reverse proxy, prefer letting +cc-backend serve HTTPS directly for now so the flag is applied. + ## Database Configuration cc-backend uses SQLite as its database. For large installations, SQLite memory diff --git a/ReleaseNotes.md b/ReleaseNotes.md index 6eea3bf2..13aa14f9 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -1,10 +1,48 @@ -# `cc-backend` version 1.5.4 +# `cc-backend` version 1.6.0 + +Supports job archive version 3 and database version 12. + +This release replaces the browser session implementation and requires a database +migration to version 12. Run `./cc-backend -migrate-db` after upgrading; the new +`sessions` table is created automatically (a fresh `-init-db` also creates it). +Existing login sessions are invalidated by the upgrade, so users have to log in +again once. See the behavior changes below regarding the session cookie `Secure` +flag and the removal of the `SESSION_KEY` environment variable. +For release specific notes visit the [ClusterCockpit Documentation](https://clusterockpit.org/docs/release/). + +## Changes in 1.6.0 + +### Behavior changes + +- **Session backend replaced (`gorilla/sessions` → `alexedwards/scs`)**: Browser + sessions are now managed by `github.com/alexedwards/scs/v2` with server-side + storage in the SQLite database (new `sessions` table, database version 12) + instead of `gorilla/sessions` client-side cookie storage. Only an opaque random + token is stored in the cookie; the session payload lives server-side. Sessions + still survive backend restarts. This requires the database migration noted above + and invalidates existing sessions. +- **Session cookie `Secure` flag**: The `Secure` attribute on the session cookie + is now derived from the server configuration — it is set when cc-backend + terminates TLS itself (`https-cert-file` and `https-key-file` configured) and + unset otherwise. Previously the flag was effectively never set. Deployments that + terminate TLS at a reverse proxy and want the `Secure` flag should serve + cc-backend over HTTPS directly for now. +- **`SESSION_KEY` removed**: The `SESSION_KEY` environment variable is no longer + used and should be removed from your `.env`. Server-side sessions use random + tokens, so no cookie-signing secret is required. It is ignored if left in place. + +### Dependencies + +- **Added** `github.com/alexedwards/scs/v2` and + `github.com/alexedwards/scs/sqlite3store`. +- **Removed** `github.com/gorilla/sessions` (and `github.com/gorilla/securecookie`). + +## Changes in 1.5.4 Supports job archive version 3 and database version 11. -This is a security and bugfix release of `cc-backend`, the API backend and +This was a security and bugfix release of `cc-backend`, the API backend and frontend implementation of ClusterCockpit. -For release specific notes visit the [ClusterCockpit Documentation](https://clusterockpit.org/docs/release/). If you are upgrading from v1.5.1 no database migration is required. If you are upgrading from v1.5.0 you need to do another DB migration. This should not take long. For optimal database performance after the migration it is @@ -15,8 +53,6 @@ While we are confident that the memory issue with the metricstore cleanup move policy is fixed, it is still recommended to use delete policy for cleanup. This is also the default. -## Changes in 1.5.4 - ### Security fixes - **JWT HMAC empty-key bypass (critical)**: `jwtSession.go` now refuses to diff --git a/cmd/cc-backend/init.go b/cmd/cc-backend/init.go index 09ad2084..cdb185fc 100644 --- a/cmd/cc-backend/init.go +++ b/cmd/cc-backend/init.go @@ -23,9 +23,6 @@ const envString = ` # You can generate your own keypair using the gen-keypair tool JWT_PUBLIC_KEY="kzfYrYy+TzpanWZHJ5qSdMj5uKUWgq74BWhQG6copP0=" JWT_PRIVATE_KEY="dtPC/6dWJFKZK7KZ78CvWuynylOmjBFyMsUWArwmodOTN9itjL5POlqdZkcnmpJ0yPm4pRaCrvgFaFAbpyik/Q==" - -# Some random bytes used as secret for cookie-based sessions (DO NOT USE THIS ONE IN PRODUCTION) -SESSION_KEY="67d829bf61dc5f87a73fd814e2c9f629" ` const configString = ` diff --git a/cmd/cc-backend/server.go b/cmd/cc-backend/server.go index 4a9e71b1..5909fb9d 100644 --- a/cmd/cc-backend/server.go +++ b/cmd/cc-backend/server.go @@ -121,6 +121,7 @@ func (s *Server) init() error { } authHandle := auth.GetAuthInstance() + sessionManager := authHandle.SessionManager() // Middleware must be defined before routes in chi s.router.Use(func(next http.Handler) http.Handler { @@ -220,10 +221,12 @@ func (s *Server) init() error { }) } - s.router.Post("/login", authHandle.Login(loginFailureHandler).ServeHTTP) - s.router.HandleFunc("/jwt-login", authHandle.Login(loginFailureHandler).ServeHTTP) + // Login/logout mutate the session, so they are wrapped with + // scs.LoadAndSave, which commits the session and writes the cookie. + s.router.Post("/login", sessionManager.LoadAndSave(authHandle.Login(loginFailureHandler)).ServeHTTP) + s.router.Handle("/jwt-login", sessionManager.LoadAndSave(authHandle.Login(loginFailureHandler))) - s.router.Post("/logout", authHandle.Logout( + s.router.Post("/logout", sessionManager.LoadAndSave(authHandle.Logout( http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "text/html; charset=utf-8") rw.WriteHeader(http.StatusOK) @@ -234,7 +237,7 @@ func (s *Server) init() error { Build: buildInfo, Infos: info, }) - })).ServeHTTP) + }))).ServeHTTP) } if flagDev { @@ -246,6 +249,10 @@ func (s *Server) init() error { // Secured routes (require authentication) s.router.Group(func(secured chi.Router) { if !config.Keys.DisableAuthentication { + // Non-buffering session load: makes the session available to + // AuthViaSession without wrapping/buffering the (potentially large, + // e.g. GraphQL /query) response. + secured.Use(authHandle.LoadSession) secured.Use(func(next http.Handler) http.Handler { return authHandle.Auth( next, @@ -309,6 +316,7 @@ func (s *Server) init() error { // the /config page route that is registered in the secured group) s.router.Group(func(configapi chi.Router) { if !config.Keys.DisableAuthentication { + configapi.Use(authHandle.LoadSession) configapi.Use(func(next http.Handler) http.Handler { return authHandle.AuthConfigAPI(next, onFailureResponse) }) @@ -319,6 +327,7 @@ func (s *Server) init() error { // Frontend API routes s.router.Route("/frontend", func(frontendapi chi.Router) { if !config.Keys.DisableAuthentication { + frontendapi.Use(authHandle.LoadSession) frontendapi.Use(func(next http.Handler) http.Handler { return authHandle.AuthFrontendAPI(next, onFailureResponse) }) diff --git a/configs/env-template.txt b/configs/env-template.txt index e62a1fae..dd51b122 100644 --- a/configs/env-template.txt +++ b/configs/env-template.txt @@ -7,8 +7,5 @@ JWT_PRIVATE_KEY="dtPC/6dWJFKZK7KZ78CvWuynylOmjBFyMsUWArwmodOTN9itjL5POlqdZkcnmpJ # Keys in PEM format can be converted, see `tools/convert-pem-pubkey/Readme.md` CROSS_LOGIN_JWT_PUBLIC_KEY="" -# Some random bytes used as secret for cookie-based sessions (DO NOT USE THIS ONE IN PRODUCTION) -SESSION_KEY="67d829bf61dc5f87a73fd814e2c9f629" - # Password for the ldap server (optional) LDAP_ADMIN_PASSWORD="mashup" diff --git a/go.mod b/go.mod index 54aec9c4..d1f3caa9 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,8 @@ require ( github.com/ClusterCockpit/cc-lib/v2 v2.12.0 github.com/ClusterCockpit/cc-line-protocol/v2 v2.4.0 github.com/Masterminds/squirrel v1.5.4 + github.com/alexedwards/scs/sqlite3store v0.0.0-20251002162104-209de6e426de + github.com/alexedwards/scs/v2 v2.9.0 github.com/aws/aws-sdk-go-v2 v1.41.7 github.com/aws/aws-sdk-go-v2/config v1.32.18 github.com/aws/aws-sdk-go-v2/credentials v1.19.17 @@ -25,7 +27,6 @@ require ( github.com/golang-jwt/jwt/v5 v5.3.1 github.com/golang-migrate/migrate/v4 v4.19.1 github.com/google/gops v0.3.29 - github.com/gorilla/sessions v1.4.0 github.com/jmoiron/sqlx v1.4.0 github.com/joho/godotenv v1.5.1 github.com/mattn/go-sqlite3 v1.14.44 @@ -80,7 +81,6 @@ require ( github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/goccy/go-yaml v1.19.2 // indirect github.com/google/uuid v1.6.0 // indirect - github.com/gorilla/securecookie v1.1.2 // indirect github.com/gorilla/websocket v1.5.3 // indirect github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect github.com/influxdata/influxdb-client-go/v2 v2.14.0 // indirect diff --git a/go.sum b/go.sum index 2d0719f8..721a6731 100644 --- a/go.sum +++ b/go.sum @@ -27,6 +27,10 @@ github.com/alecthomas/repr v0.4.0 h1:GhI2A8MACjfegCPVq9f1FLvIBS+DrQ2KQBFZP1iFzXc github.com/alecthomas/repr v0.4.0/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4= github.com/alexbrainman/sspi v0.0.0-20250919150558-7d374ff0d59e h1:4dAU9FXIyQktpoUAgOJK3OTFc/xug0PCXYCqU0FgDKI= github.com/alexbrainman/sspi v0.0.0-20250919150558-7d374ff0d59e/go.mod h1:cEWa1LVoE5KvSD9ONXsZrj0z6KqySlCCNKHlLzbqAt4= +github.com/alexedwards/scs/sqlite3store v0.0.0-20251002162104-209de6e426de h1:c72K9HLu6K442et0j3BUL/9HEYaUJouLkkVANdmqTOo= +github.com/alexedwards/scs/sqlite3store v0.0.0-20251002162104-209de6e426de/go.mod h1:Iyk7S76cxGaiEX/mSYmTZzYehp4KfyylcLaV3OnToss= +github.com/alexedwards/scs/v2 v2.9.0 h1:xa05mVpwTBm1iLeTMNFfAWpKUm4fXAW7CeAViqBVS90= +github.com/alexedwards/scs/v2 v2.9.0/go.mod h1:ToaROZxyKukJKT/xLcVQAChi5k6+Pn1Gvmdl7h3RRj8= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883 h1:bvNMNQO63//z+xNgfBlViaCIJKLlCJ6/fmUseuG0wVQ= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/andybalholm/brotli v1.2.1 h1:R+f5xP285VArJDRgowrfb9DqL18yVK0gKAW/F+eTWro= @@ -153,18 +157,12 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= github.com/google/go-tpm v0.9.8/go.mod h1:h9jEsEECg7gtLis0upRBQU+GhYVH6jMjrFxI8u6bVUY= -github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= -github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gops v0.3.29 h1:n98J2qSOK1NJvRjdLDcjgDryjpIBGhbaqph1mXKL0rY= github.com/google/gops v0.3.29/go.mod h1:8N3jZftuPazvUwtYY/ncG4iPrjp15ysNKLfq+QQPiwc= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= -github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA= -github.com/gorilla/securecookie v1.1.2/go.mod h1:NfCASbcHqRSY+3a8tlWJwsQap2VX5pwzwo4h3eOamfo= -github.com/gorilla/sessions v1.4.0 h1:kpIYOp/oi6MG/p5PgxApU8srsSw9tuFbt46Lt7auzqQ= -github.com/gorilla/sessions v1.4.0/go.mod h1:FLWm50oby91+hl7p/wRxDth9bWSuk0qVL2emc7lT5ik= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8= @@ -212,6 +210,7 @@ github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= github.com/mattn/go-sqlite3 v1.10.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc= +github.com/mattn/go-sqlite3 v1.14.6/go.mod h1:NyWgC/yNuGj7Q9rpYnZvas74GogHl5/Z4A/KQRfk6bU= github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= github.com/mattn/go-sqlite3 v1.14.44 h1:3VSe+xafpbzsLbdr2AWlAZk9yRHiBhTBakioXaCKTF8= github.com/mattn/go-sqlite3 v1.14.44/go.mod h1:pjEuOr8IwzLJP2MfGeTb0A35jauH+C2kbHKBr7yXKVQ= diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 26ee456a..a8aef889 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -170,7 +170,6 @@ func setup(t *testing.T) *api.RestAPI { archiver.Start(repository.GetJobRepository(), context.Background()) - t.Setenv("SESSION_KEY", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=") if cfg := ccconf.GetPackageConfig("auth"); cfg != nil { auth.Init(&cfg) } else { diff --git a/internal/api/nats_test.go b/internal/api/nats_test.go index cc0bdf90..b1d2a624 100644 --- a/internal/api/nats_test.go +++ b/internal/api/nats_test.go @@ -156,7 +156,6 @@ func setupNatsTest(t *testing.T) *NatsAPI { archiver.Start(repository.GetJobRepository(), context.Background()) - t.Setenv("SESSION_KEY", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=") if cfg := ccconf.GetPackageConfig("auth"); cfg != nil { auth.Init(&cfg) } else { diff --git a/internal/auth/auth.go b/internal/auth/auth.go index efb72d85..c5b44eae 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -9,16 +9,14 @@ package auth import ( "bytes" "context" - "crypto/rand" "database/sql" - "encoding/base64" + "encoding/gob" "encoding/json" "errors" "fmt" "net" "net/http" "net/url" - "os" "sync" "time" @@ -29,7 +27,8 @@ import ( cclog "github.com/ClusterCockpit/cc-lib/v2/ccLogger" "github.com/ClusterCockpit/cc-lib/v2/schema" "github.com/ClusterCockpit/cc-lib/v2/util" - "github.com/gorilla/sessions" + "github.com/alexedwards/scs/sqlite3store" + "github.com/alexedwards/scs/v2" ) // Authenticator is the interface for all authentication methods. @@ -118,7 +117,7 @@ var Keys AuthConfig // Authentication manages all authentication methods and session handling type Authentication struct { - sessionStore *sessions.CookieStore + sessionManager *scs.SessionManager LdapAuth *LdapAuthenticator JwtAuth *JWTAuthenticator LocalAuth *LocalAuthenticator @@ -126,49 +125,80 @@ type Authentication struct { SessionMaxAge time.Duration } +// SessionManager exposes the scs session manager so the HTTP router can install +// the session middleware (LoadAndSave on write paths, LoadSession on read paths). +func (auth *Authentication) SessionManager() *scs.SessionManager { + return auth.sessionManager +} + +// LoadSession is a non-buffering, read-only session middleware. It loads any +// existing session into the request context so AuthViaSession can read it, but +// (unlike scs.LoadAndSave) it never wraps the ResponseWriter and never commits, +// so large responses (e.g. the GraphQL /query endpoint) stream without being +// buffered in memory. Use it on routes that only read the session to +// authenticate; use scs.LoadAndSave on the login/logout routes that mutate it. +func (auth *Authentication) LoadSession(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + var token string + if c, err := r.Cookie(auth.sessionManager.Cookie.Name); err == nil { + token = c.Value + } + ctx, err := auth.sessionManager.Load(r.Context(), token) + if err != nil { + cclog.Errorf("session load failed: %s", err.Error()) + http.Error(rw, "internal server error", http.StatusInternalServerError) + return + } + next.ServeHTTP(rw, r.WithContext(ctx)) + }) +} + +// expireSessionCookie clears a (corrupted) session cookie on the client. Used on +// read paths where there is no commit to drive the deletion. +func expireSessionCookie(rw http.ResponseWriter) { + http.SetCookie(rw, &http.Cookie{ + Name: "session", + Path: "/", + MaxAge: -1, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + }) +} + func (auth *Authentication) AuthViaSession( rw http.ResponseWriter, r *http.Request, ) (*schema.User, error) { - session, err := auth.sessionStore.Get(r, "session") - if err != nil { - cclog.Error("Error while getting session store") - return nil, err - } - - if session.IsNew { + // The session data was loaded into the request context by the LoadSession + // middleware. No active session cookie => not logged in (mirrors session.IsNew). + ctx := r.Context() + if !auth.sessionManager.Exists(ctx, "username") { return nil, nil } // Validate session data with proper type checking - username, ok := session.Values["username"].(string) - if !ok || username == "" { + username := auth.sessionManager.GetString(ctx, "username") + if username == "" { cclog.Warn("Invalid session: missing or invalid username") - // Invalidate the corrupted session - session.Options.MaxAge = -1 - _ = auth.sessionStore.Save(r, rw, session) + expireSessionCookie(rw) return nil, errors.New("invalid session data") } - projects, ok := session.Values["projects"].([]string) + projects, ok := auth.sessionManager.Get(ctx, "projects").([]string) if !ok { cclog.Warn("Invalid session: projects not found or invalid type, using empty list") projects = []string{} } - roles, ok := session.Values["roles"].([]string) + roles, ok := auth.sessionManager.Get(ctx, "roles").([]string) if !ok || len(roles) == 0 { cclog.Warn("Invalid session: missing or invalid roles") - // Invalidate the corrupted session - session.Options.MaxAge = -1 - _ = auth.sessionStore.Save(r, rw, session) + expireSessionCookie(rw) return nil, errors.New("invalid session data") } - authSourceInt, ok := session.Values["authSource"].(int) - if !ok { - authSourceInt = int(schema.AuthViaLocalPassword) - } + // GetInt returns 0 (== schema.AuthViaLocalPassword) when the key is absent. + authSourceInt := auth.sessionManager.GetInt(ctx, "authSource") return &schema.User{ Username: username, @@ -186,31 +216,30 @@ func Init(authCfg *json.RawMessage) { // Start background cleanup of rate limiters startRateLimiterCleanup() - sessKey := os.Getenv("SESSION_KEY") - if sessKey == "" { - if !config.Keys.DisableAuthentication { - cclog.Fatal("environment variable 'SESSION_KEY' not set: refusing to start with an ephemeral session key. " + - "Set SESSION_KEY in .env (base64-encoded 32 random bytes); a random key would invalidate all sessions on every restart " + - "and prevent sessions from validating across replicas.") - } - // Authentication is disabled: no user sessions are issued, so an - // ephemeral random key is sufficient and SESSION_KEY is not required. - ephemeralKey := make([]byte, 32) - if _, err := rand.Read(ephemeralKey); err != nil { - cclog.Fatalf("Error while initializing authentication -> generating ephemeral session key failed: %v", err) - } - authInstance.sessionStore = sessions.NewCookieStore(ephemeralKey) - } else { - keyBytes, err := base64.StdEncoding.DecodeString(sessKey) - if err != nil { - cclog.Fatal("Error while initializing authentication -> decoding session key failed") - } - authInstance.sessionStore = sessions.NewCookieStore(keyBytes) - } + // Server-side sessions via scs, persisted in the existing SQLite DB so + // sessions survive restarts. Only an opaque random token is stored in the + // cookie, so no secret signing key (the former SESSION_KEY) is required. + gob.Register([]string{}) // user.Projects / user.Roles are stored as []string + sm := scs.New() + sm.Store = sqlite3store.New(repository.GetConnection().DB.DB) + sm.Cookie.Name = "session" + sm.Cookie.Path = "/" + sm.Cookie.HttpOnly = true + sm.Cookie.SameSite = http.SameSiteLaxMode + // scs sets Secure globally (no per-request option). Enable it when this + // process terminates TLS itself. Deployments terminating TLS at a reverse + // proxy can set this via a future config flag if needed. + sm.Cookie.Secure = config.Keys.HTTPSCertFile != "" - if d, err := time.ParseDuration(config.Keys.SessionMaxAge); err == nil { + if d, err := time.ParseDuration(config.Keys.SessionMaxAge); err == nil && d != 0 { + sm.Lifetime = d authInstance.SessionMaxAge = d + } else { + // SessionMaxAge of 0/empty means "do not expire": approximate with a + // long absolute lifetime (the cookie remains persistent). + sm.Lifetime = 10 * 365 * 24 * time.Hour } + authInstance.sessionManager = sm // When authentication is disabled no authenticators are required; the // session store created above is enough for the server to run with a @@ -319,36 +348,22 @@ func handleLdapUser(ldapUser *schema.User) { } func (auth *Authentication) SaveSession(rw http.ResponseWriter, r *http.Request, user *schema.User) error { - session, err := auth.sessionStore.New(r, "session") - if err != nil { - cclog.Errorf("session creation failed: %s", err.Error()) + // The login routes are wrapped by scs.LoadAndSave, which loaded the session + // into the request context and will commit it (persist to the store and write + // the Set-Cookie header) after the handler returns. + ctx := r.Context() + + // Generate a new session token to prevent session fixation. + if err := auth.sessionManager.RenewToken(ctx); err != nil { + cclog.Errorf("session renew failed: %s", err.Error()) http.Error(rw, err.Error(), http.StatusInternalServerError) return err } - if auth.SessionMaxAge != 0 { - session.Options.MaxAge = int(auth.SessionMaxAge.Seconds()) - } - if r.TLS == nil && r.Header.Get("X-Forwarded-Proto") != "https" { - // If neither TLS or an encrypted reverse proxy are used, do not mark cookies as secure. - cclog.Warn("Authenticating with unencrypted request. Session cookies will not have Secure flag set (insecure for production)") - if r.Header.Get("X-Forwarded-Proto") == "" { - // This warning will not be printed if e.g. X-Forwarded-Proto == http - cclog.Warn("If you are using a reverse proxy, make sure X-Forwarded-Proto is set") - } - session.Options.Secure = false - } - session.Options.SameSite = http.SameSiteLaxMode - session.Options.HttpOnly = true - session.Values["username"] = user.Username - session.Values["projects"] = user.Projects - session.Values["roles"] = user.Roles - session.Values["authSource"] = int(user.AuthSource) - if err := auth.sessionStore.Save(r, rw, session); err != nil { - cclog.Warnf("session save failed: %s", err.Error()) - http.Error(rw, err.Error(), http.StatusInternalServerError) - return err - } + auth.sessionManager.Put(ctx, "username", user.Username) + auth.sessionManager.Put(ctx, "projects", user.Projects) + auth.sessionManager.Put(ctx, "roles", user.Roles) + auth.sessionManager.Put(ctx, "authSource", int(user.AuthSource)) return nil } @@ -609,20 +624,13 @@ func (auth *Authentication) AuthFrontendAPI( func (auth *Authentication) Logout(onsuccess http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - session, err := auth.sessionStore.Get(r, "session") - if err != nil { + // The logout route is wrapped by scs.LoadAndSave: Destroy removes the + // session from the store and the middleware clears the cookie on the way out. + if err := auth.sessionManager.Destroy(r.Context()); err != nil { http.Error(rw, err.Error(), http.StatusInternalServerError) return } - if !session.IsNew { - session.Options.MaxAge = -1 - if err := auth.sessionStore.Save(r, rw, session); err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) - return - } - } - onsuccess.ServeHTTP(rw, r) }) } diff --git a/internal/repository/migration.go b/internal/repository/migration.go index 3a8f5e6c..b97db97d 100644 --- a/internal/repository/migration.go +++ b/internal/repository/migration.go @@ -21,11 +21,12 @@ import ( // is added to internal/repository/migrations/sqlite3/. // // Version history: +// - Version 12: Sessions table (server-side sessions via alexedwards/scs) // - Version 11: Optimize job table indexes (reduce from ~78 to 48, add covering/partial indexes) // - Version 10: Node table // // Migration files are embedded at build time from the migrations directory. -const Version uint = 11 +const Version uint = 12 //go:embed migrations/* var migrationFiles embed.FS diff --git a/internal/repository/migrations/sqlite3/12_add-sessions-table.down.sql b/internal/repository/migrations/sqlite3/12_add-sessions-table.down.sql new file mode 100644 index 00000000..63d205dc --- /dev/null +++ b/internal/repository/migrations/sqlite3/12_add-sessions-table.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS sessions; diff --git a/internal/repository/migrations/sqlite3/12_add-sessions-table.up.sql b/internal/repository/migrations/sqlite3/12_add-sessions-table.up.sql new file mode 100644 index 00000000..1d90f012 --- /dev/null +++ b/internal/repository/migrations/sqlite3/12_add-sessions-table.up.sql @@ -0,0 +1,7 @@ +CREATE TABLE sessions ( + token TEXT PRIMARY KEY, + data BLOB NOT NULL, + expiry REAL NOT NULL +); + +CREATE INDEX sessions_expiry_idx ON sessions(expiry); diff --git a/web/frontend/src/generic/utils.js b/web/frontend/src/generic/utils.js index 72a6d333..6819bf56 100644 --- a/web/frontend/src/generic/utils.js +++ b/web/frontend/src/generic/utils.js @@ -4,7 +4,7 @@ import { setContextClient, fetchExchange, } from "@urql/svelte"; -import { setContext, getContext, hasContext, onDestroy, tick } from "svelte"; +import { setContext, getContext, onDestroy, tick } from "svelte"; import { readable } from "svelte/store"; import { round } from "mathjs"; @@ -21,14 +21,11 @@ import { round } from "mathjs"; * - Adds 'getHardwareTopology' to the context, a function that takes a cluster nad subCluster and returns the subCluster topology (or undefined) */ export function init(extraInitQuery = "") { - const jwt = hasContext("jwt") - ? getContext("jwt") - : getContext("cc-config")["jwt"]; - + // The web UI authenticates GraphQL requests via the session cookie + // (same-origin), so no Authorization header is attached here. External + // clients use a JWT against /query directly. const client = new Client({ url: `${window.location.origin}/query`, - fetchOptions: - jwt != null ? { headers: { Authorization: `Bearer ${jwt}` } } : {}, exchanges: [ expiringCacheExchange({ ttl: 5 * 60 * 1000,