Skip to content
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

fix: add error codes to password login flow #1721

Merged
merged 7 commits into from
Aug 15, 2024
Merged

fix: add error codes to password login flow #1721

merged 7 commits into from
Aug 15, 2024

Conversation

kangmingtay
Copy link
Member

@kangmingtay kangmingtay commented Aug 14, 2024 •
edited
Loading

What kind of change does this PR introduce?

What is the current behavior?

  • Most errors in the password login flow are returned as oauthError, which doesn't have support for an error code as the struct conforms to the oauth error response specified in the RFC

What is the new behavior?

  • Errors which were previously returned as an oauthError struct now return badRequestError instead with the following error code invalid_login_credentials
  • In certain cases, we can return existing error codes like ErrorCodeUserBanned, ErrorCodeEmailNotConfirmed, ErrorCodePhoneNotConfirmed or ErrorCodeValidationFailed
  • Some error messages are updated to provide more clarity on the underlying error

Feel free to include screenshots if it includes visual changes.

Additional context

Add any other context or screenshots.

@kangmingtay kangmingtay requested a review from a team as a code owner August 14, 2024 18:45
@kangmingtay kangmingtay force-pushed the km/add-error-codes-password-login branch from f11f574 to 66823d9 Compare August 14, 2024 19:12
@coveralls
Copy link

coveralls commented Aug 14, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 10410723124

Details

  • 7 of 15 (46.67%) changed or added relevant lines in 3 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.006%) to 58.283%
Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/token.go 4 12 33.33%
Files with Coverage Reduction New Missed Lines %
internal/api/token.go 2 72.15%
Totals Coverage Status
Change from base Build 10316067127: -0.006%
Covered Lines: 9193
Relevant Lines: 15773
💛 - Coveralls

hf
hf reviewed Aug 15, 2024
View reviewed changes
internal/api/errorcodes.go Outdated Show resolved Hide resolved
J0
J0 reviewed Aug 15, 2024
View reviewed changes
internal/api/token.go Outdated Show resolved Hide resolved
J0
J0 reviewed Aug 15, 2024
View reviewed changes
internal/api/token.go Outdated Show resolved Hide resolved
J0
J0 approved these changes Aug 15, 2024
View reviewed changes
Copy link
Contributor

@J0 J0 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good! Consider seeking an opinion from @hf on the status codes - personally not too sure

Update - wrote this comment before seeing review from hf so think we should be good to merge.

