Skip to content

fix: update aal requirements to update user #1766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
J0 merged 7 commits into from
Sep 17, 2024
Merged

fix: update aal requirements to update user #1766

J0 merged 7 commits into from
Sep 17, 2024
+78 −0

Conversation

@J0
Copy link
Contributor

@J0 J0 commented Sep 5, 2024 •
edited
Loading

What kind of change does this PR introduce?

If a user has verified factors (mfa enabled) we should require an AAL2 session in order to proceed with any operation

We restrict phone, email, and password from updates as we consider those as sensitive fields

Context: https://supabase.slack.com/archives/C02AK9166FR/p1725466764804889

@J0 J0 marked this pull request as ready for review September 11, 2024 11:11
@J0 J0 requested a review from a team as a code owner September 11, 2024 11:11
@coveralls
Copy link

coveralls commented Sep 11, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 10812950524

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

  • 4 of 10 (40.0%) changed or added relevant lines in 2 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.01%) to 57.908%
Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/user.go 1 4 25.0%
internal/models/user.go 3 6 50.0%
Files with Coverage Reduction New Missed Lines %
internal/models/cleanup.go 4 88.37%
Totals Coverage Status
Change from base Build 10684529517: -0.01%
Covered Lines: 9142
Relevant Lines: 15787
💛 - Coveralls

hf
hf reviewed Sep 11, 2024
View reviewed changes
hf
hf reviewed Sep 11, 2024
View reviewed changes
@J0 J0 requested a review from hf September 11, 2024 13:50
cstockton
cstockton approved these changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@cstockton cstockton left a comment

Choose a reason for hiding this comment

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

Code wise I see no issues long as the logic behind the added checks is sound.

