Stop collecting PII (#153)

mikker · Jun 21, 2023 · c459557 · c459557
1 parent fc5fd7e
commit c459557
Show file tree

Hide file tree

Showing 11 changed files with 33 additions and 67 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,14 +4,31 @@
 
 ### Breaking changes
 
-#### 1. Encrypted tokens
-
-Tokens are now encrypted in the database. If you are upgrading from a previous version, you'll need to add a field to your passwordless table:
+If you're already running Passwordless, you'll need to update your database schema.
 
 ```sh
-$ bin/rails g migration AddTokenDigestToPasswordlessSessions token_digest:string:index
+$ bin/rails g migration UpgradePassswordless
+```
+
+```ruby
+class UpgradePassswordless < ActiveRecord::Migration[7.0]
+  def change
+    # Encrypted tokens
+    add_column(:passwordless_sessions, :token_digest, :string)
+    add_index(:passwordless_sessions, :token_digest)
+    remove_column(:passwordless_sessions, :token, :string, null: false)
+
+    # Remove PII
+    remove_column(:passwordless_sessions, :user_agent, :string, null: false)
+    remove_column(:passwordless_sessions, :remote_addr, :string, null: false)
+  end
+end
 ```
 
+#### 1. Encrypted tokens
+
+Tokens are now encrypted in the database.
+
 #### 2. Un-isolated namespace
 
 Passwordless no longer [_isolates namespace_](https://guides.rubyonrails.org/engines.html#routes).
@@ -23,6 +40,11 @@ Passwordless no longer [_isolates namespace_](https://guides.rubyonrails.org/eng
 
 Removes `authenticate_by_cookie` and `upgrade_passwordless_cookie` from controller helpers.
 
+#### 4. Stop collecting PII
+
+Passwordless no longer collects users' IP addresses. If you need this information, you can
+add it to your `after_session_save` callback.
+
 ### Changed
 
 - Tokens are now encrypted in the database ([#145](https://github.com/mikker/passwordless/pull/145))
@@ -31,6 +53,7 @@ Removes `authenticate_by_cookie` and `upgrade_passwordless_cookie` from controll
 ### Removed
 
 - Deprecated methods and helpers ([#147](https://github.com/mikker/passwordless/pull/147))
+- Collection of PII (IP address, User Agent) ([#153](https://github.com/mikker/passwordless/pull/153))
 
 ### Fixed
 

diff --git a/README.md b/README.md
@@ -19,7 +19,6 @@ Add authentication to your Rails app without all the icky-ness of passwords.
   - [Registering new users](#registering-new-users)
   - [URLs and links](#urls-and-links)
   - [Customize the way to send magic link](#customize-the-way-to-send-magic-link)
-  - [Generate your own magic links](#generate-your-own-magic-links)
   - [Overrides](#overrides)
 - [Configuration](#configuration)
   - [Customising token generation](#customizing-token-generation)
@@ -197,28 +196,6 @@ end
 
 You can access user model through authenticatable.
 
-### Generate your own magic links
-
-Currently there is not an officially supported way to generate your own magic links to send in your own mailers.
-
-However, you can accomplish this with the following snippet of code.
-
-```ruby
-session = Passwordless::Session.create!({
-  authenticatable: @manager,
-  user_agent: 'Command Line',
-  remote_addr: 'unknown',
-})
-
-@magic_link = users_token_sign_in_url(session.token)
-```
-
-You can further customize this URL by specifying the destination path to be redirected to after the user has logged in. You can do this by adding the `destination_path` query parameter to the end of the URL. For example
-
-```
-@magic_link = "#{@magic_link}?destination_path=/your-custom-path"
-```
-
 ### Overrides
 
 By default `passwordless` uses the `passwordless_with` column to _case insensitively_ fetch the resource.

diff --git a/app/models/passwordless/session.rb b/app/models/passwordless/session.rb
@@ -16,8 +16,6 @@ class Session < ApplicationRecord
       :authenticatable,
       :timeout_at,
       :expires_at,
-      :user_agent,
-      :remote_addr,
       :token_digest,
       presence: true
     )

diff --git a/db/migrate/20171104221735_create_passwordless_sessions.rb b/db/migrate/20171104221735_create_passwordless_sessions.rb
@@ -12,8 +12,6 @@ def change
       t.datetime(:timeout_at, null: false)
       t.datetime(:expires_at, null: false)
       t.datetime(:claimed_at)
-      t.text(:user_agent, null: false)
-      t.string(:remote_addr, null: false)
       t.string(:token_digest, null: false)
 
       t.timestamps

diff --git a/lib/passwordless/controller_helpers.rb b/lib/passwordless/controller_helpers.rb
@@ -10,17 +10,12 @@ def find_passwordless_session_for(authenticatable_class)
     end
 
     # Build a new Passwordless::Session from an _authenticatable_ record.
-    # Set's `user_agent` and `remote_addr` from Rails' `request`.
     # @param authenticatable [ActiveRecord::Base] Instance of an
     #   authenticatable Rails model
     # @return [Session] the new Session object
     # @see ModelHelpers#passwordless_with
     def build_passwordless_session(authenticatable)
-      Session.new.tap do |us|
-        us.remote_addr = request.remote_addr
-        us.user_agent = request.env["HTTP_USER_AGENT"]
-        us.authenticatable = authenticatable
-      end
+      Session.new(authenticatable: authenticatable)
     end
 
     # Authenticate a record using the session. Looks for a session key corresponding to

diff --git a/lib/passwordless/test_helpers.rb b/lib/passwordless/test_helpers.rb
@@ -7,11 +7,7 @@ def passwordless_sign_out
       end
 
       def passwordless_sign_in(resource)
-        session = Passwordless::Session.create!(
-          authenticatable: resource,
-          user_agent: "TestAgent",
-          remote_addr: "unknown"
-        )
+        session = Passwordless::Session.create!(authenticatable: resource)
         get(Passwordless::Engine.routes.url_helpers.token_sign_in_path(session.token))
         follow_redirect!
       end
@@ -23,11 +19,7 @@ def passwordless_sign_out
       end
 
       def passwordless_sign_in(resource)
-        session = Passwordless::Session.create!(
-          authenticatable: resource,
-          user_agent: "TestAgent",
-          remote_addr: "unknown"
-        )
+        session = Passwordless::Session.create!(authenticatable: resource)
         visit(Passwordless::Engine.routes.url_helpers.token_sign_in_path(session.token))
       end
     end

diff --git a/test/controllers/passwordless/sessions_controller_test.rb b/test/controllers/passwordless/sessions_controller_test.rb
@@ -5,11 +5,7 @@
 module Passwordless
   class SessionsControllerTest < ActionDispatch::IntegrationTest
     def create_session_for(user)
-      Session.create!(
-        authenticatable: user,
-        remote_addr: "yes",
-        user_agent: "James Bond"
-      )
+      Session.create!(authenticatable: user)
     end
 
     class Helpers

diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb
@@ -17,8 +17,6 @@
     t.datetime "timeout_at", precision: nil, null: false
     t.datetime "expires_at", precision: nil, null: false
     t.datetime "claimed_at", precision: nil
-    t.text "user_agent", null: false
-    t.string "remote_addr", null: false
     t.string "token_digest", null: false
     t.datetime "created_at", precision: nil, null: false
     t.datetime "updated_at", precision: nil, null: false

diff --git a/test/fixtures/passwordless/sessions.yml b/test/fixtures/passwordless/sessions.yml
@@ -3,21 +3,15 @@
 one:
   timeout_at: 2017-11-04 23:17:35
   expires_at: 2017-11-04 23:17:35
-  user_agent: MyText
-  remote_addr: MyString
   token_digest: MyString
 
 two:
   timeout_at: 2017-11-04 23:17:35
   expires_at: 2017-11-04 23:17:35
-  user_agent: MyText
-  remote_addr: MyString
   token_digest: MyString
 
 john:
   timeout_at: 2017-11-04 23:17:35
   expires_at: 2017-11-04 23:17:35
-  user_agent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.62 Safari/537.36"
-  remote_addr: 127.0.0.1
   token_digest: 3veDCdFw4VedgZtZZkCJ7um7rsc2bFNCZGB51Iih024
   authenticatable: john (User)
diff --git a/test/integration/navigation_test.rb b/test/integration/navigation_test.rb
@@ -33,8 +33,7 @@ class NavigationTest < ActionDispatch::IntegrationTest
     assert flash["notice"].include?("If we found you in the system")
 
     # Expect session created for alice
-    user_session = Passwordless::Session.find_by!(authenticatable: alice)
-    assert_equal "Mosaic v.1", user_session.user_agent
+    Passwordless::Session.find_by!(authenticatable: alice)
 
     # Expect mail for alice
     assert_equal 1, ActionMailer::Base.deliveries.count

diff --git a/test/models/passwordless/session_test.rb b/test/models/passwordless/session_test.rb
@@ -6,11 +6,7 @@ module Passwordless
   class SessionTest < ActiveSupport::TestCase
     def create_session(attrs = {})
       Session.create!(
-        attrs.reverse_merge(
-          remote_addr: "0.0.0.0",
-          user_agent: "wooden box",
-          authenticatable: User.create(email: "session_test_valid@a")
-        )
+        attrs.reverse_merge(authenticatable: User.create(email: "session_test_valid@a"))
       )
     end