Skip to content

fix(#401): update OIDC handling to use custom tabs #405

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 23, 2025
Merged

fix(#401): update OIDC handling to use custom tabs #405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1dfd215
fix: update OIDC handling to use custom tabs
jkuester Jul 11, 2025
a58a553
Fix linting
jkuester Jul 11, 2025
24ee889
Remove AppUrlIntentActivity and launch the EmbeddedBrowserActivity di…
jkuester Jul 12, 2025
cdc8c09
Fix linting
jkuester Jul 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ android {
dependencies {
implementation fileTree(dir: 'libs', include: ['*.jar'])
implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.24')
implementation 'androidx.browser:browser:1.8.0'
implementation 'androidx.core:core:1.13.1'
implementation 'androidx.activity:activity:1.9.0'
implementation 'androidx.fragment:fragment:1.7.1'
Expand Down
25 changes: 12 additions & 13 deletions src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,18 @@
</intent-filter>
</activity>
<activity android:name="EmbeddedBrowserActivity"
android:screenOrientation="portrait"
android:configChanges="orientation|screenSize"
tools:ignore="DiscouragedApi"/>
android:screenOrientation="portrait"
android:configChanges="orientation|screenSize"
android:launchMode="singleTask"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One subtle change here is switching the launchMode from singleInstance to singleTask. As far as I could tell, we always had singleInstace set for the AppUrlIntentActivity, but just reading the documentation, it feels like that is not really the right choice. With the way I currently understand things, singleTask will allow us to always re-use the existing instance of EmbeddedBrowserActivity and continue the task chain as normal. The singleInstance docs note that any additional activities launched off of the root are not part of the same task chain.

TLDR, as far as I can tell, singleTask is "more normal" than singleInstance and we get the desired behavior.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, this was a good read. I remember we had one version of the App that ended up launching multiple instances. Of course the background instances could not be reached or removed, and the only way they disappeared was when the whole app was killed.
:(

singleTask sounds like would not create a new activity for our use case, but we should be careful none the less.

android:exported="true"
tools:ignore="DiscouragedApi">
<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="@string/scheme" android:host="@string/app_host" android:pathPattern=".*"/>
Copy link
Member

Choose a reason for hiding this comment

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

I believe that this section was added for token login. Does this still open the existent activity when an intent is triggered from SMS?

Copy link
Contributor Author

@jkuester jkuester Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I believe that this section was added for token login.

Correct. This is how token login links (and really any other kind of deep-linking) works.

Does this still open the existent activity when an intent is triggered from SMS?

Yes, this is key and is the whole point of the launchMode="singleTask" configuration. With that launchMode, (and with the previous singleInstance), if we have an existing task instance of EmbeddedBrowserActivity, then incoming intents will be directed into that task (instead of creating a new one). If we did not set the launchMode, then by default Android I think Android would launch a separate instance of cht-android each time a deep-link is tapped.

</intent-filter>
</activity>
<activity android:name="ConnectionErrorActivity"
android:screenOrientation="portrait"
android:configChanges="orientation|screenSize"
Expand All @@ -65,16 +74,6 @@
<activity android:name="DomainVerificationActivity"
android:screenOrientation="portrait"
tools:ignore="DiscouragedApi" />
<activity android:name="AppUrlIntentActivity"
android:launchMode="singleInstance"
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="@string/scheme" android:host="@string/app_host" android:pathPattern=".*"/>
</intent-filter>
</activity>
<activity android:name="UpgradingActivity"
android:screenOrientation="portrait"
android:configChanges="orientation|screenSize"
Expand Down
22 changes: 0 additions & 22 deletions src/main/java/org/medicmobile/webapp/mobile/AppUrlIntentActivity.java
View file
Open in desktop

This file was deleted.

21 changes: 16 additions & 5 deletions src/main/java/org/medicmobile/webapp/mobile/EmbeddedBrowserActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,33 @@ public void onReceiveValue(String result) {

enableUrlHandlers(container);

Intent appLinkIntent = getIntent();
Uri appLinkData = appLinkIntent.getData();
browseTo(appLinkData);

if (settings.allowsConfiguration()) {
toast(redactUrl(appUrl));
}

registerRetryConnectionBroadcastReceiver();

String recentNavigation = settings.getLastUrl();
if (isValidNavigationUrl(appUrl, recentNavigation)) {
Intent appLinkIntent = getIntent();
Uri appLinkData = appLinkIntent.getData();
if (appLinkData != null) {
// The app has been opened via an app link.
browseTo(appLinkData);
} else if (isValidNavigationUrl(appUrl, recentNavigation)) {
// The app has been opened normally, and the user can start where they left off.
container.loadUrl(recentNavigation);
} else {
// The app has been opened normally, but no previous URL is available. (Maybe it is the first time.)
browseTo(null);
}
}

@Override
protected void onNewIntent(Intent intent) {
Uri appLinkData = intent.getData();
browseTo(appLinkData);
Copy link
Member

Choose a reason for hiding this comment

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

is this what makes token login not launch a new instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, it is the launchMode configuration that "makes" a new instance not get launched. But, yes, this is the code that receives the incoming intent for an existing instance of EmbeddedBrowserActivity when a token login link is tapped.

One thing to note is that based on my testing, there is a bit of a difference between the code flows for launching an app link when no cht-android instance is currently running, vs when you already have an instance running in the background or something. In the first case, the onCreate method above is called and the app-url intent is handled by the code at line 128. onNewIntent is not called when onCreate is called. In the second case (when an instance of cht-android is already running in the background), the incoming app-url intent triggers the onNewIntent method, but the onCreate is not called (since we are not creating a new instance of EmbeddedBrowserActivity. Since both methods ultimately end up called browseTo, the functionality/UX of what happens remains the same either way.

}

@SuppressWarnings("PMD.CallSuperFirst")
@Override
protected void onStart() {
Expand Down
13 changes: 12 additions & 1 deletion src/main/java/org/medicmobile/webapp/mobile/UrlHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import android.webkit.WebView;
import android.webkit.WebViewClient;

import androidx.browser.customtabs.CustomTabsIntent;

public class UrlHandler extends WebViewClient {
EmbeddedBrowserActivity parentActivity;
SettingsStore settings;
Expand All @@ -31,10 +33,14 @@ public UrlHandler(EmbeddedBrowserActivity parentActivity, SettingsStore settings
@Override
public boolean shouldOverrideUrlLoading(WebView view, WebResourceRequest request) {
Uri uri = request.getUrl();
if (isUrlRelated(this.settings.getAppUrl(), uri) || isOidcProviderUrl(uri)) {
if (isUrlRelated(this.settings.getAppUrl(), uri)) {
// Load all related URLs in the WebView
return false;
}
if (isOidcProviderUrl(uri)) {
launchCustomTab(uri);
return true;
}

// Let Android decide what to do with unrelated URLs
// unrelated URLs include `tel:` and `sms:` uri schemes
Expand All @@ -43,6 +49,11 @@ public boolean shouldOverrideUrlLoading(WebView view, WebResourceRequest request
return true;
}

private void launchCustomTab(Uri uri) {
CustomTabsIntent customTabsIntent = new CustomTabsIntent.Builder().build();
customTabsIntent.launchUrl(this.parentActivity, uri);
}

/**
* Support OIDC login flow by allowing any URL that contains the current app_url as the redirect_uri.
*/
Expand Down
24 changes: 24 additions & 0 deletions src/test/java/org/medicmobile/webapp/mobile/EmbeddedBrowserActivityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.robolectric.Shadows.shadowOf;

import android.content.Intent;
import android.net.Uri;

import androidx.core.content.ContextCompat;
import androidx.test.espresso.intent.Intents;
Expand Down Expand Up @@ -273,4 +276,25 @@ public void onActivityResult_unknownRequestCode_logRequestCode(){
});
}
}

@Test
public void onNewIntent() {
Intent intent = mock(Intent.class);
String url = "https://example.com";
when(intent.getData()).thenReturn(Uri.parse(url));
try(MockedStatic<MedicLog> medicLogMock = mockStatic(MedicLog.class)) {
scenarioRule
.getScenario()
.onActivity(embeddedBrowserActivity -> {
embeddedBrowserActivity.onNewIntent(intent);

verify(intent, times(1)).getData();
medicLogMock.verify(() -> MedicLog.trace(
eq(embeddedBrowserActivity),
eq("Pointing browser to: %s"),
eq(url)
));
});
}
}
}
40 changes: 28 additions & 12 deletions src/test/java/org/medicmobile/webapp/mobile/UrlHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package org.medicmobile.webapp.mobile;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockConstruction;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand All @@ -14,14 +16,18 @@
import android.webkit.WebResourceRequest;
import android.webkit.WebView;

import androidx.browser.customtabs.CustomTabsIntent;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.MockedConstruction;
import org.robolectric.RobolectricTestRunner;

@RunWith(RobolectricTestRunner.class)
public class UrlHandlerTest {
private static final String APP_URL = "https://project-abc.medic.org";
private EmbeddedBrowserActivity parentActivity;
private SettingsStore settingsStore;
private WebView webView;
private WebResourceRequest webResourceRequest;
Expand All @@ -30,6 +36,7 @@ public class UrlHandlerTest {

@Before
public void setup() {
parentActivity = mock(EmbeddedBrowserActivity.class);
settingsStore = mock(SettingsStore.class);
when(settingsStore.getAppUrl()).thenReturn(APP_URL);
webView = mock(WebView.class);
Expand All @@ -38,7 +45,7 @@ public void setup() {
doNothing().when(context).startActivity(any());
webResourceRequest = mock(WebResourceRequest.class);

handler = new UrlHandler(null, settingsStore);
handler = new UrlHandler(parentActivity, settingsStore);
}

@Test
Expand Down Expand Up @@ -69,17 +76,26 @@ public void shouldOverrideUrlLoading_withExternalUrl() {

@Test
public void shouldOverrideUrlLoading_withExternalOidcProviderUrl() {
when(webResourceRequest.getUrl()).thenReturn(
Uri.parse("some-external-url.com?redirect_uri=" + Uri.encode(APP_URL + "/medic/login/oidc"))
);

boolean result = handler.shouldOverrideUrlLoading(webView, webResourceRequest);

assertFalse(result);
verify(settingsStore, times(2)).getAppUrl();
verify(webResourceRequest).getUrl();
verify(webView, times(0)).getContext();
verify(context, times(0)).startActivity(any());
CustomTabsIntent intent = mock(CustomTabsIntent.class);
Uri expectedUri = Uri.parse("some-external-url.com?redirect_uri=" + Uri.encode(APP_URL + "/medic/login/oidc"));
try (MockedConstruction<CustomTabsIntent.Builder> mocked = mockConstruction(
CustomTabsIntent.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(intent)
)) {
doNothing().when(intent).launchUrl(any(), any());
when(webResourceRequest.getUrl()).thenReturn(expectedUri);

boolean result = handler.shouldOverrideUrlLoading(webView, webResourceRequest);

assertTrue(result);
verify(settingsStore, times(2)).getAppUrl();
verify(webResourceRequest).getUrl();
verify(webView, times(0)).getContext();
verify(context, times(0)).startActivity(any());
assertEquals(1, mocked.constructed().size());
verify(mocked.constructed().get(0), times(1)).build();
verify(intent, times(1)).launchUrl(parentActivity, expectedUri);
}
}

@Test
Expand Down