GRN2-233: Made account activation & password reset links based on tokens only (#959)

* GRN2-233: Hiding email in verification link and password reset link

* updating tests

* removing uid from email verificaiton link

* GRN2-233: modifying test cases

* GRN2-233: Removing uid from password reset link

* GRN2-233: Removed email_params and fixed "authenticated?" method

* GRN2-233: Fixed error when trying to sign in unverified

* GRN2-233: Changed how activation tokens are generated

app/controllers/account_activations_controller.rb

@@ -51,6 +51,7 @@ class AccountActivationsController < ApplicationController
       flash[:alert] = I18n.t("verify.already_verified")
     else
       # Resend
+      @user.create_activation_token
       send_activation_email(@user)
     end
 
@@ -60,14 +61,10 @@ class AccountActivationsController < ApplicationController
   private
 
   def find_user
-    @user = User.find_by!(email: params[:email], provider: @user_domain)
+    @user = User.find_by!(activation_digest: User.digest(params[:token]), provider: @user_domain)
   end
 
   def ensure_unauthenticated
     redirect_to current_user.main_room if current_user
   end
-
-  def email_params
-    params.require(:email).permit(:email, :token)
-  end
 end

app/controllers/concerns/emailer.rb

@@ -125,7 +125,7 @@ module Emailer
 
   # Returns the link the user needs to click to verify their account
   def user_verification_link(user)
-    edit_account_activation_url(token: user.activation_token, email: user.email)
+    edit_account_activation_url(token: user.activation_token)
   end
 
   def admin_emails
@@ -140,7 +140,7 @@ module Emailer
   end
 
   def reset_link(user)
-    edit_password_reset_url(user.reset_token, email: user.email)
+    edit_password_reset_url(user.reset_token)
   end
 
   def invitation_link(token)

app/controllers/password_resets_controller.rb

@@ -68,7 +68,7 @@ class PasswordResetsController < ApplicationController
   private
 
   def find_user
-    @user = User.find_by(email: params[:email], provider: @user_domain)
+    @user = User.find_by(reset_digest: User.digest(params[:id]), provider: @user_domain)
   end
 
   def user_params

app/controllers/sessions_controller.rb

@@ -88,7 +88,10 @@ class SessionsController < ApplicationController
       # Check that the user is a Greenlight account
       return redirect_to(root_path, alert: I18n.t("invalid_login_method")) unless user.greenlight_account?
       # Check that the user has verified their account
-      return redirect_to(account_activation_path(email: user.email)) unless user.activated?
+      unless user.activated?
+        user.create_activation_token
+        return redirect_to(account_activation_path(token: user.activation_token))
+      end
     end
 
     login(user)

app/controllers/users_controller.rb

@@ -58,6 +58,8 @@ class UsersController < ApplicationController
     # Sign in automatically if email verification is disabled or if user is already verified.
     login(@user) && return if !Rails.configuration.enable_email_verification || @user.email_verified
 
+    @user.create_activation_token
+
     send_activation_email(@user)
 
     redirect_to root_path

app/models/user.rb

@@ -21,7 +21,7 @@ require 'bbb_api'
 class User < ApplicationRecord
   include Deleteable
 
-  attr_accessor :reset_token
+  attr_accessor :reset_token, :activation_token
   after_create :setup_user
 
   before_save { email.try(:downcase!) }
@@ -122,7 +122,7 @@ class User < ApplicationRecord
   def authenticated?(attribute, token)
     digest = send("#{attribute}_digest")
     return false if digest.nil?
-    BCrypt::Password.new(digest).is_password?(token)
+    digest == Digest::SHA256.base64digest(token)
   end
 
   # Return true if password reset link expires
@@ -153,9 +153,9 @@ class User < ApplicationRecord
     social_uid.nil?
   end
 
-  def activation_token
+  def create_activation_token
-    # Create the token.
+    self.activation_token = User.new_token
-    create_reset_activation_digest(User.new_token)
+    update_attributes(activation_digest: User.digest(activation_token))
   end
 
   def admin_of?(user)
@@ -172,8 +172,7 @@ class User < ApplicationRecord
   end
 
   def self.digest(string)
-    cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine.cost
+    Digest::SHA256.base64digest(string)
-    BCrypt::Password.create(string, cost: cost)
   end
 
   # Returns a random token.
@@ -250,12 +249,6 @@ class User < ApplicationRecord
 
   private
 
-  def create_reset_activation_digest(token)
-    # Create the digest and persist it.
-    update_attribute(:activation_digest, User.digest(token))
-    token
-  end
-
   # Destory a users rooms when they are removed.
   def destroy_rooms
     rooms.destroy_all

app/views/account_activations/show.html.erb

@@ -22,7 +22,7 @@
       <div class="card-body">
         <p><%= t("verify.not_verified") %></p>
         <div class="btn-list text-right pt-8">
-          <%= button_to t("verify.resend"), resend_email_path, params: { email: params['email'], email_verified: false }, class: "btn btn-primary btn-space", "data-disable": "" %>
+          <%= button_to t("verify.resend"), resend_email_path, params: { token: params['token'], email_verified: false }, class: "btn btn-primary btn-space", "data-disable": "" %>
         </div>
       </div>
     </div>

spec/controllers/account_activations_controller_spec.rb

@@ -27,7 +27,7 @@ describe AccountActivationsController, type: :controller do
       user = create(:user, provider: "greenlight")
       @request.session[:user_id] = user.id
 
-      get :show, params: { email: user.email }
+      get :show, params: { uid: user.uid }
 
       expect(response).to redirect_to(user.main_room)
     end
@@ -35,7 +35,8 @@ describe AccountActivationsController, type: :controller do
     it "renders the verify view if the user is not signed in and is not verified" do
       user = create(:user, email_verified: false,  provider: "greenlight")
 
-      get :show, params: { email: user.email }
+      user.create_activation_token
+      get :show, params: { token: user.activation_token }
 
       expect(response).to render_template(:show)
     end
@@ -45,7 +46,8 @@ describe AccountActivationsController, type: :controller do
     it "activates a user if they have the correct activation token" do
       @user = create(:user, email_verified: false, provider: "greenlight")
 
-      get :edit, params: { email: @user.email, token: @user.activation_token }
+      @user.create_activation_token
+      get :edit, params: { token: @user.activation_token }
       @user.reload
 
       expect(@user.email_verified).to eq(true)
@@ -53,22 +55,17 @@ describe AccountActivationsController, type: :controller do
       expect(response).to redirect_to(signin_path)
     end
 
-    it "does not activate a user if they have the correct activation token" do
+    it "should not find user when given fake activation token" do
       @user = create(:user, email_verified: false, provider: "greenlight")
 
-      get :edit, params: { email: @user.email, token: "fake_token" }
+      expect { get :edit, params: { token: "fake_token" } }.to raise_error(ActiveRecord::RecordNotFound)
-      @user.reload
-
-      expect(@user.email_verified).to eq(false)
-      expect(flash[:alert]).to be_present
-      expect(response).to redirect_to(root_path)
     end
 
     it "does not allow the user to click the verify link again" do
       @user = create(:user, provider: "greenlight")
 
-      get :edit, params: { email: @user.email, token: @user.activation_token }
+      @user.create_activation_token
-
+      get :edit, params: { token: @user.activation_token }
       expect(flash[:alert]).to be_present
       expect(response).to redirect_to(root_path)
     end
@@ -78,7 +75,8 @@ describe AccountActivationsController, type: :controller do
 
       @user.add_role :pending
 
-      get :edit, params: { email: @user.email, token: @user.activation_token }
+      @user.create_activation_token
+      get :edit, params: { token: @user.activation_token }
 
       expect(flash[:success]).to be_present
       expect(response).to redirect_to(root_path)
@@ -89,7 +87,8 @@ describe AccountActivationsController, type: :controller do
     it "resends the email to the current user if the resend button is clicked" do
       user = create(:user, email_verified: false, provider: "greenlight")
 
-      expect { get :resend, params: { email: user.email } }.to change { ActionMailer::Base.deliveries.count }.by(1)
+      user.create_activation_token
+      expect { get :resend, params: { token: user.activation_token } }.to change { ActionMailer::Base.deliveries.count }.by(1)
       expect(flash[:success]).to be_present
       expect(response).to redirect_to(root_path)
     end
@@ -97,7 +96,8 @@ describe AccountActivationsController, type: :controller do
     it "redirects a verified user to the root path" do
       user = create(:user, provider: "greenlight")
 
-      get :resend, params: { email: user.email }
+      user.create_activation_token
+      get :resend, params: { token: user.activation_token }
 
       expect(flash[:alert]).to be_present
       expect(response).to redirect_to(root_path)

spec/controllers/password_resets_controller_spec.rb

@@ -116,18 +116,14 @@ describe PasswordResetsController, type: :controller do
 
       it "updates attributes if the password update is a success" do
         user = create(:user, provider: "greenlight")
-        token = "reset_token"
+        user.create_reset_digest
-
+        old_digest = user.password_digest
-        cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine.cost
-        user.reset_digest = BCrypt::Password.create(token, cost: cost)
 
         allow(controller).to receive(:valid_user).and_return(nil)
         allow(controller).to receive(:check_expiration).and_return(nil)
-        controller.instance_variable_set(:@user, user)
 
         params = {
-          id: token,
+          id: user.reset_token,
-          email: user.email,
           user: {
             password: :password,
             password_confirmation: :password,
@@ -135,6 +131,10 @@ describe PasswordResetsController, type: :controller do
         }
 
         patch :update, params: params
+
+        user.reload
+
+        expect(old_digest.eql?(user.password_digest)).to be false
         expect(response).to redirect_to(root_path)
       end
     end

spec/controllers/sessions_controller_spec.rb

@@ -143,7 +143,8 @@ describe SessionsController, type: :controller do
       }
 
       expect(@request.session[:user_id]).to be_nil
-      expect(response).to redirect_to(account_activation_path(email: @user3.email))
+      # Expect to redirect to activation path since token is not known here
+      expect(response.location.start_with?(account_activation_url(token: ""))).to be true
     end
 
     it "should not login user if account is deleted" do