Merge SEC-01: require password for TOTP enrollment

This commit is contained in:
2026-03-13 01:07:39 -07:00
13 changed files with 192 additions and 17 deletions

View File

@@ -401,9 +401,15 @@ func (c *Client) RenewToken() (token, expiresAt string, err error) {
// Returns a base32 secret and an otpauth:// URI for QR-code generation. // Returns a base32 secret and an otpauth:// URI for QR-code generation.
// The secret is shown once; it is not retrievable after this call. // The secret is shown once; it is not retrievable after this call.
// TOTP is not enforced until confirmed via ConfirmTOTP. // TOTP is not enforced until confirmed via ConfirmTOTP.
func (c *Client) EnrollTOTP() (*TOTPEnrollResponse, error) { //
// Security (SEC-01): the current password is required to prevent a stolen
// session token from being used to enroll attacker-controlled TOTP.
func (c *Client) EnrollTOTP(password string) (*TOTPEnrollResponse, error) {
var resp TOTPEnrollResponse var resp TOTPEnrollResponse
if err := c.do(http.MethodPost, "/v1/auth/totp/enroll", nil, &resp); err != nil { body := struct {
Password string `json:"password"`
}{Password: password}
if err := c.do(http.MethodPost, "/v1/auth/totp/enroll", body, &resp); err != nil {
return nil, err return nil, err
} }
return &resp, nil return &resp, nil

View File

@@ -275,7 +275,7 @@ func TestEnrollTOTP(t *testing.T) {
})) }))
defer srv.Close() defer srv.Close()
c := newTestClient(t, srv.URL) c := newTestClient(t, srv.URL)
resp, err := c.EnrollTOTP() resp, err := c.EnrollTOTP("testpass123")
if err != nil { if err != nil {
t.Fatalf("EnrollTOTP: %v", err) t.Fatalf("EnrollTOTP: %v", err)
} }

View File

@@ -148,11 +148,15 @@ class Client:
expires_at = str(data["expires_at"]) expires_at = str(data["expires_at"])
self.token = token self.token = token
return token, expires_at return token, expires_at
def enroll_totp(self) -> tuple[str, str]: def enroll_totp(self, password: str) -> tuple[str, str]:
"""POST /v1/auth/totp/enroll — begin TOTP enrollment. """POST /v1/auth/totp/enroll — begin TOTP enrollment.
Security (SEC-01): current password is required to prevent session-theft
escalation to persistent account takeover.
Returns (secret, otpauth_uri). The secret is shown only once. Returns (secret, otpauth_uri). The secret is shown only once.
""" """
data = self._request("POST", "/v1/auth/totp/enroll") data = self._request("POST", "/v1/auth/totp/enroll", json={"password": password})
assert data is not None assert data is not None
return str(data["secret"]), str(data["otpauth_uri"]) return str(data["secret"]), str(data["otpauth_uri"])
def confirm_totp(self, code: str) -> None: def confirm_totp(self, code: str) -> None:

View File

@@ -191,7 +191,7 @@ def test_enroll_totp(admin_client: Client) -> None:
json={"secret": "JBSWY3DPEHPK3PXP", "otpauth_uri": "otpauth://totp/MCIAS:alice?secret=JBSWY3DPEHPK3PXP&issuer=MCIAS"}, json={"secret": "JBSWY3DPEHPK3PXP", "otpauth_uri": "otpauth://totp/MCIAS:alice?secret=JBSWY3DPEHPK3PXP&issuer=MCIAS"},
) )
) )
secret, uri = admin_client.enroll_totp() secret, uri = admin_client.enroll_totp("testpass123")
assert secret == "JBSWY3DPEHPK3PXP" assert secret == "JBSWY3DPEHPK3PXP"
assert "otpauth://totp/" in uri assert "otpauth://totp/" in uri
@respx.mock @respx.mock

View File

@@ -484,9 +484,12 @@ impl Client {
/// Begin TOTP enrollment. Returns `(secret, otpauth_uri)`. /// Begin TOTP enrollment. Returns `(secret, otpauth_uri)`.
/// The secret is shown once; store it in an authenticator app immediately. /// The secret is shown once; store it in an authenticator app immediately.
pub async fn enroll_totp(&self) -> Result<(String, String), MciasError> { ///
/// Security (SEC-01): current password is required to prevent session-theft
/// escalation to persistent account takeover.
pub async fn enroll_totp(&self, password: &str) -> Result<(String, String), MciasError> {
let resp: TotpEnrollResponse = let resp: TotpEnrollResponse =
self.post("/v1/auth/totp/enroll", &serde_json::json!({})).await?; self.post("/v1/auth/totp/enroll", &serde_json::json!({"password": password})).await?;
Ok((resp.secret, resp.otpauth_uri)) Ok((resp.secret, resp.otpauth_uri))
} }

View File

@@ -449,7 +449,7 @@ async fn test_enroll_totp() {
.await; .await;
let c = admin_client(&server).await; let c = admin_client(&server).await;
let (secret, uri) = c.enroll_totp().await.unwrap(); let (secret, uri) = c.enroll_totp("testpass123").await.unwrap();
assert_eq!(secret, "JBSWY3DPEHPK3PXP"); assert_eq!(secret, "JBSWY3DPEHPK3PXP");
assert!(uri.starts_with("otpauth://totp/")); assert!(uri.starts_with("otpauth://totp/"));
} }

