feat: Add LimitsNavigationsToAppBoundDomains configuration key

[gazben]

Platforms affected

IOS

Motivation and Context

Currently there is no option to set the limitsNavigationsToAppBoundDomains attribute from the config.xml. If you want to use cookie authentication or browser APIs you want to set this to YES.

Official documentation: https://webkit.org/blog/10882/app-bound-domains/

Description

This changeset is copied from this tutorial: https://www.ryseapp.io/blog/engineering/step-by-step-guide-to-setup-and-configure-cordova-for-ios-and-android-to-load-a-hosted-react-app

Testing

Tested on multiple simulator instances and and physical hardware. Without it the cookie authentication for a single domain will not work. After I add this change the authentication works.

Checklist

• I've run the tests to see all new and existing tests pass
• I added automated test coverage as appropriate for this change
• Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
• I've updated the documentation if necessary

[gazben]

I don't know how can I write a test for this honestly.

[dpogue]

Thanks for the pull request! You probably want to target this to the master branch though.

I think app-bound domains are definitely something we want to support in cordova-ios, particularly because it will enable easier access to a bunch of APIs, as you mentioned.

However, I think for this to work, it will also need to populate the WKAppBoundDomains array in the application's Info.plist file with a list of domains. The blog post you linked is using config-file directives in config.xml to inject the domains they want into the WKAppBoundDomains list, but it probably makes sense to try to automatically populate that from a combination of the hostname value, access tags, and allow-navigation tags. I believe any communication with domains not listed will be blocked.

[codecov-commenter]

Codecov Report

Merging #1249 (aac88bc) into master (4efd400) will increase coverage by 3.73%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1249      +/-   ##
==========================================
+ Coverage   74.88%   78.62%   +3.73%     
==========================================
  Files          13       15       +2     
  Lines        1724     1773      +49     
==========================================
+ Hits         1291     1394     +103     
+ Misses        433      379      -54     

see 28 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[gazben]

You can only list 10 WKAppBoundDomains in your config file. I think if we automatically merge the hostname, allow-navigation etc... We can hit this limit pretty quickly, and I don't know what should we do if we hot this limit.

You are right navigating to a not set domain will trigger an error:

Setting this flag indicates to WebKit that a WKWebView will only navigate to app-bound domains. Once set, any attempt to navigate away from an app-bound domain will fail with the error: “App-bound domain failure.”

I can create a separete merge request for the master but I think it would be better to just provide this option to users so they don't have to resort to manual patching or forking the project. This flag is an optional thing even for native projects.

[bryan-hunter]

Platforms affected

IOS

Motivation and Context

Currently there is no option to set the limitsNavigationsToAppBoundDomains attribute from the config.xml. If you want to use cookie authentication or browser APIs you want to set this to YES.

Official documentation: https://webkit.org/blog/10882/app-bound-domains/

Description

This changeset is copied from this tutorial: https://www.ryseapp.io/blog/engineering/step-by-step-guide-to-setup-and-configure-cordova-for-ios-and-android-to-load-a-hosted-react-app

Testing

Tested on multiple simulator instances and and physical hardware. Without it the cookie authentication for a single domain will not work. After I add this change the authentication works.

Checklist

• I've run the tests to see all new and existing tests pass
• I added automated test coverage as appropriate for this change
• Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
• I've updated the documentation if necessary

Thanks for adding this @gazben ! I'm not sure why I didn't contribute this back here and just put it in the blog post, but you're being a better open source contributor than me :)

[gazben]

@ochakov Can you merge this, or who should I contact?

[ochakov]

@ochakov Can you merge this, or who should I contact?

I don't have merge permissions, I know @breautek can merge and some others. Would be really helpful if they do!

[gazben]

Sorry, reverted.

[dpogue]

This looks good to me as a stepping stone to exploring the full potential app-bound domains.

For now, this will allow people to turn on app-bound domains, and they can use edit-config in config.xml to add domains to their Info.plist file. In the future we can explore whether it makes sense to automatically populate app-bound domains from access/allow-navigation/allow-intent tags, but I don't think we have enough information about the exact behaviour of app-bound domains to make that call yet.

[gazben]

As an FYI I've been using this change in production for half a year now (~2k users) no problems reported yet. My comment still stands from earlier, that I don't think that the automatic population is feasible because of the heavy size limitation.

[bryan-hunter]

As an FYI I've been using this change in production for half a year now (~2k users) no problems reported yet. My comment still stands from earlier, that I don't think that the automatic population is feasible because of the heavy size limitation.

Same here 👍

[dpogue]

Thanks @gazben for the contribution! 🎉