From 81628e5069e0168b11857f276fe8e03b93102dde Mon Sep 17 00:00:00 2001 From: Rigel Kent Date: Wed, 2 Jun 2021 14:28:30 +0200 Subject: [PATCH] refactor error code values for URI compatibility --- .../middlewares/validators/videos/videos.ts | 6 +-- server/tests/api/users/users.ts | 18 +++---- .../models/server/server-error-code.enum.ts | 54 +++++++++++++++++-- support/doc/api/openapi.yaml | 38 ++++++++++--- 4 files changed, 93 insertions(+), 23 deletions(-) diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index fd0e543f1..64e09234e 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -252,7 +252,7 @@ async function checkVideoFollowConstraints (req: express.Request, res: express.R return res.fail({ status: HttpStatusCode.FORBIDDEN_403, - message: 'Cannot get this video regarding follow constraints.', + message: 'Cannot get this video regarding follow constraints', type: ServerErrorCode.DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS, data: { originUrl: video.url @@ -285,11 +285,11 @@ const videosCustomGetValidator = ( const user = res.locals.oauth ? res.locals.oauth.token.User : null - // Only the owner or a user that have blacklist rights can see the video + // Only the owner or a user that have blocklist rights can see the video if (!user || !user.canGetVideo(video)) { return res.fail({ status: HttpStatusCode.FORBIDDEN_403, - message: 'Cannot get this private/internal or blacklisted video.' + message: 'Cannot get this private/internal or blocklisted video' }) } diff --git a/server/tests/api/users/users.ts b/server/tests/api/users/users.ts index 464c11d34..87ba775f6 100644 --- a/server/tests/api/users/users.ts +++ b/server/tests/api/users/users.ts @@ -3,7 +3,7 @@ import 'mocha' import * as chai from 'chai' import { AbuseState, AbuseUpdate, MyUser, User, UserRole, Video, VideoPlaylistType } from '@shared/models' -import { CustomConfig } from '@shared/models/server' +import { CustomConfig, OAuth2ErrorCode } from '@shared/models/server' import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes' import { addVideoCommentThread, @@ -93,20 +93,20 @@ describe('Test users', function () { const client = { id: 'client', secret: server.client.secret } const res = await login(server.url, client, server.user, HttpStatusCode.BAD_REQUEST_400) - expect(res.body.code).to.equal('invalid_client') + expect(res.body.code).to.equal(OAuth2ErrorCode.INVALID_CLIENT) expect(res.body.error).to.contain('client is invalid') expect(res.body.type.startsWith('https://')).to.be.true - expect(res.body.type).to.contain('invalid_client') + expect(res.body.type).to.contain(OAuth2ErrorCode.INVALID_CLIENT) }) it('Should not login with an invalid client secret', async function () { const client = { id: server.client.id, secret: 'coucou' } const res = await login(server.url, client, server.user, HttpStatusCode.BAD_REQUEST_400) - expect(res.body.code).to.equal('invalid_client') + expect(res.body.code).to.equal(OAuth2ErrorCode.INVALID_CLIENT) expect(res.body.error).to.contain('client is invalid') expect(res.body.type.startsWith('https://')).to.be.true - expect(res.body.type).to.contain('invalid_client') + expect(res.body.type).to.contain(OAuth2ErrorCode.INVALID_CLIENT) }) }) @@ -116,20 +116,20 @@ describe('Test users', function () { const user = { username: 'captain crochet', password: server.user.password } const res = await login(server.url, server.client, user, HttpStatusCode.BAD_REQUEST_400) - expect(res.body.code).to.equal('invalid_grant') + expect(res.body.code).to.equal(OAuth2ErrorCode.INVALID_GRANT) expect(res.body.error).to.contain('credentials are invalid') expect(res.body.type.startsWith('https://')).to.be.true - expect(res.body.type).to.contain('invalid_grant') + expect(res.body.type).to.contain(OAuth2ErrorCode.INVALID_GRANT) }) it('Should not login with an invalid password', async function () { const user = { username: server.user.username, password: 'mew_three' } const res = await login(server.url, server.client, user, HttpStatusCode.BAD_REQUEST_400) - expect(res.body.code).to.equal('invalid_grant') + expect(res.body.code).to.equal(OAuth2ErrorCode.INVALID_GRANT) expect(res.body.error).to.contain('credentials are invalid') expect(res.body.type.startsWith('https://')).to.be.true - expect(res.body.type).to.contain('invalid_grant') + expect(res.body.type).to.contain(OAuth2ErrorCode.INVALID_GRANT) }) it('Should not be able to upload a video', async function () { diff --git a/shared/models/server/server-error-code.enum.ts b/shared/models/server/server-error-code.enum.ts index f9fe47830..93b9ce20d 100644 --- a/shared/models/server/server-error-code.enum.ts +++ b/shared/models/server/server-error-code.enum.ts @@ -1,6 +1,52 @@ export const enum ServerErrorCode { - DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS = 'DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS', - MAX_INSTANCE_LIVES_LIMIT_REACHED = 'MAX_INSTANCE_LIVES_LIMIT_REACHED', - MAX_USER_LIVES_LIMIT_REACHED = 'MAX_USER_LIVES_LIMIT_REACHED', - INCORRECT_FILES_IN_TORRENT = 'INCORRECT_FILES_IN_TORRENT' + /** + * Error yielded upon trying to access a video that is not federated, nor can + * be. This may be due to: remote videos on instances that are not followed by + * yours, and with your instance disallowing unknown instances being accessed. + */ + DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS = 'does_not_respect_follow_constraints', + + /** + * Pretty self-explanatory: the set maximum number of simultaneous lives was + * reached, and this error is typically there to inform the user trying to + * broadcast one. + */ + MAX_INSTANCE_LIVES_LIMIT_REACHED = 'max_instance_lives_limit_reached', + + /** + * Pretty self-explanatory: the set maximum number of simultaneous lives FOR + * THIS USER was reached, and this error is typically there to inform the user + * trying to broadcast one. + */ + MAX_USER_LIVES_LIMIT_REACHED = 'max_user_lives_limit_reached', + + /** + * A torrent should have at most one correct video file. Any more and we will + * not be able to choose automatically. + */ + INCORRECT_FILES_IN_TORRENT = 'incorrect_files_in_torrent' +} + +/** + * oauthjs/oauth2-server error codes + * @see https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 + **/ +export const enum OAuth2ErrorCode { + /** + * The provided authorization grant (e.g., authorization code, resource owner + * credentials) or refresh token is invalid, expired, revoked, does not match + * the redirection URI used in the authorization request, or was issued to + * another client. + * + * @see https://github.com/oauthjs/node-oauth2-server/blob/master/lib/errors/invalid-grant-error.js + */ + INVALID_GRANT = 'invalid_grant', + + /** + * Client authentication failed (e.g., unknown client, no client authentication + * included, or unsupported authentication method). + * + * @see https://github.com/oauthjs/node-oauth2-server/blob/master/lib/errors/invalid-client-error.js + */ + INVALID_CLIENT = 'invalid_client' } diff --git a/support/doc/api/openapi.yaml b/support/doc/api/openapi.yaml index 9f40d74c6..ae4855878 100644 --- a/support/doc/api/openapi.yaml +++ b/support/doc/api/openapi.yaml @@ -53,14 +53,30 @@ info: } ``` - We provide error types for [a growing number of cases](https://github.com/Chocobozzz/PeerTube/blob/develop/shared/models/server/server-error-code.enum.ts), - but it is still optional. + We provide error `type` values for [a growing number of cases](https://github.com/Chocobozzz/PeerTube/blob/develop/shared/models/server/server-error-code.enum.ts), + but it is still optional. Types are used to disambiguate errors that bear the same status code + and are non-obvious: + + ``` + HTTP 1.1 403 Forbidden + Content-Type: application/problem+json; charset=utf-8 + + { + "detail": "Cannot get this video regarding follow constraints", + "docs": "https://docs.joinpeertube.org/api-rest-reference.html#operation/getVideo", + "status": 403, + "title": "Forbidden", + "type": "https://docs.joinpeertube.org/api-rest-reference.html#section/Errors/does_not_respect_follow_constraints" + } + ``` + + Here a 403 error could otherwise mean that the video is private or blocklisted. ### Validation errors Each parameter is evaluated on its own against a set of rules before the route validator proceeds with potential testing involving parameter combinations. Errors coming from validation - errors appear earlier and benefit from a more detailed error type: + errors appear earlier and benefit from a more detailed error description: ``` HTTP 1.1 400 Bad Request @@ -89,6 +105,12 @@ info: `invalid-params..value` reports the value that didn't pass validation whose `invalid-params..msg` is about. + ### Deprecated error fields + + Some fields could be included with previous versions. They are still included but their use is deprecated: + - `error`: superseded by `detail` + - `code`: superseded by `type` (which is now an URI) + # Rate limits We are rate-limiting all endpoints of PeerTube's API. Custom values can be set by administrators: @@ -932,6 +954,12 @@ paths: type: integer minimum: 0 example: 1209600 + '400': + x-summary: client or credentials are invalid + description: | + Disambiguate via `type`: + - `invalid_client` for an unmatched `client_id` + - `invalid_grant` for unmatched credentials x-codeSamples: - lang: Shell source: | @@ -1812,8 +1840,6 @@ paths: application/json: schema: $ref: '#/components/schemas/VideoUploadResponse' - '400': - description: invalid file field, schedule date or parameter '403': description: video didn't pass upload filter '408': @@ -1918,8 +1944,6 @@ paths: schema: type: number example: 0 - '400': - description: invalid file field, schedule date or parameter '413': description: video file too large, due to quota, absolute max file size or concurrent partial upload limit '415': -- 2.41.0