diff --git a/CHANGELOG.md b/CHANGELOG.md index 30d5ba7..e5fd606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ # Unreleased +### Changed + +Sessions are now referenced publicly by a random UUID instead of their primary key. + +This needs a manual database migration like so: + +```ruby +class AddIndentifierToPasswordlessSessions < ActiveRecord::Migration[7.1] + def change + add_column(:passwordless_sessions, :identifier, :string) + add_index(:passwordless_sessions, :identifier, unique: true) + end +end +``` + ### Added - Add default flash notice for sign out (#178) diff --git a/app/controllers/passwordless/sessions_controller.rb b/app/controllers/passwordless/sessions_controller.rb index 90bb08f..0d8e1ba 100644 --- a/app/controllers/passwordless/sessions_controller.rb +++ b/app/controllers/passwordless/sessions_controller.rb @@ -37,7 +37,7 @@ def create end redirect_to( - url_for(id: @session.id, action: "show"), + url_for(id: @session.identifier, action: "show"), flash: {notice: I18n.t("passwordless.sessions.create.email_sent")} ) else @@ -54,7 +54,7 @@ def create # Shows the form for confirming a Session record. # renders sessions/show.html.erb. def show - @session = find_session + @session = passwordless_session end # patch "/:resource/sign_in/:id" @@ -66,7 +66,7 @@ def show # @see ControllerHelpers#sign_in # @see ControllerHelpers#save_passwordless_redirect_location! def update - @session = find_session + @session = passwordless_session artificially_slow_down_brute_force_attacks(passwordless_session_params[:token]) @@ -86,7 +86,7 @@ def confirm # safe. We don't want to sign in the user in that case. return head(:ok) if request.head? - @session = find_session + @session = passwordless_session artificially_slow_down_brute_force_attacks(params[:token]) @@ -166,10 +166,6 @@ def authenticatable_class authenticatable_type.constantize end - def find_session - Session.find_by!(id: params[:id], authenticatable_type: authenticatable_type) - end - def find_authenticatable email = passwordless_session_params[email_field].downcase.strip @@ -201,7 +197,7 @@ def redirect_to_options def passwordless_session @passwordless_session ||= Session.find_by!( - id: params[:id], + identifier: params[:id], authenticatable_type: authenticatable_type ) end diff --git a/app/mailers/passwordless/mailer.rb b/app/mailers/passwordless/mailer.rb index 0bbe859..2d7a1d1 100644 --- a/app/mailers/passwordless/mailer.rb +++ b/app/mailers/passwordless/mailer.rb @@ -16,7 +16,7 @@ def sign_in(session, token = nil) { controller: "passwordless/sessions", action: "confirm", - id: session.id, + id: session.identifier, token: token, authenticatable: "user", resource: "users" diff --git a/app/models/passwordless/session.rb b/app/models/passwordless/session.rb index e37f9fd..adbc1d5 100644 --- a/app/models/passwordless/session.rb +++ b/app/models/passwordless/session.rb @@ -61,6 +61,10 @@ def available? !expired? end + def to_param + identifier + end + private def token_digest_available?(token_digest) @@ -68,6 +72,7 @@ def token_digest_available?(token_digest) end def set_defaults + self.identifier = SecureRandom.uuid self.expires_at ||= Passwordless.config.expires_at.call self.timeout_at ||= Passwordless.config.timeout_at.call diff --git a/db/migrate/20171104221735_create_passwordless_sessions.rb b/db/migrate/20171104221735_create_passwordless_sessions.rb index 7b41f61..acfede0 100644 --- a/db/migrate/20171104221735_create_passwordless_sessions.rb +++ b/db/migrate/20171104221735_create_passwordless_sessions.rb @@ -13,6 +13,7 @@ def change t.datetime(:expires_at, null: false) t.datetime(:claimed_at) t.string(:token_digest, null: false) + t.string(:identifier, null: false, index: {unique: true}, length: 36) t.timestamps end diff --git a/docs/upgrading_to_1_0.md b/docs/upgrading_to_1_0.md index d5fa5ab..9824fc0 100644 --- a/docs/upgrading_to_1_0.md +++ b/docs/upgrading_to_1_0.md @@ -20,7 +20,7 @@ $ bin/rails g migration UpgradePassswordless ``` ```ruby -class UpgradePassswordless < ActiveRecord::Migration[7.0] +class UpgradePasswordless < ActiveRecord::Migration[7.0] def change # Encrypted tokens add_column(:passwordless_sessions, :token_digest, :string) diff --git a/test/controllers/passwordless/sessions_controller_test.rb b/test/controllers/passwordless/sessions_controller_test.rb index 39d2efb..415c91b 100644 --- a/test/controllers/passwordless/sessions_controller_test.rb +++ b/test/controllers/passwordless/sessions_controller_test.rb @@ -28,7 +28,7 @@ def create_pwless_session(attrs = {}) assert_equal 1, ActionMailer::Base.deliveries.size follow_redirect! - assert_equal "/users/sign_in/#{Session.last!.id}", path + assert_equal "/users/sign_in/#{Session.last!.identifier}", path end test("POST /:passwordless_for/sign_in -> SUCCESS / malformed email") do @@ -40,7 +40,7 @@ def create_pwless_session(attrs = {}) assert_equal 1, ActionMailer::Base.deliveries.size follow_redirect! - assert_equal "/users/sign_in/#{Session.last!.id}", path + assert_equal "/users/sign_in/#{Session.last!.identifier}", path end test("POST /:passwordless_for/sign_in -> SUCCESS / custom delivery method") do @@ -111,7 +111,7 @@ class << User test("GET /:passwordless_for/sign_in/:id") do passwordless_session = create_pwless_session - get("/users/sign_in/#{passwordless_session.id}") + get("/users/sign_in/#{passwordless_session.identifier}") assert_equal 200, status assert_equal "/users/sign_in/#{passwordless_session.to_param}", path @@ -121,7 +121,7 @@ class << User test("PATCH /:passwordless_for/sign_in/:id -> SUCCESS") do passwordless_session = create_pwless_session(token: "hi") - patch("/users/sign_in/#{passwordless_session.id}", params: {passwordless: {token: "hi"}}) + patch("/users/sign_in/#{passwordless_session.identifier}", params: {passwordless: {token: "hi"}}) assert_equal 303, status @@ -135,7 +135,7 @@ class << User test("PATCH /:passwordless_for/sign_in/:id -> ERROR") do passwordless_session = create_pwless_session(token: "hi") - patch("/users/sign_in/#{passwordless_session.id}", params: {passwordless: {token: "no"}}) + patch("/users/sign_in/#{passwordless_session.identifier}", params: {passwordless: {token: "no"}}) assert_equal 403, status assert_equal "/users/sign_in/#{passwordless_session.to_param}", path @@ -150,7 +150,7 @@ class << User with_config(restrict_token_reuse: true) do patch( - "/users/sign_in/#{passwordless_session.id}", + "/users/sign_in/#{passwordless_session.identifier}", params: {passwordless: {token: "hi"}} ) end @@ -170,7 +170,7 @@ class << User passwordless_session.update!(timeout_at: Time.current - 1.day) patch( - "/users/sign_in/#{passwordless_session.id}", + "/users/sign_in/#{passwordless_session.identifier}", params: {passwordless: {token: "hi"}} ) @@ -188,7 +188,7 @@ class << User user = User.create(email: "a@a") passwordless_session = create_pwless_session(authenticatable: user, token: "hi") - get "/users/sign_in/#{passwordless_session.id}/#{passwordless_session.token}" + get "/users/sign_in/#{passwordless_session.identifier}/#{passwordless_session.token}" assert_not_nil pwless_session(User) get "/users/sign_out" diff --git a/test/dummy/config/environments/test.rb b/test/dummy/config/environments/test.rb index 9788f55..d277093 100644 --- a/test/dummy/config/environments/test.rb +++ b/test/dummy/config/environments/test.rb @@ -28,7 +28,7 @@ config.cache_store = :null_store # Raise exceptions instead of rendering exception templates. - config.action_dispatch.show_exceptions = false + config.action_dispatch.show_exceptions = :internal # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 1091198..b71a846 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -18,9 +18,11 @@ t.datetime "expires_at", precision: nil, null: false t.datetime "claimed_at", precision: nil t.string "token_digest", null: false + t.string "identifier", null: false t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.index ["authenticatable_type", "authenticatable_id"], name: "authenticatable" + t.index ["identifier"], name: "index_passwordless_sessions_on_identifier", unique: true end create_table "users", force: :cascade do |t| diff --git a/test/fixtures/passwordless/sessions.yml b/test/fixtures/passwordless/sessions.yml index e0761db..1d1919e 100644 --- a/test/fixtures/passwordless/sessions.yml +++ b/test/fixtures/passwordless/sessions.yml @@ -1,16 +1,19 @@ # Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html one: + identifier: 381a283e-4e64-4989-8b10-d5279bb7f338 timeout_at: 2017-11-04 23:17:35 expires_at: 2017-11-04 23:17:35 token_digest: MyString two: + identifier: cb756ce8-62a0-4ac1-9cb2-7abce7eddf80 timeout_at: 2017-11-04 23:17:35 expires_at: 2017-11-04 23:17:35 token_digest: MyString john: + identifier: 3d14ed2c-4eaa-4894-82fb-e2e6f92223e6 timeout_at: 2017-11-04 23:17:35 expires_at: 2017-11-04 23:17:35 token_digest: 3veDCdFw4VedgZtZZkCJ7um7rsc2bFNCZGB51Iih024 diff --git a/test/integration/authentication_flow_test.rb b/test/integration/authentication_flow_test.rb index 515e0e5..ae34560 100644 --- a/test/integration/authentication_flow_test.rb +++ b/test/integration/authentication_flow_test.rb @@ -28,7 +28,7 @@ class AuthenticationFlowTest < ActionDispatch::SystemTestCase email = ActionMailer::Base.deliveries.last assert_equal alice.email, email.to.first - token = email.body.match(%r{/sign_in/\d+/([\w\-_]+)})[1] + token = email.body.match(%r{http://.*/sign_in/[A-Za-z0-9\-]+/(\w+)})[1] fill_in "passwordless[token]", with: token click_button "Confirm" @@ -52,7 +52,7 @@ class AuthenticationFlowTest < ActionDispatch::SystemTestCase email = ActionMailer::Base.deliveries.last assert_equal alice.email, email.to.first - magic_link = email.body.to_s.scan(%r{http://.*/sign_in/\d+/\w+}).at(0) + magic_link = email.body.to_s.scan(%r{http://.*/sign_in/[a-z0-9\-]+/\w+}).at(0) visit magic_link diff --git a/test/mailers/passwordless/mailer_test.rb b/test/mailers/passwordless/mailer_test.rb index f579876..0526fd5 100644 --- a/test/mailers/passwordless/mailer_test.rb +++ b/test/mailers/passwordless/mailer_test.rb @@ -11,6 +11,6 @@ class Passwordless::MailerTest < ActionMailer::TestCase assert_match "Signing in ✨", email.subject assert_match /sign in: hello\n/, email.body.to_s - assert_match %r{/sign_in/#{session.id}/hello}, email.body.to_s + assert_match %r{/sign_in/#{session.identifier}/hello}, email.body.to_s end end