Fix HMAC CSRF token payload#91
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This pull request updates the stateless HMAC CSRF token format so the emitted payload no longer exposes the session identity and instead carries only expiration metadata plus random data, while keeping the signature bound to the current identity. It also refreshes OWASP CSRF cheat-sheet wording/links and adds tests covering the new token semantics.
Changes:
- Reworked
HmacCsrfTokenso token payload ={expiration}~{random}and the HMAC is computed using a session-bound identity without embedding it in the payload. - Added tests to ensure token values change per call and that decoded payloads do not contain the session identity.
- Updated README/CHANGELOG to reflect the revised “HMAC signed token” terminology and OWASP link/semantics.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Hmac/HmacCsrfToken.php | Changes HMAC token encoding/validation to avoid exposing identity and to include random payload + expiration. |
| tests/Hmac/HmacCsrfTokenTest.php | Adds coverage for non-deterministic token values and verifying identity is not present in decoded payloads. |
| README.md | Updates documentation terminology, OWASP link anchor, and clarifies replay semantics. |
| CHANGELOG.md | Notes the bug fix for #32 and documentation update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private function extractData(string $token): ?array | ||
| { | ||
| try { | ||
| $raw = $this->mac->getMessage( | ||
| StringHelper::base64UrlDecode($token), | ||
| $this->secretKey, | ||
| true, | ||
| ); | ||
| } catch (DataIsTamperedException $e) { | ||
| $payload = StringHelper::base64UrlDecode($token); | ||
| $hashLength = $this->getHashLength(); | ||
|
|
| private function getHashLength(): int | ||
| { | ||
| return StringHelper::byteLength($this->generateHash('')); | ||
| } |
Summary
Fixes #32.
Tests