Repository navigation
security(auth): an MFA_REQUIRED token from a password-only login is accepted on every protected route — mint a PAT or turn MFA off without the second factor #944
Description
Problem
A password alone is enough to get past MFA. After a correct password, login returns an MFA_REQUIRED token meant only for POST /auth/mfa/challenge. That token is accepted by every route behind Protected. With a stolen or phished password, an attacker can mint a permanent personal access token with the owner's full permissions, or turn MFA off. The second factor is never asked for.
Cause (confirmed in the code on master 6999958)
backend/internal/middleware/auth.goAuthMiddlewareRS256(behindProtected,protect.go) validates the signature, expiry and blacklist, then setsuser_id,tenant_id, roles and permissions from the claims. It never readsclaims.Type.backend/pkg/auth/jwt.goValidateAccessTokendoesn't check the type either.pkg/authdefines three types:TokenTypeAccess = "",TokenTypeMFARequired,TokenTypeMFAEnrollment. OnlyMFATokenMiddlewareandMFAEnrollmentMiddlewarecheck them.cmd/server/main.go:995protected := api.Use(Protected);/auth/mfa/disable(1139) and/auth/pat(1170) sit behind it.
Reproduced live (devsecops review of #714, branch binary on throwaway Postgres + Redis)
- Root admin with a verified TOTP logs in with the password only and gets
{mfa_required: true, mfa_token}. - With
mfa_tokenas the bearer:GET /auth/me,/users/meand/auth/sessionsreturn 200. Permission-gated routes return 403, because the token carries no permissions. POST /auth/pat {scopes:["*"]}withmfa_tokenreturns 201. The PAT then reads/risks,/assetsand/organization/memberswith 200, using the owner's permissions. This works for root, who cannot turn MFA off.POST /auth/mfa/disable {password}withmfa_tokenreturns 200. The next login is password-only with a full session. Tested withMFA_REQUIRED_ROLESempty; with the defaults, this applies to every non-privileged member.- A unit probe shows the
MFA_ENROLLMENTtoken (15 min) also passesProtected. OnlyMFAPolicyGuardcan still stop it, and only once the grace period is over.
Constraint for the fix
api.Use(Protected) at main.go:995 also runs before /auth/mfa/setup and /auth/mfa/verify, which are registered later on api (1136-1137). Those two routes must keep accepting the MFA_ENROLLMENT token. A blanket refusal of non-access types in Protected is right, but those routes then have to be registered before Protected, as /auth/mfa/challenge already is (912). Otherwise the enrolment of a role that requires MFA breaks.
Acceptance criteria
- Given an
MFA_REQUIREDtoken, every route behindProtectedanswers 401TOKEN_INVALID. This includes/auth/me,/auth/pat(create, list, revoke),/auth/mfa/disableand/auth/sessions. - Given an
MFA_ENROLLMENTtoken, every route behindProtectedanswers 401, exceptPOST /auth/mfa/setupandPOST /auth/mfa/verify, which keep working. A role that requires MFA can still enrol from the sign-in screen. - Given a full access token, cookie or bearer, and given a PAT, behaviour is unchanged.
POST /auth/mfa/challengestill accepts onlyMFA_REQUIRED.- A ratchet test sends each token type to
Protected,/auth/pat,/auth/mfa/disable,/auth/mfa/setupand/auth/mfa/verify, and fails if a new route behindProtectedaccepts a typed token. - A PAT minted before the fix by an
MFA_REQUIREDtoken cannot be identified by type afterwards. The PR says how operators can find and revoke such tokens, for example PATs created within minutes of anmfa_requiredlogin with no successful challenge, or says why that isn't possible.
Definition of Done
-
TestProtected_Success(access token),TestProtected_NotFound(unknown or revoked jti),TestProtected_Unauthorized(both typed tokens), plus the route ratchet of criterion 5 -
go test ./...with output pasted; route/authz/isolation registry tests green - Live: password-only login →
mfa_token→/auth/pat401,/auth/mfa/disable401; enrolment of a role that requires MFA still succeeds end to end - Security review (devsecops) of the fix
- Claim matrix: check any MFA claim; MFA could be bypassed until this ships
Found by the devsecops review of #714 (PR #898). Not caused by #714.
Activity