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: Accept rp_id explicitly on *Response#valid? #73

Merged
merged 1 commit into from
Oct 4, 2018

Conversation

sorah
Copy link
Contributor

@sorah sorah commented Oct 2, 2018

Closes #72

RP ID may be different from request origin; i.e. client may override, or the appid extension may give legacy app id as a RP ID.

In most cases, rp_id can be expected to be a host part of request origin URI. So this patch allows valid? methods to take optional rp_id, then the methods handle the explicitly given one as a RP ID, instead of a host of origin URI, when it's given.

attestation.valid?(challenge, "https://a.example.com", rp_id: "example.com")
assertion.valid?(challenge, "https://a.example.com", rp_id: "https://a.example.com") # in case of +appid+ extension

Closes cedarcode#72

RP ID may be different from request origin; i.e. client may override,
or the appid extension may give legacy app id as a RP ID.

In most cases, rp_id can be expected to be a host part of request origin
URI. So this patch allows `valid?` methods to take optional `rp_id`, then the
methods handle the explicitly given one as a RP ID, instead of a host of
origin URI, when it's given.
@lgarron
Copy link
Contributor

lgarron commented Oct 4, 2018

Would it be possible to review this for inclusion in a release soon? This is the main thing that is blocking us from this library, and would solve #78 for us.

@brauliomartinezlm brauliomartinezlm merged commit b1e952f into cedarcode:master Oct 4, 2018
@brauliomartinezlm
Copy link
Member

Thank you for your work on this! 👍

@sorah sorah deleted the rp_id branch October 4, 2018 23:42
@brauliomartinezlm
Copy link
Member

@lgarron @sorah just released v1.1.0. Thank you for all your contributions!

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

Successfully merging this pull request may close these issues.

None yet

3 participants