Commit c92a89d5 authored by Andrey Zaytsev's avatar Andrey Zaytsev Committed by Commit Bot

Safety check on Android: added password check methods to the Java-C++ bridge

Bug: 1070620
Change-Id: I7262adc376491acdf85f0f97d9bf7e8b4eecb14c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2250071
Auto-Submit: Andrey Zaytsev <andzaytsev@google.com>
Commit-Queue: Martin Šrámek <msramek@chromium.org>
Reviewed-by: default avatarMartin Šrámek <msramek@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780461}
parent 6422dbc4
...@@ -39,7 +39,10 @@ android_library("java") { ...@@ -39,7 +39,10 @@ android_library("java") {
"//ui/android:ui_full_java", "//ui/android:ui_full_java",
] ]
annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ] annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
srcjar_deps = [ ":safety_check_enums" ] srcjar_deps = [
":bulk_leak_check_service_enums",
":safety_check_enums",
]
} }
android_library("junit") { android_library("junit") {
...@@ -76,3 +79,7 @@ android_resources("java_resources") { ...@@ -76,3 +79,7 @@ android_resources("java_resources") {
java_cpp_enum("safety_check_enums") { java_cpp_enum("safety_check_enums") {
sources = [ "//components/safety_check/safety_check.h" ] sources = [ "//components/safety_check/safety_check.h" ]
} }
java_cpp_enum("bulk_leak_check_service_enums") {
sources = [ "//components/password_manager/core/browser/bulk_leak_check_service_interface.h" ]
}
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
package org.chromium.chrome.browser.safety_check; package org.chromium.chrome.browser.safety_check;
import org.chromium.chrome.browser.password_check.BulkLeakCheckServiceState;
import org.chromium.chrome.browser.safety_check.SafetyCheckBridge.SafetyCheckCommonObserver; import org.chromium.chrome.browser.safety_check.SafetyCheckBridge.SafetyCheckCommonObserver;
/** /**
...@@ -17,17 +18,39 @@ public class SafetyCheck implements SafetyCheckCommonObserver { ...@@ -17,17 +18,39 @@ public class SafetyCheck implements SafetyCheckCommonObserver {
} }
/** /**
* Triggers a Safe Browsing status check. * Triggers all safety check child checks.
*/ */
public void checkSafeBrowsing() { public void performSafetyCheck() {
mSafetyCheckBridge.checkSafeBrowsing(); mSafetyCheckBridge.checkSafeBrowsing();
mSafetyCheckBridge.checkPasswords();
} }
/** /**
* Gets invoked once the Safe Browsing check is completed. * Gets invoked once the Safe Browsing check is completed.
* @param status SafetyCheck::SafeBrowsingStatus enum value representing the Safe Browsing *
* state (see //components/safety_check/safety_check.h). * @param status SafetyCheck::SafeBrowsingStatus enum value representing the
* Safe Browsing state (see
* //components/safety_check/safety_check.h).
*/ */
@Override @Override
public void onSafeBrowsingCheckResult(@SafeBrowsingStatus int status) {} public void onSafeBrowsingCheckResult(@SafeBrowsingStatus int status) {}
/**
* Gets invoked by the C++ code every time another credential is checked.
*
* @param checked Number of passwords already checked.
* @param total Total number of passwords to check.
*/
@Override
public void onPasswordCheckCredentialDone(int checked, int total) {}
/**
* Gets invoked by the C++ code when the status of the password check changes.
*
* @param state BulkLeakCheckService::State enum value representing the state
* (see
* //components/password_manager/core/browser/bulk_leak_check_service_interface.h).
*/
@Override
public void onPasswordCheckStateChange(@BulkLeakCheckServiceState int state) {}
} }
...@@ -6,9 +6,11 @@ package org.chromium.chrome.browser.safety_check; ...@@ -6,9 +6,11 @@ package org.chromium.chrome.browser.safety_check;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.browser.password_check.BulkLeakCheckServiceState;
/** /**
* Provides access to the C++ multi-platform Safety check code in //components/safety_check. * Provides access to the C++ multi-platform Safety check code in
* //components/safety_check.
*/ */
public class SafetyCheckBridge { public class SafetyCheckBridge {
/** /**
...@@ -17,11 +19,32 @@ public class SafetyCheckBridge { ...@@ -17,11 +19,32 @@ public class SafetyCheckBridge {
public interface SafetyCheckCommonObserver { public interface SafetyCheckCommonObserver {
/** /**
* Gets invoked by the C++ code once the Safe Browsing check has results. * Gets invoked by the C++ code once the Safe Browsing check has results.
* @param status SafetyCheck::SafeBrowsingStatus enum value representing the Safe Browsing *
* state (see //components/safety_check/safety_check.h). * @param status SafetyCheck::SafeBrowsingStatus enum value representing the
* Safe Browsing state (see
* //components/safety_check/safety_check.h).
*/ */
@CalledByNative("SafetyCheckCommonObserver") @CalledByNative("SafetyCheckCommonObserver")
void onSafeBrowsingCheckResult(@SafeBrowsingStatus int status); void onSafeBrowsingCheckResult(@SafeBrowsingStatus int status);
/**
* Gets invoked by the C++ code every time another credential is checked.
*
* @param checked Number of passwords already checked.
* @param total Total number of passwords to check.
*/
@CalledByNative("SafetyCheckCommonObserver")
void onPasswordCheckCredentialDone(int checked, int total);
/**
* Gets invoked by the C++ code when the status of the password check changes.
*
* @param state BulkLeakCheckService::State enum value representing the state
* (see //components/password_manager/core/browser
* /bulk_leak_check_service_interface.h).
*/
@CalledByNative("SafetyCheckCommonObserver")
void onPasswordCheckStateChange(@BulkLeakCheckServiceState int state);
} }
/** /**
...@@ -31,9 +54,9 @@ public class SafetyCheckBridge { ...@@ -31,9 +54,9 @@ public class SafetyCheckBridge {
/** /**
* Initializes the C++ side. * Initializes the C++ side.
* @param safetyCheckCommonObserver an observer instance that will receive the result of the *
* check using * @param safetyCheckCommonObserver An observer instance that will receive the
* {@link SafetyCheckCommonObserver#onSafeBrowsingCheckResult(SafeBrowsingStatus)}. * result of the check.
*/ */
public SafetyCheckBridge(SafetyCheckCommonObserver safetyCheckCommonObserver) { public SafetyCheckBridge(SafetyCheckCommonObserver safetyCheckCommonObserver) {
mNativeSafetyCheckBridge = mNativeSafetyCheckBridge =
...@@ -48,6 +71,37 @@ public class SafetyCheckBridge { ...@@ -48,6 +71,37 @@ public class SafetyCheckBridge {
mNativeSafetyCheckBridge, SafetyCheckBridge.this); mNativeSafetyCheckBridge, SafetyCheckBridge.this);
} }
/**
* Triggers the passwords check on the C++ side.
*/
void checkPasswords() {
SafetyCheckBridgeJni.get().checkPasswords(mNativeSafetyCheckBridge, SafetyCheckBridge.this);
}
/**
* @return Number of password leaks discovered during the last check.
*/
int getNumberOfPasswordLeaksFromLastCheck() {
return SafetyCheckBridgeJni.get().getNumberOfPasswordLeaksFromLastCheck(
mNativeSafetyCheckBridge, SafetyCheckBridge.this);
}
/**
* @return Whether the user has some passwords saved.
*/
boolean savedPasswordsExist() {
return SafetyCheckBridgeJni.get().savedPasswordsExist(
mNativeSafetyCheckBridge, SafetyCheckBridge.this);
}
/**
* Stops observing the events of the passwords leak check.
*/
void stopObservingPasswordsCheck() {
SafetyCheckBridgeJni.get().stopObservingPasswordsCheck(
mNativeSafetyCheckBridge, SafetyCheckBridge.this);
}
/** /**
* Destroys the C++ side of the Bridge, freeing up all the associated memory. * Destroys the C++ side of the Bridge, freeing up all the associated memory.
*/ */
...@@ -64,6 +118,13 @@ public class SafetyCheckBridge { ...@@ -64,6 +118,13 @@ public class SafetyCheckBridge {
interface Natives { interface Natives {
long init(SafetyCheckBridge safetyCheckBridge, SafetyCheckCommonObserver observer); long init(SafetyCheckBridge safetyCheckBridge, SafetyCheckCommonObserver observer);
void checkSafeBrowsing(long nativeSafetyCheckBridge, SafetyCheckBridge safetyCheckBridge); void checkSafeBrowsing(long nativeSafetyCheckBridge, SafetyCheckBridge safetyCheckBridge);
void checkPasswords(long nativeSafetyCheckBridge, SafetyCheckBridge safetyCheckBridge);
int getNumberOfPasswordLeaksFromLastCheck(
long nativeSafetyCheckBridge, SafetyCheckBridge safetyCheckBridge);
boolean savedPasswordsExist(
long nativeSafetyCheckBridge, SafetyCheckBridge safetyCheckBridge);
void stopObservingPasswordsCheck(
long nativeSafetyCheckBridge, SafetyCheckBridge safetyCheckBridge);
void destroy(long nativeSafetyCheckBridge, SafetyCheckBridge safetyCheckBridge); void destroy(long nativeSafetyCheckBridge, SafetyCheckBridge safetyCheckBridge);
} }
} }
...@@ -20,24 +20,40 @@ import org.mockito.MockitoAnnotations; ...@@ -20,24 +20,40 @@ import org.mockito.MockitoAnnotations;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.JniMocker; import org.chromium.base.test.util.JniMocker;
import org.chromium.chrome.browser.password_check.BulkLeakCheckServiceState;
import org.chromium.chrome.browser.safety_check.SafetyCheckBridge.SafetyCheckCommonObserver; import org.chromium.chrome.browser.safety_check.SafetyCheckBridge.SafetyCheckCommonObserver;
/** Unit tests for {@link SafetyCheckBridge}. */ /** Unit tests for {@link SafetyCheckBridge}. */
@RunWith(BaseRobolectricTestRunner.class) @RunWith(BaseRobolectricTestRunner.class)
public class SafetyCheckBridgeTest { public class SafetyCheckBridgeTest {
class TestObserver implements SafetyCheckCommonObserver { class TestObserver implements SafetyCheckCommonObserver {
public boolean callbackInvoked = false; public boolean sbCallbackInvoked = false;
public boolean passwordsStateChangeInvoked = false;
private @SafeBrowsingStatus int mExpected = -1; private @SafeBrowsingStatus int mSBExpected = -1;
private @BulkLeakCheckServiceState int mPasswordsStateExpected = -1;
@Override @Override
public void onSafeBrowsingCheckResult(@SafeBrowsingStatus int status) { public void onSafeBrowsingCheckResult(@SafeBrowsingStatus int status) {
callbackInvoked = true; sbCallbackInvoked = true;
assertEquals(mExpected, status); assertEquals(mSBExpected, status);
} }
public void setExpected(@SafeBrowsingStatus int expected) { @Override
mExpected = expected; public void onPasswordCheckCredentialDone(int checked, int total) {}
@Override
public void onPasswordCheckStateChange(@BulkLeakCheckServiceState int state) {
passwordsStateChangeInvoked = true;
assertEquals(mPasswordsStateExpected, state);
}
public void setSafeBrowsingExpected(@SafeBrowsingStatus int expected) {
mSBExpected = expected;
}
public void setPasswordsExpected(@BulkLeakCheckServiceState int expected) {
mPasswordsStateExpected = expected;
} }
} }
...@@ -59,7 +75,7 @@ public class SafetyCheckBridgeTest { ...@@ -59,7 +75,7 @@ public class SafetyCheckBridgeTest {
@Test @Test
public void testCheckSafeBrowsing() { public void testCheckSafeBrowsing() {
assertFalse(mObserver.callbackInvoked); assertFalse(mObserver.sbCallbackInvoked);
@SafeBrowsingStatus @SafeBrowsingStatus
int expected = SafeBrowsingStatus.DISABLED_BY_ADMIN; int expected = SafeBrowsingStatus.DISABLED_BY_ADMIN;
...@@ -69,10 +85,29 @@ public class SafetyCheckBridgeTest { ...@@ -69,10 +85,29 @@ public class SafetyCheckBridgeTest {
}) })
.when(mNativeMock) .when(mNativeMock)
.checkSafeBrowsing(anyLong(), any(SafetyCheckBridge.class)); .checkSafeBrowsing(anyLong(), any(SafetyCheckBridge.class));
mObserver.setExpected(expected); mObserver.setSafeBrowsingExpected(expected);
mSafetyCheckBridge.checkSafeBrowsing(); mSafetyCheckBridge.checkSafeBrowsing();
assertTrue(mObserver.callbackInvoked); assertTrue(mObserver.sbCallbackInvoked);
}
@Test
public void testCheckPasswords() {
assertFalse(mObserver.passwordsStateChangeInvoked);
@BulkLeakCheckServiceState
int expected = BulkLeakCheckServiceState.HASHING_FAILURE;
doAnswer(invocation -> {
mObserver.onPasswordCheckStateChange(expected);
return null;
})
.when(mNativeMock)
.checkPasswords(anyLong(), any(SafetyCheckBridge.class));
mObserver.setPasswordsExpected(expected);
mSafetyCheckBridge.checkPasswords();
assertTrue(mObserver.passwordsStateChangeInvoked);
} }
} }
\ No newline at end of file
...@@ -26,11 +26,13 @@ SafetyCheckBridge::SafetyCheckBridge( ...@@ -26,11 +26,13 @@ SafetyCheckBridge::SafetyCheckBridge(
->GetPrefs()), ->GetPrefs()),
j_safety_check_observer_(j_safety_check_observer) { j_safety_check_observer_(j_safety_check_observer) {
safety_check_.reset(new safety_check::SafetyCheck(this)); safety_check_.reset(new safety_check::SafetyCheck(this));
password_check_controller_.reset(new BulkLeakCheckControllerAndroid());
} }
void SafetyCheckBridge::Destroy( void SafetyCheckBridge::Destroy(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) { const base::android::JavaParamRef<jobject>& obj) {
password_check_controller_.reset();
safety_check_.reset(); safety_check_.reset();
delete this; delete this;
} }
...@@ -41,6 +43,31 @@ void SafetyCheckBridge::CheckSafeBrowsing( ...@@ -41,6 +43,31 @@ void SafetyCheckBridge::CheckSafeBrowsing(
safety_check_->CheckSafeBrowsing(pref_service_); safety_check_->CheckSafeBrowsing(pref_service_);
} }
void SafetyCheckBridge::CheckPasswords(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
password_check_controller_->AddObserver(this);
password_check_controller_->StartPasswordCheck();
}
int SafetyCheckBridge::GetNumberOfPasswordLeaksFromLastCheck(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
return password_check_controller_->GetNumberOfLeaksFromLastCheck();
}
bool SafetyCheckBridge::SavedPasswordsExist(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
return password_check_controller_->GetNumberOfSavedPasswords() != 0;
}
void SafetyCheckBridge::StopObservingPasswordsCheck(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
password_check_controller_->RemoveObserver(this);
}
void SafetyCheckBridge::OnSafeBrowsingCheckResult( void SafetyCheckBridge::OnSafeBrowsingCheckResult(
safety_check::SafetyCheck::SafeBrowsingStatus status) { safety_check::SafetyCheck::SafeBrowsingStatus status) {
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
...@@ -48,4 +75,22 @@ void SafetyCheckBridge::OnSafeBrowsingCheckResult( ...@@ -48,4 +75,22 @@ void SafetyCheckBridge::OnSafeBrowsingCheckResult(
env, j_safety_check_observer_, static_cast<int>(status)); env, j_safety_check_observer_, static_cast<int>(status));
} }
void SafetyCheckBridge::OnStateChanged(
password_manager::BulkLeakCheckService::State state) {
JNIEnv* env = base::android::AttachCurrentThread();
Java_SafetyCheckCommonObserver_onPasswordCheckStateChange(
env, j_safety_check_observer_, static_cast<int>(state));
}
void SafetyCheckBridge::OnCredentialDone(
const password_manager::LeakCheckCredential& credential,
password_manager::IsLeaked is_leaked,
SafetyCheckBridge::DoneCount credentials_checked,
SafetyCheckBridge::TotalCount total_to_check) {
JNIEnv* env = base::android::AttachCurrentThread();
Java_SafetyCheckCommonObserver_onPasswordCheckCredentialDone(
env, j_safety_check_observer_, credentials_checked.value(),
total_to_check.value());
}
SafetyCheckBridge::~SafetyCheckBridge() = default; SafetyCheckBridge::~SafetyCheckBridge() = default;
...@@ -8,13 +8,15 @@ ...@@ -8,13 +8,15 @@
#include <jni.h> #include <jni.h>
#include "base/android/scoped_java_ref.h" #include "base/android/scoped_java_ref.h"
#include "chrome/browser/password_check/android/bulk_leak_check_controller_android.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/safety_check/safety_check.h" #include "components/safety_check/safety_check.h"
// Allows the Java code to make use of cross-platform browser safety checks in // Allows the Java code to make use of cross-platform browser safety checks in
// //components/safety_check. // //components/safety_check.
class SafetyCheckBridge class SafetyCheckBridge
: public safety_check::SafetyCheck::SafetyCheckHandlerInterface { : public safety_check::SafetyCheck::SafetyCheckHandlerInterface,
public BulkLeakCheckControllerAndroid::Observer {
public: public:
// Takes an observer object that will get invoked on check results. // Takes an observer object that will get invoked on check results.
SafetyCheckBridge( SafetyCheckBridge(
...@@ -29,17 +31,44 @@ class SafetyCheckBridge ...@@ -29,17 +31,44 @@ class SafetyCheckBridge
void CheckSafeBrowsing(JNIEnv* env, void CheckSafeBrowsing(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj); const base::android::JavaParamRef<jobject>& obj);
// Gets invoked by |safety_check::SafetyCheck| once the |SafeBrowsingStatus| // Checks the passwords and invokes |OnPasswordCheckCredentialDone| and
// is available. Invokes a method with the same name on the Java-side // |onPasswordCheckStateChange| on the observer.
// observer. void CheckPasswords(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
// Returns the number of leaked passwords without running a new check.
int GetNumberOfPasswordLeaksFromLastCheck(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
// Returns whether the user has a non-zero amount of passwords saved.
bool SavedPasswordsExist(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
// Stops observing |BulkLeakCheckControllerAndroid| events.
void StopObservingPasswordsCheck(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
// safety_check::SafetyCheck::SafetyCheckHandlerInterface implementation.
void OnSafeBrowsingCheckResult( void OnSafeBrowsingCheckResult(
safety_check::SafetyCheck::SafeBrowsingStatus status) override; safety_check::SafetyCheck::SafeBrowsingStatus status) override;
// BulkLeakCheckControllerAndroid::Observer implementation.
void OnStateChanged(
password_manager::BulkLeakCheckService::State state) override;
void OnCredentialDone(const password_manager::LeakCheckCredential& credential,
password_manager::IsLeaked is_leaked,
DoneCount credentials_checked,
TotalCount total_to_check) override;
private: private:
virtual ~SafetyCheckBridge(); ~SafetyCheckBridge() override;
PrefService* pref_service_ = nullptr; PrefService* pref_service_ = nullptr;
std::unique_ptr<safety_check::SafetyCheck> safety_check_; std::unique_ptr<safety_check::SafetyCheck> safety_check_;
std::unique_ptr<BulkLeakCheckControllerAndroid> password_check_controller_;
base::android::ScopedJavaGlobalRef<jobject> j_safety_check_observer_; base::android::ScopedJavaGlobalRef<jobject> j_safety_check_observer_;
}; };
......
...@@ -19,6 +19,8 @@ namespace password_manager { ...@@ -19,6 +19,8 @@ namespace password_manager {
// passwords against the database of leaked credentials. // passwords against the database of leaked credentials.
class BulkLeakCheckServiceInterface : public KeyedService { class BulkLeakCheckServiceInterface : public KeyedService {
public: public:
// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.password_check
// GENERATED_JAVA_CLASS_NAME_OVERRIDE: BulkLeakCheckServiceState
enum class State { enum class State {
// The service is idle and there was no previous error. // The service is idle and there was no previous error.
kIdle, kIdle,
......
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