Title: [271856] branches/safari-611-branch/Source/WebKit
Revision
271856
Author
alanc...@apple.com
Date
2021-01-25 14:13:38 -0800 (Mon, 25 Jan 2021)

Log Message

Cherry-pick r271613. rdar://problem/73477407

    [WebAuthn] Polish the new WebAuthn UI
    https://bugs.webkit.org/show_bug.cgi?id=220617
    <rdar://problem/73185470>

    Reviewed by Brent Fulgham.

    This patch does the following few things:
    1. It updates the way how the PIN error for security keys is handled.
    2. It uses the credential name to identify a credential that passed to the UI instead of the login choice object
    as it turns out that the UI won't return the same object at all.
    3. It delays to show the UI if the platform authenticator is involved given the platform authenticator might not contain
    the requested credentials. If not, we should either show an error or just requesting the security key ones.

    Covered by manual tests.

    * Platform/spi/Cocoa/AuthenticationServicesCoreSPI.h:
    (NS_ERROR_ENUM):
    * UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.h:
    * UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.mm:
    (WebKit::AuthenticatorPresenterCoordinator::AuthenticatorPresenterCoordinator):
    (WebKit::AuthenticatorPresenterCoordinator::updatePresenter):
    (WebKit::AuthenticatorPresenterCoordinator::selectAssertionResponse):
    (WebKit::AuthenticatorPresenterCoordinator::didSelectAssertionResponse):
    * UIProcess/WebAuthentication/Cocoa/WKASCAuthorizationPresenterDelegate.mm:
    (-[WKASCAuthorizationPresenterDelegate authorizationPresenter:credentialRequestedForLoginChoice:authenticatedContext:completionHandler:]):

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271613 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-611-branch/Source/WebKit/ChangeLog (271855 => 271856)


--- branches/safari-611-branch/Source/WebKit/ChangeLog	2021-01-25 22:13:32 UTC (rev 271855)
+++ branches/safari-611-branch/Source/WebKit/ChangeLog	2021-01-25 22:13:38 UTC (rev 271856)
@@ -1,5 +1,66 @@
 2021-01-25  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r271613. rdar://problem/73477407
+
+    [WebAuthn] Polish the new WebAuthn UI
+    https://bugs.webkit.org/show_bug.cgi?id=220617
+    <rdar://problem/73185470>
+    
+    Reviewed by Brent Fulgham.
+    
+    This patch does the following few things:
+    1. It updates the way how the PIN error for security keys is handled.
+    2. It uses the credential name to identify a credential that passed to the UI instead of the login choice object
+    as it turns out that the UI won't return the same object at all.
+    3. It delays to show the UI if the platform authenticator is involved given the platform authenticator might not contain
+    the requested credentials. If not, we should either show an error or just requesting the security key ones.
+    
+    Covered by manual tests.
+    
+    * Platform/spi/Cocoa/AuthenticationServicesCoreSPI.h:
+    (NS_ERROR_ENUM):
+    * UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.h:
+    * UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.mm:
+    (WebKit::AuthenticatorPresenterCoordinator::AuthenticatorPresenterCoordinator):
+    (WebKit::AuthenticatorPresenterCoordinator::updatePresenter):
+    (WebKit::AuthenticatorPresenterCoordinator::selectAssertionResponse):
+    (WebKit::AuthenticatorPresenterCoordinator::didSelectAssertionResponse):
+    * UIProcess/WebAuthentication/Cocoa/WKASCAuthorizationPresenterDelegate.mm:
+    (-[WKASCAuthorizationPresenterDelegate authorizationPresenter:credentialRequestedForLoginChoice:authenticatedContext:completionHandler:]):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271613 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-01-19  Jiewen Tan  <jiewen_...@apple.com>
+
+            [WebAuthn] Polish the new WebAuthn UI
+            https://bugs.webkit.org/show_bug.cgi?id=220617
+            <rdar://problem/73185470>
+
+            Reviewed by Brent Fulgham.
+
+            This patch does the following few things:
+            1. It updates the way how the PIN error for security keys is handled.
+            2. It uses the credential name to identify a credential that passed to the UI instead of the login choice object
+            as it turns out that the UI won't return the same object at all.
+            3. It delays to show the UI if the platform authenticator is involved given the platform authenticator might not contain
+            the requested credentials. If not, we should either show an error or just requesting the security key ones.
+
+            Covered by manual tests.
+
+            * Platform/spi/Cocoa/AuthenticationServicesCoreSPI.h:
+            (NS_ERROR_ENUM):
+            * UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.h:
+            * UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.mm:
+            (WebKit::AuthenticatorPresenterCoordinator::AuthenticatorPresenterCoordinator):
+            (WebKit::AuthenticatorPresenterCoordinator::updatePresenter):
+            (WebKit::AuthenticatorPresenterCoordinator::selectAssertionResponse):
+            (WebKit::AuthenticatorPresenterCoordinator::didSelectAssertionResponse):
+            * UIProcess/WebAuthentication/Cocoa/WKASCAuthorizationPresenterDelegate.mm:
+            (-[WKASCAuthorizationPresenterDelegate authorizationPresenter:credentialRequestedForLoginChoice:authenticatedContext:completionHandler:]):
+
+2021-01-25  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r271537. rdar://problem/73478698
 
     REGRESSION(r267763) Uploading zipped directories does not work on iOS

