diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d7a40d4..cc04a07 100755 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -36,7 +36,7 @@ class UsersController < ApplicationController message = true if user.save! respond_to do |format| format.html { redirect_to user_account_settings_path(user_id: current_user.id) } - format.json { render :json => {:msg => message ? "success" : "false "} } + format.json { render json: {msg: message ? "success" : "false "} } end else flash[:error] = "Could not update user!" diff --git a/app/models/user.rb b/app/models/user.rb index 1c6e585..baa2342 100755 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -10,7 +10,7 @@ class User < ApplicationRecord validates_presence_of :email validates_uniqueness_of :email - validates_format_of :email, :with => /.+@.+\..+/i + validates_format_of :email, with: /.+@.+\..+/i has_one :retirement, dependent: :destroy has_one :paid_time_off, dependent: :destroy @@ -57,9 +57,10 @@ class User < ApplicationRecord end def generate_token(column) - begin + loop do self[column] = Encryption.encrypt_sensitive_value(self.id) - end while User.exists?(column => self[column]) + break unless User.exists?(column => self[column]) + end self.save! end diff --git a/db/migrate/20171007010129_remove_users_user_id.rb b/db/migrate/20171007010129_remove_users_user_id.rb index ba43e03..52775ed 100644 --- a/db/migrate/20171007010129_remove_users_user_id.rb +++ b/db/migrate/20171007010129_remove_users_user_id.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class RemoveUsersUserId < ActiveRecord::Migration[5.1] def change remove_column :users, :user_id, :integer diff --git a/db/seeds.rb b/db/seeds.rb index 4d81fac..338f956 100755 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -14,12 +14,12 @@ users = [ }, { - :email => "jmmastey@metacorp.com", - :admin => false, - :password => "railsgoat!", - :password_confirmation => "railsgoat!", - :first_name => "Joseph", - :last_name => "Mastey", + email: "jmmastey@metacorp.com", + admin: false, + password: "railsgoat!", + password_confirmation: "railsgoat!", + first_name: "Joseph", + last_name: "Mastey", }, { @@ -252,25 +252,25 @@ messages = [ { creator: "ken@metacorp.com", receiver: "jack@metacorp.com", - message: 'Your benefits have been updated.', + message: "Your benefits have been updated.", read: false }, { creator: "mike@metacorp.com", receiver: "jim@metacorp.com", - message: 'Please update your profile.', + message: "Please update your profile.", read: false }, { creator: "jim@metacorp.com", receiver: "mike@metacorp.com", - message: 'Welcome to Railsgoat.', + message: "Welcome to Railsgoat.", read: false }, { creator: "jack@metacorp.com", receiver: "ken@metacorp.com", - message: 'Hello friend.', + message: "Hello friend.", read: false } ] diff --git a/spec/support/user_fixture.rb b/spec/support/user_fixture.rb index 6c9b445..e89cdda 100644 --- a/spec/support/user_fixture.rb +++ b/spec/support/user_fixture.rb @@ -6,11 +6,11 @@ class UserFixture end def self.normal_user - password = 'thi$ 1s cOmplExEr' - User.create!(first_name: 'Joe', last_name: 'Schmoe', email: 'joe@schmoe.com', + password = "thi$ 1s cOmplExEr" + User.create!(first_name: "Joe", last_name: "Schmoe", email: "joe@schmoe.com", password: password, password_confirmation: password).tap do |user| def user.clear_password - 'thi$ 1s cOmplExEr' + "thi$ 1s cOmplExEr" end end end diff --git a/spec/vulnerabilities/insecure_dor_spec.rb b/spec/vulnerabilities/insecure_dor_spec.rb index f64c80b..e0743ea 100644 --- a/spec/vulnerabilities/insecure_dor_spec.rb +++ b/spec/vulnerabilities/insecure_dor_spec.rb @@ -11,8 +11,8 @@ feature "insecure direct object reference" do login(@normal_user) visit "/users/#{@normal_user.id}/benefit_forms" - download_url = first('.widget-body a')[:href] - visit download_url.sub(/name=(.*?)&/, 'name=config/database.yml&') + download_url = first(".widget-body a")[:href] + visit download_url.sub(/name=(.*?)&/, "name=config/database.yml&") pending if verifying_fixed? @@ -29,6 +29,6 @@ feature "insecure direct object reference" do visit "/users/#{another_user.id}/work_info" pending if verifying_fixed? - expect(first('td').text).to eq(another_user.full_name) + expect(first("td").text).to eq(another_user.full_name) end end diff --git a/spec/vulnerabilities/sql_injection_spec.rb b/spec/vulnerabilities/sql_injection_spec.rb index 4a3b7a7..79a5270 100644 --- a/spec/vulnerabilities/sql_injection_spec.rb +++ b/spec/vulnerabilities/sql_injection_spec.rb @@ -16,13 +16,13 @@ feature "sql injection" do visit "/users/#{@normal_user.id}/account_settings" - within('#account_edit') do - fill_in 'Email', :with => 'joe.admin@schmoe.com' - fill_in 'user_password', :with => 'H4cketyhack' - fill_in 'user_password_confirmation', :with => 'H4cketyhack' + within("#account_edit") do + fill_in "Email", with: "joe.admin@schmoe.com" + fill_in "user_password", with: "H4cketyhack" + fill_in "user_password_confirmation", with: "H4cketyhack" # this is a hidden field, so cannot use fill_in to access it. - find(:xpath, "//input[@id='user_id']", :visible => false).set "8' OR admin='t') --" + find(:xpath, "//input[@id='user_id']", visible: false).set "8' OR admin='t') --" end click_on "Submit" diff --git a/spec/vulnerabilities/unvalidated_redirects_spec.rb b/spec/vulnerabilities/unvalidated_redirects_spec.rb index 70fa4ee..c3ff7d7 100644 --- a/spec/vulnerabilities/unvalidated_redirects_spec.rb +++ b/spec/vulnerabilities/unvalidated_redirects_spec.rb @@ -7,12 +7,12 @@ feature "unvalidated redirect" do @normal_user = UserFixture.normal_user end - scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/A10-Unvalidated-Redirects-and-Forwards-(redirect_to)", :js => true do - visit '/?url=http://example.com/do/evil/things' + scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/A10-Unvalidated-Redirects-and-Forwards-(redirect_to)", js: true do + visit "/?url=http://example.com/do/evil/things" - within('.signup') do - fill_in 'email', with: @normal_user.email - fill_in 'password', with: @normal_user.clear_password + within(".signup") do + fill_in "email", with: @normal_user.email + fill_in "password", with: @normal_user.clear_password end within(".actions") do click_on "Login" diff --git a/spec/vulnerabilities/xss_spec.rb b/spec/vulnerabilities/xss_spec.rb index e5f8026..7a63d5f 100644 --- a/spec/vulnerabilities/xss_spec.rb +++ b/spec/vulnerabilities/xss_spec.rb @@ -11,8 +11,8 @@ feature "xss" do login @normal_user visit "/users/#{@normal_user.id}/account_settings" - within('#account_edit') do - fill_in 'First name', :with => "" + within("#account_edit") do + fill_in "First name", with: "" # password gets screwed up if you don't re-submit - need to fix fill_in "user_password", with: @normal_user.clear_password