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

build/authentication/fix-ci-build-apk : Fix secrets not properly applied in CI build apk #171

Merged
merged 7 commits into from
Nov 12, 2024

Conversation

charliemangano
Copy link
Contributor

@charliemangano charliemangano commented Nov 11, 2024

Fix secrets not properly applied in CI build apk

Description

This PR introduces a fix for issue #161. It uses the Secrets Gradle plugin to handle our app's secrets (API keys and client URLs). As explained in #161, does not fix the sync/double click issue.

Changes

I introduced the use of the Secrets Gradle plugin to handle our app's secrets.
How it works:
TL;DR: contact me to get the new secrets.properties and updated local.properties files (should not pushed to VCS!)

The local secrets.properties contains the necessary API keys and URLs, but it is not to be checked into our VCS (contact me to get it). Rather, its contents are also stored in GitHub Secrets. The new CI workflow ensures that these values are securely copied from that tab to construct a secrets.properties from a template (secrets.default.properties, checked into VCS), whose default values are replaced with the correct, securely stored ones by the sed command. The plugin then handles copying them into BuildConfig when building, where they're available to our code.

Files

Added

  • secrets.defaults.properties: created template for secrets.properties with default, safe values for every field
  • secrets.properties: not checked into VCS, contains the API keys and client URLs

Modified

  • local.properties: not checked into VCS, removed the API keys and client URLs to conform to best practices mentioned in docs
  • .gitignore: added secrets.properties
  • build.gradle.kts, app/build.gradle.kts, gradle/libs.versions.toml: added Secrets Gradle plugin and initialised it
  • .github/workflows/AndroidBuild.yml, .github/workflows/ci.yml: updated workflow to pipe the variables during CI

Removed

None

Dependencies Added

Testing

Built using the CI pipeline and tested by hand the apk using Logs: now works and fetches correct URLs and keys.
To test for yourself: go to Checks > AndroidBuild > Artifacts > dsTemplate.apk. This will download the apk (zipped). Extract it and install it on your android phone. Please take care to uninstall any previous version of the app that you might have installed!

Added the [Secrets Gradle plugin](https://github.com/google/secrets-gradle-plugin) to the project. This plugin securely provides our secrets to our project, including during CI build.
This file is stored locally on everyone's computer and contains our API keys and client URLs and should never be checked to VCS
The `secrets.default.properties` provides a template for `secrets.properties` for the CI pipeline. The plugin will fill it with our secrets during build.
… workflow

This fetches the secrets from GitHub Secrets and pastes them into the template file `secrets.defaults.properties`. This file is then copied over to a `secrets.properties` created in the CI.
This ensures proper and secure secrets handling.
@charliemangano charliemangano added the enhancement New feature or request label Nov 11, 2024
@charliemangano charliemangano self-assigned this Nov 11, 2024
@charliemangano charliemangano linked an issue Nov 11, 2024 that may be closed by this pull request
This fetches the secrets from GitHub Secrets and pastes them into the template file `secrets.defaults.properties`. This file is then copied over to a `secrets.properties` created in the CI.
This ensures proper and secure secrets handling.
Copy link

@charliemangano charliemangano marked this pull request as ready for review November 11, 2024 11:31
Copy link
Contributor

@coaguila coaguila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the secret problem with the ci build pipeline for the apk !! LGTM

@francelu francelu requested review from francelu and removed request for lazarinibruno November 12, 2024 10:07
Copy link
Contributor

@francelu francelu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Good work on addressing the issue from #161! Your research into the root cause and the detailed documentation of your findings makes it easy to follow your thought process. You’ve clearly put effort into understanding the changes and their impact.

Important

Code Quality

  • The code is clean, well-organized, and adheres to the team's conventions.
  • Good readability and consistent formatting throughout.

Functionality

  • The changes appear correct and address the issue as expected.
  • Edge cases seem well-handled.

Performance

  • No performance concerns were observed in the changes made.

Security

  • No security issues were introduced.

Testing

  • Android download APK testing.
  • I asked @lazarinibruno to test it for me as I don't have an Android. Bug is fixed !

Documentation

  • Clear and detailed documentation of your research and findings. Great job keeping the process transparent.

Suggestions

  • None !

Steps before PR approved

  • No additional changes needed; this PR is good to go!

@charliemangano charliemangano merged commit 721b58c into main Nov 12, 2024
3 checks passed
@charliemangano charliemangano deleted the build/authentication/fix-ci-build-apk branch November 14, 2024 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix secrets not properly applied in CI build apk
3 participants