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

CCCT-469 || HQ User Invite Connect #2927

Open
wants to merge 11 commits into
base: connect_qa
Choose a base branch
from
21 changes: 18 additions & 3 deletions app/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest package="org.commcare.dalvik"
xmlns:android="http://schemas.android.com/apk/res/android"
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
package="org.commcare.dalvik"
android:versionCode="106"
android:versionName="2.55">

Expand Down Expand Up @@ -130,10 +130,25 @@
android:exported="true"
android:permission="com.google.android.gms.auth.api.phone.permission.SEND">
<intent-filter>
<action android:name="com.google.android.gms.auth.api.phone.SMS_RETRIEVED"/>
<action android:name="com.google.android.gms.auth.api.phone.SMS_RETRIEVED" />
</intent-filter>
</receiver>

<activity
android:name="org.commcare.activities.connect.HQUserInviteActivity"
android:exported="true">
<intent-filter android:autoVerify="true">
<action android:name="android.intent.action.VIEW" />

<category android:name="android.intent.category.DEFAULT" />
<category android:name="android.intent.category.BROWSABLE" />

<data android:scheme="https"/>
<data android:host="connectid.dimagi.com"/>
<data android:scheme="http"/>
<data android:host="connectid.dimagi.com"/>
</intent-filter>
</activity>
Comment on lines +137 to +151
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Additional security measures recommended for deep link handling

Based on the code analysis, while the activity implements basic deep link handling, several security improvements are needed:

  1. The activity accepts and processes deep link parameters without any validation of the callback_url parameter, which could lead to potential security vulnerabilities
  2. No null checks are performed on required parameters before processing the invitation
  3. The activity accepts both HTTP and HTTPS schemes without enforcing HTTPS-only communication
  4. No validation exists for the domain of callback URLs to ensure they match expected patterns

Recommended actions:

  • Add URL validation for the callback_url parameter to ensure it points to trusted domains
  • Implement null/empty checks for all required parameters (callback_url, invite_code, etc.)
  • Consider removing the HTTP scheme from the intent filter to enforce secure communications
  • Add path patterns to the intent filter to restrict the accepted URLs to specific endpoints
🔗 Analysis chain

Verify security measures for exported activity handling deep links

The exported activity with deep link handling requires careful security review:

  1. Ensure proper validation of incoming deep link data
  2. Verify that android:autoVerify="true" is intended as it allows automatic verification of deep links
  3. Consider adding intent filters for specific paths rather than allowing all paths
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for URL validation in HQUserInviteActivity
ast-grep --pattern 'class HQUserInviteActivity {
  $$$
  getIntent() {
    $$$
  }
  $$$
}'

Length of output: 93


Script:

#!/bin/bash
# Search for HQUserInviteActivity implementation
fd -e java -e kt "HQUserInviteActivity" --exec cat {}

# Search for URL/URI validation patterns in the activity
rg -l "HQUserInviteActivity" | xargs rg -A 5 "getIntent|getData|parse.*Uri|validate"

Length of output: 7692


Script:

#!/bin/bash
# Check if ConnectManager performs any validation
rg -A 10 "class ConnectManager" 

# Check ApiConnectId implementation for security measures
rg -A 10 "hqUserInvitation.*callback" 

# Look for any URL validation utilities
rg -l "validateUrl|validateUri|isValidUrl|isValidUri"

Length of output: 3910

<activity
android:name="org.commcare.activities.DispatchActivity"
android:exported="true"
Expand Down
114 changes: 114 additions & 0 deletions app/res/layout/activity_hquser_invite.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools"
android:id="@+id/main"
android:layout_width="match_parent"
android:layout_height="match_parent"
tools:context="org.commcare.activities.connect.HQUserInviteActivity">

<ImageView
android:id="@+id/imageView2"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginHorizontal="30dp"
android:adjustViewBounds="true"
android:src="@drawable/commcare_logo"
app:layout_constraintBottom_toTopOf="@+id/guideline3"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent" />

<org.commcare.views.connect.connecttextview.ConnectMediumTextView
android:id="@+id/tvHqInvitationHeaderTitle"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginHorizontal="30dp"
android:layout_marginTop="20dp"
android:gravity="center"
android:text=""
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/imageView2" />

