Merge pull request #176 from azul/feature/api-authenticated-configs

API: Authenticated access to config settings

13 files changed, 45 insertions, 45 deletions

diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb
index c4c922b..b558ad8 100644
--- a/test/functional/application_controller_test.rb
+++ b/test/functional/application_controller_test.rb
@@ -9,13 +9,13 @@ class ApplicationControllerTest < ActionController::TestCase
 
   def test_require_login_redirect
     @controller.send(:require_login)
-    assert_access_denied(true, false)
+    assert_login_required
   end
 
   def test_require_login
     login
     @controller.send(:require_login)
-    assert_access_denied(false)
+    assert_access_granted
   end
 
   def test_require_admin
diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index 4af9ca6..7d1745c 100644
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -52,7 +52,7 @@ class UsersControllerTest < ActionController::TestCase
     nonid = 'thisisnotanexistinguserid'
 
     get :show, :id => nonid
-    assert_access_denied(true, false)
+    assert_login_required
   end
 
   test "may not show non-existing user without admin" do
diff --git a/test/functional/v1/messages_controller_test.rb b/test/functional/v1/messages_controller_test.rb
index 24a5b1f..a50fded 100644
--- a/test/functional/v1/messages_controller_test.rb
+++ b/test/functional/v1/messages_controller_test.rb
@@ -51,7 +51,7 @@ class V1::MessagesControllerTest < ActionController::TestCase
 
   test "fails if not authenticated" do
     get :index, :format => :json
-    assert_access_denied
+    assert_login_required
   end
 
 end
diff --git a/test/functional/v1/users_controller_test.rb b/test/functional/v1/users_controller_test.rb
index fe3cfe7..ffe2484 100644
--- a/test/functional/v1/users_controller_test.rb
+++ b/test/functional/v1/users_controller_test.rb
@@ -34,7 +34,9 @@ class V1::UsersControllerTest < ActionController::TestCase
   test "user cannot update other user" do
     user = find_record :user
     login
-    put :update, :user => record_attributes_for(:user_with_settings), :id => user.id, :format => :json
+    put :update, id: user.id,
+      user: record_attributes_for(:user_with_settings),
+      :format => :json
     assert_access_denied
   end
 
diff --git a/test/integration/api/cert_test.rb b/test/integration/api/cert_test.rb
index 74d439a..118fb9f 100644
--- a/test/integration/api/cert_test.rb
+++ b/test/integration/api/cert_test.rb
@@ -14,7 +14,7 @@ class CertTest < ApiIntegrationTest
 
   test "fetching certs requires login by default" do
     get '/1/cert', {}, RACK_ENV
-    assert_json_response error: I18n.t(:not_authorized)
+    assert_login_required
   end
 
   test "retrieve anonymous eip cert" do
diff --git a/test/integration/api/login_test.rb b/test/integration/api/login_test.rb
index 92d153f..f37639e 100644
--- a/test/integration/api/login_test.rb
+++ b/test/integration/api/login_test.rb
@@ -45,6 +45,6 @@ class LoginTest < SrpTest
   test "logout requires token" do
     authenticate
     logout(nil, {})
-    assert_equal 422, last_response.status
+    assert_login_required
   end
 end
diff --git a/test/integration/api/smtp_cert_test.rb b/test/integration/api/smtp_cert_test.rb
index 7697e44..b1bfd43 100644
--- a/test/integration/api/smtp_cert_test.rb
+++ b/test/integration/api/smtp_cert_test.rb
@@ -42,13 +42,13 @@ class SmtpCertTest < ApiIntegrationTest
   test "fetching smtp certs requires email account" do
     login
     post '/1/smtp_cert', {}, RACK_ENV
-    assert_json_response error: I18n.t(:not_authorized)
+    assert_access_denied
   end
 
   test "no anonymous smtp certs" do
     with_config allow_anonymous_certs: true do
       post '/1/smtp_cert', {}, RACK_ENV
-      assert_json_response error: I18n.t(:not_authorized)
+      assert_login_required
     end
   end
 end
diff --git a/test/integration/api/srp_test.rb b/test/integration/api/srp_test.rb
index 26adc8c..946450e 100644
--- a/test/integration/api/srp_test.rb
+++ b/test/integration/api/srp_test.rb
@@ -1,5 +1,4 @@
 class SrpTest < RackTest
-  include AssertResponses
 
   teardown do
     if @user
diff --git a/test/integration/api/update_account_test.rb b/test/integration/api/update_account_test.rb
index 63429e7..16bbb8c 100644
--- a/test/integration/api/update_account_test.rb
+++ b/test/integration/api/update_account_test.rb
@@ -16,7 +16,7 @@ class UpdateAccountTest < SrpTest
     authenticate
     put "http://api.lvh.me:3000/1/users/" + @user.id + '.json',
       user_params(password: "No! Verify me instead.")
-    assert_access_denied
+    assert_login_required
   end
 
   test "update password via api" do
diff --git a/test/support/assert_responses.rb b/test/support/assert_responses.rb
index 19c2768..1c9d49d 100644
--- a/test/support/assert_responses.rb
+++ b/test/support/assert_responses.rb
@@ -55,6 +55,25 @@ module AssertResponses
       get_response.headers["Content-Disposition"]
   end
 
+  def assert_login_required
+    assert_error_response :not_authorized_login, :unauthorized
+  end
+
+  def assert_access_denied
+    assert_error_response :not_authorized, :forbidden
+  end
+
+  def assert_error_response(key, status=nil)
+    message = I18n.t(key)
+    if content_type == 'application/json'
+      status ||= :unprocessable_entity
+      assert_json_response('error' => key.to_s, 'message' => message)
+      assert_response status
+    else
+      assert_equal({:alert => message}, flash.to_hash)
+    end
+  end
+
 end
 
 class ::ActionController::TestCase
diff --git a/test/support/auth_test_helper.rb b/test/support/auth_test_helper.rb
index 38c2ea1..7af3341 100644
--- a/test/support/auth_test_helper.rb
+++ b/test/support/auth_test_helper.rb
@@ -19,27 +19,9 @@ module AuthTestHelper
     return @current_user
   end
 
-  def assert_login_required
-    assert_access_denied(true, false)
-  end
-
-  def assert_access_denied(denied = true, logged_in = true)
-    if denied
-      if @response.content_type == 'application/json'
-        assert_json_response('error' => I18n.t(:not_authorized))
-        assert_response :unprocessable_entity
-      else
-        if logged_in
-          assert_equal({:alert => I18n.t(:not_authorized)}, flash.to_hash)
-          assert_redirected_to home_url
-        else
-          assert_equal({:alert => I18n.t(:not_authorized_login)}, flash.to_hash)
-          assert_redirected_to login_url
-        end
-      end
-    else
-      assert flash[:alert].blank?
-    end
+  def assert_access_granted
+    assert flash[:alert].blank?,
+      "expected to have access but there was a flash alert"
   end
 
   def expect_logout
diff --git a/test/support/rack_test.rb b/test/support/rack_test.rb
index 806339a..2c9fa9a 100644
--- a/test/support/rack_test.rb
+++ b/test/support/rack_test.rb
@@ -3,6 +3,7 @@ require_relative 'assert_responses'
 class RackTest < ActiveSupport::TestCase
   include Rack::Test::Methods
   include Warden::Test::Helpers
+  include AssertResponses
 
   CONFIG_RU = (Rails.root + 'config.ru').to_s
   OUTER_APP = Rack::Builder.parse_file(CONFIG_RU).first
@@ -11,11 +12,6 @@ class RackTest < ActiveSupport::TestCase
     OUTER_APP
   end
 
-  def assert_access_denied
-    assert_json_response('error' => I18n.t(:not_authorized))
-    assert_response :unprocessable_entity
-  end
-
   # inspired by rails 4
   # -> actionpack/lib/action_dispatch/testing/assertions/response.rb
   def assert_response(type, message = nil)
diff --git a/test/unit/identity_test.rb b/test/unit/identity_test.rb
index cb0f6bd..f5c95f8 100644
--- a/test/unit/identity_test.rb
+++ b/test/unit/identity_test.rb
@@ -14,23 +14,23 @@ class IdentityTest < ActiveSupport::TestCase
     end
   end
 
-  test "blank @identity does not crash on valid?" do
+  test "blank identity does not crash on valid?" do
     @id = Identity.new
     assert !@id.valid?
   end
 
-  test "enabled @identity requires destination" do
+  test "enabled identity requires destination" do
     @id = Identity.new user: @user, address: @user.email_address
     assert !@id.valid?
     assert_equal ["can't be blank"], @id.errors[:destination]
   end
 
-  test "disabled @identity requires no destination" do
+  test "disabled identity requires no destination" do
     @id = Identity.new address: @user.email_address
     assert @id.valid?
   end
 
-  test "initial @identity for a user" do
+  test "initial identity for a user" do
     @id = Identity.for(@user)
     assert_equal @user.email_address, @id.address
     assert_equal @user.email_address, @id.destination
@@ -90,7 +90,7 @@ class IdentityTest < ActiveSupport::TestCase
     assert_equal @id.keys[:pgp], result["value"]
   end
 
-  test "fail to add non-local email address as @identity address" do
+  test "fail to add non-local email address as identity address" do
     @id = Identity.for @user, address: forward_address
     assert !@id.valid?
     assert_match /needs to end in/, @id.errors[:address].first
@@ -110,7 +110,7 @@ class IdentityTest < ActiveSupport::TestCase
     assert @id.errors.messages[:destination].include? "needs to be a valid email address"
   end
 
-  test "disabled @identity" do
+  test "disabled identity" do
     @id = Identity.for(@user)
     @id.disable
     assert_equal @user.email_address, @id.address
@@ -120,7 +120,7 @@ class IdentityTest < ActiveSupport::TestCase
     assert @id.valid?
   end
 
-  test "disabled @identity blocks handle" do
+  test "disabled identity blocks handle" do
     @id = Identity.for(@user)
     @id.disable
     @id.save
@@ -177,7 +177,9 @@ class IdentityTest < ActiveSupport::TestCase
   end
 
   def cert_stub
-    @cert_stub ||= stub expiry: 1.month.from_now,
+    # make this expire later than the others so it's on top
+    # when sorting by expiry descending.
+    @cert_stub ||= stub expiry: 2.month.from_now,
     fingerprint: SecureRandom.hex
   end
 end