Phase 1A: Identity & Access — carddemo-auth Spring Boot module - #210
devin-ai-integration[bot] wants to merge 5 commits into
Conversation
Add new Maven module services/carddemo-auth implementing:
- Spring Boot 3.2.x with Spring Security and JWT authentication
- JPA User entity mapped from COBOL copybook CSUSR01Y (SEC-USER-DATA)
- REST endpoints: POST /auth/login, GET/POST/PUT/DELETE /users
- Role-based access control (ADMIN vs USER roles)
- BCrypt password hashing (replacing COBOL plaintext storage)
- Unit tests for AuthService and UserService
- Integration tests for all endpoints with H2
- README documenting COBOL-to-Java mapping
Modernizes legacy COBOL programs:
- COSGN00C (sign-on) -> POST /auth/login
- COUSR00C (list) -> GET /users
- COUSR01C (add) -> POST /users
- COUSR02C (update) -> PUT /users/{id}
- COUSR03C (delete) -> DELETE /users/{id}
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| String userId = tokenProvider.getUserIdFromToken(token); | ||
| String role = tokenProvider.getRoleFromToken(token); | ||
|
|
||
| List<SimpleGrantedAuthority> authorities = List.of( | ||
| new SimpleGrantedAuthority("ROLE_" + role) | ||
| ); | ||
|
|
||
| UsernamePasswordAuthenticationToken authentication = | ||
| new UsernamePasswordAuthenticationToken(userId, null, authorities); | ||
| SecurityContextHolder.getContext().setAuthentication(authentication); |
There was a problem hiding this comment.
🚩 Deleted or role-changed users retain JWT-based access for up to 24 hours
The JwtAuthenticationFilter (JwtAuthenticationFilter.java:32-42) validates the JWT signature and expiration but never checks whether the user still exists in the database or whether their role has changed. This means if an admin deletes a user via DELETE /users/{id}, that user's JWT remains valid for up to 24 hours (application.yml:23 sets expiration-ms: 86400000). A deleted admin user could continue to create users, update any user, or delete other users until token expiry. This is an inherent trade-off of stateless JWT design (which the PR explicitly chose per the README's "Stateless Auth" section), but it's worth noting for an IAM service where user revocation may be expected to be immediate. Consider adding a token blacklist, a short-lived token + refresh token pattern, or a lightweight DB existence check in the filter if immediate revocation is a requirement.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — this is an intentional trade-off of stateless JWT architecture. For immediate revocation, a token blacklist or short-lived tokens + refresh flow can be added in a future iteration. The 24h TTL is configurable via app.jwt.expiration-ms.
There was a problem hiding this comment.
Acknowledged — as noted, this is an intentional trade-off of stateless JWT architecture documented in the README. For immediate revocation needs, a token blacklist (e.g., Redis-backed) or short-lived access tokens + refresh token pattern can be added in a future iteration. The 24h TTL is configurable via app.jwt.expiration-ms and can be lowered to reduce the revocation window.
- Guard against NPE in GlobalExceptionHandler when getFieldError() is null - Reject blank strings in UpdateUserRequest partial updates (treat as no-op) - Change ddl-auto from 'update' to 'validate' for production profile
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Add @transactional(readOnly=true) at class level, @transactional on mutating methods to prevent lost updates and TOCTOU races - Add DataIntegrityViolationException handler returning 409 Conflict as defense-in-depth for concurrent create races - Add @SiZe(max=72) on password fields to prevent BCrypt DoS via oversized inputs (BCrypt truncates at 72 bytes anyway)
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Add dummy BCrypt comparison when user not found to ensure constant-time response regardless of userId existence (prevents user enumeration) - Remove hardcoded default JWT secret from application.yml; app now requires JWT_SECRET env var to start (fail-fast in production)
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized")) | ||
| ) | ||
| .authorizeHttpRequests(auth -> auth | ||
| .requestMatchers(HttpMethod.POST, "/auth/login").permitAll() |
There was a problem hiding this comment.
🚩 No brute-force / rate-limiting protection on the login endpoint
The POST /auth/login endpoint is permitAll() (line 38 of SecurityConfig) with no rate limiting or account lockout mechanism. While this isn't strictly a code bug (it's a missing feature), it's worth flagging for a security-focused auth service: an attacker can make unlimited login attempts. The original COBOL COSGN00C program typically had lockout after N failed attempts. Consider adding Spring's rate limiting, a lockout counter, or delegating to an API gateway rate limiter.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — rate limiting / account lockout is a valid hardening concern. The original COBOL COSGN00C did enforce lockout after failed attempts. This can be added in a follow-up (e.g., a failed-attempt counter in the users table or an API gateway rate limiter like Bucket4j or Spring Cloud Gateway filters). Keeping this PR scoped to the core auth functionality for now.
Replace the malformed 51-char dummy hash with a proper 60-char BCrypt hash so that BCryptPasswordEncoder.matches() actually performs the full computation on the user-not-found path.
1 participant
Summary
Adds
services/carddemo-auth, a Spring Boot 3.2 module that replaces the COBOL sign-on and user-management programs with a stateless JWT-based REST API.COBOL → Java mapping:
COSGN00C(sign-on against VSAMUSRSEC)POST /auth/login→ returns JWTCOUSR00C(list users)GET /usersCOUSR01C(add user)POST /usersCOUSR02C(update user)PUT /users/{id}COUSR03C(delete user)DELETE /users/{id}Key design decisions:
Userentity mirrors copybookCSUSR01Y(SEC-USR-ID8-char PK,SEC-USR-FNAME/LNAME20-char,SEC-USR-TYPEA/U → enumADMIN/USER)PIC X(08))ADMINrole can CRUD all users;USERrole restricted to own profile (cannot escalate viauserTypefield)JwtAuthenticationFilterextractingBearertokens and settingSecurityContextAuthService, 8 unit forUserService, 11 integration covering auth flows + RBAC enforcement)Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/7fbaff67c2b94193a54161b7559bd27f
Requested by: @DhrovS