From 86eb9062f1e81302647bf18ce0f5fd981202b68a Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 13 May 2014 09:51:36 +0200 Subject: allow for usernames with dots preparing for #5664 with some test improvements i ran into this issue This commit includes a fix and the test improvements. In particular it adds BrowserIntegrationTest#login - so there is no need to go through the signup procedure everytime you want a user to be logged in. --- app/controllers/v1/sessions_controller.rb | 2 +- app/models/identity.rb | 6 ++++++ app/models/token.rb | 4 ++++ config/routes.rb | 3 ++- test/integration/browser/account_test.rb | 27 +++++++++++++++------------ test/integration/browser/session_test.rb | 20 +++++--------------- test/support/browser_integration_test.rb | 16 ++++++++++++++++ 7 files changed, 49 insertions(+), 29 deletions(-) diff --git a/app/controllers/v1/sessions_controller.rb b/app/controllers/v1/sessions_controller.rb index eae3a1e..d88fcdc 100644 --- a/app/controllers/v1/sessions_controller.rb +++ b/app/controllers/v1/sessions_controller.rb @@ -38,7 +38,7 @@ module V1 def login_response handshake = session.delete(:handshake) || {} - handshake.to_hash.merge(:id => current_user.id, :token => @token.id) + handshake.to_hash.merge(:id => current_user.id, :token => @token.to_s) end end diff --git a/app/models/identity.rb b/app/models/identity.rb index 9b97b51..ad8c01e 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -70,6 +70,12 @@ class Identity < CouchRest::Model::Base end end + def self.destroy_all_for(user) + Identity.by_user_id.key(user.id).each do |identity| + identity.destroy + end + end + def self.destroy_all_disabled Identity.disabled.each do |identity| identity.destroy diff --git a/app/models/token.rb b/app/models/token.rb index 4856c31..e759ee3 100644 --- a/app/models/token.rb +++ b/app/models/token.rb @@ -30,6 +30,10 @@ class Token < CouchRest::Model::Base end end + def to_s + id + end + def authenticate if expired? destroy diff --git a/config/routes.rb b/config/routes.rb index f612b47..745b97d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -20,7 +20,8 @@ LeapWeb::Application.routes.draw do namespace "api", { module: "v1", path: "/1/", defaults: {format: 'json'} } do - resources :sessions, :only => [:new, :create, :update] + resources :sessions, :only => [:new, :create, :update], + :constraints => { :id => /[^\/]+(?=\.json\z)|[^\/]+/ } delete "logout" => "sessions#destroy", :as => "logout" resources :users, :only => [:create, :update, :destroy, :index] resources :messages, :only => [:index, :update] diff --git a/test/integration/browser/account_test.rb b/test/integration/browser/account_test.rb index 4e11520..491a9e1 100644 --- a/test/integration/browser/account_test.rb +++ b/test/integration/browser/account_test.rb @@ -6,7 +6,7 @@ class AccountTest < BrowserIntegrationTest Identity.destroy_all_disabled end - test "normal account workflow" do + test "signup successfully" do username, password = submit_signup assert page.has_content?("Welcome #{username}") click_on 'Logout' @@ -16,6 +16,12 @@ class AccountTest < BrowserIntegrationTest user.account.destroy end + test "signup with username ending in dot json" do + username = Faker::Internet.user_name + '.json' + submit_signup username + assert page.has_content?("Welcome #{username}") + end + test "successful login" do username, password = submit_signup click_on 'Logout' @@ -51,7 +57,7 @@ class AccountTest < BrowserIntegrationTest end test "default user actions" do - username, password = submit_signup + login click_on "Account Settings" assert page.has_content? I18n.t('destroy_my_account') assert page.has_no_css? '#update_login_and_password' @@ -59,8 +65,8 @@ class AccountTest < BrowserIntegrationTest end test "default admin actions" do - username, password = submit_signup - with_config admins: [username] do + login + with_config admins: [@user.login] do click_on "Account Settings" assert page.has_content? I18n.t('destroy_my_account') assert page.has_no_css? '#update_login_and_password' @@ -70,7 +76,7 @@ class AccountTest < BrowserIntegrationTest test "change password" do with_config user_actions: ['change_password'] do - username, password = submit_signup + login click_on "Account Settings" within('#update_login_and_password') do fill_in 'Password', with: "other password" @@ -78,16 +84,15 @@ class AccountTest < BrowserIntegrationTest click_on 'Save' end click_on 'Logout' - attempt_login(username, "other password") - assert page.has_content?("Welcome #{username}") - User.find_by_login(username).account.destroy + attempt_login(@user.login, "other password") + assert page.has_content?("Welcome #{@user.login}") end end test "change pgp key" do with_config user_actions: ['change_pgp_key'] do pgp_key = FactoryGirl.build :pgp_key - username, password = submit_signup + login click_on "Account Settings" within('#update_pgp_key') do fill_in 'Public key', with: pgp_key @@ -97,9 +102,7 @@ class AccountTest < BrowserIntegrationTest # at some point we're done: page.assert_no_selector 'input[value="Saving..."]' assert page.has_field? 'Public key', with: pgp_key.to_s - user = User.find_by_login(username) - assert_equal pgp_key, user.public_key - user.account.destroy + assert_equal pgp_key, @user.reload.public_key end end diff --git a/test/integration/browser/session_test.rb b/test/integration/browser/session_test.rb index 3a41b3a..fb20847 100644 --- a/test/integration/browser/session_test.rb +++ b/test/integration/browser/session_test.rb @@ -2,26 +2,16 @@ require 'test_helper' class SessionTest < BrowserIntegrationTest - setup do - @username, password = submit_signup - end - - teardown do - user = User.find_by_login(@username) - id = user.identity - id.destroy - user.destroy - end - test "valid session" do - assert page.has_content?("Welcome #{@username}") + login + assert page.has_content?("Logout") end test "expired session" do - assert page.has_content?("Welcome #{@username}") - pretend_now_is(Time.now + 40.minutes) do + login + pretend_now_is(Time.now + 80.minutes) do visit '/' - assert page.has_no_content?("Welcome #{@username}") + assert page.has_content?("Log In") end end end diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb index 9cae8cb..836eb63 100644 --- a/test/support/browser_integration_test.rb +++ b/test/support/browser_integration_test.rb @@ -53,6 +53,22 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest return username, password end + # currently this only works for tests with poltergeist. + def login(user = nil) + user ||= @user ||= FactoryGirl.create(:user) + token = Token.create user_id: user.id + page.driver.add_header "Authorization", + 'Token token="' + token.to_s + '"' + visit '/' + end + + teardown do + if @user && @user.reload + Identity.destroy_all_for @user + @user.destroy + end + end + add_teardown_hook do |testcase| unless testcase.passed? testcase.save_state -- cgit v1.2.3