Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Vault42 issues its own tokens and is an OAuth2 *client* of other providers. It i
| ![Go](https://img.shields.io/badge/Go-1.26.6-00ADD8?style=flat&logo=go&logoColor=white) | ![Vue](https://img.shields.io/badge/Vue-3.5.41-4FC08D?style=flat&logo=vuedotjs&logoColor=white) | ![.NET](https://img.shields.io/badge/.NET-10.0-512BD4?style=flat&logo=dotnet&logoColor=white) | ![License](https://img.shields.io/badge/License-MIT-155724?style=flat&labelColor=000) |
| ![Go Tests](https://img.shields.io/badge/Tests-4910-155724?style=flat&labelColor=000) | ![Vue Tests](https://img.shields.io/badge/Tests-1305-155724?style=flat&labelColor=000) | ![C# Tests](https://img.shields.io/badge/Tests-264-155724?style=flat&labelColor=000) | ![Total](https://img.shields.io/badge/Total-6479_tests-155724?style=flat&labelColor=000) |
| ![Go Coverage](https://img.shields.io/badge/Coverage-100.00%25_reachable-155724?style=flat&labelColor=000) | ![Vue Coverage](https://img.shields.io/badge/Coverage-99.76%25-155724?style=flat&labelColor=000) | ![C# Coverage](https://img.shields.io/badge/Coverage-100.00%25-155724?style=flat&labelColor=000) | ![Locales](https://img.shields.io/badge/Locales-38-555?style=flat&labelColor=000) |
| ![Go Lines](https://img.shields.io/badge/Lines-48278-555?style=flat&labelColor=000) | ![Vue Lines](https://img.shields.io/badge/Lines-6798-555?style=flat&labelColor=000) | ![C# Lines](https://img.shields.io/badge/Lines-2435-555?style=flat&labelColor=000) | ![Standards](https://img.shields.io/badge/Standards-11-555?style=flat&labelColor=000) |
| ![Go Lines](https://img.shields.io/badge/Lines-48350-555?style=flat&labelColor=000) | ![Vue Lines](https://img.shields.io/badge/Lines-6798-555?style=flat&labelColor=000) | ![C# Lines](https://img.shields.io/badge/Lines-2435-555?style=flat&labelColor=000) | ![Standards](https://img.shields.io/badge/Standards-11-555?style=flat&labelColor=000) |
| ![Go Deps](https://img.shields.io/badge/Deps-3-555?style=flat&labelColor=000) | ![Vue Deps](https://img.shields.io/badge/Deps-3-555?style=flat&labelColor=000) | ![C# Deps](https://img.shields.io/badge/Deps-6-555?style=flat&labelColor=000) | ![Requirements](https://img.shields.io/badge/Requirements-456-555?style=flat&labelColor=000) |
| ![Go Transitive Deps](https://img.shields.io/badge/Transitive-15-555?style=flat&labelColor=000) | ![Vue Transitive Deps](https://img.shields.io/badge/Transitive-95-555?style=flat&labelColor=000) | ![C# Transitive Deps](https://img.shields.io/badge/Transitive-26-555?style=flat&labelColor=000) | ![Total Deps](https://img.shields.io/badge/Deps-148_total-555?style=flat&labelColor=000) |
<!-- /badges -->
Expand Down
8 changes: 4 additions & 4 deletions docs/badges.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
"reachableCoverage": "100.00%",
"reachableCoverageNum": 100.00,
"packages": 43,
"goFiles": 192,
"goLines": 48278,
"testFiles": 932,
"goFiles": 193,
"goLines": 48350,
"testFiles": 934,
"directDeps": 3,
"transitiveDeps": 15,
"totalTests": 6479,
Expand All @@ -20,7 +20,7 @@
"tests": 4910,
"coverage": "100.00% of reachable",
"coverageNum": 100.00,
"lines": 48278,
"lines": 48350,
"deps": 3,
"transitiveDeps": 15
},
Expand Down
4 changes: 2 additions & 2 deletions docs/compliance-register.json
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@
"accepted_by": "Project maintainer (vault42)",
"accepted_on": "2026-08-18",
"security_md": "Register-scoped. docs/security.md owns the AR-nn namespace and defines a different risk under this number; the two lists are deliberately disjoint.",
"rationale": "internal/server/server.go:546 mounts GET /auth/verify-email, and internal/handler/auth.go:298 mutates state on it by marking the address verified. ASVS V3.5.3 requires either a non-safe method or strict Sec-Fetch-* validation.\n\nThis entry previously said no Sec-Fetch-* header is read anywhere in the tree, and that was false: cmd/bridge/proxy.go:373 reads all three. docs/COMPLIANCE.md recorded the correction and it never propagated back here, which is how a register sentence outlives the fact it rests on. The accurate statement is narrower and still decides the row: no route served by the vault itself validates fetch metadata, and adding it here would not help, because the rule only has teeth if it rejects requests that carry no fetch metadata at all -- which is every non-browser client, including the curl in docs/api.md and both shipped SDKs.\n\nThe method flip is the honest remedy and is a breaking change to a published API: @vault42/vue ships the GET in its built output, and a pinned older SDK would start receiving 405. It is timed with an SDK major rather than dropped into a patch release.",
"rationale": "internal/server/server.go:559 mounts GET /auth/verify-email, and internal/handler/auth.go:298 mutates state on it by marking the address verified. ASVS V3.5.3 requires either a non-safe method or strict Sec-Fetch-* validation.\n\nThis entry previously said no Sec-Fetch-* header is read anywhere in the tree, and that was false: cmd/bridge/proxy.go:373 reads all three. docs/COMPLIANCE.md recorded the correction and it never propagated back here, which is how a register sentence outlives the fact it rests on. The accurate statement is narrower and still decides the row: no route served by the vault itself validates fetch metadata, and adding it here would not help, because the rule only has teeth if it rejects requests that carry no fetch metadata at all -- which is every non-browser client, including the curl in docs/api.md and both shipped SDKs.\n\nThe method flip is the honest remedy and is a breaking change to a published API: @vault42/vue ships the GET in its built output, and a pinned older SDK would start receiving 405. It is timed with an SDK major rather than dropped into a patch release.",
"compensating_control": "The token is single-use and consumed atomically by a get-and-delete -- Redis GETDEL, Postgres DELETE ... RETURNING, or the in-memory equivalent -- so a replayed or prefetched link verifies at most once, proven across replicas by the multi-replica end-to-end suite. The token is 256 bits from crypto/rand, stored hashed, and expires in 24 hours. The effect is bounded to marking an address the operator already sent mail to as verified: it confers no session and issues no token. The route is rate limited to 10 per hour per source address, and Referrer-Policy: no-referrer is set globally.",
"residual_risk": "A security scanner that executes JavaScript performs the verification without the user acting. The precision matters and the earlier wording did not have it: the link in the email is the SPA route /verify-email, not the API route, so a plain prefetcher or link scanner that follows the URL receives index.html and changes nothing. The mutation happens only when the page's script runs and calls the API itself, which a headless renderer of the SafeLinks class does. Anything fetching /auth/verify-email directly also mutates, and that URL is published as a copyable curl in docs/api.md.\n\nThe consequence is not cosmetic. email_verified gates password login and is one half of the test deciding whether a federated identity may attach to an existing local account, so the realistic harm is an attacker registering with somebody else's address and that person's mail scanner completing the verification. A user whose token a scanner consumed also sees \"link expired\" while being, in fact, verified. Nothing else is reachable through the route.",
"revisit_when": "Any additional state change is added behind a GET route, or the verification link starts conferring a session rather than only marking an address verified.",
Expand All @@ -190,7 +190,7 @@
"accepted_by": "Project maintainer (vault42)",
"accepted_on": "2026-08-18",
"security_md": "Register-scoped. docs/security.md owns the AR-nn namespace and defines a different risk under this number; the two lists are deliberately disjoint.",
"rationale": "ASVS V3.5.4 wants separate applications on separate hostnames so the same-origin policy separates them. When VAULT_SERVE_FRONTEND is on, internal/server/server.go:847 mounts the embedded SPA at the catch-all route of the same server that answers /auth, /user and /client, so the two share an origin by construction and the policy separates nothing.",
"rationale": "ASVS V3.5.4 wants separate applications on separate hostnames so the same-origin policy separates them. When VAULT_SERVE_FRONTEND is on, internal/server/server.go:861 mounts the embedded SPA at the catch-all route of the same server that answers /auth, /user and /client, so the two share an origin by construction and the policy separates nothing.",
"compensating_control": "VAULT_SERVE_FRONTEND is off by default (internal/config/config.go:505) and the chart ships serveFrontend: false, so the single-origin topology is opt-in; the honeypot profile is the one place that forces it on. The Content-Security-Policy applied when the frontend is served names script-src 'self' with no unsafe-inline, so even in the shared-origin shape a script the SPA did not author does not execute.\n\nThe chart offers the separated topology rather than shipping it, and the earlier wording did not make that distinction: charts/vault/templates/frontend.yaml deploys the SPA as its own nginx workload, but frontend.enabled is false by default and the template carries no hostname of its own. The separate hostname comes from ingress.hosts, which the operator supplies. So the preferred topology is available and configured, not default.",
"residual_risk": "An operator who enables the embedded frontend gives up the origin boundary between the SPA and the API, so an XSS in the SPA runs on the API's origin.",
"revisit_when": "The embedded frontend becomes the default, or the SPA gains a surface that renders content it did not author.",
Expand Down
59 changes: 59 additions & 0 deletions internal/middleware/liveaccount.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package middleware

import (
"net/http"

"github.com/42-v/vault42/internal/httputil"
"github.com/42-v/vault42/internal/repository"
)

// RequireLiveAccount refuses a request whose subject has been erased.
//
// An access token outlives the erasure that invalidated it. Auth validates the
// signature, issuer, audience and token type and never reads the database, which
// is the whole point of a self-contained token and is a trade docs/security.md
// makes deliberately: revocation is bounded by the access-token lifetime, not
// immediate. AR-5 states that for roles.
//
// Erasure is the case that trade does not cover. A stale role grants no more
// than it did when the token was minted; a write reaches into storage the
// erasure has already scrubbed and puts personal data back. The subject asked
// for it to be gone, the operator answered that it was, and neither is true
// afterwards. So this is not a general account-state check bolted onto every
// route -- it is applied to the handful of routes that persist subject-owned
// data, and nowhere else.
//
// Which routes: the ones that write and cannot be guarded in SQL.
// PUT /user/profile is not among them, because auth.users can state the rule
// itself and does -- UserRepo.Update carries AND deleted = FALSE. identity
// profiles cannot: identity.profiles is keyed by an unlinkable pseudonym with no
// user_id and no foreign key, which is what makes it pseudonymous and also what
// makes the database unable to see the connection. Blobs and the MFA setup
// routes are the same shape. For those, the only thing that can know is a
// lookup, so a lookup is what this does.
//
// The cost is one indexed primary-key read on a write path that is already
// doing storage work. Read routes are untouched, and the stateless fast path
// that self-contained tokens exist to provide is unchanged.
//
// Failure is closed. A lookup error is answered 401 rather than passed through:
// this guard exists because the alternative is writing personal data back onto
// an erased subject, and a database that cannot answer is not permission to do
// that.
func RequireLiveAccount(users repository.UserRepository) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
claims := GetClaims(r.Context())
if claims == nil {
httputil.WriteError(w, http.StatusUnauthorized, "unauthorized")
return
}
user, err := users.GetByID(r.Context(), claims.Subject)
if err != nil || user == nil || user.Deleted {
httputil.WriteError(w, http.StatusUnauthorized, "unauthorized")
return
}
next.ServeHTTP(w, r)
})
}
}
125 changes: 125 additions & 0 deletions internal/middleware/liveaccount_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package middleware

import (
"context"
"errors"
"net/http"
"net/http/httptest"
"testing"

vaultcrypto "github.com/42-v/vault42/internal/crypto"
vjwt "github.com/42-v/vault42/internal/jwt"
"github.com/42-v/vault42/internal/model"
"github.com/42-v/vault42/tests/mocks"
)

// The routes behind this guard write personal data that an erasure has already
// scrubbed, into stores the database cannot check for itself.
//
// identity.profiles is keyed by an unlinkable pseudonym -- no user_id, no
// foreign key -- which is exactly what makes it pseudonymous and also what makes
// SQL unable to see that the subject is gone. PUT /user/identity therefore
// recreated a name, a date of birth, a billing address and a VAT id on a
// tombstoned subject, and the handler never resolved the user at all, so nothing
// in the request path knew.
//
// PUT /user/profile is deliberately not behind this: auth.users states the rule
// in its own UPDATE. A guard that duplicates one the database already enforces
// costs a round trip and hides where the real invariant lives.

func liveAccountRequest(t *testing.T, subject string) *http.Request {
t.Helper()
req := httptest.NewRequest(http.MethodPut, "/user/identity", nil)
ctx := context.WithValue(req.Context(), ClaimsKey, &vaultcrypto.VaultClaims{RegisteredClaims: vjwt.RegisteredClaims{Subject: subject}})
return req.WithContext(ctx)
}

func runLiveAccount(t *testing.T, users *mocks.MockUserRepo, req *http.Request) (int, bool) {
t.Helper()
var reached bool
h := RequireLiveAccount(users)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
reached = true
w.WriteHeader(http.StatusOK)
}))
rec := httptest.NewRecorder()
h.ServeHTTP(rec, req)
return rec.Code, reached
}

