Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use ClientException instead of try catch block in sample app #24

Merged
merged 12 commits into from
Jun 14, 2024
Merged

Use ClientException instead of try catch block in sample app #24

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1e75f27
Remove try catch block. Instead use ClientException
Yuki-YuXin Apr 16, 2024
02ed65a
update displayAccount()
Yuki-YuXin Apr 17, 2024
ecb76ae
use errorMessage instead of exception?.message
Yuki-YuXin Apr 17, 2024
077bc17
Remove kotlin-android-extensions deprecated dependency
Yuki-YuXin Apr 17, 2024
9d334c0
Remove ClientExceptionError from when is branch. Use error.exception?…
Yuki-YuXin Apr 30, 2024
379f269
Update the sample app
Yuki-YuXin May 9, 2024
7006b2d
Revert unintended changes
Yuki-YuXin May 14, 2024
e74502c
Use accountResult.errorMessage to simplify
Yuki-YuXin May 15, 2024
4eb7e2d
Use accountResult.errorMessage to simplify
Yuki-YuXin May 15, 2024
524c3a2
Merge branch 'main' into yuki/wrap-exception
Yuki-YuXin May 15, 2024
baf7625
Merge branch 'main' into yuki/wrap-exception
Yuki-YuXin Jun 12, 2024
ebcc1c4
Remove try finally block
Yuki-YuXin Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ apply plugin: 'com.android.application'

apply plugin: 'kotlin-android'

apply plugin: 'kotlin-android-extensions'
rmccahill marked this conversation as resolved.
Show resolved Hide resolved

