Bug 217401 – [WebAuthn] Implement a dummy WebAuthenticationAgent Part 1

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

[Jiewen Tan]

Implement a dummy WebAuthenticationAgent.

[Radar WebKit Bug Importer]

<rdar://problem/70012011>

[Jiewen Tan]

Created attachment 410685 [details]
Part 1

[Darin Adler]

Comment on attachment 410685 [details]
Part 1

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

> Source/WebKit/ChangeLog:12
> +        from the UI Process such that we can isolated high privileged entitlements to this standalone daemon and therefore are able

isolate

> Source/WebKit/ChangeLog:13
> +        to offer WebAuthn to third party WKWebView clients. One of the future feautre will require this new process to listen to

“to offer” -> “offer”

“features”

> Source/WebKit/ChangeLog:14
> +        LaunchEvents, which only daemons can do. That's why it is implemented as a user agent instead of a xpc service.

XPC

> Source/WebKit/ChangeLog:16
> +        This is the first part to establish such a dummy daemon. What it does is to setup a new build target for the daemon and

“First part to establish” -> “first part of establishing” or “first step on the path of establishing”

“setup”-> “set up”

> Source/WebKit/ChangeLog:34
> +        Not sure why libWTF.a is needed. Will fix that in the later part.

My guess is that WTFLogAlways is the reason.

> Source/WebKit/Daemons/WebAuthenticationAgent/Info.plist:6
> +	<string>${BUNDLE_VERSION}, Copyright 2003-2020 Apple Inc.</string>

Not sure this needs a copyright going back to 2003

> Source/WebKit/Daemons/WebAuthenticationAgent/Info.plist:22
> +	<string>????</string>

Is this correct?

> Source/WebKit/Daemons/WebAuthenticationAgent/com.apple.webkit.WebAuthenticationAgent.plist:17
> +	<string>/Users/jwtan/Documents/Build/Products/Debug/com.apple.WebKit.WebAuthenticationAgent.Development</string>

Change log does mention this, but it’s not great!

> Source/WebKit/Daemons/WebAuthenticationAgent/main.mm:31
> +        // FIXME: Implement it.

Don’t need this. We won’t forget.

[Jiewen Tan]

Comment on attachment 410685 [details]
Part 1

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

Thanks Darin for r+ this patch.

>> Source/WebKit/ChangeLog:12
>> +        from the UI Process such that we can isolated high privileged entitlements to this standalone daemon and therefore are able
> 
> isolate

Fixed.

>> Source/WebKit/ChangeLog:13
>> +        to offer WebAuthn to third party WKWebView clients. One of the future feautre will require this new process to listen to
> 
> “to offer” -> “offer”
> 
> “features”

Fixed.

>> Source/WebKit/ChangeLog:14
>> +        LaunchEvents, which only daemons can do. That's why it is implemented as a user agent instead of a xpc service.
> 
> XPC

Fixed.

>> Source/WebKit/ChangeLog:16
>> +        This is the first part to establish such a dummy daemon. What it does is to setup a new build target for the daemon and
> 
> “First part to establish” -> “first part of establishing” or “first step on the path of establishing”
> 
> “setup”-> “set up”

Fixed.

>> Source/WebKit/ChangeLog:34
>> +        Not sure why libWTF.a is needed. Will fix that in the later part.
> 
> My guess is that WTFLogAlways is the reason.

Right. I tried to only link WebKit.framework like the GPU process does but it doesn't work. Anyway, I will figure that out.

>> Source/WebKit/Daemons/WebAuthenticationAgent/Info.plist:6
>> +	<string>${BUNDLE_VERSION}, Copyright 2003-2020 Apple Inc.</string>
> 
> Not sure this needs a copyright going back to 2003

I copied it from the GPU process which is just created... I'm not sure as well.

>> Source/WebKit/Daemons/WebAuthenticationAgent/Info.plist:22
>> +	<string>????</string>
> 
> Is this correct?

I copied it from the GPU process...

>> Source/WebKit/Daemons/WebAuthenticationAgent/com.apple.webkit.WebAuthenticationAgent.plist:17
>> +	<string>/Users/jwtan/Documents/Build/Products/Debug/com.apple.WebKit.WebAuthenticationAgent.Development</string>
> 
> Change log does mention this, but it’s not great!

Sure! Will fix that!

>> Source/WebKit/Daemons/WebAuthenticationAgent/main.mm:31
>> +        // FIXME: Implement it.
> 
> Don’t need this. We won’t forget.

Fixed.

[Jiewen Tan]

Comment on attachment 410685 [details]
Part 1

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

>>> Source/WebKit/Daemons/WebAuthenticationAgent/Info.plist:22
>>> +	<string>????</string>
>> 
>> Is this correct?
> 
> I copied it from the GPU process...

It's the default value: https://stackoverflow.com/questions/1875912/naming-convention-for-cfbundlesignature-and-cfbundleidentifier.

[Jiewen Tan]

Created attachment 410788 [details]
Part 1 for landing

[EWS]

Committed r268155: <https://trac.webkit.org/changeset/268155>

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

[Jiewen Tan]

Reopened for Part 2.

[Hector Lopez]

Reverted r268155 for reason:

This reverts r268155 becasue it broke internal builds

Committed r268217: <https://trac.webkit.org/changeset/268217>

[Jiewen Tan]

(In reply to Hector Lopez from comment #9)
> Reverted r268155 for reason:
> 
> This reverts r268155 becasue it broke internal builds
> 
> Committed r268217: <https://trac.webkit.org/changeset/268217>

Foundation.framework is needed.

[Jiewen Tan]

Created attachment 410909 [details]
Part 1 for landing

[EWS]

Committed r268248: <https://trac.webkit.org/changeset/268248>

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

[Jiewen Tan]

Reopened for Part 2.

[Darin Adler]

(In reply to Jiewen Tan from comment #13)
> Reopened for Part 2.

That seems like a bad idea. Why use the same bug for two patches?

[Jiewen Tan]

(In reply to Darin Adler from comment #14)
> (In reply to Jiewen Tan from comment #13)
> > Reopened for Part 2.
> 
> That seems like a bad idea. Why use the same bug for two patches?

Given they are all building the same dummy agent? Let me open up a new bug.