Bug 232390 - [WebAuthn] +[_WKWebAuthenticationPanel getAllLocalAuthenticatorCredentials] should include kSecAttrCreationDate and kSecAttrModificationDate in the data dictionaries
Summary: [WebAuthn] +[_WKWebAuthenticationPanel getAllLocalAuthenticatorCredentials] s...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P3 Normal
Assignee: pascoe@apple.com
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-10-27 10:59 PDT by pascoe@apple.com
Modified: 2021-10-29 14:48 PDT (History)
2 users (show)

See Also:


Attachments
Patch (6.66 KB, patch)
2021-10-27 11:12 PDT, pascoe@apple.com
no flags Details | Formatted Diff | Diff
Patch (6.67 KB, patch)
2021-10-27 11:17 PDT, pascoe@apple.com
no flags Details | Formatted Diff | Diff
Patch (6.49 KB, patch)
2021-10-27 17:23 PDT, pascoe@apple.com
no flags Details | Formatted Diff | Diff
Patch (6.40 KB, patch)
2021-10-27 17:28 PDT, pascoe@apple.com
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description pascoe@apple.com 2021-10-27 10:59:22 PDT
<rdar://problem/84689277>
Comment 1 pascoe@apple.com 2021-10-27 11:12:06 PDT
Created attachment 442610 [details]
Patch
Comment 2 pascoe@apple.com 2021-10-27 11:17:52 PDT
Created attachment 442612 [details]
Patch
Comment 3 Darin Adler 2021-10-27 15:00:30 PDT
Comment on attachment 442612 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:256
> +            _WKLocalAuthenticatorCredentialIDKey : attributes[bridge_id_cast(kSecAttrApplicationLabel)],
> +            _WKLocalAuthenticatorCredentialRelyingPartyIDKey : attributes[bridge_id_cast(kSecAttrLabel)],
> +            _WKLocalAuthenticatorCredentialLastModificationDateKey : attributes[bridge_id_cast(kSecAttrModificationDate)],
> +            _WKLocalAuthenticatorCredentialCreationDateKey : attributes[bridge_id_cast(kSecAttrCreationDate)]

bridge_cast also works here, and I suggest always using bridge_cast unless you must use bridge_id cast.

Also, there’s a formatting change here where you are added spaces before the colons and before the "{", and I am not sure why.
Comment 4 pascoe@apple.com 2021-10-27 15:04:57 PDT
(In reply to Darin Adler from comment #3)
> Comment on attachment 442612 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=442612&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:256
> > +            _WKLocalAuthenticatorCredentialIDKey : attributes[bridge_id_cast(kSecAttrApplicationLabel)],
> > +            _WKLocalAuthenticatorCredentialRelyingPartyIDKey : attributes[bridge_id_cast(kSecAttrLabel)],
> > +            _WKLocalAuthenticatorCredentialLastModificationDateKey : attributes[bridge_id_cast(kSecAttrModificationDate)],
> > +            _WKLocalAuthenticatorCredentialCreationDateKey : attributes[bridge_id_cast(kSecAttrCreationDate)]
> 
> bridge_cast also works here, and I suggest always using bridge_cast unless
> you must use bridge_id cast.
> 
> Also, there’s a formatting change here where you are added spaces before the
> colons and before the "{", and I am not sure why.

I tried running "webkit-patch format" for the first time on this. Noticed how it changed the formatting here: https://bugs.webkit.org/attachment.cgi?id=442610&action=diff#a/Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.h_sec1 and undid that, but didn't catch the difference here.
Comment 5 pascoe@apple.com 2021-10-27 17:23:15 PDT
Created attachment 442657 [details]
Patch
Comment 6 pascoe@apple.com 2021-10-27 17:25:24 PDT
Also, thank you for the review!
Comment 7 Darin Adler 2021-10-27 17:25:55 PDT
Comment on attachment 442657 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:251
> +        [result addObject:@ {

We normally don’t put a space after the "@" and before the "{".
Comment 8 pascoe@apple.com 2021-10-27 17:28:08 PDT
Created attachment 442658 [details]
Patch
Comment 9 EWS 2021-10-29 14:48:23 PDT
Committed r285056 (243699@main): <https://commits.webkit.org/243699@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442658 [details].