Skip to content

Add Spring Boot backend module for DPC community data API with account management#110

Open
Copilot wants to merge 16 commits intodevelopfrom
copilot/add-spring-boot-backend-module
Open

Add Spring Boot backend module for DPC community data API with account management#110
Copilot wants to merge 16 commits intodevelopfrom
copilot/add-spring-boot-backend-module

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 27, 2026

Adds a dpc-api Spring Boot module providing RESTful endpoints for cross-server community data, starting with Medieval Factions. Minecraft servers running DPC plugins can POST faction data; the website (and anyone) can GET it. Aligned with Medieval Factions PR #1959.

New module: dpc-api/

  • Stack: Spring Boot 3.4.4, Spring Security, Spring Data JPA, PostgreSQL, Flyway 10.x (flyway-database-postgresql), Lombok, springdoc-openapi, JJWT 0.12.6, Java 17
  • Endpoints:
    • POST /api/v1/factions — batch faction sync (API key required); accepts a JSON array of factions, de-duplicates and upserts by (name, serverId)
    • GET /api/v1/factions — paginated listing (public, default 20 / max 200 per page)
    • GET /api/v1/factions/{id} — single faction (public)
    • POST /api/v1/accounts/register — account registration (public); accepts { "username": "...", "password": "..." }, returns JWT token
    • POST /api/v1/accounts/login — account login (public); accepts { "username": "...", "password": "..." }, returns JWT token (supports headless Minecraft plugin auth via command line)
    • GET /api/v1/accounts/me — account profile (JWT required); returns account details and linked API keys
    • POST /api/v1/accounts/me/api-keys — create API key linked to account (JWT required); accepts { "serverName": "..." }, returns one-time raw key
    • DELETE /api/v1/accounts/me/api-keys/{keyId} — delete an API key (JWT required)
  • Auth: Dual auth model — X-API-Key header filter on write methods, validated against SHA-256 hashes in the api_keys table; Spring Security with stateless JWT for account management endpoints. BCrypt password hashing for accounts. Single registration flow through account system (no standalone API key registration endpoint).
  • Schema: Flyway-managed (V1__create_factions_table.sql, V2__create_api_keys_and_faction_fields.sql, V3__create_accounts_table.sql), UNIQUE constraint on (name, server_id), api_keys.owner_id NOT NULL FK to accounts, entity uses @PrePersist/@PreUpdate lifecycle callbacks, UUIDs generated by Hibernate
  • Data model: faction name, server ID, member count, description (TEXT), optional serverIp and discordLink, timestamps; accounts with username/password (BCrypt), API keys linked to accounts via owner_id
  • Lombok: @Getter/@Setter/@NoArgsConstructor/@RequiredArgsConstructor on entities, services, controllers, and filters to reduce boilerplate
  • OpenAPI/Swagger: springdoc-openapi integration with Swagger UI at /swagger-ui.html, OpenAPI spec at /v3/api-docs, @Operation/@Tag/@SecurityScheme annotations on all endpoints (both API key and JWT bearer token)
  • Global exception handler: @RestControllerAdvice with handlers for validation errors (400), malformed JSON (400), type mismatches (400), and a catch-all (500) returning consistent JSON error responses. ApiKeyAuthFilter error responses also use the same structured format (timestamp/status/error/message) for consistency.
  • Shared utilities: HashUtil consolidates SHA-256 hashing used by both AccountService and ApiKeyService into a single security-critical code path
  • JWT secret validation: Startup check ensures dpc.jwt.secret meets the minimum 256-bit (32-byte) length requirement for HS256, with a clear error message on misconfiguration
  • CORS: Configurable cross-origin support via dpc.cors.allowed-origins property (defaults to *), enabling the Next.js frontend to call the API from a different origin. Allows Authorization, X-API-Key, and Content-Type headers.
  • Maven Wrapper: Includes maven-wrapper.jar in .mvn/wrapper/ for reliable Docker builds on Alpine Linux without requiring curl or wget

