Fix SEC-01: require password for TOTP enroll
- REST handleTOTPEnroll now requires password field in request body - gRPC EnrollTOTP updated with password field in proto message - Both handlers check lockout status and record failures on bad password - Updated Go, Python, and Rust client libraries to pass password - Updated OpenAPI specs with new requestBody schema - Added TestTOTPEnrollRequiresPassword with no-password, wrong-password, and correct-password sub-tests Security: TOTP enrollment now requires the current password to prevent session-theft escalation to persistent account takeover. Lockout and failure recording use the same Argon2id constant-time path as login. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
// The secret is shown once; it is not retrievable after this call.
|
||||
// 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
|
||||
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 &resp, nil
|
||||
|
||||
@@ -275,7 +275,7 @@ func TestEnrollTOTP(t *testing.T) {
|
||||
}))
|
||||
defer srv.Close()
|
||||
c := newTestClient(t, srv.URL)
|
||||
resp, err := c.EnrollTOTP()
|
||||
resp, err := c.EnrollTOTP("testpass123")
|
||||
if err != nil {
|
||||
t.Fatalf("EnrollTOTP: %v", err)
|
||||
}
|
||||
|
||||
@@ -148,11 +148,15 @@ class Client:
|
||||
expires_at = str(data["expires_at"])
|
||||
self.token = token
|
||||
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.
|
||||
|
||||
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.
|
||||
"""
|
||||
data = self._request("POST", "/v1/auth/totp/enroll")
|
||||
data = self._request("POST", "/v1/auth/totp/enroll", json={"password": password})
|
||||
assert data is not None
|
||||
return str(data["secret"]), str(data["otpauth_uri"])
|
||||
def confirm_totp(self, code: str) -> None:
|
||||
|
||||
@@ -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"},
|
||||
)
|
||||
)
|
||||
secret, uri = admin_client.enroll_totp()
|
||||
secret, uri = admin_client.enroll_totp("testpass123")
|
||||
assert secret == "JBSWY3DPEHPK3PXP"
|
||||
assert "otpauth://totp/" in uri
|
||||
@respx.mock
|
||||
|
||||
@@ -484,9 +484,12 @@ impl Client {
|
||||
|
||||
/// Begin TOTP enrollment. Returns `(secret, otpauth_uri)`.
|
||||
/// 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 =
|
||||
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))
|
||||
}
|
||||
|
||||
|
||||
@@ -449,7 +449,7 @@ async fn test_enroll_totp() {
|
||||
.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!(uri.starts_with("otpauth://totp/"));
|
||||
}
|
||||
|
||||
@@ -304,9 +304,12 @@ func (x *RenewTokenResponse) GetExpiresAt() *timestamppb.Timestamp {
|
||||
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 {
|
||||
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
|
||||
sizeCache protoimpl.SizeCache
|
||||
}
|
||||
@@ -341,6 +344,13 @@ func (*EnrollTOTPRequest) Descriptor() ([]byte, []int) {
|
||||
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.
|
||||
// Security: the secret is shown once; it is stored only in encrypted form.
|
||||
type EnrollTOTPResponse struct {
|
||||
@@ -578,8 +588,9 @@ const file_mcias_v1_auth_proto_rawDesc = "" +
|
||||
"\x12RenewTokenResponse\x12\x14\n" +
|
||||
"\x05token\x18\x01 \x01(\tR\x05token\x129\n" +
|
||||
"\n" +
|
||||
"expires_at\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampR\texpiresAt\"\x13\n" +
|
||||
"\x11EnrollTOTPRequest\"M\n" +
|
||||
"expires_at\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampR\texpiresAt\"/\n" +
|
||||
"\x11EnrollTOTPRequest\x12\x1a\n" +
|
||||
"\bpassword\x18\x01 \x01(\tR\bpassword\"M\n" +
|
||||
"\x12EnrollTOTPResponse\x12\x16\n" +
|
||||
"\x06secret\x18\x01 \x01(\tR\x06secret\x12\x1f\n" +
|
||||
"\votpauth_uri\x18\x02 \x01(\tR\n" +
|
||||
|
||||
@@ -195,13 +195,39 @@ func (a *authServiceServer) RenewToken(ctx context.Context, _ *mciasv1.RenewToke
|
||||
}
|
||||
|
||||
// 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)
|
||||
acct, err := a.s.db.GetAccountByUUID(claims.Subject)
|
||||
if err != nil {
|
||||
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()
|
||||
if err != nil {
|
||||
return nil, status.Error(codes.Internal, "internal error")
|
||||
|
||||
@@ -780,6 +780,10 @@ func (s *Server) handleRevokeRole(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// ---- TOTP endpoints ----
|
||||
|
||||
type totpEnrollRequest struct {
|
||||
Password string `json:"password"` // security: current password required to prevent session-theft escalation
|
||||
}
|
||||
|
||||
type totpEnrollResponse struct {
|
||||
Secret string `json:"secret"` // base32-encoded
|
||||
OTPAuthURI string `json:"otpauth_uri"`
|
||||
@@ -789,6 +793,12 @@ type totpConfirmRequest struct {
|
||||
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) {
|
||||
claims := middleware.ClaimsFromContext(r.Context())
|
||||
acct, err := s.db.GetAccountByUUID(claims.Subject)
|
||||
@@ -797,6 +807,38 @@ func (s *Server) handleTOTPEnroll(w http.ResponseWriter, r *http.Request) {
|
||||
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()
|
||||
if err != nil {
|
||||
middleware.WriteError(w, http.StatusInternalServerError, "internal error", "internal_error")
|
||||
|
||||
@@ -519,8 +519,10 @@ func TestTOTPEnrollDoesNotRequireTOTP(t *testing.T) {
|
||||
t.Fatalf("TrackToken: %v", err)
|
||||
}
|
||||
|
||||
// Start enrollment.
|
||||
rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", nil, tokenStr)
|
||||
// Start enrollment (password required since SEC-01 fix).
|
||||
rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", totpEnrollRequest{
|
||||
Password: "testpass123",
|
||||
}, tokenStr)
|
||||
if rr.Code != http.StatusOK {
|
||||
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) {
|
||||
srv, _, priv, _ := newTestServer(t)
|
||||
acct := createTestHumanAccount(t, srv, "renew-user")
|
||||
|
||||
11
openapi.yaml
11
openapi.yaml
@@ -550,6 +550,17 @@ paths:
|
||||
tags: [Auth]
|
||||
security:
|
||||
- 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:
|
||||
"200":
|
||||
description: TOTP secret generated.
|
||||
|
||||
@@ -45,8 +45,12 @@ message RenewTokenResponse {
|
||||
|
||||
// --- TOTP enrollment ---
|
||||
|
||||
// EnrollTOTPRequest carries no body; the acting account is from the JWT.
|
||||
message EnrollTOTPRequest {}
|
||||
// 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.
|
||||
message EnrollTOTPRequest {
|
||||
string password = 1; // security: current password required; never logged
|
||||
}
|
||||
|
||||
// EnrollTOTPResponse returns the TOTP secret and otpauth URI for display.
|
||||
// Security: the secret is shown once; it is stored only in encrypted form.
|
||||
|
||||
@@ -435,6 +435,17 @@ paths:
|
||||
tags: [Auth]
|
||||
security:
|
||||
- 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:
|
||||
"200":
|
||||
description: TOTP secret generated.
|
||||
|
||||
Reference in New Issue
Block a user