proyecto26 / react-native-inappbrowser

πŸ“±InAppBrowser for React Native (Android & iOS) 🀘
https://www.npmjs.com/package/react-native-inappbrowser-reborn
MIT License
1.32k stars 227 forks source link

Why the need for `QUERY_ALL_PACKAGES` permission? #311

Closed petlys closed 2 years ago

petlys commented 2 years ago

Could you elaborate on the need for the QUERY_ALL_PACKAGES permission? This is quite a scary permission for many users, and a dealbreaker for us.

jdnichollsc commented 2 years ago

Hey @petlys, what Android options are you using? πŸ‘

petlys commented 2 years ago

Hey @petlys, what Android options are you using? πŸ‘

None. Just InAppBrowser.open(url)

But it doesn't matter does it? The problem is that the project itself requires it in order to query which system browser to use here

jdnichollsc commented 2 years ago

Sure, you're right! We're now using that to detect if that browser is available <3

androideveloper commented 2 years ago

Hi @jdnichollsc . As this is considered a dangerous permission, apps that are using it need to explain the reason to Google Play. If I look the invalid use cases for it, there is one point mentioning: When the required task can be done with a less broad app-visibility method. I was wondering do we actually need this broad permission to check for available browsers that support custom tabs?

Per google documentation, for android 11 it's only required to provide the manifest queries section and from my previous experience with custom tabs, this was enough https://developer.chrome.com/docs/android/custom-tabs/integration-guide/#how-can-i-check-whether-the-android-device-has-a-browser-that-supports-custom-tab

jdnichollsc commented 2 years ago

@androideveloper we're using that permission since Android 11 to be able to return a list of apps that support android.support.customtabs.action.CustomTabsService intent, it looks like we need to use this other approach instead: https://developer.android.com/training/package-visibility/use-cases#check-browser-available

Any pull request is welcome! <3

androideveloper commented 2 years ago

Hey @jdnichollsc I did more investigation on this and the permission is not needed, as we already have

<intent>
  <action android:name="android.support.customtabs.action.CustomTabsService" />
</intent>

What you refer to is covered here https://developer.android.com/training/package-visibility/use-cases#open-urls-custom-tabs, so no need to change the approach. I tested it by removing the permission from our app manifest and testing with different browsers that have custom tabs.

I prepared a PR to remove it https://github.com/proyecto26/react-native-inappbrowser/pull/335 πŸ™

jdnichollsc commented 2 years ago

Hey @jdnichollsc I did more investigation on this and the permission is not needed, as we already have

<intent>
  <action android:name="android.support.customtabs.action.CustomTabsService" />
</intent>

What you refer to is covered here https://developer.android.com/training/package-visibility/use-cases#open-urls-custom-tabs, so no need to change the approach. I tested it by removing the permission from our app manifest and testing with different browsers that have custom tabs.

I prepared a PR to remove it #335 πŸ™

But are you testing with different browsers? what values are you testing with the browserPackage option from Android?

And why not use the recommendation from the official documentation to check if a browser is available?

<!-- Place inside the <queries> element. -->
<intent>
  <action android:name="android.intent.action.VIEW" />
  <category android:name="android.intent.category.BROWSABLE" />
  <data android:scheme="https" />
</intent>
androideveloper commented 2 years ago

But are you testing with different browsers? what values are you testing with the browserPackage option from Android?

I tested with different browsers (Chrome, Edge, Phoenix). I was checking isAvailable function without custom browserPackage option. But in this case you're also only interested in browsers that support custom tabs, right?

And why not use the recommendation from the official documentation to check if a browser is available?

The one that you're referring to is for checking if there are browsers available in general, but in this case and per your previous message we are interested only in browsers that support custom tabs. If we look at the section here

you might want to check whether the device has a browser that supports Custom Tabs, or select a specific browser to launch with Custom Tabs using CustomTabsClient.getPackageName(). In those cases, include the following element as part of the element in your manifest:

 <intent>
   <action android:name="android.support.customtabs.action.CustomTabsService" />
 </intent>
androideveloper commented 2 years ago

This is the place where you need permission for querying what is available on the device https://github.com/proyecto26/react-native-inappbrowser/blob/4c15018fb0942e95cd53d5866dbbb5daf3ff52c2/android/src/main/java/com/proyecto26/inappbrowser/RNInAppBrowser.java#L291 And to query it, having the mentioned intent above is enough

jdnichollsc commented 2 years ago

Oh sounds great, thanks for letting me know! <3

androideveloper commented 2 years ago

Thanks πŸš€ When are you planning the new release? 😊

rosskhanas commented 2 years ago

@jdnichollsc any chance to have this published to NPM? πŸ₯Ί

elliotdickison commented 2 years ago

@androideveloper Thanks for the fix!!

ghost commented 2 years ago

Hey @jdnichollsc! Will this get published in a new package update? I am using this in my app and can't publish without this getting removed.

