Skip to content

Commit

Permalink
feat: add error codes (#1377)
Browse files Browse the repository at this point in the history
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:

```json
{
  "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:

```json
{
  "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!

<table>
<thead class="simple-table-header"><tr
id="865bd663-796b-4bdc-a53d-8aecbdc35ab5"><th id="=Aef"
class="simple-table-header-color simple-table-header"
style="width:142.9666748046875px">HTTP Status Code</th><th id="UuUa"
class="simple-table-header-color simple-table-header"
style="width:390px">When to use?</th><th>Primary Fault
At</th></tr></thead><tbody><tr
id="794cf424-14a8-449c-880f-b6073206e70b"><td id="=Aef" class=""
style="width:142.9666748046875px">400 Bad Request</td><td id="UuUa"
class="" style="width:390px">Inputs (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…<br><br>If the answer to this question is
<br><em>yes</em> then you should probably use 400: Is there some
<em>technical</em> thing the developer should do to get a different
status code?</td><td>Developer.<br><br>MUST NEVER OCCUR WHEN USING AN
OFFICIAL SUPABASE LIBRARY.<br><br>Why?<br>- Library should not send
invalid requests.<br>- If occurring, means: improper types, no
client-side validation.<br></td></tr><tr
id="11fb5eb7-0d25-41f3-998b-86162dedcf93"><td id="=Aef" class=""
style="width:142.9666748046875px">401 Unauthorized</td><td id="UuUa"
class="" style="width:390px">You 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.<br><br>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.<br><br>Do not use this code to signal that the user does not
have sufficient application privileges.<br><br>If the answer to this
question is <br><em>yes</em> then you should use 401: Are the
credentials the user/client sending missing or invalid in form,
structure, encoding? </td><td>Developer.<br><br>MUST NEVER OCCUR WHEN
USING AN OFFICIAL SUPABASE LIBRARY. <br><br>Why?<br>- Library should
never send improper requests (missing authorization headers for features
that require authorization).<br>- If occurring means: broken logic,
improper types, no client-side validation.<br></td></tr><tr
id="fa0e33d5-425c-442d-bdfb-18d68e11cc9e"><td id="=Aef" class=""
style="width:142.9666748046875px">403 Forbidden</td><td id="UuUa"
class="" style="width:390px">Do not use this code for bad JWT format,
missing headers or other validation on security sensitive payloads.
Return 400 on those.<br><br>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.<br><br>If the answer to this question is
<br><em>yes</em> then you should use 403: Should the user/client be
someone else to get a different status code?<br><br>In some cases you
should prefer 200 responses with empty bodies, akin to RLS
behavior.<br></td><td>User.<br><br>Developer is at fault for not hiding
the feature sufficiently.<br><br>Can rarely occur when using Supabase
libraries, and in such cases it means docs / explanation
problems.<br></td></tr><tr id="c16ac87c-7433-4750-bdf8-a8a47148b5da"><td
id="=Aef" class="" style="width:142.9666748046875px">404 Not
Found</td><td id="UuUa" class="" style="width:390px">Do not use this for
missing objects in the database. Prefer using 422 instead.<br><br>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.<br><br>Good:<br>- /users/&lt;not-uuid&gt;<br>-
/users/&lt;uuid&gt; (but such a user does not exist)<br><br>Bad:<br>-
/token?grant_type=password (no variables in URL)<br>- /sessions (no
variables in URL)<br><br>For cases where a feature is disabled on a
server consider using 501 or 422.<br><br>For requests that “look up”
entities consider using 200 with an empty/null response body or 204 No
Content instead.<br></td><td>Developer.<br><br>MUST NEVER OCCUR WHEN
USING AN OFFICIAL SUPABASE LIBRARY. <br><br>Why?<br>- Library should
never send improperly formatted URLs, or encode data in URLs that it
knows to be invalid.<br>- 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.<br>- In some situations it’s inevitable (like in
admin APIs).<br></td></tr><tr
id="ab865315-a594-4490-ab71-254cd536c461"><td id="=Aef" class=""
style="width:142.9666748046875px"><a
href="https://www.rfc-editor.org/rfc/rfc9110#section-15.5.21">422
Unprocessable Entity</a></td><td id="UuUa" class=""
style="width:390px">Do not use for bad inputs!<br><br>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.<br><br>If the answer
to this question is <br><em>yes</em> then you should use 422: Is there
something different that the user should do to get a different status
code?</td><td>User.<br><br>Developer is at fault for using the feature
in an improper part of the flow.<br><br>Can rarely occur when using
Supabase libraries, and in such cases it means docs / explanation
problems.<br></td></tr><tr id="58d30ecb-ca21-4bcf-ab4f-79e352eea516"><td
id="=Aef" class="" style="width:142.9666748046875px">429 Too Many
Requests</td><td id="UuUa" class="" style="width:390px">Only use this
for rate-limiting or other cases that limit the number of requests.
Third-party rate-limits should be propagated with this
error.</td><td>User.<br><br>Developer should have implemented a better
UX to prevent reaching this error for legitimate users. (Disabling
buttons, adding timeout UI elements…)<br></td></tr><tr
id="305e93a6-65e9-4670-a778-523470696d4a"><td id="=Aef" class=""
style="width:142.9666748046875px">500 Internal Server Error</td><td
id="UuUa" class="" style="width:390px">Use this for any
<em>unexpected</em> errors when processing a request. Default to this
code if you can’t find a 4XX error code for it.</td><td>Supabase.
Developer in some cases (such as when changing database
contents).<br><br>The cause of this error should not be situations
arising from official Supabase libraries (such as sending inputs that
crash the server).<br><br></td></tr><tr
id="3fdd4130-137b-49d4-807e-003ce7baf4af"><td id="=Aef" class=""
style="width:142.9666748046875px">501 Not Implemented</td><td id="UuUa"
class="" style="width:390px">A feature is disabled, not configured
(properly), blocked or otherwise unavailable.</td><td>Developer, for not
enabling or configuring features properly.</td></tr></tbody>
</table>

---------

Co-authored-by: joel <joel@joels-MacBook-Pro.local>
  • Loading branch information
hf and joel committed Mar 13, 2024
1 parent 5c94207 commit e4beea1
Show file tree
Hide file tree
Showing 53 changed files with 787 additions and 455 deletions.
7 changes: 6 additions & 1 deletion .github/workflows/conventional-commits-lint.js
Expand Up @@ -46,7 +46,12 @@ let failed = false;

validate.forEach((payload) => {
if (payload.title) {
const { groups } = payload.title.match(TITLE_PATTERN);
const match = payload.title.match(TITLE_PATTERN);
if (!match) {
return
}

const { groups } = match

if (groups) {
if (groups.breaking) {
Expand Down
29 changes: 15 additions & 14 deletions internal/api/admin.go
Expand Up @@ -50,15 +50,15 @@ func (a *API) loadUser(w http.ResponseWriter, r *http.Request) (context.Context,

userID, err := uuid.FromString(chi.URLParam(r, "user_id"))
if err != nil {
return nil, badRequestError("user_id must be an UUID")
return nil, notFoundError(ErrorCodeValidationFailed, "user_id must be an UUID")
}

observability.LogEntrySetField(r, "user_id", userID)

u, err := models.FindUserByID(db, userID)
if err != nil {
if models.IsNotFoundError(err) {
return nil, notFoundError("User not found")
return nil, notFoundError(ErrorCodeUserNotFound, "User not found")
}
return nil, internalServerError("Database error loading user").WithInternalError(err)
}
Expand All @@ -69,15 +69,15 @@ func (a *API) loadUser(w http.ResponseWriter, r *http.Request) (context.Context,
func (a *API) loadFactor(w http.ResponseWriter, r *http.Request) (context.Context, error) {
factorID, err := uuid.FromString(chi.URLParam(r, "factor_id"))
if err != nil {
return nil, badRequestError("factor_id must be an UUID")
return nil, notFoundError(ErrorCodeValidationFailed, "factor_id must be an UUID")
}

observability.LogEntrySetField(r, "factor_id", factorID)

f, err := models.FindFactorByFactorID(a.db, factorID)
if err != nil {
if models.IsNotFoundError(err) {
return nil, notFoundError("Factor not found")
return nil, notFoundError(ErrorCodeMFAFactorNotFound, "Factor not found")
}
return nil, internalServerError("Database error loading factor").WithInternalError(err)
}
Expand All @@ -101,12 +101,12 @@ func (a *API) adminUsers(w http.ResponseWriter, r *http.Request) error {

pageParams, err := paginate(r)
if err != nil {
return badRequestError("Bad Pagination Parameters: %v", err)
return badRequestError(ErrorCodeValidationFailed, "Bad Pagination Parameters: %v", err).WithInternalError(err)
}

sortParams, err := sort(r, map[string]bool{models.CreatedAt: true}, []models.SortField{{Name: models.CreatedAt, Dir: models.Descending}})
if err != nil {
return badRequestError("Bad Sort Parameters: %v", err)
return badRequestError(ErrorCodeValidationFailed, "Bad Sort Parameters: %v", err)
}

filter := r.URL.Query().Get("filter")
Expand Down Expand Up @@ -160,7 +160,7 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error {
if params.BanDuration != "none" {
duration, err = time.ParseDuration(params.BanDuration)
if err != nil {
return badRequestError("invalid format for ban duration: %v", err)
return badRequestError(ErrorCodeValidationFailed, "invalid format for ban duration: %v", err)
}
}
if terr := user.Ban(a.db, duration); terr != nil {
Expand Down Expand Up @@ -308,7 +308,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error {
}

if params.Email == "" && params.Phone == "" {
return unprocessableEntityError("Cannot create a user without either an email or phone")
return badRequestError(ErrorCodeValidationFailed, "Cannot create a user without either an email or phone")
}

var providers []string
Expand All @@ -320,7 +320,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error {
if user, err := models.IsDuplicatedEmail(db, params.Email, aud, nil); err != nil {
return internalServerError("Database error checking email").WithInternalError(err)
} else if user != nil {
return unprocessableEntityError(DuplicateEmailMsg)
return unprocessableEntityError(ErrorCodeEmailExists, DuplicateEmailMsg)
}
providers = append(providers, "email")
}
Expand All @@ -333,7 +333,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error {
if exists, err := models.IsDuplicatedPhone(db, params.Phone, aud); err != nil {
return internalServerError("Database error checking phone").WithInternalError(err)
} else if exists {
return unprocessableEntityError("Phone number already registered by another user")
return unprocessableEntityError(ErrorCodePhoneExists, "Phone number already registered by another user")
}
providers = append(providers, "phone")
}
Expand Down Expand Up @@ -429,7 +429,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error {
if params.BanDuration != "none" {
duration, err = time.ParseDuration(params.BanDuration)
if err != nil {
return badRequestError("invalid format for ban duration: %v", err)
return badRequestError(ErrorCodeValidationFailed, "invalid format for ban duration: %v", err)
}
}
if terr := user.Ban(a.db, duration); terr != nil {
Expand Down Expand Up @@ -460,11 +460,11 @@ func (a *API) adminUserDelete(w http.ResponseWriter, r *http.Request) error {
params := &adminUserDeleteParams{}
body, err := getBodyBytes(r)
if err != nil {
return badRequestError("Could not read body").WithInternalError(err)
return internalServerError("Could not read body").WithInternalError(err)
}
if len(body) > 0 {
if err := json.Unmarshal(body, params); err != nil {
return badRequestError("Could not read params: %v", err)
return badRequestError(ErrorCodeBadJSON, "Could not read params: %v", err)
}
} else {
params.ShouldSoftDelete = false
Expand Down Expand Up @@ -559,6 +559,7 @@ func (a *API) adminUserUpdateFactor(w http.ResponseWriter, r *http.Request) erro
user := getUser(ctx)
adminUser := getAdminUser(ctx)
params := &adminUserUpdateFactorParams{}

if err := retrieveRequestParams(r, params); err != nil {
return err
}
Expand All @@ -571,7 +572,7 @@ func (a *API) adminUserUpdateFactor(w http.ResponseWriter, r *http.Request) erro
}
if params.FactorType != "" {
if params.FactorType != models.TOTP {
return badRequestError("Factor Type not valid")
return badRequestError(ErrorCodeValidationFailed, "Factor Type not valid")
}
if terr := factor.UpdateFactorType(tx, params.FactorType); terr != nil {
return terr
Expand Down
2 changes: 1 addition & 1 deletion internal/api/anonymous.go
Expand Up @@ -15,7 +15,7 @@ func (a *API) SignupAnonymously(w http.ResponseWriter, r *http.Request) error {
aud := a.requestAud(ctx, r)

if config.DisableSignup {
return forbiddenError("Signups not allowed for this instance")
return unprocessableEntityError(ErrorCodeSignupDisabled, "Signups not allowed for this instance")
}

params := &SignupParams{}
Expand Down
2 changes: 1 addition & 1 deletion internal/api/api.go
Expand Up @@ -155,7 +155,7 @@ func NewAPIWithVersion(ctx context.Context, globalConfig *conf.GlobalConfigurati
}
if params.Email == "" && params.Phone == "" {
if !api.config.External.AnonymousUsers.Enabled {
return unprocessableEntityError("Anonymous sign-ins are disabled")
return unprocessableEntityError(ErrorCodeAnonymousProviderDisabled, "Anonymous sign-ins are disabled")
}
if _, err := api.limitHandler(limiter)(w, r); err != nil {
return err
Expand Down
35 changes: 35 additions & 0 deletions internal/api/apiversions.go
@@ -0,0 +1,35 @@
package api

import (
"time"
)

const APIVersionHeaderName = "X-Supabase-Api-Version"

type APIVersion = time.Time

var (
APIVersionInitial = time.Time{}
APIVersion20240101 = time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC)
)

func DetermineClosestAPIVersion(date string) (APIVersion, error) {
if date == "" {
return APIVersionInitial, nil
}

parsed, err := time.ParseInLocation("2006-01-02", date, time.UTC)
if err != nil {
return APIVersionInitial, err
}

if parsed.Compare(APIVersion20240101) >= 0 {
return APIVersion20240101, nil
}

return APIVersionInitial, nil
}

func FormatAPIVersion(apiVersion APIVersion) string {
return apiVersion.Format("2006-01-02")
}
29 changes: 29 additions & 0 deletions internal/api/apiversions_test.go
@@ -0,0 +1,29 @@
package api

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestDetermineClosestAPIVersion(t *testing.T) {
version, err := DetermineClosestAPIVersion("")
require.NoError(t, err)
require.Equal(t, APIVersionInitial, version)

version, err = DetermineClosestAPIVersion("Not a date")
require.Error(t, err)
require.Equal(t, APIVersionInitial, version)

version, err = DetermineClosestAPIVersion("2023-12-31")
require.NoError(t, err)
require.Equal(t, APIVersionInitial, version)

version, err = DetermineClosestAPIVersion("2024-01-01")
require.NoError(t, err)
require.Equal(t, APIVersion20240101, version)

version, err = DetermineClosestAPIVersion("2024-01-02")
require.NoError(t, err)
require.Equal(t, APIVersion20240101, version)
}
4 changes: 2 additions & 2 deletions internal/api/audit.go
Expand Up @@ -20,7 +20,7 @@ func (a *API) adminAuditLog(w http.ResponseWriter, r *http.Request) error {
// aud := a.requestAud(ctx, r)
pageParams, err := paginate(r)
if err != nil {
return badRequestError("Bad Pagination Parameters: %v", err)
return badRequestError(ErrorCodeValidationFailed, "Bad Pagination Parameters: %v", err)
}

var col []string
Expand All @@ -31,7 +31,7 @@ func (a *API) adminAuditLog(w http.ResponseWriter, r *http.Request) error {
qparts := strings.SplitN(q, ":", 2)
col, exists = filterColumnMap[qparts[0]]
if !exists || len(qparts) < 2 {
return badRequestError("Invalid query scope: %s", q)
return badRequestError(ErrorCodeValidationFailed, "Invalid query scope: %s", q)
}
qval = qparts[1]
}
Expand Down
22 changes: 11 additions & 11 deletions internal/api/auth.go
Expand Up @@ -39,7 +39,7 @@ func (a *API) requireNotAnonymous(w http.ResponseWriter, r *http.Request) (conte
ctx := r.Context()
claims := getClaims(ctx)
if claims.IsAnonymous {
return nil, forbiddenError("Anonymous user not allowed to perform these actions")
return nil, forbiddenError(ErrorCodeNoAuthorization, "Anonymous user not allowed to perform these actions")
}
return ctx, nil
}
Expand All @@ -49,7 +49,7 @@ func (a *API) requireAdmin(ctx context.Context, r *http.Request) (context.Contex
claims := getClaims(ctx)
if claims == nil {
fmt.Printf("[%s] %s %s %d %s\n", time.Now().Format("2006-01-02 15:04:05"), r.Method, r.RequestURI, http.StatusForbidden, "Invalid token")
return nil, unauthorizedError("Invalid token")
return nil, forbiddenError(ErrorCodeBadJWT, "Invalid token")
}

adminRoles := a.config.JWT.AdminRoles
Expand All @@ -60,14 +60,14 @@ func (a *API) requireAdmin(ctx context.Context, r *http.Request) (context.Contex
}

fmt.Printf("[%s] %s %s %d %s\n", time.Now().Format("2006-01-02 15:04:05"), r.Method, r.RequestURI, http.StatusForbidden, "this token needs role 'supabase_admin' or 'service_role'")
return nil, unauthorizedError("User not allowed")
return nil, forbiddenError(ErrorCodeNotAdmin, "User not allowed")
}

func (a *API) extractBearerToken(r *http.Request) (string, error) {
authHeader := r.Header.Get("Authorization")
matches := bearerRegexp.FindStringSubmatch(authHeader)
if len(matches) != 2 {
return "", unauthorizedError("This endpoint requires a Bearer token")
return "", httpError(http.StatusUnauthorized, ErrorCodeNoAuthorization, "This endpoint requires a Bearer token")
}

return matches[1], nil
Expand All @@ -82,7 +82,7 @@ func (a *API) parseJWTClaims(bearer string, r *http.Request) (context.Context, e
return []byte(config.JWT.Secret), nil
})
if err != nil {
return nil, unauthorizedError("invalid JWT: unable to parse or verify signature, %v", err)
return nil, forbiddenError(ErrorCodeBadJWT, "invalid JWT: unable to parse or verify signature, %v", err).WithInternalError(err)
}

return withToken(ctx, token), nil
Expand All @@ -93,23 +93,23 @@ func (a *API) maybeLoadUserOrSession(ctx context.Context) (context.Context, erro
claims := getClaims(ctx)

if claims == nil {
return ctx, unauthorizedError("invalid token: missing claims")
return ctx, forbiddenError(ErrorCodeBadJWT, "invalid token: missing claims")
}

if claims.Subject == "" {
return nil, unauthorizedError("invalid claim: missing sub claim")
return nil, forbiddenError(ErrorCodeBadJWT, "invalid claim: missing sub claim")
}

var user *models.User
if claims.Subject != "" {
userId, err := uuid.FromString(claims.Subject)
if err != nil {
return ctx, badRequestError("invalid claim: sub claim must be a UUID").WithInternalError(err)
return ctx, badRequestError(ErrorCodeBadJWT, "invalid claim: sub claim must be a UUID").WithInternalError(err)
}
user, err = models.FindUserByID(db, userId)
if err != nil {
if models.IsNotFoundError(err) {
return ctx, notFoundError(err.Error())
return ctx, forbiddenError(ErrorCodeUserNotFound, "User from sub claim in JWT does not exist")
}
return ctx, err
}
Expand All @@ -120,11 +120,11 @@ func (a *API) maybeLoadUserOrSession(ctx context.Context) (context.Context, erro
if claims.SessionId != "" && claims.SessionId != uuid.Nil.String() {
sessionId, err := uuid.FromString(claims.SessionId)
if err != nil {
return ctx, err
return ctx, forbiddenError(ErrorCodeBadJWT, "invalid claim: session_id claim must be a UUID").WithInternalError(err)
}
session, err = models.FindSessionByID(db, sessionId, false)
if err != nil && !models.IsNotFoundError(err) {
return ctx, err
return ctx, forbiddenError(ErrorCodeSessionNotFound, "Session from session_id claim in JWT does not exist")
}
ctx = withSession(ctx, session)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/api/auth_test.go
Expand Up @@ -96,7 +96,7 @@ func (ts *AuthTestSuite) TestMaybeLoadUserOrSession() {
},
Role: "authenticated",
},
ExpectedError: unauthorizedError("invalid claim: missing sub claim"),
ExpectedError: forbiddenError(ErrorCodeBadJWT, "invalid claim: missing sub claim"),
ExpectedUser: nil,
},
{
Expand All @@ -118,7 +118,7 @@ func (ts *AuthTestSuite) TestMaybeLoadUserOrSession() {
},
Role: "authenticated",
},
ExpectedError: badRequestError("invalid claim: sub claim must be a UUID"),
ExpectedError: badRequestError(ErrorCodeBadJWT, "invalid claim: sub claim must be a UUID"),
ExpectedUser: nil,
},
{
Expand Down

0 comments on commit e4beea1

Please sign in to comment.