Overview

It provides a login function using WebAuthn.

Base module features:

* Register/Delete authentication information for WebAuthn
* Logging in to Drupal using WebAuthn

Project page

https://www.drupal.org/project/webauthn_authenticator

Git instructions

git clone --branch 8.x-1.x https://git.drupalcode.org/project/webauthn_authenticator.git

Pareview checklist

https://pareview.sh/pareview/https-git.drupal.org-project-webauthn_authe...

Comments

otofu created an issue. See original summary.

ankushgautam76@gmail.com’s picture

Hi otofu,

Thanks for your contribution.

No issue found in pareview report.

Below are my findings :

1. In js files you have mention doc comments in other language, is there any reason

// 認証キーを登録する為の情報をサーバより取得.

2. I didn't find any configure link on module installation page for webauthn credentials.

apaderno’s picture

shaktik’s picture

Hi otofu,

Kindly solve these errors/warnings also.

12/12 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   src/Entity/WebauthnCredential.php
 ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------------------
  155    Method Drupal\webauthn_authenticator\Entity\WebauthnCredential::getUserEntity() should return Drupal\user\Entity\User but returns Drupal\Core\Entity\EntityInterface.
  205    Method Drupal\webauthn_authenticator\Entity\WebauthnCredential::getCreatedTime() should return int but returns string.
  219    Method Drupal\webauthn_authenticator\Entity\WebauthnCredential::findBy() should return array<Drupal\webauthn_authenticator\Entity\WebauthnCredential> but returns
         array<Drupal\Core\Entity\EntityInterface>.
 ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ ----------------------------------------------------------------------------------------------------------------------
  Line   src/Form/RegistrationCredentialForm.php
 ------ --------------------------------------------------------------------------------------------------------------------
  117    PHPDoc tag @var for variable $credential_options contains unknown class Webauthn\PublicKeyCredentialCreationOptions.
 ------ ----------------------------------------------------------------------------------------------------------------------

 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   src/Repository/PublicKeyCredentialSourceRepository.php
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  18     Return typehint of method Drupal\webauthn_authenticator\Repository\PublicKeyCredentialSourceRepository::findOneByCredentialId() has invalid type Webauthn\PublicKeyCredentialSource.
  26     Parameter $public_key_credential_user_entity of method Drupal\webauthn_authenticator\Repository\PublicKeyCredentialSourceRepository::findAllForUserEntity() has invalid typehint type
         Webauthn\PublicKeyCredentialUserEntity.
  29     Call to method getId() on an unknown class Webauthn\PublicKeyCredentialUserEntity.
  40     Parameter $public_key_credential_source of method Drupal\webauthn_authenticator\Repository\PublicKeyCredentialSourceRepository::saveCredentialSource() has invalid typehint type
         Webauthn\PublicKeyCredentialSource.
  41     Call to method getPublicKeyCredentialId() on an unknown class Webauthn\PublicKeyCredentialSource.
  49     Call to method getUserHandle() on an unknown class Webauthn\PublicKeyCredentialSource.
  64     Return typehint of method Drupal\webauthn_authenticator\Repository\PublicKeyCredentialSourceRepository::generatePublicKeyCredentialSource() has invalid type Webauthn\PublicKeyCredentialSource.
  64     Return typehint of method Drupal\webauthn_authenticator\Repository\PublicKeyCredentialSourceRepository::generatePublicKeyCredentialSource() has invalid type Webauthn\PublicKeyCredentialSource.
  70     Call to static method createFromArray() on an unknown class Webauthn\PublicKeyCredentialSource.
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------------
  Line   src/Services/RpServer.php
 ------ -------------------------------------------------------------------------------
         Internal error: Class 'Webauthn\PublicKeyCredentialCreationOptions' not found
         Run PHPStan with --debug option and post the stack trace to:
         https://github.com/phpstan/phpstan/issues/new
 ------ -------------------------------------------------------------------------------

 ------ ----------------------------------------------------------------------------------------------------------
  Line   src/Validate/WebAuthnAuthenticatorUserLoginFormValidate.php
 ------ ----------------------------------------------------------------------------------------------------------
  32     PHPDoc tag @var for variable $user_entity contains unknown class Webauthn\PublicKeyCredentialUserEntity.
  45     Call to method getId() on an unknown class Webauthn\PublicKeyCredentialUserEntity.
 ------ ----------------------------------------------------------------------------------------------------------
otofu’s picture

Assigned: Unassigned » otofu
otofu’s picture

Thank you all for your reviews.

1. In js files you have mention doc comments in other language, is there any reason

I was writing in my native language due to an omission to change my comment. It was Changed.

2. I didn't find any configure link on module installation page for webauthn credentials.

This module creates WebAuthn credentials tab on user profile pages(/user/{uid}/webauthn-authenticator), but doesn’t create general configuration page.

I guess the reason of the unknown class errors happening here is that third party libraries are not read by PHPStan. Does anyone know how to solve this?
(ex: PHPDoc tag @var for variable $credential_options contains unknown class Webauthn\PublicKeyCredentialCreationOptions.)

