extractCredentials: do not read json from the request.#1727
extractCredentials: do not read json from the request.#1727mauritsvanrees wants to merge 1 commit intomainfrom
Conversation
The result was never used, and it may fail when the request is too large to read. This is a problem since at least Zope 5.8.4, introduced in Plone 6.0.7. See plone/Products.CMFPlone#3848 and zopefoundation/Zope#1180. This PR is an alternative to #1726. See discussion there.
✅ Deploy Preview for plone-restapi canceled.
|
|
@mauritsvanrees thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
|
This fails because in the What this test tries to do, and what no longer works with this PR:
When I instead request It has to do with The So I guess this approach is not good enough. |
|
@mauritsvanrees Okay, makes sense: we need to extract the credentials in a PAS extract credentials plugin so that they are available for use by other authenticateCredentials plugins. I still kind of feel like this would belong more cleanly in a separate plugin, since it is not really directly related to authenticating a JWT token. But let's leave it for now. |
The result was never used, and it may fail when the request is too large to read. This is a problem since at least Zope 5.8.4, introduced in Plone 6.0.7. See plone/Products.CMFPlone#3848 and zopefoundation/Zope#1180.
This PR is an alternative to #1726. See discussion there.