Missing fields added to AuthResult

[mr-kew]

trafficstars

I added all the missing fields to AuthResult type and also made it's constructor public, so it can be used to pass data from platform specific methods back to commonMain.

Example:

I wanna use signInWithProvider which is available only in platformMain. It will probably never be available in commonMain because of the activity parameter. So I make a method for it. It takes commonMain type OAuthProvider.

Before this PR:

androidMain:

suspend fun main(activity: Activity) {
    val provider = getOAuthProvider()
    val result = Firebase.auth.signInWithProvider(activity, provider)
    processResult(result)
}

// This method cannot be moved into commonMain because of the [activity] parameter, but it should have commonMain parameters and return types, so the surrounding code can be moved into commonMain.
@Suppress("INVISIBLE_MEMBER", "INVISIBlE_REFERENCE") // This allows access to internal constructor and is bad
suspend fun FirebaseAuth.signInWithProvider(activity: Activity, provider: OAuthProvider) = suspendCoroutine { continuation ->
    this.android.startActivityForSignInWithProvider(activity, provider.android)
        .addOnSuccessListener { result ->
            val commonResult = AuthResult(result) // This is internal
            if (commonResult.user != null) {
                continuation.resume(commonResult)
            } else {
                continuation.resumeWithException(IllegalStateException())
            }
        }
        .addOnFailureListener {
            continuation.resumeWithException(it)
        }
}

commonMain:

// This code is the same for both/all platforms, so it makes sense to be in commonMain

fun getOAuthProvider() = ...

fun processResult(result: AuthResult) = ...

Issues

1. JVM version does not provide some fields at all. I used throw NotImplementedError() to communicate that in commonMain.

2. I have a problem with testing this. Can anyone give me some advice? I should verify whether all the fields are actually present in js, but I don't really know how to test that. With iOS and android it should be fine as the result is properly typed.

[nbransby]

Thanks for this. Issue 1 is fine. Issue 2 you can just write a test to get the new fields you've added and assert they are the values you expect, the test will run on all platforms and will fail on JS if you have got a property name wrong

[mr-kew]

@nbransby

Thanks for this. Issue 1 is fine. Issue 2 you can just write a test to get the new fields you've added and assert they are the values you expect, the test will run on all platforms and will fail on JS if you have got a property name wrong

Well the problem is that I don't really know what values to expect from the testing FirebaseApp. Also should the tests be running on my device? They are all crashing on FirebaseNetworkException for me. I never worked with kmp js and dont have much experience with this library either, so I really have no idea what to do.

[nbransby]

Ok well commit the test, let the CI run it and we'll take it from there

[nbransby]

@mr-kew you can review the test reports now to see what is failing, also maybe add some messages to the assert calls so you know which ones are failing. Also the lint is failing too

[mr-kew]

@nbransby Hm, I have no idea why the lint fails. It fails in jvmMain, but I really didn't do many changes there. It's mostly empty computed properties with throw NotImplementedError() inside.

The tests are failing on the first assert for credential. I think the problem is that all of the added fields are optional and therefore doesn't have to be provided by the firebase in all cases. I think modifying the testing FirebaseApp so it actually returns all the additional info, will be needed.

[nbransby]

scroll up and it tells you what the issue are:

/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:132:17: Lint error > [standard:statement-wrapping] Missing newline after '{'
> Task :firebase-firestore:lintKotlinAndroidUnitTest
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:132:45: Lint error > [standard:statement-wrapping] Missing newline before '}'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:134:17: Lint error > [standard:statement-wrapping] Missing newline after '{'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:134:45: Lint error > [standard:statement-wrapping] Missing newline before '}'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:139:17: Lint error > [standard:statement-wrapping] Missing newline after '{'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:139:45: Lint error > [standard:statement-wrapping] Missing newline before '}'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:141:17: Lint error > [standard:statement-wrapping] Missing newline after '{'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:141:45: Lint error > [standard:statement-wrapping] Missing newline before '}'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:143:17: Lint error > [standard:statement-wrapping] Missing newline after '{'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:143:45: Lint error > [standard:statement-wrapping] Missing newline before '}'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:145:17: Lint error > [standard:statement-wrapping] Missing newline after '{'
/home/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/src/jvmMain/kotlin/dev/gitlive/firebase/auth/auth.kt:145:45: Lint error > [standard:statement-wrapping] Missing newline before '}'

[nbransby]

I think modifying the testing FirebaseApp so it actually returns all the additional info, will be needed.

How do we do this?

[mr-kew]

I think modifying the testing FirebaseApp so it actually returns all the additional info, will be needed.

How do we do this?

I have no idea, I was hoping you would help me with that. It seems the tests are accessing some regular firebase project, but I don't have a permissions (and don't really want them) to do anything with that.

[nbransby]

Yes I can edit it just not sure what you need doing

[mr-kew]

Yes I can edit it just not sure what you need doing

Hmm, I'm really not entirely sure either. In my project, the additional info & credential come from the SSO. To test it we are using Okta, which then after signInWithProvider returns given additional info. I did not set it up, so I sadly don't know much more about it.

From the Firebase docs it looks like the additional info is indeed not null only while using the signInWithProvider. I was hoping it could be done using regular email+password sign in, but sadly it seems not, so you would have to setup something like that.

Additional issue is that the okta sign in redirects to okta sign in form in browser, so idk if it is possible in unit tests without running the app.

[nbransby]

Ok well for now I guess we can simply update the test to confirm the values are null - that will at least assert no exceptions are thrown

[mr-kew]

Yeah, that makes sense. If someone discovers it's not working later, it can be solved as a separate issue.

[mr-kew]

@nbransby Android & JVM tests are now passing, but iOS and JS does not show my assert messages so I don't know how exactly is the test failing. Can I somehow access the file with results?

There were failing tests. See the report at: file:///Users/runner/work/firebase-kotlin-sdk/firebase-kotlin-sdk/firebase-auth/build/reports/tests/iosSimulatorArm64Test/index.html

[nbransby]

You can find the test reports here: https://github.com/GitLiveApp/firebase-kotlin-sdk/actions/runs/9940703433?pr=556#artifacts

[nbransby]

JS tests still falling with FirebaseAuthException: app/app-deleted: Firebase: Firebase App named '[DEFAULT]' already deleted (app/app-deleted).

[mr-kew]

@nbransby Yeah, it might be JSON, nice find.

But from the exception it seems like there is an issue with FirebaseApp initialization in the tests. So the test itself does not even run, I think. So I still can't verify it.

[mr-kew]

@nbransby So I fixed the js tests (it was about the position of the test code in a file), but I had to make them more forgiving because in JS the additionalUserInfo is not provided at all, so I cannot test it's properties.

But I think it is fine

If someone discovers it's not working later, it can be solved as a separate issue.

[nbransby]

Ok, btw there are conflicts you'll need to resolve before we can merge it in

[mr-kew]

@nbransby Really thank you for all the help. It was quite complicated and I wouldn't have managed it all on my own.

[nbransby]

No problem, it was 99% all you!