Fix Part of #5084: Initialize Appcheck #5115
Conversation
Some of the production code was updated in the previous commit.
These scenarios are covered in other tests so I'm fine with not trying to make them work.
This commit sets up the logging infrastructure, including tests. Since Firestore is introduced, some tests in the app module may be broken, and will be fixed in the next commit. This commit soes not handle actual upload to firestore yet, because firebase auth is not yet properly configured.
…loguploader/LogUploadWorkerTest.kt
This new controller provides functionality to retrieve the current signed in user and also sign in user anonymously, and can be called from anywhere. This decouples authentication from a firestore-only use case. The implementation relies on an AuthenticationListener to switch out the real and fake authentication controllers for testing purposes. This commit has not included the Auth module in app module test files(Fragment/Acticity tests), hence these will fail to build with a dagger error. This is fixed in the subsequent commit.
There was a problem hiding this comment.
Thanks @adhiamboperes! Largely LGTM, just had a few follow-up comments (most of which I think might be on base code that was submitted).
Feel free to submit once comments are addressed, or re-assign me if you'd like another pass.
app/src/main/java/org/oppia/android/app/application/AbstractOppiaApplication.kt
Show resolved
Hide resolved
domain/src/main/java/org/oppia/android/domain/auth/AuthenticationController.kt
Outdated
Show resolved
Hide resolved
domain/src/main/java/org/oppia/android/domain/auth/AuthenticationController.kt
Outdated
Show resolved
Hide resolved
testing/src/test/java/org/oppia/android/testing/FakeFirebaseAuthWrapperImplTest.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/application/AbstractOppiaApplication.kt
Outdated
Show resolved
Hide resolved
|
Assigning @seanlip for code owner reviews. Thanks! |
|
Addressed all review comments. PTAL @BenHenning. |
|
@adhiamboperes I'm not totally sure why I'm assigned to this. Do I need to do anything here? |
|
@seanlip, I think you were assigned as code owner when Ben was out (old PR). There is nothing for you to do. |
There was a problem hiding this comment.
Thanks @adhiamboperes! Just had one follow-up comment, so the PR overall LGTM. Please feel free to merge whenever the latest comment is addressed.
app/src/main/java/org/oppia/android/app/application/AbstractOppiaApplication.kt
Show resolved
Hide resolved
|
Assigning @seanlip for code owner reviews. Thanks! |
Explanation
Fixes Part of #5084. This is PR 6 of 6 Planned PRs.
This PR adds Firebase AppCheck and Play Integrity dependencies to the project. It also initializes AppCheck.
Once a new production build is released with this commit, requests to all our Firebase resources will include an attestation token, and we will be able to begin monitoring the type of requests we receive. We will use this data to inform when we will fully enforce AppCheck for all Firebase access, after gaining an understanding of how many active users would be impacted(unable to upload logs to Firestore).
To test this, I added a debug token to the firebase console, to be used to verify our requests to Firestore.
The Appcheck console
Test on the release token fetch
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then: