Merge pull request #432 from debtcollective/od/remove-old-confirm-flow

remove old email confirmation methods

app/controllers/user_confirmations_controller.rb

@@ -3,52 +3,6 @@
 class UserConfirmationsController < ApplicationController
   layout "minimal"
 
-  # GET /user_confirmations?confirmation_token=abcdef
-  def index
-    @confirmation_token = params[:confirmation_token]
-    @user = User.find_by_confirmation_token(@confirmation_token)
-
-    respond_to do |format|
-      if @user
-        format.html { render :index, status: :ok }
-      else
-        format.html { render :index, status: :not_found }
-      end
-    end
-  end
-
-  # POST /user_confirmations
-  def create
-    user = User.send_confirmation_instructions(email: current_user&.email || params[:email])
-
-    respond_to do |format|
-      if user.errors.empty?
-        format.json { render json: {status: "success", message: "Confirmation email sent"}, status: :ok }
-      else
-        format.json { render json: {status: "failed", message: "Invalid confirmation token"}, status: :not_found }
-      end
-    end
-  end
-
-  # POST /user_confirmations/confirm
-  def confirm
-    @user = User.confirm_by_token(user_confirmation_params[:confirmation_token])
-    @user_confirmed = @user.errors.empty?
-
-    respond_to do |format|
-      if @user_confirmed
-        # run the Discourse link account job to fetch and link with a Discourse account
-        LinkDiscourseAccountJob.perform_later(@user)
-
-        format.html { render :confirm, notice: "Email confirmed", status: :ok }
-        format.json { render json: {status: "success", message: "Email confirmed"}, status: :ok }
-      else
-        format.html { render :confirm, notice: "Invalid confirmation token", status: :not_found }
-        format.json { render json: {status: "failed", message: "Invalid confirmation token"}, status: :not_found }
-      end
-    end
-  end
-
   # GET /user_confirmations/confirm_email/:email_token
   def confirm_email_token
     @email_token = params[:email_token]
@@ -69,7 +23,7 @@ def confirm_email
     @user = User.find_by_email_token!(@email_token)
 
     respond_to do |format|
-      if @user&.activate!
+      if @user&.confirm!
         email_login_url = "#{ENV["DISCOURSE_URL"]}/session/email-login/#{@user.email_token}"
 
         format.html { redirect_to email_login_url }

app/jobs/link_discourse_account_job.rb

@@ -13,31 +13,39 @@ def perform(user)
     discourse_user = discourse.find_user_by_email
 
     if discourse_user
-      if user.confirmed?
-        user.update(external_id: discourse_user["id"])
-      else
-        # send confirmation email
-        User.send_confirmation_instructions(email: user.email)
-      end
+      user.external_id = discourse_user["id"]
+      user.username = discourse_user["username"]
 
-      return
-    end
+      response = discourse.create_email_token
 
-    # Create discourse account
-    response = discourse.create_user
+      if [true, "OK"].exclude?(response["success"])
+        return Raven.capture_message(
+          "Couldn't create a Discourse invite",
+          extra: {user_id: user.id, user_email: user.email}
+        )
+      end
+
+      user.email_token = response["email_token"]
+      logger.info("Discourse account #{discourse_user["id"]} linked with user id #{user.id}")
+    else
+      # Create discourse account
+      response = discourse.create_user
+
+      if [true, "OK"].exclude?(response["success"])
+        return Raven.capture_message(
+          "Couldn't create a Discourse invite",
+          extra: {user_id: user.id, user_email: user.email}
+        )
+      end
 
-    if [true, "OK"].exclude?(response["success"])
-      return Raven.capture_message(
-        "Couldn't create a Discourse invite",
-        extra: {user_id: user.id, user_email: user.email}
-      )
+      user.email_token = response["email_token"]
+      user.external_id = response["user_id"]
+      user.username = response["username"]
+      logger.info("Discourse account created. discourse_id: #{response["user_id"]} user_id: #{user.id}")
     end
 
-    user.username = response["username"]
-    user.email_token = response["email_token"]
-    user.external_id = response["user_id"]
     user.save!
 
-    UserMailer.welcome_email(user: user).deliver_later
+    user.send_welcome_email
   end
 end

app/models/user.rb

@@ -5,13 +5,10 @@
 # Table name: users
 #
 #  id                   :bigint           not null, primary key
-#  activated_at         :datetime
-#  active               :boolean          default(FALSE)
 #  admin                :boolean          default(FALSE)
 #  avatar_url           :string
 #  banned               :boolean          default(FALSE)
 #  confirmation_sent_at :datetime
-#  confirmation_token   :string
 #  confirmed_at         :datetime
 #  custom_fields        :jsonb
 #  email                :string
@@ -64,47 +61,21 @@ def self.find_or_create_from_sso(payload)
       user[sso_attribute] = payload[sso_attribute]
     end
 
-    new_record = user.new_record?
+    # Since the user comes from SSO, we can confirm the account
+    user.confirmed_at ||= DateTime.now
 
+    new_record = user.new_record?
     user.save
 
     [user, new_record]
   end
 
-  def self.send_confirmation_instructions(email:)
-    user = User.find_by_email(email.downcase)
-
-    if user
-      confirmation_token = user.confirmation_token || SecureRandom.hex(20)
-      user.update(confirmation_token: confirmation_token, confirmation_sent_at: DateTime.now)
-      UserMailer.confirmation_email(user: user).deliver_later
-    else
-      user = User.new
-      user.errors.add(:base, "User not found")
-    end
-
-    user
-  end
-
-  def self.confirm_by_token(confirmation_token)
-    user = User.find_by_confirmation_token(confirmation_token)
-
-    if user
-      user.update(confirmation_token: nil, confirmed_at: DateTime.now)
-    else
-      user = User.new
-      user.errors.add(:base, "Invalid confirmation token")
-    end
-
-    user
-  end
-
-  def admin?
-    !!admin
+  def confirm!
+    update!(confirmed_at: DateTime.now)
   end
 
-  def activate!
-    update!(active: true, activated_at: DateTime.now)
+  def send_welcome_email
+    UserMailer.welcome_email(user: self).deliver_later
   end
 
   # TODO: We are getting first_name and last_name from users when the join the union

app/services/discourse_service.rb

@@ -30,7 +30,7 @@ def create_user
     user_fields = {}
     USER_FIELDS_MAP.each { |key, sym| user_fields[key] = user.custom_fields.fetch(sym, "") }
 
-    password = SecureRandom.hex(rand(20...24))
+    password = SecureRandom.hex(rand(24...32))
 
     client.create_user({
       email: user.email,
@@ -43,6 +43,11 @@ def create_user
     })
   end
 
+  # This is used to create an email login link
+  def create_email_token
+    client.post("/u/email-token.json", {login: user.email})
+  end
+
   # Check if username is available
   def check_username(username)
     client.check_username(username)

app/views/user_confirmations/confirm_email_token.html.erb

@@ -1,5 +1,5 @@
 <% if @user %>
-  <% if @user.active? %>
+  <% if @user.confirmed? %>
     <p>Your email has been already confirmed</p>
 
     <p><%= link_to "Click here to go to the login page", discourse_url('/login') %></p>

db/migrate/20210224211702_remove_user_confirmation_email.rb

@@ -0,0 +1,6 @@
+class RemoveUserConfirmationEmail < ActiveRecord::Migration[6.1]
+  def change
+    remove_column :users, :active, :boolean, default: false
+    remove_column :users, :confirmation_token, :string, index: true
+  end
+end

db/migrate/20210301230150_remove_activated_at_from_user.rb

@@ -0,0 +1,5 @@
+class RemoveActivatedAtFromUser < ActiveRecord::Migration[6.1]
+  def change
+    remove_column :users, :activated_at, :datetime
+  end
+end

