I don't understand how the signin flow is implemented. It seems that we make a url request to something like

chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/success.html?access_point=0&source=0?attemptToken=1491504688536

But this request gets blocked at https://cs.chromium.org/chromium/src/extensions/browser/extension_protocols.cc?q=ERR_BLOCKED_BY_CLIENT.&sq=package:chromium&dr=C&l=450

Sending to ewald for triage.

 Issue 705120  has been merged into this issue.

I'm unable to repro this on Mac Canary. Screencast attached.

Sky, back to you to confirm I'm doing the right thing. Is this only a Linux issue or something? What platforms are affected?

Deleted: chrome-signin.mov 4.6 MB

chrome-signin.mov 4.6 MB Download

Yeah, that's basically what I was doing. I can seemingly sometimes repro on Mac canary (again I'm testing on 59.0.3064.0). But after a bit it starts working.

Linux on the other hand seems to consistently repro, and this is both off 59.0.3064.0 and tip of tree.

Re-assigning to Mihai for when he gets back in office. Mihai - we need to make sure to address this. We can't let chrome://chrome-signin be broken on stable.

Marking "Linux" for platform for now, since that's where it's consistently repro'ing for Sky.

Broken signin seems like P1, if not higher, no?

I am able to repro this on Mac as well as on windows. So this is all desktop OS. Checked on 59.0.3067.0/59.0.3067.6

Up'ing the priority to P1 and adding in the other platforms. Note that chrome://chrome-signin is being replaced with chrome://welcome as the page that's shown during the first run experience in 57 (Mac/Linux) and 58 (Win), so this page is not as high-visibility/priority anymore.

That being said, it's still used as part of the chrome.identity extensions API flow, so it's still important for it to be working correctly. Mihai - PTAL when you're back in office.

I think this should be a stable blocker for M59.

At the end of the sign-in flow, Gaia needs to load a continue URL. That continue URL is chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/success.html?access_point=0&source=0?attemptToken=1491504688536 .

Something changed in the code that checks if this load is allowed and it looks like this load is no longer allowed. I need to understand what and why.

A bisect would be useful to understand what is blocking the request to load the continue URL chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/success.html?access_point=0&source=0?attemptToken=1491504688536

I am not aware of any changes that could affect this. Bisect might be best way to find out why and when it regressed.

ERR_BLOCKED_BY_CLIENT might be from here [1]. Also make sure gaia_auth extension is loaded. It is a component extension but only loaded with InlineLoginUI [2].

[1]: https://cs.chromium.org/chromium/src/extensions/browser/extension_protocols.cc?rcl=ed96115726663cd5ed367c807310400686dc0927&l=451

[2]: https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/signin/inline_login_ui.cc?rcl=cd675450f3ecf0afadb537c18cbe29aaed80164e&l=61

Adding extension owners for guidance (rdevlin.cronin@chromium.org and lazyboy@chromium.org)

I managed to validated the following:

1. The request is indeed blocked in https://cs.chromium.org/chromium/src/extensions/browser/extension_protocols.cc?rcl=ed96115726663cd5ed367c807310400686dc0927&l=451
This is triggered by the fact that https://cs.chromium.org/chromium/src/extensions/browser/extension_protocols.cc?type=cs&q=AllowExtensionResourceLoad+package:%5Echromium$&l=342 returns false:
* It reached the end of this function and returns after the comment "// No special exceptions for cross-process loading. Block the load."
* If I change this function to return true when extension id matches "mfffpogegjflfpflabcdkioaeobkgjik", then chrome://chrome-signin flow works fine.

2. I can see the GaiaAuthExtension is loaded, so something changed in the way the extension permissions are checked.


@rdevlin.cronin@, lazyboy@: Would you have any idea if anything changed in the way in which the behavior of AllowExtensionResourceLoad has changed? Are there any changes in the way we check what resources an extension may load?

+alexmos@chromium.org

I found the guilty CL that broke the chrome://chrome-signin flow: https://codereview.chromium.org/2768863003 (CL landed by alexmos@ on 2017-03-24)

The problem is that the new check in https://codereview.chromium.org/2768863003/diff/20001/extensions/browser/url_request_util.cc no longer allows to load the continue URL in the chrome signin extension environment (the continue URL is chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/success.html)

It looks like alexmos@chromium.org is OOO until 5-2, and we need a fix quickly for this bug that must be merged back to M59.

lazyboy@, rdevlin.cronin@: You were the reviewers for the CL https://codereview.chromium.org/2768863003 . My initial reaction to this bug would be to revert that CL as it is breaking the Chrome sign-in flow (still used for SAML accounts and other cases). Could you please advise on what to here?

Looks like Charlie, Nick, and Lucas are still in office; adding them for opinions.

My guess is the check that broke this was
extensions/browser/url_request_util.cc

  if (is_guest) {
    // An extension's resources should only be accessible to WebViews owned by
    // that extension.
    if (owner_extension != extension) {  // <<<< Fail here
      *allowed = false;
      return true;
    }

    *allowed = WebviewInfo::IsResourceWebviewAccessible(extension, partition_id,
                                                        resource_path);
    return true;
  }

The chrome://chrome-signin page has a webview to the gaia auth extension's success.html page.  success.html is correctly included in the extension's web_accessible_resources, but since it's being hosted in the chrome-signin page, it fails the check.

My recollection is that in the CL, we realized the old behavior was different, but didn't think this behavior was desirable.  So now the question is - is this something we should allow, or should signin change?  And in the meantime, how should we fix this?

