Commit 44c50eaa authored by Rayan Kanso's avatar Rayan Kanso Committed by Commit Bot

Display digitial verification warnings in DevTools console.

Pipe the WebContents to the DigitalAssetLinksHandler to add warning
messages to the DevTools console where applicable.

Bug: 910228
Change-Id: I711c2fe37df3686f00cdbbb6d7f6e1f947868b4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824462Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Reviewed-by: default avatarPeter Conn <peconn@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701100}
parent 18a29e3a
......@@ -33,6 +33,7 @@ import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.util.UrlConstants;
import org.chromium.content_public.browser.BrowserStartupController;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import org.chromium.content_public.browser.WebContents;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
......@@ -76,6 +77,8 @@ public class OriginVerifier {
@Nullable private OriginVerificationListener mListener;
private Origin mOrigin;
private long mVerificationStartTime;
@Nullable
private WebContents mWebContents;
/**
* A collection of Relationships (stored as Strings, with the signature set to an empty String)
......@@ -89,8 +92,9 @@ public class OriginVerifier {
@Inject
public Factory() {}
public OriginVerifier create(String packageName, @Relation int relation) {
return new OriginVerifier(packageName, relation);
public OriginVerifier create(
String packageName, @Relation int relation, @Nullable WebContents webContents) {
return new OriginVerifier(packageName, relation, webContents);
}
}
......@@ -205,11 +209,15 @@ public class OriginVerifier {
* Use {@link OriginVerifier#start}
* @param packageName The package for the Android application for verification.
* @param relation Digital Asset Links {@link Relation} to use during verification.
* @param webContents The web contents of the tab used for reporting errors to DevTools. Can be
* null if unavailable.
*/
public OriginVerifier(String packageName, @Relation int relation) {
public OriginVerifier(
String packageName, @Relation int relation, @Nullable WebContents webContents) {
mPackageName = packageName;
mSignatureFingerprint = getCertificateSHA256FingerprintForPackage(mPackageName);
mRelation = relation;
mWebContents = webContents;
}
/**
......@@ -257,8 +265,9 @@ public class OriginVerifier {
// Early return for testing without native.
return;
}
mNativeOriginVerifier = OriginVerifierJni.get().init(
OriginVerifier.this, Profile.getLastUsedProfile().getOriginalProfile());
if (mWebContents != null && mWebContents.isDestroyed()) mWebContents = null;
mNativeOriginVerifier = OriginVerifierJni.get().init(OriginVerifier.this, mWebContents,
Profile.getLastUsedProfile().getOriginalProfile());
assert mNativeOriginVerifier != 0;
String relationship = null;
switch (mRelation) {
......@@ -451,7 +460,7 @@ public class OriginVerifier {
@NativeMethods
interface Natives {
long init(OriginVerifier caller, Profile profile);
long init(OriginVerifier caller, @Nullable WebContents webContents, Profile profile);
boolean verifyOrigin(long nativeOriginVerifier, OriginVerifier caller, String packageName,
String signatureFingerprint, String origin, String relationship);
void destroy(long nativeOriginVerifier, OriginVerifier caller);
......
......@@ -34,6 +34,7 @@ import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabObserver;
import org.chromium.chrome.browser.tab.TabObserverRegistrar;
import org.chromium.content_public.browser.NavigationHandle;
import org.chromium.content_public.browser.WebContents;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
......@@ -53,7 +54,7 @@ import dagger.Lazy;
public class TrustedWebActivityVerifier implements NativeInitObserver, Destroyable,
SaveInstanceStateObserver {
/** The Digital Asset Link relationship used for Trusted Web Activities. */
private final static int RELATIONSHIP = CustomTabsService.RELATION_HANDLE_ALL_URLS;
private static final int RELATIONSHIP = CustomTabsService.RELATION_HANDLE_ALL_URLS;
/** Used in activity instance state */
private static final String KEY_CLIENT_PACKAGE = "twaClientPackageName";
......@@ -145,7 +146,10 @@ public class TrustedWebActivityVerifier implements NativeInitObserver, Destroyab
}
assert mClientPackageName != null;
mOriginVerifier = originVerifierFactory.create(mClientPackageName, RELATIONSHIP);
WebContents webContents =
tabProvider.getTab() != null ? tabProvider.getTab().getWebContents() : null;
mOriginVerifier =
originVerifierFactory.create(mClientPackageName, RELATIONSHIP, webContents);
tabObserverRegistrar.registerTabObserver(mVerifyOnPageLoadObserver);
tabProvider.addObserver(mVerifyOnTabSwitchObserver);
......
......@@ -477,7 +477,8 @@ class ClientManager {
}
};
params.originVerifier = new OriginVerifier(params.getPackageName(), relation);
params.originVerifier =
new OriginVerifier(params.getPackageName(), relation, /* webContents= */ null);
PostTask.runOrPostTask(UiThreadTaskTraits.DEFAULT,
() -> { params.originVerifier.start(listener, origin); });
if (relation == CustomTabsService.RELATION_HANDLE_ALL_URLS
......
......@@ -7,6 +7,8 @@ package org.chromium.chrome.browser.browserservices;
import android.net.Uri;
import android.support.test.filters.SmallTest;
import androidx.browser.customtabs.CustomTabsService;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
......@@ -28,6 +30,7 @@ import org.chromium.chrome.browser.preferences.privacy.BrowsingDataBridge;
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import org.chromium.content_public.browser.test.mock.MockWebContents;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import java.util.HashSet;
......@@ -36,8 +39,6 @@ import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import androidx.browser.customtabs.CustomTabsService;
/** Tests for OriginVerifier. */
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
......@@ -91,10 +92,10 @@ public class OriginVerifierTest {
mHttpsOrigin = new Origin("https://www.example.com");
mHttpOrigin = new Origin("http://www.android.com");
mHandleAllUrlsVerifier =
new OriginVerifier(PACKAGE_NAME, CustomTabsService.RELATION_HANDLE_ALL_URLS);
mUseAsOriginVerifier =
new OriginVerifier(PACKAGE_NAME, CustomTabsService.RELATION_USE_AS_ORIGIN);
mHandleAllUrlsVerifier = new OriginVerifier(
PACKAGE_NAME, CustomTabsService.RELATION_HANDLE_ALL_URLS, new MockWebContents());
mUseAsOriginVerifier = new OriginVerifier(
PACKAGE_NAME, CustomTabsService.RELATION_USE_AS_ORIGIN, /* webContents= */ null);
mVerificationResultSemaphore = new Semaphore(0);
}
......
......@@ -93,7 +93,8 @@ public class TrustedWebActivityVerifierTest {
public void setUp() {
MockitoAnnotations.initMocks(this);
when(mCustomTabsConnection.getClientPackageNameForSession(any())).thenReturn(PACKAGE_NAME);
when(mOriginVerifierFactory.create(any(), anyInt())).thenReturn(mOriginVerifier.mock);
when(mOriginVerifierFactory.create(any(), anyInt(), any()))
.thenReturn(mOriginVerifier.mock);
when(mTabProvider.getTab()).thenReturn(mTab);
when(mIntentDataProvider.getTrustedWebActivityAdditionalOrigins()).thenReturn(
Arrays.asList("https://www.origin2.com/"));
......
yusufo@chromium.org
lizeb@chromium.org
peconn@chromium.org
yusufo@chromium.org
......@@ -16,6 +16,7 @@
#include "chrome/browser/profiles/profile_manager.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "services/network/public/cpp/simple_url_loader.h"
using base::android::ConvertJavaStringToUTF16;
......@@ -30,12 +31,14 @@ int OriginVerifier::clear_browsing_data_call_count_for_tests_;
OriginVerifier::OriginVerifier(JNIEnv* env,
const JavaRef<jobject>& obj,
const JavaRef<jobject>& jweb_contents,
const JavaRef<jobject>& jprofile) {
jobject_.Reset(obj);
Profile* profile = ProfileAndroid::FromProfileAndroid(jprofile);
DCHECK(profile);
asset_link_handler_ =
std::make_unique<digital_asset_links::DigitalAssetLinksHandler>(
content::WebContents::FromJavaWebContents(jweb_contents),
content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess());
}
......@@ -98,11 +101,13 @@ int OriginVerifier::GetClearBrowsingDataCallCountForTesting() {
static jlong JNI_OriginVerifier_Init(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
const base::android::JavaParamRef<jobject>& jweb_contents,
const base::android::JavaParamRef<jobject>& jprofile) {
if (!g_browser_process)
return 0;
OriginVerifier* native_verifier = new OriginVerifier(env, obj, jprofile);
OriginVerifier* native_verifier =
new OriginVerifier(env, obj, jweb_contents, jprofile);
return reinterpret_cast<intptr_t>(native_verifier);
}
......
......@@ -13,7 +13,7 @@
namespace digital_asset_links {
enum class RelationshipCheckResult;
class DigitalAssetLinksHandler;
}
} // namespace digital_asset_links
namespace customtabs {
......@@ -22,6 +22,7 @@ class OriginVerifier {
public:
OriginVerifier(JNIEnv* env,
const base::android::JavaRef<jobject>& obj,
const base::android::JavaRef<jobject>& jweb_contents,
const base::android::JavaRef<jobject>& jprofile);
~OriginVerifier();
......
......@@ -3,7 +3,7 @@
include_rules = [
"-chrome",
"-content",
"+content/public/test",
"+content/public",
"+chrome/browser/android/digital_asset_links",
"+net",
]
......@@ -12,6 +12,7 @@
#include "base/logging.h"
#include "base/strings/stringprintf.h"
#include "base/values.h"
#include "content/public/browser/web_contents.h"
#include "net/base/load_flags.h"
#include "net/base/url_util.h"
#include "net/http/http_response_headers.h"
......@@ -102,6 +103,19 @@ bool StatementHasMatchingFingerprint(const base::Value& statement,
return false;
}
// Shows a warning message in the DevTools console.
void AddMessageToConsole(content::WebContents* web_contents,
const std::string& message) {
if (web_contents) {
web_contents->GetMainFrame()->AddMessageToConsole(
blink::mojom::ConsoleMessageLevel::kWarning, message);
return;
}
// Fallback to LOG.
LOG(WARNING) << message;
}
} // namespace
namespace digital_asset_links {
......@@ -109,8 +123,10 @@ namespace digital_asset_links {
const char kDigitalAssetLinksCheckResponseKeyLinked[] = "linked";
DigitalAssetLinksHandler::DigitalAssetLinksHandler(
content::WebContents* web_contents,
scoped_refptr<network::SharedURLLoaderFactory> factory)
: shared_url_loader_factory_(std::move(factory)) {}
: content::WebContentsObserver(web_contents),
shared_url_loader_factory_(std::move(factory)) {}
DigitalAssetLinksHandler::~DigitalAssetLinksHandler() = default;
......@@ -127,13 +143,17 @@ void DigitalAssetLinksHandler::OnURLLoadComplete(
int net_error = url_loader_->NetError();
if (net_error == net::ERR_INTERNET_DISCONNECTED ||
net_error == net::ERR_NAME_NOT_RESOLVED) {
LOG(WARNING) << "Digital Asset Links connection failed.";
AddMessageToConsole(web_contents(),
"Digital Asset Links connection failed.");
std::move(callback_).Run(RelationshipCheckResult::NO_CONNECTION);
return;
}
LOG(WARNING) << base::StringPrintf(
"Digital Asset Links endpoint responded with code %d.", response_code);
AddMessageToConsole(
web_contents(),
base::StringPrintf(
"Digital Asset Links endpoint responded with code %d.",
response_code));
std::move(callback_).Run(RelationshipCheckResult::FAILURE);
return;
}
......@@ -157,7 +177,7 @@ void DigitalAssetLinksHandler::OnJSONParseSucceeded(
base::Value statement_list) {
if (!statement_list.is_list()) {
std::move(callback_).Run(RelationshipCheckResult::FAILURE);
LOG(WARNING) << "Statement List is not a list.";
AddMessageToConsole(web_contents(), "Statement List is not a list.");
return;
}
......@@ -190,17 +210,17 @@ void DigitalAssetLinksHandler::OnJSONParseSucceeded(
}
for (const auto& failure_reason : failures)
LOG(WARNING) << failure_reason;
AddMessageToConsole(web_contents(), failure_reason);
std::move(callback_).Run(RelationshipCheckResult::FAILURE);
}
void DigitalAssetLinksHandler::OnJSONParseFailed(
const std::string& error_message) {
LOG(WARNING)
<< base::StringPrintf(
"Digital Asset Links response parsing failed with message:")
<< error_message;
AddMessageToConsole(
web_contents(),
"Digital Asset Links response parsing failed with message: " +
error_message);
std::move(callback_).Run(RelationshipCheckResult::FAILURE);
}
......
......@@ -8,6 +8,7 @@
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "base/values.h"
#include "content/public/browser/web_contents_observer.h"
namespace network {
class SharedURLLoaderFactory;
......@@ -34,11 +35,13 @@ using RelationshipCheckResultCallback =
// for details of usage and APIs. These APIs are used to verify declared
// relationships between different asset types like web domains or Android apps.
// The lifecycle of this handler will be governed by the owner.
class DigitalAssetLinksHandler {
// The WebContents are used for logging console messages.
class DigitalAssetLinksHandler : public content::WebContentsObserver {
public:
explicit DigitalAssetLinksHandler(
DigitalAssetLinksHandler(
content::WebContents* web_contents,
scoped_refptr<network::SharedURLLoaderFactory> factory);
~DigitalAssetLinksHandler();
~DigitalAssetLinksHandler() override;
// Checks whether the given "relationship" has been declared by the target
// |web_domain| for the source Android app which is uniquely defined by the
......
......@@ -121,7 +121,8 @@ class DigitalAssetLinksHandlerTest : public ::testing::Test {
} // namespace
TEST_F(DigitalAssetLinksHandlerTest, CorrectAssetLinksUrl) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -133,7 +134,8 @@ TEST_F(DigitalAssetLinksHandlerTest, CorrectAssetLinksUrl) {
}
TEST_F(DigitalAssetLinksHandlerTest, PositiveResponse) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -145,7 +147,8 @@ TEST_F(DigitalAssetLinksHandlerTest, PositiveResponse) {
}
TEST_F(DigitalAssetLinksHandlerTest, PackageMismatch) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -157,7 +160,8 @@ TEST_F(DigitalAssetLinksHandlerTest, PackageMismatch) {
}
TEST_F(DigitalAssetLinksHandlerTest, SignatureMismatch) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -169,7 +173,8 @@ TEST_F(DigitalAssetLinksHandlerTest, SignatureMismatch) {
}
TEST_F(DigitalAssetLinksHandlerTest, RelationshipMismatch) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -182,7 +187,8 @@ TEST_F(DigitalAssetLinksHandlerTest, RelationshipMismatch) {
TEST_F(DigitalAssetLinksHandlerTest, StatementIsolation) {
// Ensure we don't merge separate statements together.
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -194,7 +200,8 @@ TEST_F(DigitalAssetLinksHandlerTest, StatementIsolation) {
}
TEST_F(DigitalAssetLinksHandlerTest, BadAssetLinks_Empty) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -206,7 +213,8 @@ TEST_F(DigitalAssetLinksHandlerTest, BadAssetLinks_Empty) {
}
TEST_F(DigitalAssetLinksHandlerTest, BadAssetLinks_NotList) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -218,7 +226,8 @@ TEST_F(DigitalAssetLinksHandlerTest, BadAssetLinks_NotList) {
}
TEST_F(DigitalAssetLinksHandlerTest, BadAssetLinks_StatementNotDict) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -230,7 +239,8 @@ TEST_F(DigitalAssetLinksHandlerTest, BadAssetLinks_StatementNotDict) {
}
TEST_F(DigitalAssetLinksHandlerTest, BadAssetLinks_MissingFields) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -242,7 +252,8 @@ TEST_F(DigitalAssetLinksHandlerTest, BadAssetLinks_MissingFields) {
}
TEST_F(DigitalAssetLinksHandlerTest, BadRequest) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -254,7 +265,8 @@ TEST_F(DigitalAssetLinksHandlerTest, BadRequest) {
}
TEST_F(DigitalAssetLinksHandlerTest, NetworkError) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......@@ -266,7 +278,8 @@ TEST_F(DigitalAssetLinksHandlerTest, NetworkError) {
}
TEST_F(DigitalAssetLinksHandlerTest, NetworkDisconnected) {
DigitalAssetLinksHandler handler(GetSharedURLLoaderFactory());
DigitalAssetLinksHandler handler(/* web_contents= */ nullptr,
GetSharedURLLoaderFactory());
handler.CheckDigitalAssetLinkRelationship(
base::BindOnce(&DigitalAssetLinksHandlerTest::OnRelationshipCheckComplete,
base::Unretained(this)),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment