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

Allow to pass expected origin to verify #365

Closed
wants to merge 2 commits into from

Conversation

manubo
Copy link

@manubo manubo commented Aug 4, 2022

Both

  • WebAuthn::AuthenticatorAttestationResponse#verify and
  • WebAuthn::AuthenticatorAssertionResponse#verify

accept the parameter expected_origin, but the calling methods

  • PublicKeyCredentialWithAttestation#verify and
  • PublicKeyCredentialWithAssertion#verify,

respectively, do not.

We have a multi-tenant system where each tenant has its own subdomain and need to be able to pass the origin when validating the data.

Comment on lines -177 to +209
expect(public_key_credential.client_extension_outputs)
.to eq({ "txAuthSimple" => "Could you please verify yourself?" })
expect(
public_key_credential.client_extension_outputs
).to eq({ "txAuthSimple" => "Could you please verify yourself?" })
Copy link
Author

Choose a reason for hiding this comment

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

Only code style fixes.

Comment on lines -231 to +264
expect(public_key_credential.authenticator_extension_outputs)
.to eq({ "txAuthSimple" => "Could you please verify yourself?" })
expect(
public_key_credential.authenticator_extension_outputs
).to eq({ "txAuthSimple" => "Could you please verify yourself?" })
Copy link
Author

Choose a reason for hiding this comment

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

Only code style fixes.

@brauliomartinezlm
Copy link
Member

Hi @manubo ! thank you for the agency on putting this PR up. For a multi-tenant usage of this gem, I'd recommend using our v3 which is in alpha right now but it's been highly tested at this point. We're about to release v3 alpha2 so please stayed tuned for #368

@manubo
Copy link
Author

manubo commented Sep 15, 2022

Hi @brauliomartinezlm, thanks for your feedback. I have seen that #368 was merged, which is great. There is no v3 alpha2 tag yet, but I assume it's the same as master at the moment, right? I will give it a try, thanks for your support 👍🏼

@manubo
Copy link
Author

manubo commented Sep 15, 2022

Closed in favour of using v3 of this gem.

@manubo manubo closed this Sep 15, 2022
@manubo manubo deleted the verify-with-expected-origin branch September 15, 2022 13:32
@brauliomartinezlm
Copy link
Member

@manubo I've just released 3.0.0.alpha2. Hope it worked as you needed.

@manubo
Copy link
Author

manubo commented Sep 15, 2022

Yes, works like a charm, thanks @brauliomartinezlm 👍🏼

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.

2 participants