func TestRequireLiveAccount_RefusesAnErasedSubject(t *testing.T) {
users := &mocks.MockUserRepo{
GetByIDFn: func(_ context.Context, id string) (*model.User, error) {
// What SoftDeleteScrub leaves: the row survives so foreign keys stay
// valid, with the personal columns cleared and the tombstone set.
return &model.User{ID: id, Email: "deleted-" + id + "@deleted.invalid", Deleted: true}, nil
},
}
code, reached := runLiveAccount(t, users, liveAccountRequest(t, "erased-user"))
if code != http.StatusUnauthorized {
t.Errorf("status = %d, want 401", code)
}
if reached {
t.Error("the handler ran for an erased subject. The status code is not the point " +
"on its own: what matters is that the write never happens.")
}
}

func TestRequireLiveAccount_AllowsALiveSubject(t *testing.T) {
users := &mocks.MockUserRepo{
GetByIDFn: func(_ context.Context, id string) (*model.User, error) {
return &model.User{ID: id, Email: "alice@example.com"}, nil
},
}
code, reached := runLiveAccount(t, users, liveAccountRequest(t, "user-1"))
if code != http.StatusOK || !reached {
t.Fatalf("a live subject was refused: status = %d, reached = %v", code, reached)
}
}

// Fail closed. A database that cannot answer is not permission to write personal
// data back onto a subject that may have been erased.
func TestRequireLiveAccount_FailsClosed(t *testing.T) {
cases := []struct {
name string
repo *mocks.MockUserRepo
}{
{"lookup error", &mocks.MockUserRepo{
GetByIDFn: func(_ context.Context, _ string) (*model.User, error) {
return nil, errors.New("connection refused")
},
}},
{"no such user", &mocks.MockUserRepo{
GetByIDFn: func(_ context.Context, _ string) (*model.User, error) {
return nil, nil
},
}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
code, reached := runLiveAccount(t, tc.repo, liveAccountRequest(t, "user-1"))
if code != http.StatusUnauthorized || reached {
t.Fatalf("status = %d, reached = %v; want 401 and no handler", code, reached)
}
})
}
}

// No claims means the guard is mounted somewhere Auth is not, which is a wiring
// mistake. It must refuse rather than dereference nil.
func TestRequireLiveAccount_RefusesWithoutClaims(t *testing.T) {
var looked bool
users := &mocks.MockUserRepo{
GetByIDFn: func(_ context.Context, _ string) (*model.User, error) {
looked = true
return &model.User{}, nil
},
}
req := httptest.NewRequest(http.MethodPut, "/user/identity", nil)
code, reached := runLiveAccount(t, users, req)
if code != http.StatusUnauthorized || reached {
t.Fatalf("status = %d, reached = %v; want 401 and no handler", code, reached)
}
if looked {
t.Error("looked a subject up with no claims to name one")
}
}
Loading
Loading