From eedf6e9bec2162c6b0204020b44f90d3049bad53 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 8 Jul 2024 17:21:25 -0300 Subject: [PATCH] [POC] Bug 37018: Add 400 to all GET routes --- Koha/REST/Plugin/Exceptions.pm | 2 +- api/v1/swagger/paths/cities.yaml | 7 ++++ api/v1/swagger/paths/patrons.yaml | 7 ++++ api/v1/swagger/paths/preservation_trains.yaml | 35 +++++++++++++++---- .../paths/preservation_waiting_list.yaml | 5 ++- api/v1/swagger/paths/public_oauth.yaml | 7 ++-- api/v1/swagger/paths/quotes.yaml | 14 ++++++++ api/v1/swagger/paths/record_sources.yaml | 10 ++++-- api/v1/swagger/paths/rotas.yaml | 14 ++++++++ api/v1/swagger/paths/search_filters.yaml | 14 ++++++++ api/v1/swagger/paths/suggestions.yaml | 21 +++++++++++ api/v1/swagger/paths/tickets.yaml | 21 +++++++++++ api/v1/swagger/paths/transfer_limits.yaml | 5 ++- t/db_dependent/api/v1/query.t | 2 +- xt/api.t | 34 +++++++++++++++++- 15 files changed, 183 insertions(+), 15 deletions(-) diff --git a/Koha/REST/Plugin/Exceptions.pm b/Koha/REST/Plugin/Exceptions.pm index 22f0b314430..5107af103d5 100644 --- a/Koha/REST/Plugin/Exceptions.pm +++ b/Koha/REST/Plugin/Exceptions.pm @@ -61,7 +61,7 @@ sub register { if ( blessed $exception && ref($exception) eq 'Koha::Exceptions::REST::Query::InvalidOperator' ) { return $c->render( - status => 500, + status => 400, json => { error => printf( "Invalid operator in query: %s", $exception->operator ), error_code => 'invalid_query', diff --git a/api/v1/swagger/paths/cities.yaml b/api/v1/swagger/paths/cities.yaml index b7e43a32817..fb1e7aa006c 100644 --- a/api/v1/swagger/paths/cities.yaml +++ b/api/v1/swagger/paths/cities.yaml @@ -43,6 +43,13 @@ type: array items: $ref: "../swagger.yaml#/definitions/city" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: diff --git a/api/v1/swagger/paths/patrons.yaml b/api/v1/swagger/paths/patrons.yaml index 26ef12e0ce0..ab9a39e135f 100644 --- a/api/v1/swagger/paths/patrons.yaml +++ b/api/v1/swagger/paths/patrons.yaml @@ -372,6 +372,13 @@ type: array items: $ref: "../swagger.yaml#/definitions/patron" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "401": description: Authentication required schema: diff --git a/api/v1/swagger/paths/preservation_trains.yaml b/api/v1/swagger/paths/preservation_trains.yaml index 9644581fd7c..88910719ede 100644 --- a/api/v1/swagger/paths/preservation_trains.yaml +++ b/api/v1/swagger/paths/preservation_trains.yaml @@ -62,8 +62,11 @@ items: $ref: "../swagger.yaml#/definitions/preservation_train" type: array - 400: - description: Bad request + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` schema: $ref: "../swagger.yaml#/definitions/error" 403: @@ -178,6 +181,13 @@ schema: items: $ref: "../swagger.yaml#/definitions/preservation_train" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" 401: description: Authentication required schema: @@ -228,8 +238,11 @@ schema: items: $ref: "../swagger.yaml#/definitions/preservation_train" - 400: - description: Bad parameter + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` schema: $ref: "../swagger.yaml#/definitions/error" 403: @@ -399,8 +412,11 @@ type: array items: type: object - 400: - description: Bad parameter + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` schema: $ref: "../swagger.yaml#/definitions/error" 401: @@ -578,6 +594,13 @@ description: An item in train schema: $ref: "../swagger.yaml#/definitions/preservation_train_item" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" 401: description: Authentication required schema: diff --git a/api/v1/swagger/paths/preservation_waiting_list.yaml b/api/v1/swagger/paths/preservation_waiting_list.yaml index 26f1ef65486..e3e2d7fe6df 100644 --- a/api/v1/swagger/paths/preservation_waiting_list.yaml +++ b/api/v1/swagger/paths/preservation_waiting_list.yaml @@ -41,7 +41,10 @@ items: $ref: "../swagger.yaml#/definitions/item" "400": - description: Bad parameter + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` schema: $ref: "../swagger.yaml#/definitions/error" "401": diff --git a/api/v1/swagger/paths/public_oauth.yaml b/api/v1/swagger/paths/public_oauth.yaml index 80ac4e3741d..75bcd10666a 100644 --- a/api/v1/swagger/paths/public_oauth.yaml +++ b/api/v1/swagger/paths/public_oauth.yaml @@ -63,9 +63,12 @@ schema: type: string "400": - description: Bad Request + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` schema: - $ref: ../swagger.yaml#/definitions/error + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: diff --git a/api/v1/swagger/paths/quotes.yaml b/api/v1/swagger/paths/quotes.yaml index 901b7af9337..127d2780b81 100644 --- a/api/v1/swagger/paths/quotes.yaml +++ b/api/v1/swagger/paths/quotes.yaml @@ -43,6 +43,13 @@ type: array items: $ref: "../swagger.yaml#/definitions/quote" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: @@ -119,6 +126,13 @@ description: A Quote schema: $ref: "../swagger.yaml#/definitions/quote" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "404": description: Quote not found schema: diff --git a/api/v1/swagger/paths/record_sources.yaml b/api/v1/swagger/paths/record_sources.yaml index c1b1c182d8e..f2e3c2d8175 100644 --- a/api/v1/swagger/paths/record_sources.yaml +++ b/api/v1/swagger/paths/record_sources.yaml @@ -25,7 +25,10 @@ items: $ref: "../swagger.yaml#/definitions/record_source" "400": - description: Missing or wrong parameters + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` schema: $ref: "../swagger.yaml#/definitions/error" "401": @@ -117,7 +120,10 @@ schema: $ref: "../swagger.yaml#/definitions/record_source" "400": - description: Missing or wrong parameters + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` schema: $ref: "../swagger.yaml#/definitions/error" "401": diff --git a/api/v1/swagger/paths/rotas.yaml b/api/v1/swagger/paths/rotas.yaml index 69c079afc75..118d65d2b89 100644 --- a/api/v1/swagger/paths/rotas.yaml +++ b/api/v1/swagger/paths/rotas.yaml @@ -23,6 +23,13 @@ type: array items: $ref: "../swagger.yaml#/definitions/rota" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: @@ -99,6 +106,13 @@ description: A rota schema: $ref: "../swagger.yaml#/definitions/rota" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "404": description: Rota not found schema: diff --git a/api/v1/swagger/paths/search_filters.yaml b/api/v1/swagger/paths/search_filters.yaml index 1948ad86af2..85e34c0276a 100644 --- a/api/v1/swagger/paths/search_filters.yaml +++ b/api/v1/swagger/paths/search_filters.yaml @@ -23,6 +23,13 @@ type: array items: $ref: "../swagger.yaml#/definitions/search_filter" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" '403': description: Access forbidden schema: @@ -97,6 +104,13 @@ description: A search filter schema: $ref: "../swagger.yaml#/definitions/search_filter" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" '403': description: Access forbidden schema: diff --git a/api/v1/swagger/paths/suggestions.yaml b/api/v1/swagger/paths/suggestions.yaml index 6e500bcf4cb..ad6a928097a 100644 --- a/api/v1/swagger/paths/suggestions.yaml +++ b/api/v1/swagger/paths/suggestions.yaml @@ -24,6 +24,13 @@ type: array items: $ref: "../swagger.yaml#/definitions/suggestion" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: @@ -118,6 +125,13 @@ description: A suggestion schema: $ref: "../swagger.yaml#/definitions/suggestion" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: @@ -264,6 +278,13 @@ type: array items: $ref: "../swagger.yaml#/definitions/patron" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: diff --git a/api/v1/swagger/paths/tickets.yaml b/api/v1/swagger/paths/tickets.yaml index a76239c66f4..83a172ae01b 100644 --- a/api/v1/swagger/paths/tickets.yaml +++ b/api/v1/swagger/paths/tickets.yaml @@ -38,6 +38,13 @@ type: array items: $ref: "../swagger.yaml#/definitions/ticket" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: @@ -114,6 +121,13 @@ description: A ticket schema: $ref: "../swagger.yaml#/definitions/ticket" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "404": description: Ticket not found schema: @@ -249,6 +263,13 @@ type: array items: $ref: "../swagger.yaml#/definitions/ticket_update" + "400": + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` + schema: + $ref: "../swagger.yaml#/definitions/error" "403": description: Access forbidden schema: diff --git a/api/v1/swagger/paths/transfer_limits.yaml b/api/v1/swagger/paths/transfer_limits.yaml index e6f60186028..c53145edde6 100644 --- a/api/v1/swagger/paths/transfer_limits.yaml +++ b/api/v1/swagger/paths/transfer_limits.yaml @@ -44,7 +44,10 @@ items: $ref: "../swagger.yaml#/definitions/transfer_limit" "400": - description: Bad request + description: | + Bad request. Possible `error_code` attribute values: + + * `invalid_query` schema: $ref: "../swagger.yaml#/definitions/error" "401": diff --git a/t/db_dependent/api/v1/query.t b/t/db_dependent/api/v1/query.t index 353760ca996..f807f21ea2b 100755 --- a/t/db_dependent/api/v1/query.t +++ b/t/db_dependent/api/v1/query.t @@ -171,7 +171,7 @@ subtest 'SQL injection in "query" handling' => sub { "[{\"-and\":[[{\"me.patron_id\":{\"like(IF(ASCII(SUBSTRING((SELECT version()),1,1))=ASCII('1'),SLEEP(1/100000),0))or\":\"\%a\%\"}}]]}]"; $t->get_ok("//$userid:$password@/api/v1/patrons?q=$q") - ->status_is( 500, 'Attempt to inject SQL through operators is rejected' ); + ->status_is( 400, 'Attempt to inject SQL through operators is rejected' ); $schema->storage->txn_rollback; }; diff --git a/xt/api.t b/xt/api.t index 3165f8ff6e5..61e3016fdb5 100755 --- a/xt/api.t +++ b/xt/api.t @@ -14,7 +14,7 @@ use Modern::Perl; -use Test::More tests => 4; +use Test::More tests => 5; use Test::Mojo; use Data::Dumper; @@ -128,3 +128,35 @@ subtest 'tags tests' => sub { print STDERR "$error\n"; } }; + +subtest '400 response tests' => sub { + + plan tests => 1; + + my @errors; + + foreach my $route ( sort keys %{$paths} ) { + foreach my $verb ( keys %{ $paths->{$route} } ) { + + next unless $verb eq 'get'; + + my $response_400 = $paths->{$route}->{$verb}->{responses}->{400}; + + push @errors, "$verb $route -> 'description' does not include 'Bad request': ($response_400->{description})" + unless $response_400->{description} =~ /^Bad request/; + + push @errors, + "$verb $route -> 'description' does not include '* invalid_query': ($response_400->{description})" + unless $response_400->{description} =~ /\* \`invalid_query\`/; + + push @errors, "$verb $route -> '\$ref' is not '#/definitions/error'" + unless $response_400->{schema}->{'$ref'} eq '#/definitions/error'; + } + } + + is( scalar @errors, 0, 'No 400 definitions missing for GET in the spec' ); + + foreach my $error (@errors) { + print STDERR "$error\n"; + } +}; -- 2.45.2