nickoneill / PermissionScope

Intelligent iOS permissions UI and unified API
MIT License
4.85k stars 507 forks source link

Too many permissions for AppStore #194

Open MaeseppTarvo opened 7 years ago

MaeseppTarvo commented 7 years ago

Hey. If you are using PermissionScope with Xcode 8 GM and try to upload it to appstore you are gonna get errors like:

This app attempts to access privacy-sensitive data without a usage description. The app's Info.plist must contain an NSContactsUsageDescription key with a string value explaining to the user how the app uses this data.

This app attempts to access privacy-sensitive data without a usage description. The app's Info.plist must contain an NSCalendarsUsageDescription key with a string value explaining to the user how the app uses this data.

This app attempts to access privacy-sensitive data without a usage description. The app's Info.plist must contain an NSBluetoothPeripheralUsageDescription key with a string value explaining to the user how the app uses this data.

This app attempts to access privacy-sensitive data without a usage description. The app's Info.plist must contain an NSMicrophoneUsageDescription key with a string value explaining to the user how the app uses this data.

This app attempts to access privacy-sensitive data without a usage description. The app's Info.plist must contain an NSMotionUsageDescription key with a string value explaining to the user how the app uses this data.

For some reason it thinks that I ask all those permissions. I even do not use any of these. Just the notification one.

nickoneill commented 7 years ago

Thanks @MaeseppTarvo, I'll add this to the readme. At the very least you should be able to add those string values and get past submission.

@bre7 Perhaps it's really time to do subspecs for each permission :/ Not sure how Carthage could work similarly.

bre7 commented 7 years ago

One subspec per permission seems....too much. Isn't there a way to use weak linking like in ObjC ?

sansari commented 7 years ago

+1, seeing this as well.

Feels really weird to be adding usage descriptions for permissions we don't even use (Apple is already so picky about exactly how one uses different capabilities/permissions, so it makes me a bit nervous to "declare" we're using all of these).

robbiet480 commented 7 years ago

:+1: Also just had a build rejected for the same reason.

bre7 commented 7 years ago

For now I'd suggest forking and removing unused permissions (or editing the Pods project) 😞 Not sure if there's a perfect solution to these problem

robbiet480 commented 7 years ago

Agree with @bre7. This feels pretty bad to add all these keys to my Info.plist, especially since i'm not using any of these. I think i'm going to remove PermissionScope for the time being until a fix can be determined.

@nickoneill I'd suggest putting a giant warning in the README. I have been waiting for my build for almost 2 hours and called Developer Support who also had no idea what was going on, and then thought to check my email which is when I found the problem.

bre7 commented 7 years ago

There are some issues in SO like these:

[...] AdMob seems to have references to the Calendar so an app I have which uses ads, and does not use the Calendar itself, can't be submitted. Thanks Google!

