diff --git a/.rubocop.yml b/.rubocop.yml index 9f0d8164..9d2b7e23 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -230,7 +230,7 @@ Rails/I18nLocaleAssignment: Enabled: true Rails/I18nLocaleTexts: - Enabled: true + Enabled: false Rails/IgnoredColumnsAssignment: Enabled: true diff --git a/Gemfile b/Gemfile index 68b786cb..726264c5 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,7 @@ gem 'bootsnap', '~> 1.18', require: false gem 'importmap-rails', '~> 2.2' gem 'propshaft', '~> 1.2' gem 'puma', '~> 7.0' +gem 'requestjs-rails' gem "rollbar", "~> 3.6" gem 'sqlite3', '>= 1.4' gem 'stimulus-rails', '~> 1.3' diff --git a/Gemfile.lock b/Gemfile.lock index d0b97fe9..873420ae 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -246,6 +246,8 @@ GEM regexp_parser (2.11.2) reline (0.6.2) io-console (~> 0.5) + requestjs-rails (0.0.13) + railties (>= 7.1.0) rexml (3.4.1) rollbar (3.6.2) rubocop (1.80.2) @@ -342,6 +344,7 @@ DEPENDENCIES puma (~> 7.0) rack-mini-profiler (~> 4.0) rails (~> 8.0.1) + requestjs-rails rollbar (~> 3.6) rubocop (~> 1.80) rubocop-rails (~> 2.33) diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css index 56a08c7b..6f9a9bd4 100644 --- a/app/assets/stylesheets/application.css +++ b/app/assets/stylesheets/application.css @@ -32,6 +32,7 @@ body { .center { display: flex; + justify-content: center; } .center input { diff --git a/app/controllers/credentials_controller.rb b/app/controllers/credentials_controller.rb index b86d607c..6d2ce38e 100644 --- a/app/controllers/credentials_controller.rb +++ b/app/controllers/credentials_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class CredentialsController < ApplicationController - def create + def options create_options = WebAuthn::Credential.options_for_create( user: { id: current_user.webauthn_id, @@ -18,8 +18,8 @@ def create end end - def callback - webauthn_credential = WebAuthn::Credential.from_create(params) + def create + webauthn_credential = WebAuthn::Credential.from_create(JSON.parse(credential_params[:public_key_credential])) begin webauthn_credential.verify(session[:current_registration]["challenge"], user_verification: true) @@ -29,16 +29,18 @@ def callback ) if credential.update( - nickname: params[:credential_nickname], + nickname: credential_params[:nickname], public_key: webauthn_credential.public_key, sign_count: webauthn_credential.sign_count ) - render json: { status: "ok" }, status: :ok + render json: { message: "Security Key registered successfully", redirect_to: root_path }, status: :ok else - render json: "Couldn't add your Security Key", status: :unprocessable_content + render json: { message: "Couldn't add your Security Key", redirect_to: credentials_path }, + status: :unprocessable_content end rescue WebAuthn::Error => e - render json: "Verification failed: #{e.message}", status: :unprocessable_content + render json: { message: "Verification failed: #{e.message}", redirect_to: credentials_path }, + status: :unprocessable_content ensure session.delete(:current_registration) end @@ -51,4 +53,8 @@ def destroy redirect_to root_path end + + def credential_params + params.expect(credential: [:public_key_credential, :nickname]) + end end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 9a73df62..9cd9d8c3 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -4,12 +4,12 @@ class RegistrationsController < ApplicationController def new end - def create - user = User.new(username: params[:registration][:username]) + def options + user = User.new(username: registration_params[:username]) create_options = WebAuthn::Credential.options_for_create( user: { - name: params[:registration][:username], + name: registration_params[:username], id: user.webauthn_id }, authenticator_selection: { user_verification: "required" } @@ -28,8 +28,8 @@ def create end end - def callback - webauthn_credential = WebAuthn::Credential.from_create(params) + def create + webauthn_credential = WebAuthn::Credential.from_create(JSON.parse(registration_params[:public_key_credential])) user = User.new(session[:current_registration]["user_attributes"]) @@ -38,7 +38,7 @@ def callback user.credentials.build( external_id: webauthn_credential.id, - nickname: params[:credential_nickname], + nickname: registration_params[:nickname], public_key: webauthn_credential.public_key, sign_count: webauthn_credential.sign_count ) @@ -46,14 +46,21 @@ def callback if user.save sign_in(user) - render json: { status: "ok" }, status: :ok + render json: { message: "Security Key registered successfully", redirect_to: root_path }, + status: :ok else - render json: "Couldn't register your Security Key", status: :unprocessable_content + render json: { message: "Couldn't register your Security Key", redirect_to: registration_path }, + status: :unprocessable_content end rescue WebAuthn::Error => e - render json: "Verification failed: #{e.message}", status: :unprocessable_content + render json: { message: "Verification failed: #{e.message}", redirect_to: registration_path }, + status: :unprocessable_content ensure session.delete(:current_registration) end end + + def registration_params + params.expect(registration: [:username, :nickname, :public_key_credential]) + end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 33f04050..b1aa9bfc 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -4,7 +4,7 @@ class SessionsController < ApplicationController def new end - def create + def options user = User.find_by(username: session_params[:username]) if user @@ -25,8 +25,8 @@ def create end end - def callback - webauthn_credential = WebAuthn::Credential.from_get(params) + def create + webauthn_credential = WebAuthn::Credential.from_get(JSON.parse(session_params[:public_key_credential])) user = User.find_by(username: session[:current_authentication]["username"]) raise "user #{session[:current_authentication]["username"]} never initiated sign up" unless user @@ -44,9 +44,10 @@ def callback credential.update!(sign_count: webauthn_credential.sign_count) sign_in(user) - render json: { status: "ok" }, status: :ok + render json: { message: "Security Key authenticated successfully", redirect_to: root_path }, status: :ok rescue WebAuthn::Error => e - render json: "Verification failed: #{e.message}", status: :unprocessable_content + render json: { message: "Verification failed: #{e.message}", redirect_to: session_path }, + status: :unprocessable_content ensure session.delete(:current_authentication) end @@ -61,6 +62,6 @@ def destroy private def session_params - params.require(:session).permit(:username) + params.expect(session: [:username, :public_key_credential]) end end diff --git a/app/javascript/application.js b/app/javascript/application.js index 81a77f89..34e73fc8 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -1,7 +1,7 @@ // Configure your import map in config/importmap.rb. Read more: https://github.com/rails/importmap-rails import "controllers" -import "credential" import "messenger" import Rails from "@rails/ujs"; +import "@rails/request.js" Rails.start(); diff --git a/app/javascript/controllers/add_credential_controller.js b/app/javascript/controllers/add_credential_controller.js deleted file mode 100644 index d6bdf334..00000000 --- a/app/javascript/controllers/add_credential_controller.js +++ /dev/null @@ -1,13 +0,0 @@ -import { Controller } from "@hotwired/stimulus" -import * as Credential from "credential"; - -export default class extends Controller { - create(event) { - var [data, status, xhr] = event.detail; - var credentialOptions = data; - var credential_nickname = event.target.querySelector("input[name='credential[nickname]']").value; - var callback_url = `/credentials/callback?credential_nickname=${credential_nickname}` - - Credential.create(encodeURI(callback_url), credentialOptions); - } -} diff --git a/app/javascript/controllers/new_registration_controller.js b/app/javascript/controllers/new_registration_controller.js deleted file mode 100644 index 95f186c1..00000000 --- a/app/javascript/controllers/new_registration_controller.js +++ /dev/null @@ -1,29 +0,0 @@ -import { Controller } from "@hotwired/stimulus" -import * as Credential from "credential"; - -import { MDCTextField } from '@material/textfield'; - -export default class extends Controller { - static targets = ["usernameField"] - - create(event) { - var [data, status, xhr] = event.detail; - console.log(data); - var credentialOptions = data; - - // Registration - if (credentialOptions["user"]) { - var credential_nickname = event.target.querySelector("input[name='registration[nickname]']").value; - var callback_url = `/registration/callback?credential_nickname=${credential_nickname}` - - Credential.create(encodeURI(callback_url), credentialOptions); - } - } - - error(event) { - let response = event.detail[0]; - let usernameField = new MDCTextField(this.usernameFieldTarget); - usernameField.valid = false; - usernameField.helperTextContent = response["errors"][0]; - } -} diff --git a/app/javascript/controllers/new_session_controller.js b/app/javascript/controllers/new_session_controller.js deleted file mode 100644 index ddaa9aac..00000000 --- a/app/javascript/controllers/new_session_controller.js +++ /dev/null @@ -1,22 +0,0 @@ -import { Controller } from "@hotwired/stimulus" -import * as Credential from "credential"; - -import { MDCTextField } from '@material/textfield'; - -export default class extends Controller { - static targets = ["usernameField"] - - create(event) { - var [data, status, xhr] = event.detail; - console.log(data); - var credentialOptions = data; - Credential.get(credentialOptions); - } - - error(event) { - let response = event.detail[0]; - let usernameField = new MDCTextField(this.usernameFieldTarget); - usernameField.valid = false; - usernameField.helperTextContent = response["errors"][0]; - } -} diff --git a/app/javascript/controllers/webauthn_credential_controller.js b/app/javascript/controllers/webauthn_credential_controller.js new file mode 100644 index 00000000..8981f990 --- /dev/null +++ b/app/javascript/controllers/webauthn_credential_controller.js @@ -0,0 +1,79 @@ +import { Controller } from "@hotwired/stimulus" +import { showMessage } from "messenger"; + +export default class extends Controller { + static targets = ["hiddenCredentialInput", "submitButton"] + static values = { optionsUrl: String, submitUrl: String } + + async create() { + try { + const response = await fetch(this.optionsUrlValue, { + method: "POST", + body: new FormData(this.element), + }); + + const credentialOptionsJson = await response.json(); + console.log(credentialOptionsJson); + + if (response.ok) { + console.log("Creating new public key credential..."); + + const credential = await navigator.credentials.create({ publicKey: PublicKeyCredential.parseCreationOptionsFromJSON(credentialOptionsJson) }); + this.hiddenCredentialInputTarget.value = JSON.stringify(credential); + + const submitResponse = await fetch(this.submitUrlValue, { + method: "POST", + body: new FormData(this.element), + }); + + const submitResponseJson = await submitResponse.json(); + + const { redirect_to } = submitResponseJson; + + window.location.replace(redirect_to || "/"); + } else { + showMessage(credentialOptionsJson.errors?.[0] || "Sorry, something wrong happened."); + this.submitButtonTarget.disabled = false; + } + } catch (error) { + showMessage(error.message || "Sorry, something wrong happened."); + this.submitButtonTarget.disabled = false; + } + } + + async get() { + try { + const response = await fetch(this.optionsUrlValue, { + method: "POST", + body: new FormData(this.element), + }); + + const credentialOptionsJson = await response.json(); + console.log(credentialOptionsJson); + + if (response.ok) { + console.log("Getting public key credential..."); + + const credential = await navigator.credentials.get({ publicKey: PublicKeyCredential.parseRequestOptionsFromJSON(credentialOptionsJson) }) + this.hiddenCredentialInputTarget.value = JSON.stringify(credential); + + const submitResponse = await fetch(this.submitUrlValue, { + method: "POST", + body: new FormData(this.element), + }); + + const submitResponseJson = await submitResponse.json(); + + const { redirect_to } = submitResponseJson; + + window.location.replace(redirect_to || "/"); + } else { + showMessage(credentialOptionsJson.errors?.[0] || "Sorry, something wrong happened."); + this.submitButtonTarget.disabled = false; + } + } catch (error) { + showMessage(error.message || "Sorry, something wrong happened."); + this.submitButtonTarget.disabled = false; + } + } +} diff --git a/app/javascript/credential.js b/app/javascript/credential.js deleted file mode 100644 index d2d63099..00000000 --- a/app/javascript/credential.js +++ /dev/null @@ -1,57 +0,0 @@ -import { showMessage } from "messenger"; - -function getCSRFToken() { - var CSRFSelector = document.querySelector('meta[name="csrf-token"]') - if (CSRFSelector) { - return CSRFSelector.getAttribute("content") - } else { - return null - } -} - -function callback(url, body) { - fetch(url, { - method: "POST", - body: JSON.stringify(body), - headers: { - "Content-Type": "application/json", - "Accept": "application/json", - "X-CSRF-Token": getCSRFToken() - }, - credentials: 'same-origin' - }).then(function(response) { - if (response.ok) { - window.location.replace("/") - } else if (response.status < 500) { - response.text().then(showMessage); - } else { - showMessage("Sorry, something wrong happened."); - } - }); -} - -function create(callbackUrl, data) { - const credentialOptions = PublicKeyCredential.parseCreationOptionsFromJSON(data); - - navigator.credentials.create({ "publicKey": credentialOptions }).then(function(credential) { - callback(callbackUrl, credential); - }).catch(function(error) { - showMessage(error); - }); - - console.log("Creating new public key credential..."); -} - -function get(data) { - const credentialOptions = PublicKeyCredential.parseRequestOptionsFromJSON(data); - - navigator.credentials.get({ "publicKey": credentialOptions }).then(function(credential) { - callback("/session/callback", credential); - }).catch(function(error) { - showMessage(error); - }); - - console.log("Getting public key credential..."); -} - -export { create, get } diff --git a/app/views/home/index.html.erb b/app/views/home/index.html.erb index 868a3f95..cd6a5f58 100644 --- a/app/views/home/index.html.erb +++ b/app/views/home/index.html.erb @@ -22,17 +22,26 @@ <% end %> -
- <%= form_with scope: :credential, url: credentials_path, local: false, data: { action: "ajax:success->add-credential#create" } do |form| %> +
+ <%= form_with( + scope: :credential, + url: credentials_path, + data: { + controller: "webauthn-credential", + action: "webauthn-credential#create:prevent", + "webauthn-credential-options-url-value": options_credentials_path, + "webauthn-credential-submit-url-value": credentials_path, + }) do |form| %>
<%= form.text_field :nickname, class: "mdc-text-field__input", placeholder: "New Security Key nickname", required: true %>
+ <%= form.hidden_field :public_key_credential, data: { "webauthn-credential-target": "hiddenCredentialInput" } %>
- <%= form.submit "Add Security Key", class: "mdc-button mdc-button--unelevated" %> + <%= form.submit "Add Security Key", class: "mdc-button mdc-button--unelevated", data: { "webauthn-credential-target": "submitButton" } %>
<% end %>
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index ac338995..85a3720d 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -44,7 +44,7 @@
-
+
@@ -80,7 +80,6 @@ <%= link_to "Cedarcode", "https://cedarcode.com", class: "mdc-theme--text-primary-on-light" %> -
diff --git a/app/views/registrations/new.html.erb b/app/views/registrations/new.html.erb index bcdeb074..7187820a 100644 --- a/app/views/registrations/new.html.erb +++ b/app/views/registrations/new.html.erb @@ -5,9 +5,17 @@

Register

- <%= form_with scope: :registration, url: registration_path, local: false, id: "new-registration", data: { controller: "new-registration", action: "ajax:success->new-registration#create ajax:error->new-registration#error" } do |form| %> + <%= form_with( + scope: :registration, + url: registration_path, + data: { + controller: "webauthn-credential", + action: "webauthn-credential#create:prevent", + "webauthn-credential-options-url-value": options_registration_path, + "webauthn-credential-submit-url-value": registration_path, + }) do |form| %>
-
+
<%= form.text_field :username, class: "mdc-text-field__input", placeholder: "Username", required: true, autocapitalize: "none", "aria-controls" => "username-helper-text" %>
@@ -23,10 +31,11 @@ <%= form.text_field :nickname, class: "mdc-text-field__input", placeholder: "Security Key nickname", required: true %>
+ <%= form.hidden_field :public_key_credential, data: { "webauthn-credential-target": "hiddenCredentialInput" } %>
- <%= form.submit "Register using WebAuthn", class: "mdc-button mdc-button--raised" %> + <%= form.submit "Register using WebAuthn", class: "mdc-button mdc-button--raised", data: { "webauthn-credential-target": "submitButton" } %>
<% end %>
diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 415e1d00..42e61cbb 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -5,12 +5,21 @@

Sign in

- <%= form_with scope: :session, url: session_path, local: false, id: "new-session", data: { controller: "new-session", action: "ajax:success->new-session#create ajax:error->new-session#error" } do |form| %> + <%= form_with( + scope: :session, + url: session_path, + data: { + controller: "webauthn-credential", + action: "webauthn-credential#get:prevent", + "webauthn-credential-options-url-value": options_session_path, + "webauthn-credential-submit-url-value": session_path, + }) do |form| %>
-
+
<%= form.text_field :username, class: "mdc-text-field__input", placeholder: "Username", required: true, autocapitalize: "none", "aria-controls" => "username-helper-text" %>
+ <%= form.hidden_field :public_key_credential, data: { "webauthn-credential-target": "hiddenCredentialInput" } %>
@@ -19,7 +28,7 @@
- <%= form.submit "Sign in using WebAuthn", class: "mdc-button mdc-button--raised" %> + <%= form.submit "Sign in using WebAuthn", class: "mdc-button mdc-button--raised", data: { "webauthn-credential-target": "submitButton" } %>
<% end %>
diff --git a/config/importmap.rb b/config/importmap.rb index c4352c7e..9983a3f1 100644 --- a/config/importmap.rb +++ b/config/importmap.rb @@ -5,7 +5,6 @@ pin "@hotwired/stimulus-loading", to: "stimulus-loading.js", preload: true pin_all_from "app/javascript/controllers", under: "controllers" pin "@rails/ujs", to: "https://ga.jspm.io/npm:@rails/ujs@7.1.2/app/assets/javascripts/rails-ujs.esm.js" -pin "credential" pin "messenger" pin "@material/list", to: "https://ga.jspm.io/npm:@material/list@4.0.0/dist/mdc.list.js" pin "@material/menu", to: "https://ga.jspm.io/npm:@material/menu@4.0.0/dist/mdc.menu.js" diff --git a/config/routes.rb b/config/routes.rb index 8a3d43e0..962ad847 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,18 +8,16 @@ get "up" => "rails/health#show", as: :rails_health_check resource :session, only: [:new, :create, :destroy] do - post :callback + post :options, on: :collection end resource :registration, only: [:new, :create] do - post :callback + post :options, on: :collection end resources :credentials, only: [:create, :destroy] do - post :callback, on: :collection + post :options, on: :collection end - # post "session_callback", to: "sessions#callback" - # post "registration_callback", to: "registrations#callback" root to: "home#index" end diff --git a/test/controllers/registrations_controller_test.rb b/test/controllers/registrations_controller_test.rb index 375ba275..a67c18a4 100644 --- a/test/controllers/registrations_controller_test.rb +++ b/test/controllers/registrations_controller_test.rb @@ -5,7 +5,7 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest test "should initiate registration successfully" do - post registration_url, params: { registration: { username: "alice" }, format: :json } + post options_registration_url, params: { registration: { username: "alice" } }, as: :json assert_response :success end @@ -13,14 +13,14 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest test "should return error if registrating taken username" do User.create!(username: "alice") - post registration_url, params: { registration: { username: "alice" }, format: :json } + post options_registration_url, params: { registration: { username: "alice" } }, as: :json assert_response :unprocessable_content assert_equal ["Username has already been taken"], response.parsed_body["errors"] end test "should return error if registrating blank username" do - post registration_url, params: { registration: { username: "" }, format: :json } + post options_registration_url, params: { registration: { username: "" } }, as: :json assert_response :unprocessable_content assert_equal ["Username can't be blank"], response.parsed_body["errors"] @@ -31,7 +31,7 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest 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 } + post options_registration_url, params: { registration: { username: "alice" } }, as: :json assert_response :success end @@ -57,13 +57,18 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest assert_no_difference -> { User.count } do post( - callback_registration_url, - params: { credential_nickname: "USB Key" }.merge(public_key_credential) + registration_url, + params: { + registration: { + nickname: "USB Key", + public_key_credential: public_key_credential.to_json + } + } ) end assert_response :unprocessable_content - assert_equal "Couldn't register your Security Key", response.body + assert_equal "Couldn't register your Security Key", response.parsed_body["message"] end test "should register successfully" do @@ -71,7 +76,7 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest 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 } + post options_registration_url, params: { registration: { username: "alice" } }, as: :json assert_response :success end @@ -84,12 +89,18 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest 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) + registration_url, + params: { + registration: { + nickname: "USB Key", + public_key_credential: public_key_credential.to_json + } + } ) end end assert_response :success + assert_equal "Security Key registered successfully", response.parsed_body["message"] end end diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 4bf2a7d0..99a6250a 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -6,20 +6,20 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest test "should initiate registration successfully" do User.create!(username: "alice") - post session_url, params: { session: { username: "alice" }, format: :json } + post options_session_url, params: { session: { username: "alice" } }, as: :json assert_response :success end test "should return error if creating session with inexisting username" do - post session_url, params: { session: { username: "alice" }, format: :json } + post options_session_url, params: { session: { username: "alice" } }, as: :json assert_response :unprocessable_content assert_equal ["Username doesn't exist"], response.parsed_body["errors"] end test "should return error if creating session with blank username" do - post session_url, params: { session: { username: "" }, format: :json } + post options_session_url, params: { session: { username: "" } }, as: :json assert_response :unprocessable_content assert_equal ["Username doesn't exist"], response.parsed_body["errors"]