<org.commcare.views.connect.RoundedButton
android:id="@+id/btn_accept_invitation"
android:layout_width="match_parent"
android:layout_height="40dp"
android:layout_marginHorizontal="50dp"
android:layout_marginTop="30dp"
android:text="@string/connect_hq_invitation_accept"
android:visibility="visible"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/tvHqInvitationHeaderTitle"
app:roundButtonBackgroundColor="@color/connect_blue_color"
app:roundButtonTextColor="@color/white"
app:roundButtonTextSize="6sp"
tools:text="@string/connect_hq_invitation_accept" />

<org.commcare.views.connect.RoundedButton
android:id="@+id/btn_denied_invitation"
android:layout_width="match_parent"
android:layout_height="40dp"
android:layout_marginHorizontal="50dp"
android:layout_marginTop="15dp"
android:text="@string/connect_hq_invitation_denied"
android:visibility="visible"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/btn_accept_invitation"
app:roundButtonBackgroundColor="@color/connect_blue_color"
app:roundButtonTextColor="@color/white"
app:roundButtonTextSize="6sp"
tools:text="@string/connect_hq_invitation_denied" />

<org.commcare.views.connect.RoundedButton
android:id="@+id/btn_go_to_recovery"
android:layout_width="match_parent"
android:layout_height="40dp"
android:layout_marginHorizontal="50dp"
android:layout_marginTop="15dp"
android:text="@string/connect_hq_invitation_go_to_recovery"
android:visibility="visible"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/btn_denied_invitation"
app:roundButtonBackgroundColor="@color/connect_blue_color"
app:roundButtonTextColor="@color/white"
app:roundButtonTextSize="6sp"
tools:text="@string/connect_hq_invitation_go_to_recovery" />

<org.commcare.views.connect.connecttextview.ConnectMediumTextView
android:layout_width="0dp"
android:layout_height="wrap_content"
android:text=""
android:id="@+id/connect_phone_verify_error"
android:textSize="16sp"
android:layout_marginTop="10dp"
android:textColor="@color/connect_red"
app:layout_constraintEnd_toEndOf="@+id/btn_denied_invitation"
app:layout_constraintStart_toStartOf="@+id/btn_denied_invitation"
app:layout_constraintTop_toBottomOf="@+id/btn_denied_invitation" />

<androidx.constraintlayout.widget.Guideline
android:id="@+id/guideline3"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:orientation="horizontal"
app:layout_constraintGuide_percent="0.35" />

<ProgressBar
android:id="@+id/progress_bar"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
style="?android:attr/progressBarStyle"
android:layout_centerHorizontal="true"
android:progressTint="@color/connect_blue_color"
android:progressBackgroundTint="@color/connect_blue_color"
android:visibility="gone"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/btn_go_to_recovery" />

</androidx.constraintlayout.widget.ConstraintLayout>
7 changes: 7 additions & 0 deletions app/res/values-fr/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,13 @@ License.
<string name="connect_learn">Apprendre</string>
<string name="connect_delivery">Livraison</string>
<string name="connect_commcare_app">Application CommCare</string>

<string name="connect_hq_invitation_heading">Vous êtes invité au domaine du projet CommCareHQ en tant que %s</string>
<string name="connect_hq_invitation_accept">Accepter</string>
<string name="connect_hq_invitation_denied">Refusé</string>
<string name="connect_hq_invitation_go_to_recovery">Aller à la récupération</string>
<string name="connect_hq_invitation_connectId_not_configure">Vous n\'avez pas configuré votre ConnectID. Pour accepter cette invitation, veuillez dabord configurer votre ConnectID. Une fois configuré, vous pourrez accepter invitation.</string>
<string name="connect_hq_invitation_wrong_user">Oups ! Il semble que l’invitation ait été envoyée à la mauvaise personne.</string>
<string name="connect_appbar_title_app_lock">Verrouillage d\'application</string>
<string name="connect_appbar_title_password_verification">Vérification du mot de passe</string>
</resources>
8 changes: 8 additions & 0 deletions app/res/values-pt/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,14 @@
<string name="connect_new_opportunity">Nova oportunidade</string>
<string name="connect_learn">Aprender</string>
<string name="connect_delivery">Entrega</string>
<string name="connect_commcare_app">Aplicativo CommCare</string>

<string name="connect_hq_invitation_heading">Está convidado para o domínio do projeto CommCareHQ como %s</string>
<string name="connect_hq_invitation_accept">Aceitar</string>
<string name="connect_hq_invitation_denied">Negado</string>
<string name="connect_hq_invitation_go_to_recovery">Vá para a recuperação</string>
<string name="connect_hq_invitation_connectId_not_configure">Não configurou o seu ConnectID. Para aceitar este convite, configure primeiro o seu ConnectID. Depois de configurado, poderá aceitar o convite.</string>
<string name="connect_hq_invitation_wrong_user">Ops! Parece que o convite foi enviado para a pessoa errada.</string>
<string name="connect_expired">Expirado</string>
<string name="connect_job_tile_daily_limit_description">Limite diário atingido. Nenhum pagamento pelo envio de formulários</string>
<string name="connect_job_tile_daily_limit">Acima do limite</string>
Expand Down
8 changes: 8 additions & 0 deletions app/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
<string name="ConnectPaymentConfirmationURL">https://%s/api/payment/%s/confirm</string>
<string name="ConnectInitiateUserAccountDeactivationURL">https://connectid.dimagi.com/users/recover/initiate_deactivation</string>
<string name="ConnectConfirmUserAccountDeactivationURL">https://connectid.dimagi.com/users/recover/confirm_deactivation</string>
<string name="ConnectConfirmUserInvitation">https://connectid.dimagi.com/users/confirm_hq_invite</string>

<!-- region: All strings for multiple apps and app-agnostic properties -->

Expand Down Expand Up @@ -875,6 +876,13 @@
<string name="connect_delivery">Delivery</string>
<string name="connect_expired">Expired</string>
<string name="connect_commcare_app">CommCare App</string>

<string name="connect_hq_invitation_heading">You are invited to CommCareHQ project domain as %s</string>
<string name="connect_hq_invitation_accept">Accept</string>
<string name="connect_hq_invitation_denied">Denied</string>
<string name="connect_hq_invitation_go_to_recovery">Go To Recovery</string>
<string name="connect_hq_invitation_connectId_not_configure">You have not configured your ConnectID. To accept this invitation, please configure your ConnectID first. Once configured, you\'ll be able to accept the invitation.</string>
<string name="connect_hq_invitation_wrong_user">Oops! It seems like the invitation was sent to the wrong person.</string>
<string name="connect_job_tile_daily_limit_description">Daily Limit reached. No Payment for submitting forms</string>
<string name="connect_job_tile_daily_limit">Over Limit</string>
<string name="connect_job_tile_daily_visits">Daily Visits</string>
Expand Down
169 changes: 169 additions & 0 deletions app/src/org/commcare/activities/connect/HQUserInviteActivity.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
package org.commcare.activities.connect;

import android.content.Intent;
import android.net.Uri;
import android.os.Bundle;
import android.util.Log;
import android.view.View;
import android.widget.Toast;

import org.commcare.activities.CommCareActivity;
import org.commcare.activities.DispatchActivity;
import org.commcare.android.database.connect.models.ConnectUserRecord;
import org.commcare.connect.ConnectManager;
import org.commcare.connect.network.ApiConnectId;
import org.commcare.connect.network.IApiCallback;
import org.commcare.dalvik.R;
import org.commcare.dalvik.databinding.ActivityHquserInviteBinding;
import org.javarosa.core.io.StreamsUtil;
import org.javarosa.core.services.Logger;

import java.io.IOException;
import java.io.InputStream;
import java.util.Locale;

public class HQUserInviteActivity extends CommCareActivity<HQUserInviteActivity> {

private ActivityHquserInviteBinding binding;
String domain;
String inviteCode;
String username;
String callBackURL;
String connectUserName;
boolean isProgressVisible = false;

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
binding = ActivityHquserInviteBinding.inflate(getLayoutInflater());
setContentView(binding.getRoot());
Intent intent = getIntent();
Uri data = intent.getData();
if (data != null) {
callBackURL = data.getQueryParameter("callback_url");
username = data.getQueryParameter("hq_username");
inviteCode = data.getQueryParameter("invite_code");
domain = data.getQueryParameter("hq_domain");
connectUserName = data.getQueryParameter("connect_username");
}
handleButtons();
}
Comment on lines +25 to +50
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure domain & parameter validation in onCreate()
This block relies on Intent data to populate fields like domain, inviteCode, etc. It's safer to handle malformed or missing query parameters by checking for null or empty strings here to avoid potential NullPointerExceptions or incorrect state.


private void handleButtons() {
ConnectManager.init(this);

ConnectUserRecord user = ConnectManager.getUser(this);
boolean isTokenPresent = ConnectManager.isConnectIdConfigured();
boolean isCorrectUser = true;
if (user != null) {
isCorrectUser = user.getUserId().equals(connectUserName);
}

if (isCorrectUser) {
binding.tvHqInvitationHeaderTitle.setText(isTokenPresent
? getString(R.string.connect_hq_invitation_heading, username)
: getString(R.string.connect_hq_invitation_connectId_not_configure));
setButtonVisibility(isTokenPresent);
setButtonListeners(isTokenPresent);
} else {
binding.tvHqInvitationHeaderTitle.setText(getString(R.string.connect_hq_invitation_wrong_user));
hideInvitationButtons();
}
}

private void setButtonVisibility(boolean isTokenPresent) {
binding.btnAcceptInvitation.setVisibility(isTokenPresent ? View.VISIBLE : View.GONE);
binding.btnDeniedInvitation.setVisibility(isTokenPresent ? View.VISIBLE : View.GONE);
binding.btnGoToRecovery.setVisibility(isTokenPresent ? View.GONE : View.VISIBLE);
}

private void hideInvitationButtons() {
binding.btnAcceptInvitation.setVisibility(View.GONE);
binding.btnDeniedInvitation.setVisibility(View.GONE);
binding.btnGoToRecovery.setVisibility(View.GONE);
}

private void setButtonListeners(boolean isTokenPresent) {
if (isTokenPresent) {
binding.btnAcceptInvitation.setOnClickListener(view -> handleInvitation(callBackURL, inviteCode));
binding.btnDeniedInvitation.setOnClickListener(view -> finish());
} else {
binding.btnGoToRecovery.setOnClickListener(view -> ConnectManager.registerUser(this, success -> {
if (success) {
ConnectManager.goToConnectJobsList(this);
}
})
);
}
}

private void handleInvitation(String callBackUrl, String inviteCode) {
IApiCallback callback = new IApiCallback() {
@Override
public void processSuccess(int responseCode, InputStream responseData) {
binding.progressBar.setVisibility(View.GONE);
try {
String responseAsString = new String(StreamsUtil.inputStreamToByteArray(responseData));
if (responseAsString.length() > 0) {
startActivity(new Intent(HQUserInviteActivity.this, DispatchActivity.class));
finish();
}
} catch (IOException e) {
Logger.exception("Parsing return from OTP request", e);
}
}

@Override
public void processFailure(int responseCode, IOException e) {
binding.progressBar.setVisibility(View.GONE);
String message = "";
if (responseCode > 0) {
message = String.format(Locale.getDefault(), "(%d)", responseCode);
} else if (e != null) {
message = e.toString();
}
setErrorMessage("Error requesting SMS code" + message);
}

@Override
public void processNetworkFailure() {
binding.progressBar.setVisibility(View.GONE);
setErrorMessage(getString(R.string.recovery_network_unavailable));
}

@Override
public void processOldApiError() {
binding.progressBar.setVisibility(View.GONE);
setErrorMessage(getString(R.string.recovery_network_outdated));
}
};
ConnectUserRecord user = ConnectManager.getUser(this);
binding.progressBar.setVisibility(View.VISIBLE);
boolean isBusy = !ApiConnectId.hqUserInvitation(HQUserInviteActivity.this,user.getUserId(),user.getPassword(), callBackUrl, inviteCode, callback);
if (isBusy) {
Toast.makeText(HQUserInviteActivity.this, R.string.busy_message, Toast.LENGTH_SHORT).show();
}
}
Comment on lines +100 to +146
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error messaging & handle potential null connectIdToken
Before calling connectIdToken.toString() (line 432), verify that connectIdToken is not null to avoid a NullPointerException. Also, consider providing more user-friendly or localized error messages when setErrorMessage is called, to help guide users on how to recover from invitation errors.


public void setErrorMessage(String message) {
if (message == null) {
binding.connectPhoneVerifyError.setVisibility(View.GONE);
} else {
binding.connectPhoneVerifyError.setVisibility(View.VISIBLE);
binding.connectPhoneVerifyError.setText(message);
}
}

@Override
protected void onDestroy() {
super.onDestroy();
binding = null;
}

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
super.onActivityResult(requestCode, resultCode, data);
finish();
ConnectManager.handleFinishedActivity(this, requestCode, resultCode, data);
}
}
Loading