A possible "solution" (albeit hacky) would be to enter all the required keys in Info.plist (it's even suggested by AdMob's team: https://groups.google.com/forum/#!msg/google-admob-ads-sdk/UmeVUDrcDaw/HIXR0kjUAgAJ)

robbiet480 commented 7 years ago

If anyone needs it, I pushed a branch that's compatible with Swift 3/iOS 10 and uses the new UserNotifications framework. It only supports notifications and location. It's available here.

h3smith commented 7 years ago

While more work, perhaps the correct solution is a core cocoapod, with the logic for each individual permission in its own pod. Or, while hacky if you are building from source there are always preprocessor flags you could wrap around each permission.

yuvalt commented 7 years ago

I know it won't be the prettiest or swift friendly option -- but what about using preprocessor flag per type of permission and then having to include the list of preprocessors in the project file?

nickoneill commented 7 years ago

@yuvalt We're not even sure what's causing the requirement at the moment - is it just linking the frameworks or the presence of the code itself? Waiting for more information right now.

robbiet480 commented 7 years ago

@nickoneill It's the presence of the code itself. I didn't remove any framework links and my builds passed. I bet that Apple is looking for the import statement or specific classes.

bre7 commented 7 years ago

Yeah. The frameworks are being linked because of the import statements.

robbiet480 commented 7 years ago

Shows how new I am to Swift 😿. Sorry @nickoneill

nickoneill commented 7 years ago

I'm filing a code-level support request in an attempt to get more information on the issue.

MaeseppTarvo commented 7 years ago

I think the problem is actually much Apple sided. I think that if you have whatever piece of line about some permission inside code it just detects it and check if it is allowed. So I think there is nothing to do.

h3smith commented 7 years ago

They are performing static analysis and finding the calls for the specific permissions and noticing the lack of the description in the plist file. Nothing more than that and you aren't going to get around it.

yuvalt commented 7 years ago

That's why I think preprocessing is the only way to go (need to be verified). So we can add a preprocessor definition like PS_INDIVIDUAL_PERMISSIONS and then PS_LOCATION, PS_CONTACTS etc... this way we can make sure that only the relevant code gets compiled. I can implement it quickly if people think it's the right approach.

emreavsar commented 7 years ago

also looking for a fix.

nickoneill commented 7 years ago

@yuvalt I'm interested in this approach but wary of asking people to create flags in build settings because it's not always straightforward. Can you think of another way to turn preprocessor definitions on and off?

@emreavsar Your best bet in the short term is adding those plist items for the permissions that you're not using. Users will never see them until they're requested (which they never are) so no impact on your app.

emreavsar commented 7 years ago

@nickoneill thanks for the quick answer. Yep this looks like the quick-fix for the moment. Are you 100% sure that they're not listed somewhere but only requesting? (App Store Entry, Settings etc.)?

Thanks anyway. Looking for the update mate!

nickoneill commented 7 years ago

They're not listed in the app settings until they're requested. The code hasn't changed, it's strictly a change in Apple's review process.

yuvalt commented 7 years ago

@nickoneill I hear you, but that's the only thing I could think of. The user will need to define it under Build Settings -> Swift Compiler -> Custom Flags (it's not under Basic but rather under All). Painful. Anyway, all the could be optional and people can still just include all the descriptions in their Info.plist or just opt to use this. I'll be happy to issue a pull-request for this, if you're interested.

h3smith commented 7 years ago

The only solution is that preprocessor flags have to be defined for the build process. There really isn't a way around it. Sadly you can't #define at the top of the .swift file because that would take care of it and make it easier.

But, it'll allow you to pick and choose which support you want which could be seen as a huge plus instead of having everything in there...

nickoneill commented 7 years ago

Hey friends, I got a suggested fix from Apple support, importing the framework is what causes the detection for API usage so give me your thoughts on this: each permission will have a single framework associated with it and you would have to import the framework specific to the permission that you're requesting, such as import PermissionScopeCamera, import PermissionScopeNotification, etc.

robbiet480 commented 7 years ago

@nickoneill What would the Podfile look like in that case?

bre7 commented 7 years ago

One Pod per permission ?

robbiet480 commented 7 years ago

Why not a subspec for each framework instead of an entire Pod?

nickoneill commented 7 years ago

@bre7 I'm not convinced this is required... if we include many frameworks in a single podspec but don't import them anywhere, I've been given the impression that it will pass app review.

bre7 commented 7 years ago

Meant subspec :D

@bre7 I'm not convinced this is required... if we include many frameworks in a single podspec but don't import them anywhere, I've been given the impression that it will pass app review.

Interesting. Guess we'll need to test it

basememara commented 7 years ago

Adding the privacy entries in the plist is definitely not a long-term solution. You would be misrepresenting your app in some way and you'll have more explaining to do with Apple. Also down the road who knows what those will be used for (maybe a popup like Android).

I totally support using compiler flags, the code isn't in the compilation this way.

Segmenting the code seems overkill for a relatively small lib. However, like @h3smith mentioned, there can be a core dependency and each permission would be a new, stand-alone library. I can see grabbing only notifications and bluetooth libs for example and leaving the rest; PermissionScope can be more of a suite of libs.

Panajev commented 7 years ago

Sent from my iPhone

On 21 Sep 2016, at 02:20, Basem Emara notifications@github.com wrote:

Adding the privacy entries in the plist is definitely not a long-term solution. You would be misrepresenting your app in some way and you'll have more explaining to do with Apple. Also down the road who knows what those will be used for (maybe a popup like Android).

I totally support using compiler flags, the code isn't in the compilation this way.

Segmenting the code seems overkill for a relatively small lib. However, like @h3smith mentioned, there can be a core dependency and each permission would be a new, stand-alone library. I can see grabbing only notifications and bluetooth libs for example and leaving the rest; PermissionScope can be more of a suite of libs.

+1

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

ValiDraganescu commented 7 years ago

I added the requested descriptions in my plist but I still get the error after the app processing starts in iTunes Connect

ValiDraganescu commented 7 years ago

