Skip to content

trying to fix bug changing in class - #268

Open
avdheshpatidar27 wants to merge 1 commit into
Vault-Web:mainfrom
avdheshpatidar27:fix/#260
Open

trying to fix bug changing in class#268
avdheshpatidar27 wants to merge 1 commit into
Vault-Web:mainfrom
avdheshpatidar27:fix/#260

Conversation

@avdheshpatidar27

Copy link
Copy Markdown

Summary

Fixed refresh token replay-attack detection in AuthService. Previously,
refresh() and logout() used findByTokenIdAndRevokedFalse(), which excluded
already-revoked tokens from the query — so a revoked (reused/replayed) token,
an expired token, and a token that never existed all returned the same generic
401 response with zero logging.

Changes:

  • Added RefreshTokenRepository.findByTokenId(String tokenId) — looks up a
    token by id without filtering out revoked tokens.
  • Added RefreshTokenRepository.revokeAllByUser(Long userId) — revokes all
    active refresh tokens for a user (defense-in-depth on detected replay).
  • Reworked AuthService.refresh() to distinguish three cases:
    1. Token id not found → normal 401
    2. Token found but already revoked → treated as a replay attack: logs a
      SECURITY ALERT, persists a SecurityEvent (new
      SecurityEventType.TOKEN_REPLAY_DETECTED), revokes all other sessions
      for the user, returns 401
    3. Token found, not revoked, but hash mismatch/expired → normal 401
  • Applied the same detection logic to AuthService.logout().
  • Added SecurityEventType.TOKEN_REPLAY_DETECTED enum value.
  • Injected SecurityEventRepository into AuthService to persist the audit
    event.

Linked issue

Closes #

How to test

  1. Log in — refresh_token HttpOnly cookie is set.
  2. Call POST /api/auth/refresh

@GideonBa GideonBa left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for working on the replay-token handling. I found a few compile blockers that need to be fixed before this can be merged.

update RefreshToken rt
set rt.revoked = true
where rt.user.id = :userId
where rt.user.id = :userId AND r.revoked = false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This JPQL uses alias r, but the update declares rt. Please change this to rt.revoked = false; otherwise the revoke-all query will fail.

VAULT_UNLOCKED,
VAULT_LOCKED
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AuthService now references TOKEN_REPLAY_DETECTED, but this enum still does not define it. Please add the new enum value here.

import org.slf4j.Logger;

import static vaultWeb.services.auth.RefreshTokenCleanupService.log;
private final SecurityEventRepository securityEventRepository;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This field is currently outside the AuthService class body, which makes the file invalid Java. Please move it into the class so @RequiredArgsConstructor can inject it.

}

String tokenId = claims.getId();
RefreshToken storedToken = refreshTokenRepository.findByTokenId(tokenId).orElse(null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tokenId is used here before it is assigned. Please restore String tokenId = claims.getId() after parsing the refresh token.

@DenizAltunkapan

Copy link
Copy Markdown
Member

@avdheshpatidar27 please fix the failing workflows

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