You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2022/08/26 13:32:27 UTC
[GitHub] [cordova-ios] gazben opened a new pull request, #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
gazben opened a new pull request, #1249:
URL: https://github.com/apache/cordova-ios/pull/1249
### 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](https://help.github.com/articles/closing-issues-using-keywords/))
- [ ] I've updated the documentation if necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] bryan-hunter commented on pull request #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
Posted by GitBox <gi...@apache.org>.
bryan-hunter commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1251483839
> ### 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
> * [x] I've run the tests to see all new and existing tests pass
> * [ ] I added automated test coverage as appropriate for this change
> * [x] 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](https://help.github.com/articles/closing-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 :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] gazben commented on pull request #1249: feat: Add LimitsNavigationsToAppBoundDomains configuration key
Posted by "gazben (via GitHub)" <gi...@apache.org>.
gazben commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1583148060
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] bryan-hunter commented on pull request #1249: feat: Add LimitsNavigationsToAppBoundDomains configuration key
Posted by "bryan-hunter (via GitHub)" <gi...@apache.org>.
bryan-hunter commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1583155240
> 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 ๐
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] dpogue commented on pull request #1249: feat: Add LimitsNavigationsToAppBoundDomains configuration key
Posted by "dpogue (via GitHub)" <gi...@apache.org>.
dpogue commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1583175438
Thanks @gazben for the contribution! ๐
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] erisu commented on a diff in pull request #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
Posted by "erisu (via GitHub)" <gi...@apache.org>.
erisu commented on code in PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#discussion_r1219839627
##########
CordovaLib/cordova.js:
##########
@@ -1,5 +1,5 @@
// Platform: ios
-// 538a985db128858c0a0eb4dd40fb9c8e5433fc94
+// cordova-js rel/6.0.0-10-g07379820
Review Comment:
This should be reverted.
```suggestion
// 538a985db128858c0a0eb4dd40fb9c8e5433fc94
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] gazben commented on pull request #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
Posted by "gazben (via GitHub)" <gi...@apache.org>.
gazben commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1514382181
@ochakov Can you merge this, or who should I contact?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] gazben commented on pull request #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
Posted by GitBox <gi...@apache.org>.
gazben commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1228503578
I don't know how can I write a test for this honestly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] ochakov commented on pull request #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
Posted by "ochakov (via GitHub)" <gi...@apache.org>.
ochakov commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1514387420
> @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!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] codecov-commenter commented on pull request #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1228649407
# [Codecov](https://codecov.io/gh/apache/cordova-ios/pull/1249?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1249](https://codecov.io/gh/apache/cordova-ios/pull/1249?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d26a465) into [6.2.x](https://codecov.io/gh/apache/cordova-ios/commit/87c6d53a390d6157bca78a8a0d4ea8d6dd5e6fcb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (87c6d53) will **not change** coverage.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## 6.2.x #1249 +/- ##
=======================================
Coverage 74.88% 74.88%
=======================================
Files 13 13
Lines 1724 1724
=======================================
Hits 1291 1291
Misses 433 433
```
:mega: Weโre building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] dpogue merged pull request #1249: feat: Add LimitsNavigationsToAppBoundDomains configuration key
Posted by "dpogue (via GitHub)" <gi...@apache.org>.
dpogue merged PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] gazben commented on pull request #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
Posted by GitBox <gi...@apache.org>.
gazben commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1229969753
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 pathing or forking the project. This flag is an optional thing even for native projects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] dpogue commented on pull request #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
Posted by GitBox <gi...@apache.org>.
dpogue commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1228640719
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-ios] gazben commented on pull request #1249: (ios) Add LimitsNavigationsToAppBoundDomains configuration key
Posted by "gazben (via GitHub)" <gi...@apache.org>.
gazben commented on PR #1249:
URL: https://github.com/apache/cordova-ios/pull/1249#issuecomment-1583120947
Sorry, reverted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org