Skip to content

Allow accepting any topOrigin value #493

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow accepting any topOrigin value #493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00763f3
feat: allow any top origin via :allow_all
nicolastemciuc Oct 30, 2025
ce56271
chore: use `:all` instead of `:allow_all`
nicolastemciuc Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ WebAuthn.configure do |config|
#
# config.allowed_top_origins = []
#
# (C) Allow all top-level origins:
#
# config.allowed_top_origins = :all
#
# Note: if `verify_cross_origin` is not enabled, any values set in `allowed_top_origins`
# will be ignored.

Expand Down
5 changes: 3 additions & 2 deletions lib/webauthn/authenticator_response.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,10 @@ def valid_token_binding?
end

def valid_top_origin?
return false unless client_data.cross_origin
return false unless client_data.cross_origin && client_data.top_origin

relying_party.allowed_top_origins&.include?(client_data.top_origin)
relying_party.allowed_top_origins == :all ||
relying_party.allowed_top_origins&.include?(client_data.top_origin)
end

def valid_challenge?(expected_challenge)
Expand Down
112 changes: 108 additions & 4 deletions spec/webauthn/authenticator_assertion_response_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,8 @@
end
end

context "when allowed_top_origins is set" do
let(:allowed_top_origins) { [top_origin] }
context "when allowed_top_origins is a collection of origins" do
let(:allowed_top_origins) { [top_origin, "http://another.example.com"] }

context "when cross_origin is true" do
let(:cross_origin) { true }
Expand Down Expand Up @@ -448,6 +448,58 @@
end
end
end

context "when allowed_top_origins is :all" do
let(:allowed_top_origins) { :all }

context "when cross_origin is true" do
let(:cross_origin) { true }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "a valid assertion response"
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid assertion response"
end
end

context "when cross_origin is false" do
let(:cross_origin) { false }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "a valid assertion response"
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid assertion response"
end
end

context "when cross_origin is not set" do
let(:cross_origin) { nil }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "a valid assertion response"
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid assertion response"
end
end
end
end

context "when verify_cross_origin is true" do
Expand Down Expand Up @@ -505,8 +557,8 @@
end
end

context "when allowed_top_origins is set" do
let(:allowed_top_origins) { [top_origin] }
context "when allowed_top_origins is a collection of origins" do
let(:allowed_top_origins) { [top_origin, "http://another.example.com"] }

context "when cross_origin is true" do
let(:cross_origin) { true }
Expand Down Expand Up @@ -580,6 +632,58 @@
end
end
end

context "when allowed_top_origins is :all" do
let(:allowed_top_origins) { :all }

context "when cross_origin is true" do
let(:cross_origin) { true }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "a valid assertion response"
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "an invalid assertion response that raises", WebAuthn::TopOriginVerificationError
end
end

context "when cross_origin is false" do
let(:cross_origin) { false }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "an invalid assertion response that raises", WebAuthn::TopOriginVerificationError
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid assertion response"
end
end

context "when cross_origin is not set" do
let(:cross_origin) { nil }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "an invalid assertion response that raises", WebAuthn::TopOriginVerificationError
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid assertion response"
end
end
end
end
end

Expand Down
112 changes: 108 additions & 4 deletions spec/webauthn/authenticator_attestation_response_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,8 +702,8 @@
end
end

context "when allowed_top_origins is set" do
let(:allowed_top_origins) { [top_origin] }
context "when allowed_top_origins is a collection of origins" do
let(:allowed_top_origins) { [top_origin, "https://another.example.com"] }

context "when cross_origin is true" do
let(:cross_origin) { true }
Expand Down Expand Up @@ -777,6 +777,58 @@
end
end
end

context "when allowed_top_origins is :all" do
let(:allowed_top_origins) { :all }

context "when cross_origin is true" do
let(:cross_origin) { true }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "a valid attestation response"
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid attestation response"
end
end

context "when cross_origin is false" do
let(:cross_origin) { false }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "a valid attestation response"
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid attestation response"
end
end

context "when cross_origin is not set" do
let(:cross_origin) { nil }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "a valid attestation response"
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid attestation response"
end
end
end
end

context "when verify_cross_origin is true" do
Expand Down Expand Up @@ -834,8 +886,8 @@
end
end

context "when allowed_top_origins is set" do
let(:allowed_top_origins) { [top_origin] }
context "when allowed_top_origins is a collection of origins" do
let(:allowed_top_origins) { [top_origin, "https://another.example.com"] }

context "when cross_origin is true" do
let(:cross_origin) { true }
Expand Down Expand Up @@ -909,6 +961,58 @@
end
end
end

context "when allowed_top_origins is :all" do
let(:allowed_top_origins) { :all }

context "when cross_origin is true" do
let(:cross_origin) { true }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "a valid attestation response"
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "an invalid attestation response that raises", WebAuthn::TopOriginVerificationError
end
Comment on lines +977 to +981
Copy link
Contributor

@santiagorodriguez96 santiagorodriguez96 Oct 30, 2025

Choose a reason for hiding this comment

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

I'm not entirely sure about raising an error if allowed_top_origins is allow_all and topOrigin is not set. Why do you think we should do it that way?

Copy link
Member Author

@nicolastemciuc nicolastemciuc Oct 31, 2025

Choose a reason for hiding this comment

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

I understood that in L3 when crossOrigin is true there MUST be something set in topOrigin, but I'm not sure TBH

Copy link
Contributor

@santiagorodriguez96 santiagorodriguez96 Oct 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I was thinking about a relying party with verify_cross_origin enabled and allowed_top_origins as :all more as a L2 relying party – as mentioned on this comment:

It is probably correct that an "L2" RP, expecting crossOrigin: true, topOrigin: undefined, should accept an L3 client data with "crossOrigin":true,"topOrigin":"", since "topOrigin" is an unknown attribute to that RP and so it falls under the "allow any trailing attributes" rule in the final step.

However, by doing this then it would be basically the same as ignoring the cross origin verification, with the difference that it will fail is crossOrigin is false but topOrigin is present, which is not really the behavior of a L2 RP 🤷

Copy link
Contributor

@santiagorodriguez96 santiagorodriguez96 Oct 31, 2025

Choose a reason for hiding this comment

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

I think we should consider if it's worth adding this?

The value that it provides is that a RP that sets :all as the allowed origin is that responses with crossOrigin being false and topOrigin being nil and with crossOrigin being false and topOrigin being something will both be considered invalid – but I'm not sure if users of the gem that wish to allow any top origin will care for those scenarios...

We could leave it as it's currently is and make users of the gem that wish to allow any top origin to simply ignore the cross origin verification in the configuration.

What do you think? Am I'm missing something?

end

context "when cross_origin is false" do
let(:cross_origin) { false }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "an invalid attestation response that raises", WebAuthn::TopOriginVerificationError
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid attestation response"
end
end

context "when cross_origin is not set" do
let(:cross_origin) { nil }

context "when top_origin is set" do
let(:client_top_origin) { top_origin }

it_behaves_like "an invalid attestation response that raises", WebAuthn::TopOriginVerificationError
end

context "when top_origin is not set" do
let(:client_top_origin) { nil }

it_behaves_like "a valid attestation response"
end
end
end
end
end

Expand Down