New issue
Advanced search Search tips

Issue 670465 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Dec 2016
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

User authentication via Chrome (AppAuth) doesn't always work

Project Member Reported by yusukes@chromium.org, Dec 1 2016

Issue description

This is for b/33208965.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 2 2016

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

commit 0c7e06f5b85f0304bd62f61a9b2ae391e97f3183
Author: yusukes <yusukes@chromium.org>
Date: Fri Dec 02 21:12:31 2016

Allow an external URL with FROM_API qualifier to be forwarded to ARC

for better AppAuth compatibility.

In a highly unusual situation, allowing FROM_API could trigger
infinite Chrome tab creation. This CL also adds code for detecting
and stopping the loop, just in case. End users will almost never
see the safety checks to kick in, though.

BUG= 670465 
TEST=unit_tests passed, also manually confirmed that when
 |always_ask_user| is true, "Always" preference set by
 the user is ignored.

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

[modify] https://crrev.com/0c7e06f5b85f0304bd62f61a9b2ae391e97f3183/chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.cc
[modify] https://crrev.com/0c7e06f5b85f0304bd62f61a9b2ae391e97f3183/chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.h
[modify] https://crrev.com/0c7e06f5b85f0304bd62f61a9b2ae391e97f3183/chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog_unittest.cc
[modify] https://crrev.com/0c7e06f5b85f0304bd62f61a9b2ae391e97f3183/components/arc/intent_helper/page_transition_util.cc
[modify] https://crrev.com/0c7e06f5b85f0304bd62f61a9b2ae391e97f3183/components/arc/intent_helper/page_transition_util.h
[modify] https://crrev.com/0c7e06f5b85f0304bd62f61a9b2ae391e97f3183/components/arc/intent_helper/page_transition_util_unittest.cc

(Note to self: Chrome OS 9045.0.0 has the CL.)

Labels: -Pri-2 M-56 Pri-1
Changing priority per b/33208965#comment9

Labels: Merge-Request-56
+Merge-Request

The patch above has been live on Chrome OS canary channel for a few days now. Since the patch only affects ARC-enabled Chrome OS, the risk should be low.


Comment 5 by dimu@chromium.org, Dec 6 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 6 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f5d5e34508dfe64379c170ad1e3236b20c685b97

commit f5d5e34508dfe64379c170ad1e3236b20c685b97
Author: Yusuke Sato <yusukes@google.com>
Date: Tue Dec 06 21:26:02 2016

Allow an external URL with FROM_API qualifier to be forwarded to ARC

for better AppAuth compatibility.

In a highly unusual situation, allowing FROM_API could trigger
infinite Chrome tab creation. This CL also adds code for detecting
and stopping the loop, just in case. End users will almost never
see the safety checks to kick in, though.

BUG= 670465 
TEST=unit_tests passed, also manually confirmed that when
 |always_ask_user| is true, "Always" preference set by
 the user is ignored.

Review-Url: https://codereview.chromium.org/2540013004
Cr-Commit-Position: refs/heads/master@{#436017}
(cherry picked from commit 0c7e06f5b85f0304bd62f61a9b2ae391e97f3183)

Review URL: https://codereview.chromium.org/2556903002 .

Cr-Commit-Position: refs/branch-heads/2924@{#364}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/f5d5e34508dfe64379c170ad1e3236b20c685b97/chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.cc
[modify] https://crrev.com/f5d5e34508dfe64379c170ad1e3236b20c685b97/chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.h
[modify] https://crrev.com/f5d5e34508dfe64379c170ad1e3236b20c685b97/chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog_unittest.cc
[modify] https://crrev.com/f5d5e34508dfe64379c170ad1e3236b20c685b97/components/arc/intent_helper/page_transition_util.cc
[modify] https://crrev.com/f5d5e34508dfe64379c170ad1e3236b20c685b97/components/arc/intent_helper/page_transition_util.h
[modify] https://crrev.com/f5d5e34508dfe64379c170ad1e3236b20c685b97/components/arc/intent_helper/page_transition_util_unittest.cc

Status: Fixed (was: Started)
(Note: Chrome OS 9000.21.0 (M56) has the fix.)


Status: Verified (was: Fixed)

Sign in to add a comment