Skip to content

Commit

Permalink
Handled NPE in the AuthenticationActivity (#759)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pmathew92
pmathew92 committed Sep 17, 2024
2 parents dc844e3 + 9af9473 commit 59fad7e
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 24 deletions.
6 changes: 3 additions & 3 deletions auth0/src/main/java/com/auth0/android/Auth0.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public open class Auth0 private constructor(
* @return Url to call to perform the web flow of OAuth
*/
public open val authorizeUrl: String
get() = domainUrl!!.newBuilder()
get() = domainUrl.newBuilder()
.addEncodedPathSegment("authorize")
.build()
.toString()
Expand All @@ -83,7 +83,7 @@ public open class Auth0 private constructor(
* @return Url to call to perform the web logout
*/
public open val logoutUrl: String
get() = domainUrl!!.newBuilder()
get() = domainUrl.newBuilder()
.addEncodedPathSegment("v2")
.addEncodedPathSegment("logout")
.build()
Expand Down Expand Up @@ -134,7 +134,7 @@ public open class Auth0 private constructor(
): Auth0 {
val domainUrl = ensureValidUrl(domain)
requireNotNull(domainUrl) { String.format("Invalid domain url: '%s'", domain) }
if (instance == null || instance!!.clientId != clientId || instance!!.domainUrl.host != domainUrl.host || instance!!.configurationDomain != configurationDomain) {
if (instance == null || instance?.clientId != clientId || instance?.domainUrl?.host != domainUrl.host || instance?.configurationDomain != configurationDomain) {
instance = Auth0(clientId, domainUrl, configurationDomain)
}
return instance!!
Expand Down
18 changes: 15 additions & 3 deletions auth0/src/main/java/com/auth0/android/authentication/AuthenticationException.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ public class AuthenticationException : Auth0Exception {

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

public constructor(code: String, description: String, cause: Exception) : this(DEFAULT_MESSAGE, cause) {
public constructor(code: String, description: String, cause: Exception) : this(
DEFAULT_MESSAGE,
cause
) {
this.code = code
this.description = description
}
Expand Down Expand Up @@ -126,7 +129,10 @@ public class AuthenticationException : Auth0Exception {
get() = "a0.invalid_configuration" == code

// When a user closes the browser app and in turn, cancels the authentication
@Deprecated("This property can refer to both log in and log out actions.", replaceWith = ReplaceWith("isCanceled"))
@Deprecated(
"This property can refer to both log in and log out actions.",
replaceWith = ReplaceWith("isCanceled")
)
public val isAuthenticationCanceled: Boolean
get() = isCanceled

Expand Down Expand Up @@ -183,7 +189,7 @@ public class AuthenticationException : Auth0Exception {
/// When authenticating with web-based authentication using prompt=none and the auth0 session had expired
public val isLoginRequired: Boolean
get() = "login_required" == code

/// User is deleted
public val isRefreshTokenDeleted: Boolean
get() = "invalid_grant" == code
Expand All @@ -205,6 +211,12 @@ public class AuthenticationException : Auth0Exception {

internal companion object {
internal const val ERROR_VALUE_AUTHENTICATION_CANCELED = "a0.authentication_canceled"
internal const val ERROR_KEY_URI_NULL = "a0.auth.authorize_uri"
internal const val ERROR_VALUE_AUTHORIZE_URI_INVALID =
"Authorization URI is received as null from the intent"
internal const val ERROR_KEY_CT_OPTIONS_NULL = "a0.auth.ct_options"
internal const val ERROR_VALUE_CT_OPTIONS_INVALID =
"Custom tab options are received as null from the intent"
private const val ERROR_KEY = "error"
private const val CODE_KEY = "code"
private const val DESCRIPTION_KEY = "description"
Expand Down
50 changes: 36 additions & 14 deletions auth0/src/main/java/com/auth0/android/provider/AuthenticationActivity.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import android.net.Uri
import android.os.Bundle
import androidx.annotation.VisibleForTesting
import com.auth0.android.authentication.AuthenticationException
import com.auth0.android.authentication.AuthenticationException.Companion.ERROR_KEY_CT_OPTIONS_NULL
import com.auth0.android.authentication.AuthenticationException.Companion.ERROR_KEY_URI_NULL
import com.auth0.android.authentication.AuthenticationException.Companion.ERROR_VALUE_AUTHORIZE_URI_INVALID
import com.auth0.android.authentication.AuthenticationException.Companion.ERROR_VALUE_CT_OPTIONS_INVALID
import com.auth0.android.callback.RunnableTask
import com.auth0.android.provider.WebAuthProvider.failure
import com.auth0.android.provider.WebAuthProvider.resume
Expand Down Expand Up @@ -68,23 +72,44 @@ public open class AuthenticationActivity : Activity() {
}

private fun launchAuthenticationIntent() {
val extras = intent.extras
val authorizeUri = extras!!.getParcelable<Uri>(EXTRA_AUTHORIZE_URI)
val customTabsOptions: CustomTabsOptions = extras.getParcelable(EXTRA_CT_OPTIONS)!!
val extras: Bundle? = intent.extras

val authorizeUri = extras?.getParcelable<Uri>(EXTRA_AUTHORIZE_URI)
authorizeUri ?: run {
deliverAuthenticationFailure(
AuthenticationException(
ERROR_KEY_URI_NULL, ERROR_VALUE_AUTHORIZE_URI_INVALID
)
)
return
}

val customTabsOptions: CustomTabsOptions? = extras.getParcelable(EXTRA_CT_OPTIONS)
customTabsOptions ?: run {
deliverAuthenticationFailure(
AuthenticationException(
ERROR_KEY_CT_OPTIONS_NULL, ERROR_VALUE_CT_OPTIONS_INVALID
)
)
return
}

val launchAsTwa: Boolean = extras.getBoolean(EXTRA_LAUNCH_AS_TWA, false)
customTabsController = createCustomTabsController(this, customTabsOptions)
customTabsController!!.bindService()
customTabsController!!.launchUri(authorizeUri!!, launchAsTwa, getInstance(), object : RunnableTask<AuthenticationException> {
override fun apply(error: AuthenticationException) {
deliverAuthenticationFailure(error)
}
})
customTabsController!!.launchUri(authorizeUri,
launchAsTwa,
getInstance(),
object : RunnableTask<AuthenticationException> {
override fun apply(error: AuthenticationException) {
deliverAuthenticationFailure(error)
}
})
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal open fun createCustomTabsController(
context: Context,
options: CustomTabsOptions
context: Context, options: CustomTabsOptions
): CustomTabsController {
return CustomTabsController(context, options, TwaLauncher(context))
}
Expand All @@ -107,10 +132,7 @@ public open class AuthenticationActivity : Activity() {

@JvmStatic
internal fun authenticateUsingBrowser(
context: Context,
authorizeUri: Uri,
launchAsTwa: Boolean,
options: CustomTabsOptions
context: Context, authorizeUri: Uri, launchAsTwa: Boolean, options: CustomTabsOptions
) {
val intent = Intent(context, AuthenticationActivity::class.java)
intent.putExtra(EXTRA_AUTHORIZE_URI, authorizeUri)
Expand Down
4 changes: 4 additions & 0 deletions auth0/src/main/java/com/auth0/android/provider/WebAuthProvider.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ public object WebAuthProvider {
}

internal fun failure(exception: AuthenticationException) {
if (managerInstance == null) {
Log.w(TAG, "There is no previous instance of this provider.")
return
}
managerInstance!!.failure(exception)
}

Expand Down
8 changes: 8 additions & 0 deletions auth0/src/test/java/com/auth0/android/provider/AuthenticationActivityMock.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ package com.auth0.android.provider

import android.content.Context
import android.content.Intent
import com.auth0.android.authentication.AuthenticationException

public class AuthenticationActivityMock : AuthenticationActivity() {
internal var customTabsController: CustomTabsController? = null
public var deliveredIntent: Intent? = null
private set
public var deliveredException: AuthenticationException? = null
private set

override fun createCustomTabsController(
context: Context,
Expand All @@ -19,4 +22,9 @@ public class AuthenticationActivityMock : AuthenticationActivity() {
deliveredIntent = result
super.deliverAuthenticationResult(result)
}

override fun deliverAuthenticationFailure(ex: AuthenticationException) {
deliveredException = ex
super.deliverAuthenticationFailure(ex)
}
}
72 changes: 68 additions & 4 deletions auth0/src/test/java/com/auth0/android/provider/AuthenticationActivityTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ 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.AuthenticationActivity.Companion.EXTRA_AUTHORIZE_URI
import com.auth0.android.provider.AuthenticationActivity.Companion.EXTRA_CT_OPTIONS
import com.auth0.android.provider.AuthenticationActivity.Companion.EXTRA_LAUNCH_AS_TWA
import com.auth0.android.provider.CustomTabsOptions
import com.nhaarman.mockitokotlin2.any
import org.hamcrest.CoreMatchers
Expand Down Expand Up @@ -92,7 +95,12 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), any(), failureCallbackCaptor.capture())
Mockito.verify(customTabsController).launchUri(
uriCaptor.capture(),
launchAsTwaCaptor.capture(),
any(),
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 @@ -123,7 +131,12 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), any(), failureCallbackCaptor.capture())
Mockito.verify(customTabsController).launchUri(
uriCaptor.capture(),
launchAsTwaCaptor.capture(),
any(),
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 @@ -154,7 +167,12 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), any(), failureCallbackCaptor.capture())
Mockito.verify(customTabsController).launchUri(
uriCaptor.capture(),
launchAsTwaCaptor.capture(),
any(),
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 @@ -184,7 +202,12 @@ public class AuthenticationActivityTest {
createActivity(intentCaptor.value)
activityController.create().start().resume()
Mockito.verify(customTabsController).bindService()
Mockito.verify(customTabsController).launchUri(uriCaptor.capture(), launchAsTwaCaptor.capture(), any(), failureCallbackCaptor.capture())
Mockito.verify(customTabsController).launchUri(
uriCaptor.capture(),
launchAsTwaCaptor.capture(),
any(),
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 @@ -243,6 +266,47 @@ public class AuthenticationActivityTest {
MatcherAssert.assertThat(controller, Is.`is`(Matchers.notNullValue()))
}


@Test
public fun shouldThrowExceptionIfAuthorizeUriIsNullInIntent() {
AuthenticationActivity.authenticateUsingBrowser(
callerActivity,
uri,
false,
customTabsOptions
)
Mockito.verify(callerActivity).startActivity(intentCaptor.capture())
createActivity(intentCaptor.value)
activity.intent = Intent().apply {
putExtra(EXTRA_LAUNCH_AS_TWA, false)
putExtra(EXTRA_CT_OPTIONS, customTabsOptions)
}
activityController.create().start().resume()
MatcherAssert.assertThat(activity.deliveredException, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(activity.isFinishing, Is.`is`(true))
activityController.destroy()
}

@Test
public fun shouldThrowExceptionIfCustomTabsIsNullInIntent() {
AuthenticationActivity.authenticateUsingBrowser(
callerActivity,
uri,
false,
customTabsOptions
)
Mockito.verify(callerActivity).startActivity(intentCaptor.capture())
createActivity(intentCaptor.value)
activity.intent = Intent().apply {
putExtra(EXTRA_LAUNCH_AS_TWA, false)
putExtra(EXTRA_AUTHORIZE_URI, uri)
}
activityController.create().start().resume()
MatcherAssert.assertThat(activity.deliveredException, Is.`is`(Matchers.notNullValue()))
MatcherAssert.assertThat(activity.isFinishing, Is.`is`(true))
activityController.destroy()
}

private fun recreateAndCallNewIntent(data: Intent) {
val outState = Bundle()
activityController.saveInstanceState(outState)
Expand Down

0 comments on commit 59fad7e

Please sign in to comment.