Skip to content

Conversation

@cstockton
Copy link
Contributor

This change sets EmailData.Token to the new OTP when the secure email change setting is set to true.

This should fix:
#1744
#2042

Note:
I am fixing this bug in a way that is consistent with what I view to be the current bug. I believe for email changes token_new and token_hash_new should always contain the values for the users new_email field. This should be fixed in the future in a way that doesn't break BC.

Chris Stockton added 2 commits June 2, 2025 15:53
First I added send email tests for the basic EmailChange flows
to better understand the existing behavior.

There is currently a bug in the existing send-email hooks which
omits the token during email changes for anonymous users when
the SecureEmailChange config value is set to false. I've also
identified a bug with the token hashes being mixed up. The tests
lock in this behavior so we don't break existing hooks code.
This change sets EmailData.Token to the new OTP when the secure
email change setting is set to true.

Note:
I am fixing this bug in a way that is consistent with what I view
to be the current bug. I believe for email changes token_new and
token_hash_new should always contain the values for the users
new_email field. This should be fixed in the future in a way that
doesn't break BC.
Copy link
Contributor

@hf hf left a comment

Choose a reason for hiding this comment

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

Looks good

@hf
Copy link
Contributor

hf commented Jun 3, 2025

Don't forget to update public docs

@cstockton cstockton merged commit be20654 into master Jun 3, 2025
6 checks passed
@cstockton cstockton deleted the cs/bug-fix-send-email-hook branch June 3, 2025 20:25
@coveralls
Copy link

coveralls commented Jun 4, 2025

Pull Request Test Coverage Report for Build 15426761119

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 27 of 27 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.07%) to 70.217%

Totals Coverage Status
Change from base Build 15402553041: 0.07%
Covered Lines: 11312
Relevant Lines: 16110

💛 - Coveralls

cstockton pushed a commit that referenced this pull request Jun 5, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.175.0](v2.174.0...v2.175.0)
(2025-06-03)


### Features

* hooks round 5 (Option 2) - add before-user-created hook
([#2034](#2034))
([b53f6b0](b53f6b0))


### Bug Fixes

* email-sendhook - bug in email change verification
([#2044](#2044))
([be20654](be20654))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@cstockton cstockton changed the title fix: email-sendhook - bug in email change verification fix: email-send hook - bug in email change verification Jun 10, 2025
@cstockton cstockton changed the title fix: email-send hook - bug in email change verification fix: send-email hook - bug in email change verification Jun 10, 2025
cemalkilic pushed a commit that referenced this pull request Aug 7, 2025
This change sets EmailData.Token to the new OTP when the secure email
change setting is set to true.

This should fix:
#1744
#2042

Note:
I am fixing this bug in a way that is consistent with what I view to be
the current bug. I believe for email changes token_new and
token_hash_new should always contain the values for the users new_email
field. This should be fixed in the future in a way that doesn't break
BC.

---------

Co-authored-by: Chris Stockton <chris.stockton@supabase.io>
cemalkilic pushed a commit that referenced this pull request Aug 7, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.175.0](v2.174.0...v2.175.0)
(2025-06-03)


### Features

* hooks round 5 (Option 2) - add before-user-created hook
([#2034](#2034))
([b53f6b0](b53f6b0))


### Bug Fixes

* email-sendhook - bug in email change verification
([#2044](#2044))
([be20654](be20654))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
issuedat pushed a commit that referenced this pull request Sep 30, 2025
This change sets EmailData.Token to the new OTP when the secure email
change setting is set to true.

This should fix:
#1744
#2042

Note:
I am fixing this bug in a way that is consistent with what I view to be
the current bug. I believe for email changes token_new and
token_hash_new should always contain the values for the users new_email
field. This should be fixed in the future in a way that doesn't break
BC.

---------

Co-authored-by: Chris Stockton <chris.stockton@supabase.io>
issuedat pushed a commit that referenced this pull request Sep 30, 2025
🤖 I have created a release *beep* *boop*
---


##
[2.175.0](v2.174.0...v2.175.0)
(2025-06-03)


### Features

* hooks round 5 (Option 2) - add before-user-created hook
([#2034](#2034))
([b53f6b0](b53f6b0))


### Bug Fixes

* email-sendhook - bug in email change verification
([#2044](#2044))
([be20654](be20654))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

4 participants