db/schema.rb

@@ -10,7 +10,8 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_02_17_192628) do
+ActiveRecord::Schema.define(version: 2021_03_01_230150) do
+
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -108,11 +109,8 @@
     t.bigint "external_id"
     t.string "name"
     t.string "username"
-    t.string "confirmation_token"
     t.datetime "confirmed_at"
     t.datetime "confirmation_sent_at"
-    t.boolean "active", default: false
-    t.datetime "activated_at"
     t.string "email_token"
     t.index ["email"], name: "index_users_on_email", unique: true
   end

spec/controllers/user_confirmations_controller_spec.rb

@@ -3,67 +3,6 @@
 require "rails_helper"
 
 RSpec.describe UserConfirmationsController, type: :controller do
-  describe "GET #index" do
-    it "returns a success response if token is found" do
-      user = FactoryBot.create(:user_with_confirmation_token)
-
-      get :index, params: {confirmation_token: user.confirmation_token}
-
-      expect(response.status).to eq(200)
-    end
-
-    it "returns not found if token is invalid" do
-      get :index, params: {confirmation_token: SecureRandom.hex(20)}
-
-      expect(response.status).to eq(404)
-    end
-  end
-
-  describe "POST #create" do
-    # This endpoint only supports json
-    before(:each) do
-      request.accept = "application/json"
-    end
-
-    it "creates and send user confirmation instructions" do
-      user = FactoryBot.create(:user)
-
-      expect(UserMailer).to receive_message_chain(:confirmation_email, :deliver_later)
-
-      post :create, params: {email: user.email}
-
-      expect(response.status).to eq(200)
-    end
-
-    it "returns not found if token is invalid" do
-      post :create, params: {email: Faker::Internet.email}
-
-      expect(response.status).to eq(404)
-    end
-  end
-
-  describe "POST #confirm" do
-    it "confirms user email if token is valid" do
-      user = FactoryBot.create(:user_with_confirmation_token)
-
-      expect(LinkDiscourseAccountJob).to receive_message_chain(:perform_later)
-
-      post :confirm, params: {user_confirmation: {confirmation_token: user.confirmation_token}}
-      user.reload
-
-      expect(response.status).to eq(200)
-      expect(user.confirmed_at).to be_within(1.second).of DateTime.now
-      expect(user.confirmed?).to eq(true)
-      expect(user.confirmation_token).to be_nil
-    end
-
-    it "returns not found if token is invalid" do
-      post :confirm, params: {user_confirmation: {confirmation_token: SecureRandom.hex(20)}}
-
-      expect(response.status).to eq(404)
-    end
-  end
-
   describe "GET #confirm_email_token" do
     render_views
 
@@ -75,7 +14,7 @@
 
       expect(response.status).to eq(200)
       expect(response.body).to include("Confirm my email")
-      expect(user.active?).to eq(false)
+      expect(user.confirmed?).to eq(false)
     end
 
     it "returns not found if token is invalid" do
@@ -91,8 +30,8 @@
       user.reload
 
       expect(response.status).to eq(302)
-      expect(user.active?).to eq(true)
-      expect(user.activated_at).to be_within(1.second).of DateTime.now
+      expect(user.confirmed?).to eq(true)
+      expect(user.confirmed_at).to be_within(1.second).of DateTime.now
     end
 
     it "returns not found if token is invalid" do

spec/factories/users.rb

@@ -5,13 +5,10 @@
 # Table name: users
 #
 #  id                   :bigint           not null, primary key
-#  activated_at         :datetime
-#  active               :boolean          default(FALSE)
 #  admin                :boolean          default(FALSE)
 #  avatar_url           :string
 #  banned               :boolean          default(FALSE)
 #  confirmation_sent_at :datetime
-#  confirmation_token   :string
 #  confirmed_at         :datetime
 #  custom_fields        :jsonb
 #  email                :string