From 5a162c6f4ff923c2d8c1faddbbafdd0a2e4dd4f9 Mon Sep 17 00:00:00 2001
From: Riccardo Graziosi <31478034+riggraz@users.noreply.github.com>
Date: Tue, 14 May 2024 17:47:17 +0200
Subject: [PATCH] Add instructions to set password for OAuth users (#346)

---
 app/controllers/passwords_controller.rb       | 19 +++++++++
 app/controllers/registrations_controller.rb   | 10 +++++
 app/controllers/tenants_controller.rb         |  1 +
 app/javascript/components/Board/NewPost.tsx   |  2 +-
 .../UserProfile/SetPasswordDialog.tsx         | 41 +++++++++++++++++++
 app/javascript/components/common/Button.tsx   |  4 +-
 app/views/devise/registrations/edit.html.erb  | 22 +++++++++-
 app/workflows/o_auth_sign_in_user_workflow.rb |  1 +
 config/locales/en.yml                         |  5 ++-
 config/routes.rb                              |  7 +++-
 ...514112836_add_has_set_password_to_users.rb |  5 +++
 db/schema.rb                                  |  3 +-
 spec/system/user_edit_profile_spec.rb         |  5 +--
 13 files changed, 116 insertions(+), 9 deletions(-)
 create mode 100644 app/controllers/passwords_controller.rb
 create mode 100644 app/javascript/components/UserProfile/SetPasswordDialog.tsx
 create mode 100644 db/migrate/20240514112836_add_has_set_password_to_users.rb

diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
new file mode 100644
index 000000000..552813995
--- /dev/null
+++ b/app/controllers/passwords_controller.rb
@@ -0,0 +1,19 @@
+class PasswordsController < Devise::PasswordsController
+  # Needed to have Current.tenant available in Devise's controllers
+  prepend_before_action :load_tenant_data
+
+  # Needed for OAuth users (otherwise an already logged in user wouldn't be able to reset their password)
+  skip_before_action :require_no_authentication, :only => [:edit, :update]
+
+  def update
+    super
+
+    if resource.errors.empty?
+      resource.update!(has_set_password: true) unless resource.has_set_password?
+
+      # See: https://stackoverflow.com/a/22110985/1857435
+      sign_out(resource_name)
+      sign_in(resource_name, resource)
+    end
+  end
+end
\ No newline at end of file
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 43a8699af..11e59088a 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -17,6 +17,16 @@ def destroy
     respond_with_navigational(resource){ redirect_to after_sign_out_path_for(resource_name) }
   end
 
+  def send_set_password_instructions
+    user = User.find_by_email(params[:email])
+    
+    if user.present?
+      user.send_reset_password_instructions
+    end
+    
+    render json: { success: true } # always return true, even if user not found
+  end
+    
   private
 
     def set_page_title_new
diff --git a/app/controllers/tenants_controller.rb b/app/controllers/tenants_controller.rb
index dbf97a5de..c1cd27328 100644
--- a/app/controllers/tenants_controller.rb
+++ b/app/controllers/tenants_controller.rb
@@ -42,6 +42,7 @@ def create
         full_name: params[:user][:full_name] || I18n.t('defaults.user_full_name'),
         email: params[:user][:email],
         password: is_o_auth_login ? Devise.friendly_token : params[:user][:password],
+        has_set_password: !is_o_auth_login,
         role: "owner"
       )
 
diff --git a/app/javascript/components/Board/NewPost.tsx b/app/javascript/components/Board/NewPost.tsx
index 328526aff..f0ab0510d 100644
--- a/app/javascript/components/Board/NewPost.tsx
+++ b/app/javascript/components/Board/NewPost.tsx
@@ -125,7 +125,7 @@ class NewPost extends React.Component<Props, State> {
 
     } catch (e) {
       this.setState({
-        error: I18n.t('board.new_post.submit_error')
+        error: I18n.t('common.errors.unknown')
       });
     }
   }
