feat: add test-login endpoint and integration testing skills#198
Conversation
Add POST /api/v1/auth/test-login gated behind --enable-test-login flag. The endpoint provisions a test user, generates JWTs, and sets a session cookie — enabling Playwright and browser-based integration tests without requiring OAuth. Separate from --dev-auth to keep workstation mode clean.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new /api/v1/auth/test-login endpoint to facilitate integration testing by bypassing OAuth and provisioning test users. It includes command-line flag integration, route registration, and comprehensive unit tests. The review feedback highlights two main areas of improvement: limiting the request body size on the unauthenticated endpoint to prevent potential Denial of Service (DoS) attacks, and addressing issues in the user update logic, specifically handling display name updates correctly and avoiding silent failures by checking the error returned from UpdateUser.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| var req TestLoginRequest | ||
| if err := json.NewDecoder(r.Body).Decode(&req); err != nil { | ||
| http.Error(w, "invalid request body", http.StatusBadRequest) | ||
| return | ||
| } |
There was a problem hiding this comment.
To prevent potential Denial of Service (DoS) attacks via large request payloads on this unauthenticated endpoint, it is recommended to limit the request body size using http.MaxBytesReader.
| var req TestLoginRequest | |
| if err := json.NewDecoder(r.Body).Decode(&req); err != nil { | |
| http.Error(w, "invalid request body", http.StatusBadRequest) | |
| return | |
| } | |
| var req TestLoginRequest | |
| if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)).Decode(&req); err != nil { | |
| http.Error(w, "invalid request body", http.StatusBadRequest) | |
| return | |
| } |
| if req.DisplayName == "" { | ||
| req.DisplayName = req.Email | ||
| } | ||
|
|
||
| ctx := r.Context() | ||
|
|
||
| // Find or create user | ||
| user, err := ws.store.GetUserByEmail(ctx, req.Email) | ||
| if err != nil { | ||
| user = &store.User{ | ||
| ID: generateID(), | ||
| Email: req.Email, | ||
| DisplayName: req.DisplayName, | ||
| Role: req.Role, | ||
| Status: "active", | ||
| Created: time.Now(), | ||
| LastLogin: time.Now(), | ||
| } | ||
| if err := ws.store.CreateUser(ctx, user); err != nil { | ||
| slog.Error("test-login: failed to create user", "email", req.Email, "error", err) | ||
| http.Error(w, "failed to create user", http.StatusInternalServerError) | ||
| return | ||
| } | ||
| } else { | ||
| user.LastLogin = time.Now() | ||
| user.Role = req.Role | ||
| if req.DisplayName != user.Email { | ||
| user.DisplayName = req.DisplayName | ||
| } | ||
| _ = ws.store.UpdateUser(ctx, user) | ||
| } |
There was a problem hiding this comment.
There are two issues in this block:
- If an existing user has a custom display name and the client explicitly requests to update it to their email address, the update is ignored because of the
req.DisplayName != user.Emailcheck. - The error returned by
ws.store.UpdateUseris ignored using the blank identifier_, which can lead to silent failures.
We can resolve both issues by keeping req.DisplayName empty if not provided, defaulting it only during new user creation, and properly logging any errors from UpdateUser.
ctx := r.Context()
// Find or create user
user, err := ws.store.GetUserByEmail(ctx, req.Email)
if err != nil {
displayName := req.DisplayName
if displayName == "" {
displayName = req.Email
}
user = &store.User{
ID: generateID(),
Email: req.Email,
DisplayName: displayName,
Role: req.Role,
Status: "active",
Created: time.Now(),
LastLogin: time.Now(),
}
if err := ws.store.CreateUser(ctx, user); err != nil {
slog.Error("test-login: failed to create user", "email", req.Email, "error", err)
http.Error(w, "failed to create user", http.StatusInternalServerError)
return
}
} else {
user.LastLogin = time.Now()
user.Role = req.Role
if req.DisplayName != "" && req.DisplayName != user.DisplayName {
user.DisplayName = req.DisplayName
}
if err := ws.store.UpdateUser(ctx, user); err != nil {
slog.Warn("test-login: failed to update user on login", "email", req.Email, "error", err)
}
}1 participant
Summary
Closes #197
POST /api/v1/auth/test-loginendpoint gated behind--enable-test-loginflag for browser/Playwright integration testingintegration-testingskill in contrib-repo documenting 4 auth scenarios (hubclient, CLI, curl, Playwright/browser)qa-testertemplate andscion-processlifecycle to reference the new skillChanges
Go (Track 1)
pkg/hub/handlers_test_login.go— new handler: accepts email/role/displayName, provisions user, generates JWT, sets session cookiepkg/hub/web.go—EnableTestLoginconfig field, route registrationpkg/hub/auth.go— add test-login to unauthenticated endpoints listcmd/server.go—--enable-test-loginflagcmd/server_foreground.go— wire flag to WebServerConfigpkg/hub/handlers_test_login_test.go— 8 test cases covering success, defaults, validation, gating, existing usersContrib-repo (Track 2)
skills/integration-testing/SKILL.md— new skill with 4 auth scenario docstemplates/qa-tester/agents.md— reference integration-testing skill, add auth step to workflowskills/scion-process/SKILL.md— integration testing lifecycle guidanceTest plan
go test ./pkg/hub/ -run TestHandleTestLogin)go build ./cmd/scion/)--enable-test-loginand validate browser session flow