parse-server
parse-server copied to clipboard
Use newer jwks-rsa library and its async/await functions
New Pull Request Checklist
- [x] I am not disclosing a vulnerability.
- [x] I am creating this PR in reference to an issue. See #7304
Issue Description
This adds the requirement "parse-server works with Node 10 and newer" a real thing in package.json.
This PR intends to upgrade a dependency and switch from promisify to async/await.
Bumping the min Node.js requirement is merely a side-effect, but it makes also sense for another reason because Parse Server is officially only compatible (and tested) with Node >= 10.
Related issue: #7304
Approach
- Set engines to include Node 10+, since the library requires that. This makes sense, also for parse-server, since it is tested with Node 10+, already.
- Change the apple & facebook auth files to use jwks client with its new async/await-ready functions as seen in the upgrade guide for 2.x: https://github.com/auth0/node-jwks-rsa/blob/master/CHANGELOG.md#migrated-callbacks-to-asyncawait
(I attempted to run npm install, but that happened to have a newer npm (7) which bumped the lockfileVersion. Hm. Not what I wanted to do.)
TODOs before merging
Current trouble w/ my bad use of the tests:
1) facebook limited auth adapter (using client id as string) should verify id_token
- Error: <spyOn> : getSigningKey() method does not exist
Usage: spyOn(<object>, <methodName>)
- [ ] use spyOn correctly
- [x] Use the 2.x version of the library
- [x] Use the new functions in facebook auth file
- [x] Use the new functions in apple auth file
- [x] Add entry to changelog?
Codecov Report
Merging #7305 (82d1018) into master (7042552) will decrease coverage by
0.01%. The diff coverage isn/a.
:exclamation: Current head 82d1018 differs from pull request most recent head 2757960. Consider uploading reports for the commit 2757960 to get more accurate results
@@ Coverage Diff @@
## master #7305 +/- ##
==========================================
- Coverage 93.93% 93.91% -0.02%
==========================================
Files 181 179 -2
Lines 13194 13168 -26
==========================================
- Hits 12394 12367 -27
- Misses 800 801 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/batch.js | 91.37% <0.00%> (-1.73%) |
:arrow_down: |
| src/Adapters/Files/GridFSBucketAdapter.js | 79.50% <0.00%> (-0.82%) |
:arrow_down: |
| src/Routers/FilesRouter.js | 88.28% <0.00%> (-0.27%) |
:arrow_down: |
| ...dapters/Storage/Postgres/PostgresStorageAdapter.js | 95.36% <0.00%> (-0.16%) |
:arrow_down: |
| src/ParseServer.js | 89.77% <0.00%> (-0.12%) |
:arrow_down: |
| src/cli/utils/commander.js | 100.00% <0.00%> (ø) |
|
| src/ParseServerRESTController.js | 98.50% <0.00%> (ø) |
|
| src/Deprecator/Deprecations.js | ||
| src/Deprecator/Deprecator.js | ||
| src/Adapters/Storage/Mongo/MongoStorageAdapter.js | 93.24% <0.00%> (+0.65%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 7042552...2757960. Read the comment docs.
To clarify for reviewers, this PR intents to upgrade a dependency and switch from promisify to async/await.
Bumping the min Node.js requirement is merely a side effect, but it makes also sense for another reason because Parse Server is officially only compatible (and tested) with Node >= 10.
@olleolleolle Could you please change the PR issue and approach to be specific to the referring issue to prevent any confusion?
Ah, perfect, the right tests are failing, I'll get to that.
(I can see how I missed them locally: they are xited there. They do not run, by default. Will investigate.)
@mtrezza 👋 Hi, I know that I won't have time to get the tests in shape in the forseeable future, so please do investigate.
Acceptable alternative: close this PR.
Thanks for the update, let's leave this open for now, if someone wants to pick this up.
⚠️ Important change for merging PRs from Parse Server 5.0 onwards!
We are planning to release the first beta version of Parse Server 5.0 in October 2021.
If a PR contains a breaking change and is not merged before the beta release of Parse Server 5.0, it cannot be merged until the end of 2022. Instead it has to follow the Deprecation Policy and phase-in breaking changes to be merged during the course of 2022.
One of the most voiced community feedbacks was the demand for predictability in breaking changes to make it easy to upgrade Parse Server. We have made a first step towards this by introducing the Deprecation Policy in February 2021 that assists to phase-in breaking changes, giving developers time to adapt. We will follow-up with the introduction of Release Automation and a branch model that will allow breaking changes only with a new major release, scheduled for the beginning of each calendar year.
We understand that some PRs are a long time in the making and we very much appreciate your contribution. We want to make it easy for PRs that contain a breaking change and were created before the introduction of the Deprecation Policy. These PRs can be merged with a breaking change without being phased-in before the beta release of Parse Server 5.0. We are making this exception because we appreciate that this is a time of transition that requires additional effort from contributors to adapt. We encourage everyone to prepare their PRs until the end of September and account for review time and possible adaptions.
If a PR contains a breaking change and should be merged before the beta release, please mention @parse-community/server-maintenance and we will coordinate with you to merge the PR.
Thanks for your contribution and support during this transition to Parse Server release automation!