Bug 203830 – [WebAuthn] Guard towards unexpected -[_WKWebAuthenticationPanel cancel]

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Jiewen Tan]

-[_WKWebAuthenticationPanel cancel] can be called by clients in many different scenarios other than to express a real user cancel. We should expect that as well.

[Jiewen Tan]

<rdar://problem/56797134>

[Jiewen Tan]

Created attachment 382785 [details]
Patch

[Brent Fulgham]

Comment on attachment 382785 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382785&action=review

R=me

> Source/WebKit/ChangeLog:3
> +        [WebAuthn] Guard towards unexpected -[_WKWebAuthenticationPanel cancel]

Guard against

> Source/WebKit/ChangeLog:9
> +        -[_WKWebAuthenticationPanel cancel] was only expected to be called on behave of an

On behalf of

> Source/WebKit/ChangeLog:11
> +        unexpected scenarios as well. We should guard ourselves towards that.

We should guard against that.

> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:182
> +    if (m_pendingRequestData.frameID) {

If (auto* pendingFrameID = m_pendingRequestData.frameID)

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:62
> +    // Call delegates in the next run loop to prevent clients' reentrance that would potentially modify the states

State should be singular in this use.

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:63
> +    // of current run loop in unexpected ways.

Of the current

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:94
> +    // of current run loop in unexpected ways.

Ditto for both prior comments.

[Jiewen Tan]

Comment on attachment 382785 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382785&action=review

Thanks Brent for r+ this patch.

>> Source/WebKit/ChangeLog:3
>> +        [WebAuthn] Guard towards unexpected -[_WKWebAuthenticationPanel cancel]
> 
> Guard against

Fixed.

>> Source/WebKit/ChangeLog:9
>> +        -[_WKWebAuthenticationPanel cancel] was only expected to be called on behave of an
> 
> On behalf of

Fixed.

>> Source/WebKit/ChangeLog:11
>> +        unexpected scenarios as well. We should guard ourselves towards that.
> 
> We should guard against that.

Fixed.

>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:182
>> +    if (m_pendingRequestData.frameID) {
> 
> If (auto* pendingFrameID = m_pendingRequestData.frameID)

Fixed.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:62
>> +    // Call delegates in the next run loop to prevent clients' reentrance that would potentially modify the states
> 
> State should be singular in this use.

Fixed.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:63
>> +    // of current run loop in unexpected ways.
> 
> Of the current

Fixed.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:94
>> +    // of current run loop in unexpected ways.
> 
> Ditto for both prior comments.

Fixed.

[Jiewen Tan]

Comment on attachment 382785 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382785&action=review

>>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:182
>>> +    if (m_pendingRequestData.frameID) {
>> 
>> If (auto* pendingFrameID = m_pendingRequestData.frameID)
> 
> Fixed.

m_pendingRequestData.frameID is of type Optional<>. So I modified it to auto pendingFrameID = m_pendingRequestData.frameID.

[Jiewen Tan]

Created attachment 382797 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 382797 [details]
Patch for landing

Clearing flags on attachment: 382797

Committed r252035: <https://trac.webkit.org/changeset/252035>

[Jiewen Tan]

*** Bug 204034 has been marked as a duplicate of this bug. ***