From f94c7fa15b5f7ea57353ab2809ee448a046735aa Mon Sep 17 00:00:00 2001 From: Prommah Date: Thu, 24 Sep 2015 01:32:16 +0100 Subject: [PATCH 1/8] Stricter SteamID validation --- app/models/ban.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/ban.rb b/app/models/ban.rb index 44fa7e1..ef68856 100644 --- a/app/models/ban.rb +++ b/app/models/ban.rb @@ -35,7 +35,8 @@ class Ban < ActiveRecord::Base validate :validate_type validate :validate_ventban - validates_format_of :steamid, with: /\A([0-9]{1,10}:){2}[0-9]{1,10}\Z/, allow_blank: true + validates_length_of :steamid, maximum: 14, allow_blank: true + validates_format_of :steamid, with: /\A0:[01]:[0-9]{1,10}\Z/, allow_blank: true validates_format_of :addr, with: /\A([0-9]{1,3}\.){3}[0-9]{1,3}:?[0-9]{0,5}\z/, allow_blank: true validates_length_of :reason, maximum: 255, allow_nil: true, allow_blank: true From cbd33d97f42554ed93214362ac722bf34642914e Mon Sep 17 00:00:00 2001 From: Prommah Date: Thu, 24 Sep 2015 01:35:30 +0100 Subject: [PATCH 2/8] Stricter SteamID validation --- app/models/user.rb | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 12554f6..04465fd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -100,7 +100,8 @@ class User < ActiveRecord::Base scope :idle, :conditions => ["lastvisit < ?", 30.minutes.ago.utc] - validates_uniqueness_of :username, :email, :steamid + validates_uniqueness_of :username, :email + validates_uniqueness_of :steamid, :allow_nil => true validates_length_of :firstname, :in => 1..15, :allow_blank => true validates_length_of :lastname, :in => 1..25, :allow_blank => true validates_length_of :username, :in => 2..20 @@ -108,8 +109,9 @@ class User < ActiveRecord::Base validates_presence_of :raw_password, :on => :create validates_length_of :email, :maximum => 50 validates_format_of :email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i - validates_length_of :steamid, :maximum => 30 - validates_format_of :steamid, :with => /\A([0-9]{1,10}:){2}[0-9]{1,10}\Z/ + validates_length_of :steamid, :maximum => 14 + validates :steamid, presence: true, on: :create + validate :validate_steamid validates_length_of :time_zone, :maximum => 100, :allow_blank => true, :allow_nil => true validates_inclusion_of [:public_email], :in => [true, false], :allow_nil => true validate :validate_team @@ -117,6 +119,8 @@ class User < ActiveRecord::Base before_create :init_variables before_validation :update_password + before_save :correct_steamid_universe + accepts_nested_attributes_for :profile acts_as_versioned @@ -238,6 +242,18 @@ class User < ActiveRecord::Base issues.unread_by(self) end + def validate_steamid + if !(self.steamid.nil? || (m = self.steamid.match(/\A([01]):([01]):(\d{1,10})\Z/) and accid = (m[3].to_i<<1)+m[2].to_i and accid > 0 and accid <= 4294967295)) + errors.add :steamid + end + end + + def correct_steamid_universe + if self.steamid + self.steamid[0] = "0" + end + end + def validate_team if team and !active_teams.exists?({:id => team.id}) errors.add :team From 0cc67184062408921c99a6a8c71bb899b2a0c10d Mon Sep 17 00:00:00 2001 From: Prommah Date: Thu, 24 Sep 2015 03:16:35 +0100 Subject: [PATCH 3/8] Return null steam data for previously invalid SteamIDs --- app/controllers/api/v1/users_controller.rb | 2 +- spec/controllers/api/v1/users_controller_spec.rb | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) mode change 100644 => 100755 app/controllers/api/v1/users_controller.rb mode change 100644 => 100755 spec/controllers/api/v1/users_controller_spec.rb diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb old mode 100644 new mode 100755 index 161772b..7c992d2 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -14,7 +14,7 @@ class Api::V1::UsersController < Api::V1::BaseController time_zone: @user.time_zone, avatar: @user.profile.avatar.url, admin: @user.admin?, - steam: { + steam: @user.steamid.nil? ? nil : { id: @user.steamid, url: @steam.nil? ? nil : @steam.base_url, nickname: @steam.nil? ? nil : @steam.nickname diff --git a/spec/controllers/api/v1/users_controller_spec.rb b/spec/controllers/api/v1/users_controller_spec.rb old mode 100644 new mode 100755 index f9e362b..c2a7059 --- a/spec/controllers/api/v1/users_controller_spec.rb +++ b/spec/controllers/api/v1/users_controller_spec.rb @@ -29,16 +29,14 @@ describe Api::V1::UsersController do expect(json["team"]).to be_nil end - it "returns data for users with invalid steam ids" do - @user.steamid = "0:0:000" + it "returns nulled steam data for users who had invalid steam ids" do + @user.steamid = nil @user.save! get :show, id: @user.id expect(response).to be_success - expect(json["steam"]["id"]).to_not be_nil - expect(json["steam"]["url"]).to be_nil - expect(json["steam"]["nickname"]).to be_nil + expect(json["steam"]).to be_nil end it "returns 404 if user does not exist" do From 3214239caad02e17031fbcbf47b6338adaf9f514 Mon Sep 17 00:00:00 2001 From: Prommah Date: Thu, 24 Sep 2015 04:03:24 +0100 Subject: [PATCH 4/8] Skip unnecessary condenser lookups --- app/controllers/api/v1/users_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 7c992d2..accc35f 100755 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -5,7 +5,9 @@ class Api::V1::UsersController < Api::V1::BaseController def show @user = User.find(params[:id]) - @steam = steam_profile @user + if @user.steamid.present? + @steam = steam_profile @user + end render json: { id: @user.id, From d57ea707163072db3f40f5d2c21d47aa12b6127e Mon Sep 17 00:00:00 2001 From: Prommah Date: Thu, 24 Sep 2015 06:58:21 +0100 Subject: [PATCH 5/8] Add SteamID range spec --- spec/features/users/user_signs_up_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) mode change 100644 => 100755 spec/features/users/user_signs_up_spec.rb diff --git a/spec/features/users/user_signs_up_spec.rb b/spec/features/users/user_signs_up_spec.rb old mode 100644 new mode 100755 index e73e0e3..51de9ba --- a/spec/features/users/user_signs_up_spec.rb +++ b/spec/features/users/user_signs_up_spec.rb @@ -43,6 +43,15 @@ feature 'Visitor signs up', js: :true do expect(page).to have_content(error_message('steamid.invalid')) end + scenario 'with out of range Steam ID' do + within registration_form do + fill_form(:user, user.slice(*sign_up_attributes).merge({ steamid: "0:0:2147483648" })) + click_button submit(:user, :create) + end + + expect(page).to have_content(error_message('steamid.invalid')) + end + def sign_up_attributes [:username, :email, :raw_password, :steamid] end From 19e63f5cdcab248dd1223c96d3a003ef2784dc55 Mon Sep 17 00:00:00 2001 From: Prommah Date: Thu, 24 Sep 2015 07:15:49 +0100 Subject: [PATCH 6/8] Add test for nil SteamID --- spec/features/users/user_signs_up_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/features/users/user_signs_up_spec.rb b/spec/features/users/user_signs_up_spec.rb index 51de9ba..ff098f6 100755 --- a/spec/features/users/user_signs_up_spec.rb +++ b/spec/features/users/user_signs_up_spec.rb @@ -52,6 +52,15 @@ feature 'Visitor signs up', js: :true do expect(page).to have_content(error_message('steamid.invalid')) end + scenario 'with nil Steam ID' do + within registration_form do + fill_form(:user, user.slice(*sign_up_attributes).merge({ steamid: nil })) + click_button submit(:user, :create) + end + + expect(page).to have_content(error_message('steamid.invalid')) + end + def sign_up_attributes [:username, :email, :raw_password, :steamid] end From 4d83b2a06d7b1a8ed0b8bb2fc00148b4f5ab1d9b Mon Sep 17 00:00:00 2001 From: Prommah Date: Thu, 24 Sep 2015 07:23:05 +0100 Subject: [PATCH 7/8] Improve & clean SteamID validation --- app/models/user.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) mode change 100644 => 100755 app/models/user.rb diff --git a/app/models/user.rb b/app/models/user.rb old mode 100644 new mode 100755 index 04465fd..4d50141 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -243,13 +243,14 @@ class User < ActiveRecord::Base end def validate_steamid - if !(self.steamid.nil? || (m = self.steamid.match(/\A([01]):([01]):(\d{1,10})\Z/) and accid = (m[3].to_i<<1)+m[2].to_i and accid > 0 and accid <= 4294967295)) - errors.add :steamid - end + errors.add :steamid unless self.steamid.nil? || + (m = self.steamid.match(/\A([01]):([01]):(\d{1,10})\Z/)) && + (id = m[3].to_i) && + id >= 1 && id <= 2147483647 end def correct_steamid_universe - if self.steamid + if self.steamid.present? self.steamid[0] = "0" end end From c4a0dd62ffa6d524f069897bef27fb9214c1896d Mon Sep 17 00:00:00 2001 From: Prommah Date: Thu, 24 Sep 2015 10:19:00 +0100 Subject: [PATCH 8/8] Fix styling and tidy validations --- app/models/ban.rb | 11 +++---- app/models/user.rb | 33 +++++++++----------- spec/features/users/user_signs_up_spec.rb | 38 +++++++++++------------ 3 files changed, 39 insertions(+), 43 deletions(-) mode change 100644 => 100755 app/models/ban.rb diff --git a/app/models/ban.rb b/app/models/ban.rb old mode 100644 new mode 100755 index ef68856..2d07a1f --- a/app/models/ban.rb +++ b/app/models/ban.rb @@ -33,14 +33,13 @@ class Ban < ActiveRecord::Base scope :effective, conditions: "expiry > UTC_TIMESTAMP()" scope :ineffective, conditions: "expiry < UTC_TIMESTAMP()" + before_validation :check_user + validate :validate_type validate :validate_ventban - validates_length_of :steamid, maximum: 14, allow_blank: true - validates_format_of :steamid, with: /\A0:[01]:[0-9]{1,10}\Z/, allow_blank: true - validates_format_of :addr, with: /\A([0-9]{1,3}\.){3}[0-9]{1,3}:?[0-9]{0,5}\z/, allow_blank: true - validates_length_of :reason, maximum: 255, allow_nil: true, allow_blank: true - - before_validation :check_user + validates :steamid, length: {maximum: 14}, format: /\A0:[01]:[0-9]{1,10}\Z/, allow_blank: true + validates :addr, format: /\A([0-9]{1,3}\.){3}[0-9]{1,3}:?[0-9]{0,5}\z/, allow_blank: true + validates :reason, length: {maximum: 255}, allow_blank: true belongs_to :user belongs_to :server diff --git a/app/models/user.rb b/app/models/user.rb index 4d50141..13d0407 100755 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -100,24 +100,20 @@ class User < ActiveRecord::Base scope :idle, :conditions => ["lastvisit < ?", 30.minutes.ago.utc] - validates_uniqueness_of :username, :email - validates_uniqueness_of :steamid, :allow_nil => true - validates_length_of :firstname, :in => 1..15, :allow_blank => true - validates_length_of :lastname, :in => 1..25, :allow_blank => true - validates_length_of :username, :in => 2..20 - validates_format_of :username, :with => /\A[A-Za-z0-9_\-\+]{2,20}\Z/ - validates_presence_of :raw_password, :on => :create - validates_length_of :email, :maximum => 50 - validates_format_of :email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i - validates_length_of :steamid, :maximum => 14 - validates :steamid, presence: true, on: :create + before_validation :update_password + + validates :username, uniqueness: true, length: {in: 2..20}, format: /\A[A-Za-z0-9_\-\+]{2,20}\Z/ + validates :firstname, length: {in: 1..15}, allow_blank: true + validates :lastname, length: {in: 1..25}, allow_blank: true + validates :raw_password, presence: {on: :create} + validates :email, uniqueness: true, length: {maximum: 50}, format: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i + validates :steamid, uniqueness: {allow_nil: true}, length: {maximum: 14}, presence: {on: :create} validate :validate_steamid - validates_length_of :time_zone, :maximum => 100, :allow_blank => true, :allow_nil => true - validates_inclusion_of [:public_email], :in => [true, false], :allow_nil => true + validates :time_zone, length: {maximum: 100}, allow_blank: true + validates :public_email, inclusion: [true, false], allow_nil: true validate :validate_team before_create :init_variables - before_validation :update_password before_save :correct_steamid_universe @@ -243,15 +239,16 @@ class User < ActiveRecord::Base end def validate_steamid - errors.add :steamid unless self.steamid.nil? || - (m = self.steamid.match(/\A([01]):([01]):(\d{1,10})\Z/)) && + errors.add :steamid unless + steamid.nil? || + (m = steamid.match(/\A([01]):([01]):(\d{1,10})\Z/)) && (id = m[3].to_i) && id >= 1 && id <= 2147483647 end def correct_steamid_universe - if self.steamid.present? - self.steamid[0] = "0" + if steamid.present? + steamid[0] = "0" end end diff --git a/spec/features/users/user_signs_up_spec.rb b/spec/features/users/user_signs_up_spec.rb index ff098f6..2d5b2e7 100755 --- a/spec/features/users/user_signs_up_spec.rb +++ b/spec/features/users/user_signs_up_spec.rb @@ -1,64 +1,64 @@ -require 'spec_helper' +require "spec_helper" -feature 'Visitor signs up', js: :true do +feature "Visitor signs up", js: :true do let(:user) { attributes_for(:user) } before do visit new_user_path end - scenario 'with valid Username, Email, Password and Steam ID' do + scenario "with valid Username, Email, Password and Steam ID" do within registration_form do fill_form(:user, user.slice(*sign_up_attributes)) click_button submit(:user, :create) end - expect(user_status).to have_content('ACCOUNT') + expect(user_status).to have_content("ACCOUNT") end - scenario 'with invalid Email' do + scenario "with invalid Email" do within registration_form do - fill_form(:user, user.slice(*sign_up_attributes).merge({ email: "invalid" })) + fill_form(:user, user.slice(*sign_up_attributes).merge(email: "invalid")) click_button submit(:user, :create) end - expect(page).to have_content(error_message('email.invalid')) + expect(page).to have_content(error_message("email.invalid")) end - scenario 'with blank Password' do + scenario "with blank Password" do within registration_form do - fill_form(:user, user.slice(*sign_up_attributes).merge({ raw_password: "" })) + fill_form(:user, user.slice(*sign_up_attributes).merge(raw_password: "")) click_button submit(:user, :create) end - expect(page).to have_content(error_message('raw_password.blank')) + expect(page).to have_content(error_message("raw_password.blank")) end - scenario 'with invalid Steam ID' do + scenario "with invalid Steam ID" do within registration_form do - fill_form(:user, user.slice(*sign_up_attributes).merge({ steamid: "invalid" })) + fill_form(:user, user.slice(*sign_up_attributes).merge(steamid: "invalid")) click_button submit(:user, :create) end - expect(page).to have_content(error_message('steamid.invalid')) + expect(page).to have_content(error_message("steamid.invalid")) end - scenario 'with out of range Steam ID' do + scenario "with out of range Steam ID" do within registration_form do - fill_form(:user, user.slice(*sign_up_attributes).merge({ steamid: "0:0:2147483648" })) + fill_form(:user, user.slice(*sign_up_attributes).merge(steamid: "0:0:2147483648")) click_button submit(:user, :create) end - expect(page).to have_content(error_message('steamid.invalid')) + expect(page).to have_content(error_message("steamid.invalid")) end - scenario 'with nil Steam ID' do + scenario "with nil Steam ID" do within registration_form do - fill_form(:user, user.slice(*sign_up_attributes).merge({ steamid: nil })) + fill_form(:user, user.slice(*sign_up_attributes).merge(steamid: nil)) click_button submit(:user, :create) end - expect(page).to have_content(error_message('steamid.invalid')) + expect(page).to have_content(error_message("steamid.invalid")) end def sign_up_attributes