Skip to content

Commit

Permalink
Use UUIDs as indentifiers for sessions in public (#176)
Browse files Browse the repository at this point in the history
  • Loading branch information
mikker committed Nov 7, 2023
1 parent 8855a80 commit 3af9774
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 23 deletions.
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 5 additions & 9 deletions app/controllers/passwordless/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand All @@ -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])

Expand All @@ -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])

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/mailers/passwordless/mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 5 additions & 0 deletions app/models/passwordless/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,18 @@ def available?
!expired?
end

def to_param
identifier
end

private

def token_digest_available?(token_digest)
Session.available.where(token_digest: token_digest).none?
end

def set_defaults
self.identifier = SecureRandom.uuid
self.expires_at ||= Passwordless.config.expires_at.call
self.timeout_at ||= Passwordless.config.timeout_at.call

Expand Down
1 change: 1 addition & 0 deletions db/migrate/20171104221735_create_passwordless_sessions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/upgrading_to_1_0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 8 additions & 8 deletions test/controllers/passwordless/sessions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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"}}
)

Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion test/dummy/config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/dummy/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/passwordless/sessions.yml
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
# Read about fixtures at http:https://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
Expand Down
4 changes: 2 additions & 2 deletions test/integration/authentication_flow_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:https://.*/sign_in/[A-Za-z0-9\-]+/(\w+)})[1]

fill_in "passwordless[token]", with: token
click_button "Confirm"
Expand All @@ -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:https://.*/sign_in/\d+/\w+}).at(0)
magic_link = email.body.to_s.scan(%r{http:https://.*/sign_in/[a-z0-9\-]+/\w+}).at(0)

visit magic_link

Expand Down
2 changes: 1 addition & 1 deletion test/mailers/passwordless/mailer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 3af9774

Please sign in to comment.