https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28787
--- Comment #16 from Marcel de Rooy <[email protected]> --- 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. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
