Add CORS credentials, transactional endpoints, and container deployment

CORS
  cors.go never set Access-Control-Allow-Credentials, so the
  cookie-authenticated API was unreadable from any cross-origin frontend:
  the server answered correctly and the browser blocked the page from
  reading it. Set for allowlisted origins on both the preflight and the
  actual response. Three tests added.

  HTTP_COOKIE_SAMESITE (lax|none|strict, default lax) is new. CORS is only
  half of what a cross-origin browser call needs; SameSite is judged on
  registrable domain, so a frontend on an unrelated domain gets perfect CORS
  headers and still no cookie. "none" is the only value that survives that,
  and validate() refuses it without the Secure flag.

  The "*" rejection now explains itself: browsers refuse Allow-Origin "*"
  together with credentials, so it would break every authenticated call
  rather than loosen anything.

Transactional endpoints (api-contract.md 12.1)
  POST /api/v1/job-applications/{id}/hire
  POST /api/v1/job-postings/{id}/assignments

  Replaces two client-side loops that wrote several records with no
  transaction and no rollback. Each is now one endpoint and one transaction,
  built over repo.Repo so org scoping, derived columns, type casts and error
  translation are not re-derived. Authorization reuses the existing policy
  table rather than adding a parallel one: a workflow is exactly as
  privileged as the writes it performs. 13 tests, including both rollback
  paths.

Bug fix in the repository layer
  repo.bindValue handled int64/int/float64/string but not int32, which is
  what pgx returns for a PostgreSQL `int` column. Nothing previously read a
  record and wrote one of its fields elsewhere, so it never surfaced; the
  hire flow does exactly that and failed with "ai_score must be a number".
  Both KindInt and KindFloat now accept the widths pgx actually produces.

Deployment
  infrastructure/Dockerfile.api  multi-stage, cross-compiling (BUILDPLATFORM
    + GOARCH) so linux/amd64 builds from arm64 are compiled rather than
    emulated. Alpine runtime, non-root uid 10001, 22.1 MB. Ships api, seed,
    setpassword and migrate, plus the migrations, so a Kubernetes
    initContainer can apply the schema from the same image and tag as the
    API. HEALTHCHECK keys on status code, not body, so a "degraded" instance
    is not pulled from rotation during a migration window.

  infrastructure/docker-compose.yml  migrations run to completion before the
    API starts. Assumes a managed PostgreSQL; the local-db overlay adds one
    with TLS enabled so APP_ENV=production is met rather than dodged.

  scripts/drop_public_tables.go  the one-off used to clear an unrelated
    schema from krowdb on 2026-08-24, kept for the record. Build-tagged
    ignore and gated on CONFIRM_DROP=yes.

Verified against PostgreSQL: 16/16 new tests pass, and the image was built,
run and exercised end to end (login, CORS preflight, authenticated reads,
transaction rollback).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CmQiGq73Uyfq7J4yR8Vxxw
This commit is contained in:
Suriyakumarvijayanayagam
2026-08-25 11:33:01 +05:30
parent 7d12ebef3d
commit 954ba9076f
17 changed files with 1868 additions and 9 deletions

View File