Account management

  • Account system: Users (including Minecraft players via plugins) can register and log in with username/password via REST API, receiving a JWT token for authenticated operations. This is the single entry point for registration — API keys are created through the account profile after authentication.
  • Headless CLI support: Login and registration endpoints accept JSON POST requests, making them compatible with Minecraft plugin command-line flows (no browser required)
  • API key association: API keys created through the account system are linked to the user's profile via owner_id NOT NULL foreign key
  • Profile management: Authenticated users can view their profile and manage (create/delete) their API keys
  • Website UI: Account management page at /account (Next.js/MUI) with login, registration, profile view, and API key CRUD. Clipboard copy with error handling, proper error feedback for all non-OK HTTP responses, and accessible aria-label attributes on all interactive IconButtons.

Infrastructure

  • Docker Compose: dpc-db (PostgreSQL 16 Alpine) and dpc-api services with multi-stage Dockerfile (JDK builder + JRE runtime). Maven wrapper JAR included for offline builds.
  • CI: build-api job in build.yml runs ./mvnw verify with Java 17 (Temurin) on push/PR to main and develop
  • Tests: 69 tests total — AccountControllerTest (20), FactionControllerTest (20), AccountServiceTest (8), FactionServiceTest (9), ApiKeyServiceTest (5), JwtServiceTest (6), DpcApiApplicationTests (1). Integration tests use MockMvc + H2 in-memory DB.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits March 27, 2026 02:29
…Flyway migration, tests, Dockerfile, and Docker Compose integration

Agent-Logs-Url: https://github.com/Dans-Plugins/dansplugins-dot-com/sessions/bd6589fe-5971-41d2-842b-e5a071d82195

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
…ey comparison, and fail-fast on missing API key

Agent-Logs-Url: https://github.com/Dans-Plugins/dansplugins-dot-com/sessions/bd6589fe-5971-41d2-842b-e5a071d82195

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Copilot AI changed the title [WIP] Add Spring Boot backend module for DPC community data API Add Spring Boot backend module for DPC community data API Mar 27, 2026
Copilot AI requested a review from dmccoystephenson March 27, 2026 02:35
@dmccoystephenson dmccoystephenson requested review from Copilot and removed request for Copilot March 27, 2026 05:31
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new dpc-api/ Spring Boot module to serve a community data REST API (starting with Factions) alongside the existing Next.js website, plus local-dev infrastructure via Docker Compose.

Changes:

  • Introduces a Spring Boot 3.2.x + JPA + Flyway + PostgreSQL-backed API module with /api/v1/factions endpoints and API-key auth on write methods.
  • Adds Flyway migration + JPA entity/DTO/service/controller layers for faction ingestion and public reads.
  • Adds Docker Compose services for dpc-db (Postgres) and dpc-api, plus MockMvc + H2 integration tests.

Reviewed changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
dpc-api/src/test/resources/application-test.yml Test profile config for H2 + disables Flyway.
dpc-api/src/test/java/com/dansplugins/api/controller/FactionControllerTest.java MockMvc integration tests for auth/validation/paging/read endpoints.
dpc-api/src/test/java/com/dansplugins/api/DpcApiApplicationTests.java Basic Spring context load test.
dpc-api/src/main/resources/db/migration/V1__create_factions_table.sql Flyway migration to create factions table + index.
dpc-api/src/main/resources/application.yml Runtime configuration for Postgres/Flyway and API key property.
dpc-api/src/main/java/com/dansplugins/api/service/FactionService.java Service layer for create/list/get faction(s).
dpc-api/src/main/java/com/dansplugins/api/repository/FactionRepository.java Spring Data JPA repository for Faction.
dpc-api/src/main/java/com/dansplugins/api/filter/ApiKeyAuthFilter.java API-key auth filter applied to write HTTP methods.
dpc-api/src/main/java/com/dansplugins/api/entity/Faction.java JPA entity mapping and timestamp lifecycle hooks.
dpc-api/src/main/java/com/dansplugins/api/dto/FactionResponse.java API response DTO mapping from entity.
dpc-api/src/main/java/com/dansplugins/api/dto/FactionRequest.java Request DTO with Bean Validation constraints.
dpc-api/src/main/java/com/dansplugins/api/controller/FactionController.java REST controller for create/list/get endpoints.
dpc-api/src/main/java/com/dansplugins/api/config/ApiKeyConfig.java Fail-fast validation that API key is configured.
dpc-api/src/main/java/com/dansplugins/api/DpcApiApplication.java Spring Boot application entrypoint.
dpc-api/pom.xml Maven build definition and dependencies.
dpc-api/mvnw.cmd Maven wrapper script (Windows).
dpc-api/mvnw Maven wrapper script (Unix).
dpc-api/README.md Module documentation (local dev, config, endpoints).
dpc-api/Dockerfile Container image definition for the API.
dpc-api/.mvn/wrapper/maven-wrapper.properties Maven wrapper distribution configuration.
compose.yml Adds dpc-db and dpc-api services for local dev.
.gitignore Ignores Maven build outputs and un-ignores dpc-api/pom.xml.

