Skip to content

Commit

Permalink
fix: add authentification through JSON body for API v3 WRITE requests (…
Browse files Browse the repository at this point in the history 
…#7813)


Adds authentification for API v3 WRITE requests, by providing user_id and password fields in the JSON body of the request (it was documented in OpenAPI, but not implemented).

Also added tests for authentification for API READ + WRITE for v2 + v3
  • Loading branch information
@stephanegigandet
stephanegigandet authored Dec 12, 2022
1 parent 5d465ef commit e072afa
Show file tree
Hide file tree
Showing 25 changed files with 1,285 additions and 264 deletions.
80 changes: 49 additions & 31 deletions cgi/display.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,45 +40,63 @@
use Apache2::RequestRec ();
use Apache2::Const qw(:common);

# The API V3 write product request uses POST / PUT / PATCH with a JSON body
# if we have such a request, we need to read the body before CGI.pm tries to read it to get multipart/form-data parameters

my $env_query_string = $ENV{QUERY_STRING};

$log->debug("display.pl - start", {env_query_string => $env_query_string});
my $request_ref = {};

my $body_request_ref = {};
my $r = Apache2::RequestUtil->request();
$request_ref->{method} = $r->method();

if ($env_query_string =~ /^\/?api\/v3(\.\d+)?\/product/) {
read_request_body($body_request_ref);
}
$log->debug("display.pl - start", {env_query_string => $env_query_string, request_ref => $request_ref})
if $log->is_debug();

# The nginx reverse proxy turns /somepath?someparam=somevalue to /cgi/display.pl?/somepath?someparam=somevalue
# so that all non /cgi/ queries are sent to display.pl and that we can get the path in the query string
# CGI.pm thus adds somepath? at the start of the name of the first parameter.
# we need to remove it so that we can use the CGI.pm param() function to later access the parameters

my @params = multi_param();
if (defined $params[0]) {
my $first_param = $params[0];
my $first_param_value = single_param($first_param);
$log->debug(
"replacing first param to remove path from parameter name",
{first_param => $first_param, $first_param_value => $first_param_value}
);
CGI::delete($first_param);
$first_param =~ s/^(.*?)\?//;
param($first_param, $first_param_value);
}
# Special behaviors for API v3 requests

if ($env_query_string =~ /^\/?api\/v(3(\.\d+)?)\//) {

# Record that we have an API v3 request, as errors (e.g. bad userid and password) will be handled differently
# (through API::process_api_request instead of returning an error page in HTML)
$request_ref->{api_version} = $1;

my $request_ref = ProductOpener::Display::init_request();
# Initialize the api_response field for errors and warnings
init_api_response($request_ref);

# Add the HTTP request body if we have one
if (defined $body_request_ref->{body}) {
$request_ref->{body} = $body_request_ref->{body};
# The API V3 write product request uses POST / PUT / PATCH with a JSON body
# if we have such a request, we need to read the body before CGI.pm tries to read it to get multipart/form-data parameters
# We also need to do this before the call to init_request() which calls init_user()
# so that authentification credentials user_id and password from the JSON body can be used to authenticate the user
if ($request_ref->{method} =~ /^(POST|PUT|PATCH)$/) {
read_request_body($request_ref);
decode_json_request_body($request_ref);
}
}

$log->debug("before analyze_request", {query_string => $request_ref->{query_string}});
if (($env_query_string !~ /^\/?api\/v(3(\.\d+)?)\//) or ($request_ref->{method} !~ /^(POST|PUT|PATCH)$/)) {
# Not an API v3 POST/PUT/PATCH request: we will use CGI.pm param() method to access query string or multipart/form-data parameters

# The nginx reverse proxy turns /somepath?someparam=somevalue to /cgi/display.pl?/somepath?someparam=somevalue
# so that all non /cgi/ queries are sent to display.pl and that we can get the path in the query string
# CGI.pm thus adds somepath? at the start of the name of the first parameter.
# we need to remove it so that we can use the CGI.pm param() function to later access the parameters

my @params = multi_param();
if (defined $params[0]) {
my $first_param = $params[0];
my $first_param_value = single_param($first_param);
$log->debug(
"replacing first param to remove path from parameter name",
{first_param => $first_param, $first_param_value => $first_param_value}
);
CGI::delete($first_param);
$first_param =~ s/^(.*?)\?//;
param($first_param, $first_param_value);
}
}

# Initialize the request object, and authenticate the user
init_request($request_ref);

$log->debug("before analyze_request", {query_string => $request_ref->{query_string}}) if $log->is_debug();

# analyze request will fill request with action and parameters
analyze_request($request_ref);
Expand All @@ -101,7 +119,7 @@
user => $request_ref->{user},
query => $request_ref->{query}
}
);
) if $log->is_debug();