@@ -47,6 +47,27 @@ type HTTPConfig struct {
IdleTimeout time.Duration
ShutdownTimeout time.Duration
// CookieSameSite is the SameSite attribute on the session cookie:
// "lax" (default), "none" or "strict".
//
// This exists because CORS is only half of what a cross-origin browser call
// needs, and the other half is easy to miss. SameSite is judged on SITE
// (registrable domain), not origin:
//
// app.krow.com → api.krow.com SAME site. Lax sends the cookie. ✓
// krow.vercel.app → api.krow.com CROSS site. Lax does NOT send it. ✗
// localhost:5173 → 127.0.0.1:8080 CROSS site — different hosts. ✗
//
// So a deployment whose frontend is on an unrelated domain gets a perfect
// set of CORS headers and still no session, because the browser never
// attaches the cookie. "none" is the only value that survives that, and it
// requires Secure, which means HTTPS.
//
// Default stays "lax": it is the safe value, it is correct for the
// same-site and same-origin deployments this is normally run as, and it
// gives CSRF protection that "none" gives up.
CookieSameSite string
// CORSOrigins is the exact set of browser origins allowed to call the API.
//
// It exists for one reason: in local development the Vite dev server is an
@@ -137,6 +158,7 @@ func Load() (*Config, error) {
IdleTimeout: durationDefault("HTTP_IDLE_TIMEOUT", 60*time.Second),
ShutdownTimeout: durationDefault("HTTP_SHUTDOWN_TIMEOUT", 10*time.Second),
CORSOrigins: corsOrigins(withDefault("APP_ENV", "development")),
CookieSameSite: strings.ToLower(withDefault("HTTP_COOKIE_SAMESITE", "lax")),
},
Seed: SeedConfig{
FixturePath: withDefault("SEED_FIXTURE_PATH", "./seed/fixtures/seed.json"),
@@ -194,12 +216,38 @@ func (c *Config) validate() error {
if c.AppEnv == "production" && c.DB.SSLMode == "disable" {
return fmt.Errorf("DATABASE_SSLMODE=disable is not allowed when APP_ENV=production")
}
switch c.HTTP.CookieSameSite {
case "lax", "strict":
case "none":
// SameSite=None without Secure is ignored — and in current browsers,
// rejected outright — so the cookie would simply never be stored. The
// Secure flag is set for every APP_ENV except development, so this is
// the one combination that produces a silently sessionless deployment.
if c.AppEnv == "development" {
return fmt.Errorf("HTTP_COOKIE_SAMESITE=none requires the Secure cookie flag, " +
"which is not set when APP_ENV=development; SameSite=None over plain HTTP " +
"is rejected by browsers")
}
default:
return fmt.Errorf("HTTP_COOKIE_SAMESITE must be lax, none or strict, got %q",
c.HTTP.CookieSameSite)
}
for _, origin := range c.HTTP.CORSOrigins {
// "*" is rejected rather than quietly honoured. The middleware echoes a
// single matched origin, so a wildcard could only ever be a
// misunderstanding of what this setting does.
// "*" is not a stricter-than-necessary policy choice — it cannot work
// here at all. Authentication is a cookie, so the API must answer
// Access-Control-Allow-Credentials: true, and every browser REFUSES
// the pairing of that header with Allow-Origin: "*". A deployment
// configured this way would send correct-looking headers and have
// every authenticated call blocked client-side.
if origin == "*" {
return fmt.Errorf("HTTP_CORS_ORIGINS must list explicit origins; \"*\" is not accepted")
return fmt.Errorf(`HTTP_CORS_ORIGINS must list explicit origins; "*" cannot be used ` +
`because this API authenticates with a cookie, and browsers reject ` +
`Access-Control-Allow-Origin: "*" together with credentials. ` +
`List each frontend origin, or serve the frontend from the API's own origin ` +
`(then leave this unset and CORS is not involved at all)`)
}
if !strings.HasPrefix(origin, "http://") && !strings.HasPrefix(origin, "https://") {
return fmt.Errorf("HTTP_CORS_ORIGINS entry %q must be a full origin including the scheme", origin)

View File

@@ -42,6 +42,29 @@ const sessionCookieName = "krow_session"
// that never authenticate anything.
func (s *Server) secureCookies() bool { return s.cfg.AppEnv != "development" }
// sameSite resolves the configured SameSite mode.
//
// Lax remains the default and the recommendation. "none" exists for the one
// deployment shape that cannot work without it: a frontend on a different
// registrable domain from the API. In that case Lax withholds the cookie on
// every cross-site fetch, so the sign-in succeeds, the Set-Cookie arrives, and
// the next request carries nothing — which reads as a broken session rather
// than as a cookie policy.
//
// An unrecognised value falls back to Lax rather than to None. config.validate
// rejects those before startup, so this is only a belt-and-braces default in
// the safe direction.
func (s *Server) sameSite() http.SameSite {
switch s.cfg.HTTP.CookieSameSite {
case "none":
return http.SameSiteNoneMode
case "strict":
return http.SameSiteStrictMode
default:
return http.SameSiteLaxMode
}
}
// setSessionCookie writes the raw token to the browser.
//
// This is the only place the raw token is written to a response, and it goes
@@ -59,12 +82,14 @@ func (s *Server) setSessionCookie(w http.ResponseWriter, token string, lifetime
Path: "/",
// HttpOnly: script cannot read it.
HttpOnly: true,
// Lax, not Strict and not None. Strict would drop the cookie on any
// cross-site navigation, so following a link into the app would land on
// a login page despite a live session. None would require Secure and
// would send the cookie on cross-site POSTs, which is the CSRF hole Lax
// exists to close.
SameSite: http.SameSiteLaxMode,
// Lax by default, and Strict/None available through
// HTTP_COOKIE_SAMESITE. Strict would drop the cookie on any cross-site
// navigation, so following a link into the app would land on a login
// page despite a live session. None sends it on cross-site requests,
// which is the CSRF hole Lax exists to close — and is nonetheless the
// only workable value when the frontend is on a different registrable
// domain. See Server.sameSite.
SameSite: s.sameSite(),
Secure: s.secureCookies(),
MaxAge: int(lifetime.Seconds()),
})
@@ -82,7 +107,9 @@ func (s *Server) clearSessionCookie(w http.ResponseWriter) {
Value: "",
Path: "/",
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
// Must match the attributes it was set with, SameSite included, or the
// browser treats this as a different cookie and leaves the original.
SameSite: s.sameSite(),
Secure: s.secureCookies(),
MaxAge: -1,
})

View File

@@ -73,6 +73,19 @@ func cors(origins []string) func(http.Handler) http.Handler {
w.Header().Set("Access-Control-Allow-Origin", origin)
// Authentication is a cookie, so the browser will neither send it
// nor expose the response without this. It is set for allowlisted
// origins only, and the origin above is always a specific one —
// the pairing of Allow-Credentials with "*" is rejected outright by
// browsers, which is the second reason this middleware never echoes
// a wildcard.
//
// Both the preflight and the actual response need it: the preflight
// decides whether the browser is willing to SEND the cookie, and the
// actual response decides whether the page may READ the result.
// Setting it here, before the preflight branch, covers both.
w.Header().Set("Access-Control-Allow-Credentials", "true")
if isPreflight(r) {
w.Header().Add("Vary", "Access-Control-Request-Method")
w.Header().Add("Vary", "Access-Control-Request-Headers")

View File

@@ -133,6 +133,62 @@ func TestCORSOffByDefault(t *testing.T) {
}
}
// Authentication is a cookie, so a cross-origin frontend calling with
// `credentials: 'include'` needs Access-Control-Allow-Credentials on the actual
// response. Without it the browser blocks the page from reading a reply the
// server answered perfectly well, and the app sees an opaque network failure
// beside a 200 in the server log.
func TestCORSAllowsCredentialsOnResponse(t *testing.T) {
handler := corsAPI(t, devOrigin)
rec := send(handler, "GET", "/api/v1/job-postings", map[string]string{"Origin": devOrigin})
if rec.Code != http.StatusOK {
t.Fatalf("expected 200, got %d", rec.Code)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" {
t.Fatalf("Access-Control-Allow-Credentials = %q, want \"true\" — "+
"a cookie-authenticated API is unreadable cross-origin without it", got)
}
// Allow-Credentials with a wildcard origin is rejected by every browser, so
// the two must never appear together.
if got := rec.Header().Get("Access-Control-Allow-Origin"); got == "*" {
t.Fatal("Access-Control-Allow-Origin is \"*\" alongside credentials; browsers refuse that pairing")
}
}
// The preflight decides whether the browser is willing to SEND the cookie at
// all, so it needs the header too — separately from the actual response.
func TestCORSAllowsCredentialsOnPreflight(t *testing.T) {
handler := corsAPI(t, devOrigin)
rec := send(handler, "OPTIONS", "/api/v1/job-postings", map[string]string{
"Origin": devOrigin,
"Access-Control-Request-Method": "POST",
})
if rec.Code != http.StatusNoContent {
t.Fatalf("expected 204, got %d", rec.Code)
}
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" {
t.Fatalf("preflight Access-Control-Allow-Credentials = %q, want \"true\"", got)
}
}
// An origin that is not on the allowlist must not be handed credentials
// permission — the header is worthless on its own, but pairing it with a
// reflected origin would be the classic misconfiguration.
func TestCORSWithholdsCredentialsFromUnknownOrigin(t *testing.T) {
handler := corsAPI(t, devOrigin)
rec := send(handler, "GET", "/api/v1/job-postings",
map[string]string{"Origin": "http://evil.example"})
if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" {
t.Fatalf("an unlisted origin was granted credentials: %q", got)
}
if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" {
t.Fatalf("an unlisted origin was echoed back: %q", got)
}
}
func contains(haystack, needle string) bool {
for i := 0; i+len(needle) <= len(haystack); i++ {
if haystack[i:i+len(needle)] == needle {

View File

@@ -37,6 +37,7 @@ type Server struct {
db *db.DB
api *service.Registry
definitions *service.DefinitionsService
workflows *service.WorkflowService
log *slog.Logger
http *http.Server
started time.Time
@@ -124,6 +125,7 @@ func New(cfg *config.Config, database *db.DB, log *slog.Logger, opts ...Option)
cfg: cfg, db: database, log: log,
api: service.NewRegistry(database.Pool),
definitions: service.NewDefinitions(database.Pool),
workflows: service.NewWorkflows(database.Pool).WithClock(o.now),
started: o.now(),
sessions: sessions,
users: users,
@@ -135,7 +137,8 @@ func New(cfg *config.Config, database *db.DB, log *slog.Logger, opts ...Option)
mux := http.NewServeMux()
mux.HandleFunc("GET /health", s.handleHealth)
s.endpoints = s.routeAuth(mux) + s.routeResources(mux) + s.routeMe(mux) + s.routeDefinitions(mux)
s.endpoints = s.routeAuth(mux) + s.routeResources(mux) + s.routeMe(mux) +
s.routeDefinitions(mux) + s.routeWorkflows(mux)
handler := jsonErrors(mux)
// Authentication sits where devOrgMiddleware used to, so every route below

View File

@@ -0,0 +1,153 @@
package httpserver
import (
"net/http"
"github.com/krow/krow-backend/go-api/internal/authctx"
"github.com/krow/krow-backend/go-api/internal/domain"
"github.com/krow/krow-backend/go-api/internal/service"
)
// The multi-record endpoints from api-contract.md §12.1.
//
// These are the first routes that are not a plain CRUD projection of a table,
// and they are shaped as verbs on the record they act on — `.../{id}/hire`,
// `.../{id}/assignments` — rather than as new collections. The action is the
// thing being requested, and it has no independent existence to GET.
//
// AUTHORIZATION REUSES THE POLICY TABLE RATHER THAN ADDING TO IT.
//
// A workflow is exactly as privileged as the writes it performs, so each one
// is gated on the operations it will actually carry out — hire needs UPDATE on
// job-applications and CREATE on staff; assign needs CREATE on assignments and
// UPDATE on job-applications. Inventing a separate `hire` permission would
// create a second place where the answer to "who may do this" lives, and the
// two would eventually disagree. Every pair below resolves to `operators`
// today, which is the intended answer: a talent user cannot hire themselves or
// place themselves on a shift.
// requirement is one (resource, operation) pair a workflow depends on.
type requirement struct {
path string
op domain.Op
}
// authorizeAll refuses unless the caller may perform every listed operation.
//
// All-or-nothing, checked before any transaction opens: a caller who may update
// an application but not create staff must not get halfway through a hire and
// be rolled back. The refusal is the same 403 a single-operation handler gives,
// and names no resource — see domain.Forbidden.
func (s *Server) authorizeAll(w http.ResponseWriter, r *http.Request,
reqs ...requirement) (authctx.Identity, bool) {
ident, err := authctx.MustFrom(r.Context())
if err != nil {
// Unreachable: the middleware refuses an unauthenticated request before
// the router sees it. A missing identity here is a wiring bug.
writeError(w, s.log, domain.Internal(err))
return authctx.Identity{}, false
}
role, known := domain.ParseRole(ident.Role)
if !known {
s.log.Warn("workflow refused: unknown role",
"user_id", ident.UserID, "role", ident.Role, "path", r.URL.Path)
writeError(w, s.log, domain.Forbidden())
return authctx.Identity{}, false
}
for _, req := range reqs {
svc, ok := s.api.Get(req.path)
if !ok {
writeError(w, s.log, domain.Internal(
errUnregisteredResource(req.path)))
return authctx.Identity{}, false
}
if !svc.Resource().Policy.Allows(req.op, role) {
s.log.Warn("workflow authorization refused",
"user_id", ident.UserID, "role", ident.Role,
"required_resource", req.path, "path", r.URL.Path)
writeError(w, s.log, domain.Forbidden())
return authctx.Identity{}, false
}
}
return ident, true
}
type unregisteredResourceError string
func (e unregisteredResourceError) Error() string {
return "httpserver: workflow depends on unregistered resource " + string(e)
}
func errUnregisteredResource(path string) error { return unregisteredResourceError(path) }
func (s *Server) routeWorkflows(mux *http.ServeMux) int {
mux.HandleFunc("POST /api/v1/job-applications/{id}/hire", s.handleHire)
mux.HandleFunc("POST /api/v1/job-postings/{id}/assignments", s.handleAssign)
return 2
}
// handleHire moves an application to `hired` and creates the staff record in
// one transaction. Replaces the two-call sequence at krowHooks.js:302-303.
func (s *Server) handleHire(w http.ResponseWriter, r *http.Request) {
ident, ok := s.authorizeAll(w, r,
requirement{"job-applications", domain.OpUpdate},
requirement{"staff", domain.OpCreate},
requirement{"user-activity", domain.OpCreate},
)
if !ok {
return
}
body, err := decodeBody(r)
if err != nil {
writeError(w, s.log, err)
return
}
result, err := s.workflows.Hire(r.Context(), ident, r.PathValue("id"), body)
if err != nil {
writeError(w, s.log, err)
return
}
s.log.Info("candidate hired", "user_id", ident.UserID,
"application_id", r.PathValue("id"), "staff_id", result.Staff["id"])
// 201: the request created a staff record. The application it also updated
// is returned alongside so the caller can render the new state without a
// second read.
writeJSON(w, http.StatusCreated, envelope{Data: result})
}
// handleAssign places workers on a posting in one transaction. Replaces the 3n
// sequential round-trips at krowHooks.js:421/449/466.
func (s *Server) handleAssign(w http.ResponseWriter, r *http.Request) {
ident, ok := s.authorizeAll(w, r,
requirement{"assignments", domain.OpCreate},
requirement{"job-applications", domain.OpUpdate},
requirement{"user-activity", domain.OpCreate},
)
if !ok {
return
}
var req service.AssignRequest
if err := decodeInto(r, &req); err != nil {
writeError(w, s.log, err)
return
}
result, err := s.workflows.Assign(r.Context(), ident, r.PathValue("id"), req)
if err != nil {
writeError(w, s.log, err)
return
}
s.log.Info("workers assigned", "user_id", ident.UserID,
"job_posting_id", r.PathValue("id"), "count", result.Count)
writeJSON(w, http.StatusCreated, envelope{Data: result})
}

View File

@@ -0,0 +1,331 @@
package httpserver_test
import (
"context"
"net/http"
"testing"
)
// The multi-record endpoints from api-contract.md §12.1.
//
// The property worth testing here is not that the happy path works — it is that
// a failure part-way through leaves NOTHING behind. Every rollback test below
// counts rows before and after, because "the request returned an error" and
// "the request changed nothing" are different claims and only the second one is
// what a transaction is for.
// applicationFor creates an application that can be hired.
func applicationFor(t *testing.T, r *rbac, posting, name, email string) string {
t.Helper()
return mustCreate(t, r, r.admin, "/api/v1/job-applications", map[string]any{
"job_posting_id": posting,
"applicant_name": name,
"email": email,
"status": "shortlisted",
"ai_score": 77,
})
}
func countRows(t *testing.T, r *rbac, table string) int {
t.Helper()
var n int
if err := r.h.Pool.QueryRow(context.Background(),
`SELECT count(*) FROM `+table+` WHERE org_id = $1::uuid`, r.orgID).Scan(&n); err != nil {
t.Fatalf("count %s: %v", table, err)
}
return n
}
/* ── Hire ───────────────────────────────────────────────────────────────── */
func TestHireCreatesStaffAndMovesApplication(t *testing.T) {
r := newRBAC(t)
app := applicationFor(t, r, r.activePosting, "Hire Me", "hire-me@example.test")
before := countRows(t, r, "staff")
got := r.as(r.admin, "POST", "/api/v1/job-applications/"+app+"/hire", map[string]any{
"role": "Event Server", "profile_tier": "Skilled",
})
if got.code != http.StatusCreated {
t.Fatalf("hire: got %d, want 201 (%v)", got.code, got.body)
}
data, _ := got.body["data"].(map[string]any)
application, _ := data["application"].(map[string]any)
staff, _ := data["staff"].(map[string]any)
if application == nil || staff == nil {
t.Fatalf("hire response is missing application or staff: %v", got.body)
}
if application["status"] != "hired" {
t.Errorf("application.status = %v, want hired", application["status"])
}
if staff["name"] != "Hire Me" {
t.Errorf("staff.name = %v, want the applicant's name", staff["name"])
}
if staff["email"] != "hire-me@example.test" {
t.Errorf("staff.email = %v, want the application's email", staff["email"])
}
// The caller's overrides win over the derived defaults.
if staff["role"] != "Event Server" {
t.Errorf("staff.role = %v, want the supplied role", staff["role"])
}
if staff["profile_tier"] != "Skilled" {
t.Errorf("staff.profile_tier = %v, want the supplied tier", staff["profile_tier"])
}
// ai_score is carried across from the application. It is an `int` column,
// so it arrives from pgx as int32 — the case repo.bindValue did not handle
// until this endpoint existed to read a record and write it elsewhere.
if score, ok := staff["ai_score"].(float64); !ok || int(score) != 77 {
t.Errorf("staff.ai_score = %v, want 77 carried from the application", staff["ai_score"])
}
if staff["application_id"] != app {
t.Errorf("staff.application_id = %v, want %s", staff["application_id"], app)
}
if after := countRows(t, r, "staff"); after != before+1 {
t.Errorf("staff rows: %d -> %d, want exactly one more", before, after)
}
}
// Hiring the same application twice would create a second employment record for
// one person, so the second attempt is a conflict rather than a repeat.
func TestHireIsNotRepeatable(t *testing.T) {
r := newRBAC(t)
app := applicationFor(t, r, r.activePosting, "Twice", "twice@example.test")
if got := r.as(r.admin, "POST", "/api/v1/job-applications/"+app+"/hire", map[string]any{}); got.code != http.StatusCreated {
t.Fatalf("first hire: got %d, want 201 (%v)", got.code, got.body)
}
before := countRows(t, r, "staff")
got := r.as(r.admin, "POST", "/api/v1/job-applications/"+app+"/hire", map[string]any{})
if got.code != http.StatusConflict {
t.Fatalf("second hire: got %d, want 409 (%v)", got.code, got.body)
}
if after := countRows(t, r, "staff"); after != before {
t.Errorf("a refused hire still wrote a staff row: %d -> %d", before, after)
}
}
// The whole point of the endpoint: the two writes succeed together or not at
// all. A staff insert that violates a constraint must leave the application
// untouched, not merely report an error.
func TestHireRollsBackTheApplicationWhenStaffFails(t *testing.T) {
r := newRBAC(t)
app := applicationFor(t, r, r.activePosting, "Rollback", "rollback@example.test")
staffBefore := countRows(t, r, "staff")
// profile_tier is a native enum; a value outside it fails the staff INSERT
// after the application UPDATE has already been issued in this transaction.
got := r.as(r.admin, "POST", "/api/v1/job-applications/"+app+"/hire", map[string]any{
"profile_tier": "NotARealTier",
})
if got.code == http.StatusCreated {
t.Fatalf("an invalid profile_tier was accepted: %v", got.body)
}
if after := countRows(t, r, "staff"); after != staffBefore {
t.Errorf("staff rows changed despite a failed hire: %d -> %d", staffBefore, after)
}
// The decisive assertion: the application must NOT be hired.
reread := r.as(r.admin, "GET", "/api/v1/job-applications?limit=500", nil)
if reread.code != http.StatusOK {
t.Fatalf("re-read applications: %d", reread.code)
}
for _, raw := range reread.body["data"].([]any) {
rec := raw.(map[string]any)
if rec["id"] == app && rec["status"] == "hired" {
t.Fatal("the application was left hired after the staff insert failed — " +
"the two writes are not in one transaction")
}
}
}
// Hiring is an operator action. A talent user must not be able to hire anyone,
// including themselves.
func TestHireIsRefusedToTalent(t *testing.T) {
r := newRBAC(t)
app := applicationFor(t, r, r.activePosting, "Self", r.talA.email)
before := countRows(t, r, "staff")
got := r.as(r.talA, "POST", "/api/v1/job-applications/"+app+"/hire", map[string]any{})
if got.code != http.StatusForbidden {
t.Fatalf("talent hire: got %d, want 403 (%v)", got.code, got.body)
}
if after := countRows(t, r, "staff"); after != before {
t.Errorf("a refused hire still wrote a staff row: %d -> %d", before, after)
}
}
func TestHireRejectsUnknownApplication(t *testing.T) {
r := newRBAC(t)
got := r.as(r.admin, "POST",
"/api/v1/job-applications/00000000-0000-0000-0000-000000000000/hire", map[string]any{})
if got.code != http.StatusNotFound {
t.Fatalf("hire unknown application: got %d, want 404 (%v)", got.code, got.body)
}
}
// Another organization's application is absent, not forbidden — the same 404 a
// nonexistent id gets, so existence does not leak across tenants.
func TestHireCannotReachAnotherOrganization(t *testing.T) {
r := newRBAC(t)
app := applicationFor(t, r, r.activePosting, "Ours", "ours@example.test")
got := r.as(r.outsider, "POST", "/api/v1/job-applications/"+app+"/hire", map[string]any{})
if got.code != http.StatusNotFound {
t.Fatalf("cross-tenant hire: got %d, want 404 (%v)", got.code, got.body)
}
}
/* ── Assign ─────────────────────────────────────────────────────────────── */
func TestAssignPlacesWorkersAndUpdatesApplications(t *testing.T) {
r := newRBAC(t)
a1 := applicationFor(t, r, r.activePosting, "Worker One", "w1@example.test")
a2 := applicationFor(t, r, r.activePosting, "Worker Two", "w2@example.test")
before := countRows(t, r, "assignments")
got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", map[string]any{
"workers": []map[string]any{
{"worker_email": "w1@example.test", "worker_name": "Worker One",
"starts_at": "2026-09-01T09:00:00Z", "application_id": a1, "match_score": 91},
{"worker_email": "w2@example.test", "worker_name": "Worker Two",
"starts_at": "2026-09-01T09:00:00Z", "application_id": a2},
},
})
if got.code != http.StatusCreated {
t.Fatalf("assign: got %d, want 201 (%v)", got.code, got.body)
}
data, _ := got.body["data"].(map[string]any)
if count, ok := data["count"].(float64); !ok || int(count) != 2 {
t.Errorf("count = %v, want 2", data["count"])
}
if after := countRows(t, r, "assignments"); after != before+2 {
t.Errorf("assignment rows: %d -> %d, want two more", before, after)
}
// Both applications must now read as assigned.
list := r.as(r.admin, "GET", "/api/v1/job-applications?status=assigned", nil)
assigned := map[string]bool{}
for _, raw := range list.body["data"].([]any) {
assigned[raw.(map[string]any)["id"].(string)] = true
}
if !assigned[a1] || !assigned[a2] {
t.Errorf("applications were not moved to assigned: a1=%v a2=%v", assigned[a1], assigned[a2])
}
}
// A worker with no application is legitimate — that is what the talent pool is
// for — and must not be invented one.
func TestAssignAcceptsWorkerWithoutApplication(t *testing.T) {
r := newRBAC(t)
got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", map[string]any{
"workers": []map[string]any{
{"worker_email": "pool@example.test", "worker_name": "Pool Worker",
"starts_at": "2026-09-01T09:00:00Z"},
},
})
if got.code != http.StatusCreated {
t.Fatalf("assign without application: got %d, want 201 (%v)", got.code, got.body)
}
}
// The batch is all-or-nothing. A bad reference on the SECOND worker must undo
// the first worker's assignment, not leave it stranded.
func TestAssignRollsBackTheWholeBatch(t *testing.T) {
r := newRBAC(t)
before := countRows(t, r, "assignments")
got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", map[string]any{
"workers": []map[string]any{
{"worker_email": "first@example.test", "worker_name": "First",
"starts_at": "2026-09-01T09:00:00Z"},
{"worker_email": "second@example.test", "worker_name": "Second",
"starts_at": "2026-09-01T09:00:00Z",
"application_id": "00000000-0000-0000-0000-000000000000"},
},
})
if got.code == http.StatusCreated {
t.Fatalf("a batch naming a nonexistent application was accepted: %v", got.body)
}
if after := countRows(t, r, "assignments"); after != before {
t.Fatalf("the first worker survived the second's failure: %d -> %d — "+
"the batch is not one transaction", before, after)
}
}
func TestAssignIsRefusedToTalent(t *testing.T) {
r := newRBAC(t)
before := countRows(t, r, "assignments")
got := r.as(r.talA, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", map[string]any{
"workers": []map[string]any{
{"worker_email": r.talA.email, "worker_name": "Self",
"starts_at": "2026-09-01T09:00:00Z"},
},
})
if got.code != http.StatusForbidden {
t.Fatalf("talent assign: got %d, want 403 (%v)", got.code, got.body)
}
if after := countRows(t, r, "assignments"); after != before {
t.Errorf("a refused assign still wrote a row: %d -> %d", before, after)
}
}
func TestAssignValidatesTheBatchBeforeWriting(t *testing.T) {
r := newRBAC(t)
before := countRows(t, r, "assignments")
cases := []struct {
name string
body map[string]any
}{
{"no workers", map[string]any{"workers": []map[string]any{}}},
{"missing email", map[string]any{"workers": []map[string]any{
{"worker_name": "No Email", "starts_at": "2026-09-01T09:00:00Z"}}}},
{"missing starts_at", map[string]any{"workers": []map[string]any{
{"worker_email": "x@example.test", "worker_name": "No Start"}}}},
{"malformed application_id", map[string]any{"workers": []map[string]any{
{"worker_email": "x@example.test", "starts_at": "2026-09-01T09:00:00Z",
"application_id": "not-a-uuid"}}}},
}
for _, tc := range cases {
got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", tc.body)
if got.code != http.StatusUnprocessableEntity {
t.Errorf("%s: got %d, want 422 (%v)", tc.name, got.code, got.body)
}
}
if after := countRows(t, r, "assignments"); after != before {
t.Errorf("a rejected batch wrote rows: %d -> %d", before, after)
}
}
func TestAssignRejectsUnknownPosting(t *testing.T) {
r := newRBAC(t)
got := r.as(r.admin, "POST",
"/api/v1/job-postings/00000000-0000-0000-0000-000000000000/assignments", map[string]any{
"workers": []map[string]any{
{"worker_email": "x@example.test", "starts_at": "2026-09-01T09:00:00Z"},
},
})
if got.code != http.StatusNotFound {
t.Fatalf("assign to unknown posting: got %d, want 404 (%v)", got.code, got.body)
}
}
// Both endpoints are behind the session like everything else.
func TestWorkflowEndpointsRequireASession(t *testing.T) {
r := newRBAC(t)
for _, path := range []string{
"/api/v1/job-applications/00000000-0000-0000-0000-000000000000/hire",
"/api/v1/job-postings/00000000-0000-0000-0000-000000000000/assignments",
} {
if got := r.doAnon("POST", path, map[string]any{}); got.code != http.StatusUnauthorized {
t.Errorf("%s unauthenticated: got %d, want 401", path, got.code)
}
}
}

