diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb
index cb7b7bc7839ebab655c9aa46f920abb4b3567329..65b80c7d7d4eeb82990f885b2122a99989604b2f 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 d77c61f52b562212ede08c76eba0ddc968aff246..5a4111d323df31fc4ae601aaf8dfbfd57b0691f9 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 4c7fb4c73069cd7677bb4efd40699dff1d2bf584..f2744e118448357702e061bee3074b18240fd580 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 61b40b5f1b8b23218fe740ff8662eb3b6b3640bb..ebfff6b993ef3afc370007d19d7d08af5a2e9b1c 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