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.

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

feat: add error codes #1377

Merged
merged 5 commits into from
Mar 13, 2024
Merged

feat: add error codes #1377

merged 5 commits into from
Mar 13, 2024

Conversation

hf
Copy link
Contributor

@hf hf commented Jan 17, 2024

Adds proper error codes with API versioning.

From now on, all responses that end in a 4XX HTTP status code will include a textual description of the error that occurred.

Error responses on API versions before 2024-01-01 have this schema:

{
  "code": "<http status code>",
  "msg": "<descriptive error message>",
  "error_code": "<textual error code>"
}

Error responses on API version on or after 2024-01-01 have this schema:

{
  "code": "<textual error code>",
  "message": "<descriptive error message>"
}

API versions are controlled by submitting an X-Supabase-Api-Version header to the request. A missing or invalid value assumes the "initial" API version as used before the introduction of API version 2024-01-01.

Error code contract for API version 2024-01-01:

  1. Error codes will not be renamed. You can safely rely on them.
  2. HTTP status codes are mostly fixed, but you should not rely on them except the class 4XX vs 5XX.
  3. Error messages are a developer aid. They may change across deployed version. You should not rely on them, but if you want you can show them to your users. Error translations should be based on the error code!

Of the 4XX HTTP status code class, only these codes are allowed to be used in API version 2024-01-01 according to these rules. The purpose of this is to keep proper HTTP semantics. The tuple (http_status_code, error_code) shouldn't be used by clients!

HTTP Status CodeWhen to use?Primary Fault At
400 Bad RequestInputs (body, headers) and their contents are not valid as a whole, or parts of them. Example: bad JSON, bad JSON object, using two mutually exclusive JSON fields, missing required fields, wrong encoding…

If the answer to this question is
yes then you should probably use 400: Is there some technical thing the developer should do to get a different status code?
Developer.

MUST NEVER OCCUR WHEN USING AN OFFICIAL SUPABASE LIBRARY.

Why?
- Library should not send invalid requests.
- If occurring, means: improper types, no client-side validation.
401 UnauthorizedYou must use this code if security headers or inputs are missing, and are not valid to some extent. Example: missing JWT, missing CAPTCHA token, missing important query params that serve to authenticate the caller.

You may use 400 instead if the security headers or inputs are provided and relatively valid (valid JWT signature, bad claims) instead, though prefer 401 over 400 unless it aids in DX.

Do not use this code to signal that the user does not have sufficient application privileges.

If the answer to this question is
yes then you should use 401: Are the credentials the user/client sending missing or invalid in form, structure, encoding?
Developer.

MUST NEVER OCCUR WHEN USING AN OFFICIAL SUPABASE LIBRARY.

Why?
- Library should never send improper requests (missing authorization headers for features that require authorization).
- If occurring means: broken logic, improper types, no client-side validation.
403 ForbiddenDo not use this code for bad JWT format, missing headers or other validation on security sensitive payloads. Return 400 on those.

Once security payloads have been validated in structure, only return this error if the user/client can be authenticated properly but they do not possess the proper authorization to access the resource.

If the answer to this question is
yes then you should use 403: Should the user/client be someone else to get a different status code?

In some cases you should prefer 200 responses with empty bodies, akin to RLS behavior.
User.

Developer is at fault for not hiding the feature sufficiently.

Can rarely occur when using Supabase libraries, and in such cases it means docs / explanation problems.
404 Not FoundDo not use this for missing objects in the database. Prefer using 422 instead.

Use only if the URL cannot be fully validated, resulting in a resource that cannot be properly identified. If there’s no variables in the path, this code must not be used.

Good:
- /users/<not-uuid>
- /users/<uuid> (but such a user does not exist)

Bad:
- /token?grant_type=password (no variables in URL)
- /sessions (no variables in URL)

For cases where a feature is disabled on a server consider using 501 or 422.

For requests that “look up” entities consider using 200 with an empty/null response body or 204 No Content instead.
Developer.

