Move Firebase Database URL to Android secrets #2717
Conversation
It seems that CommCare is not using Firebase Realtime Database so parking this for now
|
@shubham1g5 I don't think we use |
|
We don't use Firebase Cloud Database but are you sure the url is only required for cloud database and not for other Firebase services ? |
| @@ -290,7 +291,7 @@ android { | |||
|
|
|||
| buildConfigField "String", "HQ_API_USERNAME", "\"${project.ext.HQ_API_USERNAME}\"" | |||
| buildConfigField "String", "HQ_API_PASSWORD", "\"${project.ext.HQ_API_PASSWORD}\"" | |||
|
|
|||
| buildConfigField "String", "FIREBASE_DATABASE_URL", "\"${project.ext.FIREBASE_DATABASE_URL}\"" | |||
There was a problem hiding this comment.
how does this feeds into google-services.json ?
There was a problem hiding this comment.
it doesn't, it will come from ~/.gradle/gradle.properties now
There was a problem hiding this comment.
Doesn't firebase need it to be in google-services.json ?
There was a problem hiding this comment.
From the tests I ran, it doesn't seems that there aren't services relying on it. I can re-run them just to make sure.
There was a problem hiding this comment.
can you add what tests you run to the safety story for this PR ?
There was a problem hiding this comment.
we are adding FIREBASE_DATABASE_URL as a build config field but never utilising this field. As such I am struggling to understand the approach behind this change in how Firebase will utilise it from the build config ?
There was a problem hiding this comment.
At the moment it seems that there is no use for it, so we could actually remove it. But because the initial reasoning was just to 'move' it, I decided to put it there. Should we just remove it?
There was a problem hiding this comment.
I really don't understand the need to move or delete it and a bit hesitant to go ahead with it without testing all Firebase services we use. The url is a public url and is behind authentication, since we already have the project_id visible in google services json anyone can anyway construct this url easily with format https://{project_id}.firebaseio.com
There was a problem hiding this comment.
Exactly, that was also my thinking, with the project ID it's easy to put the url together. Is there any particular Firebase we should be concerned about? I can run additional tests.
|
@damagatchi retest this please |
They were tested with the release apk from this build ? How did you test Crashlytics (as in how did you made sure to trigger a crash with the release apk on this branch ?) |
By throwing a |
Summary
This addresses an issue raised by the auditors in which sensitive information was hard-coded in the app, in this case, the Firebase Database URL. More on ticket: INDIV-97.
Safety Assurance
Safety story
The Firebase services CommCare relies on, Crashlytics and Firebase Cloud Messaging, were tested after this move, and they worked as expected.