New issue
Advanced search Search tips

Issue 919170 link

Starred by 1 user
Status: Assigned
Owner:
peconn@chromium.org
Cc:
pjmclachlan@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Cached assetlinks.json can cause asset link verification to fail

Project Member Reported by pjmclachlan@google.com, Jan 4 
Chrome Version: 73.0.3660.2
OS: Android

What steps will reproduce the problem?
(1) Create a PWA with asset link JSON file in the correct location
(2) Place stale information in the assetlink.json (incorrect package name or fingerprint for example) 
(3) Launch a TWA in WPWA mode, configured to load the asset link file.  The TWA will correctly show the URL bar at the top.  
(4) Update the assetlink.json file to specify the correct package name & fingerprint.  
(5) Launch the TWA in WPWA mode again

What is the expected result?

The TWA should display in full screen mode with no URL bar. 

What happens instead?

The TWA continues to show the URL bar. Asset verification fails.  

If cache is cleared in Chrome, and the TWA is restarted, it switches to full screen as expected.

Comment 1 by chelamcherla@chromium.org, Jan 7

Labels: Needs-triage-Mobile

Comment 2 by chelamcherla@chromium.org, Jan 7

Labels: Triaged-Mobile Needs-Feedback
Status: Assigned (was: Unconfirmed)
@ pjmclachlan: Could you please provide test url to reproduce this issue from TE end.

Also changing status to Assigned as Owner is already available. Please feel free to change if this is not the case.

Thanks!

Comment 3 by pjmclachlan@google.com, Jan 8 

The URL I was using was https://petermclachlan.com but it won't help much for testing as you won't have access to modify assetlinks.json on my site. :/

For a TE to test the issue you'll need a PWA available from a web server set up somewhere you can modify .well-known/assetlinks.json.  

The TWA will need to be created in Android Studio and then the assetlinks updated to use the TWA SHA256 fingerprint obtained with `keytool -printcert -jarfile app-debug.apk`.  

Some details (TWA pre-release Google-only) on building a PWA here: https://docs.google.com/document/d/1kWuzWX4tyGZtEfztHQ3KcpOYIyAz7Ar4e3ONWud1QRA/edit#heading=h.5um847bx2m33

Comment 4 by peconn@chromium.org, Jan 11 

We cache the results of Digital Asset Link verification but that cache doesn't include the signature fingerprint of the client app, which I'm guessing is what has caused this problem.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 17 (5 days ago) 

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

commit 4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70
Author: Peter E Conn <peconn@chromium.org>
Date: Thu Jan 17 17:00:22 2019

🤝 Account for signature in verification result cache.

Previously we didn't cache the signature of the package with the
verification result - this caused bugs when someone is developing and
switching signatures around.

Additionally removed checking the cache from the main verification
logic, this resulted in verification never failing for an app that it
had previously succeeded with (originally this made sense when we had
two separate stores - one in Android Preferences and on in a static
variable, but is a bug now).

Bug: 919170
Change-Id: Ia98658cb9d2dc2721b7fef075b2538d65fbe17d5
Reviewed-on: https://chromium-review.googlesource.com/c/1407076
Commit-Queue: Peter Conn <peconn@chromium.org>
Reviewed-by: Pavel Shmakov <pshmakov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623723}
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentUtils.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/java/src/org/chromium/chrome/browser/browserservices/ManageTrustedWebActivityDataActivity.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/java/src/org/chromium/chrome/browser/browserservices/OriginVerifier.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/java/src/org/chromium/chrome/browser/browserservices/Relationship.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityVerifier.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/OriginVerifierTest.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/TrustedWebActivityTest.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/ClientManagerTest.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
[modify] https://crrev.com/4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/DetachedResourceRequestTest.java
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 17 (5 days ago) 

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

commit b8d0628b20379f8ff238864f18f20d6199503e32
Author: Shimi Zhang <ctzsm@chromium.org>
Date: Thu Jan 17 20:48:15 2019

Revert "🤝 Account for signature in verification result cache."

This reverts commit 4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70.

Reason for revert:

The downstream change could not easily land. I think this change is not only making compiling issue but has some logic change so the downstream tests will fail.

