Commit f50ddd34 authored by Adam Langley's avatar Adam Langley Committed by Commit Bot

webauthn: a batch of caBLE Java cleanups.

I'm flushing my queue of niggles that I've noticed over time:

1. The |Callback| interface in |CableAuthenticator| is moot. It always
   calls |CableAuthenticatorUI|, which is in the same package. There's
   even an |mUi| member already.
2. In a couple of cases calls were moved to the |mTaskRunner| thread,
   but since the Mojo network interface only works on the UI thread, we
   know that's the UI thread now and thus the transition is moot.
   Replace them with asserts.
3. A number of methods were more public than needed. If not specified,
   Java defaults to package-private, which works for a lot of cases
   here.

BUG=1002262

Change-Id: Ieee4ab7fc9c1f3ae339d661b2ed5ce6e1adb28ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2531178
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: default avatarMartin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#826980}
parent 1b5d2b01
...@@ -83,7 +83,6 @@ class CableAuthenticator { ...@@ -83,7 +83,6 @@ class CableAuthenticator {
private final Context mContext; private final Context mContext;
private final CableAuthenticatorUI mUi; private final CableAuthenticatorUI mUi;
private final Callback mCallback;
private final SingleThreadTaskRunner mTaskRunner; private final SingleThreadTaskRunner mTaskRunner;
public enum Result { public enum Result {
...@@ -94,27 +93,11 @@ class CableAuthenticator { ...@@ -94,27 +93,11 @@ class CableAuthenticator {
OTHER, OTHER,
} }
// Callbacks notifying the UI of certain CableAuthenticator state changes. These may run on any
// thread.
public interface Callback {
// Invoked when the authenticator has completed a handshake with a client device.
void onAuthenticatorConnected();
// Invoked when a transaction has completed. The response may still be transmitting and
// onComplete will follow.
void onAuthenticatorResult(Result result);
// Invoked when the authenticator has finished. The UI should be dismissed at this point.
void onComplete();
// Called when an informative status update is available. The argument has the same values
// as the Status enum from v2_authenticator.h.
void onStatus(int code);
}
public CableAuthenticator(Context context, CableAuthenticatorUI ui, long networkContext, public CableAuthenticator(Context context, CableAuthenticatorUI ui, long networkContext,
long registration, String activityClassName, boolean isFcmNotification, long registration, String activityClassName, boolean isFcmNotification,
UsbAccessory accessory) { UsbAccessory accessory) {
mContext = context; mContext = context;
mUi = ui; mUi = ui;
mCallback = ui;
// networkContext can only be used from the UI thread, therefore all // networkContext can only be used from the UI thread, therefore all
// short-lived work is done on that thread. // short-lived work is done on that thread.
...@@ -169,7 +152,7 @@ class CableAuthenticator { ...@@ -169,7 +152,7 @@ class CableAuthenticator {
// as the Status enum from v2_authenticator.h. // as the Status enum from v2_authenticator.h.
@CalledByNative @CalledByNative
public void onStatus(int code) { public void onStatus(int code) {
mCallback.onStatus(code); mUi.onStatus(code);
} }
@CalledByNative @CalledByNative
...@@ -306,10 +289,10 @@ class CableAuthenticator { ...@@ -306,10 +289,10 @@ class CableAuthenticator {
@CalledByNative @CalledByNative
public void onComplete() { public void onComplete() {
assert mTaskRunner.belongsToCurrentThread(); assert mTaskRunner.belongsToCurrentThread();
mCallback.onComplete(); mUi.onComplete();
} }
public void onActivityResult(int requestCode, int resultCode, Intent data) { void onActivityResult(int requestCode, int resultCode, Intent data) {
Log.i(TAG, "onActivityResult " + requestCode + " " + resultCode); Log.i(TAG, "onActivityResult " + requestCode + " " + resultCode);
Result result = Result.OTHER; Result result = Result.OTHER;
...@@ -332,10 +315,10 @@ class CableAuthenticator { ...@@ -332,10 +315,10 @@ class CableAuthenticator {
Log.i(TAG, "invalid requestCode: " + requestCode); Log.i(TAG, "invalid requestCode: " + requestCode);
assert (false); assert (false);
} }
mCallback.onAuthenticatorResult(result); mUi.onAuthenticatorResult(result);
} }
public boolean onRegisterResponse(int resultCode, Intent data) { private boolean onRegisterResponse(int resultCode, Intent data) {
if (resultCode != Activity.RESULT_OK || data == null) { if (resultCode != Activity.RESULT_OK || data == null) {
Log.e(TAG, "Failed with result code" + resultCode); Log.e(TAG, "Failed with result code" + resultCode);
onAuthenticatorAssertionResponse(CTAP2_ERR_OPERATION_DENIED, null, null, null, null); onAuthenticatorAssertionResponse(CTAP2_ERR_OPERATION_DENIED, null, null, null, null);
...@@ -384,7 +367,7 @@ class CableAuthenticator { ...@@ -384,7 +367,7 @@ class CableAuthenticator {
return true; return true;
} }
public boolean onSignResponse(int resultCode, Intent data) { private boolean onSignResponse(int resultCode, Intent data) {
if (resultCode != Activity.RESULT_OK || data == null) { if (resultCode != Activity.RESULT_OK || data == null) {
Log.e(TAG, "Failed with result code" + resultCode); Log.e(TAG, "Failed with result code" + resultCode);
onAuthenticatorAssertionResponse(CTAP2_ERR_OPERATION_DENIED, null, null, null, null); onAuthenticatorAssertionResponse(CTAP2_ERR_OPERATION_DENIED, null, null, null, null);
...@@ -457,15 +440,14 @@ class CableAuthenticator { ...@@ -457,15 +440,14 @@ class CableAuthenticator {
* @param value contents of the QR code, which will be a valid caBLE * @param value contents of the QR code, which will be a valid caBLE
* URL, i.e. "fido://"... * URL, i.e. "fido://"...
*/ */
public void onQRCode(String value) { void onQRCode(String value) {
mTaskRunner.postTask(() -> { assert mTaskRunner.belongsToCurrentThread();
CableAuthenticatorJni.get().startQR(this, getName(), value); CableAuthenticatorJni.get().startQR(this, getName(), value);
// TODO: show the user an error if that returned false. // TODO: show the user an error if that returned false.
// that indicates that the QR code was invalid. // that indicates that the QR code was invalid.
});
} }
public void unlinkAllDevices() { void unlinkAllDevices() {
Log.i(TAG, "Unlinking devices"); Log.i(TAG, "Unlinking devices");
byte[] newStateBytes = CableAuthenticatorJni.get().unlink(); byte[] newStateBytes = CableAuthenticatorJni.get().unlink();
SharedPreferences prefs = ContextUtils.getAppSharedPreferences(); SharedPreferences prefs = ContextUtils.getAppSharedPreferences();
...@@ -475,8 +457,9 @@ class CableAuthenticator { ...@@ -475,8 +457,9 @@ class CableAuthenticator {
.apply(); .apply();
} }
public void close() { void close() {
mTaskRunner.postTask(() -> { CableAuthenticatorJni.get().stop(); }); assert mTaskRunner.belongsToCurrentThread();
CableAuthenticatorJni.get().stop();
} }
static String getName() { static String getName() {
...@@ -486,7 +469,7 @@ class CableAuthenticator { ...@@ -486,7 +469,7 @@ class CableAuthenticator {
/** /**
* onCloudMessage is called by {@link CableAuthenticatorUI} when a GCM message is received. * onCloudMessage is called by {@link CableAuthenticatorUI} when a GCM message is received.
*/ */
public static void onCloudMessage( static void onCloudMessage(
long event, long systemNetworkContext, long registration, String activityClassName) { long event, long systemNetworkContext, long registration, String activityClassName) {
setup(registration, activityClassName, systemNetworkContext); setup(registration, activityClassName, systemNetworkContext);
CableAuthenticatorJni.get().onCloudMessage(event); CableAuthenticatorJni.get().onCloudMessage(event);
......
...@@ -36,8 +36,8 @@ import java.lang.ref.WeakReference; ...@@ -36,8 +36,8 @@ import java.lang.ref.WeakReference;
/** /**
* A fragment that provides a UI for scanning caBLE v2 QR codes. * A fragment that provides a UI for scanning caBLE v2 QR codes.
*/ */
public class CableAuthenticatorUI extends Fragment public class CableAuthenticatorUI
implements OnClickListener, QRScanDialog.Callback, CableAuthenticator.Callback { extends Fragment implements OnClickListener, QRScanDialog.Callback {
private enum Mode { private enum Mode {
QR, // Triggered from Settings; can scan QR code to start handshake. QR, // Triggered from Settings; can scan QR code to start handshake.
FCM, // Triggered by user selecting notification; handshake already running. FCM, // Triggered by user selecting notification; handshake already running.
...@@ -188,8 +188,7 @@ public class CableAuthenticatorUI extends Fragment ...@@ -188,8 +188,7 @@ public class CableAuthenticatorUI extends Fragment
mAuthenticator.onQRCode(value); mAuthenticator.onQRCode(value);
} }
@Override void onStatus(int code) {
public void onStatus(int code) {
if (mMode != Mode.QR) { if (mMode != Mode.QR) {
// In FCM mode, the handshake is done before the UI appears. For // In FCM mode, the handshake is done before the UI appears. For
// USB everything should happen immediately. // USB everything should happen immediately.
...@@ -230,12 +229,10 @@ public class CableAuthenticatorUI extends Fragment ...@@ -230,12 +229,10 @@ public class CableAuthenticatorUI extends Fragment
mAuthenticator.onActivityResult(requestCode, resultCode, data); mAuthenticator.onActivityResult(requestCode, resultCode, data);
} }
@Override
@SuppressLint("SetTextI18n") @SuppressLint("SetTextI18n")
public void onAuthenticatorConnected() {} void onAuthenticatorConnected() {}
@Override void onAuthenticatorResult(CableAuthenticator.Result result) {
public void onAuthenticatorResult(CableAuthenticator.Result result) {
getActivity().runOnUiThread(() -> { getActivity().runOnUiThread(() -> {
// TODO: Temporary UI, needs i18n. // TODO: Temporary UI, needs i18n.
String toast = "An error occured. Please try again."; String toast = "An error occured. Please try again.";
...@@ -259,8 +256,7 @@ public class CableAuthenticatorUI extends Fragment ...@@ -259,8 +256,7 @@ public class CableAuthenticatorUI extends Fragment
}); });
} }
@Override void onComplete() {
public void onComplete() {
getActivity().runOnUiThread(() -> { getActivity().finish(); }); getActivity().runOnUiThread(() -> { getActivity().finish(); });
} }
......
...@@ -68,7 +68,7 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback { ...@@ -68,7 +68,7 @@ public class CameraView extends SurfaceView implements SurfaceHolder.Callback {
* Called to indicate that the callback that was passed to the constructor has finished * Called to indicate that the callback that was passed to the constructor has finished
* processing and thus is free to receive another camera frame. * processing and thus is free to receive another camera frame.
*/ */
public void rearmCallback() { void rearmCallback() {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
if (mCamera != null) { if (mCamera != null) {
......
...@@ -31,7 +31,7 @@ import java.nio.ByteBuffer; ...@@ -31,7 +31,7 @@ import java.nio.ByteBuffer;
* Displays a preview of what the default (rear) camera can see and processes images for QR * Displays a preview of what the default (rear) camera can see and processes images for QR
* codes. Closes once an applicable QR code has been found. * codes. Closes once an applicable QR code has been found.
*/ */
public class QRScanDialog extends DialogFragment implements Camera.PreviewCallback { class QRScanDialog extends DialogFragment implements Camera.PreviewCallback {
/** /**
* FIDO QR codes begin with this prefix. This class will ignore QR codes that don't match * FIDO QR codes begin with this prefix. This class will ignore QR codes that don't match
* this. * 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