Inconsistency in Handling App Secrets Between resValue and Secrets Plugin

[dzuluaga]

I've noticed an inconsistency in the way our sample app handles app secrets, specifically how we're using resValue for reading FACEBOOK_APP_ID, FACEBOOK_CLIENT_TOKEN, MICROSOFT_CLIENT_ID, etc., while simultaneously using the secrets plugin for the GOOGLE_CLIENT_ID. This approach seems to diverge in methodology for handling similar types of secrets, which could potentially lead to confusion and maintenance issues down the line.

In the context of our project's goal to maintain consistency and ease of maintainability, I believe it would be beneficial to standardize the method we use for retrieving these variables. My suggestion aligns with the comments I've made in the pull request here: Pull Request #23 Comment.

To summarize my recommendation from the PR comment:

• We should adopt a single, consistent approach for managing app secrets, ideally through the local.properties file for local development, which can then be easily overridden by CI for production builds.
• This method should apply uniformly to all external service client IDs and tokens (including GOOGLE_CLIENT_ID, FACEBOOK_APP_ID, FACEBOOK_CLIENT_TOKEN, MICROSOFT_CLIENT_ID, etc.).
• By doing so, we'll streamline the developer experience by providing a approach to developers adopting the libraries.

I believe that adopting a uniform approach will not only simplify the codebase but also make it easier for new contributors to understand and for existing contributors to maintain. It would be great to hear your thoughts on this and discuss potential paths forward.

Thank you for considering my feedback.

[andrei-zgirvaci]

Thanks @dzuluaga for bringing this to our attention!

While integrating Google Secrets plugin might sound optimal at first. However, after partially trying to integrate it into the project, it became clear to us that it's quite limited in it's functionality.

For instance, beside injecting FACEBOOK_APP_ID and FACEBOOK_CLIENT_TOKEN values into the AndroidManifest.xml file, we also need to specify some additional values like: android:path="@string/microsoft_path" android:scheme="@string/fb_login_protocol_scheme" and android:scheme="@string/db_login_protocol_scheme" for the Facebook, Microsoft and Dropbox plugins to work properly. Unfortunately Google Secrets Plugin only supports injecting values into android:value="".

Moreover, we need to access the MICROSOFT_CLIENT_ID and MICROSOFT_SIGNATURE_HASH values inside the build.gradle file for additional configuration. As @artus9033 mentioned, Google Secrets plugin is limited and won't allow us to access those values through resString which instead we will have to manually read from the local.properties file like we are doing currently.

We might be able to get rid of 2 lines of code for injecting the FACEBOOK_APP_ID and FACEBOOK_CLIENT_TOKEN values automatically using the Google Secrets Plugin but with the cost of making the documentation less clear as using this plugin is not common among Android developers so we would have to explain the magic that it does behind and also it's limitations.

Ultimately, we will have an AndroidManifest.xml file that both injects values from the Google Secrets Plugin and String resources. In my opinion this creates unnecessary complications and confusion with some potential code minimizations that are not worth it.

Happy to hear your thoughts!

[dzuluaga]

Thanks, Andrei. Please see my inline comments. If it's easier I can jump on a call to discuss further.

For instance, beside injecting FACEBOOK_APP_ID and FACEBOOK_CLIENT_TOKEN values into the AndroidManifest.xml file, we also need to specify some additional values like: android:path="@string/microsoft_path" android:scheme="@string/fb_login_protocol_scheme" and android:scheme="@string/db_login_protocol_scheme" for the Facebook, Microsoft and Dropbox plugins to work properly. Unfortunately Google Secrets Plugin only supports injecting values into android:value="".

I noticed the requirements for integrating the Facebook, Microsoft, and Dropbox plugins involve injecting FACEBOOK_APP_ID and FACEBOOK_CLIENT_TOKEN into the AndroidManifest.xml, along with specifying additional values like android:path and android:scheme for each plugin. It's mentioned that these additional values, such as microsoft_path and fb_login_protocol_scheme, are stored in local.properties, which is unconventional. Typically, non-secret configurations are placed in the res/values/strings.xml file, simplifying management and access without extra code in the build.gradle.

Could you elaborate on the decision to store these non-secret arguments in local.properties? Is there a specific requirement or advantage to this approach?

Moreover, we need to access the MICROSOFT_CLIENT_ID and MICROSOFT_SIGNATURE_HASH values inside the build.gradle file for additional configuration. As @artus9033 openmobilehub/android-omh-maps#23 (comment), Google Secrets plugin is limited and won't allow us to access those values through resString which instead we will have to manually read from the local.properties file like we are doing currently.

Thanks for highlighting the limitations with the Google Secrets plugin and our current method for accessing MICROSOFT_CLIENT_ID and MICROSOFT_SIGNATURE_HASH. I propose a simpler approach: instead of generating the ms_auth_config.json dynamically, we could guide developers to manually create this file, as detailed in our readme.md. This method removes the need to fetch these values in the build.gradle file and utilizes the MSAL SDK's ability to read the ms_auth_config.json directly, aligning with the MSAL documentation. This should make the setup process more straightforward and our build scripts less complex.