allprojects {
repositories {
maven {
Expand Down
84 changes: 46 additions & 38 deletions app/src/main/java/com/azuresamples/msalnativeauthandroidkotlinsampleapp/AccessApiFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import android.widget.Toast
import androidx.appcompat.app.AppCompatActivity
import androidx.fragment.app.Fragment
import com.azuresamples.msalnativeauthandroidkotlinsampleapp.databinding.FragmentAccessApiBinding
import com.microsoft.identity.client.exception.MsalException
import com.microsoft.identity.common.java.util.StringUtil
import com.microsoft.identity.nativeauth.INativeAuthPublicClientApplication
import com.microsoft.identity.nativeauth.statemachine.errors.GetAccessTokenError
import com.microsoft.identity.nativeauth.statemachine.errors.GetAccountError
import com.microsoft.identity.nativeauth.statemachine.errors.SignInError
import com.microsoft.identity.nativeauth.statemachine.results.GetAccessTokenResult
import com.microsoft.identity.nativeauth.statemachine.results.GetAccountResult
Expand Down Expand Up @@ -80,47 +81,46 @@ class AccessApiFragment : Fragment() {
is GetAccountResult.NoAccountFound -> {
displaySignedOutState()
}
is GetAccountError -> {
displayDialog(getString(R.string.msal_exception_title), accountResult.exception?.message ?: accountResult.errorMessage)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For simplicity, let's just use accountResult.errorMessage here. That's the ultimate error. For more details on the error, developers can look into the exception's message. What do you think?
If you agree, this comment applies to multiple places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Will make the update

}
}
}
}

private fun signIn() {
CoroutineScope(Dispatchers.Main).launch {
val email = binding.emailText.text.toString()
val password = CharArray(binding.passwordText.length())
binding.passwordText.text?.getChars(0, binding.passwordText.length(), password, 0)

val actionResult: SignInResult
try {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we still need this try? Also, a try without a catch?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applies to multiple places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's because there is a try-finally block which has relation with the password security/privacy fix completed by Burak before.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the point of the try-finally now, but I don't think we need a try at all now anymore. The SDK shouldn't throw an exception, so developers don't need to expect it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed try finally block

val email = binding.emailText.text.toString()
val password = CharArray(binding.passwordText.length())
binding.passwordText.text?.getChars(0, binding.passwordText.length(), password, 0)

val actionResult: SignInResult
try {
actionResult = authClient.signIn(
username = email,
password = password,
scopes = scopes
)
} finally {
binding.passwordText.text?.clear()
StringUtil.overwriteWithNull(password)
}
actionResult = authClient.signIn(
username = email,
password = password,
scopes = scopes
)
} finally {
binding.passwordText.text?.clear()
StringUtil.overwriteWithNull(password)
}

when (actionResult) {
is SignInResult.Complete -> {
Toast.makeText(
requireContext(),
getString(R.string.sign_in_successful_message),
Toast.LENGTH_SHORT
).show()
displaySignedInState(accountState = actionResult.resultValue)
}
is SignInResult.CodeRequired -> {
displayDialog(message = getString(R.string.sign_in_switch_to_otp_message))
}
is SignInError -> {
handleSignInError(actionResult)
}
when (actionResult) {
is SignInResult.Complete -> {
Toast.makeText(
requireContext(),
getString(R.string.sign_in_successful_message),
Toast.LENGTH_SHORT
).show()
displaySignedInState(accountState = actionResult.resultValue)
}
is SignInResult.CodeRequired -> {
displayDialog(message = getString(R.string.sign_in_switch_to_otp_message))
}
is SignInError -> {
handleSignInError(actionResult)
}
} catch (exception: MsalException) {
displayDialog(getString(R.string.msal_exception_title), exception.message.toString())
}
}
}
Expand Down Expand Up @@ -169,6 +169,9 @@ class AccessApiFragment : Fragment() {
is GetAccountResult.NoAccountFound -> {
displaySignedOutState()
}
is GetAccountError -> {
displayDialog(getString(R.string.msal_exception_title), accountResult.exception?.message ?: accountResult.errorMessage)
}
}
}
}
Expand All @@ -180,7 +183,7 @@ class AccessApiFragment : Fragment() {
}
else -> {
// Unexpected error
displayDialog(getString(R.string.unexpected_sdk_error_title), error.toString())
displayDialog(getString(R.string.unexpected_sdk_error_title), error.exception?.message ?: error.errorMessage)
}
}
}
Expand Down Expand Up @@ -216,10 +219,15 @@ class AccessApiFragment : Fragment() {

private fun displayAccount(accountState: AccountState) {
CoroutineScope(Dispatchers.Main).launch {
val accessTokenState = accountState.getAccessToken()
if (accessTokenState is GetAccessTokenResult.Complete) {
val accessToken = accessTokenState.resultValue.accessToken
binding.resultText.text = accessToken
val accessTokenResult = accountState.getAccessToken()
when (accessTokenResult) {
is GetAccessTokenResult.Complete -> {
val accessToken = accessTokenResult.resultValue.accessToken
binding.resultText.text = getString(R.string.result_access_token_text) + accessToken
}
is GetAccessTokenError -> {
displayDialog(getString(R.string.msal_exception_title), accessTokenResult.exception?.message ?: accessTokenResult.errorMessage)
}
}
}
}
Expand Down
118 changes: 56 additions & 62 deletions ...ava/com/azuresamples/msalnativeauthandroidkotlinsampleapp/EmailAttributeSignUpFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@ import android.widget.Toast
import androidx.core.text.set
import androidx.fragment.app.Fragment
import com.azuresamples.msalnativeauthandroidkotlinsampleapp.databinding.FragmentEmailAttributeBinding
import com.microsoft.identity.client.exception.MsalException
import com.microsoft.identity.common.java.util.StringUtil
import com.microsoft.identity.nativeauth.INativeAuthPublicClientApplication
import com.microsoft.identity.nativeauth.UserAttributes
import com.microsoft.identity.nativeauth.statemachine.errors.GetAccessTokenError
import com.microsoft.identity.nativeauth.statemachine.errors.GetAccountError
import com.microsoft.identity.nativeauth.statemachine.errors.SignInContinuationError
import com.microsoft.identity.nativeauth.statemachine.errors.SignInError
import com.microsoft.identity.nativeauth.statemachine.errors.SignUpError
import com.microsoft.identity.nativeauth.statemachine.results.GetAccessTokenResult
import com.microsoft.identity.nativeauth.statemachine.results.GetAccountResult
Expand Down Expand Up @@ -83,61 +82,60 @@ class EmailAttributeSignUpFragment : Fragment() {
is GetAccountResult.NoAccountFound -> {
displaySignedOutState()
}
is GetAccountError -> {
displayDialog(getString(R.string.msal_exception_title), accountResult.exception?.message ?: accountResult.errorMessage)
}
}
}
}