Comment on lines +1 to +5
FROM eclipse-temurin:17-jre-alpine

WORKDIR /app

COPY target/dpc-api-*.jar app.jar
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image build expects a pre-built JAR at target/dpc-api-*.jar, but docker compose up --build will run docker build without running Maven first (and dpc-api/target/ is gitignored). Use a multi-stage Dockerfile that builds the JAR (e.g., run ./mvnw -DskipTests package in a builder stage) or adjust Compose to build the JAR before building the image.

Suggested change
FROM eclipse-temurin:17-jre-alpine
WORKDIR /app
COPY target/dpc-api-*.jar app.jar
FROM eclipse-temurin:17-jdk-alpine AS builder
WORKDIR /build
COPY mvnw pom.xml ./
COPY .mvn .mvn
COPY src src
RUN ./mvnw -DskipTests package
FROM eclipse-temurin:17-jre-alpine
WORKDIR /app
COPY --from=builder /build/target/dpc-api-*.jar app.jar

Copilot uses AI. Check for mistakes.
compose.yml Outdated
DB_NAME: dpc
DB_USERNAME: dpc
DB_PASSWORD: dpc
DPC_API_KEY: ${DPC_API_KEY:-changeme}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DPC_API_KEY defaults to changeme in Compose, which makes it easy to accidentally deploy/run with a known API key. Consider removing the default and failing fast (let the API refuse to start when the key isn’t provided), or at minimum generate a random dev-only key and document it.

