Skip to content

use HMAC-SHA256 for the cookie string - #450

Merged
briskt merged 4 commits into
mainfrom
fix-cookie-string
Jul 30, 2026
Merged

use HMAC-SHA256 for the cookie string #450
briskt merged 4 commits into
mainfrom
fix-cookie-string

Conversation

@briskt

@briskt briskt commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

IDP-2186 Verify and fix bcrypt in sspbase


Fixed

  • Use HMAC-SHA256 for the cookie string to avoid bcrypt's 72-char limit.

@briskt
briskt requested a review from a team as a code owner July 29, 2026 05:49
@briskt
briskt requested review from ethancanne, forevermatt, jason-jackson and mtompset and removed request for a team July 29, 2026 05:49

@hobbitronics hobbitronics left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good

@briskt

briskt commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@devon-sil do you think this will be a change that requires an email blast? It will cause everyone to be prompted for MFA on their next login.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the MFA “remember me” cookie hashing/verification logic to avoid bcrypt’s 72-character input limit by switching to an HMAC-based approach for cookie integrity checking.

Changes:

  • Replace password_hash/password_verify with hash_hmac('sha256', ...) + hash_equals() for remember-me cookie validation.
  • Update generateRememberMeCookieString() to no longer embed the secret in the string being hashed.
  • Adjust the MFA unit test to generate the cookie hash using HMAC-SHA256.

Reviewed changes

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

File Description
modules/mfa/src/Auth/Process/Mfa.php Switch remember-me cookie hashing/verification to HMAC-SHA256 and update cookie string generation accordingly.
modules/mfa/tests/MfaTest.php Update test to reflect the new remember-me cookie hashing method.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modules/mfa/tests/MfaTest.php Outdated
Comment thread modules/mfa/src/Auth/Process/Mfa.php
Comment thread modules/mfa/src/Auth/Process/Mfa.php Outdated
briskt and others added 2 commits July 29, 2026 15:51
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@briskt
briskt merged commit eeb1ea2 into main Jul 30, 2026
2 checks passed
@briskt
briskt deleted the fix-cookie-string branch July 30, 2026 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants