Skip to content

fix: update origin handling to support wildcard origins #472

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

fix: update origin handling to support wildcard origins #472

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
030c70e
fix: update origin handling to support wildcard origins
randikabanura Aug 11, 2025
cff4edf
fix: update origin handling to support wildcard origins
randikabanura Aug 11, 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
6 changes: 4 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 @@ -91,7 +91,7 @@
def valid_origin?(expected_origin)
return false unless expected_origin

expected_origin.include?(client_data.origin)
Array(expected_origin).any? { |allowed_origin| allowed_origin === client_data.origin }

Check failure on line 94 in lib/webauthn/authenticator_response.rb

View workflow job for this annotation

GitHub Actions / lint 

Style/CaseEquality: Avoid the use of the case equality operator `===`.
Copy link
Author

Choose a reason for hiding this comment

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

This follows how the wildcard origin matching handled in ActionCable: https://github.com/rails/rails/blob/cfc9b90f70d64a83b0aadec72bd858b197e2d733/actioncable/lib/action_cable/connection/base.rb#L234

Copy link
Contributor

Choose a reason for hiding this comment

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

chore: although it's not implied by the name of the variable, expected_origin is, in fact, already an array so we can avoid the conversion:

Suggested change
Array(expected_origin).any? { |allowed_origin| allowed_origin === client_data.origin }
expected_origin.any? { |allowed_origin| allowed_origin === client_data.origin }

end

def valid_rp_id?(rp_id)
Expand All @@ -115,7 +115,9 @@
end

def rp_id_from_origin(expected_origin)
URI.parse(expected_origin.first).host if expected_origin.size == 1
return unless valid_origin?(expected_origin)

URI.parse(client_data.origin).host if expected_origin.size == 1
Comment on lines +118 to +120
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: I see why we have to do this, but I don't think we should be changing the rpId depending on the origin of the request. After all, the credentials are scoped to the rpId so by doing this we will be generating credentials scoped to the different origins.

If that's desired, I think users of this gem should instead use our instance based configuration for handling this.

However, I think we have other options similar to what we did when we added the support for declaring multiple allowed origins: we could assume we won't be able to infer the rpId correctly if a wildcard origin is used so we just simply don't do it – which will force the users of the gem to explicitly set it in their configuration.

What do you think?

end

def type
Expand Down
4 changes: 4 additions & 0 deletions lib/webauthn/fake_client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ def fake_origin
"http://localhost#{rand(1000)}.test"
end

def fake_wildcard_origin
/http:\/\/localhost.*/
end
Comment on lines +163 to +165
Copy link
Contributor

Choose a reason for hiding this comment

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

chore: this is not used, right? Should we remove it or was it added for a particular reason?


def type_for(method)
TYPES[method]
end
Expand Down
4 changes: 4 additions & 0 deletions spec/spec_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ def fake_origin
"http://localhost"
end

def fake_wildcard_origin
/http:\/\/localhost.*/
Copy link
Contributor

@santiagorodriguez96 santiagorodriguez96 Aug 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

thought: perhaps we should document in some way that, in order for wildcard domains to work, a regexp should be provided.

From the Rails' guides I found that they do it for ActionCable's allowed_request_origins:

Action Cable will only accept requests from specified origins, which are
passed to the server config as an array. The origins can be instances of
strings or regular expressions, against which a check for the match will be performed.

config.action_cable.allowed_request_origins = ["https://rubyonrails.com", %r{http://ruby.*}]

What do you think?

end

def fake_challenge
SecureRandom.random_bytes(32)
end
Expand Down
8 changes: 4 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 @@ -12,8 +12,8 @@
let!(:credential) { create_credential(client: client) }
let(:credential_public_key) { credential[1] }

let(:origin) { fake_origin }
let(:actual_origin) { origin }
let(:origin) { fake_wildcard_origin }
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: I would keep the "normal" origin declaration here and just add an additional context under the "origin validation" section. What do you think?

let(:actual_origin) { fake_origin }
let(:original_challenge) { fake_challenge }
let(:assertion) { client.get(challenge: original_challenge) }
let(:authenticator_data) { assertion["response"]["authenticatorData"] }
Expand Down Expand Up @@ -429,7 +429,7 @@
original_challenge,
public_key: credential_public_key,
sign_count: 0,
rp_id: URI.parse(origin).host
rp_id: URI.parse(actual_origin).host
)
).to be_truthy
end
Expand All @@ -440,7 +440,7 @@
original_challenge,
public_key: credential_public_key,
sign_count: 0,
rp_id: URI.parse(origin).host
rp_id: URI.parse(actual_origin).host
)
).to be_truthy
end
Expand Down
Loading