Skip to content

Conversation

GuilhermeOchoa
Copy link
Contributor

📍 Título: Correção de endpoints de autenticação JWT (login, logout e refresh token)

📌 Descrição:
Esta PR corrige problemas de validação nos endpoints de autenticação JWT, garantindo o funcionamento adequado dos fluxos de login, logout e refresh token. Inclui correções na validação de tokens, organização dos filtros de segurança e melhorias na cobertura de testes.

🛠️ O que foi feito?

  • Correção de bug
  • Melhoria de testes
  • Implementação de nova funcionalidade
  • Refatoração de código

🔍 Arquivos modificados:

  • pointtils/src/main/java/com/pointtils/pointtils/src/application/controllers/JwtController.java
  • pointtils/src/main/java/com/pointtils/pointtils/src/application/dto/RefreshTokenRequestDTO.java
  • pointtils/src/main/java/com/pointtils/pointtils/src/application/dto/TokensDTO.java
  • pointtils/src/test/java/com/pointtils/pointtils/src/application/controllers/AuthControllerTest.java
  • pointtils/src/main/java/com/pointtils/pointtils/src/infrastructure/configs/JwtAuthenticationFilter.java
  • pointtils/src/main/java/com/pointtils/pointtils/src/infrastructure/configs/SecurityConfiguration.java

🧪 Testes realizados:

  • Testes unitários do AuthController (20 testes passando)
  • Testes manuais com curl para verificar respostas HTTP
  • Validação de tokens de acesso e refresh
  • Testes de logout com blacklist de tokens
  • Cobertura de testes: 91% instruções, 70% branches

👀 Problemas resolvidos:

  • Validação incorreta de tokens no endpoint de logout
  • Organização inadequada dos filtros de segurança
  • Respostas HTTP inconsistentes (200/401)
  • Permissão para logout com tokens já invalidados

Checklist

  • Testes foram atualizados e passando
  • Endpoints retornam códigos HTTP corretos
  • Tokens são invalidados corretamente na blacklist
  • Fluxo completo de autenticação validado

📎 Referências

Status dos endpoints:

  • ✅ POST /v1/auth/login - 200 OK
  • ✅ POST /v1/auth/refresh - 200 OK
  • ✅ POST /v1/auth/logout - 200 OK (1ª vez), 401 Unauthorized (reações)
  • ✅ Validação adequada de tokens JWT
  • ✅ Blacklist funcionando corretamente

…tamente

- Corrigir problema de validação no endpoint de logout
- Reorganizar ordem dos filtros no SecurityConfiguration
- Permitir logout mesmo com tokens na blacklist
- Garantir respostas HTTP adequadas (200 OK/401 Unauthorized)
- Melhorar cobertura de testes dos endpoints de autenticação

Resolve: #35 US03 - Criar endpoint de login e logout
- Corrigir problema de validação no endpoint de logout
- Reorganizar ordem dos filtros no SecurityConfiguration
- Permitir logout mesmo com tokens na blacklist
- Garantir respostas HTTP adequadas (200 OK/401 Unauthorized)
- Melhorar cobertura de testes dos endpoints de autenticação
- Adequar DTOs para validação correta dos tokens

Resolve: #35 US03 - Criar endpoint de login e logout
@GuilhermeOchoa GuilhermeOchoa requested a review from a team as a code owner September 13, 2025 17:41
@Copilot Copilot AI review requested due to automatic review settings September 13, 2025 17:41
Copy link
Contributor

@Copilot 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

This PR fixes JWT authentication endpoints by standardizing token field names to snake_case format and improving token validation logic. The changes ensure consistency between API contracts and improve the authentication flow for login, logout, and refresh token operations.

Key changes:

  • Standardized JSON field names from camelCase to snake_case for token fields
  • Enhanced refresh token validation with proper null checks and expiration validation
  • Fixed JWT filter ordering and blacklist checking logic for logout endpoints

Reviewed Changes

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