apaderno’s picture

Assigned: otofu » Unassigned
sharma.amitt16’s picture

Thanks for your contribution. Please find my review below.

Coding standard and drupal check for drupal 9

~/Sites/drupal-9.0.0-beta2/modules/contrib(9.1.x*) » drupal-check webauthn_authenticator                                                                                           
 12/12 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------------------------------
  Line   src/Controller/AuthenticationController.php
 ------ ------------------------------------------------------------------------------
         Internal error: Class 'Webauthn\PublicKeyCredentialRequestOptions' not found
         Run PHPStan with --debug option and post the stack trace to:
         https://github.com/phpstan/phpstan/issues/new
 ------ ------------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------------
  Line   src/Controller/RegistrationController.php
 ------ -------------------------------------------------------------------------------
         Internal error: Class 'Webauthn\PublicKeyCredentialCreationOptions' not found
         Run PHPStan with --debug option and post the stack trace to:
         https://github.com/phpstan/phpstan/issues/new
 ------ -------------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------------------------
  Line   src/Repository/PublicKeyCredentialSourceRepository.php
 ------ -------------------------------------------------------------------------------------------
  13     Class Webauthn\PublicKeyCredentialSourceRepository not found and could not be autoloaded.
 ------ -------------------------------------------------------------------------------------------


 [ERROR] Found 3 errors


------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
~/Sites/drupal-9.0.0-beta2/modules/contrib(9.1.x*) » phpcs --standard=Drupal webauthn_authenticator                                                                                

FILE: /Users/amitsharma/Sites/drupal-9.0.0-beta2/modules/contrib/webauthn_authenticator/js/utils.js
---------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------
 4 | ERROR | [x] There must be exactly one blank line after the file comment
---------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------

Time: 300ms; Memory: 8MB

AuthenticationController.php

   $username = $request->query->get('username');
   $user = user_load_by_name($username);
   if (!$user) {

It will be better if you will use loadByProperties() function of entityTypeManager() to load the user instead of user_load_by_name

RegistrationController.php

$credential_options = $this->rpServer->generatePublicKeyCredentialCreationOptions($user_entity, PublicKeyCredentialCreationOptions::ATTESTATION_CONVEYANCE_PREFERENCE_NONE, $exclude_credentials);

    try {
      $this->tempstore->set('credential_creation_options', $credential_options);
    }
    catch (\Exception $e) {
    }

    return new JsonResponse($credential_options);

You should log the exception or throw the exception instead of keeping it empty.

WebAuthncontroller.php

   $build = [];

    $rows = [];

Instead, you can use
$build = $rows = [];

Rest you did a great job.

otofu’s picture

Thanks for the review.

The drupal-check is a 3rd party library error, so I don't know how to fix it.

I've fixed everything else, so please check back.

apaderno’s picture

The internal errors aren't errors in the project code, but in the tool used to check the project code, so they don't need any change in the project used for this application.

klausi’s picture

Status: Needs review » Needs work
Issue tags: +PAreview: security

Thanks for your contribution!

  1. WebAuthnAuthenticatorUserLoginFormValidate: All code comments should be in English, see https://www.drupal.org/docs/develop/standards/api-documentation-and-comm...
  2. "/user/{user}/webauthn-authenticator/register/get-registration-options": this path is vulnerable to CSRF exploits. The GET request performs a data changing action by writing to the tempstore, so this needs to be protected with a confirmation form or a CSRF token. See https://www.drupal.org/docs/8/api/routing-system/access-checking-on-rout...
  3. Same for /webauthn-authenticator/get-authentication-options. Please check all your routes in the routing.yml file if they perform data changing actions and protect them if necessary.
  4. WebAuthnAuthenticatorUserLoginFormValidate: where do you check if the login request is valid? You call loadAndCheckAssertionResponse() but don't check the return value. Or does it throw exceptions when the login request is not valid? Please add a comment in code for this security critical part.

And please don't remove the security tag, we keep that for statistics and to show examples of security problems.

otofu’s picture

Status: Needs work » Needs review

Thanks for the review.

I have responded to the comments you made and I would ask you to check again.

Same for /webauthn-authenticator/get-authentication-options. Please check all your routes in the routing.yml file if they perform data changing actions and protect them if necessary.

No changes have been made to this routing because it is not possible to check the CSRF Token because an anonymous user will access it.

https://www.drupal.org/docs/8/api/routing-system/access-checking-on-rout...

klausi’s picture

Status: Needs review » Fixed

Thanks, good point about anonymous user CSRF - so that should be fine!

  1. The Git commits are not connected to your user account. You need to specify an email address. See https://www.drupal.org/node/1022156 and https://www.drupal.org/node/1051722
  2. Project page is a bit short can you expand it according to https://www.drupal.org/docs/develop/documenting-your-project/project-pag... ?

Not really familiar with webauthn and the user authentication security implications, but I don't see any obvious security problems. Looks good to me!

Thanks for your contribution, Motoki!

I updated your account so you can opt into security advisory coverage now.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on Slack or IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.