New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Specify if clients are expected to follow redirects for icon URLs #1285
Comments
Both * Or by the user under the RP's supervision, unless the user modifies the client-side script. In either case, the RP controls the values for the purposes of this discussion. |
Ah, I'm sorry, I meant to ask about clients! #1139 is moderately relevant, but it would be useful to know if we (as an RP) can count on clients to do at least one redirect! |
For practical purposes, no user agent is going to actively fetch these icons, as they would be potent correlation mechanisms for resident credentials -- after so much effort is taken elsewhere in the specification to avoid correlation. Perhaps the right thing to do here is to amend the definition for these icons to specify that these are only valid if they are |
In that case, it would be nice to specify a more reasonable expectation than 128 bytes of storage for the icon. 128 bytes is reasonable for a URL, but squeezing icons into 128 |
My understanding of the correlation issue is that a client may cache the icon at the conclusion of authentication and registration, but a client will not resolve the icon before authentication using the network due to correlation concerns. I agree with @lgarron that very few relying parties will manage to fit a usable icon within the 85 bytes you can hypothetically squeeze out of a 128 character data URL. It is more likely they will use a regular URL and let clients deal with the UX of not having an icon. |
Sorry, this is not going to happen for hardware authenticators at least. Resident credentials already require a significant amount of storage space. |
The browser could cache it during make credential, and after authenticating. I am guessing that browsers may not be able to easily display only cached icons. |
Thats my concern. We also have two icons:
If desired, the RP icon would have a good shot of being changed to be widely usable. For example, we could reference an image source set via .well-known of the RP domain, and have clients fetch that without session state. Of course, a well-known lookup also technically no longer needs the URL field. We don't have infrastructure to use the user icon ad-hoc (such as an intermediate, anonymizing cache). So the user icon is only really displayable if previously cached by a registration or authentication action, loaded by user action, etc. We still would have the problem that not all RPs have a usable picture for this, or guidance on how it will be displayed. |
@jcjones Create PR |
I will make a PR to deprecate the image fields and any others that are unlikely to be utilized by platforms or authenticators. |
Still working on this! |
As discussed in issue w3c#1285, the image URL fields for PublicKeyCredentialEntity, while intended for user interface design, are potent correlation mechanisms if they are downloaded by RPs. RPs would have to take extraordinary care, beyond reasonable measures, to avoid uses by RPs with mal-intent to cross-correlate accounts. It is better for User Agents to use existing origin/icon mechanisms for their UX designs, or to define new such mechanisms as-needed, that are origin-wide rather than provide the possibility to embed detailed tracking information into these URLs.
Fix #1285 - Remove icons from PublicKeyCredentialEntity
As discussed in issue #1285, the image URL fields for PublicKeyCredentialEntity, while intended for user interface design, are potent correlation mechanisms if they are downloaded by RPs. RPs would have to take extraordinary care, beyond reasonable measures, to avoid uses by RPs with mal-intent to cross-correlate accounts. It is better for User Agents to use existing origin/icon mechanisms for their UX designs, or to define new such mechanisms as-needed, that are origin-wide rather than provide the possibility to embed detailed tracking information into these URLs.
Fix #1285 - Remove icons from PublicKeyCredentialEntity
https://bugs.webkit.org/show_bug.cgi?id=203346 <rdar://problem/56558488> Reviewed by Brent Fulgham. Source/WebCore: Covered by new tests within existing test files. * Modules/webauthn/AuthenticatorAssertionResponse.h: (WebCore::AuthenticatorAssertionResponse::setName): (WebCore::AuthenticatorAssertionResponse::name const): (WebCore::AuthenticatorAssertionResponse::setDisplayName): (WebCore::AuthenticatorAssertionResponse::displayName const): (WebCore::AuthenticatorAssertionResponse::setNumberOfCredentials): (WebCore::AuthenticatorAssertionResponse::numberOfCredentials const): Adds new members to store new fields of the response from the authenticator. Field "icon" is omitted given it could be used to track users according to w3c/webauthn#1285. * Modules/webauthn/fido/DeviceResponseConverter.cpp: (fido::readCTAPGetAssertionResponse): Adds new logic to parse above fields from an authenticator response. Source/WebKit: This patch implements authenticatorGetNextAssertion as suggested by the spec: https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#authenticatorGetNextAssertion The work flow is as follow: 1) When a valid assertion response is received, check its numberOfCredentials member; 2) When it is larger then 1, use authenticatorGetNextAssertion to get all remaining responses; 3) Once all responses are gathered, ask UI clients to pick one to return. * UIProcess/API/APIWebAuthenticationPanelClient.h: (API::WebAuthenticationPanelClient::selectAssertionResponses const): * UIProcess/WebAuthentication/Authenticator.h: * UIProcess/WebAuthentication/AuthenticatorManager.cpp: (WebKit::AuthenticatorManager::selectAssertionResponses): * UIProcess/WebAuthentication/AuthenticatorManager.h: * UIProcess/WebAuthentication/Mock/MockHidConnection.cpp: (WebKit::MockHidConnection::parseRequest): * UIProcess/WebAuthentication/fido/CtapAuthenticator.cpp: (WebKit::CtapAuthenticator::continueGetAssertionAfterResponseReceived): (WebKit::CtapAuthenticator::continueGetNextAssertionAfterResponseReceived): * UIProcess/WebAuthentication/fido/CtapAuthenticator.h: Tools: * TestWebKitAPI/Tests/WebCore/CtapResponseTest.cpp: (TestWebKitAPI::TEST): * TestWebKitAPI/Tests/WebCore/FidoTestData.h: Adds new test case for new logic in DeviceResponseConverter. LayoutTests: * http/wpt/webauthn/public-key-credential-get-failure-hid.https-expected.txt: * http/wpt/webauthn/public-key-credential-get-failure-hid.https.html: * http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt: * http/wpt/webauthn/public-key-credential-get-success-hid.https.html: * http/wpt/webauthn/resources/util.js: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254356 268f45cc-cd09-0410-ab3c-d52691b4dbfc
https://bugs.webkit.org/show_bug.cgi?id=203346 <rdar://problem/56558488> Reviewed by Brent Fulgham. Source/WebCore: Covered by new tests within existing test files. * Modules/webauthn/AuthenticatorAssertionResponse.h: (WebCore::AuthenticatorAssertionResponse::setName): (WebCore::AuthenticatorAssertionResponse::name const): (WebCore::AuthenticatorAssertionResponse::setDisplayName): (WebCore::AuthenticatorAssertionResponse::displayName const): (WebCore::AuthenticatorAssertionResponse::setNumberOfCredentials): (WebCore::AuthenticatorAssertionResponse::numberOfCredentials const): Adds new members to store new fields of the response from the authenticator. Field "icon" is omitted given it could be used to track users according to w3c/webauthn#1285. * Modules/webauthn/fido/DeviceResponseConverter.cpp: (fido::readCTAPGetAssertionResponse): Adds new logic to parse above fields from an authenticator response. Source/WebKit: This patch implements authenticatorGetNextAssertion as suggested by the spec: https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#authenticatorGetNextAssertion The work flow is as follow: 1) When a valid assertion response is received, check its numberOfCredentials member; 2) When it is larger then 1, use authenticatorGetNextAssertion to get all remaining responses; 3) Once all responses are gathered, ask UI clients to pick one to return. * UIProcess/API/APIWebAuthenticationPanelClient.h: (API::WebAuthenticationPanelClient::selectAssertionResponses const): * UIProcess/WebAuthentication/Authenticator.h: * UIProcess/WebAuthentication/AuthenticatorManager.cpp: (WebKit::AuthenticatorManager::selectAssertionResponses): * UIProcess/WebAuthentication/AuthenticatorManager.h: * UIProcess/WebAuthentication/Mock/MockHidConnection.cpp: (WebKit::MockHidConnection::parseRequest): * UIProcess/WebAuthentication/fido/CtapAuthenticator.cpp: (WebKit::CtapAuthenticator::continueGetAssertionAfterResponseReceived): (WebKit::CtapAuthenticator::continueGetNextAssertionAfterResponseReceived): * UIProcess/WebAuthentication/fido/CtapAuthenticator.h: Tools: * TestWebKitAPI/Tests/WebCore/CtapResponseTest.cpp: (TestWebKitAPI::TEST): * TestWebKitAPI/Tests/WebCore/FidoTestData.h: Adds new test case for new logic in DeviceResponseConverter. LayoutTests: * http/wpt/webauthn/public-key-credential-get-failure-hid.https-expected.txt: * http/wpt/webauthn/public-key-credential-get-failure-hid.https.html: * http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt: * http/wpt/webauthn/public-key-credential-get-success-hid.https.html: * http/wpt/webauthn/resources/util.js: Canonical link: https://commits.webkit.org/219195@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254356 268f45cc-cd09-0410-ab3c-d52691b4dbfc
At GitHub, we have "nice" profile icon URLs like https://github.com/lgarron.png
That URL redirects to https://avatars2.githubusercontent.com/u/248078?v=4 , which is moderately stable but could change some day. Could I ask for the spec to specify whether RPs are expected to follow the redirect from the more permanent URL?
I can't find information on this; #1139 is the closest I've found
The text was updated successfully, but these errors were encountered: