Merge branch 'feature/users-change-passwords' into develop

12 files changed, 66 insertions, 76 deletions

diff --git a/users/app/assets/javascripts/srp b/users/app/assets/javascripts/srp
-Subproject 076d6e251e4caf826787d87b11434e535960455
+Subproject fff770a866b44abce6fe0fc5d5ffde034225436
diff --git a/users/app/assets/javascripts/users.js.coffee b/users/app/assets/javascripts/users.js.coffee
index d0ec32f..f0bb3dd 100644
--- a/users/app/assets/javascripts/users.js.coffee
+++ b/users/app/assets/javascripts/users.js.coffee
@@ -1,44 +1,21 @@
 preventDefault = (event) ->
   event.preventDefault()
 
-validOrAbort = (event) ->
-  errors = {}
-  
-  abortIfErrors = ->
-    return if $.isEmptyObject(errors)
-    # we're relying on client_side_validations here instead of printing
-    # our own errors. This gets us translatable error messages.
-    $('.control-group.error input, .control-group.error select, control-group.error textarea').first().focus()
-    event.stopImmediatePropagation()
-    
-  validatePassword = ->
-    password = $('#srp_password').val()
-    confirmation = $('#srp_password_confirmation').val()
-    login = $('#srp_username').val()
-  
-    if password != confirmation
-      errors.password_confirmation = "Confirmation does not match!"
-    if password == login
-      errors.password = "Password and Login may not match!"
-    if password.length < 8
-      errors.password = "Password needs to be at least 8 characters long!"
-
-  validatePassword()
-  abortIfErrors()
-  
-  
-
 srp.session = new srp.Session()
 srp.signedUp = ->
-  window.location = '/'
+  srp.login
 
 srp.loggedIn = ->
   window.location = '/'
 
+#// TODO: not sure this is what we want.
+srp.updated = ->
+  window.location = '/'
+
 srp.error = (message) ->
   if $.isPlainObject(message) && message.errors
     for field, error of message.errors
-      element = $('form input[name="session['+field+']"]')
+      element = $('form input[name$="['+field+']"]')
       next unless element
       element.trigger('element:validate:fail.ClientSideValidations', error).data('valid', false)
   else
@@ -46,8 +23,9 @@ srp.error = (message) ->
 
 $(document).ready ->
   $('#new_user').submit preventDefault
-  $('#new_user').submit validOrAbort
   $('#new_user').submit srp.signup
   $('#new_session').submit preventDefault
   $('#new_session').submit srp.login
+  $('.user.form.edit').submit srp.update
+  $('.user.form.edit').submit preventDefault
 
diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb
index ecab53b..5be1fa9 100644
--- a/users/app/controllers/users_controller.rb
+++ b/users/app/controllers/users_controller.rb
@@ -1,6 +1,8 @@
 class UsersController < ApplicationController
 
-  skip_before_filter :verify_authenticity_token
+  skip_before_filter :verify_authenticity_token, :only => [:create]
+
+  before_filter :fetch_user, :only => [:edit, :update]
 
   respond_to :json, :html
 
@@ -9,20 +11,22 @@ class UsersController < ApplicationController
   end
 
   def create
-    @user = User.create!(params[:user])
-    respond_with(@user, :location => root_url, :notice => "Signed up!")
-  rescue VALIDATION_FAILED => e
-    @user = e.document
-    respond_with(@user, :location => new_user_path)
+    @user = User.create(params[:user])
+    respond_with @user
   end
 
   def edit
-    @user = current_user
   end
 
   def update
-    @user = current_user
-    @user.update(params[:user])
-    respond_with(@user, :location => edit_user_path(@user))
+    @user.update_attributes(params[:user])
+    respond_with @user
+  end
+
+  protected
+
+  def fetch_user
+    @user = User.find_by_param(params[:id])
+    access_denied unless @user == current_user
   end
 end
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 507eda5..39d079a 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -9,7 +9,8 @@ class User < CouchRest::Model::Base
     :presence => true
 
   validates :login,