I think I've just found something interesting here. I added all the requested descriptions in my plist and still got rejected for the same thing. I removed PermissionScope from my pods and implemented my own permissions. Good thing it'a a new app and I only need location and camera access.

Now, I implemented the location and camera access but I also had the photos gallery description in the plist file, I archived and pushed the app to iTunes. Now guess what, iTunes is complaining about this exact not used feature saying : The app's Info.plist must contain an NSPhotoLibraryUsageDescription key with a string value explaining to the user how the app uses this data. But I'm sure it's there because I just put it there. I removed that description from my plist file and I'm pushing to iTunes as I write this, I'll keep you posted with what happens next.

Later edit: Nope, it's not that ....

robbiet480 commented 7 years ago

@ValiDraganescu Is there another Pod that uses it? I had to really dig through all my Pods to find even the smallest mention of photos. Also, UIImagePicker will cause that error. I found UIImagePicker in PromiseKit as an example of unexpected libraries which may cause problems.

seboslaw commented 7 years ago

@ValiDraganescu are you using ReactiveCocoa? It also has a UIImagePicker binding that causes this rejection at the moment.

ValiDraganescu commented 7 years ago

@robbiet480, @seboslaw I am using UIImagePicker because I really need it :). I did all the setup, I added the description in the plist and still got the app rejected. I ended up removing my parts of my own code that was mentioning UIImagePicker and now the app has been approved. My app will not get public for a while now but still it would be great to test the camera functionality. I guess this is an iTunes bug or something, we should file a bug to Apple.

nickoneill commented 7 years ago

Yes @ValiDraganescu, you should file a radar. It appears your issue is not related to PermissionScope.

Luccifer commented 7 years ago

Hope I will not be rejected with so many permission descriptions in Info.plist. Anybody success AppStore release with bag of premission descriptions?

emobtech commented 7 years ago

@Luccifer Just got an update approved with all those permission keys with generic placeholders. No comments from review team. Hopefully they fix it in future Xcode's versions.

nickoneill commented 7 years ago

Thanks for the followup @emobtech, we're working on a better solution for the future.

Luccifer commented 7 years ago

5 minutes ago they take my build for review as well, waiting for results, will update this comment later after their response/approve

TomMajor commented 7 years ago

Hello nickoneill and bre7,

I see my issue from last year made it to your readme :) https://github.com/nickoneill/PermissionScope/issues/61

I am in need of a short term solution for submitting an app, I don't want to fill out all privacy entries as I feel this is the wrong way and might be suspicious. I just need camera and photo library.

What would be a recommend way to remove all unused permissions from your point of view? Delete the imports statements in the two files Permissions.swift PermissionScope.swift and then go thru each // MARK: section and remove it if not needed? Will this work or is there more to keep an eye on?

Thanks,

basememara commented 7 years ago

If I may chime in, yes this is what I did. I deleted all the import statements and removed what couldn't compile anymore.

I only needed notifications so I adjusted the code accordingly and submitted it fine without all the privacy entries: https://github.com/basememara/PermissionScope/tree/feature/notifications_only

nickoneill commented 7 years ago

Thanks @basememara, that seems like the thing to do right now if you don't want to fill in the plist items for permissions you don't use. Working on an alternate method for import right now.

TomMajor commented 7 years ago

Thanks @basememara for the hints. Once I commented out the imports and the //MARK sections I didn't need, it's pretty straightforward. This way, only a few places are left where xcode complains.

Only two of them are really notable, one being the line

while self.waitingForBluetooth || self.waitingForMotion { }

where I was not really sure if just commenting it out would be ok, but I think so, and the other in

func statusForPermission(_ type: PermissionType, completion: statusRequestClosure) {

where I needed to add a default case

default:
  permissionStatus = .unknown
bre7 commented 7 years ago

Flags+Subspecs solution used by ISHPermissionKit: https://github.com/iosphere/ISHPermissionKit/pull/80/files

alanscarpa commented 7 years ago

@bre7 This seems like a solid solution to the issue. I'd like to implement the same solution for this project and make a PR, unless @nickoneill you have an alternate solution in the works?

h3smith commented 7 years ago

We sort of "butchered" @nickoneill code to get the permission prompts working as we wanted. It was basically breaking each "permission set" into its own Framework. if you want to use "Camera" you had XYZCamera.framework and viola done.

nickoneill commented 7 years ago

That's the solution I think is best @h3smith, flags and subspecs seems a bit more complicated to use. @alanscarpa if you want to work on an approach, this is the right way to go.