diff --git a/app/javascript/components/UserProfile/SetPasswordDialog.tsx b/app/javascript/components/UserProfile/SetPasswordDialog.tsx
new file mode 100644
index 000000000..50f43d0f3
--- /dev/null
+++ b/app/javascript/components/UserProfile/SetPasswordDialog.tsx
@@ -0,0 +1,41 @@
+import * as React from 'react';
+import I18n from 'i18n-js';
+
+import Button from '../common/Button';
+import buildRequestHeaders from '../../helpers/buildRequestHeaders';
+
+interface Props {
+  sendSetPasswordInstructionsEndpoint: string;
+  authenticityToken: string;
+}
+
+const SetPasswordDialog = ({ sendSetPasswordInstructionsEndpoint, authenticityToken }: Props) => {
+  
+
+  return (
+    <div style={{margin: 8, padding: 16, backgroundColor: '#cce5ff', borderRadius: 4}}>
+      <p>
+        { I18n.t('common.forms.auth.no_password_set') }
+      </p>
+
+      <Button
+        type='button'
+        onClick={async (event) => {
+          const res = await fetch(sendSetPasswordInstructionsEndpoint, {
+            method: 'GET',
+            headers: buildRequestHeaders(authenticityToken)
+          });
+
+          if (res.ok) {
+            alert(I18n.t('devise.passwords.send_instructions'));
+          } else {
+            alert(I18n.t('common.errors.unknown'));
+          }
+        }}>
+        { I18n.t('common.forms.auth.set_password') }
+      </Button>
+    </div>
+  )
+};
+
+export default SetPasswordDialog;
diff --git a/app/javascript/components/common/Button.tsx b/app/javascript/components/common/Button.tsx
index f9ce62be3..f98caf702 100644
--- a/app/javascript/components/common/Button.tsx
+++ b/app/javascript/components/common/Button.tsx
@@ -3,14 +3,16 @@ import * as React from 'react';
 interface Props {
   children: any;
   onClick(e: React.FormEvent): void;
+  type?: 'button' | 'submit';
   className?: string;
   outline?: boolean;
   disabled?: boolean;
 }
 
