Don't check for Intent.CATEGORY_BROWSABLE in isFullBrowser

[kristerkari]

I have seen from our company's Sentry that some users have been getting the ActivityNotFoundException error from AppAuth-Android. The reason is that the users are using Opera as the default browser, and AppAuth-Android does not seem to handle that properly.

The main reason seems to be that Opera does not expose Intent.CATEGORY_BROWSABLE and because of that it's not marked as a full browser and gets dropped of the browser listing.

I'm not sure if this is a good way of fixing the issue, but it at least should fix the problem. Let me know if there is some other way to handle the support for Opera.

Fixes #452

@iainmcgin

[codecov-io]

Codecov Report

Merging #469 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #469      +/-   ##
============================================
- Coverage     83.71%   83.66%   -0.05%     
+ Complexity      479      478       -1     
============================================
  Files            42       42              
  Lines          2345     2344       -1     
  Branches        233      232       -1     
============================================
- Hits           1963     1961       -2     
- Misses          299      300       +1     
  Partials         83       83

Impacted Files	Coverage Δ	Complexity Δ
...va/net/openid/appauth/browser/BrowserSelector.java	87.69% <ø> (-1.71%)	19 <0> (-1)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f125925...06937a1. Read the comment docs.

[childnode]

1. android.intent.category.APP_BROWSER might be used to identify full Browsers
2. ⚠️ intents having CATEGORY_APP_BROWSER do not always have intent filters for a http(s) scheme
3. remove query for BROWSABLE is correct for me, because this marks activities that might be called FROM a browser.

• Instead there is a new <action android:name="androidx.browser.browseractions.browser_action_open" /> intent filter that is used at least in chrom(e|ium)

4. guide https://developer.chrome.com/multidevice/android/customtabs should be read carefully
   I'm not sure anything is respected in AppAuth atm
   https://developer.chrome.com/multidevice/android/customtabs#basics-for%20launching%20custom%20tabs%20using%20the%20low%20level%20api

• do we use the latest https://developer.android.com/topic/libraries/support-library/features#custom-tabs ?
• do we respect things like "opt-out" https://github.com/GoogleChrome/custom-tabs-client/blob/master/customtabs/src/android/support/customtabs/CustomTabsIntent.java#L64

[kristerkari]

Thanks @childnode!

I'm not an Android developer myself, so I'm not familiar with the browser detection on native side. I'm only trying to get the Opera issue solved as it happening for our users.

@childnode would you be able to help me a bit to get the issue solved? I can do the changes, but I would just need to know where to check what.

So, did I understand correctly that the isFullBrowser should check for CATEGORY_APP_BROWSER instead? I did not fully understand what you say in 2. and what you are saying about the intent filter for Chromium.

[childnode]

hi @kristerkari , I'm actually in android dev but not that familar with the details of CustomTabs yet but struggling different issues with that as you do. So,

did I understand correctly that the isFullBrowser should check for CATEGORY_APP_BROWSER instead?

I think the query for fullBrowser is totally obsolete because the BROWSER_INTENT used for querying the right activities includes (implicit) check for the same thing on with your removal of the incorrect intent category browsable it is always true for any iterables from resolvedActivityList

We might / should check the package for other activities that at least one has the CATEGORY_APP_BROWSER added to that indicates this is a real browser app but not just anyone who can view http sites ... yes, this might be a good replacement.

as I read https://developer.chrome.com/multidevice/android/customtabs#preparing-for%20other%20browsers it became obvious that hasWarmupService is a misleading method name because this does the code that queries for customTabs services.
The "Warmup" of an activity is done at another level https://github.com/GoogleChrome/custom-tabs-client/blob/master/customtabs/src/android/support/customtabs/CustomTabsClient.java#L171
as I understand it for now.
-> we should rename "hasWarmupService" into "hasCustomTabsService". That's correct how it is used in

AppAuth-Android/library/java/net/openid/appauth/browser/BrowserSelector.java

Line 110 in 06937a1

if (hasWarmupService(pm, info.activityInfo.packageName)) {

another quirk code page is

AppAuth-Android/library/java/net/openid/appauth/browser/BrowserSelector.java

Line 116 in 06937a1

browsers.add(defaultBrowserIndex, customTabBrowserDescriptor);

where default browser is set first in the list.

Proof me wrong, but default browser overrides first (and second, if it has customTabs support) entry in the browsers list even there is already a browser contained in.
-> if there are two browsers, it might become you only get one of them ?!

---

ℹ️
actually we have had some information about, that certain security apps are changing the appAuth behavior! Perhaps GData internet security is resolved as browser. Opera is then not retrieved as browser

second we do have opera users, where getAllBrowsers only returns the fullBrowser descriptor but none for the customTabs service. on other devices (same OS, same device type, same Opera version) we do get the customTabs service as a second descriptor as expected ;/ << any hints welcome which might cause this

additionally we do have users, where not all browsers are reported, but this might be related to #459

[childnode]

Another implementations are

• https://github.com/uber/rides-android-sdk/blob/e37dbbc4d5367e5edc8c356d10be8f3d0071ecdf/core-android/src/main/java/com/uber/sdk/android/core/utils/CustomTabsHelper.java
• https://github.com/auth0/Auth0.Android/blob/ec931bb2e42e5a6e06448d1c9b7fd00539ca2724/auth0/src/main/java/com/auth0/android/provider/CustomTabsController.java#L165
• https://github.com/GoogleChrome/custom-tabs-client/blob/master/shared/src/main/java/org/chromium/customtabsclient/shared/CustomTabsHelper.java#L64
  ** interesting patch some days ago: GoogleChrome/custom-tabs-client@7a2c137 which shows up another difference to

  AppAuth-Android/library/java/net/openid/appauth/browser/BrowserSelector.java

  Line 82 in 06937a1

  int queryFlag = PackageManager.GET_RESOLVED_FILTER;

  in PackageManager.MATCH_DEFAULT_ONLY | PackageManager.MATCH_ALL while we use PackageManager.GET_RESOLVED_FILTER | PackageManager.MATCH_ALL

none of them check for CATEGORY_BROWSEABLE or any schemes filter
what comes next to our isFullBrowser(ResolveInfo) is CustomTabsHelper#hasSpecializedHandlerIntents(Context, Intent) while I don't get why this code is not in the customTabs library itself -> sourceCode..

the more you dig into, the more you see this CustomTab Thingy is not really straight forward as it seems ;/

Another thing that I actually see is, that we don't use the from the CustomTabs Support Library native methods like android.support.customtabs.CustomTabsClient#getPackageName(android.content.Context, java.util.List<java.lang.String>) but have our own code in BrowserSelector to determine packages ...

[agologan]

@kristerkari you were to look into the BROWSABLE category but the problem was on Opera's side.
During a recent PR we discussed the same issue: #613 (comment) and after checking Opera have fixed their implementation.

Per https://developer.android.com/training/basics/intents/package-visibility-use-cases#check-browser-available checking BROWSABLE is the right thing to do, and browser apps should ensure they declare their intents correctly.