diff options
| -rw-r--r-- | app/controllers/api/users_controller.rb | 42 | ||||
| -rw-r--r-- | app/models/account.rb | 12 | ||||
| -rw-r--r-- | test/integration/api/pgp_key_test.rb | 9 | ||||
| -rw-r--r-- | test/unit/account_test.rb | 163 |
4 files changed, 113 insertions, 113 deletions
diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb index cb7b7bc..65b80c7 100644 --- a/app/controllers/api/users_controller.rb +++ b/app/controllers/api/users_controller.rb @@ -34,14 +34,6 @@ module Api end end - def user_response - @user.to_hash.tap do |user_hash| - if @user == current_user - user_hash['is_admin'] = @user.is_admin? - end - end - end - def create if current_user.is_monitor? create_test_account @@ -53,8 +45,14 @@ module Api end def update - @user.account.update user_update_params - respond_with @user + if user_update_params.present? + @user.account.update user_update_params + respond_with @user + else + # TODO: move into identity controller + key = update_pgp_key(user_key_param[:public_key]) + respond_with key + end end def destroy @@ -67,13 +65,24 @@ module Api private + def user_response + @user.to_hash.tap do |user_hash| + if @user == current_user + user_hash['is_admin'] = @user.is_admin? + end + end + end + def user_update_params params.require(:user).permit :login, :password_verifier, :password_salt, :recovery_code_verifier, - :recovery_code_salt, - :public_key + :recovery_code_salt + end + + def user_key_param + params.require(:user).permit :public_key end def release_handles @@ -99,5 +108,14 @@ module Api end end + def update_pgp_key(key) + PgpKey.new(key).tap do |key| + if key.valid? + identity = Identity.for(@user) + identity.set_key(:pgp, key) + identity.save + end + end + end end end diff --git a/app/models/account.rb b/app/models/account.rb index d77c61f..5a4111d 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -65,9 +65,6 @@ class Account if attrs[:recovery_code_verifier].present? 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 end @@ -129,15 +126,6 @@ class Account @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.set_key(:pgp, key) - end - end - end - def save_identities @new_identity.try(:save) && @old_identity.try(:save) end diff --git a/test/integration/api/pgp_key_test.rb b/test/integration/api/pgp_key_test.rb index 4c7fb4c..f2744e1 100644 --- a/test/integration/api/pgp_key_test.rb +++ b/test/integration/api/pgp_key_test.rb @@ -14,16 +14,16 @@ class PgpKeyTest < SrpTest assert_equal key, Identity.for(@user).keys[:pgp] end - # eventually probably want to remove most of this into a non-integration - # functional test test "prevent uploading invalid key" do update_user public_key: "invalid key" + assert_invalid_key_response assert_nil Identity.for(@user).keys[:pgp] end test "prevent emptying public key" do update_user public_key: key update_user public_key: "" + assert_invalid_key_response assert_equal key, Identity.for(@user).keys[:pgp] end @@ -32,4 +32,9 @@ class PgpKeyTest < SrpTest def key @key ||= FactoryGirl.build :pgp_key end + + def assert_invalid_key_response + assert_response :unprocessable_entity + assert_json_error "public_key_block"=>["does not look like an armored pgp public key block"] + end end diff --git a/test/unit/account_test.rb b/test/unit/account_test.rb index 61b40b5..ebfff6b 100644 --- a/test/unit/account_test.rb +++ b/test/unit/account_test.rb @@ -8,164 +8,153 @@ class AccountTest < ActiveSupport::TestCase end teardown do + @user.account.destroy if @user && @user.persisted? Identity.destroy_all_orphaned end test "create a new account when invited" do - 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 - assert_equal user.email_address, id.address - assert_equal user.email_address, id.destination - user.account.destroy + @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 + assert_equal @user.email_address, id.address + assert_equal @user.email_address, id.destination end test "fail to create account without invite" do with_config invite_required: true do - 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" + @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" end end test "allow invite_required override" do with_config invite_required: true do - 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 + @user = Account.create(user_attributes, :invite_required => false) + assert @user.valid?, "unexpected errors: #{@user.errors.inspect}" + assert @user.persisted?, "user should have been saved" end end test "create a new account" do with_config invite_required: false do - user = Account.create(user_attributes) - assert user.valid?, "unexpected errors: #{user.errors.inspect}" - assert user.persisted? - user.account.destroy + @user = Account.create(user_attributes) + assert @user.valid?, "unexpected errors: #{@user.errors.inspect}" + assert @user.persisted? end end test "error on reused username" do with_config invite_required: false do attributes = user_attributes - user = Account.create attributes + @user = Account.create attributes dup = Account.create attributes assert !dup.valid? assert_has_errors dup, login: "has already been taken" - user.account.destroy end end test "error on invalid username" do with_config invite_required: false do attributes = FactoryGirl.attributes_for :user, login: "a" - user = Account.create attributes - assert !user.valid? - assert_has_errors user, login: "Must have at least two characters" + @user = Account.create attributes + assert !@user.valid? + assert_has_errors @user, login: "Must have at least two characters" end end - test "create and remove a user account" do + test "create and remove a @user account" do # 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(user_attributes( :invite_code => @testcode.invite_code)) - user.account.destroy + @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(user_attributes( :invite_code => @testcode.invite_code)) - old_id = user.identity - old_email = user.email_address - user.account.update(user_attributes) - user.reload + @user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) + old_id = @user.identity + old_email = @user.email_address + @user.account.update(user_attributes) + @user.reload old_id.reload - assert user.valid? - assert user.persisted? - assert id = user.identity + assert @user.valid? + assert @user.persisted? + assert id = @user.identity assert id.persisted? - assert_equal user.email_address, id.address - assert_equal user.email_address, id.destination - assert_equal user.email_address, old_id.destination + assert_equal @user.email_address, id.address + assert_equal @user.email_address, id.destination + assert_equal @user.email_address, old_id.destination assert_equal old_email, old_id.address - user.account.destroy end test "create recovery code if it does not exist" do - user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) - user.account.update(:recovery_code_verifier => "abc", :recovery_code_salt => "123") - user.reload + @user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) + @user.account.update(:recovery_code_verifier => "abc", :recovery_code_salt => "123") + @user.reload - assert_equal "abc", user.recovery_code_verifier - assert_equal "123", user.recovery_code_salt - - user.account.destroy + assert_equal "abc", @user.recovery_code_verifier + assert_equal "123", @user.recovery_code_salt end test "update recovery code that already exists" do - user = Account.create(user_attributes( + @user = Account.create(user_attributes( :invite_code => @testcode.invite_code, :recovery_code_verifier => "000", :recovery_code_salt => "111")) - user.account.update(:recovery_code_verifier => "abc", :recovery_code_salt => "123") - user.reload - - assert_equal "abc", user.recovery_code_verifier - assert_equal "123", user.recovery_code_salt + @user.account.update(:recovery_code_verifier => "abc", :recovery_code_salt => "123") + @user.reload - user.account.destroy + assert_equal "abc", @user.recovery_code_verifier + assert_equal "123", @user.recovery_code_salt end test "update password" do - 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 - assert_equal "551A8B", user.password_salt + @user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) + @user.account.update(:password_verifier => "551A8B", :password_salt => "551A8B") - user.account.destroy + assert_equal "551A8B", @user.password_verifier + assert_equal "551A8B", @user.password_salt end test "Invite code count goes up by 1 when the invite code is entered" do with_config invite_required: true do - user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) - user_code = InviteCode.find_by_invite_code user.invite_code - user.save - assert user.persisted? + @user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) + user_code = InviteCode.find_by_invite_code @user.invite_code + @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 + 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 + @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 + 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 + @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(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 @@ -175,16 +164,16 @@ class AccountTest < ActiveSupport::TestCase end test "disabled accounts have no cert fingerprints" do - user = Account.create(user_attributes) + @user = Account.create(user_attributes) cert = stub(expiry: 1.month.from_now, fingerprint: SecureRandom.hex) - user.identity.register_cert cert - user.identity.save - assert_equal(cert.fingerprint, Identity.for(user).cert_fingerprints.keys.first) - user.account.disable - assert_equal({}, Identity.for(user).cert_fingerprints) - assert_equal(cert.fingerprint, Identity.for(user).read_attribute(:disabled_cert_fingerprints).keys.first) - user.account.enable - assert_equal(cert.fingerprint, Identity.for(user).cert_fingerprints.keys.first) + @user.identity.register_cert cert + @user.identity.save + assert_equal(cert.fingerprint, Identity.for(@user).cert_fingerprints.keys.first) + @user.account.disable + assert_equal({}, Identity.for(@user).cert_fingerprints) + assert_equal(cert.fingerprint, Identity.for(@user).read_attribute(:disabled_cert_fingerprints).keys.first) + @user.account.enable + assert_equal(cert.fingerprint, Identity.for(@user).cert_fingerprints.keys.first) end # Pixelated relies on the ability to test invite codes without sending a @@ -192,15 +181,15 @@ class AccountTest < ActiveSupport::TestCase # So we better make sure we return the appropriate errors test "errors trying to create account with invite only" do with_config invite_required: true do - user = Account.create invite_code: @testcode.invite_code - assert user.errors[:invite_code].blank? + @user = Account.create invite_code: @testcode.invite_code + assert @user.errors[:invite_code].blank? end end test "errors trying to create account with invalid invite only" do with_config invite_required: true do - user = Account.create invite_code: "wrong_invite_code" - assert_has_errors user, invite_code: "This is not a valid code" + @user = Account.create invite_code: "wrong_invite_code" + assert_has_errors @user, invite_code: "This is not a valid code" end end |