-const Button = ({ children, onClick, className = '', outline = false, disabled = false}: Props) => (
+const Button = ({ children, onClick, type="submit", className = '', outline = false, disabled = false}: Props) => (
   <button
     onClick={onClick}
+    type={type}
     className={`${className} btn${outline ? 'Outline' : ''}Primary`}
     disabled={disabled}
   >
diff --git a/app/views/devise/registrations/edit.html.erb b/app/views/devise/registrations/edit.html.erb
index 79919931d..dbe039683 100644
--- a/app/views/devise/registrations/edit.html.erb
+++ b/app/views/devise/registrations/edit.html.erb
@@ -3,6 +3,20 @@
 
   <%= render "devise/shared/error_messages", resource: resource %>
 
+  <% if not current_user.has_set_password? %>
+    <%=
+      react_component(
+        'UserProfile/SetPasswordDialog',
+        {
+          sendSetPasswordInstructionsEndpoint: send_set_password_instructions_path(email: current_user.email),
+          authenticityToken: form_authenticity_token
+        },
+      )
+    %>
+
+    <fieldset disabled="disabled">
+  <% end %>
+
   <div class="form-group">
     <%= f.label :full_name, t('common.forms.auth.full_name') %>
     <%= f.text_field :full_name, autocomplete: "full-name", class: "form-control" %>
@@ -42,9 +56,11 @@
     <%= f.password_field :password_confirmation, autocomplete: "new-password", class: "form-control" %>
   </div>
 
+  <hr />
+
   <div class="form-group">
     <%= f.label :current_password, t('common.forms.auth.current_password') %>
-    <%= f.password_field :current_password, autocomplete: "current-password", class: "form-control" %>
+    <%= f.password_field :current_password, autocomplete: "current-password", required: true, class: "form-control" %>
     <small id="currentPasswordHelp" class="form-text text-muted">
       <%= t('common.forms.auth.current_password_required_help') %>
     </small>
@@ -53,6 +69,10 @@
   <div class="actions">
     <%= f.submit t('common.forms.auth.update_profile'), class: "btnPrimary btn-block" %>
   </div>
+
+  <% if not current_user.has_set_password? %>
+    </fieldset>
+  <% end %>
 <% end %>
 
 <br />
diff --git a/app/workflows/o_auth_sign_in_user_workflow.rb b/app/workflows/o_auth_sign_in_user_workflow.rb
index 80cfc7f81..9e09a9de7 100644
--- a/app/workflows/o_auth_sign_in_user_workflow.rb
+++ b/app/workflows/o_auth_sign_in_user_workflow.rb
@@ -44,6 +44,7 @@ def run
           email: email,
           full_name: full_name,
           password: Devise.friendly_token,
+          has_set_password: false,
           status: 'active'
         )
         user.skip_confirmation
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 84e95d3a4..30654568b 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1,5 +1,7 @@
 en:
   common:
+    errors:
+      unknown: 'An unknown error occurred, please try again'
     validations:
       required: '%{attribute} is required'
       email: 'Invalid email'
@@ -18,6 +20,8 @@ en:
         notifications_enabled: 'Notifications enabled'
         notifications_enabled_help: "if disabled, you won't receive any notification"
         waiting_confirmation: 'Currently waiting confirmation for %{email}'
+        no_password_set: 'You must set a password to update your profile'
+        set_password: 'Set password'
         password_leave_blank_help: "leave blank if you don't want to change your password"
         current_password_required_help: 'we need your current password to confirm your changes'
         remember_me: 'Remember me'
@@ -92,7 +96,6 @@ en:
       description: 'Description (optional)'
       no_title: 'Title field is mandatory'
       submit_success: 'Feedback published! You will be redirected soon...'
-      submit_error: 'An unknown error occurred, try again'
     search_box:
       title: 'Search'
     filter_box:
diff --git a/config/routes.rb b/config/routes.rb
index de12ad7fc..6d0f2dfbe 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -31,8 +31,13 @@
     
     devise_for :users, :controllers => {
       :registrations => 'registrations',
-      :sessions => 'sessions'
+      :sessions => 'sessions',
+      :passwords => 'passwords'
     }
+
+    devise_scope :user do
+      get '/users/send_set_password_instructions', to: 'registrations#send_set_password_instructions', as: :send_set_password_instructions
+    end
     
     resources :tenants, only: [:show, :update]
     resources :users, only: [:index, :update]
diff --git a/db/migrate/20240514112836_add_has_set_password_to_users.rb b/db/migrate/20240514112836_add_has_set_password_to_users.rb
new file mode 100644
index 000000000..aeecd4c70
--- /dev/null
+++ b/db/migrate/20240514112836_add_has_set_password_to_users.rb
@@ -0,0 +1,5 @@
+class AddHasSetPasswordToUsers < ActiveRecord::Migration[6.1]
+  def change
+    add_column :users, :has_set_password, :boolean, default: true, null: false
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5503ff6cf..a838d2011 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2024_04_27_140300) do
+ActiveRecord::Schema.define(version: 2024_05_14_112836) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -198,6 +198,7 @@
     t.integer "status"
     t.bigint "tenant_id", null: false
     t.string "oauth_token"
+    t.boolean "has_set_password", default: true, null: false
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
     t.index ["email", "tenant_id"], name: "index_users_on_email_and_tenant_id", unique: true
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
diff --git a/spec/system/user_edit_profile_spec.rb b/spec/system/user_edit_profile_spec.rb
index 3131da395..4e38f1791 100644
--- a/spec/system/user_edit_profile_spec.rb
+++ b/spec/system/user_edit_profile_spec.rb
@@ -93,12 +93,11 @@
     full_name = user.full_name
 
     visit edit_user_registration_path
-    fill_in 'Full name', with: 'New fantastic full name'
+    fill_in 'Full name', with: user.full_name + ' New'
     # do not fill current password textbox
     click_button 'Update profile'
+    visit edit_user_registration_path
 
-    expect(page).to have_css('#error_explanation')
-    expect(page).to have_css('.field_with_errors')
     expect(user.reload.full_name).to eq(full_name)
   end