Credential Manager API should not be available inside <webview> |
|||
Issue descriptionVersion: 55, but present likely since introduction of CM API OS: cross-platform (personally tested on GNU/Linux) What steps will reproduce the problem? (0) Put some logging into CredentialManagerClient::dispatchGet. (1) Load the extension attached in http://crbug.com/644612#c3 to Chrome. (2) Open its background page from about:extensions. (3) For SOMEURL substitute with a URL of a page attempting to call navigator.credentials.get, type into DevTools console: chrome.identity.launchWebAuthFlow({url:"SOMEURL"},(url)=>{});” (At the time of filing this bug, SOMEURL=https://pinterest.com/ worked.) What is the expected output? The logging from step (0) should not be executed. What do you see instead? The logging is executed. More explanation: The call to chrome.identity.launchWebAuthFlow opens an app window containing a <webview>, which navigates to SOMEURL. Inside the <webview> Chrome's password manager is not available. This is on purpose, because it would not and should not be able to share the data from the main profile with the <webview>, and hence would be useless. Technically, this is because the WebContents serving the <webview> does not have tab helpers attached (one of them being a ChromePasswordManagerClient). Because of the unavailability of password manager, Chrome should not pretend that navigator.credentials is defined inside the <webview>. It should not be possible to call get() on it.
,
Sep 16 2016
Another thing to verify would be behavior in Chrome Custom Tabs. Does code in CredentialManagerClient::dispatchGet know that it's inside WebView? Mike, what would be the standard way to disable an API in Blink for such a case?
,
Sep 16 2016
I don't think we generally distinguish between webview and non-webview when exposing web platform APIs. That is, it doesn't seem obviously wrong to me to expose the API to web pages, however they're loaded, and simply not return credentials (because there's no client wired up). Does exposing this API cause problems?
,
Sep 17 2016
Thanks for both comments! Ad #3: The only problem I can see is with the API not meeting expectations of the website or user. In particular, (attempt at) saving the credential is doomed to fail (PasswordStore is not available). Perhaps we need a way to let store() calls fail? Ad #2: Checking Chrome Custom Tabs is a good point. My expectation is that they should work the same way as normal tabs (including the password manager being available).
,
Sep 19 2016
I don't see a problem in not storing the credential. User can always dismiss the prompt. We need to double check that the site gets an empty credential back. Vaclav, did you observe during testing that the callback is called?
,
Sep 22 2016
Perhaps I don't understand the contract of "store()", but how is it not a problem if the user sees "Do you want to store?", answers yes, and the next time nothing is there? As for the callback -- currently calling get() causes a crash, so no time for callback :). Once https://codereview.chromium.org/2344943002/ is landed, I can check that the empty callback is called.
,
Sep 22 2016
For WebView there is no omnibox and no bubble, right?
,
Sep 27 2016
Ah, good point. There is indeed not prompt. So the user is not confused, and the app needs to handle the case when the user does not agree to save. Thanks for the clarification, this is no longer a concern, IMO. |
|||
►
Sign in to add a comment |
|||
Comment 1 by vabr@chromium.org
, Sep 15 2016