imekinox commented 2 years ago

@nichol-pennell you can try this in th meantime:

Add this to your Android Manifest:

<uses-permission android:name="android.permission.QUERY_ALL_PACKAGES" tools:node="remove" tools:ignore="QueryAllPackagesPermission" />

tools:node="remove" Remove this element from the merged manifest. Although it seems like you should instead just delete this element, using this is necessary when you discover an element in your merged manifest that you don't need, and it was provided by a lower-priority manifest file that's out of your control (such as an imported library).

Rehubbard commented 2 years ago

@jdnichollsc @androideveloper thanks for the quick resolution. Any chance this can be released ASAP?

The Google Play policy on QUERY_ALL_PACKAGES takes affect today. It blocks all app updates from being published. It's a major concern, especially when this library is the only thing that uses that permission.

jdnichollsc commented 2 years ago

Hello folks, sorry for the delay I just come back from vacations, let me publish a new version ASAP Thanks for your patience ❀️

kuorus commented 2 years ago

Thank you for the quick reaction. We are here for help and emotional support 🀣

ghost commented 2 years ago

@imekinox this worked! Thanks for the quick solution :)

@nichol-pennell you can try this in th meantime:

Add this to your Android Manifest:

<uses-permission android:name="android.permission.QUERY_ALL_PACKAGES" tools:node="remove" tools:ignore="QueryAllPackagesPermission" />

ghost commented 2 years ago

hey @jdnichollsc is there any timeline on when this change will be in a new update? The fix above helped me to be able to publish but the inappbrowser doesn't work.

androideveloper commented 2 years ago

@nichol-pennell This change will just remove the permission, same as the temp fix does. When I removed it, I tested with different browsers. See my comment https://github.com/proyecto26/react-native-inappbrowser/issues/311#issuecomment-1071030450 What exactly doesn't work? It could be because of some other reason.

ghost commented 2 years ago

@androideveloper Instead of the inappbrowser opening in our app, it opens the link in a browser outside of our app now. And the only thing I changed was adding in the permission removal in the manifest

@nichol-pennell This change will just remove the permission, same as the temp fix does. When I removed it, I tested with different browsers. See my comment #311 (comment) What exactly doesn't work? It could be because of some other reason.

androideveloper commented 2 years ago

which browsers do you have installed? Does any of those support chrome custom tabs?

ghost commented 2 years ago

@androideveloper I have chrome installed

androideveloper commented 2 years ago

Could you try another browser? It might also be that you have some custom logic that needs QUERY_ALL_PACKAGES permission.

felipemengatto commented 2 years ago

Hey @jdnichollsc! Will this get published in a new package update?

amq commented 2 years ago

My workaround:

  1. install patch-package
  2. create a file in patches/:
diff --git a/node_modules/react-native-inappbrowser-reborn/android/src/main/AndroidManifest.xml b/node_modules/react-native-inappbrowser-reborn/android/src/main/AndroidManifest.xml
index 6aaceca..d0590b1 100644
--- a/node_modules/react-native-inappbrowser-reborn/android/src/main/AndroidManifest.xml
+++ b/node_modules/react-native-inappbrowser-reborn/android/src/main/AndroidManifest.xml
@@ -3,7 +3,6 @@
           package="com.proyecto26.inappbrowser">

     <uses-permission android:name="android.permission.INTERNET" />
-    <uses-permission android:name="android.permission.QUERY_ALL_PACKAGES" />

     <application>
         <activity
petlys commented 2 years ago

Been a while since I created this issue, but to follow up on it - I ended up just using react-native-webview inside a Modal. Super simple and much more flexible.

elliotdickison commented 2 years ago

We removed the permission with Android's manifest merging tools and have seen no adverse effects in production.

kuorus commented 2 years ago

Been a while since I created this issue, but to follow up on it - I ended up just using react-native-webview inside a Modal. Super simple and much more flexible.

Depending on the use case, this won't work. e.g. Facebook 3rd-party login won't allow a webview for security reasons.

markl-vesper commented 2 years ago

This workaround seemed to work fine but we also had to add below

<manifest xmlns:android="http://schemas.android.com/apk/res/android" xmlns:tools="http://schemas.android.com/tools" package="com.cortexmonitoring">

at top of file to prevent build failing

harveyconnor commented 2 years ago

When will the npm package be updated?

mahamaad commented 2 years ago

hey @jdnichollsc thank you for your package i appreciate, but we need an update before July 12,

thank you.

informsharique commented 2 years ago

Hey @jdnichollsc Thank you for this awesome library!! We also need to push an update to play store after new release. Can you please make a release with the fix so that we may not be delayed. App update should have to be approved before July 12

jdnichollsc commented 2 years ago

Hello folks, excuse me for the delay My Mac stopped working, I'm planning to release a new version as soon as possible, sorry for the inconvenience πŸ™

Regards, Juan

KarimMassadCubic commented 2 years ago

any ideas when the release will happen? We would also need the update in order to release apps which are currently in production / in the stores.

ArturoTorresMartinez commented 2 years ago

Any update?

compwron commented 2 years ago

https://xkcd.com/2347/

MoOx commented 2 years ago

See https://github.com/proyecto26/react-native-inappbrowser/pull/335 for followup

ana-nd-s commented 2 years ago

Hey @jdnichollsc, is there any update on this?

informsharique commented 2 years ago

I guess, we won't get a build in time. He might not be free. Better to use patch-package to patch it.

markl-vesper commented 2 years ago

The workaround that @imekinox has proposed works perfectly. No need to wait for a new release. Google Store has accepted our updated APK using the workaround to build it

ghost commented 2 years ago

I don't know if anyone is still dealing with this. But I wanted to add that the fix below worked for me. The issue I had was that we had other versions in testing that were also being flagged for this permission. I just had to add the newest version with this fix to all our testing versions and then I got rid of the flag in the console. <uses-permission android:name="android.permission.QUERY_ALL_PACKAGES" tools:node="remove" tools:ignore="QueryAllPackagesPermission" />

laurenzcodes commented 2 years ago

The workaround with adding the line to AndroidManifest.xml does not work for me. I still get the same error in my fastlane pipeline.

android/app/src/main/AndroidManifest.xml

<manifest xmlns:android="http://schemas.android.com/apk/res/android" xmlns:tools="http://schemas.android.com/tools" package="...">

  <uses-permission android:name="android.permission.INTERNET" />
  <uses-permission android:name="android.permission.QUERY_ALL_PACKAGES" tools:node="remove" tools:ignore="QueryAllPackagesPermission" />
...

Getting the error Google Api Error: Invalid request - This release includes the QUERY_ALL_PACKAGES permission, which hasn't been declared in Play Console.

Please release a version to npm with the fix from #335

RayHughes commented 2 years ago

The workaround with adding the line to AndroidManifest.xml does not work for me. I still get the same error in my fastlane pipeline.

android/app/src/main/AndroidManifest.xml

<manifest xmlns:android="http://schemas.android.com/apk/res/android" xmlns:tools="http://schemas.android.com/tools" package="...">

  <uses-permission android:name="android.permission.INTERNET" />
  <uses-permission android:name="android.permission.QUERY_ALL_PACKAGES" tools:node="remove" tools:ignore="QueryAllPackagesPermission" />
...

Getting the error Google Api Error: Invalid request - This release includes the QUERY_ALL_PACKAGES permission, which hasn't been declared in Play Console.

Please release a version to npm with the fix from #335

Having the same issue with Bitrise using both the patch method and the lines added to the manifest to remove.

zholmes1 commented 2 years ago

Better to just learn how to use the patch-package than copying the patches from here. It's very easy.

  1. Navigate to node_modules/react-native-inappbrowser-reborn/android/src/main/AndroidManifest.xml
  2. Remove the Query All Packages permission
  3. Do npx patch-package. This will auto generate the patch file for you.
  4. Add "postinstall": "npx patch-package" to your scripts. This will run your patches every time you install.

It's a very useful tool for situations like this where a repo maintainer is unable to make a required change on time.

sinn1 commented 2 years ago

The workaround with adding the line to AndroidManifest.xml does not work for me. I still get the same error in my fastlane pipeline. android/app/src/main/AndroidManifest.xml

<manifest xmlns:android="http://schemas.android.com/apk/res/android" xmlns:tools="http://schemas.android.com/tools" package="...">

  <uses-permission android:name="android.permission.INTERNET" />
  <uses-permission android:name="android.permission.QUERY_ALL_PACKAGES" tools:node="remove" tools:ignore="QueryAllPackagesPermission" />
...

Getting the error Google Api Error: Invalid request - This release includes the QUERY_ALL_PACKAGES permission, which hasn't been declared in Play Console. Please release a version to npm with the fix from #335

Having the same issue with Bitrise using both the patch method and the lines added to the manifest to remove.

I had the same problem.

You may be experiencing the same issue with solution here: https://github.com/proyecto26/react-native-inappbrowser/issues/363#issuecomment-1192074701

mohamed2m2018 commented 2 years ago

@jdnichollsc please can you release this?

zholmes1 commented 2 years ago

@mohamed2m2018 As far as I know there is no commit or pull request that fixes this. You need to do it yourself with either patch-package to fix the manifest file for this package, or edit your own manifest to override and remove all attempts to add the permission. Both solutions should work fine. After a build you can drag the APK/aab onto Android Studio to inspect the package and make sure the permission is actually gone from the final manifest

mohamed2m2018 commented 2 years ago

@zholmes1 yes maybe I will do this, but it's natural to have a new release with this since I see this pr: https://github.com/proyecto26/react-native-inappbrowser/pull/335 and it's merged since Mar 17