Username checking in native auth requests

[Yuki-YuXin]

Put back ArgUtils.validateNonNullArg(username, "username") in all initial native auth requests.

Company PRs:
msal: AzureAD/microsoft-authentication-library-for-android#2080
native sample app: Azure-Samples/ms-identity-ciam-native-auth-android-sample#24

[SammyO]

why was this removed in the first place?

[Yuki-YuXin]

Not sure about the reason but I would prefer to keep the checking here.

[SammyO]

Wasn't this changed (i.e. the exception removed) as part of last year's work to align some of our errors and return an InvalidUsernameError in the case of an empty string? Giannis worked on that I believe.

[Yuki-YuXin]

Yes. I think so. But either CommandResultUtil.kt

if (this.status != ICommandResult.ResultStatus.COMPLETED) {
        var exception: Exception? = null
        var exceptionMessage: String? = ""

        if (this.result is Exception) {
            exception = this.result as Exception
            exceptionMessage = exception.message
        }

        return com.microsoft.identity.common.java.nativeauth.controllers.results.INativeAuthCommandResult.UnknownError(
            error = UNSUCCESSFUL_COMMAND_ERROR,
            errorDescription = exceptionMessage,
            exception = exception,
            correlationId = this.correlationId
        ) as ExpectedType

or NativeAuthPublicClientApplication.kt

return withContext(Dispatchers.IO) {
            try {
                verifyNoUserIsSignedIn()

                if (username.isBlank()) {
                    return@withContext SignUpError(
                        errorType = ErrorTypes.INVALID_USERNAME,
                        errorMessage = "Empty or blank username",
                        correlationId = "UNSET"
                    )
                }

can handle empty username.

I think the latter one act the main role of returning InvalidUsernameError before sending out the request/validating request parameters. Thus, I would prefer to keep the ThrowException here.

[SammyO]

Some thoughts:

• if we're already doing a username is empty check in the interface, we don't need to do it again in the API layer.
• we shouldn't just be throwing exceptions. An exception is a pretty heavy mechanism to give feedback to a developer. We should aim for something more graceful than that.
• the fact that this "throwing an exception" was removed from this API layer a few months ago, means we had a reason why. If that reason has changed, let's talk about it. We can just revert the work we did a few months ago without good reason.

[Yuki-YuXin]

Will abandon this PR.