@J0 J0 merged commit 25d9874 into master Sep 17, 2024
@J0 J0 deleted the j0/require_appropriate_aal_for_pw_update branch September 17, 2024 20:31
@github-actions github-actions bot mentioned this pull request Sep 13, 2024
Merged
hf pushed a commit that referenced this pull request Sep 24, 2024
@github-actions
chore(master): release 2.161.0 (#1760)
5b03fda 
🤖 I have created a release *beep* *boop*
---


##
[2.161.0](v2.160.0...v2.161.0)
(2024-09-24)


### Features

* add `x-sb-error-code` header, show error code in logs
([#1765](#1765))
([ed91c59](ed91c59))
* add webauthn configuration variables
([#1773](#1773))
([77d5897](77d5897))
* config reloading
([#1771](#1771))
([6ee0091](6ee0091))


### Bug Fixes

* add additional information around errors for missing content type
header ([#1576](#1576))
([c2b2f96](c2b2f96))
* add token to hook payload for non-secure email change
([#1763](#1763))
([7e472ad](7e472ad))
* update aal requirements to update user
([#1766](#1766))
([25d9874](25d9874))
* update mfa admin methods
([#1774](#1774))
([567ea7e](567ea7e))
* user sanitization should clean up email change info too
([#1759](#1759))
([9d419b4](9d419b4))

---
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>
@tjmcewan
Copy link

tjmcewan commented Oct 8, 2024 •
edited
Loading

This can break password reset workflows. 😓 Be sure to update your flow to elevate to AAL2 before asking the user to set their new password.

LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
@J0 @hf @LashaJini
fix: update aal requirements to update user (supabase#1766)
09249e7 
## What kind of change does this PR introduce?

If a user has verified factors (mfa enabled) we should require an AAL2
session in order to proceed with any operation

We restrict phone, email, and password from updates as we consider those
as sensitive fields

Context:
https://supabase.slack.com/archives/C02AK9166FR/p1725466764804889

---------

Co-authored-by: Stojan Dimitrovski <[email protected]>
cemalkilic pushed a commit that referenced this pull request Aug 7, 2025
@J0 @hf @cemalkilic
fix: update aal requirements to update user (#1766)
8676548 
## What kind of change does this PR introduce?

If a user has verified factors (mfa enabled) we should require an AAL2
session in order to proceed with any operation

We restrict phone, email, and password from updates as we consider those
as sensitive fields

Context:
https://supabase.slack.com/archives/C02AK9166FR/p1725466764804889

---------

Co-authored-by: Stojan Dimitrovski <[email protected]>
cemalkilic pushed a commit that referenced this pull request Aug 7, 2025
@github-actions @cemalkilic
chore(master): release 2.161.0 (#1760)
366e918 
🤖 I have created a release *beep* *boop*
---


##
[2.161.0](v2.160.0...v2.161.0)
(2024-09-24)


### Features

* add `x-sb-error-code` header, show error code in logs
([#1765](#1765))
([ed91c59](ed91c59))
* add webauthn configuration variables
([#1773](#1773))
([77d5897](77d5897))
* config reloading
([#1771](#1771))
([6ee0091](6ee0091))


### Bug Fixes

* add additional information around errors for missing content type
header ([#1576](#1576))
([c2b2f96](c2b2f96))
* add token to hook payload for non-secure email change
([#1763](#1763))
([7e472ad](7e472ad))
* update aal requirements to update user
([#1766](#1766))
([25d9874](25d9874))
* update mfa admin methods
([#1774](#1774))
([567ea7e](567ea7e))
* user sanitization should clean up email change info too
([#1759](#1759))
([9d419b4](9d419b4))

---
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>
xeladotbe pushed a commit to xeladotbe/supabase-auth that referenced this pull request Sep 27, 2025
@J0 @hf @xeladotbe
fix: update aal requirements to update user (supabase#1766)
8722319 
## What kind of change does this PR introduce?

If a user has verified factors (mfa enabled) we should require an AAL2
session in order to proceed with any operation

We restrict phone, email, and password from updates as we consider those
as sensitive fields

Context:
https://supabase.slack.com/archives/C02AK9166FR/p1725466764804889

---------

Co-authored-by: Stojan Dimitrovski <[email protected]>
xeladotbe pushed a commit to xeladotbe/supabase-auth that referenced this pull request Sep 27, 2025
@github-actions @xeladotbe
chore(master): release 2.161.0 (supabase#1760)
431c9a8 
🤖 I have created a release *beep* *boop*
---


##
[2.161.0](supabase/auth@v2.160.0...v2.161.0)
(2024-09-24)


### Features

* add `x-sb-error-code` header, show error code in logs
([supabase#1765](supabase#1765))
([ed91c59](supabase@ed91c59))
* add webauthn configuration variables
([supabase#1773](supabase#1773))
([77d5897](supabase@77d5897))
* config reloading
([supabase#1771](supabase#1771))
([6ee0091](supabase@6ee0091))


### Bug Fixes

* add additional information around errors for missing content type
header ([supabase#1576](supabase#1576))
([c2b2f96](supabase@c2b2f96))
* add token to hook payload for non-secure email change
([supabase#1763](supabase#1763))
([7e472ad](supabase@7e472ad))
* update aal requirements to update user
([supabase#1766](supabase#1766))
([25d9874](supabase@25d9874))
* update mfa admin methods
([supabase#1774](supabase#1774))
([567ea7e](supabase@567ea7e))
* user sanitization should clean up email change info too
([supabase#1759](supabase#1759))
([9d419b4](supabase@9d419b4))

---
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
@J0 @hf @issuedat
fix: update aal requirements to update user (#1766)
cd27e58 
## What kind of change does this PR introduce?

If a user has verified factors (mfa enabled) we should require an AAL2
session in order to proceed with any operation

We restrict phone, email, and password from updates as we consider those
as sensitive fields

Context:
https://supabase.slack.com/archives/C02AK9166FR/p1725466764804889

---------

Co-authored-by: Stojan Dimitrovski <[email protected]>
issuedat pushed a commit that referenced this pull request Sep 30, 2025
@github-actions @issuedat
chore(master): release 2.161.0 (#1760)
d592996 
🤖 I have created a release *beep* *boop*
---


##
[2.161.0](v2.160.0...v2.161.0)
(2024-09-24)


### Features

* add `x-sb-error-code` header, show error code in logs
([#1765](#1765))
([ed91c59](ed91c59))
* add webauthn configuration variables
([#1773](#1773))
([77d5897](77d5897))
* config reloading
([#1771](#1771))
([6ee0091](6ee0091))


### Bug Fixes

* add additional information around errors for missing content type
header ([#1576](#1576))
([c2b2f96](c2b2f96))
* add token to hook payload for non-secure email change
([#1763](#1763))
([7e472ad](7e472ad))
* update aal requirements to update user
([#1766](#1766))
([25d9874](25d9874))
* update mfa admin methods
([#1774](#1774))
([567ea7e](567ea7e))
* user sanitization should clean up email change info too
([#1759](#1759))
([9d419b4](9d419b4))

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

Reviewers

@cstockton cstockton cstockton approved these changes

@hf hf Awaiting requested review from hf

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@J0 @coveralls @tjmcewan @cstockton @hf