MUST NEVER OCCUR WHEN USING AN OFFICIAL SUPABASE LIBRARY.

Why?
- Library should never send improperly formatted URLs, or encode data in URLs that it knows to be invalid.
- Ideally library should not take in freeform input about entities, and should use some “proof” that the entity exists. Example: calling methods on objects returned by a list/find-by-id method.
- In some situations it’s inevitable (like in admin APIs).
422 Unprocessable EntityDo not use for bad inputs!

Once all inputs to a request have been validated to the fullest extent possible (e.g. OK to validate an email address format, but not necessary to validate that there’s a SMTP server listening), use this status code to signal errors with the processing logic. This includes all logic dependent on database state (user exists, or doesn’t). All third-party expected errors (like calling into a third service) should end with 422.

If the answer to this question is
yes then you should use 422: Is there something different that the user should do to get a different status code?
User.

Developer is at fault for using the feature in an improper part of the flow.

Can rarely occur when using Supabase libraries, and in such cases it means docs / explanation problems.
429 Too Many RequestsOnly use this for rate-limiting or other cases that limit the number of requests. Third-party rate-limits should be propagated with this error.User.

Developer should have implemented a better UX to prevent reaching this error for legitimate users. (Disabling buttons, adding timeout UI elements…)
500 Internal Server ErrorUse this for any unexpected errors when processing a request. Default to this code if you can’t find a 4XX error code for it.Supabase. Developer in some cases (such as when changing database contents).

The cause of this error should not be situations arising from official Supabase libraries (such as sending inputs that crash the server).

501 Not ImplementedA feature is disabled, not configured (properly), blocked or otherwise unavailable.Developer, for not enabling or configuring features properly.

@tom-at-pixel
Copy link

Very nice and granular error codes! This will bring the DX of Supabase's authentication module much closer to that of Firebase. Great work. 😎

@hf
Copy link
Contributor Author

hf commented Jan 17, 2024

Very nice and granular error codes! This will bring the DX of Supabase's authentication module much closer to that of Firebase. Great work. 😎

Note that at this point these are just extractions, they're not definitive in shape or form.

@kangmingtay
Copy link
Member

would be great if we can create an ErrorCode type for this instead of using normal strings for ease of maintainability

@hf hf force-pushed the hf/error-codes branch 2 times, most recently from 67c583b to 3a558bc Compare January 27, 2024 20:15
@hf hf force-pushed the hf/error-codes branch 3 times, most recently from 6f0b17b to e9b32fc Compare February 7, 2024 13:12
@hf hf changed the title [wip] feat: add error codes feat: add error codes Feb 7, 2024
internal/api/token.go Outdated Show resolved Hide resolved
@hf hf force-pushed the hf/error-codes branch 4 times, most recently from 5b94095 to 00f6ab1 Compare February 8, 2024 15:27
internal/api/token.go Outdated Show resolved Hide resolved
@hf hf marked this pull request as ready for review February 8, 2024 19:18
@hf hf requested a review from a team as a code owner February 8, 2024 19:18
internal/api/samlacs.go Outdated Show resolved Hide resolved
Copy link
Contributor

@J0 J0 left a comment

Choose a reason for hiding this comment

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

Thanks! This looks fine to me - will do another pass later in day and open PR if needed

@hf hf force-pushed the hf/error-codes branch 3 times, most recently from 57192fa to b047cf9 Compare March 11, 2024 05:39
@hf hf force-pushed the hf/error-codes branch 2 times, most recently from b480655 to a47ce87 Compare March 13, 2024 11:31
@hf hf merged commit e4beea1 into master Mar 13, 2024
3 checks passed
@hf hf deleted the hf/error-codes branch March 13, 2024 11:56
J0 pushed a commit that referenced this pull request Mar 26, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.145.0](v2.144.0...v2.145.0)
(2024-03-26)


### Features