Modified: branches/safari-611-branch/Source/WebKit/Platform/spi/Cocoa/AuthenticationServicesCoreSPI.h (271855 => 271856)


--- branches/safari-611-branch/Source/WebKit/Platform/spi/Cocoa/AuthenticationServicesCoreSPI.h	2021-01-25 22:13:32 UTC (rev 271855)
+++ branches/safari-611-branch/Source/WebKit/Platform/spi/Cocoa/AuthenticationServicesCoreSPI.h	2021-01-25 22:13:38 UTC (rev 271856)
@@ -172,16 +172,11 @@
     ASCAuthorizationErrorNoCredentialsFound,
     ASCAuthorizationErrorLAError,
     ASCAuthorizationErrorLAExcludeCredentialsMatched,
+    ASCAuthorizationErrorPINInvalid,
+    ASCAuthorizationErrorAuthenticatorTemporarilyLocked,
+    ASCAuthorizationErrorAuthenticatorPermanentlyLocked,
 };
 
-extern NSString * const ASCPINValidationResultKey;
-
-typedef NS_ENUM(NSInteger, ASCPINValidationResult) {
-    ASCPINValidationResultPINBlocked,
-    ASCPINValidationResultPINAuthBlocked,
-    ASCPINValidationResultPINInvalid,
-};
-
 NS_ASSUME_NONNULL_END
 
 //#endif // USE(APPLE_INTERNAL_SDK)

Modified: branches/safari-611-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.h (271855 => 271856)


--- branches/safari-611-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.h	2021-01-25 22:13:32 UTC (rev 271855)
+++ branches/safari-611-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.h	2021-01-25 22:13:38 UTC (rev 271856)
@@ -65,7 +65,7 @@
 
     void setCredentialRequestHandler(CredentialRequestHandler&& handler) { m_credentialRequestHandler = WTFMove(handler); }
     void setLAContext(LAContext *);
-    void didSelectAssertionResponse(ASCLoginChoiceProtocol *, LAContext *);
+    void didSelectAssertionResponse(const String& credentialName, LAContext *);
     void setPin(const String&);
 
 private:
@@ -73,6 +73,10 @@
     RetainPtr<ASCAuthorizationPresentationContext> m_context;
     RetainPtr<ASCAuthorizationPresenter> m_presenter;
     RetainPtr<WKASCAuthorizationPresenterDelegate> m_presenterDelegate;
+    Function<void()> m_delayedPresentation;
+#if HAVE(ASC_AUTH_UI)
+    bool m_delayedPresentationNeedsSecurityKey { false };
+#endif
 
     CredentialRequestHandler m_credentialRequestHandler;
 
