From a41b28ae231e1e591fcb46ec27652fb5b9cdb979 Mon Sep 17 00:00:00 2001 From: Ari Timonen Date: Sat, 21 Mar 2020 22:33:44 +0200 Subject: [PATCH] Update uers_contoller and add almost working spec. --- Gemfile | 4 + Gemfile.lock | 1 + app/controllers/application_controller.rb | 2 +- app/controllers/users_controller.rb | 57 +++-- app/models/user.rb | 5 +- config/routes.rb | 14 +- spec/controllers/users_controller_spec.rb | 262 +++++++++++++++++++++- spec/factories/user.rb | 2 +- 8 files changed, 307 insertions(+), 40 deletions(-) diff --git a/Gemfile b/Gemfile index 9cab76e..6ca30ac 100644 --- a/Gemfile +++ b/Gemfile @@ -104,7 +104,11 @@ group :test do gem 'test-unit' gem 'timecop' + # FOr JS test + gem 'mime-types' + # Use dev versions because of rspec bug + gem 'rspec-core', git: 'https://github.com/rspec/rspec-core' gem 'rspec-expectations', git: 'https://github.com/rspec/rspec-expectations' gem 'rspec-mocks', git: 'https://github.com/rspec/rspec-mocks' diff --git a/Gemfile.lock b/Gemfile.lock index c320b78..4f5b61c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -464,6 +464,7 @@ DEPENDENCIES i18n-js i18n_country_select jquery-rails + mime-types mysql2 neat (~> 1.6.0) newrelic_rpm diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3e7cfdc..95a9261 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -45,7 +45,7 @@ class ApplicationController < ActionController::Base end rescue_from Error do |exception| - render text: exception.message, layout: true + render text: exception.message, layout: true, status: 500 end rescue_from ActiveRecord::StaleObjectError do |exception| diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0e7bc5a..0b2a367 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2,9 +2,11 @@ class UsersController < ApplicationController before_action :get_user, only: [:show, :history, :popup, :agenda, :edit, :update, :destroy] respond_to :html, :js + PAGES = ["general", "favorites", "computer", "articles", "movies", "teams", "matches", "predictions", "comments"] + def index search = params[:search] - if search && search.match(/^ip:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/) && cuser && cuser.admin? + if search && search.match(/^ip:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/) && cuser&.admin? @users = User.where(lastip: $1).paginate(per_page: 40, page: params[:page]) else if params[:filter] == 'lately' @@ -19,26 +21,25 @@ class UsersController < ApplicationController @page = "general" respond_to do |format| format.js do - pages = ["general", "favorites", "computer", "articles", "movies", "teams", "matches", "predictions", "comments"] - if pages.include?(params[:page]) - @page = params[:page] - end + @page = params[:page] if self.PAGES.include?(params[:page]) end format.html {} end end + # FIXME: consider merging + def popup + render layout: false + end + def agenda + raise AccessError unless @user == cuser or cuser&.admin? @teamer = Teamer.new @teamer.user = @user end def history - raise AccessError unless cuser and cuser.admin? - end - - def popup - render layout: false + raise AccessError unless cuser&.admin? end def new @@ -72,7 +73,7 @@ class UsersController < ApplicationController end def update - raise AccessError unless @user.can_update? cuser + raise AccessError unless @user.can_update? cuser # FIXME: use permit params[:user].delete(:username) unless @user.can_change_name? cuser if @user.update_attributes(User.params(params, cuser, "update")) @@ -89,37 +90,35 @@ class UsersController < ApplicationController redirect_to users_url end + # FIXME: maybe move to session controller def login - return unless request.post? - - if u = User.authenticate(params[:login][:username].downcase, params[:login][:password]) - raise Error, t(:accounts_locked) if u.banned? Ban::TYPE_SITE - - flash[:notice] = t(:login_successful) - save_session u - - if session[:return_to] - return_to + if params[:login] && (u = User.authenticate(params[:login])) + if u.banned? Ban::TYPE_SITE + flash[:notice] = t(:accounts_locked) else - redirect_to_back + flash[:notice] = t(:login_successful) + save_session u end else flash[:error] = t(:login_unsuccessful) + end + # FIXME: check return on rails 6 + if session[:return_to] + return_to + else redirect_to_back end end def logout - if request.post? - session[:user] = nil - flash[:notice] = t(:login_out) - redirect_to :root - end + session[:user] = nil + flash[:notice] = t(:login_out) + redirect_to :root end def forgot if request.post? - if u = User.first(:conditions => {:username => params[:username], :email => params[:email]}) and u.send_new_password + if (user1 = User.where(username: params[:username], email: params[:email]).first) && user1.send_new_password flash[:notice] = t(:passwords_sent) else flash[:error] = t(:incorrect_information) @@ -137,6 +136,6 @@ class UsersController < ApplicationController session[:user] = user.id user.lastip = request.ip user.lastvisit = DateTime.now - user.save() + user.save end end diff --git a/app/models/user.rb b/app/models/user.rb index 492cf6e..54bdf6f 100755 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -299,6 +299,7 @@ class User < ActiveRecord::Base self.time_zone = "Amsterdam" end + # Password should store password and password_hash shoulds store def update_password self.password = Digest::MD5.hexdigest(raw_password) if raw_password and raw_password.length > 0 end @@ -329,8 +330,8 @@ class User < ActiveRecord::Base cuser and cuser.admin? end - def self.authenticate(username, password) - where("LOWER(username) = LOWER(?)", username).where(:password => Digest::MD5.hexdigest(password)).first + def self.authenticate(login) + where("LOWER(username) = LOWER(?)", login[:username]).where(password: Digest::MD5.hexdigest(login[:password])).first end def self.get id diff --git a/config/routes.rb b/config/routes.rb index 0fe4365..6036298 100755 --- a/config/routes.rb +++ b/config/routes.rb @@ -32,6 +32,8 @@ Ensl::Application.routes.draw do resources :categories resources :options resources :polls + resources :custom_urls, only: [:create, :update, :destroy] + resources :brackets get 'comments/quote' @@ -51,7 +53,12 @@ Ensl::Application.routes.draw do get 'forums/down' resources :forums - resources :users + resources :users do + collection do + get 'forgot' + post 'forgot' + end + end resources :locks resources :contesters @@ -84,8 +91,6 @@ Ensl::Application.routes.draw do get :quote end - resources :brackets - get 'about/action' get 'about/staff' get 'about/adminpanel' @@ -133,13 +138,10 @@ Ensl::Application.routes.draw do get 'users/login' get 'users/logout' get 'users/popup' - get 'users/forgot', to: "users#forgot" get 'votes/create' get "polls/showvotes/:id", to: "polls#showvotes", as: "polls_showvotes" - resources :custom_urls, only: [:create, :update, :destroy] - get "custom_urls", to: "custom_urls#administrate" get ':controller/:action', requirements: { action: /A-Za-z/ } diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 0d67fd8..719723e 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1,3 +1,263 @@ -RSpec.describe UsersController, type: :controller do +require 'rails_helper' +require 'mime/types' +RSpec.describe UsersController, type: :controller do + let!(:params) { FactoryBot.attributes_for(:user) } + let!(:invalid_params) { params.merge(:steamid => (50..150).map { (65 + rand(26)).chr }.join) } + let!(:admin) { create(:user, :admin) } + let!(:user) { create(:user) } + + before :all do + create(:user) + end + + # TODO: check flash + + describe 'GET #index' do + it "renders the template" do + get :index + expect(response).to render_template("index") + end + + it "assigns users" do + get :index + # TODO + expect(assigns(:users)) + end + + # TODO Test pagination + search + end + + describe 'GET #popup' do + context 'with valid user' do + it "renders the template" do + login_admin + get :popup, params: {:id => user.id}, xhr: true + expect(response).to render_template("popup") + end + # Check for pages TODO + end + end + + describe 'GET #agenda' do + context 'with valid user' do + it "renders the template" do + login user.username + get :agenda, params: {:id => user.id} + expect(response).to render_template("agenda") + end + end + + context 'with admin access' do + it "renders the template" do + login_admin + get :agenda, params: {:id => user.id} + expect(response).to render_template("agenda") + end + # Check for pages TODO + end + + context 'with valid user access another user' do + it "respond with 403" do + user2 = create(:user) + login user.username + get :agenda, params: {:id => user2.id} + expect(response).to have_http_status(:forbidden) + end + end + end + + describe 'GET #history' do + context 'with admin access' do + it "renders the template" do + login_admin + get :history, params: {:id => user.id} + expect(response).to render_template("history") + end + # Check for pages TODO + end + + context 'without admin access' do + it "respond with 403" do + user2 = create(:user) + login user.username + get :history, params: {:id => user2.id} + expect(response).to have_http_status(:forbidden) + end + end + end + + describe 'GET #edit' do + let!(:user) { create(:user) } + + it "renders the template" do + login_admin + get :edit, params: {id: user.id} + expect(response).to render_template("edit") + end + end + + context 'POST' do + before(:each) do + expect(:params).not_to eq(:invalid_params) + end + + describe 'with valid values' do + it "creates the model" do + login_admin + post :create, params: {:user => params} + # user.any_instance.should_receive(:update_attributes).with(params) + # FIXME: ignore lastvisit and raw_password + expect(User.last).to have_attributes(params.except(:raw_password,)) + end + + it "redirects correctly" do + login_admin + post :create, params: {:user => params} + expect(response).to redirect_to(user_path(User.last)) + end + end + + describe 'with invalid values' do + it "does not create the model" do + login_admin + count = User.count + post :create, params: {:user => invalid_params} + # user.any_instance.should_receive(:update_attributes).with(params) + expect(User.count).to eq(count) + end + + it "renders :new" do + login_admin + post :create, params: {:user => invalid_params} + expect(response).to render_template("new") + end + end + end + + context 'PUT' do + describe 'with valid values' do + it "updates the model" do + login_admin + params = FactoryBot.attributes_for(:user) + put :update, params: {:id => user.id, :user => params} + # user.any_instance.should_receive(:update_attributes).with(params) + expect(User.find(user.id).attributes).not_to eq(user.attributes) + end + + it "redirects correctly" do + login_admin + request.env["HTTP_REFERER"] = "where_i_came_from" + put :update, params: {:id => user.id, :user => params} + + expect(response).to redirect_to("where_i_came_from") + end + end + + describe 'with invalid values' do + it "does not update the model" do + login_admin + put :update, params: {:id => user.id, :user => invalid_params} + expect(User.find(user.id).attributes).to eq(user.attributes) + end + + it "renders :edit" do + login_admin + put :update, params: {:id => user.id, :user => invalid_params} + expect(response).to render_template("edit") + end + end + end + + context 'DELETE' do + describe 'with valid parameters' do + it "deletes the model" do + login_admin + count = User.count + delete :destroy, params: {:id => user.id} + + expect(User.where(id: user.id).count).to eq(0) + expect(User.count).to eq(count - 1) + # user.any_instance.should_receive(:update_attributes).with(params) + end + + it "redirects correctly" do + login_admin + request.env["HTTP_REFERER"] = "where_i_came_from" + delete :destroy, params: {:id => user.id} + + expect(response).to redirect_to(users_path()) + end + end + + describe 'without access' do + it "does not delete the model" do + login(user.username) + count = User.all.count + delete :destroy, params: {:id => user.id} + + expect(User.count).to eq(count) + end + end + end + + context 'POST #login' do + describe 'with valid values' do + it "set the session ID (logs in)" do + post :login, params: {login: {username: user.username, password: user.raw_password}} + expect(session[:user]).to eq(user.id) + end + + # TODO + #expect(User).to have_received(:authenticate).with(params[:login]) + + it "redirects correctly" do + request.env["HTTP_REFERER"] = "where_i_came_from" + post :login, params: {login: {username: user.username, password: user.raw_password}} + expect(response).to redirect_to("where_i_came_from") + end + end + + describe 'with invalid values' do + it "fails to set the session ID" do + post :login, params: {login: {username: user.username, password: user.raw_password + "foo"}} + expect(session[:user]).not_to eq(user.id) + end + end + + describe 'banned accounts cannot log in' do + it "fails to set the session ID" do + ban = create(:ban, user: user) + post :login, params: {login: {username: user.username, password: user.raw_password}} + expect(session[:user]).not_to eq(user.id) + end + end + end + + context 'GET #forgot' do + describe 'with valid values' do + it "renders the template" do + get :forgot + expect(response).to render_template("forgot") + end + end + end + + context 'POST #forgot' do + describe 'with valid values' do + it "renders the template" do + #TODO: mock this function + post :forgot, params: {username: user.username, email: user.email} + expect(response).to render_template("forgot") + end + + it "calls the function" do + # User.any_instance.stub(:send_new_password).and_return(true) + allow_any_instance_of(User).to receive(:send_new_password).and_return(true) + post :forgot, params: {username: user.username, email: user.email} + # FIXME + # expect_any_instance_of(User).to receive(:send_new_password) + end + end + end end \ No newline at end of file diff --git a/spec/factories/user.rb b/spec/factories/user.rb index a5d8bd3..1d4e6e3 100755 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -8,7 +8,7 @@ FactoryBot.define do lastname "Player" country "EU" raw_password "PasswordABC123" - lastvisit "Sun, 15 Mar 2020 13:31:06 +0000" + # lastvisit "Sun, 15 Mar 2020 13:31:06 +0000" after(:create) do |user| create(:profile, user: user)