From 1ef377fd76ef3988db17198c10649b50b7b67e08 Mon Sep 17 00:00:00 2001 From: Jane Sandberg Date: Sun, 5 Mar 2023 16:57:50 -0800 Subject: [PATCH] LP1808016: improve error handling by open-ils.pcrud This patch ensures that requests to open-ils.pcrud return an error code (before the request completion code) when a permissions or constraint check fails. To test ------- [1] Make an invalid request, e.g., by attempting to create a claim type whose owner is not set in the Acquisitions Claiming admin interface. [2] Note that the user interface reports that the action succeeds (although the new claim type is not actually created). [3] Apply the patch and repeat step 1. This time, the admin interface shoudl report that the creation failed. Signed-off-by: Jane Sandberg Signed-off-by: Terran McCanna Signed-off-by: Galen Charlton --- Open-ILS/src/c-apps/oils_sql.c | 12 +++++ .../live_t/lp1808016-pcrud-return-error-status.t | 54 ++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 Open-ILS/src/perlmods/live_t/lp1808016-pcrud-return-error-status.t diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c index 93f2577cea..d292efb3b8 100644 --- a/Open-ILS/src/c-apps/oils_sql.c +++ b/Open-ILS/src/c-apps/oils_sql.c @@ -2365,6 +2365,10 @@ int doCreate( osrfMethodContext* ctx ) { } if( !verifyObjectClass( ctx, target )) { + osrfAppSessionStatus(ctx->session, OSRF_STATUS_BADREQUEST, + "osrfMethodException", ctx->request, + "Invalid object or insufficient permissions" + ); osrfAppRespondComplete( ctx, NULL ); return -1; } @@ -6431,6 +6435,10 @@ int doUpdate( osrfMethodContext* ctx ) { target = jsonObjectGetIndex( ctx->params, 0 ); if(!verifyObjectClass( ctx, target )) { + osrfAppSessionStatus(ctx->session, OSRF_STATUS_BADREQUEST, + "osrfMethodException", ctx->request, + "Invalid object or insufficient permissions" + ); osrfAppRespondComplete( ctx, NULL ); return -1; } @@ -6702,6 +6710,10 @@ int doDelete( osrfMethodContext* ctx ) { char* id; if( jsonObjectGetIndex( ctx->params, _obj_pos )->classname ) { if( !verifyObjectClass( ctx, jsonObjectGetIndex( ctx->params, _obj_pos ))) { + osrfAppSessionStatus(ctx->session, OSRF_STATUS_BADREQUEST, + "osrfMethodException", ctx->request, + "Invalid object or insufficient permissions" + ); osrfAppRespondComplete( ctx, NULL ); return -1; } diff --git a/Open-ILS/src/perlmods/live_t/lp1808016-pcrud-return-error-status.t b/Open-ILS/src/perlmods/live_t/lp1808016-pcrud-return-error-status.t new file mode 100644 index 0000000000..d4b7fb0849 --- /dev/null +++ b/Open-ILS/src/perlmods/live_t/lp1808016-pcrud-return-error-status.t @@ -0,0 +1,54 @@ +#!perl +# Yes, this is a Perl test for some C code, but +# it seemed way easier to write a meaningful +# integration test here than with libcheck :-D + +use strict; use warnings; +use Test::More tests => 2; +use OpenILS::Utils::TestUtils; +use OpenILS::Const qw(:const); + +my $script = OpenILS::Utils::TestUtils->new(); +our $apputils = 'OpenILS::Application::AppUtils'; + +diag('LP108016: Pcrud returns a bad error status before complete'); + +# Login as a staff with limited permissions (just acq permissions in this case) +my $credentials = { + username => 'br1breid', + password => 'barbarar1234', + type => 'staff' +}; +my $authtoken = $script->authenticate($credentials); +ok( + $authtoken, + 'Logged in' +) or BAIL_OUT('Must log in'); + +my $pcrud_ses = $script->session('open-ils.pcrud'); +$pcrud_ses->connect(); +my $xact = $pcrud_ses->request( + 'open-ils.pcrud.transaction.begin', + $script->authtoken +)->gather(1); + +# As this user, try to do something forbidden: create a shelving location +my $acpl = Fieldmapper::asset::copy_location->new; +$acpl->owning_lib(1); +$acpl->name('My bad location'); +my $request = $pcrud_ses->request( + 'open-ils.pcrud.create.acpl', + $script->authtoken, + $acpl +); +$request->recv(); + +is(error_code($request), 400, 'We get the expected error code'); + +sub error_code { + my $request_to_check = shift; + if ($request_to_check->failed() =~ /<(\d{3})>/ms) { + return $1; + } + return 0; +} -- 2.11.0