@@ -80,7 +84,7 @@
     RetainPtr<LAContext> m_laContext;
 
     CompletionHandler<void(WebCore::AuthenticatorAssertionResponse*)> m_responseHandler;
-    HashMap<ASCLoginChoiceProtocol *, RefPtr<WebCore::AuthenticatorAssertionResponse>> m_credentials;
+    HashMap<String, RefPtr<WebCore::AuthenticatorAssertionResponse>> m_credentials;
 
     CompletionHandler<void(const String&)> m_pinHandler;
 };

Modified: branches/safari-611-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.mm (271855 => 271856)


--- branches/safari-611-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.mm	2021-01-25 22:13:32 UTC (rev 271855)
+++ branches/safari-611-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/AuthenticatorPresenterCoordinator.mm	2021-01-25 22:13:38 UTC (rev 271856)
@@ -53,7 +53,7 @@
             [m_context addLoginChoice:adoptNS([allocASCSecurityKeyPublicKeyCredentialLoginChoiceInstance() initRegistrationChoice]).get()];
         break;
     case ClientDataType::Get:
-        if (transports.contains(AuthenticatorTransport::Usb) || transports.contains(AuthenticatorTransport::Nfc))
+        if ((transports.contains(AuthenticatorTransport::Usb) || transports.contains(AuthenticatorTransport::Nfc)) && !transports.contains(AuthenticatorTransport::Internal))
             [m_context addLoginChoice:adoptNS([allocASCSecurityKeyPublicKeyCredentialLoginChoiceInstance() initAssertionPlaceholderChoice]).get()];
         break;
     default:
@@ -65,7 +65,6 @@
     [m_presenter setDelegate:m_presenterDelegate.get()];
 
     auto completionHandler = makeBlockPtr([manager = m_manager] (id<ASCCredentialProtocol> credential, NSError *error) mutable {
-        ASSERT(!RunLoop::isMain());
         if (credential || !error)
             return;
 
@@ -76,6 +75,15 @@
                 manager->cancel();
         });
     });
+
+    if (type == ClientDataType::Get && transports.contains(AuthenticatorTransport::Internal)) {
+        m_delayedPresentationNeedsSecurityKey = (transports.contains(AuthenticatorTransport::Usb) || transports.contains(AuthenticatorTransport::Nfc));
+        m_delayedPresentation = [completionHandler = WTFMove(completionHandler), this] {
+            [m_presenter presentAuthorizationWithContext:m_context.get() completionHandler:completionHandler.get()];
+        };
+        return;
+    }
+
     [m_presenter presentAuthorizationWithContext:m_context.get() completionHandler:completionHandler.get()];
 #endif // HAVE(ASC_AUTH_UI)
 }
@@ -95,17 +103,17 @@
 #if HAVE(ASC_AUTH_UI)
     switch (status) {
     case WebAuthenticationStatus::PinBlocked: {
-        auto error = adoptNS([[NSError alloc] initWithDomain:ASCAuthorizationErrorDomain code:ASCAuthorizationErrorPINRequired userInfo:@{ ASCPINValidationResultKey: @(ASCPINValidationResultPINBlocked) }]);
+        auto error = adoptNS([[NSError alloc] initWithDomain:ASCAuthorizationErrorDomain code:ASCAuthorizationErrorAuthenticatorPermanentlyLocked userInfo:nil]);
         m_credentialRequestHandler(nil, error.get());
         break;
     }
     case WebAuthenticationStatus::PinAuthBlocked: {
-        auto error = adoptNS([[NSError alloc] initWithDomain:ASCAuthorizationErrorDomain code:ASCAuthorizationErrorPINRequired userInfo:@{ ASCPINValidationResultKey: @(ASCPINValidationResultPINAuthBlocked) }]);
+        auto error = adoptNS([[NSError alloc] initWithDomain:ASCAuthorizationErrorDomain code:ASCAuthorizationErrorAuthenticatorTemporarilyLocked userInfo:nil]);
         m_credentialRequestHandler(nil, error.get());
         break;
     }
     case WebAuthenticationStatus::PinInvalid: {
-        auto error = adoptNS([[NSError alloc] initWithDomain:ASCAuthorizationErrorDomain code:ASCAuthorizationErrorPINRequired userInfo:@{ ASCPINValidationResultKey: @(ASCPINValidationResultPINInvalid) }]);
+        auto error = adoptNS([[NSError alloc] initWithDomain:ASCAuthorizationErrorDomain code:ASCAuthorizationErrorPINInvalid userInfo:nil]);
         m_credentialRequestHandler(nil, error.get());
         break;
     }
@@ -114,8 +122,12 @@
         [m_presenter updateInterfaceForUserVisibleError:error.get()];
         break;
     }
