From e1243d02953b4012d6bb216efc9b0606809ab4bb Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 6 Feb 2014 09:47:37 +0100 Subject: minor: refactor token auth a bit --- .../controller_extension/token_authentication.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/users/app/controllers/controller_extension/token_authentication.rb b/users/app/controllers/controller_extension/token_authentication.rb index 530294a..cd5c074 100644 --- a/users/app/controllers/controller_extension/token_authentication.rb +++ b/users/app/controllers/controller_extension/token_authentication.rb @@ -1,11 +1,14 @@ module ControllerExtension::TokenAuthentication extend ActiveSupport::Concern - def token_authenticate - authenticate_with_http_token do |token_id, options| - @token = Token.find(token_id) + def token + @token ||= authenticate_with_http_token do |token_id, options| + Token.find(token_id) end - @token.authenticate if @token + end + + def token_authenticate + token.authenticate if token end def logout @@ -14,10 +17,7 @@ module ControllerExtension::TokenAuthentication end def clear_token - authenticate_with_http_token do |token_id, options| - @token = Token.find(token_id) - @token.destroy if @token - end + token.destroy if token end end -- cgit v1.2.3 From 3f9dc65636afb57fed441978dca4bf7d3209bd2d Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 7 Feb 2014 14:38:56 +0100 Subject: rename authorize to require_login authorize_admin -> require_admin also add require_token which will ensure token has been used for auth. --- billing/app/controllers/billing_admin_controller.rb | 2 +- billing/app/controllers/credit_card_info_controller.rb | 2 +- billing/app/controllers/customer_controller.rb | 2 +- billing/app/controllers/payments_controller.rb | 2 +- billing/app/controllers/subscriptions_controller.rb | 2 +- certs/app/controllers/certs_controller.rb | 7 +++---- help/app/controllers/tickets_controller.rb | 2 +- users/app/controllers/controller_extension/authentication.rb | 4 ++-- .../controllers/controller_extension/token_authentication.rb | 4 ++++ users/app/controllers/users_controller.rb | 4 ++-- users/app/controllers/v1/users_controller.rb | 4 ++-- users/test/functional/application_controller_test.rb | 12 ++++++------ users/test/functional/v1/sessions_controller_test.rb | 2 +- users/test/unit/unauthenticated_user_test.rb | 7 +++++++ users/test/unit/unauthorized_user_test.rb | 7 ------- 15 files changed, 33 insertions(+), 30 deletions(-) create mode 100644 users/test/unit/unauthenticated_user_test.rb delete mode 100644 users/test/unit/unauthorized_user_test.rb diff --git a/billing/app/controllers/billing_admin_controller.rb b/billing/app/controllers/billing_admin_controller.rb index cd6149f..e11d4ee 100644 --- a/billing/app/controllers/billing_admin_controller.rb +++ b/billing/app/controllers/billing_admin_controller.rb @@ -1,5 +1,5 @@ class BillingAdminController < BillingBaseController - before_filter :authorize_admin + before_filter :require_admin def show diff --git a/billing/app/controllers/credit_card_info_controller.rb b/billing/app/controllers/credit_card_info_controller.rb index 717fa18..fbaa6f1 100644 --- a/billing/app/controllers/credit_card_info_controller.rb +++ b/billing/app/controllers/credit_card_info_controller.rb @@ -1,5 +1,5 @@ class CreditCardInfoController < ApplicationController - before_filter :authorize, :set_user + before_filter :require_login, :set_user def edit @credit_card = Braintree::CreditCard.find(params[:id]) diff --git a/billing/app/controllers/customer_controller.rb b/billing/app/controllers/customer_controller.rb index 901cb34..6cbcb44 100644 --- a/billing/app/controllers/customer_controller.rb +++ b/billing/app/controllers/customer_controller.rb @@ -1,5 +1,5 @@ class CustomerController < BillingBaseController - before_filter :authorize, :fetch_customer + before_filter :require_login, :fetch_customer def show if @customer diff --git a/billing/app/controllers/payments_controller.rb b/billing/app/controllers/payments_controller.rb index 0b5abe7..fce6570 100644 --- a/billing/app/controllers/payments_controller.rb +++ b/billing/app/controllers/payments_controller.rb @@ -1,5 +1,5 @@ class PaymentsController < BillingBaseController - before_filter :authorize, :only => [:index] + before_filter :require_login, :only => [:index] def new fetch_transparent_redirect diff --git a/billing/app/controllers/subscriptions_controller.rb b/billing/app/controllers/subscriptions_controller.rb index 01aaab4..f066b3c 100644 --- a/billing/app/controllers/subscriptions_controller.rb +++ b/billing/app/controllers/subscriptions_controller.rb @@ -1,5 +1,5 @@ class SubscriptionsController < BillingBaseController - before_filter :authorize + before_filter :require_login before_filter :fetch_subscription, :only => [:show, :destroy] before_filter :confirm_cancel_subscription, :only => [:destroy] before_filter :confirm_self_or_admin, :only => [:index] diff --git a/certs/app/controllers/certs_controller.rb b/certs/app/controllers/certs_controller.rb index 62ef3fd..82cbc44 100644 --- a/certs/app/controllers/certs_controller.rb +++ b/certs/app/controllers/certs_controller.rb @@ -1,6 +1,6 @@ class CertsController < ApplicationController - before_filter :login_if_required + before_filter :require_login, :unless => :anonymous_certs_allowed? # GET /cert def show @@ -10,10 +10,9 @@ class CertsController < ApplicationController protected - def login_if_required - authorize unless APP_CONFIG[:allow_anonymous_certs] + def anonymous_certs_allowed? + APP_CONFIG[:allow_anonymous_certs] end - # # this is some temporary logic until we store the service level in the user db. # diff --git a/help/app/controllers/tickets_controller.rb b/help/app/controllers/tickets_controller.rb index c193ff4..d65ee43 100644 --- a/help/app/controllers/tickets_controller.rb +++ b/help/app/controllers/tickets_controller.rb @@ -4,7 +4,7 @@ class TicketsController < ApplicationController respond_to :html, :json #has_scope :open, :type => boolean - before_filter :authorize, :only => [:index] + before_filter :require_login, :only => [:index] before_filter :fetch_ticket, :only => [:show, :update, :destroy] # don't now have an edit method before_filter :fetch_user before_filter :set_title diff --git a/users/app/controllers/controller_extension/authentication.rb b/users/app/controllers/controller_extension/authentication.rb index d831fbe..e83d6b2 100644 --- a/users/app/controllers/controller_extension/authentication.rb +++ b/users/app/controllers/controller_extension/authentication.rb @@ -15,7 +15,7 @@ module ControllerExtension::Authentication !!current_user end - def authorize + def require_login access_denied unless logged_in? end @@ -38,7 +38,7 @@ module ControllerExtension::Authentication current_user && current_user.is_admin? end - def authorize_admin + def require_admin access_denied unless admin? end diff --git a/users/app/controllers/controller_extension/token_authentication.rb b/users/app/controllers/controller_extension/token_authentication.rb index cd5c074..ee24f73 100644 --- a/users/app/controllers/controller_extension/token_authentication.rb +++ b/users/app/controllers/controller_extension/token_authentication.rb @@ -11,6 +11,10 @@ module ControllerExtension::TokenAuthentication token.authenticate if token end + def require_token + access_denied unless token + end + def logout super clear_token diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index a5461cd..6b32d49 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -4,9 +4,9 @@ class UsersController < UsersBaseController - before_filter :authorize, :only => [:show, :edit, :update, :destroy] + before_filter :require_login, :except => [:new] + before_filter :require_admin, :only => [:index, :deactivate, :enable] before_filter :fetch_user, :only => [:show, :edit, :update, :destroy, :deactivate, :enable] - before_filter :authorize_admin, :only => [:index, :deactivate, :enable] respond_to :html diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb index 0903888..a16c6e9 100644 --- a/users/app/controllers/v1/users_controller.rb +++ b/users/app/controllers/v1/users_controller.rb @@ -3,8 +3,8 @@ module V1 skip_before_filter :verify_authenticity_token before_filter :fetch_user, :only => [:update] - before_filter :authorize, :only => [:update] - before_filter :authorize_admin, :only => [:index] + before_filter :require_login, :only => [:update, :index] + before_filter :require_admin, :only => [:index] respond_to :json diff --git a/users/test/functional/application_controller_test.rb b/users/test/functional/application_controller_test.rb index 94b77bd..c4c922b 100644 --- a/users/test/functional/application_controller_test.rb +++ b/users/test/functional/application_controller_test.rb @@ -7,21 +7,21 @@ class ApplicationControllerTest < ActionController::TestCase @controller.response = @response end - def test_authorize_redirect - @controller.send(:authorize) + def test_require_login_redirect + @controller.send(:require_login) assert_access_denied(true, false) end - def test_authorized + def test_require_login login - @controller.send(:authorize) + @controller.send(:require_login) assert_access_denied(false) end - def test_authorize_admin + def test_require_admin login @current_user.expects(:is_admin?).returns(false) - @controller.send(:authorize_admin) + @controller.send(:require_admin) assert_access_denied end diff --git a/users/test/functional/v1/sessions_controller_test.rb b/users/test/functional/v1/sessions_controller_test.rb index 4200e8f..df0d681 100644 --- a/users/test/functional/v1/sessions_controller_test.rb +++ b/users/test/functional/v1/sessions_controller_test.rb @@ -36,7 +36,7 @@ class V1::SessionsControllerTest < ActionController::TestCase post :create, :login => @user.login, 'A' => @client_hex end - test "should authorize" do + test "should authenticate" do request.env['warden'].expects(:authenticate!) @controller.stubs(:current_user).returns(@user) handshake = stub(:to_hash => {h: "ash"}) diff --git a/users/test/unit/unauthenticated_user_test.rb b/users/test/unit/unauthenticated_user_test.rb new file mode 100644 index 0000000..e5fafb8 --- /dev/null +++ b/users/test/unit/unauthenticated_user_test.rb @@ -0,0 +1,7 @@ +require 'test_helper' + +class UnauthenticatedUserTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/users/test/unit/unauthorized_user_test.rb b/users/test/unit/unauthorized_user_test.rb deleted file mode 100644 index 5b96ae1..0000000 --- a/users/test/unit/unauthorized_user_test.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'test_helper' - -class UnauthorizedUserTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end -end -- cgit v1.2.3 From 88f8128d568daaaa122d55ac7e546a81ae60964a Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 7 Feb 2014 15:46:43 +0100 Subject: minor: more robust destruction of records in tests --- billing/test/integration/admin_customer_test.rb | 4 ++-- help/test/functional/tickets_controller_test.rb | 20 +++++--------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/billing/test/integration/admin_customer_test.rb b/billing/test/integration/admin_customer_test.rb index 58a7557..1b9953f 100644 --- a/billing/test/integration/admin_customer_test.rb +++ b/billing/test/integration/admin_customer_test.rb @@ -14,8 +14,8 @@ class AdminCustomerTest < ActionDispatch::IntegrationTest teardown do Warden.test_reset! - @user.destroy - @admin.destroy + @user.destroy if @user + @admin.destroy if @admin end test "check non customer as admin" do diff --git a/help/test/functional/tickets_controller_test.rb b/help/test/functional/tickets_controller_test.rb index 2530ba1..416fb73 100644 --- a/help/test/functional/tickets_controller_test.rb +++ b/help/test/functional/tickets_controller_test.rb @@ -2,6 +2,11 @@ require 'test_helper' class TicketsControllerTest < ActionController::TestCase + teardown do + # destroy all tickets that were created during the test + Ticket.all.each{|t| t.destroy} + end + test "should get index if logged in" do login get :index @@ -64,7 +69,6 @@ class TicketsControllerTest < ActionController::TestCase assert_equal 1, assigns(:ticket).comments.count assert_nil assigns(:ticket).comments.first.posted_by - assigns(:ticket).destroy # destroys without checking permission. is that okay? end @@ -87,7 +91,6 @@ class TicketsControllerTest < ActionController::TestCase assert_equal 1, assigns(:ticket).comments.count assert_not_nil assigns(:ticket).comments.first.posted_by assert_equal assigns(:ticket).comments.first.posted_by, @current_user.id - assigns(:ticket).destroy end test "add comment to unauthenticated ticket" do @@ -101,7 +104,6 @@ class TicketsControllerTest < ActionController::TestCase assert_equal ticket, assigns(:ticket) # still same ticket, with different comments assert_not_equal ticket.comments, assigns(:ticket).comments # ticket == assigns(:ticket), but they have different comments (which we want) - assigns(:ticket).destroy end @@ -118,7 +120,6 @@ class TicketsControllerTest < ActionController::TestCase assert_not_equal ticket.comments, assigns(:ticket).comments assert_not_nil assigns(:ticket).comments.last.posted_by assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id - assigns(:ticket).destroy end @@ -153,12 +154,9 @@ class TicketsControllerTest < ActionController::TestCase assert_not_equal ticket.comments, assigns(:ticket).comments assert_not_nil assigns(:ticket).comments.last.posted_by assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id - - assigns(:ticket).destroy end test "tickets by admin" do - begin other_user = find_record :user ticket = FactoryGirl.create :ticket, :created_by => other_user.id @@ -173,9 +171,6 @@ class TicketsControllerTest < ActionController::TestCase assigns(:tickets).first.save get :index, {:admin_status => "all", :open_status => "open"} end - ensure - ticket.reload.destroy if ticket - end end @@ -188,7 +183,6 @@ class TicketsControllerTest < ActionController::TestCase assert assigns(:all_tickets).include?(testticket) get :index, {:user_id => user.id, :open_status => "open"} assert !assigns(:all_tickets).include?(testticket) - testticket.destroy end test "commenting on a ticket adds to tickets that are mine" do @@ -204,8 +198,6 @@ class TicketsControllerTest < ActionController::TestCase assert assigns(:all_tickets).include?(assigns(:ticket)) assert_not_nil assigns(:ticket).comments.last.posted_by assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id - - assigns(:ticket).destroy end test "admin ticket ordering" do @@ -228,7 +220,6 @@ class TicketsControllerTest < ActionController::TestCase assert_not_equal first_tick, assigns(:all_tickets).first assert_not_equal last_tick, assigns(:all_tickets).last - tickets.each {|ticket| ticket.destroy} end test "tickets for regular user" do @@ -275,7 +266,6 @@ class TicketsControllerTest < ActionController::TestCase assert assigns(:all_tickets).include?(other_ticket) assert_equal assigns(:all_tickets).count, number_closed_tickets + number_open_tickets - assigns(:all_tickets).each {|t| t.destroy} end -- cgit v1.2.3 From 67f17e65b9e9e8ad2991b9c4002dba5203baa77f Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 8 Feb 2014 11:13:10 +0100 Subject: refactor tests to ease the testing of token only auth --- core/test/support/browser_integration_test.rb | 81 +++++++++++++++++ core/test/support/rack_test.rb | 13 +++ test/test_helper.rb | 73 ++------------- users/test/integration/api/account_flow_test.rb | 114 +++++------------------- users/test/integration/api/login_test.rb | 3 +- users/test/integration/api/pgp_key_test.rb | 35 ++++++++ users/test/integration/api/rack_test.rb | 9 -- users/test/integration/api/srp_test.rb | 83 +++++++++++++++++ users/test/support/integration_test_helper.rb | 12 --- 9 files changed, 240 insertions(+), 183 deletions(-) create mode 100644 core/test/support/browser_integration_test.rb create mode 100644 core/test/support/rack_test.rb create mode 100644 users/test/integration/api/pgp_key_test.rb delete mode 100644 users/test/integration/api/rack_test.rb create mode 100644 users/test/integration/api/srp_test.rb delete mode 100644 users/test/support/integration_test_helper.rb diff --git a/core/test/support/browser_integration_test.rb b/core/test/support/browser_integration_test.rb new file mode 100644 index 0000000..2885c3a --- /dev/null +++ b/core/test/support/browser_integration_test.rb @@ -0,0 +1,81 @@ +# +# BrowserIntegrationTest +# +# Use this class for capybara based integration tests for the ui. +# + +class BrowserIntegrationTest < ActionDispatch::IntegrationTest + + CONFIG_RU = (Rails.root + 'config.ru').to_s + OUTER_APP = Rack::Builder.parse_file(CONFIG_RU).first + + require 'capybara/poltergeist' + + Capybara.register_driver :rack_test do |app| + Capybara::RackTest::Driver.new(app) + end + + Capybara.register_driver :poltergeist do |app| + Capybara::Poltergeist::Driver.new(app) + end + + # this is integration testing. So let's make the whole + # rack stack available... + Capybara.app = OUTER_APP + Capybara.run_server = true + Capybara.app_host = 'http://lvh.me:3003' + Capybara.server_port = 3003 + Capybara.javascript_driver = :poltergeist + Capybara.default_wait_time = 5 + + + # Make the Capybara DSL available + include Capybara::DSL + + setup do + Capybara.current_driver = Capybara.javascript_driver + page.driver.add_headers 'ACCEPT-LANGUAGE' => 'en-EN' + end + + teardown do + Capybara.reset_sessions! # Forget the (simulated) browser state + Capybara.use_default_driver # Revert Capybara.current_driver to Capybara.default_driver + end + + def submit_signup(username = nil, password = nil) + username ||= "test_#{SecureRandom.urlsafe_base64}".downcase + password ||= SecureRandom.base64 + visit '/users/new' + fill_in 'Username', with: username + fill_in 'Password', with: password + fill_in 'Password confirmation', with: password + click_on 'Sign Up' + return username, password + end + + add_teardown_hook do |testcase| + unless testcase.passed? + testcase.save_state + end + end + + def save_state + page.save_screenshot screenshot_path + File.open(logfile_path, 'w') do |test_log| + test_log.puts self.class.name + test_log.puts "=========================" + test_log.puts __name__ + test_log.puts Time.now + test_log.puts current_path + test_log.puts page.status_code + test_log.puts page.response_headers + test_log.puts "page.html" + test_log.puts "------------------------" + test_log.puts page.html + test_log.puts "server log" + test_log.puts "------------------------" + test_log.puts `tail log/test.log -n 200` + end + end + +end diff --git a/core/test/support/rack_test.rb b/core/test/support/rack_test.rb new file mode 100644 index 0000000..0476cf7 --- /dev/null +++ b/core/test/support/rack_test.rb @@ -0,0 +1,13 @@ +class RackTest < ActiveSupport::TestCase + include Rack::Test::Methods + include Warden::Test::Helpers + include LeapWebCore::AssertResponses + + CONFIG_RU = (Rails.root + 'config.ru').to_s + OUTER_APP = Rack::Builder.parse_file(CONFIG_RU).first + + def app + OUTER_APP + end + +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 3fb2716..f63591f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -10,74 +10,6 @@ Dir["#{File.dirname(__FILE__)}/../*/test/support/**/*.rb"].each { |f| require f class ActiveSupport::TestCase # Add more helper methods to be used by all tests here... - def file_path(name) - File.join(Rails.root, 'test', 'files', name) - end - -end - -require 'capybara/poltergeist' - -CONFIG_RU = (Rails.root + 'config.ru').to_s -OUTER_APP = Rack::Builder.parse_file(CONFIG_RU).first - -Capybara.register_driver :rack_test do |app| - Capybara::RackTest::Driver.new(app) -end - -Capybara.register_driver :poltergeist do |app| - Capybara::Poltergeist::Driver.new(app) -end - -# this is integration testing. So let's make the whole -# rack stack available... -Capybara.app = OUTER_APP -Capybara.run_server = true -Capybara.app_host = 'http://lvh.me:3003' -Capybara.server_port = 3003 -Capybara.javascript_driver = :poltergeist -Capybara.default_wait_time = 5 - -class BrowserIntegrationTest < ActionDispatch::IntegrationTest - # Make the Capybara DSL available - include Capybara::DSL - include IntegrationTestHelper - - setup do - Capybara.current_driver = Capybara.javascript_driver - page.driver.add_headers 'ACCEPT-LANGUAGE' => 'en-EN' - end - - teardown do - Capybara.reset_sessions! # Forget the (simulated) browser state - Capybara.use_default_driver # Revert Capybara.current_driver to Capybara.default_driver - end - - add_teardown_hook do |testcase| - unless testcase.passed? - testcase.save_state - end - end - - def save_state - page.save_screenshot screenshot_path - File.open(logfile_path, 'w') do |test_log| - test_log.puts self.class.name - test_log.puts "=========================" - test_log.puts __name__ - test_log.puts Time.now - test_log.puts current_path - test_log.puts page.status_code - test_log.puts page.response_headers - test_log.puts "page.html" - test_log.puts "------------------------" - test_log.puts page.html - test_log.puts "server log" - test_log.puts "------------------------" - test_log.puts `tail log/test.log -n 200` - end - end - protected def logfile_path @@ -87,4 +19,9 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest def screenshot_path Rails.root + 'tmp' + "#{self.class.name.underscore}.#{__name__}.png" end + + def file_path(name) + File.join(Rails.root, 'test', 'files', name) + end + end diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb index edd0859..b56d07b 100644 --- a/users/test/integration/api/account_flow_test.rb +++ b/users/test/integration/api/account_flow_test.rb @@ -1,50 +1,10 @@ require 'test_helper' -require_relative 'rack_test' +require_relative 'srp_test' -class AccountFlowTest < RackTest +class AccountFlowTest < SrpTest setup do - @login = "integration_test_user" - Identity.find_by_address(@login + '@' + APP_CONFIG[:domain]).tap{|i| i.destroy if i} - User.find_by_login(@login).tap{|u| u.destroy if u} - @password = "srp, verify me!" - @srp = SRP::Client.new @login, :password => @password - @user_params = { - :login => @login, - :password_verifier => @srp.verifier.to_s(16), - :password_salt => @srp.salt.to_s(16) - } - post 'http://api.lvh.me:3000/1/users.json', :user => @user_params - @user = User.find_by_login(@login) - end - - teardown do - if @user.reload - @user.identity.destroy - @user.destroy - end - Warden.test_reset! - end - - # this test wraps the api and implements the interface the ruby-srp client. - def handshake(login, aa) - post "http://api.lvh.me:3000/1/sessions.json", - :login => login, - 'A' => aa, - :format => :json - response = JSON.parse(last_response.body) - if response['errors'] - raise RECORD_NOT_FOUND.new(response['errors']) - else - return response['B'] - end - end - - def validate(m) - put "http://api.lvh.me:3000/1/sessions/" + @login + '.json', - :client_auth => m, - :format => :json - return JSON.parse(last_response.body) + register_user end test "signup response" do @@ -53,25 +13,22 @@ class AccountFlowTest < RackTest end test "signup and login with srp via api" do - server_auth = @srp.authenticate(self) + authenticate assert last_response.successful? assert_nil server_auth["errors"] assert server_auth["M2"] end test "signup and wrong password login attempt" do - srp = SRP::Client.new @login, :password => "wrong password" - server_auth = srp.authenticate(self) + authenticate password: "wrong password" assert_json_error "base" => "Not a valid username/password combination" assert !last_response.successful? assert_nil server_auth["M2"] end test "signup and wrong username login attempt" do - srp = SRP::Client.new "wrong_login", :password => @password - server_auth = nil assert_raises RECORD_NOT_FOUND do - server_auth = srp.authenticate(self) + authenticate login: "wrong login" end assert_json_error "base" => "Not a valid username/password combination" assert !last_response.successful? @@ -79,58 +36,31 @@ class AccountFlowTest < RackTest end test "update password via api" do - @srp.authenticate(self) - @password = "No! Verify me instead." - @srp = SRP::Client.new @login, :password => @password - @user_params = { - # :login => @login, - :password_verifier => @srp.verifier.to_s(16), - :password_salt => @srp.salt.to_s(16) - } - put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', - :user => @user_params, - :format => :json - server_auth = @srp.authenticate(self) + authenticate + update_user password: "No! Verify me instead." + authenticate assert last_response.successful? assert_nil server_auth["errors"] assert server_auth["M2"] end + test "change login with password_verifier" do + authenticate + new_login = 'zaph' + cleanup_user new_login + update_user login: new_login, password: @password + assert last_response.successful? + assert_equal new_login, @user.reload.login + end + test "prevent changing login without changing password_verifier" do - server_auth = @srp.authenticate(self) + authenticate original_login = @user.login new_login = 'zaph' - User.find_by_login(new_login).try(:destroy) - Identity.by_address.key(new_login + '@' + APP_CONFIG[:domain]).each do |identity| - identity.destroy - end - put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:login => new_login}, :format => :json + cleanup_user new_login + update_user login: new_login assert last_response.successful? # does not change login if no password_verifier is present - assert_equal original_login, @user.login - end - - test "upload pgp key" do - server_auth = @srp.authenticate(self) - key = FactoryGirl.build :pgp_key - put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => key}, :format => :json - assert_equal key, Identity.for(@user).keys[:pgp] + assert_equal original_login, @user.reload.login end - - # eventually probably want to remove most of this into a non-integration - # functional test - test "prevent uploading invalid key" do - server_auth = @srp.authenticate(self) - put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => :blah}, :format => :json - assert_nil Identity.for(@user).keys[:pgp] - end - - test "prevent emptying public key" do - server_auth = @srp.authenticate(self) - key = FactoryGirl.build :pgp_key - put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => key}, :format => :json - put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', :user => {:public_key => ""}, :format => :json - assert_equal key, Identity.for(@user).keys[:pgp] - end - end diff --git a/users/test/integration/api/login_test.rb b/users/test/integration/api/login_test.rb index fb761e5..a760d38 100644 --- a/users/test/integration/api/login_test.rb +++ b/users/test/integration/api/login_test.rb @@ -1,7 +1,6 @@ require 'test_helper' -require_relative 'rack_test' -class AccountFlowTest < RackTest +class LoginTest < RackTest setup do @login = "integration_test_user" diff --git a/users/test/integration/api/pgp_key_test.rb b/users/test/integration/api/pgp_key_test.rb new file mode 100644 index 0000000..4c7fb4c --- /dev/null +++ b/users/test/integration/api/pgp_key_test.rb @@ -0,0 +1,35 @@ +require 'test_helper' +require_relative 'srp_test' + +class PgpKeyTest < SrpTest + + setup do + # todo: prepare user and login without doing the srp dance + register_user + authenticate + end + + test "upload pgp key" do + update_user public_key: key + 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_nil Identity.for(@user).keys[:pgp] + end + + test "prevent emptying public key" do + update_user public_key: key + update_user public_key: "" + assert_equal key, Identity.for(@user).keys[:pgp] + end + + protected + + def key + @key ||= FactoryGirl.build :pgp_key + end +end diff --git a/users/test/integration/api/rack_test.rb b/users/test/integration/api/rack_test.rb deleted file mode 100644 index 9a69f52..0000000 --- a/users/test/integration/api/rack_test.rb +++ /dev/null @@ -1,9 +0,0 @@ -class RackTest < ActiveSupport::TestCase - include Rack::Test::Methods - include Warden::Test::Helpers - include LeapWebCore::AssertResponses - - def app - OUTER_APP - end -end diff --git a/users/test/integration/api/srp_test.rb b/users/test/integration/api/srp_test.rb new file mode 100644 index 0000000..b291269 --- /dev/null +++ b/users/test/integration/api/srp_test.rb @@ -0,0 +1,83 @@ +class SrpTest < RackTest + + teardown do + if @user + cleanup_user + end + Warden.test_reset! + end + + # this test wraps the api and implements the interface the ruby-srp client. + def handshake(login, aa) + post "http://api.lvh.me:3000/1/sessions.json", + :login => login, + 'A' => aa, + :format => :json + response = JSON.parse(last_response.body) + if response['errors'] + raise RECORD_NOT_FOUND.new(response['errors']) + else + return response['B'] + end + end + + def validate(m) + put "http://api.lvh.me:3000/1/sessions/" + @login + '.json', + :client_auth => m, + :format => :json + return JSON.parse(last_response.body) + end + + protected + + attr_reader :server_auth + + def register_user(login = "integration_test_user", password = 'srp, verify me!') + cleanup_user(login) + post 'http://api.lvh.me:3000/1/users.json', + user: user_params(login: login, password: password), + format: :json + @user = User.find_by_login(login) + @login = login + @password = password + end + + def update_user(params) + put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', + :user => user_params(params), + :format => :json + end + + def authenticate(params = nil) + @server_auth = srp(params).authenticate(self) + end + + def cleanup_user(login = nil) + login ||= @user.login + Identity.by_address.key(login + '@' + APP_CONFIG[:domain]).each do |identity| + identity.destroy + end + if user = User.find_by_login(login) + user.destroy + end + end + + def user_params(params) + # if there is no srp magic needed just return the params + return params unless params.keys.include?(:password) + params.reverse_merge! login: @login, salt: @salt + @srp = SRP::Client.new params[:login], password: params.delete(:password) + @salt = srp.salt.to_s(16) + params.merge :password_verifier => srp.verifier.to_s(16), + :password_salt => @salt + end + + def srp(params = nil) + if params.nil? + @srp + else + params.reverse_merge! password: @password + SRP::Client.new(params.delete(:login) || @login, params) + end + end +end diff --git a/users/test/support/integration_test_helper.rb b/users/test/support/integration_test_helper.rb deleted file mode 100644 index 51e47c6..0000000 --- a/users/test/support/integration_test_helper.rb +++ /dev/null @@ -1,12 +0,0 @@ -module IntegrationTestHelper - def submit_signup(username = nil, password = nil) - username ||= "test_#{SecureRandom.urlsafe_base64}".downcase - password ||= SecureRandom.base64 - visit '/users/new' - fill_in 'Username', with: username - fill_in 'Password', with: password - fill_in 'Password confirmation', with: password - click_on 'Sign Up' - return username, password - end -end -- cgit v1.2.3 From 758b9a3c30a73fd985943fb7a887f0373be3a833 Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 8 Feb 2014 12:29:08 +0100 Subject: split up and expand account integration test --- core/lib/extensions/testing.rb | 2 + core/test/support/rack_test.rb | 24 +++++++++ users/test/integration/api/account_flow_test.rb | 66 ----------------------- users/test/integration/api/login_test.rb | 38 +++++++++++-- users/test/integration/api/signup_test.rb | 20 +++++++ users/test/integration/api/srp_test.rb | 5 ++ users/test/integration/api/update_account_test.rb | 44 +++++++++++++++ 7 files changed, 128 insertions(+), 71 deletions(-) delete mode 100644 users/test/integration/api/account_flow_test.rb create mode 100644 users/test/integration/api/signup_test.rb create mode 100644 users/test/integration/api/update_account_test.rb diff --git a/core/lib/extensions/testing.rb b/core/lib/extensions/testing.rb index aad7fc1..d9b6da8 100644 --- a/core/lib/extensions/testing.rb +++ b/core/lib/extensions/testing.rb @@ -22,6 +22,8 @@ module LeapWebCore end def assert_json_response(object) + assert_equal 'application/json', + get_response.content_type.split(';').first if object.is_a? Hash object.stringify_keys! if object.respond_to? :stringify_keys! assert_equal object, json_response diff --git a/core/test/support/rack_test.rb b/core/test/support/rack_test.rb index 0476cf7..2d8e5c4 100644 --- a/core/test/support/rack_test.rb +++ b/core/test/support/rack_test.rb @@ -10,4 +10,28 @@ 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) + # RackTest does not know @response + response_code = last_response.status + message ||= "Expected response to be a <#{type}>, but was <#{response_code}>" + + if Symbol === type + if [:success, :missing, :redirect, :error].include?(type) + assert last_response.send("#{type}?"), message + else + code = Rack::Utils::SYMBOL_TO_STATUS_CODE[type] + assert_equal code, response_code, message + end + else + assert_equal type, response_code, message + end + end + end diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb deleted file mode 100644 index b56d07b..0000000 --- a/users/test/integration/api/account_flow_test.rb +++ /dev/null @@ -1,66 +0,0 @@ -require 'test_helper' -require_relative 'srp_test' - -class AccountFlowTest < SrpTest - - setup do - register_user - end - - test "signup response" do - assert_json_response :login => @login, :ok => true - assert last_response.successful? - end - - test "signup and login with srp via api" do - authenticate - assert last_response.successful? - assert_nil server_auth["errors"] - assert server_auth["M2"] - end - - test "signup and wrong password login attempt" do - authenticate password: "wrong password" - assert_json_error "base" => "Not a valid username/password combination" - assert !last_response.successful? - assert_nil server_auth["M2"] - end - - test "signup and wrong username login attempt" do - assert_raises RECORD_NOT_FOUND do - authenticate login: "wrong login" - end - assert_json_error "base" => "Not a valid username/password combination" - assert !last_response.successful? - assert_nil server_auth - end - - test "update password via api" do - authenticate - update_user password: "No! Verify me instead." - authenticate - assert last_response.successful? - assert_nil server_auth["errors"] - assert server_auth["M2"] - end - - test "change login with password_verifier" do - authenticate - new_login = 'zaph' - cleanup_user new_login - update_user login: new_login, password: @password - assert last_response.successful? - assert_equal new_login, @user.reload.login - end - - test "prevent changing login without changing password_verifier" do - authenticate - original_login = @user.login - new_login = 'zaph' - cleanup_user new_login - update_user login: new_login - assert last_response.successful? - # does not change login if no password_verifier is present - assert_equal original_login, @user.reload.login - end -end diff --git a/users/test/integration/api/login_test.rb b/users/test/integration/api/login_test.rb index a760d38..82219d0 100644 --- a/users/test/integration/api/login_test.rb +++ b/users/test/integration/api/login_test.rb @@ -1,15 +1,43 @@ require 'test_helper' +require_relative 'srp_test' -class LoginTest < RackTest +class LoginTest < SrpTest setup do - @login = "integration_test_user" + register_user end - test "require json requests" do - put "http://api.lvh.me:3000/1/sessions/" + @login, - :client_auth => "This is not a valid login anyway" + test "requires handshake before validation" do + validate("bla") assert_json_error login: I18n.t(:all_strategies_failed) end + test "login with srp" do + authenticate + assert last_response.successful? + assert_nil server_auth["errors"] + assert server_auth["M2"] + end + + test "wrong password login attempt" do + authenticate password: "wrong password" + assert_json_error "base" => "Not a valid username/password combination" + assert !last_response.successful? + assert_nil server_auth["M2"] + end + + test "wrong username login attempt" do + assert_raises RECORD_NOT_FOUND do + authenticate login: "wrong login" + end + assert_json_error "base" => "Not a valid username/password combination" + assert !last_response.successful? + assert_nil server_auth + end + + test "logout" do + authenticate + logout + assert_equal 204, last_response.status + end end diff --git a/users/test/integration/api/signup_test.rb b/users/test/integration/api/signup_test.rb new file mode 100644 index 0000000..236c547 --- /dev/null +++ b/users/test/integration/api/signup_test.rb @@ -0,0 +1,20 @@ +require 'test_helper' +require_relative 'srp_test' + +class SignupTest < SrpTest + + setup do + register_user + end + + test "signup response" do + assert_json_response :login => @login, :ok => true + assert last_response.successful? + end + + test "signup creates user" do + assert @user + assert_equal @login, @user.login + end +end + diff --git a/users/test/integration/api/srp_test.rb b/users/test/integration/api/srp_test.rb index b291269..bb24f5f 100644 --- a/users/test/integration/api/srp_test.rb +++ b/users/test/integration/api/srp_test.rb @@ -52,6 +52,11 @@ class SrpTest < RackTest @server_auth = srp(params).authenticate(self) end + def logout + delete "http://api.lvh.me:3000/1/logout.json", + format: :json + end + def cleanup_user(login = nil) login ||= @user.login Identity.by_address.key(login + '@' + APP_CONFIG[:domain]).each do |identity| diff --git a/users/test/integration/api/update_account_test.rb b/users/test/integration/api/update_account_test.rb new file mode 100644 index 0000000..16c2357 --- /dev/null +++ b/users/test/integration/api/update_account_test.rb @@ -0,0 +1,44 @@ +require 'test_helper' +require_relative 'srp_test' + +class UpdateAccountTest < SrpTest + + setup do + register_user + end + + test "require authentication" do + update_user password: "No! Verify me instead." + assert_access_denied + end + + test "update password via api" do + authenticate + update_user password: "No! Verify me instead." + authenticate + assert last_response.successful? + assert_nil server_auth["errors"] + assert server_auth["M2"] + end + + test "change login with password_verifier" do + authenticate + new_login = 'zaph' + cleanup_user new_login + update_user login: new_login, password: @password + authenticate + assert last_response.successful? + assert_equal new_login, @user.reload.login + end + + test "prevent changing login without changing password_verifier" do + authenticate + original_login = @user.login + new_login = 'zaph' + cleanup_user new_login + update_user login: new_login + assert last_response.successful? + # does not change login if no password_verifier is present + assert_equal original_login, @user.reload.login + end +end -- cgit v1.2.3 From cbd757cf151cd61bfdd5637d09f43e4831fec3bb Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 8 Feb 2014 16:15:46 +0100 Subject: require token when updating user via API --- users/app/controllers/v1/users_controller.rb | 2 +- users/test/integration/api/login_test.rb | 1 + users/test/integration/api/srp_test.rb | 29 +++++++++++++++++------ users/test/integration/api/update_account_test.rb | 7 ++++++ 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb index a16c6e9..8897d01 100644 --- a/users/app/controllers/v1/users_controller.rb +++ b/users/app/controllers/v1/users_controller.rb @@ -3,8 +3,8 @@ module V1 skip_before_filter :verify_authenticity_token before_filter :fetch_user, :only => [:update] - before_filter :require_login, :only => [:update, :index] before_filter :require_admin, :only => [:index] + before_filter :require_token, :only => [:update] respond_to :json diff --git a/users/test/integration/api/login_test.rb b/users/test/integration/api/login_test.rb index 82219d0..d56dfd1 100644 --- a/users/test/integration/api/login_test.rb +++ b/users/test/integration/api/login_test.rb @@ -14,6 +14,7 @@ class LoginTest < SrpTest test "login with srp" do authenticate + assert_equal ["M2", "id", "token"], server_auth.keys assert last_response.successful? assert_nil server_auth["errors"] assert server_auth["M2"] diff --git a/users/test/integration/api/srp_test.rb b/users/test/integration/api/srp_test.rb index bb24f5f..fcda187 100644 --- a/users/test/integration/api/srp_test.rb +++ b/users/test/integration/api/srp_test.rb @@ -35,8 +35,7 @@ class SrpTest < RackTest def register_user(login = "integration_test_user", password = 'srp, verify me!') cleanup_user(login) post 'http://api.lvh.me:3000/1/users.json', - user: user_params(login: login, password: password), - format: :json + user_params(login: login, password: password) @user = User.find_by_login(login) @login = login @password = password @@ -44,14 +43,25 @@ class SrpTest < RackTest def update_user(params) put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', - :user => user_params(params), - :format => :json + user_params(params), + auth_headers end def authenticate(params = nil) @server_auth = srp(params).authenticate(self) end + def auth_headers + return {} if @server_auth.nil? + { + "HTTP_AUTHORIZATION" => encoded_token + } + end + + def encoded_token + ActionController::HttpAuthentication::Token.encode_credentials(server_auth["token"]) + end + def logout delete "http://api.lvh.me:3000/1/logout.json", format: :json @@ -68,12 +78,17 @@ class SrpTest < RackTest end def user_params(params) - # if there is no srp magic needed just return the params - return params unless params.keys.include?(:password) + if params.keys.include?(:password) + srp_process_password(params) + end + return { user: params, format: :json } + end + + def srp_process_password(params) params.reverse_merge! login: @login, salt: @salt @srp = SRP::Client.new params[:login], password: params.delete(:password) @salt = srp.salt.to_s(16) - params.merge :password_verifier => srp.verifier.to_s(16), + params.merge! :password_verifier => srp.verifier.to_s(16), :password_salt => @salt end diff --git a/users/test/integration/api/update_account_test.rb b/users/test/integration/api/update_account_test.rb index 16c2357..63429e7 100644 --- a/users/test/integration/api/update_account_test.rb +++ b/users/test/integration/api/update_account_test.rb @@ -12,6 +12,13 @@ class UpdateAccountTest < SrpTest assert_access_denied end + test "require token" do + authenticate + put "http://api.lvh.me:3000/1/users/" + @user.id + '.json', + user_params(password: "No! Verify me instead.") + assert_access_denied + end + test "update password via api" do authenticate update_user password: "No! Verify me instead." -- cgit v1.2.3 From c8fcd0d26c3ad5c1c3cfbaf6b57239f907925ed6 Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 8 Feb 2014 16:20:37 +0100 Subject: require token when logging out via API --- users/app/controllers/v1/sessions_controller.rb | 1 + users/test/integration/api/login_test.rb | 6 ++++++ users/test/integration/api/srp_test.rb | 5 +++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/users/app/controllers/v1/sessions_controller.rb b/users/app/controllers/v1/sessions_controller.rb index eb6c322..eae3a1e 100644 --- a/users/app/controllers/v1/sessions_controller.rb +++ b/users/app/controllers/v1/sessions_controller.rb @@ -2,6 +2,7 @@ module V1 class SessionsController < ApplicationController skip_before_filter :verify_authenticity_token + before_filter :require_token, only: :destroy def new @session = Session.new diff --git a/users/test/integration/api/login_test.rb b/users/test/integration/api/login_test.rb index d56dfd1..92d153f 100644 --- a/users/test/integration/api/login_test.rb +++ b/users/test/integration/api/login_test.rb @@ -41,4 +41,10 @@ class LoginTest < SrpTest logout assert_equal 204, last_response.status end + + test "logout requires token" do + authenticate + logout(nil, {}) + assert_equal 422, last_response.status + end end diff --git a/users/test/integration/api/srp_test.rb b/users/test/integration/api/srp_test.rb index fcda187..946450e 100644 --- a/users/test/integration/api/srp_test.rb +++ b/users/test/integration/api/srp_test.rb @@ -62,9 +62,10 @@ class SrpTest < RackTest ActionController::HttpAuthentication::Token.encode_credentials(server_auth["token"]) end - def logout + def logout(params=nil, headers=nil) delete "http://api.lvh.me:3000/1/logout.json", - format: :json + params || {format: :json}, + headers || auth_headers end def cleanup_user(login = nil) -- cgit v1.2.3 From 3a478804aa48b08fbeded5144677744c427c112f Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 10 Feb 2014 14:29:34 +0100 Subject: require token in messages controller --- users/app/controllers/v1/messages_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/users/app/controllers/v1/messages_controller.rb b/users/app/controllers/v1/messages_controller.rb index 1b994ca..90986e2 100644 --- a/users/app/controllers/v1/messages_controller.rb +++ b/users/app/controllers/v1/messages_controller.rb @@ -2,7 +2,7 @@ module V1 class MessagesController < ApplicationController skip_before_filter :verify_authenticity_token - before_filter :authorize + before_filter :require_token respond_to :json -- cgit v1.2.3 From b6c8279a39f933257be11fc29f5b7d59efff743f Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 10 Feb 2014 14:34:17 +0100 Subject: require_token now checks for token and login --- users/app/controllers/controller_extension/token_authentication.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/users/app/controllers/controller_extension/token_authentication.rb b/users/app/controllers/controller_extension/token_authentication.rb index ee24f73..6e0a6ce 100644 --- a/users/app/controllers/controller_extension/token_authentication.rb +++ b/users/app/controllers/controller_extension/token_authentication.rb @@ -8,11 +8,11 @@ module ControllerExtension::TokenAuthentication end def token_authenticate - token.authenticate if token + @token_authenticated ||= token.authenticate if token end def require_token - access_denied unless token + access_denied unless token_authenticate end def logout -- cgit v1.2.3 From b4719619aabbe9ebf74563b62e1eb8e4fb248c21 Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 10 Feb 2014 15:01:11 +0100 Subject: ensure we are working on a string as the content type --- core/lib/extensions/testing.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/extensions/testing.rb b/core/lib/extensions/testing.rb index d9b6da8..8f7e73c 100644 --- a/core/lib/extensions/testing.rb +++ b/core/lib/extensions/testing.rb @@ -23,7 +23,7 @@ module LeapWebCore def assert_json_response(object) assert_equal 'application/json', - get_response.content_type.split(';').first + get_response.content_type.to_s.split(';').first if object.is_a? Hash object.stringify_keys! if object.respond_to? :stringify_keys! assert_equal object, json_response -- cgit v1.2.3