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

fix(android): remove READ_PHONE_STATE permission #232

Merged
merged 2 commits into from
Apr 27, 2022

Conversation

adriano-di-giovanni
Copy link
Contributor

Platforms affected

  • Android

Motivation and Context

The plugin uses the OnAudioFocusChangeListener to listen to audio focus changes such as the ones that happen on incoming calls. There is no need to request the android.permission.READ_PHONE_STATE because it is not needed.
I think it's good to remove such permission because it has a dangerous protection level.

Description

Removed <uses-permission android:name="android.permission.READ_PHONE_STATE" /> frrom plugin.xml

@janpio janpio added this to 🐣 New PR / Untriaged in Apache Cordova: Plugin Pull Requests Jul 2, 2019
@janpio janpio moved this from 🐣 New PR / Untriaged to ⛔ Blocked: Tests failing in Apache Cordova: Plugin Pull Requests Jul 2, 2019
@janpio janpio changed the title android: remove READ_PHONE_STATE permission fix(android): remove READ_PHONE_STATE permission Jul 4, 2019
@janpio janpio moved this from ⛔ Blocked: Tests failing to ⏳ Ready for Review in Apache Cordova: Plugin Pull Requests Jul 4, 2019
@erisu erisu added this to the 6.0.0 milestone Oct 22, 2021
@breautek breautek added this to In progress in 6.x via automation Apr 12, 2022
Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

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

The changes looks good and the description about Protection level: dangerous is accurate. This permission should be drop.

Apache Cordova: Plugin Pull Requests automation moved this from ⏳ Ready for Review to ✅ Approved, waiting for Merge Apr 27, 2022
6.x automation moved this from In progress to Reviewer approved Apr 27, 2022
Copy link

@breautek breautek left a comment

Choose a reason for hiding this comment

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

Android docs states:

Allows read only access to phone state, including the current cellular network information, the status of any ongoing calls, and a list of any PhoneAccounts registered on the device.

READ_PHONE_STATE was added/released in version 0.2.7 with no attached issue or jira ticket neither in the release notes or in the commit

Furthermore, even though the permission was declared, I don't see anywheres where it actually requests the permission. Not sure what was the intent of it being added but I agree it should be removed.

@erisu erisu merged commit baf2d78 into apache:master Apr 27, 2022
Apache Cordova: Plugin Pull Requests automation moved this from ✅ Approved, waiting for Merge to 🏆 Merged, waiting for Release Apr 27, 2022
6.x automation moved this from Reviewer approved to Done Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Apache Cordova: Plugin Pull Requests
🏆 Merged, waiting for Release
6.x
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants