- Replace hardcoded JWT secret in DockerTenantProvisioner with config
property (CAMELEER_SAAS_PROVISIONING_JWTSECRET) — every provisioned
tenant server was sharing the same publicly-visible dev secret
- Add @PreAuthorize("SCOPE_tenant:manage") to 11 admin endpoints in
TenantPortalController (team invite/remove/role, password resets,
server restart/upgrade, CA cert management, MFA reset) — previously
any org member including viewers could perform admin operations
- Remove dead PATCH /api/tenant/settings endpoint (duplicate of
/auth-settings without authorization) and POST /api/tenant/password
(allowed password change without current password verification) —
frontend uses the secure alternatives
- Add @PreAuthorize("SCOPE_platform:admin") to TenantController
getById and getBySlug — were exposing serverEndpoint, adminEmail,
and provisionError to any authenticated user
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
311 lines
10 KiB
Markdown
311 lines
10 KiB
Markdown
# Security Review Fixes Implementation Plan
|
|
|
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
|
|
|
**Goal:** Fix 5 security vulnerabilities found in the full-codebase security review — hardcoded JWT secret, missing authorization on tenant portal admin endpoints, dead code password/settings endpoints, and unprotected tenant lookups.
|
|
|
|
**Architecture:** All fixes are surgical edits to existing files. No new files, no schema changes. The JWT secret fix adds one field to `ProvisioningProperties` and reads it in the provisioner. The authorization fixes add `@PreAuthorize` annotations. Dead code removal deletes unsafe endpoints and their unused frontend hooks.
|
|
|
|
**Tech Stack:** Spring Boot, Spring Security (`@PreAuthorize`), Spring Boot `@ConfigurationProperties`
|
|
|
|
---
|
|
|
|
### Task 1: Fix hardcoded JWT secret in DockerTenantProvisioner
|
|
|
|
**Files:**
|
|
- Modify: `src/main/java/io/cameleer/saas/provisioning/ProvisioningProperties.java`
|
|
- Modify: `src/main/java/io/cameleer/saas/provisioning/DockerTenantProvisioner.java:223`
|
|
|
|
- [ ] **Step 1: Add `jwtSecret` field to ProvisioningProperties**
|
|
|
|
In `src/main/java/io/cameleer/saas/provisioning/ProvisioningProperties.java`, add `jwtSecret` as a new field after `corsOrigins`:
|
|
|
|
```java
|
|
@ConfigurationProperties(prefix = "cameleer.saas.provisioning")
|
|
public record ProvisioningProperties(
|
|
String serverImage,
|
|
String serverUiImage,
|
|
String runtimeBaseImage,
|
|
String networkName,
|
|
String traefikNetwork,
|
|
String publicHost,
|
|
String publicProtocol,
|
|
String datasourceUrl,
|
|
String datasourceUsername,
|
|
String datasourcePassword,
|
|
String clickhouseUrl,
|
|
String clickhouseUser,
|
|
String clickhousePassword,
|
|
String oidcIssuerUri,
|
|
String oidcJwkSetUri,
|
|
String corsOrigins,
|
|
String jwtSecret
|
|
) {}
|
|
```
|
|
|
|
This binds from env var `CAMELEER_SAAS_PROVISIONING_JWTSECRET`. The installer already generates `CAMELEER_SERVER_SECURITY_JWTSECRET` — the compose template needs to also set `CAMELEER_SAAS_PROVISIONING_JWTSECRET` to the same value (or the deployer maps it manually). A missing value will be `null`, caught by the validation below.
|
|
|
|
- [ ] **Step 2: Replace hardcoded secret with property value**
|
|
|
|
In `src/main/java/io/cameleer/saas/provisioning/DockerTenantProvisioner.java`, replace line 223:
|
|
|
|
```java
|
|
// OLD:
|
|
"CAMELEER_SERVER_SECURITY_JWTSECRET=cameleer-dev-jwt-secret-change-in-production",
|
|
|
|
// NEW:
|
|
"CAMELEER_SERVER_SECURITY_JWTSECRET=" + props.jwtSecret(),
|
|
```
|
|
|
|
- [ ] **Step 3: Add startup validation for jwtSecret**
|
|
|
|
In `DockerTenantProvisioner.java`, add a validation check at the end of the constructor (after line 36):
|
|
|
|
```java
|
|
if (props.jwtSecret() == null || props.jwtSecret().isBlank()) {
|
|
log.warn("CAMELEER_SAAS_PROVISIONING_JWTSECRET is not set — provisioned servers will fail to start");
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Commit**
|
|
|
|
```bash
|
|
git add src/main/java/io/cameleer/saas/provisioning/ProvisioningProperties.java src/main/java/io/cameleer/saas/provisioning/DockerTenantProvisioner.java
|
|
git commit -m "fix(security): replace hardcoded JWT secret with config property
|
|
|
|
Every provisioned tenant server was using the same hardcoded dev JWT
|
|
secret ('cameleer-dev-jwt-secret-change-in-production'), visible in
|
|
source code. An attacker could forge valid JWT tokens for any tenant
|
|
server. Now reads from CAMELEER_SAAS_PROVISIONING_JWTSECRET."
|
|
```
|
|
|
|
---
|
|
|
|
### Task 2: Add authorization to TenantPortalController admin endpoints
|
|
|
|
**Files:**
|
|
- Modify: `src/main/java/io/cameleer/saas/portal/TenantPortalController.java`
|
|
|
|
- [ ] **Step 1: Add `@PreAuthorize` to team management endpoints**
|
|
|
|
Add `@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")` before each of these method annotations:
|
|
|
|
Line 76 — `inviteTeamMember`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@PostMapping("/team/invite")
|
|
```
|
|
|
|
Line 82 — `removeTeamMember`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@DeleteMapping("/team/{userId}")
|
|
```
|
|
|
|
Line 88 — `changeTeamMemberRole`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@PatchMapping("/team/{userId}/role")
|
|
```
|
|
|
|
Line 114 — `resetTeamMemberPassword`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@PostMapping("/team/{userId}/password")
|
|
```
|
|
|
|
Line 176 — `resetTeamMemberMfa`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@DeleteMapping("/users/{userId}/mfa")
|
|
```
|
|
|
|
- [ ] **Step 2: Add `@PreAuthorize` to server management endpoints**
|
|
|
|
Line 95 — `resetServerAdminPassword`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@PostMapping("/server/admin-password")
|
|
```
|
|
|
|
Line 125 — `restartServer`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@PostMapping("/server/restart")
|
|
```
|
|
|
|
Line 131 — `upgradeServer`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@PostMapping("/server/upgrade")
|
|
```
|
|
|
|
- [ ] **Step 3: Add `@PreAuthorize` to CA certificate management endpoints**
|
|
|
|
Line 289 — `stageCaCert`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@PostMapping("/ca")
|
|
```
|
|
|
|
Line 304 — `activateCaCert`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@PostMapping("/ca/{id}/activate")
|
|
```
|
|
|
|
Line 315 — `deleteCaCert`:
|
|
```java
|
|
@PreAuthorize("hasAuthority('SCOPE_tenant:manage')")
|
|
@DeleteMapping("/ca/{id}")
|
|
```
|
|
|
|
- [ ] **Step 4: Commit**
|
|
|
|
```bash
|
|
git add src/main/java/io/cameleer/saas/portal/TenantPortalController.java
|
|
git commit -m "fix(security): add authorization to tenant portal admin endpoints
|
|
|
|
All admin-level endpoints (team invite/remove/role, password resets,
|
|
server restart/upgrade, CA cert management) were accessible to any
|
|
org member including viewers. Now require SCOPE_tenant:manage,
|
|
matching the existing pattern on PATCH /auth-settings."
|
|
```
|
|
|
|
---
|
|
|
|
### Task 3: Remove dead code endpoints (settings duplicate + password without verification)
|
|
|
|
**Files:**
|
|
- Modify: `src/main/java/io/cameleer/saas/portal/TenantPortalController.java`
|
|
- Modify: `ui/src/api/tenant-hooks.ts`
|
|
|
|
- [ ] **Step 1: Remove `PATCH /settings` endpoint from controller**
|
|
|
|
Delete lines 238-242 from `TenantPortalController.java`:
|
|
|
|
```java
|
|
// DELETE THIS ENTIRE BLOCK:
|
|
@PatchMapping("/settings")
|
|
public ResponseEntity<Void> updateSettings(@RequestBody Map<String, Object> updates) {
|
|
portalService.updateTenantSettings(updates);
|
|
return ResponseEntity.ok().build();
|
|
}
|
|
```
|
|
|
|
This is a duplicate of `PATCH /auth-settings` (line 231-235) which has proper `@PreAuthorize`. The frontend uses `useUpdateTenantAuthSettings` which calls `/auth-settings`.
|
|
|
|
- [ ] **Step 2: Remove `POST /password` endpoint from controller**
|
|
|
|
Delete lines 107-112 from `TenantPortalController.java`:
|
|
|
|
```java
|
|
// DELETE THIS ENTIRE BLOCK:
|
|
@PostMapping("/password")
|
|
public ResponseEntity<Void> changeOwnPassword(@AuthenticationPrincipal Jwt jwt,
|
|
@RequestBody PasswordChangeRequest body) {
|
|
portalService.changePassword(jwt.getSubject(), body.password());
|
|
return ResponseEntity.noContent().build();
|
|
}
|
|
```
|
|
|
|
The frontend uses `POST /api/account/password` (via `AccountSettingsPage` + `AccountController`) which correctly requires current password verification.
|
|
|
|
- [ ] **Step 3: Remove unused hooks from tenant-hooks.ts**
|
|
|
|
Remove the `useChangeOwnPassword` hook (lines 124-128):
|
|
|
|
```typescript
|
|
// DELETE THIS ENTIRE BLOCK:
|
|
export function useChangeOwnPassword() {
|
|
return useMutation<void, Error, string>({
|
|
mutationFn: (password) => api.post('/tenant/password', { password }),
|
|
});
|
|
}
|
|
```
|
|
|
|
Remove the `useUpdateTenantSettings` mutation hook (lines 152-158):
|
|
|
|
```typescript
|
|
// DELETE THIS ENTIRE BLOCK:
|
|
export function useUpdateTenantSettings() {
|
|
const qc = useQueryClient();
|
|
return useMutation<void, Error, Record<string, unknown>>({
|
|
mutationFn: (updates) => api.patch('/tenant/settings', updates),
|
|
onSuccess: () => qc.invalidateQueries({ queryKey: ['tenant', 'settings'] }),
|
|
});
|
|
}
|
|
```
|
|
|
|
Note: Keep `useTenantSettings` (the GET query hook at lines 145-150) — the `GET /settings` endpoint returns tenant info (name, slug, tier) and is a legitimate read-only endpoint.
|
|
|
|
- [ ] **Step 4: Commit**
|
|
|
|
```bash
|
|
git add src/main/java/io/cameleer/saas/portal/TenantPortalController.java ui/src/api/tenant-hooks.ts
|
|
git commit -m "fix(security): remove unsafe dead code endpoints
|
|
|
|
Remove PATCH /api/tenant/settings (duplicate of /auth-settings without
|
|
authorization — any org member could disable MFA) and POST
|
|
/api/tenant/password (allowed password change without current password
|
|
verification). Both were dead code — frontend uses the secure
|
|
alternatives. Also remove corresponding unused hooks."
|
|
```
|
|
|
|
---
|
|
|
|
### Task 4: Add authorization to TenantController lookup endpoints
|
|
|
|
**Files:**
|
|
- Modify: `src/main/java/io/cameleer/saas/tenant/TenantController.java`
|
|
|
|
- [ ] **Step 1: Add `@PreAuthorize` to getById and getBySlug**
|
|
|
|
Line 58 — `getById`:
|
|
```java
|
|
@GetMapping("/{id}")
|
|
@PreAuthorize("hasAuthority('SCOPE_platform:admin')")
|
|
public ResponseEntity<TenantResponse> getById(@PathVariable UUID id) {
|
|
```
|
|
|
|
Line 65 — `getBySlug`:
|
|
```java
|
|
@GetMapping("/by-slug/{slug}")
|
|
@PreAuthorize("hasAuthority('SCOPE_platform:admin')")
|
|
public ResponseEntity<TenantResponse> getBySlug(@PathVariable String slug) {
|
|
```
|
|
|
|
This matches the existing `@PreAuthorize` on `listAll()` and `create()` in the same controller. These are vendor-only lookup endpoints — no tenant-scoped user should access arbitrary tenant records.
|
|
|
|
- [ ] **Step 2: Commit**
|
|
|
|
```bash
|
|
git add src/main/java/io/cameleer/saas/tenant/TenantController.java
|
|
git commit -m "fix(security): restrict tenant lookup endpoints to platform admins
|
|
|
|
GET /api/tenants/{id} and GET /api/tenants/by-slug/{slug} were
|
|
accessible to any authenticated user, exposing serverEndpoint,
|
|
adminEmail, and provisionError. Now require SCOPE_platform:admin,
|
|
matching listAll() and create() in the same controller."
|
|
```
|
|
|
|
---
|
|
|
|
### Task 5: Verify and build
|
|
|
|
- [ ] **Step 1: Run the build to verify all changes compile**
|
|
|
|
```bash
|
|
cd /c/Users/Hendrik/Documents/projects/cameleer-saas && ./mvnw compile -q
|
|
```
|
|
|
|
Expected: BUILD SUCCESS with no compilation errors.
|
|
|
|
- [ ] **Step 2: Run frontend type check**
|
|
|
|
```bash
|
|
cd ui && npm run typecheck
|
|
```
|
|
|
|
Expected: No type errors from removed hooks (they were unused).
|