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

htu url in DPoP payload contains query parameters and fragments #1

Open
Lithiel opened this issue Nov 26, 2024 · 3 comments
Open

htu url in DPoP payload contains query parameters and fragments #1

Lithiel opened this issue Nov 26, 2024 · 3 comments

Comments

@Lithiel
Copy link

Lithiel commented Nov 26, 2024

Hi,

When trying to send the authorization to an url that contains query params the DPoP verification fails with the following message:
[DPoPWebIdExtractor] {Primary} warn: Error verifying WebID via DPoP-bound access token: The DPoP proof htu parameter must be the HTTP request URI without query and fragment parts.
As far as I understand, the htu url is not allowed to contain any query or fragment parts as defined in RFC 9449 section 4.2 .

I noticed that the request url just gets handed through until create_dpop_header in dpop_utils.py. I think it would make sense to remove the query parts there.

Something like this:

def create_dpop_header(url: str, method: str, key: jwk.JWK) -> str:
    updated_url = urlsplit(url)._replace(query=None, fragment=None).geturl()
    payload = {
        "htu": updated_url,
    ...
@Otto-AA
Copy link
Owner

Otto-AA commented Nov 29, 2024

Hi, thank you for the detailed suggestion!

I used your code suggestion and applied it here (changing None to an empty string, because the type system required a str): e208e4e

The added test case succeeds when removing the query and fails when keeping the query, so it looks good. If you want, you can test it before I publish the package.

I would try to make a new version next week and publish it to pypi.

@Lithiel
Copy link
Author

Lithiel commented Dec 9, 2024

Hi,
thanks for the fast response and sorry that I took so long to answer.
It looks fine to me and I am looking forward to the next version.

@Otto-AA
Copy link
Owner

Otto-AA commented Dec 9, 2024

No worries :)

It should work now with v1.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants