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.

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

Add Kotlin extensions #2066

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

denis-ismailaj
Copy link

@denis-ismailaj denis-ismailaj commented Mar 23, 2024

This PR adds some wrapper methods that allow callers to write idiomatic Kotlin code when using this library.

Examples of the usage differences are highlighted below.

Creating a PublicClientApplication

PublicClientApplication.createSingleAccountPublicClientApplication(
    /* context = */ context,
    /* configFileResourceId = */ configFileResourceId,
    /* listener = */ object : ISingleAccountApplicationCreatedListener {
        override fun onCreated(application: ISingleAccountPublicClientApplication?) {
            // Use application here
        }

        override fun onError(exception: MsalException) {
            showException(exception)
        }
    }
)

can become a suspending function

scope.launch {
    try {
        val app = PublicClientApplicationKtx.createSingleAccountPublicClientApplication(
            context = context,
            configFileResourceId = R.raw.auth_config_single_account,
        )

        // Use application here

    } catch (exception: Exception) {
        showException(exception)
    }
}

The same thing applies to createMultipleAccountPublicClientApplication.

[Single Account] Signing in

val signInParameters: SignInParameters = SignInParameters.builder()
    .withActivity(activity)
    .withScopes(scopes)
    .withCallback(object : AuthenticationCallback {
        override fun onSuccess(authenticationResult: IAuthenticationResult) {
            // Use result here
        }

        override fun onError(exception: MsalException) {
            showException(exception)
        }

        override fun onCancel() {
        }
    })
    .build()

app.signIn(signInParameters)

Usage of builders is not as common in Kotlin unless they need to perform complex operations. In this case, the builder for SignInParameters merely sets values, so we can just use signIn directly with named parameters while still keeping the proper defaults for the other unspecified ones.

app.signIn(
    activity = activity,
    scopes = scopes,
    callback = object : AuthenticationCallback {
        override fun onSuccess(authenticationResult: IAuthenticationResult) {
            // Use result here
        }

        override fun onError(exception: MsalException) {
            showException(exception)
        }

        override fun onCancel() {
        }
    },
)

Furthermore, this can also become a suspending function

scope.launch {
    try {
        val authResult = app.signIn(
            activity = activity,
            scopes = scopes,
        )

        // Use result here

    } catch (exception: MsalException) {
        showException(exception)
    }
}

In this scenario, authResult would be null in the onCancel case.

I think this is a reasonable default for a lot of applications, however, when user cancellation needs to be handled explicitly, developers can still opt to use the callback.

[Single Account] Signing out

app.signOut(object : SignOutCallback {
    override fun onSignOut() {
        // handle cleanup
    }

    override fun onError(exception: MsalException) {
        showException(exception)
    }
})

can become a suspending function

scope.launch {
    try {
        app.signOutSuspend()

        // handle cleanup

    } catch (exception: MsalException) {
        showException(exception)
    }
}

[Single Account] Get signed-in account

app.getCurrentAccountAsync(object : CurrentAccountCallback {
    override fun onAccountLoaded(activeAccount: IAccount?) {
        // use account here
    }

    override fun onAccountChanged(priorAccount: IAccount?, currentAccount: IAccount?) {
    }

    override fun onError(exception: MsalException) {
        showException(exception)
    }
})

can become a suspending function

scope.launch {
    try {
        val account = app.getCurrentAccountSuspend()

        // use account here

    } catch (exception: MsalException) {
        showException(exception)
    }
}

In this scenario, onAccountChanged is ignored, but again, if an application needs to handle that in a special way, it can still use the callback.

[Multiple Account] Get signed-in accounts

app.getAccounts(object : LoadAccountsCallback {
    override fun onTaskCompleted(result: List<IAccount>?) {
        // use accounts here
    }

    override fun onError(exception: MsalException) {
        showException(exception)
    }
})

can become a suspending function

scope.launch {
    try {
        val accounts = app.getAccountsSuspend()

        // use accounts here

    } catch (exception: MsalException) {
        showException(exception)
    }
}

[Multiple Account] Remove account

app.removeAccount(account, object : RemoveAccountCallback {
    override fun onRemoved() {
        // handle cleanup
    }

    override fun onError(exception: MsalException) {
        showException(exception)
    }
})

can become a suspending function

scope.launch {
    try {
        app.removeAccountSuspend(account)

        // handle cleanup

    } catch (exception: MsalException) {
        showException(exception)
    }
}

Acquire a token

val parameters = AcquireTokenParameters.Builder()
   .startAuthorizationFromActivity(activity)
   .withScopes(scopes)
   .withCallback(object : AuthenticationCallback {
       override fun onSuccess(authenticationResult: IAuthenticationResult) {
           // use result here
       }

       override fun onError(exception: MsalException) {
           showException(exception)
       }

       override fun onCancel() {
       }
   })
   .forAccount(account)
   .build()

app.acquireToken(parameters)

can become a suspending function

val parameters = AcquireTokenParameters.Builder()
   .startAuthorizationFromActivity(activity)
   .withScopes(scopes)
   .forAccount(account)
   .build()

scope.launch {
    try {
       app.acquireTokenSuspend(parameters)

       // use result here

    } catch (exception: MsalException) {
       showException(exception)
    }
}

We can also get rid of the builder here as well.
The only builder method that has multiple overloads is fromAuthority, which we can replace by having a separate Authority class with multiple constructors.

scope.launch {
    try {
       app.acquireTokenSuspend(
           activity = activity,
           scopes = scopes,
           account = account,
           authority = Authority.from(<any of the overloads>),
       )

       // use result here

    } catch (exception: MsalException) {
       showException(exception)
    }
}

The same thing applies to acquireTokenSilent.


Throughout these examples I've added scope.launch and error handling for completeness, but in practice the difference is even more noticeable because often these functions will be called from another suspend function so you don't need to launch a new coroutine scope, or you may be handling errors at a higher level so you don't need to handle every one separately.

@denis-ismailaj denis-ismailaj requested a review from a team as a code owner March 23, 2024 17:54
@denis-ismailaj
Copy link
Author

@microsoft-github-policy-service agree

@SammyO
Copy link
Contributor

SammyO commented Apr 16, 2024

@denis-ismailaj thanks for this PR and your feedback. This is a good proposal, but it's done in a slightly different way than the approach we've taken in some new MSAL interface methods. Could I ask you to take a look at class NativeAuthPublicClientApplication and let me know what you think? We can discuss it offline and go through some of the details together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants