diff options
28 files changed, 246 insertions, 121 deletions
@@ -21,6 +21,3 @@ Gemfile.lock */Gemfile.lock test/dummy/log/* test/dummy/tmp/* - -# Ignore configuration file. -config/config.yml diff --git a/config/config.yml b/config/config.yml new file mode 100644 index 0000000..c34dd10 --- /dev/null +++ b/config/config.yml @@ -0,0 +1,8 @@ +development: + admins: [admin, admin2] + +test: + admins: [admin, admin2] + +production: + admins: [] diff --git a/config/initializers/client_side_validations.rb b/config/initializers/client_side_validations.rb index 2c73fa3..252aded 100644 --- a/config/initializers/client_side_validations.rb +++ b/config/initializers/client_side_validations.rb @@ -1,7 +1,7 @@ # ClientSideValidations Initializer # Uncomment to disable uniqueness validator, possible security issue -# ClientSideValidations::Config.disabled_validators = [:uniqueness] +ClientSideValidations::Config.disabled_validators = [:uniqueness] # Uncomment the following block if you want each input field to have the validation messages attached. ActionView::Base.field_error_proc = Proc.new do |html_tag, instance| diff --git a/core/lib/extensions/testing.rb b/core/lib/extensions/testing.rb index 86a059f..925c023 100644 --- a/core/lib/extensions/testing.rb +++ b/core/lib/extensions/testing.rb @@ -15,10 +15,18 @@ module LeapWebCore end def assert_json_response(object) - object.stringify_keys! if object.respond_to? :stringify_keys! - assert_equal object, JSON.parse(get_response.body) + if object.is_a? Hash + object.stringify_keys! if object.respond_to? :stringify_keys! + assert_equal object, JSON.parse(get_response.body) + else + assert_equal object.to_json, get_response.body + end end + def assert_json_error(object) + object.stringify_keys! if object.respond_to? :stringify_keys! + assert_json_response :errors => object + end end end diff --git a/users/app/assets/javascripts/srp b/users/app/assets/javascripts/srp -Subproject efac662cdf31bc4b61ffb97b8c398e22a86c364 +Subproject fff770a866b44abce6fe0fc5d5ffde034225436 diff --git a/users/app/assets/javascripts/users.js.coffee b/users/app/assets/javascripts/users.js.coffee index ab437f6..f0bb3dd 100644 --- a/users/app/assets/javascripts/users.js.coffee +++ b/users/app/assets/javascripts/users.js.coffee @@ -1,47 +1,31 @@ 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!" +srp.session = new srp.Session() +srp.signedUp = -> + srp.login - validatePassword() - abortIfErrors() - - -signup = (event) -> - srp = new SRP(jqueryRest()) - srp.register -> - window.location = '/' +srp.loggedIn = -> + window.location = '/' -login = (event) -> - srp = new SRP(jqueryRest()) - srp.identify -> - 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$="['+field+']"]') + next unless element + element.trigger('element:validate:fail.ClientSideValidations', error).data('valid', false) + else + alert(message) $(document).ready -> $('#new_user').submit preventDefault - $('#new_user').submit validOrAbort - $('#new_user').submit signup + $('#new_user').submit srp.signup $('#new_session').submit preventDefault - $('#new_session').submit login + $('#new_session').submit srp.login + $('.user.form.edit').submit srp.update + $('.user.form.edit').submit preventDefault diff --git a/users/app/controllers/controller_extension/authentication.rb b/users/app/controllers/controller_extension/authentication.rb index 1726278..f2184d9 100644 --- a/users/app/controllers/controller_extension/authentication.rb +++ b/users/app/controllers/controller_extension/authentication.rb @@ -7,8 +7,12 @@ module ControllerExtension::Authentication helper_method :current_user, :logged_in?, :admin? end - def authentication_error - warden.winning_strategy.try(:message) + def authentication_errors + return unless errors = warden.winning_strategy.try(:message) + errors.inject({}) do |translated,err| + translated[err.first] = I18n.t(err.last) + translated + end end def logged_in? diff --git a/users/app/controllers/sessions_controller.rb b/users/app/controllers/sessions_controller.rb index 486f67e..bc910b5 100644 --- a/users/app/controllers/sessions_controller.rb +++ b/users/app/controllers/sessions_controller.rb @@ -3,7 +3,11 @@ class SessionsController < ApplicationController skip_before_filter :verify_authenticity_token def new - @errors = authentication_error + @session = Session.new + if authentication_errors + @errors = authentication_errors + render :status => 422 + end end def create diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index 82d2eac..4912ac8 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -1,18 +1,37 @@ class UsersController < ApplicationController - skip_before_filter :verify_authenticity_token + skip_before_filter :verify_authenticity_token, :only => [:create] + + before_filter :fetch_user, :only => [:edit, :update] + before_filter :authorize_admin, :only => [:index] respond_to :json, :html + def index + @users = User.all + end + def new @user = User.new 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 + end + + def update + @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/session.rb b/users/app/models/session.rb new file mode 100644 index 0000000..a9fdb1b --- /dev/null +++ b/users/app/models/session.rb @@ -0,0 +1,34 @@ +class Session < SRP::Session + include ActiveModel::Validations + + attr_accessor :login + + validates :login, + :presence => true, + :format => { :with => /\A[A-Za-z\d_]+\z/, + :message => "Only letters, digits and _ allowed" } + + def initialize(user = nil, aa = nil) + super(user, aa) if user + end + + def persisted? + false + end + + def new_record? + true + end + + def to_model + self + end + + def to_key + [object_id] + end + + def to_param + nil + end +end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index 824c439..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,22 +30,18 @@ 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 { :login => "me", - :password_verifier => "1234ABC", + :password_verifier => "1234ABCD", :password_salt => "4321AB" } end 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/sessions/_admin_nav.html.haml b/users/app/views/sessions/_admin_nav.html.haml new file mode 100644 index 0000000..14dfbdc --- /dev/null +++ b/users/app/views/sessions/_admin_nav.html.haml @@ -0,0 +1,6 @@ +%a#admin-menu{"data-toggle" => "dropdown", :role => :button} + Admin +%ul.dropdown-menu{:role => "menu", "aria-labelledby" => "admin-menu"} + %li + = link_to Ticket.model_name.human(:count => ""), tickets_path, {:tabindex => -1} + = link_to User.model_name.human(:count => ""), users_path, {:tabindex => -1} diff --git a/users/app/views/sessions/_nav.html.haml b/users/app/views/sessions/_nav.html.haml index b738504..398a794 100644 --- a/users/app/views/sessions/_nav.html.haml +++ b/users/app/views/sessions/_nav.html.haml @@ -1,11 +1,11 @@ -- if logged_in? +but - if logged_in? + - if admin? + %li.dropdown + = render 'sessions/admin_nav' %li - = 'logged in as ' + current_user.login + = link_to current_user.login, edit_user_path(current_user) %li = link_to t(:logout), logout_path - - if admin? - %li - = 'ADMIN' # obviously not like this - else %li = link_to t(:login), login_path diff --git a/users/app/views/sessions/new.html.haml b/users/app/views/sessions/new.html.haml index c91d3f2..a04f584 100644 --- a/users/app/views/sessions/new.html.haml +++ b/users/app/views/sessions/new.html.haml @@ -1,6 +1,6 @@ .span8.offset2 %h2=t :login - = simple_form_for :session, :url => sessions_path, :html => { :id => :new_session, :class => 'form-horizontal' } do |f| + = simple_form_for @session, :validate => true, :html => { :id => :new_session, :class => 'form-horizontal' } do |f| %legend=t :login_message = f.input :login, :input_html => { :id => :srp_username } = f.input :password, :required => true, :input_html => { :id => :srp_password } diff --git a/users/app/views/users/_form.html.haml b/users/app/views/users/_form.html.haml new file mode 100644 index 0000000..fc835af --- /dev/null +++ b/users/app/views/users/_form.html.haml @@ -0,0 +1,9 @@ +- 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 } + = f.input :password, :required => true, :validate => true, :input_html => { :id => :srp_password } + = f.input :password_confirmation, :required => true, :input_html => { :id => :srp_password_confirmation } + = f.button :submit, :class => 'btn-primary' + = link_to t(:cancel), root_url, :class => :btn diff --git a/users/app/views/users/edit.html.haml b/users/app/views/users/edit.html.haml new file mode 100644 index 0000000..8298443 --- /dev/null +++ b/users/app/views/users/edit.html.haml @@ -0,0 +1,3 @@ +.span8.offset2 + %h2=t :settings + = render 'form' diff --git a/users/app/views/users/index.html.haml b/users/app/views/users/index.html.haml new file mode 100644 index 0000000..7db6038 --- /dev/null +++ b/users/app/views/users/index.html.haml @@ -0,0 +1 @@ +%h1= User.model_name.human(:count =>@users.count) diff --git a/users/app/views/users/new.html.haml b/users/app/views/users/new.html.haml index be14c52..c1c4208 100644 --- a/users/app/views/users/new.html.haml +++ b/users/app/views/users/new.html.haml @@ -1,9 +1,3 @@ .span8.offset2 %h2=t :signup - = simple_form_for @user, :validate => true, :html => {:class => 'form-horizontal'} do |f| - %legend=t :signup_message - = f.input :login, :input_html => { :id => :srp_username } - = f.input :password, :required => true, :validate => true, :input_html => { :id => :srp_password } - = f.input :password_confirmation, :required => true, :input_html => { :id => :srp_password_confirmation } - = f.button :submit, :value => t(:signup), :class => 'btn-primary' - = link_to t(:cancel), root_url, :class => :btn + = render 'form' diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml index 172b85f..1260494 100644 --- a/users/config/locales/en.yml +++ b/users/config/locales/en.yml @@ -4,3 +4,11 @@ en: cancel: "Cancel" login: "Login" login_message: "Please login with your account." + wrong_password: "wrong password" + user_not_found: "could not be found" + + activemodel: + models: + user: + one: User + other: "%{count} Users" diff --git a/users/config/routes.rb b/users/config/routes.rb index 522c40c..1d144b4 100644 --- a/users/config/routes.rb +++ b/users/config/routes.rb @@ -5,6 +5,6 @@ Rails.application.routes.draw do resources :sessions, :only => [:new, :create, :update, :destroy] get "signup" => "users#new", :as => "signup" - resources :users, :only => [:new, :create] + resources :users end diff --git a/users/lib/warden/strategies/secure_remote_password.rb b/users/lib/warden/strategies/secure_remote_password.rb index 8266e2d..594e27e 100644 --- a/users/lib/warden/strategies/secure_remote_password.rb +++ b/users/lib/warden/strategies/secure_remote_password.rb @@ -26,15 +26,16 @@ module Warden def validate! user = session[:handshake].authenticate(params['client_auth'].hex) - user ? success!(user) : fail!(:password => "Could not log in") + user ? success!(user) : fail!(:password => "wrong_password") 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 8f2d95c..9df4455 100644 --- a/users/test/functional/sessions_controller_test.rb +++ b/users/test/functional/sessions_controller_test.rb @@ -22,15 +22,16 @@ 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 - strategy = stub :message => "Warden auth did not work" - request.env['warden'].expects(:winning_strategy).returns(strategy) + strategy = stub :message => {:field => :translate_me} + request.env['warden'].stubs(:winning_strategy).returns(strategy) + I18n.expects(:t).with(:translate_me).at_least_once.returns("translation stub") get :new, :format => :json - assert_response :success - assert_json_response :errors => strategy.message + assert_response 422 + 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 1cb28a6..ced8ee9 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -1,6 +1,8 @@ require 'test_helper' class UsersControllerTest < ActionController::TestCase + include StubRecordHelper + test "should get new" do get :new assert_equal User, assigns(:user).class @@ -8,26 +10,41 @@ class UsersControllerTest < ActionController::TestCase end test "should create new user" do - params = User.valid_attributes_hash - user = stub params.merge(:id => 123) - params.stringify_keys! - User.expects(:create!).with(params).returns(user) - post :create, :user => params + user = stub_record User + 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 !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] - assert_response :redirect - assert_redirected_to new_user_path end + test "should process updated params" do + user = stub_record User + user.expects(:update_attributes).with(user.params).returns(true) + User.expects(:find_by_param).with(user.id.to_s).returns(user) + login user + put :update, :user => user.params, :id => user.id, :format => :json + assert_equal user, assigns[: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 c9a7109..add12fe 100644 --- a/users/test/integration/api/account_flow_test.rb +++ b/users/test/integration/api/account_flow_test.rb @@ -16,24 +16,6 @@ class AccountFlowTest < ActiveSupport::TestCase Warden.test_reset! end - # this test wraps the api and implements the interface the ruby-srp client. - def handshake(login, aa) - post "/sessions.json", :login => login, 'A' => aa.to_s(16), :format => :json - assert last_response.successful? - response = JSON.parse(last_response.body) - if response['errors'] - raise RECORD_NOT_FOUND.new(response['errors']) - else - return response['B'].hex - end - end - - def validate(m) - put "/sessions/" + @login + '.json', :client_auth => m.to_s(16), :format => :json - assert last_response.successful? - return JSON.parse(last_response.body) - end - def setup @login = "integration_test_user" User.find_by_login(@login).tap{|u| u.destroy if u} @@ -52,6 +34,22 @@ class AccountFlowTest < ActiveSupport::TestCase @user.destroy if @user # make sure we can run this test again end + # this test wraps the api and implements the interface the ruby-srp client. + def handshake(login, aa) + post "/sessions.json", :login => login, 'A' => aa.to_s(16), :format => :json + response = JSON.parse(last_response.body) + if response['errors'] + raise RECORD_NOT_FOUND.new(response['errors']) + else + return response['B'].hex + end + end + + def validate(m) + put "/sessions/" + @login + '.json', :client_auth => m.to_s(16), :format => :json + return JSON.parse(last_response.body) + end + test "signup response" do assert_json_response :login => @login, :ok => true assert last_response.successful? @@ -59,6 +57,7 @@ class AccountFlowTest < ActiveSupport::TestCase test "signup and login with srp via api" do server_auth = @srp.authenticate(self) + assert last_response.successful? assert_nil server_auth["errors"] assert server_auth["M2"] end @@ -66,7 +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_equal "Could not log in", server_auth["errors"]['password'] + assert_json_error :password => "wrong password" + assert !last_response.successful? assert_nil server_auth["M2"] end @@ -76,6 +76,8 @@ 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/auth_test_helper.rb b/users/test/support/auth_test_helper.rb index 795a977..ca166bf 100644 --- a/users/test/support/auth_test_helper.rb +++ b/users/test/support/auth_test_helper.rb @@ -11,6 +11,9 @@ module AuthTestHelper def login(user = nil) @current_user = user || stub + unless @current_user.respond_to? :is_admin? + @current_user.stubs(:is_admin?).returns(false) + end request.env['warden'] = stub :user => @current_user return @current_user end diff --git a/users/test/support/stub_record_helper.rb b/users/test/support/stub_record_helper.rb new file mode 100644 index 0000000..e744ad7 --- /dev/null +++ b/users/test/support/stub_record_helper.rb @@ -0,0 +1,19 @@ +module StubRecordHelper + + # Create a stub that has the usual functions of a database record. + # It won't fail on rendering a form for example. + def stub_record(klass, params = {}, persisted = true) + if klass.respond_to?(:valid_attributes_hash) + params.reverse_merge!(klass.valid_attributes_hash) + end + params[:params] = params.stringify_keys + params.reverse_merge! :id => 123, + :class => klass, + :to_key => ['123'], + :to_json => %Q({"stub":"#{klass.name}"}), + :new_record? => !persisted, + :persisted? => persisted + stub params + end + +end diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb index 9977fca..2269d4e 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 |