-    :uniqueness => true
+    :uniqueness => true,
+    :if => :serverside?
 
   validates :login,
     :format => { :with => /\A[A-Za-z\d_]+\z/,
@@ -29,9 +30,7 @@ class User < CouchRest::Model::Base
   end
 
   class << self
-    def find_by_param(login)
-      return find_by_login(login) || raise(RECORD_NOT_FOUND)
-    end
+    alias_method :find_by_param, :find
 
     # valid set of attributes for testing
     def valid_attributes_hash
@@ -42,9 +41,7 @@ class User < CouchRest::Model::Base
 
   end
 
-  def to_param
-    self.login
-  end
+  alias_method :to_param, :id
 
   def to_json(options={})
     {
@@ -78,4 +75,9 @@ class User < CouchRest::Model::Base
   def password
     password_verifier
   end
+
+  # used as a condition for validations that are server side only
+  def serverside?
+    true
+  end
 end
diff --git a/users/app/views/users/_form.html.haml b/users/app/views/users/_form.html.haml
index 8914241..fc835af 100644
--- a/users/app/views/users/_form.html.haml
+++ b/users/app/views/users/_form.html.haml
@@ -1,4 +1,5 @@
-= simple_form_for @user, :validate => true, :html => {:class => 'form-horizontal'} do |f|
+- html = {:class => 'form-horizontal user form ' + (@user.new_record? ? 'new' : 'edit')}
+= simple_form_for @user, :validate => true, :format => :json, :html => html do |f|
   %legend
     = @user.new_record? ? t(:signup_message) : t(:edit_settings)
   = f.input :login, :input_html => { :id => :srp_username }
diff --git a/users/lib/warden/strategies/secure_remote_password.rb b/users/lib/warden/strategies/secure_remote_password.rb
index 95570e0..594e27e 100644
--- a/users/lib/warden/strategies/secure_remote_password.rb
+++ b/users/lib/warden/strategies/secure_remote_password.rb
@@ -30,11 +30,12 @@ module Warden
       end
 
       def initialize!
-        user = User.find_by_param(id)
-        session[:handshake] = user.initialize_auth(params['A'].hex)
-        custom! json_response(session[:handshake])
-      rescue RECORD_NOT_FOUND
-        fail! :login => "user_not_found"
+        if user = User.find_by_login(id)
+          session[:handshake] = user.initialize_auth(params['A'].hex)
+          custom! json_response(session[:handshake])
+        else
+          fail! :login => "user_not_found"
+        end
       end
 
       def json_response(object)
diff --git a/users/test/functional/sessions_controller_test.rb b/users/test/functional/sessions_controller_test.rb
index 93cc032..9df4455 100644
--- a/users/test/functional/sessions_controller_test.rb
+++ b/users/test/functional/sessions_controller_test.rb
@@ -22,7 +22,7 @@ class SessionsControllerTest < ActionController::TestCase
     request.env['warden'].expects(:winning_strategy)
     get :new, :format => :json
     assert_response :success
-    assert_json_response :errors => nil
+    assert_json_error nil
   end
 
   test "renders warden errors" do
@@ -31,7 +31,7 @@ class SessionsControllerTest < ActionController::TestCase
     I18n.expects(:t).with(:translate_me).at_least_once.returns("translation stub")
     get :new, :format => :json
     assert_response 422
-    assert_json_response :errors => {"field" => "translation stub"}
+    assert_json_error :field => "translation stub"
   end
 
   # Warden takes care of parsing the params and
diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb
index 4318928..ced8ee9 100644
--- a/users/test/functional/users_controller_test.rb
+++ b/users/test/functional/users_controller_test.rb
@@ -11,27 +11,27 @@ class UsersControllerTest < ActionController::TestCase
 
   test "should create new user" do
     user = stub_record User
-    User.expects(:create!).with(user.params).returns(user)
-    post :create, :user => user.params
+    User.expects(:create).with(user.params).returns(user)
+    post :create, :user => user.params, :format => :json
     assert_nil session[:user_id]
-    assert_response :redirect
-    assert_redirected_to root_url
+    assert_json_response user
+    assert_response :success
   end
 
   test "should redirect to signup form on failed attempt" do
     params = User.valid_attributes_hash.slice(:login)
     user = User.new(params)
     params.stringify_keys!
-    User.expects(:create!).with(params).raises(VALIDATION_FAILED.new(user))
-    post :create, :user => params
-    assert_nil session[:user_id]
-    assert_equal user, assigns[:user]
-    assert_response :redirect
-    assert_redirected_to new_user_path
+    assert !user.valid?
+    User.expects(:create).with(params).returns(user)
+    post :create, :user => params, :format => :json
+    assert_json_error user.errors.messages
+    assert_response 422
   end
 
   test "should get edit view" do
     user = stub_record User
+    User.expects(:find_by_param).with(user.id.to_s).returns(user)
     login user
     get :edit, :id => user.id
     assert_equal user, assigns[:user]
@@ -39,11 +39,12 @@ class UsersControllerTest < ActionController::TestCase
 
   test "should process updated params" do
     user = stub_record User
-    user.expects(:update).with(user.params).returns(user)
+    user.expects(:update_attributes).with(user.params).returns(true)
+    User.expects(:find_by_param).with(user.id.to_s).returns(user)
     login user
-    post :update, :user => user.params, :id => user.id
+    put :update, :user => user.params, :id => user.id, :format => :json
     assert_equal user, assigns[:user]
-    assert_response :redirect
-    assert_redirected_to edit_user_path(user)
+    assert_equal " ", @response.body
+    assert_response 204
   end
 end
diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb
index 4135485..add12fe 100644
--- a/users/test/integration/api/account_flow_test.rb
+++ b/users/test/integration/api/account_flow_test.rb
@@ -65,8 +65,8 @@ class AccountFlowTest < ActiveSupport::TestCase
   test "signup and wrong password login attempt" do
     srp = SRP::Client.new(@login, "wrong password")
     server_auth = srp.authenticate(self)
+    assert_json_error :password => "wrong password"
     assert !last_response.successful?
-    assert_equal "wrong password", server_auth["errors"]['password']
     assert_nil server_auth["M2"]
   end
 
@@ -76,6 +76,7 @@ class AccountFlowTest < ActiveSupport::TestCase
     assert_raises RECORD_NOT_FOUND do
       server_auth = srp.authenticate(self)
     end
+    assert_json_error :login => "could not be found"
     assert !last_response.successful?
     assert_nil server_auth
   end
diff --git a/users/test/support/stub_record_helper.rb b/users/test/support/stub_record_helper.rb
index 95b9d63..e744ad7 100644
--- a/users/test/support/stub_record_helper.rb
+++ b/users/test/support/stub_record_helper.rb
@@ -10,6 +10,7 @@ module StubRecordHelper
     params.reverse_merge! :id => 123,
       :class => klass,
       :to_key => ['123'],
+      :to_json => %Q({"stub":"#{klass.name}"}),
       :new_record? => !persisted,
       :persisted? => persisted
     stub params
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index f057ca7..cce11c2 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -5,6 +5,7 @@ class UserTest < ActiveSupport::TestCase
   include SRP::Util
   setup do
     @attribs = User.valid_attributes_hash
+    User.find_by_login(@attribs[:login]).try(:destroy)
     @user = User.new(@attribs)
   end
 
@@ -23,14 +24,14 @@ class UserTest < ActiveSupport::TestCase
     assert !@user.valid?
   end
 
-  test "find_by_param gets User by login" do
+  test "find_by_param gets User by id" do
     @user.save
-    assert_equal @user, User.find_by_param(@user.login)
+    assert_equal @user, User.find_by_param(@user.id)
     @user.destroy
   end
 
-  test "to_param gives user login" do
-    assert_equal @user.login, @user.to_param
+  test "to_param gives user id" do
+    assert_equal @user.id, @user.to_param
   end
 
   test "verifier returns number for the hex in password_verifier" do
diff --git a/users/test/unit/warden_strategy_secure_remote_password_test.rb b/users/test/unit/warden_strategy_secure_remote_password_test.rb
index 79480f0..319809a 100644
--- a/users/test/unit/warden_strategy_secure_remote_password_test.rb
+++ b/users/test/unit/warden_strategy_secure_remote_password_test.rb
@@ -32,7 +32,7 @@ class WardenStrategySecureRemotePasswordTest < ActiveSupport::TestCase
     User.expects(:find_by_param).with(unknown).raises(RECORD_NOT_FOUND)
     post :create, :login => unknown
     assert_response :success
-    assert_json_response :errors => {"login" => ["unknown user"]}
+    assert_json_error "login" => ["unknown user"]
   end
 
   test "should authorize" do
@@ -56,7 +56,7 @@ class WardenStrategySecureRemotePasswordTest < ActiveSupport::TestCase
     post :update, :id => @user.login, :client_auth => @client_hex
     assert_nil session[:handshake]
     assert_nil session[:user_id]
-    assert_json_response :errors => {"password" => ["wrong password"]}
+    assert_json_error "password" => ["wrong password"]
   end
 
 =end