From b93e15c235c77ad557b73f49f93e29b0669ee82b Mon Sep 17 00:00:00 2001
From: Jonathan Druart
Date: Mon, 25 Jul 2022 16:53:02 +0200
Subject: [PATCH] Bug 30588: Add the option to require 2FA setup on first staff
login
Bug 28786 added the ability to turn on a two-factor authentication,
using a One Time Password (OTP).
Once enabled on the system, librarian had the choice to enable or
disable it for themselves.
For security reason an administrator could decide to force the
librarians to use this second authentication step.
This patch adds a third option to the existing syspref, 'Enforced', for
that purpose.
QA notes: the code we had in the members/two_factor_auth.pl controller
has been moved to REST API controller methods (with their tests and
swagger specs), for reusability reason. Code from template has been
moved to an include file for the same reason.
Test plan:
A. Regression tests
As we modified the code we need first to confirm the existing features
are still working as expected.
1. Turn off TwoFactorAuthentication (disabled) and confirm that you are not able to
enable and access the second authentication step
2. Turn it on (enabled) and confirm that you are able to enable it in your account
3. Logout and confirm then that you are able to login into Koha
B. The new option
1. Set the pref to "enforced"
2. You are not logged out, logged in users stay logged in
3. Pick a user that does not have 2FA setup, login
4. Notice the new screen (UI is a bit ugly, suggestions welcomed)
5. Try to access Koha without enabling 2FA, you shouldn't be able to
access any pages
6. Setup 2FA and confirm that you are redirected to the login screen
7. Login, send the correct pin code
=> You are fully logged in!
Note that at 6 we could redirect to the mainpage, without the need to
login again, but I think it's preferable to reduce the change to
C4::Auth. If it's considered mandatory by QA I could have a look on
another bug report.
Sponsored-by: Rijksmuseum, Netherlands
Signed-off-by: Nick Clemens
Signed-off-by: Tomas Cohen Arazi
---
C4/Auth.pm | 17 ++-
Koha/Auth/TwoFactorAuth.pm | 2 +
Koha/REST/V1/Auth.pm | 22 ++++
Koha/REST/V1/TwoFactorAuth.pm | 103 +++++++++++++++++
api/v1/swagger/paths/auth.yaml | 89 ++++++++++++++
api/v1/swagger/swagger.yaml | 4 +
.../intranet-tmpl/prog/en/modules/auth.tt | 51 +++++++-
.../en/modules/members/two_factor_auth.tt | 109 ++++++++++--------
members/two_factor_auth.pl | 65 +----------
9 files changed, 349 insertions(+), 113 deletions(-)
diff --git a/C4/Auth.pm b/C4/Auth.pm
index a292c36a92..fed9c5fd5d 100644
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -1270,9 +1270,11 @@ sub checkauth {
# Auth is completed unless an additional auth is needed
if ( $require_2FA ) {
my $patron = Koha::Patrons->find({userid => $userid});
- if ( C4::Context->preference('TwoFactorAuthentication') eq "enforced"
- || $patron->auth_method eq 'two-factor' )
- {
+ if ( C4::Context->preference('TwoFactorAuthentication') eq "enforced" && $patron->auth_method eq 'password' ) {
+ $auth_state = 'setup-additional-auth-needed';
+ $session->param('waiting-for-2FA-setup', 1);
+ %info = ();# We remove the warnings/errors we may have set incorrectly before
+ } elsif ( $patron->auth_method eq 'two-factor' ) {
# Ask for the OTP token
$auth_state = 'additional-auth-needed';
$session->param('waiting-for-2FA', 1);
@@ -1385,6 +1387,12 @@ sub checkauth {
);
}
+ if ( $auth_state eq 'setup-additional-auth-needed' ) {
+ $template->param(
+ TwoFA_setup => 1,
+ );
+ }
+
if ( $type eq 'opac' ) {
require Koha::Virtualshelves;
my $some_public_shelves = Koha::Virtualshelves->get_some_shelves(
@@ -1778,6 +1786,9 @@ sub check_cookie_auth {
return ( "additional-auth-needed", $session )
if $session->param('waiting-for-2FA');
+ return ( "setup-additional-auth-needed", $session )
+ if $session->param('waiting-for-2FA-setup');
+
return ( "ok", $session );
} else {
$session->delete();
diff --git a/Koha/Auth/TwoFactorAuth.pm b/Koha/Auth/TwoFactorAuth.pm
index 48b2a895ea..b9c785fa0e 100644
--- a/Koha/Auth/TwoFactorAuth.pm
+++ b/Koha/Auth/TwoFactorAuth.pm
@@ -58,6 +58,8 @@ sub new {
my $secret32 = $params->{secret32};
my $secret = $params->{secret};
+ # FIXME Raise an exception if the syspref is disabled
+
Koha::Exceptions::MissingParameter->throw("Mandatory patron parameter missing")
unless $patron && ref($patron) eq 'Koha::Patron';
diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm
index 621011cff0..8e8a6f675c 100644
--- a/Koha/REST/V1/Auth.pm
+++ b/Koha/REST/V1/Auth.pm
@@ -236,6 +236,28 @@ sub authenticate_api_request {
Koha::Exceptions::Authentication::Required->throw(
error => 'Authentication failure.' );
}
+ }
+ elsif ( $c->req->url->to_abs->path eq '/api/v1/auth/two-factor/registration'
+ || $c->req->url->to_abs->path eq '/api/v1/auth/two-factor/registration/verification' ) {
+
+ if ( $status eq 'setup-additional-auth-needed' ) {
+ $user = Koha::Patrons->find( $session->param('number') );
+ $cookie_auth = 1;
+ }
+ elsif ( $status eq 'ok' ) {
+ $user = Koha::Patrons->find( $session->param('number') );
+ if ( $user->auth_method ne 'password' ) {
+ # If the user already enabled 2FA they don't need to register again
+ Koha::Exceptions::Authentication->throw(
+ error => 'Cannot request this route.' );
+ }
+ $cookie_auth = 1;
+ }
+ else {
+ Koha::Exceptions::Authentication::Required->throw(
+ error => 'Authentication failure.' );
+ }
+
} else {
if ($status eq "ok") {
$user = Koha::Patrons->find( $session->param('number') );
diff --git a/Koha/REST/V1/TwoFactorAuth.pm b/Koha/REST/V1/TwoFactorAuth.pm
index e62136ac26..f9f199f74b 100644
--- a/Koha/REST/V1/TwoFactorAuth.pm
+++ b/Koha/REST/V1/TwoFactorAuth.pm
@@ -78,4 +78,107 @@ sub send_otp_token {
}
+=head3 registration
+
+Ask for a registration secret. It will return a QR code image and a secret32.
+
+The secret must be sent back to the server with the pin code for the verification step.
+
+=cut
+
+sub registration {
+
+ my $c = shift->openapi->valid_input or return;
+
+ my $patron = Koha::Patrons->find( $c->stash('koha.user')->borrowernumber );
+
+ return try {
+ my $secret = Koha::AuthUtils::generate_salt( 'weak', 16 );
+ my $auth = Koha::Auth::TwoFactorAuth->new(
+ { patron => $patron, secret => $secret } );
+
+ my $response = {
+ issuer => $auth->issuer,
+ key_id => $auth->key_id,
+ qr_code => $auth->qr_code,
+ secret32 => $auth->secret32,
+
+ # IMPORTANT: get secret32 after qr_code call !
+ };
+ $auth->clear;
+
+ return $c->render(status => 201, openapi => $response);
+ }
+ catch {
+ $c->unhandled_exception($_);
+ };
+
+}
+
+=head3 verification
+
+Verify the registration, get the pin code and the secret retrieved from the registration.
+
+The 2FA_ENABLE notice will be generated if the pin code is correct, and the patron will have their two-factor authentication setup completed.
+
+=cut
+
+sub verification {
+
+ my $c = shift->openapi->valid_input or return;
+
+ my $patron = Koha::Patrons->find( $c->stash('koha.user')->borrowernumber );
+
+ return try {
+
+ my $pin_code = $c->validation->param('pin_code');
+ my $secret32 = $c->validation->param('secret32');
+
+ my $auth = Koha::Auth::TwoFactorAuth->new(
+ { patron => $patron, secret32 => $secret32 } );
+
+ my $verified = $auth->verify(
+ $pin_code,
+ 1, # range
+ $secret32,
+ undef, # timestamp (defaults to now)
+ 30, # interval (default 30)
+ );
+
+ unless ($verified) {
+ return $c->render(
+ status => 400,
+ openapi => { error => "Invalid pin" }
+ );
+ }
+
+ # FIXME Generate a (new?) secret
+ $patron->encode_secret($secret32);
+ $patron->auth_method('two-factor')->store;
+ if ( $patron->notice_email_address ) {
+ $patron->queue_notice(
+ {
+ letter_params => {
+ module => 'members',
+ letter_code => '2FA_ENABLE',
+ branchcode => $patron->branchcode,
+ lang => $patron->lang,
+ tables => {
+ branches => $patron->branchcode,
+ borrowers => $patron->id
+ },
+ },
+ message_transports => ['email'],
+ }
+ );
+ }
+
+ return $c->render(status => 204, openapi => {});
+ }
+ catch {
+ $c->unhandled_exception($_);
+ };
+
+}
+
1;
diff --git a/api/v1/swagger/paths/auth.yaml b/api/v1/swagger/paths/auth.yaml
index c147d0549d..40c886a5ba 100644
--- a/api/v1/swagger/paths/auth.yaml
+++ b/api/v1/swagger/paths/auth.yaml
@@ -39,3 +39,92 @@
x-koha-authorization:
permissions:
catalogue: "1"
+
+/auth/two-factor/registration:
+ post:
+ x-mojo-to: TwoFactorAuth#registration
+ operationId: Two factor register
+ tags:
+ - 2fa
+ summary: Generate a secret
+ produces:
+ - application/json
+ responses:
+ "201":
+ description: OK
+ schema:
+ type: object
+ properties:
+ secret32:
+ type: string
+ qr_code:
+ type: string
+ issuer:
+ type: string
+ key_id:
+ type: string
+ additionalProperties: false
+ "400":
+ description: Bad Request
+ schema:
+ $ref: "../swagger.yaml#/definitions/error"
+ "403":
+ description: Access forbidden
+ schema:
+ $ref: "../swagger.yaml#/definitions/error"
+ "500":
+ description: |
+ Internal server error. Possible `error_code` attribute values:
+
+ * `internal_server_error`
+ schema:
+ $ref: "../swagger.yaml#/definitions/error"
+ x-koha-authorization:
+ permissions:
+ catalogue: "1"
+
+/auth/two-factor/registration/verification:
+ post:
+ x-mojo-to: TwoFactorAuth#verification
+ operationId: Two factor register verification
+ tags:
+ - 2fa
+ summary: Verify two-factor registration
+ parameters:
+ - name: secret32
+ in: formData
+ description: the secret
+ required: true
+ type: string
+ - name: pin_code
+ in: formData
+ description: the pin code
+ required: true
+ type: string
+ produces:
+ - application/json
+ responses:
+ "204":
+ description: OK
+ "401":
+ description: Authentication required
+ schema:
+ $ref: "../swagger.yaml#/definitions/error"
+ "400":
+ description: Bad Request
+ schema:
+ $ref: "../swagger.yaml#/definitions/error"
+ "403":
+ description: Access forbidden
+ schema:
+ $ref: "../swagger.yaml#/definitions/error"
+ "500":
+ description: |
+ Internal server error. Possible `error_code` attribute values:
+
+ * `internal_server_error`
+ schema:
+ $ref: "../swagger.yaml#/definitions/error"
+ x-koha-authorization:
+ permissions:
+ catalogue: "1"
diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml
index 50a35a1ef5..1bc55f73d7 100644
--- a/api/v1/swagger/swagger.yaml
+++ b/api/v1/swagger/swagger.yaml
@@ -111,6 +111,10 @@ paths:
$ref: "./paths/article_requests.yaml#/~1article_requests~1{article_request_id}"
/auth/otp/token_delivery:
$ref: paths/auth.yaml#/~1auth~1otp~1token_delivery
+ /auth/two-factor/registration:
+ $ref: paths/auth.yaml#/~1auth~1two-factor~1registration
+ /auth/two-factor/registration/verification:
+ $ref: paths/auth.yaml#/~1auth~1two-factor~1registration~1verification
"/biblios/{biblio_id}":
$ref: "./paths/biblios.yaml#/~1biblios~1{biblio_id}"
"/biblios/{biblio_id}/checkouts":
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
index e2db5e0f30..50f4d7a087 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
@@ -17,6 +17,7 @@
[% IF ( nopermission ) %]Access denied[% END %] › Koha
[% INCLUDE 'doc-head-close.inc' %]
+[% PROCESS 'auth-two-factor.inc' %]
@@ -71,7 +72,7 @@