* add error codes
([#1377](#1377))
([e4beea1](e4beea1))
* add kakao OIDC
([#1381](#1381))
([b5566e7](b5566e7))
* clean up expired factors
([#1371](#1371))
([5c94207](5c94207))
* configurable NameID format for SAML provider
([#1481](#1481))
([ef405d8](ef405d8))
* HTTP Hook - Add custom envconfig decoding for HTTP Hook Secrets
([#1467](#1467))
([5b24c4e](5b24c4e))
* refactor PKCE FlowState to reduce duplicate code
([#1446](#1446))
([b8d0337](b8d0337))


### Bug Fixes

* add http support for https hooks on localhost
([#1484](#1484))
([5c04104](5c04104))
* cleanup panics due to bad inactivity timeout code
([#1471](#1471))
([548edf8](548edf8))
* **docs:** remove bracket on file name for broken link
([#1493](#1493))
([96f7a68](96f7a68))
* impose expiry on auth code instead of magic link
([#1440](#1440))
([35aeaf1](35aeaf1))
* invalidate email, phone OTPs on password change
([#1489](#1489))
([960a4f9](960a4f9))
* move creation of flow state into function
([#1470](#1470))
([4392a08](4392a08))
* prevent user email side-channel leak on verify
([#1472](#1472))
([311cde8](311cde8))
* refactor email sending functions
([#1495](#1495))
([285c290](285c290))
* refactor factor_test to centralize setup
([#1473](#1473))
([c86007e](c86007e))
* refactor mfa challenge and tests
([#1469](#1469))
([6c76f21](6c76f21))
* Resend SMS when duplicate SMS sign ups are made
([#1490](#1490))
([73240a0](73240a0))
* unlink identity bugs
([#1475](#1475))
([73e8d87](73e8d87))

---
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>
hf added a commit to supabase/auth-js that referenced this pull request Mar 26, 2024
Adds support for error codes. All `AuthError` descendants will now have
a `code` property which will encode (when present and supported by the
server) the reason why the error occurred.

To support this, the library will now advertise `X-Supabase-Api-Version:
2024-01-01` which is the first version of a new versioning strategy that
supports a different encoding for error responses.

See:
- supabase/auth#1377
@mashwishi
Copy link

mashwishi commented Mar 30, 2024

is this only for auth-js ? and supabase-js not included?

            const response = await supabase.auth.signInWithPassword({
                email: email,
                password: password,
            })

image
This is currently "@supabase/supabase-js": "^2.41.1"

@haydn
Copy link

haydn commented Mar 30, 2024

@mashwishi From your code snippet I don't really understand how that log message is getting generated, but I believe that AuthApiError object will have a code property on it you can use.

Something like this:

console.log(response.error.code);

@mashwishi
Copy link

@mashwishi From your code snippet I don't really understand how that log message is getting generated, but I believe that AuthApiError object will have a code property on it you can use.

Something like this:

console.log(response.error.code);

Will test that out today.

@mashwishi
Copy link

mashwishi commented Mar 30, 2024

@mashwishi From your code snippet I don't really understand how that log message is getting generated, but I believe that AuthApiError object will have a code property on it you can use.

Something like this:

console.log(response.error.code);

that's weird.

image
image

it works on

            console.log({
                'code:': response?.error?.code,
                'name:': response?.error?.name,
                'status:': response?.error?.status,
                'message:': response?.error?.message
            });

with output:

{"code:": undefined, "message:": "Invalid login credentials", "name:": "AuthApiError", "status:": 400}
{"code:": undefined, "message:": "Invalid login credentials", "name:": "AuthApiError", "status:": 400}

But what i am trying to get is proper error code which i get it right now and right error message like example invalid password, account doesnt exist something like that from the auth api.

because if i am going to base it on error code, how would i determine if account doesn't exist or password is incorrect? because they have same output of error message "Invalid credentials" if using error response and "400 error code"

kangmingtay added a commit that referenced this pull request Apr 3, 2024
## What kind of change does this PR introduce?
* Fixes an error introduced in #1377 where generating a signup link for
a new user results in a "User with this email not found" error

## What is the current behavior?

Please link any relevant issues here.

## What is the new behavior?

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.
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.

None yet

7 participants