Show a summary per file
File Description
AuthControllerTest.java Updated test assertions to use snake_case token field names (access_token, refresh_token)
SecurityConfiguration.java Moved JWT filter registration after authorization rules configuration
JwtAuthenticationFilter.java Added logic to skip blacklist validation for logout endpoint requests
TokensDTO.java Added @JsonProperty annotations to map camelCase fields to snake_case JSON
RefreshTokenRequestDTO.java Added @JsonProperty annotation for refresh_token field mapping
JwtController.java Enhanced refresh token validation with null checks and token expiration validation
AuthController.java Updated refresh endpoint to accept RefreshTokenRequestDTO with validation

Comment on lines 41 to 45
// Verificar se o token está na blacklist apenas se não for uma requisição de logout
String requestURI = request.getRequestURI();
if (!requestURI.equals("/v1/auth/logout")) {
String token = authHeader.substring(7);
if (memoryBlacklistService.isBlacklisted(token)) {
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

The hardcoded URI path comparison creates a security vulnerability. If the logout endpoint path changes or if there are other logout-related endpoints, tokens could bypass blacklist validation. Consider using a more robust approach like checking if the request matches logout endpoints using Spring's RequestMatcher or maintaining a list of excluded paths.

Copilot uses AI. Check for mistakes.

throw new com.pointtils.pointtils.src.core.domain.exceptions.AuthenticationException("Refresh token não fornecido");
}

if (!jwtService.validateToken(request.getRefreshToken()) || jwtService.isTokenExpired(request.getRefreshToken())) {
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

The refresh token is being validated twice - once in validateToken() and again in isTokenExpired(). This causes redundant token parsing and validation. Consider combining these checks or using a single method that returns validation details including expiration status.

Copilot uses AI. Check for mistakes.

Comment on lines 44 to 49
String token = authHeader.substring(7);
if (memoryBlacklistService.isBlacklisted(token)) {
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
response.getWriter().write("Token inválido");
return;
}
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

The token extraction logic authHeader.substring(7) is duplicated on lines 44 and 53. Extract this to a variable at the beginning of the method to avoid duplication and ensure consistency.

Copilot uses AI. Check for mistakes.

- Corrigir problema de validação no endpoint de logout
- Reorganizar ordem dos filtros no SecurityConfiguration
- Permitir logout mesmo com tokens na blacklist
- Garantir respostas HTTP adequadas (200 OK/401 Unauthorized)
- Melhorar cobertura de testes dos endpoints de autenticação
- Adequar DTOs para validação correta dos tokens

Resolve: #35 US03 - Criar endpoint de login e logout
2. AuthService.java - Consolidated token validation
3. JwtAuthenticationFilter.java - Extracted duplicated token extraction logic (my change)
4. JwtService.java - Added new isTokenValid() method
5. AuthServiceTest.java - Updated tests
Copy link
Contributor

@Copilot 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 10 out of 10 changed files in this pull request and generated 3 comments.


// O método logout deve chamar addToBlacklist internamente
// Podemos verificar isso através do comportamento esperado
// Podemos verificar isso através del comportamento esperado
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

There's a typo in the Portuguese comment. 'del' should be 'do' (meaning 'of the' in Portuguese).

Suggested change
// Podemos verificar isso através del comportamento esperado
// Podemos verificar isso através do comportamento esperado

Copilot uses AI. Check for mistakes.

return requestURI != null && (
requestURI.equals("/v1/auth/logout") ||
requestURI.startsWith("/v1/auth/logout/") ||
requestURI.matches(".*/logout.*")
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

The regex pattern .*/logout.* is too permissive and could match unintended endpoints like /v1/some-logout-data/protected. Consider using a more specific pattern like ^/v\\d+/auth/logout/?$ to only match exact logout endpoints.

Suggested change
requestURI.matches(".*/logout.*")
requestURI.matches("^/v\\d+/auth/logout/?$")

Copilot uses AI. Check for mistakes.

Comment on lines +105 to +118
public boolean isTokenValid(String token) {
try {
Claims claims = Jwts.parserBuilder()
.setSigningKey(getSignInKey())
.build()
.parseClaimsJws(token)
.getBody();

// Check if token is expired
return !claims.getExpiration().before(new Date());
} catch (Exception e) {
return false;
}
}
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

The method catches all exceptions with a generic Exception which may hide specific JWT validation issues. Consider catching specific JWT exceptions (JwtException, ExpiredJwtException, etc.) and logging the specific error for better debugging while still returning false.

Copilot uses AI. Check for mistakes.

…utenticação

- Substitui regex vulnerável .*/logout.* por contains() mais seguro
- Adiciona 5 novos testes para JwtAuthenticationFilter
- Corrige teste de logout para verificar ambos tokens na blacklist
- Garante que access e refresh tokens sejam adicionados à blacklist no logout
…utenticação

- Substitui regex vulnerável .*/logout.* por contains() mais seguro
- Adiciona 5 novos testes para JwtAuthenticationFilter
- Corrige teste de logout para verificar ambos tokens na blacklist
- Garante que access e refresh tokens sejam adicionados à blacklist no logout
@GuilhermeOchoa GuilhermeOchoa added this to the Sprint 2 milestone Sep 18, 2025
Copy link
Contributor

@Copilot 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 12 out of 12 changed files in this pull request and generated 3 comments.

Comment on lines +85 to +86
requestURI.startsWith("/v1/auth/logout/") ||
requestURI.contains("/logout")
Copy link

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

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

A verificação requestURI.contains("/logout") é muito permissiva e pode ser explorada por atacantes criando URIs como /malicious/logout/endpoint que passariam pela validação. Recomendo usar verificação mais específica como requestURI.matches("^/v\\d+/auth/logout/?$") ou manter apenas as comparações exatas.

Suggested change
requestURI.startsWith("/v1/auth/logout/") ||
requestURI.contains("/logout")
requestURI.startsWith("/v1/auth/logout/")

Copilot uses AI. Check for mistakes.

throw new com.pointtils.pointtils.src.core.domain.exceptions.AuthenticationException("Refresh token inválido ou expirado");
}

String username = jwtService.getEmailFromToken(request.getRefreshToken());
Copy link

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

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

O método getEmailFromToken() não é chamado após a validação do token em isTokenValid(). Se o token for inválido, este método pode falhar ou retornar dados incorretos. Recomendo validar se o método getEmailFromToken() trata adequadamente tokens inválidos ou mover esta chamada para dentro de um bloco try-catch.

Suggested change
String username = jwtService.getEmailFromToken(request.getRefreshToken());
String username;
try {
username = jwtService.getEmailFromToken(request.getRefreshToken());
} catch (Exception e) {
throw new com.pointtils.pointtils.src.core.domain.exceptions.AuthenticationException("Falha ao extrair usuário do refresh token: " + e.getMessage());
}

Copilot uses AI. Check for mistakes.

}

memoryBlacklistService.addToBlacklist(accessToken);
memoryBlacklistService.addToBlacklist(refreshToken);
Copy link

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

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

Adicionar o refresh token à blacklist pode ser problemático se o mesmo refresh token for usado para gerar novos access tokens. Considere se é necessário invalidar o refresh token imediatamente ou se deveria ser invalidado apenas quando expirar naturalmente, dependendo da estratégia de rotação de tokens do sistema.

Suggested change
memoryBlacklistService.addToBlacklist(refreshToken);
// Do not blacklist the refresh token here; let it expire naturally or handle via rotation if needed.

Copilot uses AI. Check for mistakes.

@Bialves
Copy link
Contributor

Bialves commented Sep 20, 2025

@GuilhermeOchoa tudo bem?

Preciso das correções desse PR o quanto antes em dev, pois ela tem ajustes cruciais do login que está barrando outras tarefas (difícil acessar certas telas do frontend sem o fluxo de login funcionando, o que torna necessário nós alterarmos manualmente o código - hardcoded)

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
61.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@femelloffm femelloffm merged commit 6d62bdb into dev Sep 20, 2025
2 of 3 checks passed
GuilhermeOchoa pushed a commit that referenced this pull request Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Criar endpoint para verificar se as credenciais do usuário são válidas US03 - Criar endpoint de login e logout

4 participants