-
Notifications
You must be signed in to change notification settings - Fork 0
Add Spring Security with JWT authentication and environment variable support #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…t and add authentication manager
…te SecurityFilter
… in login response
…for API integration
…rove token validation error handling
…ronment variable management
…ables from .env file
…dEncoder instance
Remove unused import Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
WalkthroughThis update introduces user authentication and security features using Spring Security and JWT. It adds new DTOs, controllers, and security services, updates the project documentation, adjusts configuration to support environment variables via dotenv, and reorganizes API endpoints. The build configuration receives new dependencies for security and environment management, and some legacy configuration files are removed or replaced. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant AuthManager
participant UserRepo
participant TokenService
Client->>AuthController: POST /api/auth/login (login, password)
AuthController->>AuthManager: authenticate(login, password)
AuthManager->>UserRepo: findByLogin(login)
UserRepo-->>AuthManager: UserDetails / null
AuthManager-->>AuthController: Auth result
alt Success
AuthController->>TokenService: generateToken(User)
TokenService-->>AuthController: JWT token
AuthController-->>Client: 200 OK (token, login, role)
else Failure
AuthController-->>Client: 401 Unauthorized
end
sequenceDiagram
participant Client
participant SecurityFilter
participant TokenService
participant UserRepo
participant SecurityContext
Client->>SecurityFilter: Any request with Authorization: Bearer <token>
SecurityFilter->>TokenService: validateToken(token)
TokenService-->>SecurityFilter: login / error
alt Valid login
SecurityFilter->>UserRepo: findByLogin(login)
UserRepo-->>SecurityFilter: UserDetails / null
SecurityFilter->>SecurityContext: setAuthentication(UserDetails)
end
SecurityFilter-->>Client: Continue request processing
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
src/main/java/com/otavio/aifoodapp/model/User.java (1)
33-36:⚠️ Potential issueUse String-Based Enum Mapping
Therolefield lacks an@Enumerated(EnumType.STRING)annotation, causing JPA to default to ordinal storage (fragile if enum order changes).Apply this diff to ensure string mapping:
import com.otavio.aifoodapp.enums.UserRoles; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; ... - private UserRoles role; + @Enumerated(EnumType.STRING) + private UserRoles role;
🧹 Nitpick comments (10)
src/main/java/com/otavio/aifoodapp/dto/RegisterDTO.java (1)
5-9: Add Validation Annotations
Consider enforcing input constraints at the DTO level (e.g.,@NotBlankonloginandpassword,@NotNullonrole) to catch invalid data before hitting the service layer.src/main/java/com/otavio/aifoodapp/model/User.java (1)
55-58: Add Braces for Clarity
Wrap theif/elseblocks in braces to improve readability and avoid mistakes when extending logic:@Override public Collection<? extends GrantedAuthority> getAuthorities() { - if (this.role == UserRoles.ADMIN) - return List.of(new SimpleGrantedAuthority("ROLE_ADMIN"), new SimpleGrantedAuthority("ROLE_USER")); - else return List.of(new SimpleGrantedAuthority("ROLE_USER")); + if (this.role == UserRoles.ADMIN) { + return List.of( + new SimpleGrantedAuthority("ROLE_ADMIN"), + new SimpleGrantedAuthority("ROLE_USER") + ); + } else { + return List.of(new SimpleGrantedAuthority("ROLE_USER")); + } }src/main/java/com/otavio/aifoodapp/dto/AuthenticationDTO.java (1)
1-4: Record Definition is Appropriate
Using a Java record for the authentication payload is concise and immutable. Consider adding validation annotations (@NotBlank) on its components if you need request-level validation.src/main/java/com/otavio/aifoodapp/dto/LoginResponseDTO.java (1)
1-4: Login Response DTO Setup
The record encapsulates the JWT token, username, and role. For type safety and consistency, you could use theUserRolesenum instead ofStringfor therolefield.src/main/java/com/otavio/aifoodapp/repository/UserRepository.java (1)
11-11: Missing@Transactional(readOnly = true)Read-only repository methods benefit from marking the outer service call as
@Transactional(readOnly = true)for performance and to avoid unintended writes.
Add it inAuthorizationService.loadUserByUsernameif no higher-level transaction is active.src/main/java/com/otavio/aifoodapp/service/AuthorizationService.java (1)
21-27:nullhandling is fine, but anOptionalavoids the extra checkThe current code is correct, yet you could keep the previous
Optionalcontract and streamline the flow:-UserDetails user = userRepository.findByLogin(username); -if (user == null) { - throw new UsernameNotFoundException("User not found with username: " + username); -} -return user; +return userRepository.findByLogin(username) + .orElseThrow(() -> + new UsernameNotFoundException("User not found with username: " + username));No functional change, but it reads tighter and removes the possibility of forgetting the null-check elsewhere.
src/main/java/com/otavio/aifoodapp/config/DotenvEnvironmentPostProcessor.java (1)
17-21:.envvalues now override OS env vars – risky for prod
environment.getPropertySources().addFirst(...)gives.envhighest precedence, so a stale local file silently overrides real secrets set on the host.Safer order:
- environment.getPropertySources().addFirst(new MapPropertySource("dotenv", envMap)); + environment.getPropertySources().addLast(new MapPropertySource("dotenv", envMap));This keeps
.envas a fallback while still allowing container/orchestration-level variables to win.README.MD (1)
19-29: Minor PT-BR agreement issue“Framework para aplicações Java” → “Framework para aplicação Java”
(Not blocking, just polishing copy.)🧰 Tools
🪛 LanguageTool
[grammar] ~19-~19: Possível erro de concordância de número.
Context: ...adas - Spring Boot: Framework para aplicações Java - Spring Data JPA: Persistência de ...(GENERAL_NUMBER_AGREEMENT_ERRORS)
src/main/java/com/otavio/aifoodapp/controller/AuthenticationController.java (2)
40-47: Expose authorities as rawtoString()
auth.getAuthorities().toString()leaks internal representation ([ROLE_USER]).
Return a structured list instead:-return ResponseEntity.ok(new LoginResponseDTO(token, data.login(), auth.getAuthorities().toString())); +var roles = auth.getAuthorities() + .stream() + .map(GrantedAuthority::getAuthority) + .toList(); +return ResponseEntity.ok(new LoginResponseDTO(token, data.login(), roles));
49-59:registerendpoint should return 201 + LocationSemantically this creates a resource; prefer
201 Createdand perhaps echo the user id.-userRepository.save(newUser); -return ResponseEntity.ok().build(); +User saved = userRepository.save(newUser); +URI location = URI.create("/api/users/" + saved.getId()); +return ResponseEntity.created(location).build();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
README.MD(1 hunks)pom.xml(1 hunks)src/main/java/com/otavio/aifoodapp/AiFoodAppApplication.java(1 hunks)src/main/java/com/otavio/aifoodapp/config/CorsConfig.java(0 hunks)src/main/java/com/otavio/aifoodapp/config/DotenvEnvironmentPostProcessor.java(1 hunks)src/main/java/com/otavio/aifoodapp/config/SecurityConfig.java(0 hunks)src/main/java/com/otavio/aifoodapp/controller/AuthenticationController.java(1 hunks)src/main/java/com/otavio/aifoodapp/dto/AuthenticationDTO.java(1 hunks)src/main/java/com/otavio/aifoodapp/dto/LoginResponseDTO.java(1 hunks)src/main/java/com/otavio/aifoodapp/dto/RegisterDTO.java(1 hunks)src/main/java/com/otavio/aifoodapp/model/User.java(2 hunks)src/main/java/com/otavio/aifoodapp/repository/UserRepository.java(1 hunks)src/main/java/com/otavio/aifoodapp/security/SecurityConfig.java(1 hunks)src/main/java/com/otavio/aifoodapp/security/SecurityFilter.java(1 hunks)src/main/java/com/otavio/aifoodapp/security/TokenService.java(1 hunks)src/main/java/com/otavio/aifoodapp/service/AuthorizationService.java(1 hunks)src/main/resources/META-INF/spring.factories(1 hunks)src/main/resources/application.properties(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/otavio/aifoodapp/config/CorsConfig.java
- src/main/java/com/otavio/aifoodapp/config/SecurityConfig.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/otavio/aifoodapp/controller/AuthenticationController.java (1)
src/main/java/com/otavio/aifoodapp/dto/RegisterDTO.java (1)
RegisterDTO(5-27)
🪛 LanguageTool
README.MD
[grammar] ~19-~19: Possível erro de concordância de número.
Context: ...adas - Spring Boot: Framework para aplicações Java - Spring Data JPA: Persistência de ...
(GENERAL_NUMBER_AGREEMENT_ERRORS)
🔇 Additional comments (8)
src/main/java/com/otavio/aifoodapp/AiFoodAppApplication.java (1)
8-8: Approve minor formatting change
Removing the extra spaces inside the{}in theexcludeattribute is purely stylistic and has no impact on functionality.src/main/java/com/otavio/aifoodapp/dto/RegisterDTO.java (2)
10-14: Constructor Implementation is Correct
The parameterized constructor properly initializes all fields, aligning with the DTO’s purpose. The use offinalfields ensures immutability.
20-26: Sensitive Field Exclusion intoString()
Great job omitting the password fromtoString(). This prevents accidental logging of sensitive data.src/main/java/com/otavio/aifoodapp/model/User.java (1)
37-41: Additional Constructor for Registration
The new constructor correctly initializeslogin,password, androle, supporting the registration flow.src/main/resources/META-INF/spring.factories (1)
1-2: RegisterDotenvEnvironmentPostProcessor
The entry correctly binds the custom environment post-processor for loading.envfiles at startup.src/main/java/com/otavio/aifoodapp/security/SecurityConfig.java (1)
47-50: Expose the samePasswordEncoderbean id used byAuthenticationManagerIf you later rely on
@Bean PasswordEncoder passwordEncoder()injection elsewhere, name clashes are fine; however make sure the encoder is referenced in user-creation flow (UserDetailsService/ registration).src/main/java/com/otavio/aifoodapp/security/TokenService.java (1)
17-18: Makesecretfinal & validate presence
secretis injected but never validated. If the env/property is missing the app will start and NPE at first call.
Mark the fieldfinaland assert it’s non-blank in a@PostConstruct.pom.xml (1)
32-39: Mixingspring-boot-starter-webandwebfluxoften causes servlet/reactive clashesRunning both starters drags in two HTTP engines (Tomcat & Netty) and duplicate
DispatcherHandlers.
Unless you truly need hybrid mode, keep only one to avoid ambiguous bean errors.
| public String login() {return login;} | ||
| public String password() { return password; | ||
| } | ||
| public UserRoles role() {return role;} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Syntax Error in password() Getter
The password() method is missing its closing brace, causing a compilation error.
Apply this diff to fix the getter:
- public String password() { return password;
-}
+ public String password() {
+ return password;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String login() {return login;} | |
| public String password() { return password; | |
| } | |
| public UserRoles role() {return role;} | |
| public String login() {return login;} | |
| public String password() { | |
| return password; | |
| } | |
| public UserRoles role() {return role;} |
🤖 Prompt for AI Agents
In src/main/java/com/otavio/aifoodapp/dto/RegisterDTO.java between lines 15 and
18, the password() getter method is missing its closing brace, causing a syntax
error. Add the missing closing brace at the end of the password() method to
properly close the method and fix the compilation error.
| public interface UserRepository extends JpaRepository<User, Long> { | ||
|
|
||
|
|
||
| UserDetails findByLogin(String login); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Return type may confuse JPA projections
Spring Data usually returns the entity type (User) for finder methods.
Returning UserDetails works only if User implements that interface, but it obscures the intent and prevents using repository-level type safety for writes/updates.
Consider:
-UserDetails findByLogin(String login);
+User findByLogin(String login); // keeps repo bound to the entityLet the service layer convert User → UserDetails (or simply return the entity if it implements the interface).
This keeps repository concerns (persistence) separate from security concerns.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public interface UserRepository extends JpaRepository<User, Long> { | |
| UserDetails findByLogin(String login); | |
| public interface UserRepository extends JpaRepository<User, Long> { | |
| // keeps repo bound to the entity | |
| User findByLogin(String login); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/otavio/aifoodapp/repository/UserRepository.java around
lines 8 to 12, the method findByLogin returns UserDetails, which can confuse JPA
projections and reduce type safety. Change the return type of findByLogin to
User to keep repository concerns focused on persistence. Then, handle the
conversion from User to UserDetails in the service layer to separate persistence
logic from security concerns.
| logging.level.org.springframework.security=TRACE | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
TRACE security logging exposes sensitive information
logging.level.org.springframework.security=TRACE dumps credentials, JWTs and headers to logs.
Keep it only for local debugging and override to INFO (or remove) for any non-dev profile.
🤖 Prompt for AI Agents
In src/main/resources/application.properties at lines 21 to 22, the logging
level for org.springframework.security is set to TRACE, which exposes sensitive
information such as credentials, JWTs, and headers. Change this logging level to
INFO or remove the line entirely for any non-development profiles to prevent
sensitive data from being logged in production environments. Keep TRACE level
only for local debugging purposes.
| # Adicione esta propriedade para carregar o segredo do JWT | ||
| api.security.token.secret=${JWT_SECRET:dever-ser-definido-no-dotenv} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not ship a default JWT secret
api.security.token.secret=${JWT_SECRET:dever-ser-definido-no-dotenv} means tests & careless deployments will run with the literal string, weakening security.
At minimum, fail fast when the env var is absent:
-api.security.token.secret=${JWT_SECRET:dever-ser-definido-no-dotenv}
+api.security.token.secret=${JWT_SECRET}and document the requirement in README.
🤖 Prompt for AI Agents
In src/main/resources/application.properties at lines 23-24, remove the default
JWT secret value to avoid using a weak literal string. Instead, configure the
application to fail fast or throw an error if the JWT_SECRET environment
variable is not set. Additionally, update the README to clearly document the
requirement to set this environment variable before running the application.
| .authorizeHttpRequests(authorize -> authorize | ||
| .requestMatchers(HttpMethod.POST, "/api/auth/login").permitAll() | ||
| .requestMatchers(HttpMethod.POST, "/api/auth/register").permitAll() | ||
| .anyRequest().authenticated() | ||
| ) | ||
| .addFilterBefore(securityFilter, UsernamePasswordAuthenticationFilter.class) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Pre-flight CORS / OPTIONS requests will be rejected
Because CORS config was removed, browsers send an OPTIONS pre-flight that now hits .anyRequest().authenticated() and returns 401/403.
Allow all OPTIONS and enable CORS:
+ .cors(AbstractHttpConfigurer::disable) // if you intentionally manage CORS elsewhere
.authorizeHttpRequests(authorize -> authorize
.requestMatchers(HttpMethod.POST, "/api/auth/login").permitAll()
.requestMatchers(HttpMethod.POST, "/api/auth/register").permitAll()
+ .requestMatchers(HttpMethod.OPTIONS, "/**").permitAll()
.anyRequest().authenticated()
)Alternatively, re-introduce a central @Bean CorsConfigurationSource.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/otavio/aifoodapp/security/SecurityConfig.java around lines
32 to 38, the current security configuration rejects pre-flight CORS OPTIONS
requests because they are not explicitly permitted and fall under
.anyRequest().authenticated(). To fix this, update the authorizeHttpRequests
configuration to explicitly permit all OPTIONS requests by adding a matcher for
HttpMethod.OPTIONS with permitAll(). Additionally, enable CORS support by
calling .cors() in the HttpSecurity configuration or reintroduce a central @Bean
CorsConfigurationSource to define CORS settings globally.
| .withExpiresAt(genExperationDate()) | ||
| .sign(algorithm); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Rename typo + rely on UTC for expiry
genExperationDate is miss-spelled and the fixed -03:00 offset bakes a regional assumption into every token.
-private Instant genExperationDate() {
- return LocalDateTime.now().plusHours(2).toInstant(ZoneOffset.of("-03:00"));
+private Instant generateExpirationInstant() {
+ return Instant.now().plus(Duration.ofHours(2)); // always UTC
}
...
-.withExpiresAt(genExperationDate())
+.withExpiresAt(generateExpirationInstant())Also applies to: 47-49
🤖 Prompt for AI Agents
In src/main/java/com/otavio/aifoodapp/security/TokenService.java around lines
26-28 and 47-49, rename the method genExperationDate to genExpirationDate to fix
the typo. Additionally, update the expiration date calculation to use UTC time
instead of a fixed -03:00 offset to avoid embedding a regional assumption in the
token expiry. Adjust the code to generate the expiration date based on UTC time
zone consistently.
| public String validateToken(String token) { | ||
| try { | ||
| Algorithm algorithm = Algorithm.HMAC256(secret); | ||
| return JWT.require(algorithm) | ||
| .withIssuer("auth-api") | ||
| .build() | ||
| .verify(token) | ||
| .getSubject(); | ||
| } catch (JWTVerificationException verificationException) { | ||
| throw new RuntimeException("Invalid token", verificationException); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return 401 instead of 500 on invalid JWT
validateToken throws RuntimeException; the filter will bubble it up as a 500 error.
Wrap the call and, on JWTVerificationException, simply skip authentication so Spring Security turns the request into a 401.
🤖 Prompt for AI Agents
In src/main/java/com/otavio/aifoodapp/security/TokenService.java around lines 34
to 45, the validateToken method currently throws a RuntimeException on JWT
verification failure, causing a 500 error. Modify the method to not throw an
exception on JWTVerificationException; instead, catch the exception and return
null or an empty Optional to indicate invalid token. Then, in the authentication
filter that calls validateToken, check for this null or empty result and skip
setting authentication, allowing Spring Security to respond with a 401
Unauthorized instead of 500.
| private String recoverToken(HttpServletRequest request) { | ||
| var authHeader = request.getHeader("Authorization"); | ||
| if (authHeader == null) return null; | ||
| return authHeader.replace("Bearer ", ""); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Prefix check missing in recoverToken
Unconditionally calling replace("Bearer ", "") will also strip the word Bearer from malformed headers.
Add a prefix check:
if (!StringUtils.hasText(authHeader) || !authHeader.startsWith("Bearer "))
return null;
return authHeader.substring(7);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/otavio/aifoodapp/security/SecurityFilter.java around lines
46 to 50, the recoverToken method removes "Bearer " from the Authorization
header without verifying if the header actually starts with "Bearer ". To fix
this, add a check to confirm the header starts with "Bearer " before removing
it; if it does not, return null or handle it appropriately to avoid stripping
"Bearer" from malformed headers.
| var token = this.recoverToken(request); | ||
| if (token != null) { | ||
| var login = tokenService.validateToken(token); | ||
| if (login != null && !login.isEmpty()) { | ||
| var userExists = userRepository.findByLogin(login); | ||
| if (userExists != null) { | ||
| UserDetails user = userExists; | ||
| var authentication = new UsernamePasswordAuthenticationToken(user, null, user.getAuthorities()); | ||
| SecurityContextHolder.getContext().setAuthentication(authentication); | ||
| } | ||
| } | ||
| } | ||
| filterChain.doFilter(request, response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unhandled RuntimeException from token validation will short-circuit the filter chain
Catch RuntimeException around tokenService.validateToken(token) to avoid converting authentication failures into 500 responses.
try {
var login = tokenService.validateToken(token);
- if (login != null && !login.isEmpty()) {
+ if (StringUtils.hasText(login)) {
...
}
} catch (RuntimeException ex) {
// invalid token → leave SecurityContext empty
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var token = this.recoverToken(request); | |
| if (token != null) { | |
| var login = tokenService.validateToken(token); | |
| if (login != null && !login.isEmpty()) { | |
| var userExists = userRepository.findByLogin(login); | |
| if (userExists != null) { | |
| UserDetails user = userExists; | |
| var authentication = new UsernamePasswordAuthenticationToken(user, null, user.getAuthorities()); | |
| SecurityContextHolder.getContext().setAuthentication(authentication); | |
| } | |
| } | |
| } | |
| filterChain.doFilter(request, response); | |
| var token = this.recoverToken(request); | |
| if (token != null) { | |
| try { | |
| var login = tokenService.validateToken(token); | |
| if (StringUtils.hasText(login)) { | |
| var userExists = userRepository.findByLogin(login); | |
| if (userExists != null) { | |
| UserDetails user = userExists; | |
| var authentication = new UsernamePasswordAuthenticationToken(user, null, user.getAuthorities()); | |
| SecurityContextHolder.getContext().setAuthentication(authentication); | |
| } | |
| } | |
| } catch (RuntimeException ex) { | |
| // invalid token → leave SecurityContext empty | |
| } | |
| } | |
| filterChain.doFilter(request, response); |
🤖 Prompt for AI Agents
In src/main/java/com/otavio/aifoodapp/security/SecurityFilter.java around lines
31 to 43, the call to tokenService.validateToken(token) can throw a
RuntimeException that is currently unhandled, which would stop the filter chain
and cause a 500 error. Wrap the call to validateToken in a try-catch block that
catches RuntimeException, and handle the exception gracefully (e.g., by logging
it and skipping authentication) so the filter chain continues without
interruption.
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-maven-plugin</artifactId> | ||
| <configuration> | ||
| </configuration> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-resources-plugin</artifactId> | ||
| <version>3.3.1</version> | ||
| <configuration> | ||
| <encoding>UTF-8</encoding> | ||
| <nonFilteredFileExtensions> | ||
| <nonFilteredFileExtension>properties</nonFilteredFileExtension> | ||
| </nonFilteredFileExtensions> | ||
| </configuration> | ||
| </plugin> | ||
| </plugins> | ||
| </build> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Missing surefire plugin removes test execution from Maven lifecycle
The previous maven-surefire-plugin section is gone, so mvn test will silently skip unit tests on CI.
Re-add the plugin or enable Spring-Boot’s default surefire integration.
🤖 Prompt for AI Agents
In pom.xml around lines 122 to 142, the maven-surefire-plugin configuration is
missing, causing unit tests to be skipped during the Maven test phase. To fix
this, re-add the maven-surefire-plugin section under the build plugins with
appropriate configuration or ensure that Spring Boot's default surefire
integration is enabled so that tests run correctly during the Maven lifecycle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Olá @OtavioXimarelli - revisei suas alterações - aqui está algum feedback:
- A chave da propriedade secreta JWT (
api.security.token.secret) em TokenService não corresponde ao nome da variável.env(JWT_SECRET), então o segredo pode nunca ser injetado — alinhe esses nomes de propriedade. - Altere UserRepository.findByLogin para retornar um Optional (ou Optional) e trate-o em AuthorizationService para evitar possíveis NPEs em vez de depender de verificações nulas.
- Em SecurityFilter.recoverToken(), adicione uma verificação para o prefixo 'Bearer ' (por exemplo, startsWith) e trate cabeçalhos de Autorização inválidos ou malformados de forma mais defensiva.
Aqui está o que eu examinei durante a revisão
- 🟡 Problemas gerais: 2 problemas encontrados
- 🟢 Segurança: tudo parece bom
- 🟢 Teste: tudo parece bom
- 🟢 Complexidade: tudo parece bom
- 🟢 Documentação: tudo parece bom
Sourcery é gratuito para código aberto - se você gosta de nossas revisões, por favor, considere compartilhá-las ✨
Original comment in English
Hey @OtavioXimarelli - I've reviewed your changes - here's some feedback:
- The JWT secret property key (
api.security.token.secret) in TokenService doesn’t match the.envvariable name (JWT_SECRET), so the secret may never be injected—align these property names. - Switch UserRepository.findByLogin to return an Optional (or Optional) and handle it in AuthorizationService to avoid potential NPEs instead of relying on null checks.
- In SecurityFilter.recoverToken(), add a check for the ‘Bearer ’ prefix (e.g. startsWith) and handle invalid or malformed Authorization headers more defensively.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return JWT.create() | ||
| .withIssuer("auth-api") | ||
| .withSubject(user.getUsername()) | ||
| .withExpiresAt(genExperationDate()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): withExpiresAt espera um Date, não um Instant
Converta o Instant retornado por genExperationDate() para um Date usando Date.from(...) antes de passá-lo para withExpiresAt.
Original comment in English
issue (bug_risk): withExpiresAt expects a Date, not Instant
Convert the Instant returned by genExperationDate() to a Date using Date.from(...) before passing it to withExpiresAt.
| private String email; | ||
| private UserRoles role; | ||
|
|
||
| public User(String login, String password, UserRoles role) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Construtor sem argumentos ausente, requerido pelo JPA
Adicione um construtor sem argumentos protegido ou público para garantir a compatibilidade com JPA, ou use @NoArgsConstructor do Lombok.
Original comment in English
issue (bug_risk): Missing no-args constructor required by JPA
Add a protected or public no-arg constructor to ensure JPA compatibility, or use Lombok's @NoArgsConstructor.
PR Type
Enhancement
Description
• Implement comprehensive Spring Security with JWT authentication
• Add user registration and login endpoints with DTOs
• Configure environment variable loading via dotenv
• Restructure security configuration and add token service
Changes walkthrough 📝
1 files
Minor code formatting improvement5 files
Remove unused CORS configuration classAdd environment variable processor for dotenvRemove old security configuration fileRegister dotenv environment post processorConfigure environment variables and JWT settings10 files
Add authentication controller with login/register endpointsAdd DTO for user authentication dataAdd DTO for login response dataAdd DTO for user registration dataAdd constructor and improve code formattingUpdate method signature and import organizationImplement comprehensive security configuration with JWTAdd JWT authentication filter for request processingImplement JWT token generation and validation serviceUpdate user loading logic for authentication1 files
Update documentation for security and authentication features1 files
Add security dependencies and reorganize structureSummary by CodeRabbit
New Features
Configuration
.envfile for sensitive data.Documentation
Dependency Updates