Skip to content

Commit

Permalink
Apply patch. rdar://123724533
Browse files Browse the repository at this point in the history
    Password manager UI can cause conditional requests to pause https://bugs.webkit.org/show_bug.cgi?id=270135 rdar://123659147

    Reviewed by Charlie Wolfe and Chris Dumez.

    The API for autofill assisted passkey requests only allows one request ongoing per UI process.
    In order to handle this behavior, we currently pause requests whenever their page is not active
    and resume them whenever the page becomes active.

    However, bringing up password manager UI as part of an conditional mediation may cause the page to
    become not active, therefore pausing the request the user is selecting a credential for in the UI.

    To fix this issue, we don't pause requests whenever a page becomes inactive. Instead we pause the active
    request whenever another page becomes active. This will result in no pause when brining up password manager
    UI.

    * Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:
    (WebKit::WebAuthenticatorCoordinatorProxy::pauseConditionalAssertion):
    (WebKit::WebAuthenticatorCoordinatorProxy::makeActiveConditionalAssertion):
    (WebKit::WebAuthenticatorCoordinatorProxy::performRequest):
    * Source/WebKit/UIProcess/WebAuthentication/WebAuthenticatorCoordinatorProxy.h:
    * Source/WebKit/UIProcess/WebPageProxy.cpp:
    (WebKit::WebPageProxy::dispatchActivityStateChange):

    Canonical link: https://commits.webkit.org/275414@main

Identifier: 272448.802@safari-7618-branch
  • Loading branch information
Dan Robson committed Mar 26, 2024
1 parent bf48da0 commit b627464
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,24 @@ static inline ASAuthorizationPublicKeyCredentialLargeBlobSupportRequirement toAS
return requests;
}

void WebAuthenticatorCoordinatorProxy::pauseConditionalAssertion()
WeakPtr<WebAuthenticatorCoordinatorProxy>& WebAuthenticatorCoordinatorProxy::activeConditionalMediationProxy()
{
if (m_paused)
static MainThreadNeverDestroyed<WeakPtr<WebAuthenticatorCoordinatorProxy>> proxy;
return proxy.get();
}

void WebAuthenticatorCoordinatorProxy::pauseConditionalAssertion(CompletionHandler<void()>&& completionHandler)
{
if (m_paused) {
completionHandler();
return;
}
m_paused = true;
if (m_isConditionalAssertion)
if (m_isConditionalAssertion) {
m_cancelHandler = WTFMove(completionHandler);
[m_controller cancel];
} else
completionHandler();
}

void WebAuthenticatorCoordinatorProxy::unpauseConditionalAssertion()
Expand All @@ -318,6 +329,19 @@ static inline ASAuthorizationPublicKeyCredentialLargeBlobSupportRequirement toAS
m_paused = false;
}

void WebAuthenticatorCoordinatorProxy::makeActiveConditionalAssertion()
{
if (auto& activeProxy = activeConditionalMediationProxy()) {
if (activeProxy == this)
return;
activeProxy->pauseConditionalAssertion([weakThis = WeakPtr { *this }] () {
if (!weakThis)
return;
weakThis->unpauseConditionalAssertion();
});
}
}

#endif // HAVE(WEB_AUTHN_AS_MODERN)

void WebAuthenticatorCoordinatorProxy::performRequest(WebAuthenticationRequestData &&requestData, RequestCompletionHandler &&handler)
Expand All @@ -339,12 +363,14 @@ static inline ASAuthorizationPublicKeyCredentialLargeBlobSupportRequirement toAS
handler(WebCore::AuthenticatorResponseData { }, AuthenticatorAttachment::Platform, { ExceptionCode::NotAllowedError, @"" });
return;
}
if (m_isConditionalAssertion)
activeConditionalMediationProxy() = *this;
m_controller = WTFMove(controller);
m_completionHandler = WTFMove(handler);
m_delegate = adoptNS([[_WKASDelegate alloc] initWithPage:WTFMove(requestData.page) completionHandler:makeBlockPtr([weakThis = WeakPtr { *this }, this](ASAuthorization *auth, NSError *error) mutable {
m_delegate = adoptNS([[_WKASDelegate alloc] initWithPage:WTFMove(requestData.page) completionHandler:makeBlockPtr([weakThis = WeakPtr { *this }](ASAuthorization *auth, NSError *error) mutable {
if (!weakThis)
return;
ensureOnMainRunLoop([weakThis = WTFMove(weakThis), this, auth = retainPtr(auth)]() {
ensureOnMainRunLoop([weakThis = WTFMove(weakThis), auth = retainPtr(auth)]() {
if (!weakThis)
return;
WebCore::AuthenticatorResponseData response = { };
Expand Down Expand Up @@ -387,11 +413,17 @@ static inline ASAuthorizationPublicKeyCredentialLargeBlobSupportRequirement toAS
response.userHandle = toArrayBuffer(credential.get().userID);
response.clientDataJSON = toArrayBuffer(credential.get().rawClientDataJSON);
}
if (!m_paused) {
m_completionHandler(response, attachment, exceptionData);
m_delegate.clear();
m_controller.clear();
m_isConditionalAssertion = false;
if (weakThis) {
if (activeConditionalMediationProxy() == weakThis)
activeConditionalMediationProxy() = nullptr;
if (auto cancelHandler = WTFMove(weakThis->m_cancelHandler))
cancelHandler();
if (!weakThis->m_paused) {
weakThis->m_completionHandler(response, attachment, exceptionData);
weakThis->m_delegate.clear();
weakThis->m_controller.clear();
weakThis->m_isConditionalAssertion = false;
}
}
});
}).get()]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,10 @@ class WebAuthenticatorCoordinatorProxy : public IPC::MessageReceiver {
~WebAuthenticatorCoordinatorProxy();

#if HAVE(WEB_AUTHN_AS_MODERN)
void pauseConditionalAssertion();
static WeakPtr<WebAuthenticatorCoordinatorProxy>& activeConditionalMediationProxy();
void pauseConditionalAssertion(CompletionHandler<void()>&&);
void unpauseConditionalAssertion();
void makeActiveConditionalAssertion();
#endif

private:
Expand Down Expand Up @@ -124,6 +126,7 @@ class WebAuthenticatorCoordinatorProxy : public IPC::MessageReceiver {

RetainPtr<ASCAuthorizationRemotePresenter> m_presenter;
RetainPtr<ASCAgentProxy> m_proxy;
CompletionHandler<void()> m_cancelHandler;
#endif // HAVE(UNIFIED_ASC_AUTH_UI)
};

Expand Down
4 changes: 1 addition & 3 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2726,9 +2726,7 @@ void WebPageProxy::dispatchActivityStateChange()
#if ENABLE(WEB_AUTHN) && HAVE(WEB_AUTHN_AS_MODERN)
if ((changed & ActivityState::WindowIsActive) && m_credentialsMessenger) {
if (pageClient().isViewWindowActive())
m_credentialsMessenger->unpauseConditionalAssertion();
else
m_credentialsMessenger->pauseConditionalAssertion();
m_credentialsMessenger->makeActiveConditionalAssertion();
}
#endif

Expand Down

0 comments on commit b627464

Please sign in to comment.