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.

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

Jump to bottom

Minor fix and increase test coverage. #165

Merged
merged 11 commits into from
Oct 14, 2024
Merged

Minor fix and increase test coverage. #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7110245
Do Not Create User If Registration Fails
joel Sep 28, 2024
15c660b
Check the Creation of User & Credential When Registered Successfully
joel Sep 28, 2024
1cfe78e
Add RR Gem
joel Sep 28, 2024
c50438f
Revert "Add RR Gem"
joel Sep 28, 2024
1afd495
Use minitest-stub_any_instance
joel Sep 28, 2024
e89e6d6
Merge branch 'master' into improve-tests
joel Oct 11, 2024
e42c372
Do No Use Mix Hash Access
joel Oct 11, 2024
3f0f587
Fix Rubocop
joel Oct 11, 2024
e7dea69
Make Test More Consistent
joel Oct 11, 2024
726e3ac
Revert "Do No Use Mix Hash Access"
joel Oct 14, 2024
321a43d
Merge master
joel Oct 14, 2024
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: 3 additions & 3 deletions app/controllers/registrations_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,19 @@ def create
def callback
webauthn_credential = WebAuthn::Credential.from_create(params)

user = User.create!(session[:current_registration]["user_attributes"])
user = User.new(session[:current_registration]["user_attributes"])

begin
webauthn_credential.verify(session[:current_registration]["challenge"], user_verification: true)

credential = user.credentials.build(
user.credentials.build(
external_id: Base64.strict_encode64(webauthn_credential.raw_id),
nickname: params[:credential_nickname],
public_key: webauthn_credential.public_key,
sign_count: webauthn_credential.sign_count
)

if credential.save
if user.save
sign_in(user)

render json: { status: "ok" }, status: :ok
Expand Down
37 changes: 33 additions & 4 deletions test/controllers/registrations_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,41 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest
]
)

post(
callback_registration_url,
params: { credential_nickname: "USB Key" }.merge(public_key_credential)
)
assert_no_difference -> { User.count } do
post(
callback_registration_url,
params: { credential_nickname: "USB Key" }.merge(public_key_credential)
Comment on lines +58 to +61
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about also adding a test where the crendential.verify method fails and ensure that the user is not created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've covered it in my test, I just wasn't at ease doing it in Minitest.

describe "POST #callback" do
  context "with invalid params" do
    let(:fake_rp) do
      instance_double(WebAuthn::FakeRelyingParty)
    end

    it "renders a response with 422 status" do
      allow(fake_rp).to receive(:verify_registration).and_raise(WebAuthn::Error, "WebAuthn::UserVerifiedVerificationError")
      allow(controller).to receive(:relying_party).and_return(fake_rp)

      session = { current_registration: { challenge: "Challenge" } }

      expect do
        expect do
          post :callback, params: { registration: invalid_attributes }, session:
        end.not_to change(User, :count)
      end.not_to change(Credential, :count)

      expect(response).to have_http_status(:unprocessable_entity)

      expect(response.parsed_body).to eq({ "error" => "Verification failed: WebAuthn::UserVerifiedVerificationError" })
    end
  end
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, then don't worry about it. I'll see what I can do 🙂

)
end

assert_response :unprocessable_entity
assert_equal "Couldn't register your Security Key", response.body
end

test "should register successfully" do
raw_challenge = SecureRandom.random_bytes(32)
challenge = WebAuthn.configuration.encoder.encode(raw_challenge)

WebAuthn::PublicKeyCredential::CreationOptions.stub_any_instance(:raw_challenge, raw_challenge) do
post registration_url, params: { registration: { username: "alice" }, format: :json }

assert_response :success
end

public_key_credential =
WebAuthn::FakeClient
.new(Rails.configuration.webauthn_origin)
.create(challenge:, user_verified: true)

assert_difference 'User.count', +1 do
assert_difference 'Credential.count', +1 do
post(
callback_registration_url,
params: { credential_nickname: "USB Key" }.merge(public_key_credential)
)
end
end

assert_response :success
end
end