Original change's description:
> 🤝 Account for signature in verification result cache.
> 
> Previously we didn't cache the signature of the package with the
> verification result - this caused bugs when someone is developing and
> switching signatures around.
> 
> Additionally removed checking the cache from the main verification
> logic, this resulted in verification never failing for an app that it
> had previously succeeded with (originally this made sense when we had
> two separate stores - one in Android Preferences and on in a static
> variable, but is a bug now).
> 
> Bug: 919170
> Change-Id: Ia98658cb9d2dc2721b7fef075b2538d65fbe17d5
> Reviewed-on: https://chromium-review.googlesource.com/c/1407076
> Commit-Queue: Peter Conn <peconn@chromium.org>
> Reviewed-by: Pavel Shmakov <pshmakov@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#623723}

TBR=peconn@chromium.org,pshmakov@chromium.org

Change-Id: I4cd1f17109bceb0d6fe1549e15f4654106d15b5a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 919170
Reviewed-on: https://chromium-review.googlesource.com/c/1418437
Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623832}
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentUtils.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/java/src/org/chromium/chrome/browser/browserservices/ManageTrustedWebActivityDataActivity.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/java/src/org/chromium/chrome/browser/browserservices/OriginVerifier.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/java/src/org/chromium/chrome/browser/browserservices/Relationship.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityVerifier.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/OriginVerifierTest.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/TrustedWebActivityTest.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/ClientManagerTest.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
[modify] https://crrev.com/b8d0628b20379f8ff238864f18f20d6199503e32/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/DetachedResourceRequestTest.java
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 18 (4 days ago) 

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

commit fe20f3c962650b812e783c67c731b554ad11ed52
Author: Peter Conn <peconn@chromium.org>
Date: Fri Jan 18 12:51:49 2019

Reland "🤝 Account for signature in verification result cache."

This reverts commit b8d0628b20379f8ff238864f18f20d6199503e32.

Reason for revert: Fixed the bug

Original change's description:
> Revert "🤝 Account for signature in verification result cache."
> 
> This reverts commit 4b8dcb3f1486f3a35751d45ccf38d7d4c5dbbc70.
> 
> Reason for revert:
> 
> The downstream change could not easily land. I think this change is not only making compiling issue but has some logic change so the downstream tests will fail.
> 
> Original change's description:
> > 🤝 Account for signature in verification result cache.
> > 
> > Previously we didn't cache the signature of the package with the
> > verification result - this caused bugs when someone is developing and
> > switching signatures around.
> > 
> > Additionally removed checking the cache from the main verification
> > logic, this resulted in verification never failing for an app that it
> > had previously succeeded with (originally this made sense when we had
> > two separate stores - one in Android Preferences and on in a static
> > variable, but is a bug now).
> > 
> > Bug: 919170
> > Change-Id: Ia98658cb9d2dc2721b7fef075b2538d65fbe17d5
> > Reviewed-on: https://chromium-review.googlesource.com/c/1407076
> > Commit-Queue: Peter Conn <peconn@chromium.org>
> > Reviewed-by: Pavel Shmakov <pshmakov@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#623723}
> 
> TBR=peconn@chromium.org,pshmakov@chromium.org
> 
> Change-Id: I4cd1f17109bceb0d6fe1549e15f4654106d15b5a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 919170
> Reviewed-on: https://chromium-review.googlesource.com/c/1418437
> Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
> Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#623832}

TBR=peconn@chromium.org,ctzsm@chromium.org,pshmakov@chromium.org

Change-Id: I814350f575390c8ac6dddad156c8a0ba67b0b009
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 919170
Reviewed-on: https://chromium-review.googlesource.com/c/1421180
Reviewed-by: Peter Conn <peconn@chromium.org>
Commit-Queue: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624090}
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/java/src/org/chromium/chrome/browser/browserservices/BrowserSessionContentUtils.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/java/src/org/chromium/chrome/browser/browserservices/ManageTrustedWebActivityDataActivity.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/java/src/org/chromium/chrome/browser/browserservices/OriginVerifier.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/java/src/org/chromium/chrome/browser/browserservices/Relationship.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityVerifier.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/OriginVerifierTest.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/TrustedWebActivityTest.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/ClientManagerTest.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
[modify] https://crrev.com/fe20f3c962650b812e783c67c731b554ad11ed52/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/DetachedResourceRequestTest.java
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 18 (4 days ago) 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/40966039fdffec5c1febe2da86ae5e87c80186f7

commit 40966039fdffec5c1febe2da86ae5e87c80186f7
Author: Peter E Conn <peconn@google.com>
Date: Fri Jan 18 14:48:22 2019
►

Sign in to add a comment