Merge pull request #2764 from CartoDB/2650-fix-new-account-page-chang…

…e-passwd

Fix changing email requires new password

NEWS.md

@@ -3,6 +3,7 @@
 
 Bugfixes:
 * Fix "create table from query or clear view" banner covers zoom overlay and search box [#2762](https://github.com/CartoDB/cartodb/pull/2762)
+* Fix Changing email requires new password [#2764](https://github.com/CartoDB/cartodb/pull/2764)
 
 3.9.0 (2015-02-13)
 ------------------

app/assets/stylesheets/new_common/form-content.css.scss

@@ -292,6 +292,9 @@
   font-size: $sFontSize-normal;
   color: $cTypography-help;
 }
+.Form-rowInfoText--multipleLines {
+  line-height: 20px;
+}
 .Form-rowInfoText--error { color: #DD3B37 }
 .Form-footer {
   @include display-flex();

app/controllers/admin/users_controller.rb

@@ -36,9 +36,18 @@ def account
 
   def account_update
     attributes = params[:user]
-    @user.set_fields(attributes, [:email]) if attributes[:email].present?
-    @user.change_password(attributes[:old_password].presence, attributes[:new_password].presence,
-                                 attributes[:confirm_password].presence)
+    if attributes[:new_password].present? || attributes[:confirm_password].present?
+      @user.change_password(
+        attributes[:old_password].presence,
+        attributes[:new_password].presence,
+        attributes[:confirm_password].presence
+      )
+    end
+
+    if @user.can_change_email && attributes[:email].present?
+      @user.set_fields(attributes, [:email])
+    end
+
     @user.update_in_central
     @user.save(raise_on_failure: true)
 

app/models/user.rb

@@ -391,19 +391,26 @@ def change_password(old_password, new_password_value, new_password_confirmation_
 
     return unless new_password_value == new_password_confirmation_value && !new_password_value.nil?
 
+    # Must be set AFTER validations
+    set_last_password_change_date
+
     self.password = new_password_value
   end
 
   def validate_old_password(old_password)
-    self.class.password_digest(old_password, self.salt) == self.crypted_password
+    (self.class.password_digest(old_password, self.salt) == self.crypted_password) || (google_sign_in && last_password_change_date.nil?)
+  end
+
+  def should_display_old_password?
+    google_sign_in.nil? || !google_sign_in || !last_password_change_date.nil?
   end
 
   def password_confirmation
     @password_confirmation
   end
 
   def password_confirmation=(password_confirmation)
-    self.last_password_change_date = Time.zone.now unless new?
+    set_last_password_change_date
     @password_confirmation = password_confirmation
   end
 
@@ -1068,7 +1075,7 @@ def ghost_tables_work(job)
 
   def link_ghost_tables_working
     # search in the first 100. This is random number
-    enqeued = Resque.peek(:users, 0, 100).select { |job| 
+    enqeued = Resque.peek(:users, 0, 100).select { |job|
       job && job['class'] === 'Resque::UserJobs::SyncTables::LinkGhostTables' && !job['args'].nil? && job['args'].length == 1 && job['args'][0] === self.id
     }.length
     workers = Resque::Worker.all
@@ -2274,4 +2281,8 @@ def secure_digest(*args)
     Digest::SHA256.hexdigest(args.flatten.join)
   end
 
+  def set_last_password_change_date
+    self.last_password_change_date = Time.zone.now unless new?
+  end
+
 end

app/views/admin/users/account.html.erb

@@ -46,30 +46,32 @@
             <label class="Form-label">Email</label>
           </div>
           <div class="Form-rowData">
-            <%= f.text_field :email, :class => "Form-input Form-input--long #{ 'Form-input--error' if @user.errors[:email].present? }" %>
+            <%= f.text_field :email, class: "Form-input Form-input--long #{ 'Form-input--error' if @user.errors[:email].present? } #{ 'is-disabled' unless @user.can_change_email }", readonly: !@user.can_change_email %>
           </div>
           <div class="Form-rowInfo">
-            <% if (@user.errors[:email].present?) %>
+            <% if @user.errors[:email].present? %>
               <p class="Form-rowInfoText Form-rowInfoText--error">Email not valid</p>
-            <% else %>
-              <p class="Form-rowInfoText">You enter through <%= link_to 'here', @user.public_url %></p>
+            <% elsif @user.google_sign_in %>
+              <p class="Form-rowInfoText <%= 'Form-rowInfoText--multipleLines' unless @user.should_display_old_password? %>">Your account is linked to your Google account. <% unless @user.should_display_old_password? %> You can change the email if you set a password.</p> <% end %>
             <% end %>
           </div>
         </div>
 
-        <div class="Form-row">
-          <div class="Form-rowLabel">
-            <label class="Form-label">Old password</label>
-          </div>
-          <div class="Form-rowData">
-            <%= f.password_field :old_password, :class => "Form-input Form-input--long #{ 'Form-input--error' if @user.errors[:old_password].present? }" %>
+        <% if @user.should_display_old_password? %>
+          <div class="Form-row">
+            <div class="Form-rowLabel">
+              <label class="Form-label">Old password</label>
+            </div>
+            <div class="Form-rowData">
+              <%= f.password_field :old_password, :class => "Form-input Form-input--long #{ 'Form-input--error' if @user.errors[:old_password].present? }" %>
+            </div>
+            <div class="Form-rowInfo">
+              <% if (@user.errors[:old_password].present?) %>
+                <p class="Form-rowInfoText Form-rowInfoText--error"><%= @user.errors[:old_password].first%></p>
+              <% end %>
+            </div>
           </div>
-          <div class="Form-rowInfo">
-            <% if (@user.errors[:old_password].present?) %>
-              <p class="Form-rowInfoText Form-rowInfoText--error"><%= @user.errors[:old_password].first%></p>
-            <% end %>
-          </div>
-        </div>
+        <% end %>
 
         <div class="Form-row">
           <div class="Form-rowLabel">

spec/models/user_spec.rb

@@ -1366,6 +1366,27 @@ def stub_and_check_version_pre_mu(version, is_pre_mu)
     @user.crypted_password.should eq old_crypted_password
   end
 
+  describe "when user is signed up with google sign-in and don't have any password yet" do
+    before(:each) do
+      @user.google_sign_in = true
+      @user.last_password_change_date = nil
+      @user.save
+
+      new_valid_password = '123456'
+      @user.change_password("doesn't matter in this case", new_valid_password, new_valid_password)
+    end
+
+    it 'should allow updating password w/o a current password' do
+      @user.valid?.should eq true
+      @user.save
+    end
+
+    it 'should have updated last password change date' do
+      @user.last_password_change_date.should_not eq nil
+      @user.save
+    end
+  end
+
   def create_org(org_name, org_quota, org_seats)
     organization = Organization.new
     organization.name = org_name
@@ -1376,4 +1397,3 @@ def create_org(org_name, org_quota, org_seats)
   end
 
 end
-