I'm keen to hear your thoughts on this suggestion or if there are any alternatives or concerns I might have missed.

We might be able to get rid of 2 lines of code for injecting the FACEBOOK_APP_ID and FACEBOOK_CLIENT_TOKEN values automatically using the Google Secrets Plugin but with the cost of making the documentation less clear as using this plugin is not common among Android developers so we would have to explain the magic that it does behind and also it's limitations.

Ultimately, we will have an AndroidManifest.xml file that both injects values from the Google Secrets Plugin and String resources. In my opinion this creates unnecessary complications and confusion with some potential code minimizations that are not worth it.

Happy to hear your thoughts!

[andrei-zgirvaci]

Hey @dzuluaga, indeed, inline comments seem to clarify things a little bit better.

I noticed the requirements for integrating the Facebook, Microsoft, and Dropbox plugins involve injecting FACEBOOK_APP_ID and FACEBOOK_CLIENT_TOKEN into the AndroidManifest.xml, along with specifying additional values like android:path and android:scheme for each plugin. It's mentioned that these additional values, such as microsoft_path and fb_login_protocol_scheme, are stored in local.properties, which is unconventional. Typically, non-secret configurations are placed in the res/values/strings.xml file, simplifying management and access without extra code in the build.gradle.

Just to clarify, the microsoft_path and fb_login_protocol_scheme are NOT stored in the local.properties file. They are being created in the build.gradle and injected into the res/values/strings.xml file during build time using the resValue function.

In the local.properties file, we only store our secrets which we don't want to have in our repo. They are also being used for our GitHub build actions.

Thanks for highlighting the limitations with the Google Secrets plugin and our current method for accessing MICROSOFT_CLIENT_ID and MICROSOFT_SIGNATURE_HASH. I propose a simpler approach: instead of generating the ms_auth_config.json dynamically, we could guide developers to manually create this file, as detailed in our readme.md. This method removes the need to fetch these values in the build.gradle file and utilizes the MSAL SDK's ability to read the ms_auth_config.json directly, aligning with the MSAL documentation. This should make the setup process more straightforward and our build scripts less complex.

Initially we were using the ms_auth_config.json file to initialize the MSAL SDK, however we decided to generate this file during the build process instead, in order to be able to run the GitHub build actions with only referencing the secret values from GitHub.

Another approach we can take is to try to generate the ms_auth_config.json file during the GitHub build action, however this might be a little bit tricky and we would need to investigate this approach a little bit more.

Happy to hear your thoughts on this!

[dzuluaga]

Just to clarify, the microsoft_path and fb_login_protocol_scheme are NOT stored in the local.properties file. They are being created in the build.gradle and injected into the res/values/strings.xml file during build time using the resValue function.

Let's stick to the official guidelines for integrating values like facebook_app_id and fb_login_protocol_scheme, and so on directly into strings.xml, as recommended by the FB Login Quickstart guide. Injecting these values through build scripts complicates things unnecessarily. Unless there's a specific reason we're not following the standard approach, we should align with the official documentation. This will make our process more straightforward and in line with expectations from the framework's docs, avoiding over-reliance on build script configurations.

Another approach we can take is to try to generate the ms_auth_config.json file during the GitHub build action, however this might be a little bit tricky and we would need to investigate this approach a little bit more.

I agree that generating the ms_auth_config.json file should be part of the GitHub Actions workflow rather than the build script. This approach aligns better with its role in the process. Below is a concise example of how we can automate the creation of this file during our GitHub build process:

name: Build and Deploy

on: [push, pull_request]

jobs:
  build:
    runs-on: ubuntu-latest

    steps:
    - uses: actions/checkout@v2

    # Generate ms_auth_config.json
    - name: Generate MS Auth Config
      run: |
        cat <<EOF > ms_auth_config.json
        {
          "microsoft_client_id": "${{ secrets.MICROSOFT_CLIENT_ID }}"
        }
        EOF
      shell: bash

    # Use the generated file in your Gradle build
    - name: Build with Gradle
      run: ./gradlew build

[adamTrz]

Hi @dzuluaga, as for the facebook_app_id, fb_login_protocol_scheme and facebook_client_token we are guiding users to store them in strings.xml file just like you've proposed and this is included in Readme docs here:

https://github.com/openmobilehub/android-omh-auth/blob/main/packages/plugin-facebook/README.md#edit-your-resources-and-manifest

same goes for ms_auth_config.json file - it should be placed under /app/res/raw directory as described in MS Plugin Readme here:
https://github.com/openmobilehub/android-omh-auth/blob/main/packages/plugin-microsoft/README.md#setting-the-signature-hash

So those are the docs that users consuming our packages should use and that's how we guid them.

Only exception is our Sample App - since we want to build working application on CD pipeline we need to handle those secrets in different way - eg. inject them from GH Secrets first from setup_secrets.sh script and only then we can use those in gradle build files.

Happy to hear your thoughts on this!