Merge pull request #12 from DarkGhostHunter/master

Fixed recovery attestation failure.

Author: DarkGhostHunter

README.md

@@ -46,7 +46,7 @@ Just hit the console and require it with Composer.
 
 ## What is WebAuthn? How it uses fingerprints or else?
 
-In a nutshell, [mayor browsers are compatible with Web Authentication API](https://caniuse.com/#feat=webauthn), pushing authentication to the device (fingerprints, Face ID, patterns, codes, etc) instead of plain-text passwords.
+In a nutshell, [major browsers are compatible with Web Authentication API](https://caniuse.com/#feat=webauthn), pushing authentication to the device (fingerprints, Face ID, patterns, codes, etc) instead of plain-text passwords.
 
 This package validates the WebAuthn payload from the devices using a custom [user provider](https://laravel.com/docs/authentication#adding-custom-user-providers).
 
@@ -617,23 +617,23 @@ if (! Larapass.supportsWebAuthn()) {
 
 * **Does this stores the user's fingerprint, PIN or patterns in my site?**
 
-No.
+No. It stores the public key generated by the device.
 
 * **Can a phishing site steal WebAuthn credentials and use them in my site?**
 
 No. WebAuthn kills phishing.
 
 * **Can the WebAuthn data identify a particular device?**
 
-Not, unless explicitly requested and consented.
+No, unless explicitly requested and consented.
 
 * **Are my user's classic passwords safe?**
 
 Yes, as long you are hashing them as you should, and you have secured your application key. This is done by Laravel by default. You can also [disable them](#password-fallback).
 
 * **Can a user register two or more _devices_?**
 
-Yes, but you need to manually attest (register) these.
+Yes, but you need to manually attest (register) these. It's recommended to email him to register a new device.
 
 * **What happens if a credential is cloned?**
 
@@ -669,19 +669,19 @@ class MyCountChecker implements CounterChecker
 
 * **If a user loses his device, can he register a new device?**
 
-Yes, just send him a signed email to register a new device with secure attestation and assertion routes. You can [use these recovery helpers](#6-set-up-account-recovery-optional).
+Yes, [use these recovery helpers](#6-set-up-account-recovery-optional).
 
 * **What's the difference between disabling and deleting a credential?**
 
-Disabling a credential doesn't delete it, so it can be later enabled manually. When the credential is deleted, it goes away forever.
+Disabling a credential doesn't delete it, so it can be later enabled manually in the case the user recovers it. When the credential is deleted, it goes away forever.
 
 * **How secure is this against passwords or 2FA?**
 
-Extremely secure since it works only on HTTPS, and no password or codes are exchanged after registration.
+Extremely secure since it works only on HTTPS, and no password or codes are exchanged.
 
 * **Can I deactivate the password fallback? Can I enforce only WebAuthn authentication?**
 
-Yes. Just be sure to [use the recovery helpers](#6-set-up-account-recovery-optional) if you want a quick fix.
+Yes. Just be sure to [use the recovery helpers](#6-set-up-account-recovery-optional) to avoid users locked out.
 
 * **Does this includes a frontend Javascript?**
 
@@ -695,6 +695,10 @@ Yes, the included [WebAuthn Helper](#5-use-the-javascript-helper-optional) does
 
 [Yes.](#6-set-up-account-recovery-optional)
 
+* **Can I use my smartphone as authenticator through a PC desktop/laptop/terminal?**
+
+Depends on the OS and hardware. Some will require previously pairing the device to an "account". Others won't and will only work with USB keys. This is up to hardware and software vendor themselves.
+
 ## License
 
 The MIT License (MIT). Please see [License File](LICENSE.md) for more information.

composer.json

@@ -31,12 +31,12 @@
     },
     "autoload": {
         "psr-4": {
-            "DarkGhostHunter\\Larapass\\": "src"
+            "DarkGhostHunter\\Larapass\\": "src/"
         }
     },
     "autoload-dev": {
         "psr-4": {
-            "Tests\\": "tests"
+            "Tests\\": "tests/"
         }
     },
     "scripts": {
@@ -53,4 +53,4 @@
             ]
         }
     }
-}
+}

src/Auth/EloquentWebAuthnProvider.php

@@ -87,7 +87,7 @@ protected function isSignedChallenge(array $credentials)
      * @param  array  $credentials
      * @return bool
      */
-    public function validateCredentials(UserContract $user, array $credentials)
+    public function validateCredentials($user, array $credentials)
     {
         if ($this->isSignedChallenge($credentials)) {
             return (bool)$this->validator->validate($credentials);

src/Contracts/WebAuthnAuthenticatable.php

@@ -64,7 +64,7 @@ public function flushCredentials($except = null) : void;
      * Checks if a given credential exists and is enabled.
      *
      * @param  string  $id
-     * @return mixed
+     * @return bool
      */
     public function hasCredentialEnabled(string $id) : bool;
 

src/Eloquent/Casting/UuidCast.php

@@ -28,12 +28,12 @@ public function get($model, string $key, $value, array $attributes)
      * @param  string  $key
      * @param  mixed  $value
      * @param  array  $attributes
-     * @return \Ramsey\Uuid\UuidInterface
+     * @return array|string
      */
     public function set($model, string $key, $value, array $attributes)
     {
-        return mb_strlen($value, '8bit') === 36
+        return (mb_strlen($value, '8bit') === 36
             ? Uuid::fromString($value)
-            : Uuid::fromBytes(base64_decode($value, true));
+            : Uuid::fromBytes(base64_decode($value, true)))->toString();
     }
 }

src/Eloquent/WebAuthnCredential.php

@@ -120,7 +120,7 @@ public function isDisabled()
     /**
      * Returns the credential ID encoded in BASE64.
      *
-     * @return false
+     * @return string
      */
     public function getPrettyIdAttribute()
     {

src/Http/RecoversWebAuthn.php

@@ -77,7 +77,9 @@ public function recover(Request $request)
         ], $this->rules())->validate();
 
         $response = WebAuthn::recover($credentials, function ($user) use ($request) {
-            $this->register($request, $user);
+            if (! $this->register($request, $user)) {
+                $this->sendRecoveryFailedResponse($request, 'larapass::recovery.failed');
+            }
         });
 
         return $response === WebAuthn::RECOVERY_ATTACHED
@@ -90,7 +92,7 @@ public function recover(Request $request)
      *
      * @param  \Illuminate\Http\Request  $request
      * @param  \DarkGhostHunter\Larapass\Contracts\WebAuthnAuthenticatable  $user
-     * @return void
+     * @return bool
      */
     protected function register(Request $request, WebAuthnAuthenticatable $user)
     {
@@ -108,7 +110,11 @@ protected function register(Request $request, WebAuthnAuthenticatable $user)
             event(new AttestationSuccessful($user, $validCredential));
 
             $this->guard()->login($user);
+
+            return true;
         }
+
+        return false;
     }
 
     /**
@@ -143,6 +149,7 @@ protected function sendRecoveryResponse(Request $request, $response)
      *
      * @param  \Illuminate\Http\Request  $request
      * @param  string  $response
+     * @return \Illuminate\Http\JsonResponse|void
      * @throws \Illuminate\Validation\ValidationException
      */
     protected function sendRecoveryFailedResponse(Request $request, $response)

src/WebAuthn/WebAuthnAssertValidator.php

@@ -82,7 +82,7 @@ class WebAuthnAssertValidator
     /**
      * If the login should require explicit User verification.
      *
-     * @var bool
+     * @var string
      */
     protected $verifyLogin;
 

src/WebAuthn/WebAuthnAttestCreator.php

@@ -66,7 +66,7 @@ class WebAuthnAttestCreator
     /**
      * If the devices should be further verified.
      *
-     * @var bool
+     * @var string
      */
     protected $conveyance;
 
@@ -122,7 +122,7 @@ public function __construct(ConfigContract $config,
      * @param  \Illuminate\Contracts\Auth\Authenticatable|\DarkGhostHunter\Larapass\Contracts\WebAuthnAuthenticatable  $user
      * @return \Webauthn\PublicKeyCredentialCreationOptions|null
      */
-    public function retrieveAttestation(WebAuthnAuthenticatable $user)
+    public function retrieveAttestation($user)
     {
         return $this->cache->get($this->cacheKey($user));
     }
@@ -133,7 +133,7 @@ public function retrieveAttestation(WebAuthnAuthenticatable $user)
      * @param  \Illuminate\Contracts\Auth\Authenticatable|\DarkGhostHunter\Larapass\Contracts\WebAuthnAuthenticatable  $user
      * @return mixed|\Webauthn\PublicKeyCredentialCreationOptions
      */
-    public function generateAttestation(WebAuthnAuthenticatable $user)
+    public function generateAttestation($user)
     {
         $attestation = $this->makeAttestationRequest($user);
 

src/WebAuthn/WebAuthnAttestValidator.php

@@ -80,7 +80,7 @@ public function __construct(ConfigContract $config,
      * @param  \Illuminate\Contracts\Auth\Authenticatable|\DarkGhostHunter\Larapass\Contracts\WebAuthnAuthenticatable  $user
      * @return bool|\Webauthn\PublicKeyCredentialSource
      */
-    public function validate(array $data, WebAuthnAuthenticatable $user)
+    public function validate(array $data, $user)
     {
         if (! $attestation = $this->retrieveAttestation($user)) {
             return false;