diff --git a/tracks/app/controllers/application.rb b/tracks/app/controllers/application.rb
index db8bcee4..6477d384 100644
--- a/tracks/app/controllers/application.rb
+++ b/tracks/app/controllers/application.rb
@@ -71,6 +71,10 @@ class ApplicationController < ActionController::Base
end
end
+ def redirect_back_or_home
+ redirect_back_or_default home_url
+ end
+
private
def parse_date_per_user_prefs( s )
diff --git a/tracks/app/controllers/login_controller.rb b/tracks/app/controllers/login_controller.rb
index 2aa68589..3675cb02 100644
--- a/tracks/app/controllers/login_controller.rb
+++ b/tracks/app/controllers/login_controller.rb
@@ -18,7 +18,7 @@ class LoginController < ApplicationController
msg = (should_expire_sessions?) ? "will expire after 1 hour of inactivity." : "will not expire."
notify :notice, "Login successful: session #{msg}"
cookies[:tracks_login] = { :value => @user.login, :expires => Time.now + 1.year }
- redirect_back_or_default home_url
+ redirect_back_or_home
else
@login = params['user_login']
notify :warning, "Login unsuccessful"
@@ -69,7 +69,7 @@ class LoginController < ApplicationController
unless (@user.nil?)
notify :notice, "You have successfully verified #{open_id_response.identity_url} as your identity."
session['user_id'] = @user.id
- redirect_back_or_default home_path
+ redirect_back_or_home
else
notify :warning, "You have successfully verified #{open_id_response.identity_url} as your identity, but you do not have a Tracks account. Please ask your administrator to sign you up."
end
@@ -83,47 +83,6 @@ class LoginController < ApplicationController
redirect_to :action => 'login' unless performed?
end
- def signup
- if User.no_users_yet?
- @page_title = "Sign up as the admin user"
- @user = get_new_user
- elsif @user && @user.is_admin?
- @page_title = "Sign up a new user"
- @user = get_new_user
- else # all other situations (i.e. a non-admin is logged in, or no one is logged in, but we have some users)
- @page_title = "No signups"
- @admin_email = User.find_admin.preference.admin_email
- render :action => "nosignup"
- end
- end
-
- def create
- user = User.new(params['user'])
- unless user.valid?
- session['new_user'] = user
- redirect_to :controller => 'login', :action => 'signup'
- return
- end
-
- user.is_admin = true if User.no_users_yet?
- if user.save
- @user = User.authenticate(user.login, params['user']['password'])
- @user.create_preference
- @user.save
- notify :notice, "Signup successful for user #{@user.login}."
- redirect_back_or_default home_url
- end
- end
-
- def delete
- if params['id'] and ( params['id'] == @user.id or @user.is_admin )
- @user = User.find(params['id'])
- # TODO: Maybe it would be better to mark deleted. That way user deletes can be reversed.
- @user.destroy
- end
- redirect_back_or_default home_url
- end
-
def logout
session['user_id'] = nil
reset_session
@@ -150,17 +109,7 @@ class LoginController < ApplicationController
end
private
-
- def get_new_user
- if session['new_user']
- user = session['new_user']
- session['new_user'] = nil
- else
- user = User.new
- end
- user
- end
-
+
def should_expire_sessions?
session['noexpiry'] != "on"
end
diff --git a/tracks/app/controllers/users_controller.rb b/tracks/app/controllers/users_controller.rb
index 64ba72d8..82f9693f 100644
--- a/tracks/app/controllers/users_controller.rb
+++ b/tracks/app/controllers/users_controller.rb
@@ -5,20 +5,33 @@ class UsersController < ApplicationController
before_filter :begin_open_id_auth, :only => :update_auth_type
end
- before_filter :admin_login_required, :only => [ :index, :create, :destroy ]
+ before_filter :admin_login_required, :only => [ :index, :destroy ]
def index
+ @page_title = "TRACKS::Manage Users"
@user_pages, @users = paginate :users, :order => 'login ASC', :per_page => 10
@total_users = User.find(:all).size
- # When we call login/signup from the admin page
+ # When we call users/signup from the admin page
# we store the URL so that we get returned here when signup is successful
+ expires_now
store_location
end
-
- # verify :method => :post,
- # :only => %w( create ),
- # :render => { :text => '403 Forbidden: Only POST requests on this resource are allowed.',
- # :status => 403 }
+
+ def new
+ if User.no_users_yet?
+ @page_title = "Sign up as the admin user"
+ @user = get_new_user
+ elsif @user && @user.is_admin?
+ @page_title = "Sign up a new user"
+ @user = get_new_user
+ else # all other situations (i.e. a non-admin is logged in, or no one is logged in, but we have some users)
+ @page_title = "No signups"
+ @admin_email = User.find_admin.preference.admin_email
+ render :action => "nosignup", :layout => "login"
+ return
+ end
+ render :layout => "login"
+ end
# Example usage: curl -H 'Accept: application/xml' -H 'Content-Type: application/xml'
# -u admin:up2n0g00d
@@ -26,28 +39,56 @@ class UsersController < ApplicationController
# http://our.tracks.host/users
#
def create
- if params['exception']
- render_failure "Expected post format is valid xml like so:
You are now logged into the system...
-- Since you are here it's safe to assume the application never called store_location, otherwise - you would have been redirected somewhere else after a successful login. -
- - <%= link_to "« logout", :action=>"logout"%> - -<%= link_to 'Signup new user', :controller => 'login', :action => 'signup' %>
\ No newline at end of file +<%= link_to 'Signup new user', signup_path %>
\ No newline at end of file diff --git a/tracks/app/views/login/signup.rhtml b/tracks/app/views/users/new.rhtml similarity index 100% rename from tracks/app/views/login/signup.rhtml rename to tracks/app/views/users/new.rhtml diff --git a/tracks/app/views/login/nosignup.rhtml b/tracks/app/views/users/nosignup.rhtml similarity index 100% rename from tracks/app/views/login/nosignup.rhtml rename to tracks/app/views/users/nosignup.rhtml diff --git a/tracks/config/routes.rb b/tracks/config/routes.rb index 62608d87..8a11edb1 100644 --- a/tracks/config/routes.rb +++ b/tracks/config/routes.rb @@ -21,12 +21,14 @@ ActionController::Routing::Routes.draw do |map| # Login Routes map.connect 'login', :controller => 'login', :action => 'login' map.connect 'logout', :controller => 'login', :action => 'logout' - map.connect 'signup', :controller => 'login', :action => 'signup' map.resources :users, :member => {:change_password => :get, :update_password => :post, :change_auth_type => :get, :update_auth_type => :post, :refresh_token => :post } + map.with_options :controller => "users" do |users| + users.signup 'signup', :action => "new" + end # ToDo Routes map.resources :todos, @@ -37,7 +39,7 @@ ActionController::Routing::Routes.draw do |map| todos.tickler 'tickler', :action => "list_deferred" todos.done 'done', :action => "completed" todos.done_archive 'done/archive', :action => "completed_archive" - todos.tag '/todos/tag/:name', :action => "tag" + todos.tag 'todos/tag/:name', :action => "tag" end # Context Routes diff --git a/tracks/test/functional/login_controller_test.rb b/tracks/test/functional/login_controller_test.rb index dba99187..4171fc84 100644 --- a/tracks/test/functional/login_controller_test.rb +++ b/tracks/test/functional/login_controller_test.rb @@ -75,79 +75,15 @@ class LoginControllerTest < Test::Unit::TestCase assert_equal "Login unsuccessful", flash[:warning] assert_response :success end - - # ============================================ - # Signup and creation of new users - # ============================================ - - # Test signup of a new user by admin - # Check that newly created user can log in - # - def test_create - admin = login('admin', 'abracadabra', 'on') - assert admin.is_admin - newbie = create('newbie', 'newbiepass') - assert_equal "Signup successful for user newbie.", flash[:notice] - assert_redirected_to home_url - assert_valid newbie - get :logout # logout the admin user - assert_equal newbie.login, "newbie" - assert newbie.is_admin == false || newbie.is_admin == 0 - assert_not_nil newbie.preference # have user preferences been created? - user = login('newbie', 'newbiepass', 'on') # log in the new user - assert_redirected_to home_url - assert_equal 'newbie', user.login - assert user.is_admin == false || user.is_admin == 0 - assert_equal User.count, @num_users_in_fixture + 1 - end - - # Test whether signup of new users is denied to a non-admin user - # - def test_create_by_non_admin - non_admin = login('jane', 'sesame', 'on') - assert non_admin.is_admin == false || non_admin.is_admin == 0 - post :signup, :user => {:login => 'newbie2', :password => 'newbiepass2', :password_confirmation => 'newbiepass2'} - assert_template 'login/nosignup' - assert_number_of_users_is_unchanged - end - - # ============================================ - # Test validations - # ============================================ - - def test_create_with_invalid_password - admin = login('admin', 'abracadabra', 'on') - assert admin.is_admin - assert_equal admin.id, @response.session['user_id'] - post :create, :user => {:login => 'newbie', :password => '', :password_confirmation => ''} - assert_number_of_users_is_unchanged - assert_redirected_to :controller => 'login', :action => 'signup' - end - - def test_create_with_invalid_user - admin = login('admin', 'abracadabra', 'on') - assert admin.is_admin - assert_equal admin.id, @response.session['user_id'] - post :create, :user => {:login => 'n', :password => 'newbiepass', :password_confirmation => 'newbiepass'} - assert_number_of_users_is_unchanged - assert_redirected_to :controller => 'login', :action => 'signup' - end - - # Test uniqueness of login - # - def test_validate_uniqueness_of_login - admin = login('admin', 'abracadabra', 'on') - assert admin.is_admin - assert_equal admin.id, @response.session['user_id'] - post :create, :user => {:login => 'jane', :password => 'newbiepass', :password_confirmation => 'newbiepass'} - num_users = User.find(:all) - assert_number_of_users_is_unchanged - assert_redirected_to :controller => 'login', :action => 'signup' - end private - def assert_number_of_users_is_unchanged - assert_equal User.count, @num_users_in_fixture + # Logs in a user and returns the user object found in the session object + def login(login,password,expiry) + post :login, {:user_login => login, :user_password => password, :user_noexpiry => expiry} + assert_not_nil(session['user_id']) + return User.find(session['user_id']) end + + end diff --git a/tracks/test/functional/users_controller_test.rb b/tracks/test/functional/users_controller_test.rb index 6a040f93..286747f9 100644 --- a/tracks/test/functional/users_controller_test.rb +++ b/tracks/test/functional/users_controller_test.rb @@ -6,7 +6,7 @@ require 'user' class UsersController; def rescue_action(e) raise e end; end class UsersControllerTest < Test::Unit::TestCase - fixtures :users + fixtures :preferences, :users def setup assert_equal "test", ENV['RAILS_ENV'] @@ -14,6 +14,9 @@ class UsersControllerTest < Test::Unit::TestCase @controller = UsersController.new @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new + @num_users_in_fixture = User.count + @admin_user = users(:admin_user) + @nonadmin_user = users(:other_user) end def test_get_index_when_not_logged_in @@ -22,20 +25,20 @@ class UsersControllerTest < Test::Unit::TestCase end def test_get_index_by_nonadmin - @request.session['user_id'] = users(:other_user).id + login_as @nonadmin_user get :index - assert_redirected_to home_path + assert_response 401 end def test_get_index_by_admin - @request.session['user_id'] = users(:admin_user).id + login_as @admin_user get :index assert_response :success end def test_destroy_user + login_as @admin_user @no_users_before = User.find(:all).size - @request.session['user_id'] = users(:admin_user).id xhr :post, :destroy, :id => 3 assert_rjs :page, "user-3", :remove assert_equal @no_users_before-1, User.find(:all).size @@ -44,7 +47,7 @@ class UsersControllerTest < Test::Unit::TestCase def test_update_password_successful get :change_password # should fail because no login assert_redirected_to :controller => 'login', :action => 'login' - @request.session['user_id'] = users(:admin_user).id # log in the admin user + login_as @admin_user @user = @request.session['user_id'] get :change_password # should now pass because we're logged in assert_response :success @@ -59,7 +62,7 @@ class UsersControllerTest < Test::Unit::TestCase def test_update_password_no_confirmation post :update_password # should fail because no login assert_redirected_to :controller => 'login', :action => 'login' - @request.session['user_id'] = users(:admin_user).id # log in the admin user + login_as @admin_user post :update_password, :updateuser => {:password => 'newpassword', :password_confirmation => 'wrong'} assert_redirected_to :controller => 'users', :action => 'change_password' assert users(:admin_user).save, false @@ -69,7 +72,7 @@ class UsersControllerTest < Test::Unit::TestCase def test_update_password_validation_errors post :update_password # should fail because no login assert_redirected_to :controller => 'login', :action => 'login' - @request.session['user_id'] = users(:admin_user).id # log in the admin user + login_as @admin_user post :update_password, :updateuser => {:password => 'ba', :password_confirmation => 'ba'} assert_redirected_to :controller => 'users', :action => 'change_password' assert users(:admin_user).save, false @@ -79,4 +82,79 @@ class UsersControllerTest < Test::Unit::TestCase assert_equal flash[:warning], 'There was a problem saving the password. Please retry.' end + # ============================================ + # Signup and creation of new users + # ============================================ + + def test_create + login_as @admin_user + newbie = create('newbie', 'newbiepass') + assert_equal "Signup successful for user newbie.", flash[:notice], "expected flash notice not found" + assert_redirected_to home_url + assert_valid newbie + session['user_id'] = nil # logout the admin user + assert_equal newbie.login, "newbie" + assert newbie.is_admin == false || newbie.is_admin == 0 + assert_not_nil newbie.preference # have user preferences been created? + assert_not_nil User.authenticate('newbie', 'newbiepass') + assert_equal User.count, @num_users_in_fixture + 1 + end + + # Test whether signup of new users is denied to a non-admin user + # + def test_create_by_non_admin + non_admin = login_as @nonadmin_user + post :create, :user => {:login => 'newbie2', :password => 'newbiepass2', :password_confirmation => 'newbiepass2'} + assert_response :success + assert_template 'users/nosignup' + assert_number_of_users_is_unchanged + end + + # ============================================ + # Test validations + # ============================================ + + def test_create_with_invalid_password + login_as @admin_user + post :create, :user => {:login => 'newbie', :password => '', :password_confirmation => ''} + assert_number_of_users_is_unchanged + assert_redirected_to :controller => 'users', :action => 'new' + end + + def test_create_with_invalid_user + login_as @admin_user + post :create, :user => {:login => 'n', :password => 'newbiepass', :password_confirmation => 'newbiepass'} + assert_number_of_users_is_unchanged + assert_redirected_to :controller => 'users', :action => 'new' + end + + # Test uniqueness of login + # + def test_validate_uniqueness_of_login + login_as @admin_user + post :create, :user => {:login => 'jane', :password => 'newbiepass', :password_confirmation => 'newbiepass'} + num_users = User.find(:all) + assert_number_of_users_is_unchanged + assert_redirected_to :controller => 'users', :action => 'new' + end + + private + + def login_as(user) + returning user do |u| + @request.session['user_id'] = u.id + end + end + + # Creates a new users with the login and password given + def create(login,password) + post :create, :user => {:login => login, :password => password, :password_confirmation => password} + return User.find_by_login(login) + end + + def assert_number_of_users_is_unchanged + assert_equal User.count, @num_users_in_fixture + end + + end diff --git a/tracks/test/integration/stories_test.rb b/tracks/test/integration/stories_test.rb index 80b918b4..4cdb1482 100644 --- a/tracks/test/integration/stories_test.rb +++ b/tracks/test/integration/stories_test.rb @@ -49,17 +49,17 @@ class StoriesTest < ActionController::IntegrationTest def goes_to_signup get "/signup" assert_response :success - assert_template "login/signup" + assert_template "users/new" end def goes_to_signup_as_nonadmin get "/signup" assert_response :success - assert_template "login/nosignup" + assert_template "users/nosignup" end def signs_up_with(options) - post "/login/create", options + post "/users", options assert_response :redirect follow_redirect! assert_response :success diff --git a/tracks/test/test_helper.rb b/tracks/test/test_helper.rb index 801d59df..f2d21701 100644 --- a/tracks/test/test_helper.rb +++ b/tracks/test/test_helper.rb @@ -16,22 +16,6 @@ class Test::Unit::TestCase # Instantiated fixtures are slow, but give you @david where you otherwise would need people(:david) self.use_instantiated_fixtures = false - - # Add more helper methods to be used by all tests here... - # Logs in a user and returns the user object found in the session object - # - def login(login,password,expiry) - post :login, {:user_login => login, :user_password => password, :user_noexpiry => expiry} - assert_not_nil(session['user_id']) - return User.find(session['user_id']) - end - - # Creates a new users with the login and password given - def create(login,password) - post :create, :user => {:login => login, :password => password, :password_confirmation => password} - return User.find_by_login(login) - end - # Generates a random string of ascii characters (a-z, "1 0") # of a given length for testing assignment to fields