validate_all_requests feature, to be used in development and staging …

…environments

docs/json-schema/query_params.json

@@ -1,6 +1,10 @@
 {
   "$comment" : "NOTE: This file is for human reference ONLY. For programmatic use, use the GET '/json_schema/query_params/$schema_name' endpoints, or within conch itself, json-schema/query_params.yaml.\nNote that all parameters are parsed internally from the request URI as strings, so all type checks here use strings. When a query parameter is used more than once, its values are parsed as an arrayref. See ../modules/Conch::Plugin::JSONValidator#validate_query_params.",
   "$defs" : {
+    "Anything" : {
+      "additionalProperties" : true,
+      "type" : "object"
+    },
     "BuildDevices" : {
       "$ref" : "#/$defs/FindDevice",
       "allOf" : [
@@ -246,6 +250,10 @@
       },
       "type" : "object"
     },
+    "Null" : {
+      "additionalProperties" : false,
+      "type" : "object"
+    },
     "ProcessDeviceReport" : {
       "additionalProperties" : false,
       "default" : {

docs/json-schema/request.json

@@ -1,6 +1,7 @@
 {
   "$comment" : "NOTE: This file is for human reference ONLY. For programmatic use, use the GET '/json_schema/request/$schema_name' endpoints, or within conch itself, json-schema/request.yaml.",
   "$defs" : {
+    "Anything" : true,
     "BuildAddOrganization" : {
       "additionalProperties" : false,
       "properties" : {

docs/modules/Conch::Plugin::JSONValidator.md

@@ -80,6 +80,14 @@ Before a controller action is executed, validate the incoming query parameters a
 payloads against the schemas in the stash variables `query_params_schema` and
 `request_schema`, respectively.
 
+Performs more checks when this ["feature" in Conch::Plugin::Features](../modules/Conch%3A%3APlugin%3A%3AFeatures#feature) is enabled:
+
+- `validate_all_requests`
+
+    Assumes the query parameters schema is [query_params.json#/$defs/Null](../json-schema/query_params.json#/$defs/Null) when not provided;
+    assumes the request body schema is [request.json#/$defs/Null](../json-schema/request.json#/$defs/Null) when not provided (for
+    `POST`, `PUT`, `DELETE` requests)
+
 ## LICENSING
 
 Copyright Joyent, Inc.

json-schema/query_params.yaml

@@ -14,6 +14,12 @@ $defs:
     type: string
     pattern: '^[0-9]+$'
 
+  'Null':
+    type: object
+    additionalProperties: false
+  Anything:
+    type: object
+    additionalProperties: true
   RevokeUserTokens:
     type: object
     additionalProperties: false

json-schema/request.yaml

@@ -3,6 +3,8 @@ $schema: 'https://json-schema.org/draft/2019-09/schema'
 $defs:
   'Null':
     type: 'null'
+  Anything:
+    true
   DatacenterCreate:
     type: object
     additionalProperties: false

lib/Conch/Controller/HardwareProduct.pm

@@ -192,7 +192,7 @@ F<common.yaml#/$defs/HardwareProductSpecification>.
 sub set_specification ($c) {
   my $hardware_product_id = $c->stash('hardware_product_rs')->get_column('id')->single;
   my $rs = $c->db_hardware_products->search({ id => $hardware_product_id });
-  my $json = to_json($c->req->json);
+  my $json = to_json($c->stash('request_data'));
   my $jsonp = $c->stash('query_params')->{path};
 
   my $specification_clause = $jsonp ? do {

lib/Conch/Plugin/JSONValidator.pm

@@ -7,7 +7,7 @@ use JSON::Schema::Draft201909 '0.017';
 use YAML::PP;
 use Mojo::JSON 'to_json';
 use Path::Tiny;
-use List::Util 'none';
+use List::Util qw(any none);
 
 =pod
 
@@ -178,13 +178,29 @@ Before a controller action is executed, validate the incoming query parameters a
 payloads against the schemas in the stash variables C<query_params_schema> and
 C<request_schema>, respectively.
 
+Performs more checks when this L<Conch::Plugin::Features/feature> is enabled:
+
+=over 4
+
+=item * C<validate_all_requests>
+
+Assumes the query parameters schema is F<query_params.yaml#/$defs/Null> when not provided;
+assumes the request body schema is F<request.yaml#/$defs/Null> when not provided (for
+C<POST>, C<PUT>, C<DELETE> requests)
+
+=back
+
 =cut
 
     $app->hook(around_action => sub ($next, $c, $action, $last) {
         $c->stash('validated', { map +($_.'_schema' => []), qw(query_params request) })
             if not $c->stash('validated');
 
         my $query_params_schema = $c->stash('query_params_schema');
+        $query_params_schema = 'Null'
+            if not $query_params_schema and $last and not $c->stash('query_params')
+                and $c->feature('validate_all_requests');
+
         if ($query_params_schema
                 and none { $_ eq $query_params_schema } $c->stash('validated')->{query_params_schema}->@*) {
             my $query_params = $c->req->query_params->to_hash;
@@ -200,9 +216,15 @@ C<request_schema>, respectively.
         # is application/schema+json or application/schema-instance+json
 
         my $request_schema = $c->stash('request_schema');
+        my $method = $c->req->method;
+        $request_schema = 'Null'
+            if not $request_schema and $last and not $c->stash('request_data')
+                and $c->feature('validate_all_requests');
+
         if ($request_schema
+                and any { $method eq $_ } qw(POST PUT DELETE)
                 and none { $_ eq $request_schema } $c->stash('validated')->{request_schema}->@*) {
-            my $request_data = $c->req->json;
+            my $request_data = ($c->req->headers->content_type // '') =~ m{application/(?:[a-z-]+\+)?json}i ? $c->req->json : undef;
             return if not $c->validate_request($request_schema, $request_data);
             $c->stash('request_data', $request_data);
 

lib/Conch/Route.pm

@@ -169,7 +169,7 @@ Returns the root node.
     my @top_level_paths = (uniqstr (map find_paths($_), $root->children->@*),
         qw(validation workspace));
 
-    $root->any('/*all', sub ($c) {
+    $root->any('/*all', { query_params_schema => 'Anything', request_schema => 'Anything' }, sub ($c) {
         $c->log->warn('no endpoint found for: '.$c->req->method.' '.$c->req->url->path);
 
         if (any { $c->req->url->path =~ m{^/$_\b} } @top_level_paths) {

lib/Conch/Route/Device.pm

@@ -21,7 +21,8 @@ sub routes {
     my $device = shift; # secured, under /device
     my $app = shift;
 
-    $device->post('/:device_serial_number', sub { shift->status(308, '/device_report') });
+    $device->post('/:device_serial_number', { request_schema => 'Anything' },
+        sub { shift->status(308, '/device_report') });
 
     # GET /device?:key=:value
     $device->get('/')

lib/Conch/Route/HardwareProduct.pm

@@ -31,7 +31,8 @@ sub routes {
 
     {
         $hardware_product->any('/<:hardware_product_key>=<:hardware_product_value>/*optional',
-                [ hardware_product_key => [qw(name alias sku)] ], { optional => '' },
+                [ hardware_product_key => [qw(name alias sku)] ],
+                { optional => '', query_params_schema => 'Anything', request_schema => 'Anything' },
             sub ($c) {
                 $c->req->url->query->pairs;  # force normalization
                 $c->status(308, $c->req->url->path_query =~ s/(?:name|alias|sku)=//r)
@@ -52,7 +53,7 @@ sub routes {
         $hwp_with_admin->delete('/')->to('#delete');
 
         # PUT /hardware_product/:hardware_product_id_or_other/specification?path=:json_pointer_to_data
-        $hwp_with_admin->put('/specification')->to('#set_specification', query_params_schema => 'HardwareProductSpecification');
+        $hwp_with_admin->put('/specification')->to('#set_specification', query_params_schema => 'HardwareProductSpecification', request_schema => 'Anything');
 
         # DELETE /hardware_product/:hardware_product_id_or_other/specification?path=:json_pointer_to_data
         $hwp_with_admin->delete('/specification')->to('#delete_specification', query_params_schema => 'HardwareProductSpecification');

lib/Conch/Route/Rack.pm

@@ -68,7 +68,8 @@ sub one_rack_routes ($class, $r) {
     $one_rack->get('/layout', 'get_layouts')->to('#get_layouts');
 
     # POST .../rack/:rack_id_or_name/layout
-    $one_rack->post('/layouts', sub { shift->status(308, 'overwrite_layouts') });
+    $one_rack->post('/layouts', { request_schema => 'Anything' },
+        sub { shift->status(308, 'overwrite_layouts') });
     $one_rack->post('/layout', 'overwrite_layouts')
         ->to('#overwrite_layouts', request_schema => 'RackLayouts');
 

lib/Test/Conch.pm

@@ -104,6 +104,7 @@ sub new {
             mode => $ENV{MOJO_MODE} // 'test',
             features => {
                 no_db => ($pg ? 0 : 1),
+                validate_all_requests => 1,
                 ($args->{config}//{})->{features} ? delete($args->{config}{features})->%* : (),
             },
             database => {

t/conch-log.t

@@ -150,11 +150,11 @@ sub add_test_routes ($t) {
         $c->log->debug('this is a debug message');
         $c->status(204);
     });
-    $r->post('/_error', sub ($c) {
+    $r->post('/_error', { query_params_schema => 'Anything', request_schema => 'Anything' }, sub ($c) {
         $c->log->error('error line from controller');
         $c->status(400, { error => 'something bad happened' });
     });
-    $r->post('/_die', sub ($c) { die 'ach, I am slain' });
+    $r->post('/_die', { query_params_schema => 'Anything', request_schema => 'Anything' }, sub ($c) { die 'ach, I am slain' });
     $t->add_routes($r);
 
     return (warn => __LINE__-11, debug => __LINE__-10, error => __LINE__-6, die => __LINE__-3);

t/conch-rollbar.t

@@ -23,7 +23,7 @@ my $api_version_re = qr/^${ Test::Conch->API_VERSION_RE }$/;
 
 my $t = Test::Conch->new(
     config => {
-        features => { rollbar => 1, no_db => 1 },
+        features => { rollbar => 1, no_db => 1, validate_all_requests => 0 },
         rollbar => {
             access_token => 'TOKEN',
             environment => 'custom_environment',
@@ -91,6 +91,7 @@ package RollbarSimulator {
             $c->res->code(200);
             $c->render(json => { err => 0, result => { id => undef, uuid => $payload->{data}{uuid} } });
         });
+        $self->plugin('Conch::Plugin::Features', $self->config);
         $self->plugin('Conch::Plugin::JSONValidator', {});
     }
 }

t/database.t

@@ -120,7 +120,7 @@ subtest 'read-only database handle' => sub {
 };
 
 subtest 'transactions' => sub {
-    my $t = Test::Conch->new;
+    my $t = Test::Conch->new(config => { features => { validate_all_requests => 0 } });
 
     my $user_count = $t->app->db_user_accounts->count;
 

t/json-validation.t

@@ -15,10 +15,11 @@ my $base_uri = $t->ua->server->url; # used as the base uri for all requests
 
 subtest 'failed query params validation' => sub {
     my $r = Mojolicious::Routes->new;
-    $r->get('/_hello', sub ($c) {
-        return if not $c->validate_query_params('ChangePassword');
-        return $c->status(200);
-    });
+    $r->get('/_hello', { query_params_schema => 'Anything' },
+        sub ($c) {
+            return if not $c->validate_query_params('ChangePassword');
+            return $c->status(200);
+        });
     $t->add_routes($r);
 
     $t->get_ok('/_hello?clear_tokens=whargarbl')
@@ -367,4 +368,64 @@ subtest 'automatic validation of query params and request payloads' => sub {
         ->stash_cmp_deeply('/validated', { query_params_schema => [qw(FindDevice GetValidationState)], request_schema => [qw(UserIdOrEmail BuildAddUser)] });
 };
 
+subtest 'feature: validate_all_requests' => sub {
+    my sub add_routes ($t) {
+        my $r = Mojolicious::Routes->new;
+        $r->post('/_simple_post', sub ($c) { $c->status(204) });
+        $t->add_routes($r);
+    }
+
+    my $t = Test::Conch->new(pg => undef);
+    my $base_uri = $t->ua->server->url; # used as the base uri for all requests
+    add_routes($t);
+
+    $t->post_ok('/_simple_post?foo=1')
+        ->status_is(400)
+        ->json_schema_is('QueryParamsValidationError')
+        ->json_cmp_deeply({
+            error => 'query parameters did not match required format',
+            details => [ superhashof({ error => 'additional property not permitted' }) ],
+            schema => $base_uri.'json_schema/query_params/Null',
+            data => { foo => 1 },
+        })
+        ->log_warn_like(qr{^FAILED query_params validation for schema Null: .*additional property not permitted})
+        ->stash_cmp_deeply('/validated', { query_params_schema => [], request_schema => [] });
+
+    $t->post_ok('/_simple_post', json => $_)
+        ->status_is(400)
+        ->json_schema_is('RequestValidationError')
+        ->json_is({
+            error => 'request did not match required format',
+            details => [{
+                data_location => '',
+                schema_location => '/type',
+                absolute_schema_location => $base_uri.'json_schema/request/Null#/type',
+                error => 'wrong type (expected null)',
+            }],
+            schema => $base_uri.'json_schema/request/Null',
+        })
+        ->log_debug_is('Passed data validation for query_params schema Null')
+        ->log_warn_like(qr{^FAILED request payload validation for schema Null: .*wrong type \(expected null\)})
+        ->stash_cmp_deeply('/validated', { query_params_schema => ['Null'], request_schema => [] })
+    foreach
+        JSON::PP::true,
+        1,
+        'bad payload',
+        { bad => 'payload' },
+        [ qw(bad payload) ],
+    ;
+
+    undef $t;
+    my $t2 = Test::Conch->new(config => { features => { no_db => 1, validate_all_requests => 0 } });
+    add_routes($t2);
+
+    $t2->post_ok('/_simple_post?foo=1')
+        ->status_is(204)
+        ->stash_cmp_deeply('/validated', { query_params_schema => [], request_schema => [] });
+
+    $t2->post_ok('/_simple_post', json => { foo => 'bar' })
+        ->status_is(204)
+        ->stash_cmp_deeply('/validated', { query_params_schema => [], request_schema => [] });
+};
+
 done_testing;