# Only display texts if products are private and no owner is defined
if (
Expand Down
6 changes: 3 additions & 3 deletions docs/reference/api-v3.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ paths:
name: fields
description: |-
Comma separated list of fields requested in the response.
Special values:
* "none": returns no fields
* "raw": returns all fields as stored internally in the database
* "all": returns all fields except generated fields that need to be explicitly requested such as "knowledge_panels".
Defaults to "all" for READ requests. The "all" value can also be combined with fields like "attribute_groups" and "knowledge_panels".'
responses:
'200':
Expand Down Expand Up @@ -176,7 +176,7 @@ paths:
- $ref: ./requestBodies/fields_tags_lc.yaml
- type: object
properties:
userid:
user_id:
type: string
password:
type: string
Expand Down
58 changes: 34 additions & 24 deletions lib/ProductOpener/API.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use Log::Any qw($log);
BEGIN {
use vars qw(@ISA @EXPORT_OK %EXPORT_TAGS);
@EXPORT_OK = qw(
&init_api_response
&get_initialized_response
&add_warning
&add_error
Expand Down Expand Up @@ -90,6 +91,8 @@ sub get_initialized_response() {
sub init_api_response ($request_ref) {

$request_ref->{api_response} = get_initialized_response();

$log->debug("init_api_response - done", {request => $request_ref}) if $log->is_debug();
return $request_ref->{api_response};
}

Expand Down Expand Up @@ -170,7 +173,7 @@ sub decode_json_request_body ($request_ref) {
);
}
else {
eval {$request_ref->{request_body_json} = decode_json($request_ref->{body});};
eval {$request_ref->{body_json} = decode_json($request_ref->{body});};
if ($@) {
$log->error("JSON decoding error", {error => $@}) if $log->is_error();
add_error(
Expand Down Expand Up @@ -340,41 +343,49 @@ sub process_api_request ($request_ref) {

$log->debug("process_api_request - start", {request => $request_ref}) if $log->is_debug();

my $response_ref = init_api_response($request_ref);
my $response_ref = $request_ref->{api_response};

# Analyze the request body
# Check if we already have errors (e.g. authentification error, invalid JSON body)
if ((scalar @{$response_ref->{errors}}) > 0) {
$log->warn("process_api_request - we already have errors, skipping processing", {request => $request_ref})
if $log->is_warn();
}
else {

if ($request_ref->{api_action} eq "product") {
# Route the API request to the right processing function, based on API action (from path) and method

if ($request_ref->{api_method} eq "PATCH") {
write_product_api($request_ref);
}
elsif ($request_ref->{api_method} =~ /^(GET|HEAD|OPTIONS)$/) {
read_product_api($request_ref);
if ($request_ref->{api_action} eq "product") {

if ($request_ref->{api_method} eq "PATCH") {
write_product_api($request_ref);
}
elsif ($request_ref->{api_method} =~ /^(GET|HEAD|OPTIONS)$/) {
read_product_api($request_ref);
}
else {
$log->warn("process_api_request - invalid method", {request => $request_ref}) if $log->is_warn();
add_error(
$response_ref,
{
message => {id => "invalid_api_method"},
field => {id => "api_method", value => $request_ref->{api_method}},
impact => {id => "failure"},
}
);
}
}
else {
$log->warn("process_api_request - invalid method", {request => $request_ref}) if $log->is_warn();
$log->warn("process_api_request - unknown action", {request => $request_ref}) if $log->is_warn();
add_error(
$response_ref,
{
message => {id => "invalid_api_method"},
field => {id => "api_method", value => $request_ref->{api_method}},
message => {id => "invalid_api_action"},
field => {id => "api_action", value => $request_ref->{api_action}},
impact => {id => "failure"},
}
);
}
}
else {
$log->warn("process_api_request - unknown action", {request => $request_ref}) if $log->is_warn();
add_error(
$response_ref,
{
message => {id => "invalid_api_action"},
field => {id => "api_action", value => $request_ref->{api_action}},
impact => {id => "failure"},
}
);
}

determine_response_result($response_ref);

Expand Down Expand Up @@ -536,7 +547,6 @@ sub customize_packagings ($request_ref, $product_ref) {
}
}
}

push @$customized_packagings_ref, $customized_packaging_ref;
}
}
Expand Down
6 changes: 2 additions & 4 deletions lib/ProductOpener/APIProductWrite.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ Update product fields based on input product data.
sub update_product_fields ($request_ref, $product_ref) {

my $response_ref = $request_ref->{api_response};
my $request_body_ref = $request_ref->{request_body_json};
my $request_body_ref = $request_ref->{body_json};

$request_ref->{updated_product_fields} = {};

Expand Down Expand Up @@ -149,9 +149,7 @@ sub write_product_api ($request_ref) {
$log->debug("write_product_api - start", {request => $request_ref}) if $log->is_debug();

my $response_ref = $request_ref->{api_response};

decode_json_request_body($request_ref);
my $request_body_ref = $request_ref->{request_body_json};
my $request_body_ref = $request_ref->{body_json};

$log->debug("write_product_api - body", {request_body => $request_body_ref}) if $log->is_debug();

Expand Down
73 changes: 39 additions & 34 deletions lib/ProductOpener/APITest.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ sub wait_server() {
$count++;
if (($count % 3) == 0) {
print("Waiting for backend to be ready since more than $count seconds...\n");
print("Bad response from website:" . explain({url => $target_url, status => $response->code}) . "\n");
diag explain({url => $target_url, status => $response->code, response => $response});
}
confess("Waited too much for backend") if $count > 60;
}
Expand Down Expand Up @@ -421,42 +421,47 @@ sub execute_api_tests ($file, $tests_ref) {
$response = $ua->request($request);
}

# Check if we got the expected response status code
if (defined $test_ref->{expected_status_code}) {
is($response->code, $test_ref->{expected_status_code})
or diag(explain($test_ref), "Response status line: " . $response->status_line);
# Check if we got the expected response status code, expect 200 if not provided
if (not defined $test_ref->{expected_status_code}) {
$test_ref->{expected_status_code} = 200;
}

# Check that we got a JSON response
my $json = $response->decoded_content;

my $decoded_json;
eval {
$decoded_json = decode_json($json);
1;
} or do {
my $json_decode_error = $@;
diag("The $method request to $url returned a response that is not valid JSON: $json_decode_error");
diag("Response content: " . $json);
fail($test_case);
next;
};

# normalize for comparison
if (defined $decoded_json->{'products'}) {
normalize_products_for_test_comparison($decoded_json->{'products'});
}
if (defined $decoded_json->{'product'}) {
normalize_product_for_test_comparison($decoded_json->{'product'});
}
is($response->code, $test_ref->{expected_status_code})
or diag(explain($test_ref), "Response status line: " . $response->status_line);

if (not((defined $test_ref->{expected_type}) and ($test_ref->{expected_type} eq "html"))) {

# Check that we got a JSON response
my $json = $response->decoded_content;

my $decoded_json;
eval {
$decoded_json = decode_json($json);
1;
} or do {
my $json_decode_error = $@;
diag("The $method request to $url returned a response that is not valid JSON: $json_decode_error");
diag("Response content: " . $json);
fail($test_case);
next;
};

# normalize for comparison
if (defined $decoded_json->{'products'}) {
normalize_products_for_test_comparison($decoded_json->{'products'});
}
if (defined $decoded_json->{'product'}) {
normalize_product_for_test_comparison($decoded_json->{'product'});
}

is(
compare_to_expected_results(
$decoded_json, "$expected_result_dir/$test_case.json",
$update_expected_results, $test_ref
),
1,
);
is(
compare_to_expected_results(
$decoded_json, "$expected_result_dir/$test_case.json",
$update_expected_results, $test_ref
),
1,
);
}

}
return;
Expand Down
Loading

0 comments on commit e072afa

Please sign in to comment.