diff options
-rw-r--r-- | app/controllers/application_controller.rb | 13 | ||||
-rw-r--r-- | config/defaults.yml | 2 | ||||
-rw-r--r-- | core/app/views/common/_home_page_buttons.html.haml | 2 | ||||
-rw-r--r-- | test/functional/error_handling_test.rb | 22 | ||||
-rw-r--r-- | users/app/controllers/controller_extension/token_authentication.rb | 2 | ||||
-rw-r--r-- | users/app/controllers/email_aliases_controller.rb | 12 | ||||
-rw-r--r-- | users/app/controllers/sessions_controller.rb | 10 | ||||
-rw-r--r-- | users/app/controllers/v1/users_controller.rb | 1 | ||||
-rw-r--r-- | users/app/models/token.rb | 30 | ||||
-rw-r--r-- | users/config/routes.rb | 1 | ||||
-rw-r--r-- | users/test/functional/sessions_controller_test.rb | 21 | ||||
-rw-r--r-- | users/test/functional/test_helpers_test.rb | 2 | ||||
-rw-r--r-- | users/test/integration/browser/account_test.rb | 25 | ||||
-rw-r--r-- | users/test/support/auth_test_helper.rb | 2 | ||||
-rw-r--r-- | users/test/unit/token_test.rb | 33 |
15 files changed, 121 insertions, 57 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9734a33..b808e1c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -7,6 +7,19 @@ class ApplicationController < ActionController::Base protected + + rescue_from StandardError do |e| + respond_to do |format| + format.json { render_json_error } + format.all { raise e } # reraise the exception so the normal thing happens. + end + end + + def render_json_error + render status: 500, + json: {error: "The server failed to process your request. We'll look into it."} + end + # # Allows us to pass through bold text to flash messages. See format_flash() for where this is reversed. # diff --git a/config/defaults.yml b/config/defaults.yml index 910fbf8..8d81668 100644 --- a/config/defaults.yml +++ b/config/defaults.yml @@ -16,6 +16,8 @@ cert_options: &cert_options common: &common force_ssl: false pagination_size: 30 + auth: + token_expires_after: 60 development: <<: *dev_ca diff --git a/core/app/views/common/_home_page_buttons.html.haml b/core/app/views/common/_home_page_buttons.html.haml index 82a5cc2..7eb4c40 100644 --- a/core/app/views/common/_home_page_buttons.html.haml +++ b/core/app/views/common/_home_page_buttons.html.haml @@ -9,7 +9,7 @@ .span3 .row-fluid.second .login.span4 - %span.link= link_to(icon('ok-sign', icon_color) + t(:login), new_session_path, :class => 'btn') + %span.link= link_to(icon('ok-sign', icon_color) + t(:login), login_path, :class => 'btn') %span.info= t(:login_info) .signup.span4 %span.link= link_to(icon('user', icon_color) + t(:signup), new_user_path, :class => 'btn') diff --git a/test/functional/error_handling_test.rb b/test/functional/error_handling_test.rb new file mode 100644 index 0000000..47e44ce --- /dev/null +++ b/test/functional/error_handling_test.rb @@ -0,0 +1,22 @@ +require 'test_helper' + +class ErrorHandlingTest < ActionController::TestCase + tests HomeController + + def setup + HomeController.any_instance.stubs(:index).raises + end + + def test_json_error + get :index, format: :json + assert_equal 'application/json', @response.content_type + assert json = JSON.parse(@response.body) + assert_equal ['error'], json.keys + end + + def test_html_error_reraises + assert_raises RuntimeError do + get :index + end + end +end diff --git a/users/app/controllers/controller_extension/token_authentication.rb b/users/app/controllers/controller_extension/token_authentication.rb index 3e2816d..530294a 100644 --- a/users/app/controllers/controller_extension/token_authentication.rb +++ b/users/app/controllers/controller_extension/token_authentication.rb @@ -5,7 +5,7 @@ module ControllerExtension::TokenAuthentication authenticate_with_http_token do |token_id, options| @token = Token.find(token_id) end - @token.user if @token + @token.authenticate if @token end def logout diff --git a/users/app/controllers/email_aliases_controller.rb b/users/app/controllers/email_aliases_controller.rb deleted file mode 100644 index c90432f..0000000 --- a/users/app/controllers/email_aliases_controller.rb +++ /dev/null @@ -1,12 +0,0 @@ -class EmailAliasesController < UsersBaseController - before_filter :fetch_user - - def destroy - @alias = @user.email_aliases.delete(params[:id]) - if @user.save - flash[:notice] = t(:email_alias_destroyed_successfully, :alias => bold(@alias)) - end - redirect_to edit_user_email_settings_path(@user) #TODO: this path doesn't exist. will want to add path for identities controller - end - -end diff --git a/users/app/controllers/sessions_controller.rb b/users/app/controllers/sessions_controller.rb index d6c455b..0494b51 100644 --- a/users/app/controllers/sessions_controller.rb +++ b/users/app/controllers/sessions_controller.rb @@ -8,16 +8,6 @@ class SessionsController < ApplicationController end end - def create - logout if logged_in? - authenticate! - end - - def update - authenticate! - render :json => session.delete(:handshake) - end - def destroy logout redirect_to root_path diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb index b271152..01a1a2f 100644 --- a/users/app/controllers/v1/users_controller.rb +++ b/users/app/controllers/v1/users_controller.rb @@ -8,6 +8,7 @@ module V1 respond_to :json + # used for autocomplete for admins in the web ui def index if params[:query] @users = User.by_login.startkey(params[:query]).endkey(params[:query].succ) diff --git a/users/app/models/token.rb b/users/app/models/token.rb index 3de0059..dd87344 100644 --- a/users/app/models/token.rb +++ b/users/app/models/token.rb @@ -4,11 +4,41 @@ class Token < CouchRest::Model::Base belongs_to :user + # timestamps! does not create setters and only sets updated_at + # if the object has changed and been saved. Instead of triggering + # that we rather use our own property we have control over: + property :last_seen_at, Time, accessible: false + validates :user_id, presence: true + def authenticate + if expired? + destroy + return nil + else + touch + return user + end + end + + def touch + self.last_seen_at = Time.now + save + end + + def expired? + expires_after and + last_seen_at + expires_after.minutes < Time.now + end + + def expires_after + APP_CONFIG[:auth] && APP_CONFIG[:auth][:token_expires_after] + end + def initialize(*args) super self.id = SecureRandom.urlsafe_base64(32).gsub(/^_*/, '') + self.last_seen_at = Time.now end design do diff --git a/users/config/routes.rb b/users/config/routes.rb index d4d5933..ccecfd5 100644 --- a/users/config/routes.rb +++ b/users/config/routes.rb @@ -10,7 +10,6 @@ Rails.application.routes.draw do get "login" => "sessions#new", :as => "login" delete "logout" => "sessions#destroy", :as => "logout" - resources :sessions, :only => [:new, :create, :update] get "signup" => "users#new", :as => "signup" resources :users, :except => [:create, :update] do diff --git a/users/test/functional/sessions_controller_test.rb b/users/test/functional/sessions_controller_test.rb index b22c3a3..a630e6e 100644 --- a/users/test/functional/sessions_controller_test.rb +++ b/users/test/functional/sessions_controller_test.rb @@ -41,27 +41,6 @@ class SessionsControllerTest < ActionController::TestCase assert_json_error :login => I18n.t(:all_strategies_failed) end - # Warden takes care of parsing the params and - # rendering the response. So not much to test here. - test "should perform handshake" do - request.env['warden'].expects(:authenticate!) - # make sure we don't get a template missing error: - @controller.stubs(:render) - post :create, :login => @user.login, 'A' => @client_hex - end - - test "should authorize" do - request.env['warden'].expects(:authenticate!) - handshake = stub(:to_json => "JSON") - session[:handshake] = handshake - - post :update, :id => @user.login, :client_auth => @client_hex - - assert_nil session[:handshake] - assert_response :success - assert_json_response handshake - end - test "logout should reset warden user" do expect_warden_logout delete :destroy diff --git a/users/test/functional/test_helpers_test.rb b/users/test/functional/test_helpers_test.rb index 9bd01ad..845e516 100644 --- a/users/test/functional/test_helpers_test.rb +++ b/users/test/functional/test_helpers_test.rb @@ -21,7 +21,7 @@ class TestHelpersTest < ActionController::TestCase def test_login_stubs_token login assert @token - assert_equal @current_user, @token.user + assert_equal @current_user, @token.authenticate end def test_login_adds_token_header diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb index 3df0794..8b214a4 100644 --- a/users/test/integration/browser/account_test.rb +++ b/users/test/integration/browser/account_test.rb @@ -7,13 +7,7 @@ class AccountTest < BrowserIntegrationTest end test "normal account workflow" do - 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' + username, password = submit_signup assert page.has_content?("Welcome #{username}") click_on 'Logout' assert page.has_content?("Sign Up") @@ -37,6 +31,23 @@ class AccountTest < BrowserIntegrationTest user.destroy end + test "reports internal server errors" do + V1::UsersController.any_instance.stubs(:create).raises + submit_signup + assert page.has_content?("server failed") + end + + def submit_signup + 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 + def inject_malicious_js page.execute_script <<-EOJS var calc = new srp.Calculate(); diff --git a/users/test/support/auth_test_helper.rb b/users/test/support/auth_test_helper.rb index 47147fc..609f115 100644 --- a/users/test/support/auth_test_helper.rb +++ b/users/test/support/auth_test_helper.rb @@ -41,7 +41,7 @@ module AuthTestHelper protected def header_for_token_auth - @token = find_record(:token, :user => @current_user) + @token = find_record(:token, :authenticate => @current_user) ActionController::HttpAuthentication::Token.encode_credentials @token.id end end diff --git a/users/test/unit/token_test.rb b/users/test/unit/token_test.rb index bff6b71..f56c576 100644 --- a/users/test/unit/token_test.rb +++ b/users/test/unit/token_test.rb @@ -1,19 +1,20 @@ require 'test_helper' class ClientCertificateTest < ActiveSupport::TestCase + include StubRecordHelper setup do - @user = FactoryGirl.create(:user) + @user = find_record :user end teardown do - @user.destroy end test "new token for user" do sample = Token.new(:user_id => @user.id) assert sample.valid? assert_equal @user.id, sample.user_id + assert_equal @user, sample.authenticate end test "token id is secure" do @@ -34,4 +35,32 @@ class ClientCertificateTest < ActiveSupport::TestCase assert !sample.valid?, "Token should require a user record" end + test "token updates timestamps" do + sample = Token.new(user_id: @user.id) + sample.last_seen_at = 1.minute.ago + sample.expects(:save) + assert_equal @user, sample.authenticate + assert Time.now - sample.last_seen_at < 1.minute, "last_seen_at has not been updated" + end + + test "token will not expire if token_expires_after is not set" do + sample = Token.new(user_id: @user.id) + sample.last_seen_at = 2.years.ago + with_config auth: {} do + sample.expects(:save) + assert_equal @user, sample.authenticate + end + end + + test "expired token returns nil on authenticate" do + sample = Token.new(user_id: @user.id) + sample.last_seen_at = 2.hours.ago + with_config auth: {token_expires_after: 60} do + sample.expects(:destroy) + assert_nil sample.authenticate + end + end + + + end |