Skip to content

Commit

Permalink
Handle SecurityException thrown while launching the browser (#677)
Browse files Browse the repository at this point in the history
  • Loading branch information
poovamraj authored Aug 1, 2023
1 parent c6b47ed commit 5a44d52
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ public class AuthenticationException : Auth0Exception {
this.description = description
}

public constructor(message: String, cause: Auth0Exception? = null) : super(message, cause)
public constructor(message: String, cause: Exception? = null) : super(message, cause)

public constructor(code: String, description: String, cause: Exception) : this(DEFAULT_MESSAGE, cause) {
this.code = code
this.description = description
}

public constructor(payload: String?, statusCode: Int) : this(DEFAULT_MESSAGE) {
code = if (payload != null) NON_JSON_ERROR else EMPTY_BODY_ERROR
description = payload ?: EMPTY_RESPONSE_BODY_DESCRIPTION
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.auth0.android.callback

internal interface RunnableTask<T> {
fun apply(t: T)
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import android.content.Intent
import android.net.Uri
import android.os.Bundle
import androidx.annotation.VisibleForTesting
import com.auth0.android.authentication.AuthenticationException
import com.auth0.android.callback.RunnableTask
import com.auth0.android.provider.WebAuthProvider.failure
import com.auth0.android.provider.WebAuthProvider.resume
import com.google.androidbrowserhelper.trusted.TwaLauncher

Expand Down Expand Up @@ -70,7 +73,11 @@ public open class AuthenticationActivity : Activity() {
val launchAsTwa: Boolean = extras.getBoolean(EXTRA_LAUNCH_AS_TWA, false)
customTabsController = createCustomTabsController(this, customTabsOptions)
customTabsController!!.bindService()
customTabsController!!.launchUri(authorizeUri!!, launchAsTwa)
customTabsController!!.launchUri(authorizeUri!!, launchAsTwa, object : RunnableTask<AuthenticationException> {
override fun apply(error: AuthenticationException) {
deliverAuthenticationFailure(error)
}
})
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
Expand All @@ -86,6 +93,11 @@ public open class AuthenticationActivity : Activity() {
resume(result)
}

internal open fun deliverAuthenticationFailure(ex: AuthenticationException) {
failure(ex)
finish()
}

internal companion object {
const val EXTRA_AUTHORIZE_URI = "com.auth0.android.EXTRA_AUTHORIZE_URI"
const val EXTRA_LAUNCH_AS_TWA = "com.auth0.android.EXTRA_LAUNCH_AS_TWA"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,17 @@
import androidx.browser.customtabs.CustomTabsServiceConnection;
import androidx.browser.customtabs.CustomTabsSession;

import com.auth0.android.authentication.AuthenticationException;
import com.auth0.android.callback.RunnableTask;
import com.auth0.android.request.internal.CommonThreadSwitcher;
import com.google.androidbrowserhelper.trusted.TwaLauncher;

import java.lang.ref.WeakReference;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;


@SuppressWarnings("WeakerAccess")
class CustomTabsController extends CustomTabsServiceConnection {

Expand Down Expand Up @@ -107,7 +111,7 @@ public void unbindService() {
*
* @param uri the uri to open in a Custom Tab or Browser.
*/
public void launchUri(@NonNull final Uri uri, final boolean launchAsTwa) {
public void launchUri(@NonNull final Uri uri, final boolean launchAsTwa, final RunnableTask<AuthenticationException> failureCallback) {
final Context context = this.context.get();
if (context == null) {
Log.v(TAG, "Custom Tab Context was no longer valid.");
Expand All @@ -130,6 +134,10 @@ public void launchUri(@NonNull final Uri uri, final boolean launchAsTwa) {
}
} catch (ActivityNotFoundException ex) {
Log.e(TAG, "Could not find any Browser application installed in this device to handle the intent.");
} catch (SecurityException ex) {
AuthenticationException e = new AuthenticationException(
"a0.browser_not_available", "Error launching browser for authentication", ex);
CommonThreadSwitcher.getInstance().mainThread(() -> failureCallback.apply(e));
}
}).start();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ internal class LogoutManager(
return true
}

override fun failure(exception: AuthenticationException) {
callback.onFailure(exception)
}

private fun buildLogoutUri(): Uri {
val logoutUri = Uri.parse(account.logoutUrl)
val builder = logoutUri.buildUpon()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ internal class OAuthManager(
return true
}

public override fun failure(exception: AuthenticationException) {
callback.onFailure(exception)
}

private fun assertValidIdToken(
idToken: String?,
validationCallback: Callback<Void?, Auth0Exception>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
package com.auth0.android.provider;

import androidx.annotation.NonNull;

import com.auth0.android.Auth0Exception;
import com.auth0.android.authentication.AuthenticationException;

/**
* Internal class, used to generify the handling of different Web Auth flows.
* See {@link WebAuthProvider}
Expand All @@ -14,4 +19,6 @@ abstract class ResumableManager {
* @see AuthorizeResult
*/
abstract boolean resume(AuthorizeResult result);

abstract void failure(@NonNull AuthenticationException exception);
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ public object WebAuthProvider {
return success
}

internal fun failure(exception: AuthenticationException) {
managerInstance!!.failure(exception)
}

@JvmStatic
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun resetManagerInstance() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import android.net.Uri
import android.os.Bundle
import android.os.Parcelable
import androidx.test.espresso.intent.matcher.IntentMatchers
import com.auth0.android.authentication.AuthenticationException
import com.auth0.android.callback.RunnableTask
import com.auth0.android.provider.AuthenticationActivity
import com.auth0.android.provider.CustomTabsOptions
import org.hamcrest.CoreMatchers
Expand Down Expand Up @@ -44,6 +46,9 @@ public class AuthenticationActivityTest {
@Captor
private lateinit var launchAsTwaCaptor: ArgumentCaptor<Boolean>

@Captor
private lateinit var failureCallbackCaptor: ArgumentCaptor<RunnableTask<AuthenticationException>>

private lateinit var callerActivity: Activity
private lateinit var activity: AuthenticationActivityMock
private lateinit var activityController: ActivityController<AuthenticationActivityMock>
Expand Down Expand Up @@ -86,7 +91,7 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture())
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), failureCallbackCaptor.capture())
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(uri))
MatcherAssert.assertThat(activity.deliveredIntent, Is.`is`(Matchers.nullValue()))
Expand Down Expand Up @@ -117,7 +122,7 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture())
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), failureCallbackCaptor.capture())
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(uri))
MatcherAssert.assertThat(activity.deliveredIntent, Is.`is`(Matchers.nullValue()))
Expand Down Expand Up @@ -148,7 +153,7 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture())
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), failureCallbackCaptor.capture())
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(uri))
MatcherAssert.assertThat(launchAsTwaCaptor.value, Is.`is`(Matchers.notNullValue()))
Expand Down Expand Up @@ -178,7 +183,7 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture())
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), failureCallbackCaptor.capture())
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(uriCaptor.value, Is.`is`(uri))
MatcherAssert.assertThat(launchAsTwaCaptor.value, Is.`is`(Matchers.notNullValue()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import android.content.ServiceConnection;
import android.graphics.Color;
import android.net.Uri;
import android.os.Looper;

import androidx.browser.customtabs.CustomTabsCallback;
import androidx.browser.customtabs.CustomTabsClient;
Expand Down Expand Up @@ -37,6 +38,7 @@
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.core.Is.is;
import static org.hamcrest.core.Is.isA;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.eq;
Expand All @@ -49,6 +51,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.auth0.android.authentication.AuthenticationException;
import com.google.androidbrowserhelper.trusted.TwaLauncher;
import com.google.androidbrowserhelper.trusted.splashscreens.SplashScreenStrategy;

Expand Down Expand Up @@ -127,7 +130,7 @@ public void shouldUnbindEvenIfNotBound() throws Exception {
@Test
public void shouldBindAndLaunchUri() throws Exception {
bindService(controller, true);
controller.launchUri(uri, false);
controller.launchUri(uri, false, null);
connectBoundService();

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
Expand All @@ -146,7 +149,7 @@ public void shouldBindAndLaunchUri() throws Exception {
@Test
public void shouldBindAndLaunchUriAsTwa() throws Exception {
bindService(controller, true);
controller.launchUri(uri, true);
controller.launchUri(uri, true, null);
connectBoundService();
ArgumentCaptor<TrustedWebActivityIntentBuilder> trustedWebActivityIntentBuilderArgumentCaptor
= ArgumentCaptor.forClass(TrustedWebActivityIntentBuilder.class);
Expand Down Expand Up @@ -176,7 +179,7 @@ public void shouldLaunchUriUsingFallbackWhenNoCompatibleBrowserIsAvailable() {
when(browserPicker.getBestBrowserPackage(context.getPackageManager())).thenReturn(null);
CustomTabsOptions ctOptions = CustomTabsOptions.newBuilder().withBrowserPicker(browserPicker).build();
CustomTabsController controller = new CustomTabsController(context, ctOptions, twaLauncher);
controller.launchUri(uri, false);
controller.launchUri(uri, false, null);

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
Intent intent = launchIntentCaptor.getValue();
Expand All @@ -199,7 +202,7 @@ public void shouldBindAndLaunchUriWithCustomization() throws Exception {
CustomTabsController controller = new CustomTabsController(context, ctOptions, twaLauncher);

bindService(controller, true);
controller.launchUri(uri, false);
controller.launchUri(uri, false, null);
connectBoundService();

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
Expand Down Expand Up @@ -228,7 +231,7 @@ public void shouldBindAndLaunchUriWithCustomizationTwa() throws Exception {
CustomTabsController controller = new CustomTabsController(context, ctOptions, twaLauncher);

bindService(controller, true);
controller.launchUri(uri, true);
controller.launchUri(uri, true, null);
connectBoundService();


Expand Down Expand Up @@ -257,7 +260,7 @@ public void shouldBindAndLaunchUriWithCustomizationTwa() throws Exception {
@Test
public void shouldFailToBindButLaunchUri() {
bindService(controller, false);
controller.launchUri(uri, false);
controller.launchUri(uri, false, null);

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
Intent intent = launchIntentCaptor.getValue();
Expand All @@ -271,7 +274,7 @@ public void shouldFailToBindButLaunchUri() {
public void shouldNotLaunchUriIfContextNoLongerValid() {
bindService(controller, true);
controller.clearContext();
controller.launchUri(uri, false);
controller.launchUri(uri, false, (ex) -> {});
verify(context, never()).startActivity(any(Intent.class));
}

Expand All @@ -280,7 +283,7 @@ public void shouldLaunchUriWithFallbackIfCustomTabIntentFails() {
doThrow(ActivityNotFoundException.class)
.doNothing()
.when(context).startActivity(any(Intent.class));
controller.launchUri(uri, false);
controller.launchUri(uri, false, null);

verify(context, timeout(MAX_TEST_WAIT_TIME_MS)).startActivity(launchIntentCaptor.capture());
List<Intent> intents = launchIntentCaptor.getAllValues();
Expand All @@ -298,6 +301,20 @@ public void shouldLaunchUriWithFallbackIfCustomTabIntentFails() {
assertThat(customTabIntent.getIntExtra(CustomTabsIntent.EXTRA_SHARE_STATE, CustomTabsIntent.SHARE_STATE_OFF), is(CustomTabsIntent.SHARE_STATE_OFF));
}

@Test
public void shouldThrowExceptionIfFailedToLaunchBecauseOfException() {
Exception e = new SecurityException();
doThrow(e)
.when(context).startActivity(any(Intent.class));
controller.launchUri(uri, false, (ex) -> {
assertThat(ex, isA(AuthenticationException.class));
assertThat(ex.getCause(), is(eq(e)));
assertThat(ex.getCode(), is("a0.browser_not_available"));
assertThat(ex.getDescription(), is("Error launching browser for authentication"));
assertThat(Looper.myLooper(), is(Looper.getMainLooper()));
});
}

//Helper Methods

@SuppressWarnings("WrongConstant")
Expand Down
4 changes: 2 additions & 2 deletions sample/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<resources>
<string name="app_name">Auth0 SDK Sample</string>
<string name="com_auth0_domain">lbalmaceda.auth0.com</string>
<string name="com_auth0_client_id">PX2vJhpALUNT66NHdCdD30XWqlIR4oEZ</string>
<string name="com_auth0_domain">poovamraj.eu.auth0.com</string>
<string name="com_auth0_client_id">swarLtLAd0aW55ajPUPzYPXjtB3UtZki</string>
<string name="com_auth0_scheme">demo</string>
</resources>

0 comments on commit 5a44d52

Please sign in to comment.