openmobilehub / android-omh-auth

Apache License 2.0
4 stars 1 forks source link

Confusion Regarding `initialize()` Method in `authClientProvider` #101

Open dzuluaga opened 3 months ago

dzuluaga commented 3 months ago

The initialize() method in the authClientProvider seems to be redundant or confusing. Currently, it's possible to use the authClientProvider.getClient() method and proceed with authentication operations without explicitly calling initialize(). This raises the question: what is the purpose of the initialize() method if the client can function without it?

Steps to Reproduce:

  1. Set up the authClientProvider using the following code:

    val omhAuthClient = OmhAuthProvider.Builder()
        .addNonGmsPath("com.openmobilehub.android.auth.plugin.google.nongms.presentation.OmhAuthFactoryImpl")
        .addGmsPath("com.openmobilehub.android.auth.plugin.google.gms.OmhAuthFactoryImpl")
        .build()
        .provideAuthClient(
            context = applicationContext,
            scopes = listOf(
                "openid",
                "email",
                "profile",
                "https://www.googleapis.com/auth/drive",
                "https://www.googleapis.com/auth/drive.file"
            ),
            clientId = "XXXX"
        )
  2. Attempt to use the client without calling initialize():

    lifecycleScope.launch(Dispatchers.IO) {
        authClientProvider.getClient(requireContext())
            .addOnSuccess {
                setupUI()
            }
            .execute()
    }

Expected Behavior: If initialize() is necessary, the client should not function correctly without it. Alternatively, if it’s not necessary, it should be clarified when and why initialize() should be used.

Actual Behavior: The client appears to work fine without calling initialize(), which creates confusion about the necessity and role of the initialize() method.

Additional Context: This issue might lead to developers misunderstanding the API usage or possibly omitting initialize() without knowing its potential benefits or requirements.

adamTrz commented 1 month ago

Hi @dzuluaga Indeed initialize() method is not necessary for the packages to work. We simply have it so that users would be able to add additional steps like we can see on MS side here:

https://github.com/openmobilehub/android-omh-auth/blob/fa453848e4426dc0207b5caac3cce584dc1bc063/packages/plugin-microsoft/src/main/java/com/openmobilehub/android/auth/plugin/microsoft/MicrosoftAuthClient.kt#L26-L35

I've added bit more detailed explanation for this step in docs with this PR - https://github.com/openmobilehub/android-omh-auth/pull/109