My proposal would be to revert only a small part of Alex's CL - specifically the webview ownership check.  From cursory examination, it looks like the other bugs would still be fixed, and that would fix the signin problem.  But I'd need to double check that that would work (and an extra set of eyes would be great. :))

Re #17: I agree with your assessment, it's unfortunate that we may have to remove this security check for the signin page, but I think it's the best short-term solution.

Alternatively, we could also hardcode the check to explicitly allow just the signin case. Charlie, any thoughts?

At a quick glance, I agree with Devlin's idea as a short term plan to fix the regression, and then we should revisit what Signin is doing here.

Is sign-in loading a web page into a GuestView and then navigating it to an extension page?  That is very concerning to me-- GuestViews don't support cross-process navigations.  I hope that we aren't granting extension API bindings to the guest process!

The gaia_auth extension is down to just serving a success page. The page is empty. Navigating to this page is the signal of auth success. 

I filed issue 688565 to fully deprecate gaia_auth extension. But we need to settle a new landing page for auth before we could do that.

To see if this works, I have tried doing the hack proposed at #17 and #18. The CL that does contains this hack (and a bunch of logs) is here: https://codereview.chromium.org/2831183003/

Note that even with this in place, the extension is *not* allowed to load the success.html page.

From the logs, I get the following:
1. The hack in https://codereview.chromium.org/2831183003/diff/1/extensions/browser/url_request_util.cc, skips the first if and hits the code that does *allowed = WebviewInfo::IsResourceWebviewAccessible(...)
2. The extension does not have any "webview accessible" resource - |webview_info| is null at line https://cs.chromium.org/chromium/src/extensions/common/manifest_handlers/webview_info.cc?rcl=a7629595cadb7b48331e9ae449d715693ac8e385&l=75

Note that I have tried changing the manifest file for my extenstion (like in https://codereview.chromium.org/2831183003/diff/1/chrome/browser/resources/gaia_auth/manifest.json?context=10&column_width=80&tab_spaces=8 ), but that does not work either.

rdevlin.cronin@ / lfg@chromium.org: Could you guys patch in my CL and tell me what it is missing? Or maybe pick up this bug as I do not really know how these permissions are supposed to work?

Thank you.

rdevlin.cronin@ / lfg@chromium.org: Ping as this is a release blocking bug.

Sorry for the delay, I patched in your CL to take a look, and you are missing 2 things:

1. Add the webview to the trusted partition in chrome/browser/resources/inline_login/inline_login.html.
2. Whitelist the gaia extension in extensions/common/api/_manifest_features.json.

I uploaded a diff here from your CL: https://codereview.chromium.org/2838013003/

I added the exception suggested in comment #17 in CL https://codereview.chromium.org/2838013003/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8fdab62089307f5fe2307335dd1c71c4c6661867

commit 8fdab62089307f5fe2307335dd1c71c4c6661867
Author: msarda <msarda@chromium.org>
Date: Thu Apr 27 19:06:23 2017

Fix loading success.html at the end of chrome://chrome-signin flow

This CL lists success.html as an webview accessible resource for
the sign-in extension.

This CL adds an exception for sign-in extension when for loading
webview accessible resources.

BUG= 709117 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2831183003
Cr-Commit-Position: refs/heads/master@{#467739}

[modify] https://crrev.com/8fdab62089307f5fe2307335dd1c71c4c6661867/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
[modify] https://crrev.com/8fdab62089307f5fe2307335dd1c71c4c6661867/extensions/browser/url_request_util.cc

We need to merge this to M59.

Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Is there a better, longer-term fix that we want to pursue here? From reading the comments above, it sounds like the site isolation folks aren't 100% satisfied with our short-term solution here to un-break the sign-in page.

Should we file a follow-up bug for whatever the right long-term solution is?

I think the long-term solution is removing the need for the sign-in extension, right?

Yep, long term goal is to deprecate the extension, tracked in issue 688565.

This bug is fixed - the issue 688565 is tracking the sign-in deprecation issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89b5bfc4897cbc1efcbff39bd568386cc24f4da6

commit 89b5bfc4897cbc1efcbff39bd568386cc24f4da6
Author: Mihai Sardarescu <msarda@chromium.org>
Date: Tue May 02 13:39:49 2017

Fix loading success.html at the end of chrome://chrome-signin flow

This CL lists success.html as an webview accessible resource for
the sign-in extension.

This CL adds an exception for sign-in extension when for loading
webview accessible resources.

BUG= 709117 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2831183003
Cr-Commit-Position: refs/heads/master@{#467739}
(cherry picked from commit 8fdab62089307f5fe2307335dd1c71c4c6661867)

Review-Url: https://codereview.chromium.org/2857583002 .
Cr-Commit-Position: refs/branch-heads/3071@{#347}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/89b5bfc4897cbc1efcbff39bd568386cc24f4da6/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
[modify] https://crrev.com/89b5bfc4897cbc1efcbff39bd568386cc24f4da6/extensions/browser/url_request_util.cc

Able to sign in from the welcome page correctly on Win7/64 bit - 59.0.3071.36 

chrome://chrome-signin is still broken on mac on ToT.
When I navigate there, I'm allowed through email/password. After I hit enter on my password I get a dialog that reads:
Can't sign in
Service unavailble; try again later.
Help link leads here: https://support.google.com/chrome/answer/3097271?visit_id=0-636313383972226136-1672455844&rd=1.

Re #35: This seems like a different issue, can you file a new bug?