Skip to content
Merged
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
10 changes: 5 additions & 5 deletions docs/IMPLEMENTATION-PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ The former `database/07-seed.sql` created an administrator with publicly known c

**Current problem**

The former public seed is no longer used and a controlled bootstrap now exists. Runtime verification confirms that it creates only the first administrator, skips creation when an administrator already exists, and fails closed for invalid configuration. SEC-002 cannot yet be closed: the public registration flow currently rolls back when its audit write races the uncommitted user insert (see the verification note below).
The former public seed is no longer used and a controlled bootstrap now exists. Runtime verification confirms that it creates only the first administrator, skips creation when an administrator already exists, and fails closed for missing or invalid configuration. The registration audit transaction ordering was corrected and verified against PostgreSQL (see the verification note below).

**Expected behavior**

Expand All @@ -105,15 +105,15 @@ The former public seed is no longer used and a controlled bootstrap now exists.

- [x] First startup with valid bootstrap input creates one BCrypt-hashed `ADMIN`.
- [x] A second startup creates no additional administrator and changes no existing credentials (verified against the Docker PostgreSQL database; count remained one).
- [ ] Missing or invalid bootstrap input fails safely without exposing secrets. Invalid input was verified to stop startup with the generic message `Admin bootstrap configuration is invalid`; the missing-input case remains to be verified.
- [ ] Bootstrap secrets are absent from logs, API responses, Git history, and persisted configuration.
- [ ] `POST /auth/register` cannot create an `ADMIN`, including when the JSON includes a `role` field. The DTO has no writable role field and the attempted request rolled back without persisting a user or an administrator, but the endpoint returned `500` because its audit record referenced an uncommitted user. Fix and then repeat this functional test before accepting the criterion.
- [x] Missing or invalid bootstrap input fails safely without exposing secrets. Both cases stopped startup with the generic message `Admin bootstrap configuration is invalid`; the administrator count remained one.
- [x] Bootstrap secrets are absent from application logs, API responses, tracked configuration and the `.env` Git history path. The secret is supplied only as a runtime environment value; the database stores the BCrypt hash, not the original password.
- [x] `POST /auth/register` cannot create an `ADMIN`, including when the JSON includes a `role` field. A real request with `"role":"ADMIN"` returned `201`, persisted `USER`, created one `USER_REGISTER` audit record and left the administrator count at one.

**Verification note — 2026-10-20**

- `ADMIN_BOOTSTRAP_ENABLED=true` with an existing administrator logged that bootstrap was skipped; the database still contained exactly one `ADMIN`.
- Invalid bootstrap password caused startup to fail with `Admin bootstrap configuration is invalid`, and the database still contained exactly one `ADMIN`; the application was then restored with bootstrap disabled.
- A real `POST /auth/register` request containing `"role":"ADMIN"` persisted no user. The cause was not role assignment: `AuditService.logRegister()` attempted to create `audit_logs.user_id` before the enclosing registration transaction committed, violating `audit_logs_user_id_fkey`. This is a P0 functional/audit defect that must be fixed before the public-registration requirement can be accepted.
- The initial registration test exposed that `AuditService.logRegister()` opened a separate transaction before the enclosing registration transaction committed, violating `audit_logs_user_id_fkey`. The correction makes registration audit join the registration transaction after `UserRepository.flush()`; a repeated real request returned `201`, stored a `USER` and its audit row, and did not add an administrator.

### SEC-003 — Remove original refresh-token storage from the user record

Expand Down
15 changes: 12 additions & 3 deletions src/main/java/com/wallet/secure/audit/service/AuditService.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,18 @@ public void logLogout(UUID userId, String email, String ipAddress, String userAg
/**
* Logs a new user registration.
* Called by AuthService.register() after user is created.
*
* <p>Unlike background audit events, registration audit joins the
* registration transaction. The audit row has a foreign key to the newly
* created user, so a separate transaction cannot read that user until the
* outer transaction commits. AuthService flushes the user before calling
* this method; both records then commit or roll back together.</p>
*
* <p>OWASP A09: a successfully registered account must have its audit
* record. For this particular event, consistency is more important than
* fire-and-forget execution.</p>
*/
@Async
@Transactional(propagation = Propagation.REQUIRES_NEW)
@Transactional(propagation = Propagation.MANDATORY)
public void logRegister(UUID userId, String email, String ipAddress, String userAgent) {
String details = buildDetails(
"\"email\":\"" + email + "\"",
Expand Down Expand Up @@ -352,4 +361,4 @@ private void save(AuditLog auditLog) {
private String buildDetails(String... keyValuePairs) {
return "{" + String.join(",", keyValuePairs) + "}";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ public ApiResponse<AuthResponse> register(RegisterRequest request, HttpServletRe

// Step 3 - generate tokens and persist refresh token
AuthResponse tokens = generateAndSaveTokens(user, extractIp(httpRequest), extractUserAgent(httpRequest));

// The registration audit row references users.id through a foreign key.
// Flush the pending user insert in THIS transaction before adding the audit row.
// OWASP A09: registration and its audit record must commit atomically.
userRepository.flush();

auditService.logRegister(
user.getId(),
user.getEmail(),
Expand Down Expand Up @@ -316,4 +322,4 @@ public ApiResponse<Void> logout(String email, LogoutRequest logoutRequest, HttpS

return ApiResponse.ok("Logout successful", null);
}
}
}
12 changes: 10 additions & 2 deletions src/test/java/com/wallet/secure/auth/service/AuthServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.security.authentication.AuthenticationManager;
Expand Down Expand Up @@ -101,7 +102,7 @@ void setUp() {
lenient().when(httpRequest.getRemoteAddr()).thenReturn("127.0.0.1");
lenient().when(httpRequest.getHeader("User-Agent")).thenReturn("JUnit-Test-Agent");

// AuditService stubs — lenient because audit calls are fire-and-forget
// AuditService stubs — lenient because most tests do not reach audit calls.
// We don't assert on audit calls in most tests — they have their own tests
lenient().doNothing().when(auditService)
.logLoginSuccess(any(), anyString(), anyString(), anyString());
Expand Down Expand Up @@ -145,6 +146,13 @@ void register_validRequest_returnsTokens() {

// Refresh token was persisted in DB — enables future revocation on logout
verify(userRepository, times(1)).save(any(User.class));

// Registration audit references users.id: the user must be flushed
// before AuditService joins the same transaction (OWASP A09).
InOrder persistenceOrder = inOrder(userRepository, auditService);
persistenceOrder.verify(userRepository).flush();
persistenceOrder.verify(auditService).logRegister(
eq(testUser.getId()), eq(TEST_EMAIL), anyString(), anyString());
}

@Test
Expand Down Expand Up @@ -397,4 +405,4 @@ void logout_userNotFound_throwsException() {
verify(userRepository, never()).save(any());
}
}
}
}
Loading