OIDAuthorizationUICoordinatorIOS not compiling for App Extensions

[julienbodet]

I imported AppAuth via Cocoapods in my swift project and the library is not compiling because of the call to [UIApplication sharedApplication] in presentAuthorizeRequest:session: and leads to the following error:

It would be great to add a mechanism to take into account if we are building for an app extension or not.

EDIT: I see that the PR #130 propose a solution. When will this PR be merged?

[WilliamDenniss]

For now I would recommend forking AppAuth and merging that PR if you need this.

I think the correct way to support extensions would be to have a separate AppAuth framework target designed specifically for extensions.

[schroepf]

@WilliamDenniss I ran into the same issue and noticed that for my use case the extension didn't need any of the "platform" specific code in the "ios" and "macos" subdirectories, so I created a new target and built a framework (called AppAuthCore) from only the code in the "Source" directory and none of the subdirecotries (macOS or iOS). So that now I can use the AppAuth framework in the iOS app and the AppAuthCore framework for the extension... Now I wonder if this solution has any chance of making it into the official AppAuth-iOS code and if it is worth the effort to create a pull request?

[schroepf]

Oh... I just noticed that @julienbodet did basically the exact same thing in #218...

[julienbodet]

Hi @schroepf, yes I created a PR a while ago, but it has now a lot of conflicts. Maybe it's easier to create a new pull request. @WilliamDenniss It would be very nice to have that fixed in an upcoming release, is there any chance we could include that in the 1.0.0? For my use case, it is the only bug that prevents me to use the official AppAuth and force me to maintain a fork. Thanks!

[WilliamDenniss]

This is a very valid use-case. The reason I stalled on #218 is that it changed a lot, and impacted people who don't need the separation, which I'd like to avoid.

I did some research into subspecs, and added a review comment there in the PR. Perhaps we can address that concern fairly easily, and deliver this functionality.

[schroepf]

Oh, didn't know that something like "subspecs" exist in Cocoapods, that's nice! I created PR #310 which creates a "Core" and an "Auth" subspec (as @WilliamDenniss suggested in @julienbodet's pull request #218 ).

At first I had some issues with the umbrella header (as it was still importing the APIs which cannot be used by Extensions). But I noticed that I could completely delete the header as Cocoapods will generate it's own umbrella headers anyways. However I'm not sure if this will break anything else in the project setup which I'm not aware of...

So please take a look and let me know what you think.... :)

[WilliamDenniss]

Carthage definitely needs that header. I'm holding off on my review, until you sign the CLA, but I'd suggest making sure all the samples still build with your changes (ideally, they'll build without modification as that means our users won't have to make any changes either).

In your PR, can you add a deep-link to my proposed design which you implemented, and a reference to this bug?

[schroepf]

I added the link and the reference to the PR description.

I just tested the carthage examples and they are still building (i made sure Carthage was using my commit 985521223c4ef3ad0a22ec2a05772c0e54c4047a). As it appears carthage didn't use the header I deleted but the AppAuth.h header from the Frameworks subdirectory.

But it turned out the Header file I deleted was used by the macOS example. I'll have a look later, but probably I can fix that issue by "moving" the old header to the macOS subdirectory (and thus to the "Auth" subpod). But does this mean that we can also delete all tvOS, watchOS and iOS related imports from that header?

[WilliamDenniss]

Carthage itself may build, but I don't think people can use it without the header.

Is there another solution that doesn't involve deleting it? What if the "Auth" subspec has a #define like APPAUTH_USERAGENT, which adds the tvOS, watchOS & iOS related imports?

[schroepf]

ok, I think it's not Carthage but Cocoapods which needs this header (please refer to my comment in the PR)

I will consider your suggested solution and update my PR accordingly

[WilliamDenniss]

Resolved by #310. Thanks all!