View File

@@ -304,9 +304,12 @@ func (x *RenewTokenResponse) GetExpiresAt() *timestamppb.Timestamp {
return nil return nil
} }
// EnrollTOTPRequest carries no body; the acting account is from the JWT. // EnrollTOTPRequest carries the current password for re-authentication.
// Security (SEC-01): password is required to prevent a stolen session token
// from being used to enroll attacker-controlled TOTP on the victim's account.
type EnrollTOTPRequest struct { type EnrollTOTPRequest struct {
state protoimpl.MessageState `protogen:"open.v1"` state protoimpl.MessageState `protogen:"open.v1"`
Password string `protobuf:"bytes,1,opt,name=password,proto3" json:"password,omitempty"` // security: current password required; never logged
unknownFields protoimpl.UnknownFields unknownFields protoimpl.UnknownFields
sizeCache protoimpl.SizeCache sizeCache protoimpl.SizeCache
} }
@@ -341,6 +344,13 @@ func (*EnrollTOTPRequest) Descriptor() ([]byte, []int) {
return file_mcias_v1_auth_proto_rawDescGZIP(), []int{6} return file_mcias_v1_auth_proto_rawDescGZIP(), []int{6}
} }
func (x *EnrollTOTPRequest) GetPassword() string {
if x != nil {
return x.Password
}
return ""
}
// EnrollTOTPResponse returns the TOTP secret and otpauth URI for display. // EnrollTOTPResponse returns the TOTP secret and otpauth URI for display.
// Security: the secret is shown once; it is stored only in encrypted form. // Security: the secret is shown once; it is stored only in encrypted form.
type EnrollTOTPResponse struct { type EnrollTOTPResponse struct {
@@ -578,8 +588,9 @@ const file_mcias_v1_auth_proto_rawDesc = "" +
"\x12RenewTokenResponse\x12\x14\n" + "\x12RenewTokenResponse\x12\x14\n" +
"\x05token\x18\x01 \x01(\tR\x05token\x129\n" + "\x05token\x18\x01 \x01(\tR\x05token\x129\n" +
"\n" + "\n" +
"expires_at\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampR\texpiresAt\"\x13\n" + "expires_at\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampR\texpiresAt\"/\n" +
"\x11EnrollTOTPRequest\"M\n" + "\x11EnrollTOTPRequest\x12\x1a\n" +
"\bpassword\x18\x01 \x01(\tR\bpassword\"M\n" +
"\x12EnrollTOTPResponse\x12\x16\n" + "\x12EnrollTOTPResponse\x12\x16\n" +
"\x06secret\x18\x01 \x01(\tR\x06secret\x12\x1f\n" + "\x06secret\x18\x01 \x01(\tR\x06secret\x12\x1f\n" +
"\votpauth_uri\x18\x02 \x01(\tR\n" + "\votpauth_uri\x18\x02 \x01(\tR\n" +

View File

@@ -207,13 +207,39 @@ func (a *authServiceServer) RenewToken(ctx context.Context, _ *mciasv1.RenewToke
} }
// EnrollTOTP begins TOTP enrollment for the calling account. // EnrollTOTP begins TOTP enrollment for the calling account.
func (a *authServiceServer) EnrollTOTP(ctx context.Context, _ *mciasv1.EnrollTOTPRequest) (*mciasv1.EnrollTOTPResponse, error) { //
// Security (SEC-01): the current password is required to prevent a stolen
// session token from being used to enroll attacker-controlled TOTP on the
// victim's account. Lockout is checked and failures are recorded.
func (a *authServiceServer) EnrollTOTP(ctx context.Context, req *mciasv1.EnrollTOTPRequest) (*mciasv1.EnrollTOTPResponse, error) {
claims := claimsFromContext(ctx) claims := claimsFromContext(ctx)
acct, err := a.s.db.GetAccountByUUID(claims.Subject) acct, err := a.s.db.GetAccountByUUID(claims.Subject)
if err != nil { if err != nil {
return nil, status.Error(codes.Unauthenticated, "account not found") return nil, status.Error(codes.Unauthenticated, "account not found")
} }
if req.Password == "" {
return nil, status.Error(codes.InvalidArgument, "password is required")
}
// Security: check lockout before verifying (same as login flow).
locked, lockErr := a.s.db.IsLockedOut(acct.ID)
if lockErr != nil {
a.s.logger.Error("lockout check (gRPC TOTP enroll)", "error", lockErr)
}
if locked {
a.s.db.WriteAuditEvent(model.EventTOTPEnrolled, &acct.ID, &acct.ID, peerIP(ctx), `{"result":"locked"}`) //nolint:errcheck
return nil, status.Error(codes.ResourceExhausted, "account temporarily locked")
}
// Security: verify the current password with Argon2id (constant-time).
ok, verifyErr := auth.VerifyPassword(req.Password, acct.PasswordHash)
if verifyErr != nil || !ok {
_ = a.s.db.RecordLoginFailure(acct.ID)
a.s.db.WriteAuditEvent(model.EventTOTPEnrolled, &acct.ID, &acct.ID, peerIP(ctx), `{"result":"wrong_password"}`) //nolint:errcheck
return nil, status.Error(codes.Unauthenticated, "password is incorrect")
}
rawSecret, b32Secret, err := auth.GenerateTOTPSecret() rawSecret, b32Secret, err := auth.GenerateTOTPSecret()
if err != nil { if err != nil {
return nil, status.Error(codes.Internal, "internal error") return nil, status.Error(codes.Internal, "internal error")

View File

@@ -801,6 +801,10 @@ func (s *Server) handleRevokeRole(w http.ResponseWriter, r *http.Request) {
// ---- TOTP endpoints ---- // ---- TOTP endpoints ----
type totpEnrollRequest struct {
Password string `json:"password"` // security: current password required to prevent session-theft escalation
}
type totpEnrollResponse struct { type totpEnrollResponse struct {
Secret string `json:"secret"` // base32-encoded Secret string `json:"secret"` // base32-encoded
OTPAuthURI string `json:"otpauth_uri"` OTPAuthURI string `json:"otpauth_uri"`
@@ -810,6 +814,12 @@ type totpConfirmRequest struct {
Code string `json:"code"` Code string `json:"code"`
} }
// handleTOTPEnroll begins TOTP enrollment for the calling account.
//
// Security (SEC-01): the current password is required in the request body to
// prevent a stolen session token from being used to enroll attacker-controlled
// MFA on the victim's account. Lockout is checked and failures are recorded
// to prevent brute-force use of this endpoint as a password oracle.
func (s *Server) handleTOTPEnroll(w http.ResponseWriter, r *http.Request) { func (s *Server) handleTOTPEnroll(w http.ResponseWriter, r *http.Request) {
claims := middleware.ClaimsFromContext(r.Context()) claims := middleware.ClaimsFromContext(r.Context())
acct, err := s.db.GetAccountByUUID(claims.Subject) acct, err := s.db.GetAccountByUUID(claims.Subject)
@@ -818,6 +828,38 @@ func (s *Server) handleTOTPEnroll(w http.ResponseWriter, r *http.Request) {
return return
} }
var req totpEnrollRequest
if !decodeJSON(w, r, &req) {
return
}
if req.Password == "" {
middleware.WriteError(w, http.StatusBadRequest, "password is required", "bad_request")
return
}
// Security: check lockout before verifying (same as login and password-change flows)
// so an attacker cannot use this endpoint to brute-force the current password.
locked, lockErr := s.db.IsLockedOut(acct.ID)
if lockErr != nil {
s.logger.Error("lockout check (TOTP enroll)", "error", lockErr)
}
if locked {
s.writeAudit(r, model.EventTOTPEnrolled, &acct.ID, &acct.ID, `{"result":"locked"}`)
middleware.WriteError(w, http.StatusTooManyRequests, "account temporarily locked", "account_locked")
return
}
// Security: verify the current password with the same constant-time
// Argon2id path used at login to prevent timing oracles.
ok, verifyErr := auth.VerifyPassword(req.Password, acct.PasswordHash)
if verifyErr != nil || !ok {
_ = s.db.RecordLoginFailure(acct.ID)
s.writeAudit(r, model.EventTOTPEnrolled, &acct.ID, &acct.ID, `{"result":"wrong_password"}`)
middleware.WriteError(w, http.StatusUnauthorized, "password is incorrect", "unauthorized")
return
}
rawSecret, b32Secret, err := auth.GenerateTOTPSecret() rawSecret, b32Secret, err := auth.GenerateTOTPSecret()
if err != nil { if err != nil {
middleware.WriteError(w, http.StatusInternalServerError, "internal error", "internal_error") middleware.WriteError(w, http.StatusInternalServerError, "internal error", "internal_error")

View File

@@ -519,8 +519,10 @@ func TestTOTPEnrollDoesNotRequireTOTP(t *testing.T) {
t.Fatalf("TrackToken: %v", err) t.Fatalf("TrackToken: %v", err)
} }
// Start enrollment. // Start enrollment (password required since SEC-01 fix).
rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", nil, tokenStr) rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", totpEnrollRequest{
Password: "testpass123",
}, tokenStr)
if rr.Code != http.StatusOK { if rr.Code != http.StatusOK {
t.Fatalf("enroll status = %d, want 200; body: %s", rr.Code, rr.Body.String()) t.Fatalf("enroll status = %d, want 200; body: %s", rr.Code, rr.Body.String())
} }
@@ -558,6 +560,61 @@ func TestTOTPEnrollDoesNotRequireTOTP(t *testing.T) {
} }
} }
// TestTOTPEnrollRequiresPassword verifies that TOTP enrollment (SEC-01)
// requires the current password. A stolen session token alone must not be
// sufficient to add attacker-controlled MFA to the victim's account.
func TestTOTPEnrollRequiresPassword(t *testing.T) {
srv, _, priv, _ := newTestServer(t)
acct := createTestHumanAccount(t, srv, "totp-pw-check")
handler := srv.Handler()
tokenStr, claims, err := token.IssueToken(priv, testIssuer, acct.UUID, nil, time.Hour)
if err != nil {
t.Fatalf("IssueToken: %v", err)
}
if err := srv.db.TrackToken(claims.JTI, acct.ID, claims.IssuedAt, claims.ExpiresAt); err != nil {
t.Fatalf("TrackToken: %v", err)
}
t.Run("no password", func(t *testing.T) {
rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", totpEnrollRequest{}, tokenStr)
if rr.Code != http.StatusBadRequest {
t.Errorf("enroll without password: status = %d, want %d; body: %s",
rr.Code, http.StatusBadRequest, rr.Body.String())
}
})
t.Run("wrong password", func(t *testing.T) {
rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", totpEnrollRequest{
Password: "wrong-password",
}, tokenStr)
if rr.Code != http.StatusUnauthorized {
t.Errorf("enroll with wrong password: status = %d, want %d; body: %s",
rr.Code, http.StatusUnauthorized, rr.Body.String())
}
})
t.Run("correct password", func(t *testing.T) {
rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", totpEnrollRequest{
Password: "testpass123",
}, tokenStr)
if rr.Code != http.StatusOK {
t.Fatalf("enroll with correct password: status = %d, want 200; body: %s",
rr.Code, rr.Body.String())
}
var resp totpEnrollResponse
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("unmarshal: %v", err)
}
if resp.Secret == "" {
t.Error("expected non-empty TOTP secret")
}
if resp.OTPAuthURI == "" {
t.Error("expected non-empty otpauth URI")
}
})
}
func TestRenewToken(t *testing.T) { func TestRenewToken(t *testing.T) {
srv, _, priv, _ := newTestServer(t) srv, _, priv, _ := newTestServer(t)
acct := createTestHumanAccount(t, srv, "renew-user") acct := createTestHumanAccount(t, srv, "renew-user")

View File

@@ -550,6 +550,17 @@ paths:
tags: [Auth] tags: [Auth]
security: security:
- bearerAuth: [] - bearerAuth: []
requestBody:
required: true
content:
application/json:
schema:
type: object
required: [password]
properties:
password:
type: string
description: Current account password (required to prevent session-theft escalation).
responses: responses:
"200": "200":
description: TOTP secret generated. description: TOTP secret generated.

View File

@@ -45,8 +45,12 @@ message RenewTokenResponse {
// --- TOTP enrollment --- // --- TOTP enrollment ---
// EnrollTOTPRequest carries no body; the acting account is from the JWT. // EnrollTOTPRequest carries the current password for re-authentication.
message EnrollTOTPRequest {} // Security (SEC-01): password is required to prevent a stolen session token
// from being used to enroll attacker-controlled TOTP on the victim's account.
message EnrollTOTPRequest {
string password = 1; // security: current password required; never logged
}
// EnrollTOTPResponse returns the TOTP secret and otpauth URI for display. // EnrollTOTPResponse returns the TOTP secret and otpauth URI for display.
// Security: the secret is shown once; it is stored only in encrypted form. // Security: the secret is shown once; it is stored only in encrypted form.

View File

@@ -435,6 +435,17 @@ paths:
tags: [Auth] tags: [Auth]
security: security:
- bearerAuth: [] - bearerAuth: []
requestBody:
required: true
content:
application/json:
schema:
type: object
required: [password]
properties:
password:
type: string
description: Current account password (required to prevent session-theft escalation).
responses: responses:
"200": "200":
description: TOTP secret generated. description: TOTP secret generated.