-
-
Notifications
You must be signed in to change notification settings - Fork 45
Personal ID Sign Up: Adds Integrity token to 'start_configuration' #3127
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
Conversation
📝 WalkthroughWalkthroughThis change set updates the process for initiating a configuration API call in the application. It introduces a new mechanism for obtaining and verifying a Google Play Integrity token before making the API request. The API endpoint and method signatures are updated to require additional headers: an integrity token and a request hash. New utility classes are added for hash computation and integrity token management using a ViewModel, while older utility files for nonce generation and integrity token retrieval are removed. The error message for API failure is updated to be more specific, and the build configuration is modified to include a Google Cloud project number accessible at runtime. Sequence Diagram(s)sequenceDiagram
participant User
participant PersonalIdPhoneFragment
participant IntegrityTokenViewModel
participant ApiPersonalId
participant ApiService
participant Server
User->>PersonalIdPhoneFragment: Click "Continue"
PersonalIdPhoneFragment->>IntegrityTokenViewModel: Prepare token provider
IntegrityTokenViewModel-->>PersonalIdPhoneFragment: Provider ready (Success)
PersonalIdPhoneFragment->>HashUtils: Compute request hash
PersonalIdPhoneFragment->>IntegrityTokenViewModel: Request integrity token (with hash)
IntegrityTokenViewModel-->>PersonalIdPhoneFragment: Return integrity token
PersonalIdPhoneFragment->>ApiPersonalId: startConfiguration(context, body, token, hash, callback)
ApiPersonalId->>ApiService: startConfiguration(token, hash, body)
ApiService->>Server: POST /users/start_configuration (with headers and body)
Server-->>ApiService: Response
ApiService-->>ApiPersonalId: Response
ApiPersonalId-->>PersonalIdPhoneFragment: Callback (success or failure)
PersonalIdPhoneFragment-->>User: Show result (success or error message)
Suggested labels
Suggested reviewers
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
app/src/org/commcare/connect/network/PersonalIdApiHandler.java (1)
72-81: 🛠️ Refactor suggestionAdd parameter validation for security and robustness.
The method signature change correctly implements the new integrity token requirements. However, consider adding parameter validation to prevent potential runtime issues.
public void makeStartConfigurationCall(Activity activity, Map<String, String> body, String integrityToken, String requestHash) { + if (body == null || body.isEmpty()) { + throw new IllegalArgumentException("Request body cannot be null or empty"); + } + if (integrityToken == null || integrityToken.trim().isEmpty()) { + throw new IllegalArgumentException("Integrity token cannot be null or empty"); + } + if (requestHash == null || requestHash.trim().isEmpty()) { + throw new IllegalArgumentException("Request hash cannot be null or empty"); + } + PersonalIdSessionData sessionData = new PersonalIdSessionData(); ApiPersonalId.startConfiguration(activity, body, integrityToken, requestHash, createCallback(sessionData, new StartConfigurationResponseParser(), PersonalIdApiErrorCodes.INVALID_RESPONSE_ERROR)); }
🧹 Nitpick comments (6)
app/src/org/commcare/connect/network/connectId/ApiService.java (1)
12-15: LGTM! Consider adding validation annotations for better API documentation.The API interface changes correctly implement the new integrity token requirements. The endpoint change from
registerUsertostartConfigurationaligns with the updated endpoint constants.Consider adding validation annotations to make the API contract more explicit:
@POST(ApiEndPoints.startConfiguration) -Call<ResponseBody> startConfiguration(@Header("CC-Integrity-Token") String integrityToken, - @Header("CC-Request-Hash") String requestHash, - @Body Map<String, String> registrationRequest); +Call<ResponseBody> startConfiguration(@Header("CC-Integrity-Token") @NonNull String integrityToken, + @Header("CC-Request-Hash") @NonNull String requestHash, + @Body @NonNull Map<String, String> registrationRequest);app/src/org/commcare/connect/network/ApiPersonalId.java (1)
289-294: Implementation looks correct. Consider adding parameter validation for consistency.The method correctly implements the new signature and properly forwards parameters to the API service. The implementation aligns well with the integrity token requirements.
For consistency with security best practices, consider adding parameter validation:
public static void startConfiguration(Context context, Map<String, String> body, String integrityToken, String requestHash, IApiCallback callback) { + if (body == null || body.isEmpty()) { + callback.processFailure(400); // Bad request + return; + } + if (integrityToken == null || integrityToken.trim().isEmpty()) { + callback.processFailure(400); + return; + } + if (requestHash == null || requestHash.trim().isEmpty()) { + callback.processFailure(400); + return; + } + ApiService apiService = ApiClient.getClientApi(); Call<ResponseBody> call = apiService.startConfiguration(integrityToken, requestHash, body); callApi(context, call, callback); }app/src/org/commcare/utils/IntegrityTokenViewModel.kt (2)
15-15: Remove the empty default constructor.As flagged by static analysis, the empty default constructor is unnecessary and can be removed.
-class IntegrityTokenViewModel() : ViewModel() { +class IntegrityTokenViewModel : ViewModel() {🧰 Tools
🪛 detekt (1.23.8)
[warning] 15-15: An empty default constructor can be removed.
(detekt.empty-blocks.EmptyDefaultConstructor)
54-67: Consider more graceful error handling for token provider readiness.The current implementation throws an exception if the token provider isn't ready. Consider a more graceful approach that notifies the caller through the callback.
fun requestIntegrityToken(requestHash: String, callback: IntegrityTokenCallback) { - require(integrityTokenProvider != null) {"StandardIntegrityTokenProvider is not warmed up yet. Please try again"} + if (integrityTokenProvider == null) { + Logger.log("Integrity token provider not ready yet") + callback.onTokenReceived(null) + return + } + val integrityTokenResponse = integrityTokenProvider!!.request( StandardIntegrityTokenRequest.builder() .setRequestHash(requestHash) .build() )app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java (2)
206-206: Fix typo in comment.- // This is called when api returns success but with a a failure code + // This is called when api returns success but with a failure code
188-195: Consider adding specific error handling for integrity token failures.When the integrity token is null, the user receives a generic error message without knowing the specific cause.
Consider passing error context to provide more specific feedback:
tokenViewModel.requestIntegrityToken(requestHash, integrityToken -> { if (integrityToken != null) { makeStartConfigurationCall(integrityToken, requestHash, body); } else { - onConfigurationFailure(); + onConfigurationFailure(ConfigurationFailureReason.INTEGRITY_TOKEN_FAILED); } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/build.gradle(2 hunks)app/res/values/strings.xml(1 hunks)app/src/org/commcare/connect/network/ApiPersonalId.java(2 hunks)app/src/org/commcare/connect/network/PersonalIdApiHandler.java(2 hunks)app/src/org/commcare/connect/network/connectId/ApiEndPoints.java(1 hunks)app/src/org/commcare/connect/network/connectId/ApiService.java(1 hunks)app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java(5 hunks)app/src/org/commcare/utils/GooglePlayIntegrityUtil.kt(0 hunks)app/src/org/commcare/utils/HashUtils.kt(1 hunks)app/src/org/commcare/utils/IntegrityTokenViewModel.kt(1 hunks)app/src/org/commcare/utils/NonceGenerator.kt(0 hunks)
💤 Files with no reviewable changes (2)
- app/src/org/commcare/utils/NonceGenerator.kt
- app/src/org/commcare/utils/GooglePlayIntegrityUtil.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/org/commcare/connect/network/connectId/ApiService.java (1)
app/src/org/commcare/connect/network/connectId/ApiEndPoints.java (1)
ApiEndPoints(3-36)
🪛 detekt (1.23.8)
app/src/org/commcare/utils/IntegrityTokenViewModel.kt
[warning] 15-15: An empty default constructor can be removed.
(detekt.empty-blocks.EmptyDefaultConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
app/build.gradle (1)
188-188: LGTM! Google Cloud project number configuration is properly implemented.The configuration follows the established pattern with appropriate fallback value and will enable the Play Integrity API integration.
Also applies to: 315-315
app/src/org/commcare/utils/HashUtils.kt (1)
7-22: Excellent implementation of cryptographic hash utility.The implementation correctly uses:
- Secure SHA-256 as the default algorithm
- Proper UTF-8 encoding for string input
- Base64 encoding without wrapping/padding (appropriate for integrity tokens)
- Thread-safe MessageDigest usage
The code follows security best practices and provides a clean API for the integrity token generation flow.
app/src/org/commcare/connect/network/connectId/ApiEndPoints.java (1)
8-8:Details
✅ Verification successful
Verify all references to the old constant have been updated.
The endpoint constant rename looks correct. Please ensure all references to the old
registerUserconstant have been updated throughout the codebase.
🏁 Script executed:
#!/bin/bash # Description: Verify that no references to the old registerUser constant remain # Expected: No occurrences of registerUser should be found (except possibly in comments or strings) echo "Searching for any remaining references to 'registerUser':" rg -n "registerUser" --type java --type kt echo -e "\nSearching for references to the new 'startConfiguration' constant:" rg -n "startConfiguration" --type java --type ktLength of output: 431
I’ll re-run the search without relying on ripgrep’s file-type definitions:
🏁 Script executed:
#!/bin/bash echo "Searching for any remaining references to 'registerUser' in .java and .kt files:" rg -n "registerUser" -g '*.java' -g '*.kt' || true echo -e "\nSearching for references to 'startConfiguration' in .java and .kt files:" rg -n "startConfiguration" -g '*.java' -g '*.kt' || trueLength of output: 1852
Let’s narrow the search to only whole-word and direct constant references:
🏁 Script executed:
#!/bin/bash echo "Looking for any direct uses of ApiEndPoints.registerUser:" rg -n "ApiEndPoints\.registerUser" -g '*.java' -g '*.kt' || true echo -e "\nLooking for whole-word occurrences of 'registerUser' in Java/Kotlin files:" rg -n '\bregisterUser\b' -g '*.java' -g '*.kt' || trueLength of output: 417
Rename validated – no lingering
registerUserreferences
All occurrences of the oldApiEndPoints.registerUserconstant have been removed and replaced withstartConfiguration. No direct or whole-word matches forregisterUserremain in.javaor.ktfiles.app/res/values/strings.xml (1)
923-923: Great improvement to user-facing error messaging.The updated error message is more specific and actionable, providing clear guidance to users when server communication fails. This enhances the user experience during the Personal ID configuration process.
app/src/org/commcare/utils/IntegrityTokenViewModel.kt (2)
15-77: Excellent implementation following Android best practices!This ViewModel correctly implements the Google Play Integrity API following the recommended two-step process. The use of LiveData for state management and proper error handling demonstrates good Android architecture practices.
The implementation includes:
- Proper ViewModel lifecycle management
- Asynchronous token provider preparation
- LiveData for state observation
- Comprehensive error handling and logging
- Clean separation of concerns with sealed classes and functional interfaces
🧰 Tools
🪛 detekt (1.23.8)
[warning] 15-15: An empty default constructor can be removed.
(detekt.empty-blocks.EmptyDefaultConstructor)
35-36:Details
✅ Verification successful
Verify the Google Cloud Project Number configuration.
Ensure that the
GOOGLE_CLOUD_PROJECT_NUMBERis properly configured in the build system and matches your Google Cloud project.
🏁 Script executed:
#!/bin/bash # Description: Check if Google Cloud Project Number is properly configured in build files # Search for GOOGLE_CLOUD_PROJECT_NUMBER in build files fd -e gradle -e groovy | xargs rg -l "GOOGLE_CLOUD_PROJECT_NUMBER" # Look for buildConfigField definitions rg "buildConfigField.*GOOGLE_CLOUD_PROJECT_NUMBER" --type gradleLength of output: 276
🏁 Script executed:
#!/bin/bash # Verify where GOOGLE_CLOUD_PROJECT_NUMBER is defined across the project # Search in all Gradle build scripts and properties files for the property fd -e gradle -e properties | xargs rg -n "GOOGLE_CLOUD_PROJECT_NUMBER"Length of output: 312
GOOGLE_CLOUD_PROJECT_NUMBER BuildConfig Setup Verified
The property is already defined in app/build.gradle (line 188) with a
-1Lfallback and exposed viabuildConfigField(line 315). No code changes are needed:
- Ensure you set
GOOGLE_CLOUD_PROJECT_NUMBER(for example in gradle.properties or via ‐P flag) so it doesn’t fall back to-1Land trigger the runtime check.app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java (1)
167-176:Details
✅ Verification successful
Verify the integrity token provider warm-up implementation.
According to the PR description, the implementation should include a "Warm Up" call as the first step, but this code only observes the provider state without explicitly triggering the warm-up.
🏁 Script executed:
#!/bin/bash # Description: Check if the IntegrityTokenViewModel includes automatic warm-up or if it needs explicit triggering # Search for the IntegrityTokenViewModel implementation fd -e java -e kt "IntegrityTokenViewModel" --exec grep -l "IntegrityTokenViewModel" {} \; | head -5 | xargs cat # Look for warm-up related methods ast-grep --pattern 'class IntegrityTokenViewModel { $$$ }' # Search for warm-up or initialization methods rg -A 10 "(warmUp|warm|initialize|prepare)" --glob "*IntegrityTokenViewModel*"Length of output: 7733
Warm-up is already invoked in the ViewModel init
The
IntegrityTokenViewModel’sinitblock callsprepareTokenProvider(), which performs the asynchronous “warm-up” (prepareIntegrityToken) before any observers are notified. No explicit warm-up call is missing.
| tokenViewModel.getProviderState().observe(getViewLifecycleOwner(), state -> { | ||
| if (state instanceof IntegrityTokenViewModel.TokenProviderState.Success) { | ||
| startConfigurationRequest(); | ||
| } else if (state instanceof IntegrityTokenViewModel.TokenProviderState.Failure) { | ||
| Exception exception = ((IntegrityTokenViewModel.TokenProviderState.Failure) state).getException(); | ||
| Logger.exception("Unable to warm up Integrity Token Provider", exception); | ||
| onConfigurationFailure(); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix memory leak: Move LiveData observation to fragment lifecycle method.
Observing LiveData inside a click handler creates a new observer on each button click, leading to memory leaks and potential duplicate callbacks. The observer will persist even after the fragment is destroyed unless properly removed.
Move the LiveData observation to onCreateView or onViewCreated:
private void onContinueClicked() {
- tokenViewModel.getProviderState().observe(getViewLifecycleOwner(), state -> {
- if (state instanceof IntegrityTokenViewModel.TokenProviderState.Success) {
- startConfigurationRequest();
- } else if (state instanceof IntegrityTokenViewModel.TokenProviderState.Failure) {
- Exception exception = ((IntegrityTokenViewModel.TokenProviderState.Failure) state).getException();
- Logger.exception("Unable to warm up Integrity Token Provider", exception);
- onConfigurationFailure();
- }
- });
+ // Trigger the token provider warm-up or check if already warmed up
+ tokenViewModel.warmUp(); // or similar method to initiate the process
}Then in onViewCreated:
@Override
public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) {
super.onViewCreated(view, savedInstanceState);
tokenViewModel.getProviderState().observe(getViewLifecycleOwner(), state -> {
if (state instanceof IntegrityTokenViewModel.TokenProviderState.Success) {
startConfigurationRequest();
} else if (state instanceof IntegrityTokenViewModel.TokenProviderState.Failure) {
Exception exception = ((IntegrityTokenViewModel.TokenProviderState.Failure) state).getException();
Logger.exception("Unable to warm up Integrity Token Provider", exception);
onConfigurationFailure();
}
});
}🤖 Prompt for AI Agents
In app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java around
lines 167 to 176, the LiveData observation is currently inside a click handler,
causing multiple observers to be added on each click and leading to memory
leaks. To fix this, move the tokenViewModel.getProviderState().observe(...) call
from the click handler to the fragment's onViewCreated method so that the
observer is registered once per fragment view lifecycle and automatically
cleaned up when the view is destroyed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should not cause a memory leak as the observer is tied to fragment's lifecycle here right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 You're absolutely right! Since you're using getViewLifecycleOwner(), the observers are automatically cleaned up when the fragment's view is destroyed, so there's no memory leak in the traditional sense.
However, there's still a potential issue with multiple observers being registered. Each time the button is clicked, a new observer is added to the same LiveData. If a user clicks the button multiple times quickly, you'll have multiple active observers that will all fire when the state changes, potentially causing:
startConfigurationRequest()to be called multiple timesonConfigurationFailure()to be called multiple times- Duplicate API calls or navigation actions
So while it's not a memory leak, it can still cause unintended duplicate behavior. A simple fix would be to either:
- Disable the button after the first click until the operation completes
- Check if an observer is already registered before adding a new one
- Move the observation to
onViewCreated()and use a flag to track when to proceed
The lifecycle-aware behavior you mentioned is correct though - good catch!
| private void onConfigurationFailure() { | ||
| Logger.log(LogTypes.TYPE_USER, | ||
| personalIdSessionDataViewModel.getPersonalIdSessionData().getSessionFailureCode()); | ||
| String failureMessage = getString(R.string.configuration_process_failed_subtitle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the string resource exists.
The string resource was changed from configuration_process_api_failed to configuration_process_failed_subtitle.
🏁 Script executed:
#!/bin/bash
# Description: Verify that the new string resource exists in the strings.xml file
# Search for the new string resource
rg -A 2 'name="configuration_process_failed_subtitle"' --glob 'strings*.xml'
# Also check if the old resource was removed
rg -A 2 'name="configuration_process_api_failed"' --glob 'strings*.xml'Length of output: 827
Confirm presence of new string and remove obsolete resource
- Verified that
configuration_process_failed_subtitleis defined in app/res/values/strings.xml. - The old
configuration_process_api_failedentry still exists—please remove it if it's no longer referenced in the codebase.
🤖 Prompt for AI Agents
In app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java at
line 226, the string resource was updated to use
configuration_process_failed_subtitle. Confirm that this new string resource is
defined in app/res/values/strings.xml, which it is. Next, locate and remove the
obsolete configuration_process_api_failed string resource from the strings.xml
file if it is no longer referenced anywhere in the codebase to keep resources
clean and avoid confusion.
| HashMap<String, String> body = new HashMap<>(); | ||
| body.put("phone_number", phone); | ||
|
|
||
| String requestHash = HashUtils.computeHash(body.toString(), HashUtils.HashAlgorithm.SHA256); | ||
| tokenViewModel.requestIntegrityToken(requestHash, integrityToken -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Use deterministic serialization for hash computation.
Using HashMap.toString() for cryptographic hashing is problematic because:
- HashMap doesn't guarantee iteration order, causing inconsistent hashes for the same data
- The
toString()format is implementation-specific and not suitable for security purposes
Use JSON serialization or a sorted map for consistent hashing:
HashMap<String, String> body = new HashMap<>();
body.put("phone_number", phone);
-String requestHash = HashUtils.computeHash(body.toString(), HashUtils.HashAlgorithm.SHA256);
+// Option 1: Use JSON serialization
+String jsonBody = new JSONObject(body).toString();
+String requestHash = HashUtils.computeHash(jsonBody, HashUtils.HashAlgorithm.SHA256);
+
+// Option 2: Use TreeMap for consistent ordering
+// TreeMap<String, String> sortedBody = new TreeMap<>(body);
+// String requestHash = HashUtils.computeHash(sortedBody.toString(), HashUtils.HashAlgorithm.SHA256);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| HashMap<String, String> body = new HashMap<>(); | |
| body.put("phone_number", phone); | |
| String requestHash = HashUtils.computeHash(body.toString(), HashUtils.HashAlgorithm.SHA256); | |
| tokenViewModel.requestIntegrityToken(requestHash, integrityToken -> { | |
| HashMap<String, String> body = new HashMap<>(); | |
| body.put("phone_number", phone); | |
| // Option 1: Use JSON serialization | |
| String jsonBody = new JSONObject(body).toString(); | |
| String requestHash = HashUtils.computeHash(jsonBody, HashUtils.HashAlgorithm.SHA256); | |
| // Option 2: Use TreeMap for consistent ordering | |
| // TreeMap<String, String> sortedBody = new TreeMap<>(body); | |
| // String requestHash = HashUtils.computeHash(sortedBody.toString(), HashUtils.HashAlgorithm.SHA256); | |
| tokenViewModel.requestIntegrityToken(requestHash, integrityToken -> { |
🤖 Prompt for AI Agents
In app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java around
lines 184 to 188, the code uses HashMap.toString() to generate a string for
hashing, which is non-deterministic and insecure. Replace this with a
deterministic serialization method by converting the map to a JSON string using
a JSON library or by using a sorted map to ensure consistent key order before
hashing. This will guarantee the same input string for identical data and
produce consistent hash values.
|
@shubham1g5 App might start adding the integrity token to many other APIs in future, shouldn't it better that we maintain list of APIs where token needs to be added and we create some generic code (may be at ApiPersonalId.java) where it can fetch integrity token and append when calling API is in that list? That way, each and every fragments/activity doesn't need to take care of fetching the integrity token? |
| * Also note that each app instance can only prepare the integrity token up to 5 times per minute. | ||
| */ | ||
| fun prepareTokenProvider() { | ||
| val standardIntegrityManager = IntegrityManagerFactory.createStandard(CommCareApplication.instance()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 Does this implementation have extra benefit over the simple method which was defined in GooglePlay IntegrityUtil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
earlier code implemented Classic Integrity request which we don't need to rely on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 +1 Good catch, yeah seems like standard API are less expensive than classical.
It also seems that Google is mentioning to instantiate StandardIntegrityTokenProvider whenever app opens and same can be used for every token request. Here, ViewModel's initialization will create new StandardIntegrityTokenProvider everytime, which is bit expensive.
This actually reminds to have Dagger injection in our app but as its not there, we can use some singleton class as of now to hold StandardIntegrityTokenProvider? and same can be used with API calling class like ApiPersonalId.java to add the token?
| buildConfigField "String", "FIREBASE_DATABASE_URL", "\"${project.ext.FIREBASE_DATABASE_URL}\"" | ||
| buildConfigField 'String', 'CCC_HOST', "\"connect.dimagi.com\"" | ||
|
|
||
| buildConfigField 'long', 'GOOGLE_CLOUD_PROJECT_NUMBER', "${project.ext.GOOGLE_CLOUD_PROJECT_NUMBER}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 Does this mean that while running the app in local, we will not able to fetch integrity token?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we will need add this to our local .gradle/gradle.properties files to fetch it.
|
|
||
| private void onContinueClicked() { | ||
| binding.personalidPhoneContinueButton.setEnabled(false); | ||
| tokenViewModel.getProviderState().observe(getViewLifecycleOwner(), state -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 every button clicks starts adding the observer, shouldn't be defined outside the function?
…integrity token to multiple requests
|
@Jignesh-dimagi Made some changes based on your feedback, to get token is now a simple one line call from the code component making an API call. |
|
@shubham1g5 +1 for the PR changes, looks good to me. |
Product Description
https://dimagi.atlassian.net/browse/CCCT-1076
https://dimagi.atlassian.net/browse/CCCT-1077
Technical Summary
Implements Standard Integrity request to get the token. The call to get token is divided into a Warm Up call + subsequent fetch token call which is why I decided to use a View Model for the token provider class to handle synchronisation easily my monitoring on live Data.
Safety Assurance
Tested that getting the token works correctly but have not been able to test the
start_configurationAPI endpoint with it as it gives 404 currently.Labels and Review
FYI @Charl1996