private fun signUp() {
CoroutineScope(Dispatchers.Main).launch {
try {
val email = binding.emailText.text.toString()
val password = CharArray(binding.passwordText.length())
binding.passwordText.text?.getChars(0, binding.passwordText.length(), password, 0)
val country = binding.countryText.text.toString()
val city = binding.cityText.text.toString()
val email = binding.emailText.text.toString()
val password = CharArray(binding.passwordText.length())
binding.passwordText.text?.getChars(0, binding.passwordText.length(), password, 0)
val country = binding.countryText.text.toString()
val city = binding.cityText.text.toString()

val attributes = UserAttributes.Builder
.country(country)
.city(city)
.build()
val attributes = UserAttributes.Builder
.country(country)
.city(city)
.build()

val actionResult: SignUpResult
try {
actionResult = authClient.signUp(
username = email,
password = password,
attributes = attributes
)
} finally {
binding.passwordText.text?.set(0, binding.passwordText.text?.length?.minus(1) ?: 0, 0)
StringUtil.overwriteWithNull(password)
}

val actionResult: SignUpResult
try {
actionResult = authClient.signUp(
username = email,
password = password,
attributes = attributes
when (actionResult) {
is SignUpResult.CodeRequired -> {
navigateToSignUp(
nextState = actionResult.nextState
)
} finally {
binding.passwordText.text?.set(0, binding.passwordText.text?.length?.minus(1) ?: 0, 0)
StringUtil.overwriteWithNull(password)
}

when (actionResult) {
is SignUpResult.CodeRequired -> {
navigateToSignUp(
nextState = actionResult.nextState
)
}
is SignUpResult.Complete -> {
Toast.makeText(
requireContext(),
getString(R.string.sign_up_successful_message),
Toast.LENGTH_SHORT
).show()
signInAfterSignUp(
nextState = actionResult.nextState
)
}
is SignUpError -> {
handleSignUpError(actionResult)
}
is SignUpResult.AttributesRequired -> {
displayDialog(getString(R.string.unexpected_sdk_result_title), actionResult.toString())
}
is SignUpResult.Complete -> {
Toast.makeText(
requireContext(),
getString(R.string.sign_up_successful_message),
Toast.LENGTH_SHORT
).show()
signInAfterSignUp(
nextState = actionResult.nextState
)
}
is SignUpError -> {
handleSignUpError(actionResult)
}
is SignUpResult.AttributesRequired -> {
displayDialog(getString(R.string.unexpected_sdk_result_title), actionResult.toString())
}
} catch (exception: MsalException) {
displayDialog(getString(R.string.msal_exception_title), exception.message.toString())
}
}
}
Expand All @@ -156,7 +154,7 @@ class EmailAttributeSignUpFragment : Fragment() {
displaySignedInState(accountState = actionResult.resultValue)
}
is SignInContinuationError -> {
displayDialog(getString(R.string.unexpected_sdk_error_title), actionResult.toString())
displayDialog(getString(R.string.unexpected_sdk_error_title), actionResult.exception?.message ?: actionResult.errorMessage)
}
else -> {
displayDialog(getString(R.string.unexpected_sdk_result_title), actionResult.toString())
Expand Down Expand Up @@ -222,22 +220,18 @@ class EmailAttributeSignUpFragment : Fragment() {

private fun displayAccount(accountState: AccountState) {
CoroutineScope(Dispatchers.Main).launch {
try {
val accessTokenResult = accountState.getAccessToken()
when (accessTokenResult) {
is GetAccessTokenResult.Complete -> {
binding.resultAccessToken.text =
getString(R.string.result_access_token_text) + accessTokenResult.resultValue.accessToken
val accessTokenResult = accountState.getAccessToken()
when (accessTokenResult) {
is GetAccessTokenResult.Complete -> {
val accessToken = accessTokenResult.resultValue.accessToken
binding.resultAccessToken.text = getString(R.string.result_access_token_text) + accessToken

val idToken = accountState.getIdToken()
binding.resultIdToken.text = getString(R.string.result_id_token_text) + idToken
}
is GetAccessTokenError -> {
displayDialog(getString(R.string.msal_exception_title), accessTokenResult.exception?.message.toString())
}
val idToken = accountState.getIdToken()
binding.resultIdToken.text = getString(R.string.result_id_token_text) + idToken
}
is GetAccessTokenError -> {
displayDialog(getString(R.string.msal_exception_title), accessTokenResult.exception?.message ?: accessTokenResult.errorMessage)
}
} catch (exception: Exception) {
displayDialog(getString(R.string.msal_exception_title), exception.message.toString())
}
}
}
Expand All @@ -251,7 +245,7 @@ class EmailAttributeSignUpFragment : Fragment() {
}
else -> {
// Unexpected error
displayDialog(getString(R.string.unexpected_sdk_error_title), error.toString())
displayDialog(getString(R.string.unexpected_sdk_error_title), error.exception?.message ?: error.errorMessage)
}
}
}
Expand Down
Loading