Suggested change
DPC_API_KEY: ${DPC_API_KEY:-changeme}
DPC_API_KEY: ${DPC_API_KEY:?DPC_API_KEY must be set}

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +3
CREATE TABLE factions (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
name VARCHAR(255) NOT NULL,
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gen_random_uuid() requires the pgcrypto extension in Postgres; on a fresh postgres:16-alpine instance this migration can fail unless the extension is enabled. Either add CREATE EXTENSION IF NOT EXISTS pgcrypto; in the migration or remove the DB-side default if UUIDs are generated by Hibernate.

Copilot uses AI. Check for mistakes.

@Column(name = "member_count", nullable = false)
private int memberCount;

Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The migration defines description as TEXT, but the entity maps it as a default String (typically VARCHAR(255)), and spring.jpa.hibernate.ddl-auto is set to validate. This can cause schema validation failures at startup. Align the mappings (e.g., mark the field as @Lob / @Column(columnDefinition = "TEXT") or change the column type in the migration).

Suggested change
@Column(columnDefinition = "TEXT")

Copilot uses AI. Check for mistakes.
public void validateApiKey() {
if (apiKey == null || apiKey.isBlank()) {
throw new IllegalStateException(
"DPC_API_KEY environment variable must be set. "
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The startup validation message says the DPC_API_KEY environment variable must be set, but the actual requirement is that dpc.api.key is configured (which could come from other property sources). Consider wording this as “dpc.api.key must be configured (e.g., via DPC_API_KEY env var)” to avoid misleading operators.

Suggested change
"DPC_API_KEY environment variable must be set. "
"`dpc.api.key` must be configured (e.g., via DPC_API_KEY environment variable). "

Copilot uses AI. Check for mistakes.
ddl-auto: validate
open-in-view: false
flyway:
enabled: true
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pageable parameters are fully client-controlled here; without a configured max page size, callers can request extremely large size values and force expensive DB queries. Consider setting spring.data.web.pageable.max-page-size (and possibly a default page size) in application.yml to put an upper bound on size.

Suggested change
enabled: true
enabled: true
data:
web:
pageable:
default-page-size: 20
max-page-size: 200

Copilot uses AI. Check for mistakes.
Comment on lines +37 to +45
<dependency>
<groupId>org.flywaydb</groupId>
<artifactId>flyway-core</artifactId>
</dependency>
<dependency>
<groupId>org.postgresql</groupId>
<artifactId>postgresql</artifactId>
<scope>runtime</scope>
</dependency>
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Spring Boot 3.2.x the managed Flyway version is in the 10.x line, where several databases (including PostgreSQL) require an additional flyway-database-postgresql dependency. With only flyway-core present, Flyway can fail at runtime with an “Unsupported Database: PostgreSQL …” error. Add the PostgreSQL Flyway database module (or pin Flyway to a version where Postgres support is bundled).

Copilot uses AI. Check for mistakes.
@dmccoystephenson
Copy link
Copy Markdown
Member

@copilot address comments

…emove gen_random_uuid, TEXT column type, pageable limits, flyway-database-postgresql

Agent-Logs-Url: https://github.com/Dans-Plugins/dansplugins-dot-com/sessions/b23ddfca-6a02-4589-9023-ab80ea2b9f0b

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 27, 2026

@copilot address comments

All 7 review comments addressed in 7b85b1b:

  1. Dockerfile → multi-stage build (JDK builder stage + JRE runtime stage) so docker compose up --build works without pre-building the JAR
  2. compose.ymlDPC_API_KEY now uses ${DPC_API_KEY:?DPC_API_KEY must be set} — Compose will fail if unset
  3. Flyway migration → removed DEFAULT gen_random_uuid() since Hibernate generates UUIDs, no pgcrypto needed
  4. Faction entity → added @Column(columnDefinition = "TEXT") on description to match the migration
  5. ApiKeyConfig → updated error message to reference dpc.api.key property name
  6. application.yml → added spring.data.web.pageable.default-page-size: 20 and max-page-size: 200
  7. pom.xml → upgraded to Spring Boot 3.4.4 (Flyway 10.x) and added flyway-database-postgresql dependency

All 9 tests pass.

@dmccoystephenson
Copy link
Copy Markdown
Member

@copilot How do operators get an API key are we gonna have account creation in this PR?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 28, 2026

@copilot How do operators get an API key are we gonna have account creation in this PR?

No account creation in this PR. The current auth model is a single shared secret: the person deploying the API sets DPC_API_KEY as an environment variable, then distributes that same key to Minecraft server operators who need write access (they send it via the X-API-Key header on POST requests). GET endpoints are public — no key needed.

If per-operator keys or self-service registration is needed down the road, that would be a separate feature (e.g., an api_keys table with hashed keys, scopes, and an admin UI or CLI to issue them).

@dmccoystephenson
Copy link
Copy Markdown
Member

@copilot the ability to auto register is a requirement; ensure this PR aligns with Dans-Plugins/Medieval-Factions#1959

Copilot AI and others added 2 commits March 28, 2026 01:11
- POST /api/v1/register: self-service API key registration (no auth required)
- API keys stored as SHA-256 hashes in new api_keys table (V2 migration)
- Auth filter validates against DB-stored keys instead of single env var
- POST /api/v1/factions now accepts JSON array (batch sync) with upsert
- Added serverIp and discordLink optional fields to faction model
- Removed single shared DPC_API_KEY requirement from compose.yml
- All 14 tests pass

Agent-Logs-Url: https://github.com/Dans-Plugins/dansplugins-dot-com/sessions/2aca508b-3b95-439a-a987-ad12f6e61b0b

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
…6 private

Agent-Logs-Url: https://github.com/Dans-Plugins/dansplugins-dot-com/sessions/2aca508b-3b95-439a-a987-ad12f6e61b0b

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
@dmccoystephenson
Copy link
Copy Markdown
Member

@copilot there should only be one register endpoint

…v1/register

Remove the legacy API key registration endpoint (POST /api/v1/register) in favor
of the single account-based registration flow (POST /api/v1/accounts/register).

Files removed:
- RegisterController.java
- RegisterControllerTest.java (8 tests)
- RegisterRequest.java
- RegisterResponse.java

Files updated:
- ApiKeyService: removed register() method (keep isValidKey/sha256)
- ApiKeyServiceTest: refactored to 5 tests without register dependency
- ApiKeyAuthFilter: removed /api/v1/register exemption
- SecurityConfig: removed /api/v1/register permit
- ApiKey entity: removed unused 2-arg constructor
- FactionControllerTest: create test keys via AccountService
- README.md: removed legacy register section

69 tests pass (down from 79 — 8 removed RegisterControllerTest, 2 removed
ApiKeyServiceTest register tests).

Agent-Logs-Url: https://github.com/Dans-Plugins/dansplugins-dot-com/sessions/240f05e4-a70e-4840-a80c-e060d766a4fb

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 28, 2026

@copilot there should only be one register endpoint

Consolidated to a single register endpoint in 582166d. Removed the standalone POST /api/v1/register (legacy API key registration). All registration now goes through POST /api/v1/accounts/register, and API keys are created via POST /api/v1/accounts/me/api-keys (JWT-authenticated).

Removed: RegisterController, RegisterRequest, RegisterResponse, RegisterControllerTest (8 tests). Updated: SecurityConfig, ApiKeyAuthFilter, ApiKeyService (removed register() method), ApiKey entity, FactionControllerTest, ApiKeyServiceTest, README. 69 tests pass, 0 CodeQL alerts.

@copilot Backwards compatibility is not a concern here as this is all new

Understood — since this is all new code, I'm not maintaining backwards compatibility with any prior API contract. The standalone POST /api/v1/register endpoint has been removed in 582166d to consolidate to a single register endpoint at POST /api/v1/accounts/register.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 47 out of 48 changed files in this pull request and generated 5 comments.

Comment on lines +64 to +70
if (res.ok) {
setProfile(await res.json())
} else if (res.status === 401) {
setToken(null)
localStorage.removeItem('dpc-token')
setError('Session expired. Please log in again.')
}
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchProfile only handles res.ok and 401; for other non-OK statuses it silently does nothing, leaving the UI without feedback. Consider handling the else case by setting an error message (and optionally clearing profile) so failed requests don’t appear to succeed.

Copilot uses AI. Check for mistakes.
Comment on lines +182 to +186
const copyToClipboard = (text: string) => {
navigator.clipboard.writeText(text)
setSuccess('Copied to clipboard!')
setTimeout(() => setSuccess(null), 2000)
}
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

navigator.clipboard.writeText returns a Promise and can reject (e.g., insecure context/permissions). This call isn’t awaited or caught, which can cause unhandled promise rejections; handle the Promise and surface a failure message when copy isn’t available.

Copilot uses AI. Check for mistakes.
Comment on lines +72 to +79
private String sha256(String input) {
try {
MessageDigest digest = MessageDigest.getInstance("SHA-256");
byte[] hash = digest.digest(input.getBytes(StandardCharsets.UTF_8));
return HexFormat.of().formatHex(hash);
} catch (NoSuchAlgorithmException e) {
throw new IllegalStateException("SHA-256 not available", e);
}
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AccountService re-implements SHA-256 hashing logic that already exists in ApiKeyService, creating duplicated security-critical code paths. Consolidate hashing into a single shared method/service so changes (e.g., salting/encoding) can’t accidentally diverge.

Copilot uses AI. Check for mistakes.
Comment on lines +23 to +28
public JwtService(
@Value("${dpc.jwt.secret}") String secret,
@Value("${dpc.jwt.expiration:24h}") Duration expiration) {
this.key = Keys.hmacShaKeyFor(secret.getBytes(StandardCharsets.UTF_8));
this.expiration = expiration;
}
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keys.hmacShaKeyFor(...) will throw at runtime if dpc.jwt.secret is too short for the chosen HMAC algorithm; right now that failure will be a somewhat opaque startup exception. Add an explicit length/strength check with a clear error message so misconfiguration is easier to diagnose.

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +9
ALTER TABLE api_keys ADD COLUMN owner_id UUID REFERENCES accounts(id);
CREATE INDEX idx_api_keys_owner_id ON api_keys (owner_id);
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

owner_id is nullable in the schema, but the application logic assumes API keys are account-owned (e.g., deletion checks key.getOwner() != null). Consider enforcing owner_id as NOT NULL (after any needed backfill) to prevent creating keys that can’t be managed through the account APIs.

Copilot uses AI. Check for mistakes.
@dmccoystephenson
Copy link
Copy Markdown
Member

@copilot address comments and ensure that the backend has a global exception handler

…WT secret validation, NOT NULL owner_id, UI fixes

Agent-Logs-Url: https://github.com/Dans-Plugins/dansplugins-dot-com/sessions/f424f365-3ce5-4f97-8046-71bbd9014ce0

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 28, 2026

@copilot address comments and ensure that the backend has a global exception handler

All 5 review comments addressed and global exception handler added in e9de2d3:

  1. fetchProfile → handles all non-OK HTTP statuses (not just 200/401)
  2. copyToClipboard → properly awaits clipboard Promise with error handling
  3. SHA-256 hashing → consolidated into shared HashUtil utility (removed duplication between AccountService and ApiKeyService)
  4. JWT secret → validates minimum 32-byte length at startup with clear error message
  5. owner_id → NOT NULL constraint in V3 migration, entity updated with optional = false

Global exception handler (GlobalExceptionHandler.java): @RestControllerAdvice with handlers for MethodArgumentNotValidException, HandlerMethodValidationException, ConstraintViolationException, HttpMessageNotReadableException, MethodArgumentTypeMismatchException, IllegalArgumentException, and a catch-all Exception handler. All return structured JSON error responses with timestamp, status, error, and message.

69 tests pass, 0 CodeQL alerts.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 49 out of 50 changed files in this pull request and generated 4 comments.

Comment on lines +28 to +48
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
http
.csrf(csrf -> csrf.disable())
.sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
.exceptionHandling(ex -> ex.authenticationEntryPoint(
new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED)))
.authorizeHttpRequests(auth -> auth
// Public endpoints
.requestMatchers(HttpMethod.POST, "/api/v1/accounts/register").permitAll()
.requestMatchers(HttpMethod.POST, "/api/v1/accounts/login").permitAll()
.requestMatchers(HttpMethod.GET, "/api/v1/factions/**").permitAll()
// Swagger/OpenAPI
.requestMatchers("/swagger-ui/**", "/swagger-ui.html", "/v3/api-docs/**").permitAll()
// Account endpoints require JWT auth
.requestMatchers("/api/v1/accounts/me/**").authenticated()
// Write endpoints require API key (handled by ApiKeyAuthFilter)
.requestMatchers(HttpMethod.POST, "/api/v1/factions").permitAll()
.anyRequest().permitAll()
)
.addFilterBefore(jwtAuthFilter, UsernamePasswordAuthenticationFilter.class)
.addFilterAfter(apiKeyAuthFilter, JwtAuthFilter.class);
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The frontend calls the API on a different origin by default (API_BASE falls back to http://localhost:8080 while the site runs on :3000). The API currently has no CORS configuration, so browser requests from the website will be blocked. Add explicit CORS support (e.g., enable http.cors() and provide a CorsConfigurationSource that allows the website origin(s) and required headers like Authorization and X-API-Key).

Copilot uses AI. Check for mistakes.
Comment on lines +60 to +66
if (WRITE_METHODS.contains(request.getMethod())) {
String providedKey = request.getHeader(API_KEY_HEADER);
if (providedKey == null || !apiKeyService.isValidKey(providedKey)) {
response.setStatus(HttpStatus.UNAUTHORIZED.value());
response.setContentType("application/json");
response.getWriter().write("{\"error\":\"Invalid or missing API key\"}");
return;
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This filter writes a hard-coded JSON body ("{"error":...}") directly to the response for invalid/missing API keys, which is inconsistent with the structured error format returned elsewhere (GlobalExceptionHandler). Consider routing this through Spring’s exception handling (e.g., throw an authentication/authorization exception) or use a shared error-response builder/ObjectMapper so clients get consistent fields (timestamp/status/message) and content-type/encoding are handled uniformly.

Copilot uses AI. Check for mistakes.
Comment on lines +298 to +300
<IconButton size="small" onClick={() => copyToClipboard(newApiKey)} sx={{ml: 1}}>
<ContentCopyIcon fontSize="small"/>
</IconButton>
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The copy IconButton is missing an accessible label. Add an aria-label (and/or title) so screen readers can announce the action (e.g., "Copy API key to clipboard").

Copilot uses AI. Check for mistakes.
Comment on lines +311 to +313
<IconButton edge="end" onClick={() => handleDeleteApiKey(key.id)}>
<DeleteIcon/>
</IconButton>
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delete IconButton is missing an accessible label. Add an aria-label (and/or title) so assistive tech users know this deletes the API key.

Copilot uses AI. Check for mistakes.
@dmccoystephenson
Copy link
Copy Markdown
Member

@copilot address comments

Copilot AI and others added 2 commits March 29, 2026 03:15
…-labels on IconButtons

Agent-Logs-Url: https://github.com/Dans-Plugins/dansplugins-dot-com/sessions/b3ede653-7dcc-4c9f-907c-75f4a75ab9fb

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
…redential support

Agent-Logs-Url: https://github.com/Dans-Plugins/dansplugins-dot-com/sessions/b3ede653-7dcc-4c9f-907c-75f4a75ab9fb

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 29, 2026

@copilot address comments

All 4 review comments addressed in 19ff1a1 and 5d001fc:

  1. CORSCorsConfigurationSource bean in SecurityConfig with configurable dpc.cors.allowed-origins property (defaults to http://localhost:3000). Allows Authorization, Content-Type, and X-API-Key headers. Uses allowedOriginPatterns for safe wildcard+credential support with whitespace trimming.
  2. ApiKeyAuthFilter → error responses now use structured JSON body matching GlobalExceptionHandler format (timestamp/status/error/message) via Jackson ObjectMapper instead of hard-coded string.
  3. Copy IconButton → added aria-label="Copy API key to clipboard"
  4. Delete IconButton → added aria-label="Delete API key"

69 tests pass, 0 CodeQL alerts.

@dmccoystephenson
Copy link
Copy Markdown
Member

@copilot Ran into the following while building Docker image

=> ERROR [dpc-website_dpc-api builder 6/6] RUN ./mvnw -DskipTests package

Add maven wrapper to prevent this

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 29, 2026

@copilot Ran into the following while building Docker image

=> ERROR [dpc-website_dpc-api builder 6/6] RUN ./mvnw -DskipTests package

...

Added the Maven wrapper JAR (v3.3.4) to .mvn/wrapper/ and removed distributionType=only-script from maven-wrapper.properties in 261b2e2. The Alpine-based Docker image was failing because only-script mode requires curl/wget to download Maven, but Alpine doesn't have either. With the wrapper JAR included, ./mvnw works without network download tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants