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

Should the keys in the VerifyResult covered_components OrderedDict be double quoted? #4

Open
chrisinmtown opened this issue Jul 17, 2022 · 3 comments

Comments

@chrisinmtown
Copy link

I'm trying to check that the signature verify result includes certain fields, for example on a POST with a body, the content-digest should be covered. I discovered that the keys in the VerifyResult covered_components OrderedDict are quoted; i.e., start and end with double-quote characters. Is this essential? The values are not double-quoted so I am wondering if maybe this was an oversight. Please see example:

OrderedDict([('"@method"', 'POST'), ('"@authority"', 'localhost'), ('"@target-uri"', 'http://localhost/some/path'),
('"content-digest"', 'sha-256=:yUnXGcm2X2HRcRX87e2yhRNdlvZHIIggm6zgJgJiCYw=:'), ('"content-length"', '657'),
('"@signature-params"', '("@method" "@authority" "@target-uri" "content-digest" "content-length");created=1658067851;keyid="test-key";alg="rsa-pss-sha512"')])

Just one more thing, would you please use the verify result in the test.py tests? I'm sure I'm not the only person who wants to check the covered components, so this seems like it would be a very helpful demonstration.

@chrisinmtown
Copy link
Author

chrisinmtown commented Jul 17, 2022

Would you consider an enhancement, maybe even a PR, so the verify method accepts an optional list of required component IDs, and declares failure if one is missing? Then I can ignore the double-quoted keys and simplify my code :) So instead of:

verify_result = verifier.verify(request)
check_result(verify_result, ['content-digest'])

I would like to write just this:

verifier.verify(request, covered_components=['content-digest'])

@kislyuk
Copy link
Member

kislyuk commented Oct 18, 2023

Sorry I missed this, yes, would be happy to merge a PR for this. The other library I maintain that depends on this (https://github.com/pyauth/requests-http-signature) already has an API for this, so it will need to be updated to work with this new API.

@chrisinmtown
Copy link
Author

Thanks @kislyuk for getting back to me. I see I have combined many issues here :) The first is why the covered_components keys are double-double quoted-quoted, the second is asking for an example in the tests area, and the third is for a new behavior in the verify method. Please advise how to separate & handle.

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