@charislam charislam mentioned this pull request Aug 15, 2024
Open
@kangmingtay kangmingtay merged commit 4351226 into master Aug 15, 2024
2 checks passed
@kangmingtay kangmingtay deleted the km/add-error-codes-password-login branch August 15, 2024 21:26
@github-actions github-actions bot mentioned this pull request Aug 9, 2024
Merged
kangmingtay pushed a commit that referenced this pull request Aug 21, 2024
@github-actions
chore(master): release 2.159.0 (#1716)
a6de8c9 
🤖 I have created a release *beep* *boop*
---


##
[2.159.0](v2.158.1...v2.159.0)
(2024-08-21)


### Features

* Vercel marketplace OIDC
([#1731](#1731))
([a9ff361](a9ff361))


### Bug Fixes

* add error codes to password login flow
([#1721](#1721))
([4351226](4351226))
* change phone constraint to per user
([#1713](#1713))
([b9bc769](b9bc769))
* custom SMS does not work with Twilio Verify
([#1733](#1733))
([dc2391d](dc2391d))
* ignore errors if transaction has closed already
([#1726](#1726))
([53c11d1](53c11d1))
* redirect invalid state errors to site url
([#1722](#1722))
([b2b1123](b2b1123))
* remove TOTP field for phone enroll response
([#1717](#1717))
([4b04327](4b04327))
* use signing jwk to sign oauth state
([#1728](#1728))
([66fd0c8](66fd0c8))

---
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>
This was referenced Aug 27, 2024
Closed
Open
Closed
Closed
@tom-at-pixel
Copy link

Awesome! Thanks for doing this! Should these docs be updated? https://supabase.com/docs/reference/javascript/auth-error-codes

This was referenced Aug 28, 2024
Open
Merged
@tiltmaster
Copy link

is this fix live on stable version?

@devangpadhiyar
Copy link

devangpadhiyar commented Oct 27, 2024 •
edited
Loading

This is still happening on supabase local dev environment.
I tried using python SDK and js SDK as well. but still code is always undefined in case of signinwith password

@kangmingtay kangmingtay mentioned this pull request Oct 30, 2024
Merged
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
@kangmingtay
fix: add error codes to password login flow (supabase#1721)
6ae7118 
## What kind of change does this PR introduce?
* Add error codes to the password login flow, which fixes supabase#1631 

## What is the current behavior?
* Most errors in the password login flow are returned as `oauthError`,
which doesn't have support for an error code as the struct conforms to
the [oauth error
response](https://datatracker.ietf.org/doc/html/rfc6749#section-5.2)
specified in the RFC

## What is the new behavior?
* Errors which were previously returned as an `oauthError` struct now
return `badRequestError` instead with the following error code
`invalid_login_credentials`
* In certain cases, we can return existing error codes like
`ErrorCodeUserBanned`, `ErrorCodeEmailNotConfirmed`,
`ErrorCodePhoneNotConfirmed` or `ErrorCodeValidationFailed`
* Some error messages are updated to provide more clarity on the
underlying error

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
@github-actions
chore(master): release 2.159.0 (supabase#1716)
f8c8ea8 
🤖 I have created a release *beep* *boop*
---


##
[2.159.0](supabase/auth@v2.158.1...v2.159.0)
(2024-08-21)


### Features

* Vercel marketplace OIDC
([supabase#1731](supabase#1731))
([a9ff361](supabase@a9ff361))


### Bug Fixes

* add error codes to password login flow
([supabase#1721](supabase#1721))
([4351226](supabase@4351226))
* change phone constraint to per user
([supabase#1713](supabase#1713))
([b9bc769](supabase@b9bc769))
* custom SMS does not work with Twilio Verify
([supabase#1733](supabase#1733))
([dc2391d](supabase@dc2391d))
* ignore errors if transaction has closed already
([supabase#1726](supabase#1726))
([53c11d1](supabase@53c11d1))
* redirect invalid state errors to site url
([supabase#1722](supabase#1722))
([b2b1123](supabase@b2b1123))
* remove TOTP field for phone enroll response
([supabase#1717](supabase#1717))
([4b04327](supabase@4b04327))
* use signing jwk to sign oauth state
([supabase#1728](supabase#1728))
([66fd0c8](supabase@66fd0c8))

---
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>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
@kangmingtay @LashaJini
fix: add error codes to password login flow (supabase#1721)
918e3e4 
## What kind of change does this PR introduce?
* Add error codes to the password login flow, which fixes supabase#1631 

## What is the current behavior?
* Most errors in the password login flow are returned as `oauthError`,
which doesn't have support for an error code as the struct conforms to
the [oauth error
response](https://datatracker.ietf.org/doc/html/rfc6749#section-5.2)
specified in the RFC

## What is the new behavior?
* Errors which were previously returned as an `oauthError` struct now
return `badRequestError` instead with the following error code
`invalid_login_credentials`
* In certain cases, we can return existing error codes like
`ErrorCodeUserBanned`, `ErrorCodeEmailNotConfirmed`,
`ErrorCodePhoneNotConfirmed` or `ErrorCodeValidationFailed`
* Some error messages are updated to provide more clarity on the
underlying error

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
@github-actions @LashaJini
chore(master): release 2.159.0 (supabase#1716)
8f59687 
🤖 I have created a release *beep* *boop*
---


##
[2.159.0](supabase/auth@v2.158.1...v2.159.0)
(2024-08-21)


### Features

* Vercel marketplace OIDC
([supabase#1731](supabase#1731))
([a9ff361](supabase@a9ff361))


### Bug Fixes

* add error codes to password login flow
([supabase#1721](supabase#1721))
([4351226](supabase@4351226))
* change phone constraint to per user
([supabase#1713](supabase#1713))
([b9bc769](supabase@b9bc769))
* custom SMS does not work with Twilio Verify
([supabase#1733](supabase#1733))
([dc2391d](supabase@dc2391d))
* ignore errors if transaction has closed already
([supabase#1726](supabase#1726))
([53c11d1](supabase@53c11d1))
* redirect invalid state errors to site url
([supabase#1722](supabase#1722))
([b2b1123](supabase@b2b1123))
* remove TOTP field for phone enroll response
([supabase#1717](supabase#1717))
([4b04327](supabase@4b04327))
* use signing jwk to sign oauth state
([supabase#1728](supabase#1728))
([66fd0c8](supabase@66fd0c8))

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

@J0 J0 J0 approved these changes

@hf hf hf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Invalid Login Credientals" AuthApiError should have an error code
7 participants
@kangmingtay @coveralls @tom-at-pixel @tiltmaster @devangpadhiyar @hf @J0