View File

@@ -572,6 +572,13 @@ func bindValue(c domain.Column, v any) (any, error) {
return raw, nil
case domain.KindInt:
// The int8/16/32 cases are not JSON shapes — encoding/json only ever
// produces float64. They are the widths pgx hands back when a value was
// READ from the database and is being written somewhere else: an `int`
// column arrives as int32, and a service that copies a field from one
// record to another (see service.Hire, which carries ai_score from an
// application onto the staff row) would otherwise be told its own
// database's value "must be a number".
switch t := v.(type) {
case float64:
if t != float64(int64(t)) {
@@ -592,19 +599,45 @@ func bindValue(c domain.Column, v any) (any, error) {
return t, nil
case int:
return int64(t), nil
case int32:
return int64(t), nil
case int16:
return int64(t), nil
case int8:
return int64(t), nil
case float32:
if t != float32(int64(t)) {
return nil, domain.Validation(
fmt.Sprintf("%s must be a whole number", c.Name),
map[string]string{c.Name: "expected an integer"})
}
return int64(t), nil
}
return nil, domain.Validation(fmt.Sprintf("%s must be a number", c.Name), nil)
case domain.KindFloat:
// float32 and the integer widths for the same reason as above: a
// numeric column is projected as float8 and returns float64, but an
// integer read from elsewhere may legitimately be written into one.
switch t := v.(type) {
case float64:
return t, nil
case float32:
return float64(t), nil
case string:
f, err := strconv.ParseFloat(t, 64)
if err != nil {
return nil, domain.Validation(fmt.Sprintf("%s must be a number", c.Name), nil)
}
return f, nil
case int64:
return float64(t), nil
case int:
return float64(t), nil
case int32:
return float64(t), nil
case int16:
return float64(t), nil
}
return nil, domain.Validation(fmt.Sprintf("%s must be a number", c.Name), nil)

View File

@@ -0,0 +1,443 @@
package service
// Multi-record writes, in one transaction each.
//
// api-contract.md §12.1 lists four flows that the frontend performs as a
// sequence of independent HTTP calls, with no transaction and no rollback:
// hiring a candidate, assigning workers to a posting, screening a whole list,
// and submitting a challenge. A failure halfway through leaves the database
// inconsistent — an application marked hired with no staff row, or an
// assignment with an application still showing as merely shortlisted.
//
// This file collapses the first two into one endpoint and one transaction
// each, which is what §12.1 says Phase 3 should do. The other two are not here:
// `useScreenAllCandidates` is n independent PATCHes that are individually
// meaningful (a partial screen is not a corrupt state), and `useSubmitChallenge`
// writes evidence the talent user owns, which needs the talent-scoped predicate
// and is a different shape of problem.
//
// WHY THE REPOSITORY IS REUSED RATHER THAN HAND-WRITTEN SQL
//
// Every write below goes through repo.Repo, built over the transaction rather
// than the pool. That is deliberate: the repository is where org_id is forced
// from the session, where server-derived columns are filled, where values are
// bound and cast to their declared types, and where a constraint violation is
// translated into the contract's error codes. Writing raw SQL here would mean
// re-deriving all four, and getting one of them subtly wrong.
import (
"context"
"errors"
"fmt"
"time"
"github.com/jackc/pgx/v5"
"github.com/krow/krow-backend/go-api/internal/authctx"
"github.com/krow/krow-backend/go-api/internal/domain"
"github.com/krow/krow-backend/go-api/internal/repo"
)
// maxAssignmentBatch bounds one assign call.
//
// `useAssignWorkers` assigns a selection from the talent pool, which is a
// human-sized list. The cap exists so a single request cannot hold a
// transaction open across thousands of inserts, blocking every other write to
// these tables for the duration.
const maxAssignmentBatch = 200
// TxBeginner is the part of the pool this file needs. An interface rather than
// *pgxpool.Pool so the service stays testable and consistent with repo.Querier.
type TxBeginner interface {
Begin(ctx context.Context) (pgx.Tx, error)
}
// WorkflowService owns the multi-record flows.
type WorkflowService struct {
db TxBeginner
// now is injectable so tests can assert on generated dates without
// depending on the day they run.
now func() time.Time
}
// NewWorkflows builds the workflow service over a pool.
func NewWorkflows(db TxBeginner) *WorkflowService {
return &WorkflowService{db: db, now: time.Now}
}
// WithClock replaces the clock. For tests.
func (s *WorkflowService) WithClock(now func() time.Time) *WorkflowService {
if now != nil {
s.now = now
}
return s
}
// inTx runs fn inside a transaction, rolling back on any error.
//
// The rollback is deferred rather than called on each error path: an early
// return, a panic in a callee, and an explicit failure all have to undo the
// work, and only a deferred rollback covers the second. Rolling back an
// already-committed transaction is a no-op in pgx, so the deferred call is safe
// on the success path too.
func (s *WorkflowService) inTx(ctx context.Context, fn func(tx pgx.Tx) error) error {
tx, err := s.db.Begin(ctx)
if err != nil {
return fmt.Errorf("begin transaction: %w", err)
}
defer func() { _ = tx.Rollback(ctx) }()
if err := fn(tx); err != nil {
return err
}
return tx.Commit(ctx)
}
func resourceByPath(path string) (*domain.Resource, error) {
res, ok := domain.ResourceByPath[path]
if !ok {
return nil, domain.Internal(fmt.Errorf("service: resource %q is not registered", path))
}
return res, nil
}
/* ── Hire ───────────────────────────────────────────────────────────────── */
// HireResult is what a completed hire returns: both records the flow touched,
// so the caller does not need a follow-up read to render the outcome.
type HireResult struct {
Application domain.Record `json:"application"`
Staff domain.Record `json:"staff"`
}
// Hire moves an application to `hired` and creates the staff record, atomically.
//
// Replaces the two-call sequence at krowHooks.js:302-303. The failure that
// motivated it: the PATCH succeeds, the POST fails, and the candidate is now
// hired with no employment record and no way for the UI to notice.
//
// Fields the caller may supply are the ones a hiring form collects — hire_date,
// role, phone, profile_tier, status, reviewer_name. Everything else is carried
// across from the application, because it is already the truth about this
// person and retyping it is how the two records drift apart.
func (s *WorkflowService) Hire(ctx context.Context, ident authctx.Identity,
applicationID string, body domain.Record) (*HireResult, error) {
if !isUUID(applicationID) {
return nil, domain.NotFound("JobApplication", applicationID)
}
apps, err := resourceByPath("job-applications")
if err != nil {
return nil, err
}
staffRes, err := resourceByPath("staff")
if err != nil {
return nil, err
}
activity, err := resourceByPath("user-activity")
if err != nil {
return nil, err
}
var out HireResult
err = s.inTx(ctx, func(tx pgx.Tx) error {
appRepo := repo.New(apps, tx)
// Read inside the transaction. Reading outside it would leave a window
// where two concurrent hires both see a not-yet-hired application.
app, err := appRepo.Get(ctx, ident, applicationID)
if err != nil {
return err
}
if app == nil {
return domain.NotFound("JobApplication", applicationID)
}
// Hiring twice is a conflict, not an idempotent repeat: the second call
// would create a second employment record for one application. 409
// rather than 200 because the caller asked for something that cannot be
// done, and silently returning the first hire would hide a double
// submission rather than report it.
if status, _ := app["status"].(string); status == "hired" {
return domain.Conflict("this application has already been hired")
}
email, _ := app["email"].(string)
name, _ := app["applicant_name"].(string)
if email == "" || name == "" {
return domain.Validation(
"this application cannot be hired: it has no applicant name or email",
map[string]string{"application": "incomplete"})
}
staffRecord := domain.Record{
"application_id": applicationID,
"job_posting_id": app["job_posting_id"],
"worker_profile_id": app["worker_profile_id"],
"name": name,
"email": email,
"phone": pick(body, "phone", app["phone"]),
"role": pick(body, "role", app["job_title"]),
"hire_date": pick(body, "hire_date", s.now().UTC().Format("2006-01-02")),
"ai_score": app["ai_score"],
"profile_tier": pick(body, "profile_tier", "Beginner"),
"status": pick(body, "status", "onboarding"),
"client_rating": app["client_rating"],
"reviewer_name": pick(body, "reviewer_name", ""),
}
// A null worker_profile_id is legitimate — not every applicant has a
// profile — but the column list must not carry an explicit nil for a
// NOT NULL column, so drop the keys the application had nothing for.
dropNil(staffRecord, "worker_profile_id", "job_posting_id")
hired, err := appRepo.Update(ctx, ident, applicationID, domain.Record{"status": "hired"})
if err != nil {
return err
}
if hired == nil {
// Unreachable: Get above found it under the same predicate.
return domain.NotFound("JobApplication", applicationID)
}
created, err := repo.New(staffRes, tx).Insert(ctx, ident, staffRecord)
if err != nil {
return err
}
// The audit entry is part of the transaction on purpose: a hire that
// happened without a log entry, or a log entry for a hire that rolled
// back, are both worse than neither.
if _, err := repo.New(activity, tx).Insert(ctx, ident, domain.Record{
"event_type": "candidate_hired",
"details": fmt.Sprintf("%s was hired", name),
"application_id": applicationID,
"position_id": app["job_posting_id"],
"worker_email": email,
}); err != nil {
return err
}
out.Application, out.Staff = hired, created
return nil
})
if err != nil {
return nil, err
}
return &out, nil
}
/* ── Assign ─────────────────────────────────────────────────────────────── */
// AssignWorker is one worker in an assign request.
type AssignWorker struct {
WorkerEmail string `json:"worker_email"`
WorkerName string `json:"worker_name"`
StartsAt string `json:"starts_at"`
EndsAt *string `json:"ends_at"`
ApplicationID *string `json:"application_id"`
WorkerProfileID *string `json:"worker_profile_id"`
MatchScore *int `json:"match_score"`
Source string `json:"source"`
}
// AssignRequest is the body of POST /job-postings/{id}/assignments.
type AssignRequest struct {
Workers []AssignWorker `json:"workers"`
}
// AssignResult reports what the batch created.
type AssignResult struct {
Assignments []domain.Record `json:"assignments"`
Count int `json:"count"`
}
// Assign places workers on a posting, atomically.
//
// Replaces the 3n sequential round-trips at krowHooks.js:421/449/466 with one
// request and one transaction. All-or-nothing across the whole batch: assigning
// six workers and having the fourth fail should not leave three assigned, three
// not, and the caller unsure which.
func (s *WorkflowService) Assign(ctx context.Context, ident authctx.Identity,
postingID string, req AssignRequest) (*AssignResult, error) {
if !isUUID(postingID) {
return nil, domain.NotFound("JobPosting", postingID)
}
if len(req.Workers) == 0 {
return nil, domain.Validation("at least one worker is required",
map[string]string{"workers": "must not be empty"})
}
if len(req.Workers) > maxAssignmentBatch {
return nil, domain.Validation(
fmt.Sprintf("at most %d workers can be assigned in one request", maxAssignmentBatch),
map[string]string{"workers": "too many"})
}
// Validate the whole batch before opening a transaction. A malformed
// request should never have caused a BEGIN.
details := map[string]string{}
for i, w := range req.Workers {
if w.WorkerEmail == "" {
details[fmt.Sprintf("workers[%d].worker_email", i)] = "required"
}
if w.StartsAt == "" {
details[fmt.Sprintf("workers[%d].starts_at", i)] = "required"
}
if w.ApplicationID != nil && !isUUID(*w.ApplicationID) {
details[fmt.Sprintf("workers[%d].application_id", i)] = "must be a uuid"
}
if w.WorkerProfileID != nil && !isUUID(*w.WorkerProfileID) {
details[fmt.Sprintf("workers[%d].worker_profile_id", i)] = "must be a uuid"
}
}
if len(details) > 0 {
return nil, domain.Validation("assignment payload is not valid", details)
}
postings, err := resourceByPath("job-postings")
if err != nil {
return nil, err
}
assignments, err := resourceByPath("assignments")
if err != nil {
return nil, err
}
apps, err := resourceByPath("job-applications")
if err != nil {
return nil, err
}
activity, err := resourceByPath("user-activity")
if err != nil {
return nil, err
}
out := &AssignResult{Assignments: []domain.Record{}}
err = s.inTx(ctx, func(tx pgx.Tx) error {
posting, err := repo.New(postings, tx).Get(ctx, ident, postingID)
if err != nil {
return err
}
if posting == nil {
return domain.NotFound("JobPosting", postingID)
}
assignRepo := repo.New(assignments, tx)
appRepo := repo.New(apps, tx)
activityRepo := repo.New(activity, tx)
for i, w := range req.Workers {
record := domain.Record{
"job_posting_id": postingID,
"worker_email": w.WorkerEmail,
"worker_name": w.WorkerName,
"starts_at": w.StartsAt,
"status": "active",
"source": orDefault(w.Source, "manual"),
}
if w.EndsAt != nil {
record["ends_at"] = *w.EndsAt
}
if w.ApplicationID != nil {
record["application_id"] = *w.ApplicationID
}
if w.WorkerProfileID != nil {
record["worker_profile_id"] = *w.WorkerProfileID
}
if w.MatchScore != nil {
record["match_score"] = *w.MatchScore
}
created, err := assignRepo.Insert(ctx, ident, record)
if err != nil {
return annotate(err, i)
}
out.Assignments = append(out.Assignments, created)
// The application moves to `assigned` only when one was named. A
// worker can be placed without having applied — that is what the
// talent pool is for — and inventing an application for them would
// be worse than leaving the link absent.
if w.ApplicationID != nil {
updated, err := appRepo.Update(ctx, ident, *w.ApplicationID,
domain.Record{"status": "assigned"})
if err != nil {
return annotate(err, i)
}
if updated == nil {
return domain.NotFound("JobApplication", *w.ApplicationID)
}
}
entry := domain.Record{
"event_type": "worker_assigned",
"details": fmt.Sprintf("%s was assigned", orDefault(w.WorkerName, w.WorkerEmail)),
"position_id": postingID,
"worker_email": w.WorkerEmail,
}
if w.ApplicationID != nil {
entry["application_id"] = *w.ApplicationID
}
if _, err := activityRepo.Insert(ctx, ident, entry); err != nil {
return annotate(err, i)
}
}
return nil
})
if err != nil {
return nil, err
}
out.Count = len(out.Assignments)
return out, nil
}
/* ── Helpers ────────────────────────────────────────────────────────────── */
// pick takes the caller's value for a key when they supplied a usable one, and
// the fallback otherwise. A present-but-null key means "use the fallback"
// rather than "write null", because these columns are NOT NULL.
func pick(body domain.Record, key string, fallback any) any {
if body == nil {
return fallback
}
v, ok := body[key]
if !ok || v == nil {
return fallback
}
if s, isStr := v.(string); isStr && s == "" {
return fallback
}
return v
}
// dropNil removes keys whose value is nil, so a NOT NULL column is left to its
// default instead of being sent an explicit null.
func dropNil(rec domain.Record, keys ...string) {
for _, k := range keys {
if v, ok := rec[k]; ok && v == nil {
delete(rec, k)
}
}
}
func orDefault(v, fallback string) string {
if v == "" {
return fallback
}
return v
}
// annotate points a validation error at the batch element that produced it.
// Without this, "worker_email must not be null" on a batch of forty says
// nothing about which one.
func annotate(err error, index int) error {
var apiErr *domain.Error
if !errors.As(err, &apiErr) || apiErr.Details == nil {
return err
}
moved := make(map[string]string, len(apiErr.Details))
for k, v := range apiErr.Details {
moved[fmt.Sprintf("workers[%d].%s", index, k)] = v
}
return domain.Validation(apiErr.Message, moved)
}