Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Pushed Authorization Request (PAR) #313

Merged
merged 3 commits into from
Dec 18, 2023

Conversation

paulswartz
Copy link
Collaborator

@paulswartz paulswartz commented Dec 17, 2023

RFC: https://datatracker.ietf.org/doc/html/rfc9126

Open Questions

  • what to do if the PAR request fails, but PAR isn't required? Currently, we always use PAR if it's supported, and fail the URL generation if PAR fails.

Copy link
Member

@maennchen maennchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks quite good on a first glance. I’ll however read the specification properly before doing a full review.

what to do if the PAR request fails, but PAR isn't required

I think we should error and not try without. If the provider signals support, we should use the most safe option our client supports. I don’t want to introduce any possible downgrade attacks or unnoticed less safe behavior.

If PAR is supported by a provider but broken, the user if this lib should consciously opt out. (#307)

@maennchen maennchen self-assigned this Dec 17, 2023
@maennchen maennchen added this to the v3.2.0 milestone Dec 17, 2023
@maennchen
Copy link
Member

Preliminary review:

  • Add require_pushed_authorization_requests to client registration
  • Mention PAR in Supported Features of README

@paulswartz
Copy link
Collaborator Author

@maennchen let me know if the latest commit looks good and I'll rebase it into the previous commit.

@maennchen maennchen merged commit 9f5198b into erlef:main Dec 18, 2023
25 checks passed
@maennchen
Copy link
Member

@paulswartz Great job, thanks. A rebase is not necessary since the default is to squash PRs anyways at the moment.

@paulswartz paulswartz deleted the pushed-auth branch January 1, 2024 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants