From 8619cd650bfdf3ace52b9ab203f450c587287dbe Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 17 Jul 2023 15:48:20 -0300 Subject: [PATCH] Bug 32739: Allow other patron identifier on pwd validation This patch takes a step forward on the password validation endpoint, by adding the `identifier` parameter and making it be allowed to be the patron's `cardnumber` or the `userid`. The current `userid` only validation option is kept as-is. The implementation relies on `C4::Auth::checkpw` to query for the patron. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/password_validation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 69653a281dcbe2d0d610cbc2be6cc2718b52fca3) Signed-off-by: Fridolin Somers --- Koha/REST/V1/Auth/Password.pm | 34 ++++++++++++++++++--- api/v1/swagger/paths/auth.yaml | 20 +++++++++--- t/db_dependent/api/v1/password_validation.t | 24 ++++++--------- 3 files changed, 55 insertions(+), 23 deletions(-) diff --git a/Koha/REST/V1/Auth/Password.pm b/Koha/REST/V1/Auth/Password.pm index 6ed7073921..d082435a94 100644 --- a/Koha/REST/V1/Auth/Password.pm +++ b/Koha/REST/V1/Auth/Password.pm @@ -44,15 +44,39 @@ sub validate { my $userid = $body->{userid} // ''; my $patron = Koha::Patrons->find({ userid => $userid }); - unless ($patron) { - return $c->render( status => 400, openapi => { error => "Validation failed" } ); + my $body = $c->req->json; + + my $identifier = $body->{identifier}; + my $userid = $body->{userid}; + + unless ( defined $identifier or defined $userid ) { + return $c->render( + status => 400, + openapi => { error => "Validation failed" }, + ); + } + + if ( defined $identifier and defined $userid ) { + return $c->render( + status => 400, + openapi => { error => "Bad request. Only one identifier attribute can be passed." }, + ); } - my $password = $body->{password} // ""; + if ($userid) { + return $c->render( + status => 400, + openapi => { error => "Validation failed" }, + ) unless Koha::Patrons->find( { userid => $userid } ); + } + + $identifier //= $userid; + + my $password = $body->{password} // ""; return try { - my ($status, $cardnumber, $userid) = C4::Auth::checkpw($patron->userid, $password ); - unless ( $status ) { + my ( $status, $cardnumber, $userid ) = C4::Auth::checkpw( $identifier, $password ); + unless ($status) { return $c->render( status => 400, openapi => { error => "Validation failed" } diff --git a/api/v1/swagger/paths/auth.yaml b/api/v1/swagger/paths/auth.yaml index 90765f1d20..44d6035f05 100644 --- a/api/v1/swagger/paths/auth.yaml +++ b/api/v1/swagger/paths/auth.yaml @@ -1066,18 +1066,30 @@ parameters: - name: body in: body - description: A JSON object containing username and password information + description: | + A JSON object containing a patron identifier and password information. + + The identifier will be used to match patrons on the database using the + following order: + + * userid + * cardnumber + + Optionally, you can specify the `userid` attribute if you don't want it + to be checked against the patron cardnumbers. schema: type: object properties: - userid: - description: Username + identifier: + description: A patron identifier (`userid` or `cardnumber`) type: string password: description: Password (plain text) type: string + userid: + description: A patron userid + type: string required: - - userid - password additionalProperties: false produces: diff --git a/t/db_dependent/api/v1/password_validation.t b/t/db_dependent/api/v1/password_validation.t index 51687d6648..21ef7fec83 100755 --- a/t/db_dependent/api/v1/password_validation.t +++ b/t/db_dependent/api/v1/password_validation.t @@ -136,8 +136,7 @@ subtest 'Password validation - authorized requests tests' => sub { }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) - ->status_is(204, 'Validating using `cardnumber` works') - ->content_is(q{}); + ->status_is( 204, 'Validating using `cardnumber` works' )->content_is(q{}); $json = { identifier => $librarian->cardnumber, @@ -145,8 +144,7 @@ subtest 'Password validation - authorized requests tests' => sub { }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) - ->status_is(204, 'Validating using `cardnumber` works') - ->content_is(q{}); + ->status_is( 204, 'Validating using `cardnumber` works' )->content_is(q{}); $json = { identifier => $deleted_cardnumber, @@ -154,8 +152,8 @@ subtest 'Password validation - authorized requests tests' => sub { }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) - ->status_is(400, 'Validating using and invalid identifier fails') - ->json_is({ error => 'Validation failed' }); + ->status_is( 400, 'Validating using and invalid identifier fails' ) + ->json_is( { error => 'Validation failed' } ); $json = { identifier => $deleted_userid, @@ -163,8 +161,8 @@ subtest 'Password validation - authorized requests tests' => sub { }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) - ->status_is(400, 'Validating using and invalid identifier fails') - ->json_is({ error => 'Validation failed' }); + ->status_is( 400, 'Validating using and invalid identifier fails' ) + ->json_is( { error => 'Validation failed' } ); $json = { password => $password, @@ -172,8 +170,7 @@ subtest 'Password validation - authorized requests tests' => sub { }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) - ->status_is(400, 'Validating using and invalid userid fails') - ->json_is({ error => 'Validation failed' }); + ->status_is( 400, 'Validating using and invalid userid fails' )->json_is( { error => 'Validation failed' } ); $json = { password => $password, @@ -181,8 +178,7 @@ subtest 'Password validation - authorized requests tests' => sub { }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) - ->status_is(204, 'Validating using the `userid` attribute works') - ->content_is(q{}); + ->status_is( 204, 'Validating using the `userid` attribute works' )->content_is(q{}); $json = { password => $password, @@ -199,8 +195,8 @@ subtest 'Password validation - authorized requests tests' => sub { }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) - ->status_is(400, 'Passing both parameters forbidden') - ->json_is({ error => 'Bad request. Only one identifier attribute can be passed.' }); + ->status_is( 400, 'Passing both parameters forbidden' ) + ->json_is( { error => 'Bad request. Only one identifier attribute can be passed.' } ); $schema->storage->txn_rollback; }; -- 2.39.5