From 231e9d7259034d991f184c9488da51a33954f8a6 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 14 Oct 2016 15:21:53 +0000 Subject: [PATCH] Bug 17428: QA Followup - Add readOnly marker to cityid This should prevent api consumers from specifying an id on PUT or POST requests and thus restrict them from creating new cities with id's of their choosing, or worse updating cities via the POST method --- api/v1/swagger/x-primitives.json | 3 +- t/db_dependent/api/v1/cities.t | 69 ++++++++++++++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/api/v1/swagger/x-primitives.json b/api/v1/swagger/x-primitives.json index f5ff2e9..1b90b30 100644 --- a/api/v1/swagger/x-primitives.json +++ b/api/v1/swagger/x-primitives.json @@ -17,7 +17,8 @@ }, "cityid": { "type": "string", - "description": "internally assigned city identifier" + "description": "internally assigned city identifier", + "readOnly": true }, "email": { "type": ["string", "null"], diff --git a/t/db_dependent/api/v1/cities.t b/t/db_dependent/api/v1/cities.t index 0a95338..c0a0c3e 100644 --- a/t/db_dependent/api/v1/cities.t +++ b/t/db_dependent/api/v1/cities.t @@ -127,7 +127,7 @@ subtest 'get() tests' => sub { subtest 'add() tests' => sub { - plan tests => 11; + plan tests => 17; $schema->storage->txn_begin; @@ -149,17 +149,6 @@ subtest 'add() tests' => sub { $tx->req->env( { REMOTE_ADDR => $remote_address } ); $t->request_ok($tx)->status_is(403); - # Authorized attempt to write - $tx = $t->ua->build_tx( POST => "/api/v1/cities/" => json => $city ); - $tx->req->cookies( - { name => 'CGISESSID', value => $authorized_session_id } ); - $tx->req->env( { REMOTE_ADDR => $remote_address } ); - $t->request_ok($tx)->status_is(200) - ->json_is( '/city_name' => $city->{city_name} ) - ->json_is( '/city_state' => $city->{city_state} ) - ->json_is( '/city_zipcode' => $city->{city_zipcode} ) - ->json_is( '/city_country' => $city->{city_country} ); - # Authorized attempt to write invalid data my $city_with_invalid_field = { city_blah => "City Blah", @@ -182,12 +171,56 @@ subtest 'add() tests' => sub { ] ); + # Authorized attempt to write + $tx = $t->ua->build_tx( POST => "/api/v1/cities/" => json => $city ); + $tx->req->cookies( + { name => 'CGISESSID', value => $authorized_session_id } ); + $tx->req->env( { REMOTE_ADDR => $remote_address } ); + my $cityid = $t->request_ok($tx)->status_is(200) + ->json_is( '/city_name' => $city->{city_name} ) + ->json_is( '/city_state' => $city->{city_state} ) + ->json_is( '/city_zipcode' => $city->{city_zipcode} ) + ->json_is( '/city_country' => $city->{city_country} ) + ->tx->res->json->{cityid}; + + # Authorized attempt to create with null id + $city->{cityid} = undef; + $tx = $t->ua->build_tx( + POST => "/api/v1/cities/" => json => $city ); + $tx->req->cookies( + { name => 'CGISESSID', value => $authorized_session_id } ); + $tx->req->env( { REMOTE_ADDR => $remote_address } ); + $t->request_ok($tx)->status_is(400)->json_is( + "/errors" => [ + { + message => "Read-only.", + path => "/body/cityid" + } + ] + ); + + # Authorized attempt to create with existing id + $city->{cityid} = $cityid; + $tx = $t->ua->build_tx( + POST => "/api/v1/cities/" => json => $city ); + $tx->req->cookies( + { name => 'CGISESSID', value => $authorized_session_id } ); + $tx->req->env( { REMOTE_ADDR => $remote_address } ); + $t->request_ok($tx)->status_is(400)->json_is( + "/errors" => [ + { + message => "Read-only.", + path => "/body/cityid" + } + ] + ); + $schema->storage->txn_rollback; }; subtest 'update() tests' => sub { - plan tests => 13; + plan tests => 15; $schema->storage->txn_begin; @@ -270,6 +303,16 @@ subtest 'update() tests' => sub { $t->request_ok($tx)->status_is(404); $schema->storage->txn_rollback; + + # Wrong mathod (POST) + $city_with_updated_field->{cityid} = 2; + + $tx = $t->ua->build_tx( + POST => "/api/v1/cities/$city_id" => json => $city_with_updated_field ); + $tx->req->cookies( + { name => 'CGISESSID', value => $authorized_session_id } ); + $tx->req->env( { REMOTE_ADDR => $remote_address } ); + $t->request_ok($tx)->status_is(404); }; subtest 'delete() tests' => sub { -- 2.1.4