[Koha-bugs] [Bug 28787] Send a notice with the TOTP token

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28787

--- Comment #16 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
Generally, I have some doubts about the API path api/v1/auth/send_otp_token.
Sending a token is not a normally expected API action; it sounds like a
'misused verb'. You could think of creating a OTP code as an API action,
although we do not really add it as entity.
Apart from that it works. See some details hereunder.

[1] Your TODO - I am not sure about the following line, so I commented it but 
let it in the patch
+            #|| $c->req->url->to_abs->path eq '/api/v1/auth/send_otp_token' )
{
The otp path should go thru the chain. So this line should not be here although
commented. Removed it.

[2] Code segment from Koha/REST/V1/Auth.pm
    if ( !$authorization and
         ( $params->{is_public} and
          ( C4::Context->preference('RESTPublicAnonymousRequests') or
            $user) or $params->{is_plugin} )
        or $pending_auth
This does not look good to me. Do we need pending_auth here ? If so, at least
we need parentheses etc. My follow-up removes the line now.

[3] This segment is incomplete:
        elsif ($status eq "additional-auth-needed") {
            if ( $c->req->url->to_abs->path eq '/api/v1/auth/send_otp_token' )
{
                $user = Koha::Patrons->find( $session->param('number') );
                $cookie_auth = 1;
                $pending_auth = 1;
            }
I think we should raise an exception if we have this status and the api path
does not match (so add an else). Removed pending_auth. Added a simple exception
in my follow-up.

[4] When I tested this API path via API keys, I got no authorization. I added a
permission catalogue (staff access) to get around that. If you dont have that
permission, we should not even send a code.

[5] Letters:
+    if ( $content =~ m|\[% otp_token %\]| ) {
+        my $patron = Koha::Patrons->find(C4::Context->userenv->{number});
+        $tt_params->{otp_token} = Koha::Auth::TwoFactorAuth->new({patron =>
$patron})->code;
+    }
This seems quite hacky. Why not pass it to Letters from the api module? Moved
it now.
This still needs updating the notice stuff.

[6] QA question: Is 400 the correct error code to tell the email has not been
sent?
I guess it is not. The client did nothing wrong. Maybe just plain 500? But
having some doubts about that too.
Or always 200/201 and refer for details to JSON body?

[7] TODO Hardcoded phrase: It is valid one minute.

[8] Functional question:
When you want to enable 2FA without a mobile phone, what should you do? There
is no link to send the code on that form.

[9] Current code:
C4::Context->config('encryption_key')
<encryption_key>__ENCRYPTION_KEY__</encryption_key>
Do we still need to replace it in koha-create by the actual key ?
Enable 2FA: Form text: Can't scan the code?  To add the entry manually, provide
the following details to the application on your phone.  Account: BRANCH Key:
BRANCH_EMAIL Time based: Yes
But the form does not show the Secret. So telling the user to enter the details
on their phone is useless?

Let me know if you agree with the follow-up.

-- 
You are receiving this mail because:
You are watching all bug changes.