monicahq / monica

Personal CRM. Remember everything about your friends, family and business relationships.
https://beta.monicahq.com
GNU Affero General Public License v3.0
21.7k stars 2.18k forks source link

unset GOOGLE_SAFETYNET_API_KEY environment var causes 500 for webauthn devices #2917

Closed quinndiggity closed 5 years ago

quinndiggity commented 5 years ago

Describe the bug When the environment var GOOGLE_SAFETYNET_API_KEY is not set, a 500 error is received when trying to add a webauthn device.

The specific issue is:

if ($this->config->get('webauthn.google_safetynet_api_key', '') !== '') {

^ this check is done, but the default value of $this->config->get('webauthn.google_safetynet_api_key', '') is NULL

Likely due to:

root@monica:/var/www/monica# grep -ir google_safetynet_api_key .
./vendor/asbiin/laravel-webauthn/src/Services/Webauthn/AbstractValidatorFactory.php:        if ($this->config->get('webauthn.google_safetynet_api_key', '') !== '') {
./vendor/asbiin/laravel-webauthn/src/Services/Webauthn/AbstractValidatorFactory.php:            $attestationStatementSupportManager->add(new AndroidSafetyNetAttestationStatementSupport(new Client(), $this->config->get('webauthn.google_safetynet_api_key')));
./vendor/asbiin/laravel-webauthn/config/webauthn.php:    'google_safetynet_api_key' => '',
./bootstrap/cache/config.php:    'google_safetynet_api_key' => NULL,
./config/webauthn.php:    'google_safetynet_api_key' => env('GOOGLE_SAFETYNET_API_KEY'),

./bootstrap/cache/config.php: 'google_safetynet_api_key' => NULL, it appears bootstrap cache config sets google_safetynet_api_key to NULL, therefore the previous check fails.

500 error response body:

{
    "message": "Argument 2 passed to Webauthn\\AttestationStatement\\AndroidSafetyNetAttestationStatementSupport::__construct() must be of the type string, null given, called in /var/www/monica/vendor/asbiin/laravel-webauthn/src/Services/Webauthn/AbstractValidatorFactory.php on line 54"
}
Screenshot 2019-08-18 at 3 44 17 PM
quinndiggity commented 5 years ago

note, setting the following in .env and regenerating the cached config resolves the issue:

# no value needed, but without the declaration "NULL" is cached instead of an empty string
GOOGLE_SAFETYNET_API_KEY=
php artisan cache:clear
php artisan config:cache
asbiin commented 5 years ago

Thank you for the report @quinndiggity

This is already fixed in https://github.com/asbiin/laravel-webauthn/pull/62 https://github.com/monicahq/monica/pull/2706

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.