diff options
| -rw-r--r-- | Gemfile.lock | 4 | ||||
| -rw-r--r-- | app/controllers/api/users_controller.rb | 11 | ||||
| -rw-r--r-- | app/controllers/application_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/invite_codes_controller.rb | 4 | ||||
| -rw-r--r-- | app/models/account.rb | 78 | ||||
| -rw-r--r-- | app/models/invite_code.rb | 2 | ||||
| -rw-r--r-- | app/views/invite_codes/_invite_code.html.haml | 2 | ||||
| -rw-r--r-- | app/views/invite_codes/index.html.haml | 4 | ||||
| -rw-r--r-- | lib/leap_web/version.rb | 2 | ||||
| -rw-r--r-- | test/functional/api/users_controller_test.rb | 22 | ||||
| -rw-r--r-- | test/integration/api/srp_test.rb | 2 | ||||
| -rw-r--r-- | test/integration/api/update_account_test.rb | 8 | ||||
| -rw-r--r-- | test/integration/browser/account_livecycle_test.rb | 15 | ||||
| -rw-r--r-- | test/unit/account_test.rb | 56 |
14 files changed, 149 insertions, 63 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index 8a2abc2..2ee1037 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,13 +1,13 @@ PATH remote: engines/billing specs: - leap_web_billing (0.9.1) + leap_web_billing (0.9.2) braintree PATH remote: engines/support specs: - leap_web_help (0.9.1) + leap_web_help (0.9.2) PATH remote: vendor/gems/certificate_authority diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb index 709e076..cb7b7bc 100644 --- a/app/controllers/api/users_controller.rb +++ b/app/controllers/api/users_controller.rb @@ -53,7 +53,7 @@ module Api end def update - @user.account.update params[:user] + @user.account.update user_update_params respond_with @user end @@ -67,6 +67,15 @@ module Api private + def user_update_params + params.require(:user).permit :login, + :password_verifier, + :password_salt, + :recovery_code_verifier, + :recovery_code_salt, + :public_key + end + def release_handles current_user.is_monitor? || params[:identities] == "destroy" end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1f37fea..d3cfc2b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -99,7 +99,7 @@ class ApplicationController < ActionController::Base # # URL paths for which we don't enforce the locale as the prefix of the path. # - NON_LOCALE_PATHS = /^\/(assets|webfinger|.well-known|rails|key|[0-9]+)($|\/)/ + NON_LOCALE_PATHS = /^\/(assets|webfinger|.well-known|rails|key|[0-9]+|new)($|\/)/ # # For some requests, we ignore locale determination. diff --git a/app/controllers/invite_codes_controller.rb b/app/controllers/invite_codes_controller.rb index 6a7fef3..96836ee 100644 --- a/app/controllers/invite_codes_controller.rb +++ b/app/controllers/invite_codes_controller.rb @@ -7,7 +7,9 @@ class InviteCodesController < ApplicationController def index @invite = InviteCode.new # for the creation form. - @invites = InviteCode.all.page(params[:page]).per(APP_CONFIG[:pagination_size]) + @invites = InviteCode.by_updated_at.descending. + page(params[:page]). + per(APP_CONFIG[:pagination_size]) respond_with @invites end diff --git a/app/models/account.rb b/app/models/account.rb index 3283bcc..d77c61f 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -25,57 +25,57 @@ class Account # configuration by same name. # def self.create(attrs, options={}) + User.new(attrs).tap do |user| + self.new(user).create(options) + end + end + + def create(options={}) identity = nil - user = nil - user = User.new(attrs) if options[:invite_required] == false user.ignore_invites! end user.save # this is not very atomic, but we do the best we can: - if !user.is_tmp? && user.persisted? + return unless user.persisted? + if !user.is_tmp? identity = user.identity identity.user_id = user.id identity.save identity.errors.each do |attr, msg| user.errors.add(attr, msg) end - if user.invite_required? - user_invite_code = InviteCode.find_by_invite_code user.invite_code - user_invite_code.invite_count += 1 - user_invite_code.save - end end + consume_invite_code if user.invite_required? rescue VALIDATION_FAILED => ex - user.errors.add(:base, ex.to_s) if user + user.errors.add(:base, ex.to_s) ensure - if creation_problem?(user, identity) + if creation_problem?(identity) user.destroy if user && user.persisted? identity.destroy if identity && identity.persisted? end - return user end def update(attrs) if attrs[:password_verifier].present? update_login(attrs[:login]) - @user.update_attributes attrs.slice(:password_verifier, :password_salt) + user.update_attributes attrs.slice(:password_verifier, :password_salt) end if attrs[:recovery_code_verifier].present? - @user.update_attributes attrs.slice(:recovery_code_verifier, :recovery_code_salt) + user.update_attributes attrs.slice(:recovery_code_verifier, :recovery_code_salt) end # TODO: move into identity controller key = update_pgp_key(attrs[:public_key]) - @user.errors.set :public_key, key.errors.full_messages - @user.save && save_identities - @user.refresh_identity + user.errors.set :public_key, key.errors.full_messages + user.save && save_identities + user.refresh_identity end def destroy(release_handles=false) - return unless @user - if !@user.is_tmp? - @user.identities.each do |id| + return unless user + if !user.is_tmp? + user.identities.each do |id| if release_handles == false id.orphan! else @@ -83,44 +83,56 @@ class Account end end end - @user.destroy + user.destroy end # when a user is disable, all their data and associations remain # in place, but the user should not be able to send email or # create new authentication certificates. def disable - if @user && !@user.is_tmp? - @user.enabled = false - @user.save - @user.identities.each do |id| + if user && !user.is_tmp? + user.enabled = false + user.save + user.identities.each do |id| id.disable! end end end def enable - @user.enabled = true - @user.save - @user.identities.each do |id| + user.enabled = true + user.save + user.identities.each do |id| id.enable! end end protected + attr_reader :user + + def consume_invite_code + invite_code = InviteCode.find_by_invite_code user.invite_code + if user.is_test? && invite_code.max_uses == 1 + invite_code.destroy + else + invite_code.invite_count += 1 + invite_code.save + end + end + def update_login(login) return unless login.present? - @old_identity = Identity.for(@user) - @user.login = login - @new_identity = Identity.for(@user) # based on the new login - @old_identity.destination = @user.email_address # alias old -> new + @old_identity = Identity.for(user) + user.login = login + @new_identity = Identity.for(user) # based on the new login + @old_identity.destination = user.email_address # alias old -> new end def update_pgp_key(key) PgpKey.new(key).tap do |key| if key.present? && key.valid? - @new_identity ||= Identity.for(@user) + @new_identity ||= Identity.for(user) @new_identity.set_key(:pgp, key) end end @@ -130,7 +142,7 @@ class Account @new_identity.try(:save) && @old_identity.try(:save) end - def self.creation_problem?(user, identity) + def creation_problem?(identity) return true if user.nil? || !user.persisted? || user.errors.any? if !user.is_tmp? return true if identity.nil? || !identity.persisted? || identity.errors.any? diff --git a/app/models/invite_code.rb b/app/models/invite_code.rb index 9c6df66..8a56484 100644 --- a/app/models/invite_code.rb +++ b/app/models/invite_code.rb @@ -12,6 +12,8 @@ class InviteCode < CouchRest::Model::Base design do view :by_invite_code view :by_invite_count + view :by_created_at + view :by_updated_at end def initialize(attributes = {}, options = {}) diff --git a/app/views/invite_codes/_invite_code.html.haml b/app/views/invite_codes/_invite_code.html.haml index a3c420d..a167432 100644 --- a/app/views/invite_codes/_invite_code.html.haml +++ b/app/views/invite_codes/_invite_code.html.haml @@ -1,5 +1,7 @@ %tr %td + = simple_date(invite_code.updated_at) + %td = simple_date(invite_code.created_at) %td %input.invite-code{:value => invite_code.invite_code} diff --git a/app/views/invite_codes/index.html.haml b/app/views/invite_codes/index.html.haml index 40fccdf..98c49c7 100644 --- a/app/views/invite_codes/index.html.haml +++ b/app/views/invite_codes/index.html.haml @@ -11,10 +11,10 @@ %td= f.text_field :max_uses, style: 'width: 4em' %td= f.submit t("helpers.submit.invites.create"), style: 'margin-bottom: 10px', class: 'btn btn-default' -= table @invites, %w(created code uses actions) += table @invites, %w(updated created code uses actions) -# select the text of the invite code when you click on it: :javascript $("input.invite-code").focus(function(){ this.select(); - });
\ No newline at end of file + }); diff --git a/lib/leap_web/version.rb b/lib/leap_web/version.rb index 0c4f19d..fcce54a 100644 --- a/lib/leap_web/version.rb +++ b/lib/leap_web/version.rb @@ -1,3 +1,3 @@ module LeapWeb - VERSION = "0.9.1" unless defined?(LeapWeb::VERSION) + VERSION = "0.9.2" unless defined?(LeapWeb::VERSION) end diff --git a/test/functional/api/users_controller_test.rb b/test/functional/api/users_controller_test.rb index 88ecae0..ee183f9 100644 --- a/test/functional/api/users_controller_test.rb +++ b/test/functional/api/users_controller_test.rb @@ -4,28 +4,42 @@ class Api::UsersControllerTest < ApiControllerTest test "user can change settings" do user = find_record :user - changed_attribs = record_attributes_for :user_with_settings + attribs = record_attributes_for(:user) + changed_attribs = attribs.slice 'login', + 'password_verifier', + 'password_salt' account_settings = stub account_settings.expects(:update).with(changed_attribs) Account.expects(:new).with(user).returns(account_settings) login user - api_put :update, :user => changed_attribs, :id => user.id, :format => :json + api_put :update, :user => attribs, :id => user.id, :format => :json assert_equal user, assigns[:user] assert_response 204 assert @response.body.blank?, "Response should be blank" end + test "deal with empty settings" do + user = find_record :user + login user + assert_raises ActionController::ParameterMissing do + api_put :update, :id => user.id, :format => :json + end + end + test "admin can update user" do user = find_record :user - changed_attribs = record_attributes_for :user_with_settings + attribs = record_attributes_for(:user) + changed_attribs = attribs.slice 'login', + 'password_verifier', + 'password_salt' account_settings = stub account_settings.expects(:update).with(changed_attribs) Account.expects(:new).with(user).returns(account_settings) login :is_admin? => true - api_put :update, :user => changed_attribs, :id => user.id, :format => :json + api_put :update, :user => attribs, :id => user.id, :format => :json assert_equal user, assigns[:user] assert_response 204 diff --git a/test/integration/api/srp_test.rb b/test/integration/api/srp_test.rb index b9605f9..ef5d9b8 100644 --- a/test/integration/api/srp_test.rb +++ b/test/integration/api/srp_test.rb @@ -46,7 +46,7 @@ class SrpTest < RackTest @password = password end - def update_user(params) + def update_user(params = {}) put api_url("users/#{@user.id}.json"), user_params(params), auth_headers diff --git a/test/integration/api/update_account_test.rb b/test/integration/api/update_account_test.rb index f083dbc..dd28b06 100644 --- a/test/integration/api/update_account_test.rb +++ b/test/integration/api/update_account_test.rb @@ -19,6 +19,14 @@ class UpdateAccountTest < SrpTest assert_login_required end + test "empty request" do + authenticate + update_user + refute last_response.successful? + assert_equal 400, last_response.status + assert_equal '', last_response.body + end + test "update password via api" do authenticate update_user password: "No! Verify me instead." diff --git a/test/integration/browser/account_livecycle_test.rb b/test/integration/browser/account_livecycle_test.rb index cfab444..68775d3 100644 --- a/test/integration/browser/account_livecycle_test.rb +++ b/test/integration/browser/account_livecycle_test.rb @@ -63,6 +63,16 @@ class AccountLivecycleTest < BrowserIntegrationTest assert_invalid_login(page) end + test "failed login with locale" do + page.driver.add_header 'Accept-Language', 'de' + visit '/' + click_on 'Anmelden' + fill_in 'Nutzername', with: 'username' + fill_in 'Password', with: 'falsches password' + click_on 'Session erstellen' + assert_invalid_login(page, locale: :de) + end + test "account destruction" do username, password = submit_signup @@ -115,9 +125,10 @@ class AccountLivecycleTest < BrowserIntegrationTest click_on 'Log In' end - def assert_invalid_login(page) + def assert_invalid_login(page, locale: nil) assert page.has_selector? '.btn-primary.disabled' - assert page.has_content? sanitize(I18n.t(:invalid_user_pass), tags: []) + message = I18n.t :invalid_user_pass, locale: locale + assert page.has_content? sanitize(message, tags: []) assert page.has_no_selector? '.btn-primary.disabled' end diff --git a/test/unit/account_test.rb b/test/unit/account_test.rb index 058e196..61b40b5 100644 --- a/test/unit/account_test.rb +++ b/test/unit/account_test.rb @@ -12,7 +12,7 @@ class AccountTest < ActiveSupport::TestCase end test "create a new account when invited" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) assert user.valid?, "unexpected errors: #{user.errors.inspect}" assert user.persisted? assert id = user.identity @@ -23,7 +23,7 @@ class AccountTest < ActiveSupport::TestCase test "fail to create account without invite" do with_config invite_required: true do - user = Account.create(FactoryGirl.attributes_for(:user)) + user = Account.create(user_attributes) assert !user.valid?, "user should not be valid" assert !user.persisted?, "user should not have been saved" assert_has_errors user, invite_code: "This is not a valid code" @@ -32,7 +32,7 @@ class AccountTest < ActiveSupport::TestCase test "allow invite_required override" do with_config invite_required: true do - user = Account.create(FactoryGirl.attributes_for(:user), :invite_required => false) + user = Account.create(user_attributes, :invite_required => false) assert user.valid?, "unexpected errors: #{user.errors.inspect}" assert user.persisted?, "user should have been saved" user.account.destroy @@ -41,7 +41,7 @@ class AccountTest < ActiveSupport::TestCase test "create a new account" do with_config invite_required: false do - user = Account.create(FactoryGirl.attributes_for(:user)) + user = Account.create(user_attributes) assert user.valid?, "unexpected errors: #{user.errors.inspect}" assert user.persisted? user.account.destroy @@ -50,7 +50,7 @@ class AccountTest < ActiveSupport::TestCase test "error on reused username" do with_config invite_required: false do - attributes = FactoryGirl.attributes_for :user + attributes = user_attributes user = Account.create attributes dup = Account.create attributes assert !dup.valid? @@ -72,17 +72,17 @@ class AccountTest < ActiveSupport::TestCase # We keep an identity that will block the handle from being reused. assert_difference "Identity.count" do assert_no_difference "User.count" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) user.account.destroy end end end test "change username and create alias" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) old_id = user.identity old_email = user.email_address - user.account.update(FactoryGirl.attributes_for(:user)) + user.account.update(user_attributes) user.reload old_id.reload assert user.valid? @@ -97,7 +97,7 @@ class AccountTest < ActiveSupport::TestCase end test "create recovery code if it does not exist" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) user.account.update(:recovery_code_verifier => "abc", :recovery_code_salt => "123") user.reload @@ -108,7 +108,7 @@ class AccountTest < ActiveSupport::TestCase end test "update recovery code that already exists" do - user = Account.create(FactoryGirl.attributes_for(:user, + user = Account.create(user_attributes( :invite_code => @testcode.invite_code, :recovery_code_verifier => "000", :recovery_code_salt => "111")) @@ -123,7 +123,7 @@ class AccountTest < ActiveSupport::TestCase end test "update password" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) user.account.update(:password_verifier => "551A8B", :password_salt => "551A8B") assert_equal "551A8B", user.password_verifier @@ -134,18 +134,38 @@ class AccountTest < ActiveSupport::TestCase test "Invite code count goes up by 1 when the invite code is entered" do with_config invite_required: true do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) user_code = InviteCode.find_by_invite_code user.invite_code - user_code.save user.save assert user.persisted? assert_equal 1, user_code.invite_count end + end + + test "Single use invite code is destroyed when used by test user" do + with_config invite_required: true do + attrs = user_attributes invite_code: @testcode.invite_code + attrs[:login] = 'test_user_' + attrs[:login] + user = Account.create(attrs) + user.save + assert user.persisted?, user.errors.inspect + assert_nil InviteCode.find_by_invite_code user.invite_code + end + end + test "Single use invite code is destroyed when used by tmp user" do + with_config invite_required: true do + attrs = user_attributes invite_code: @testcode.invite_code + attrs[:login] = 'tmp_user_' + attrs[:login] + user = Account.create(attrs) + user.save + assert user.persisted?, user.errors.inspect + assert_nil InviteCode.find_by_invite_code user.invite_code + end end test "Invite code stays zero when invite code is not used" do - #user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + #user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) invalid_user = FactoryGirl.build(:user, :invite_code => @testcode.invite_code) invalid_user.save user_code = InviteCode.find_by_invite_code invalid_user.invite_code @@ -155,7 +175,7 @@ class AccountTest < ActiveSupport::TestCase end test "disabled accounts have no cert fingerprints" do - user = Account.create(FactoryGirl.attributes_for(:user)) + user = Account.create(user_attributes) cert = stub(expiry: 1.month.from_now, fingerprint: SecureRandom.hex) user.identity.register_cert cert user.identity.save @@ -184,6 +204,8 @@ class AccountTest < ActiveSupport::TestCase end end + protected + # Tests for the presence of the errors given. # Does not test for the absence of other errors - so there may be more. def assert_has_errors(record, errors) @@ -194,4 +216,8 @@ class AccountTest < ActiveSupport::TestCase end end + def user_attributes(attrs = {}) + FactoryGirl.attributes_for :user, attrs + end + end |