-    case WebAuthenticationStatus::NoCredentialsFound:
-    case WebAuthenticationStatus::LANoCredential: {
+    case WebAuthenticationStatus::LANoCredential:
+        if (m_delayedPresentationNeedsSecurityKey)
+            [m_context addLoginChoice:adoptNS([allocASCSecurityKeyPublicKeyCredentialLoginChoiceInstance() initAssertionPlaceholderChoice]).get()];
+        m_delayedPresentation();
+        break;
+    case WebAuthenticationStatus::NoCredentialsFound: {
         auto error = adoptNS([[NSError alloc] initWithDomain:ASCAuthorizationErrorDomain code:ASCAuthorizationErrorNoCredentialsFound userInfo:nil]);
         [m_presenter updateInterfaceForUserVisibleError:error.get()];
         break;
@@ -155,6 +167,7 @@
     if (source == WebAuthenticationSource::External) {
         auto loginChoices = adoptNS([[NSMutableArray alloc] init]);
 
+        m_credentials.clear();
         for (auto& response : responses) {
             RetainPtr<NSData> userHandle;
             if (response->userHandle())
@@ -163,7 +176,7 @@
             auto loginChoice = adoptNS([allocASCSecurityKeyPublicKeyCredentialLoginChoiceInstance() initWithName:response->name() displayName:response->displayName() userHandle:userHandle.get()]);
             [loginChoices addObject:loginChoice.get()];
 
-            m_credentials.add((ASCLoginChoiceProtocol *)loginChoice.get(), WTFMove(response));
+            m_credentials.add(response->name(), WTFMove(response));
         }
 
         [m_presenter updateInterfaceWithLoginChoices:loginChoices.get()];
@@ -171,8 +184,7 @@
     }
 
     if (source == WebAuthenticationSource::Local) {
-        auto loginChoices = adoptNS([[NSMutableArray alloc] init]);
-
+        m_credentials.clear();
         for (auto& response : responses) {
             RetainPtr<NSData> userHandle;
             if (response->userHandle())
@@ -179,13 +191,14 @@
                 userHandle = adoptNS([[NSData alloc] initWithBytes:response->userHandle()->data() length:response->userHandle()->byteLength()]);
 
             auto loginChoice = adoptNS([allocASCPlatformPublicKeyCredentialLoginChoiceInstance() initWithName:response->name() displayName:response->displayName() userHandle:userHandle.get()]);
-            [loginChoices addObject:loginChoice.get()];
+            [m_context addLoginChoice:loginChoice.get()];
 
-            m_credentials.add((ASCLoginChoiceProtocol *)loginChoice.get(), WTFMove(response));
+            m_credentials.add(response->name(), WTFMove(response));
         }
 
-        [loginChoices addObjectsFromArray:[m_context loginChoices]]; // Adds the security key option if exists.
-        [m_presenter updateInterfaceWithLoginChoices:loginChoices.get()];
+        if (m_delayedPresentationNeedsSecurityKey)
+            [m_context addLoginChoice:adoptNS([allocASCSecurityKeyPublicKeyCredentialLoginChoiceInstance() initAssertionPlaceholderChoice]).get()];
+        m_delayedPresentation();
         return;
     }
 #endif // HAVE(ASC_AUTH_UI)
@@ -225,9 +238,9 @@
     m_laContext = context;
 }
 
-void AuthenticatorPresenterCoordinator::didSelectAssertionResponse(ASCLoginChoiceProtocol *loginChoice, LAContext *context)
+void AuthenticatorPresenterCoordinator::didSelectAssertionResponse(const String& credentialName, LAContext *context)
 {
-    auto response = m_credentials.take(loginChoice);
+    auto response = m_credentials.take(credentialName);
     if (!response)
         return;
 

Modified: branches/safari-611-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WKASCAuthorizationPresenterDelegate.mm (271855 => 271856)


--- branches/safari-611-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WKASCAuthorizationPresenterDelegate.mm	2021-01-25 22:13:32 UTC (rev 271855)
+++ branches/safari-611-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WKASCAuthorizationPresenterDelegate.mm	2021-01-25 22:13:38 UTC (rev 271856)
@@ -55,7 +55,9 @@
     }];
 
     if ([loginChoice isKindOfClass:WebKit::getASCPlatformPublicKeyCredentialLoginChoiceClass()]) {
-        if ([(ASCPlatformPublicKeyCredentialLoginChoice *)loginChoice isRegistrationRequest]) {
+        auto *platformLoginChoice = (ASCPlatformPublicKeyCredentialLoginChoice *)loginChoice;
+
+        if ([platformLoginChoice isRegistrationRequest]) {
             [self dispatchCoordinatorCallback:[context = retainPtr(context)] (WebKit::AuthenticatorPresenterCoordinator& coordinator) mutable {
                 coordinator.setLAContext(context.get());
             }];
@@ -63,19 +65,21 @@
             return;
         }
 
-        if (![(ASCPlatformPublicKeyCredentialLoginChoice *)loginChoice isRegistrationRequest]) {
-            [self dispatchCoordinatorCallback:[loginChoice, context = retainPtr(context)] (WebKit::AuthenticatorPresenterCoordinator& coordinator) mutable {
-                coordinator.didSelectAssertionResponse((ASCLoginChoiceProtocol *)loginChoice, context.get());
-            }];
+        String loginChoiceName = platformLoginChoice.name;
+        [self dispatchCoordinatorCallback:[loginChoiceName = WTFMove(loginChoiceName), context = retainPtr(context)] (WebKit::AuthenticatorPresenterCoordinator& coordinator) mutable {
+            coordinator.didSelectAssertionResponse(loginChoiceName, context.get());
+        }];
 
-            return;
-        }
+        return;
     }
 
     if ([loginChoice isKindOfClass:WebKit::getASCSecurityKeyPublicKeyCredentialLoginChoiceClass()]) {
-        if ([(ASCSecurityKeyPublicKeyCredentialLoginChoice *)loginChoice loginChoiceKind] == ASCSecurityKeyPublicKeyCredentialLoginChoiceKindAssertion) {
-            [self dispatchCoordinatorCallback:[loginChoice] (WebKit::AuthenticatorPresenterCoordinator& coordinator) mutable {
-                coordinator.didSelectAssertionResponse((ASCLoginChoiceProtocol *)loginChoice, nil);
+        auto *securityKeyLoginChoice = (ASCSecurityKeyPublicKeyCredentialLoginChoice *)loginChoice;
+
+        if ([securityKeyLoginChoice loginChoiceKind] == ASCSecurityKeyPublicKeyCredentialLoginChoiceKindAssertion) {
+            String loginChoiceName = securityKeyLoginChoice.name;
+            [self dispatchCoordinatorCallback:[loginChoiceName = WTFMove(loginChoiceName)] (WebKit::AuthenticatorPresenterCoordinator& coordinator) mutable {
+                coordinator.didSelectAssertionResponse(loginChoiceName, nil);
             }];
 
             return;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to