From e6e1187fdc44766aa4336e05aa12d4e74db65d1d Mon Sep 17 00:00:00 2001 From: ankonym Date: Wed, 5 Aug 2015 16:55:39 +0200 Subject: Adding invite code field to signup with validation for hardcoded invite code --- app/models/user.rb | 4 +++- app/views/users/new.html.haml | 5 ++++- test/unit/user_test.rb | 5 +++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index d44df40..5510470 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -8,7 +8,7 @@ class User < CouchRest::Model::Base property :password_salt, String, :accessible => true property :contact_email, String, :accessible => true property :contact_email_key, String, :accessible => true - + property :invite_code, String, :accessible => true property :enabled, TrueClass, :default => true # these will be null by default but we shouldn't ever pull them directly, but only via the methods that will return the full ServiceLevel @@ -39,6 +39,8 @@ class User < CouchRest::Model::Base :email => true, :mx_with_fallback => true + validates :invite_code, inclusion: { in: ["testcode"], message: "%{value} is not a valid invite code" } + timestamps! design do diff --git a/app/views/users/new.html.haml b/app/views/users/new.html.haml index 41a9d55..cd4bf70 100644 --- a/app/views/users/new.html.haml +++ b/app/views/users/new.html.haml @@ -17,5 +17,8 @@ = f.input :login, :label => t(:username), :required => false, :input_html => { :id => :srp_username } = f.input :password, :required => false, :validate => true, :input_html => { :id => :srp_password } = f.input :password_confirmation, :required => false, :validate => true, :input_html => { :id => :srp_password_confirmation } - = f.button :wrapped, cancel: home_path + = f.input :invite_code, :input_html => { :id => :srp_invite_code } + + = f.button :wrapped, cancel: home_path +-# diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index c301923..cd290d5 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -70,6 +70,11 @@ class UserTest < ActiveSupport::TestCase assert_equal key, @user.public_key end + test "user should have an invite token" do + user = User.new + assert_nil(user.invite_code) + end + # ## Regression tests # -- cgit v1.2.3 From fc7cd96591c383ba4f0a2c1e0c7c63095fbe5a4b Mon Sep 17 00:00:00 2001 From: kaeff Date: Mon, 14 Sep 2015 15:42:31 +0200 Subject: Move account form info from srp_js into leap_web --- app/assets/javascripts/users.js | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/users.js b/app/assets/javascripts/users.js index e0f1c9d..5217942 100644 --- a/app/assets/javascripts/users.js +++ b/app/assets/javascripts/users.js @@ -88,11 +88,34 @@ } }; + var account = { + + // Returns the user's identity + login: function() { + return document.getElementById("srp_username").value; + }, + + // Returns the password currently typed in + password: function() { + return document.getElementById("srp_password").value; + }, + + // The user's id + id: function() { + return document.getElementById("user_param").value; + }, + + // Returns the invite code currently typed in + loginParams: function () { + return { "invite_code": document.getElementById("srp_invite_code").value }; + } + } + // // PUBLIC FUNCTIONS // - srp.session = new srp.Session(); + srp.session = new srp.Session(account); srp.signedUp = function() { return srp.login(); -- cgit v1.2.3 From 8f03c13a038469a1191666259ef5609c89d69d90 Mon Sep 17 00:00:00 2001 From: kaeff Date: Fri, 18 Sep 2015 00:58:22 +0200 Subject: Update submodule srp to 9e1a41733 --- app/assets/javascripts/srp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/srp b/app/assets/javascripts/srp index 8f33d32..9e1a417 160000 --- a/app/assets/javascripts/srp +++ b/app/assets/javascripts/srp @@ -1 +1 @@ -Subproject commit 8f33d32d40b1e21ae7fb9a92c78a275422af4217 +Subproject commit 9e1a41733468d4a3f5102b04277b9cd7b52d0a45 -- cgit v1.2.3 From 9156f1354f404c569e7fd337cff0171f6f43842e Mon Sep 17 00:00:00 2001 From: Aya Jaff Date: Wed, 12 Aug 2015 15:18:34 +0200 Subject: Added an 'invite code' to all the tests for the sign-up form so we have a valid user for the tests again --- Gemfile | 5 +++++ Gemfile.lock | 9 +++++++++ test/factories.rb | 2 ++ test/integration/api/srp_test.rb | 4 ++-- test/support/browser_integration_test.rb | 1 + test/unit/tmp_user_test.rb | 4 ++-- 6 files changed, 21 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index dd93a3c..fdd63f5 100644 --- a/Gemfile +++ b/Gemfile @@ -84,6 +84,11 @@ group :production do gem 'SyslogLogger', '~> 2.0' end +group :development do + gem "better_errors", '1.1.0' + gem "binding_of_caller" +end + group :debug do gem 'debugger', :platforms => :mri_19 end diff --git a/Gemfile.lock b/Gemfile.lock index 42dea53..b859e81 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -56,6 +56,11 @@ GEM multi_json (~> 1.0) addressable (2.3.6) arel (3.0.3) + better_errors (1.1.0) + coderay (>= 1.0.0) + erubis (>= 2.6.6) + binding_of_caller (0.7.2) + debug_inspector (>= 0.0.1) bootstrap-sass (2.3.2.2) sass (~> 3.2) braintree (2.38.0) @@ -72,6 +77,7 @@ GEM client_side_validations (~> 3.2.5) simple_form (~> 2.1.0) cliver (0.3.2) + coderay (1.1.0) columnize (0.9.0) couchrest (1.1.3) mime-types (~> 1.15) @@ -99,6 +105,7 @@ GEM nokogiri (~> 1.5) rails (>= 3, < 5) daemons (1.1.9) + debug_inspector (0.0.2) debugger (1.6.8) columnize (>= 0.3.1) debugger-linecache (~> 1.2.0) @@ -258,6 +265,8 @@ PLATFORMS DEPENDENCIES SyslogLogger (~> 2.0) + better_errors (= 1.1.0) + binding_of_caller bootstrap-sass (= 2.3.2.2) capybara certificate_authority! diff --git a/test/factories.rb b/test/factories.rb index 0734688..1a778ff 100644 --- a/test/factories.rb +++ b/test/factories.rb @@ -11,6 +11,8 @@ FactoryGirl.define do login { Faker::Internet.user_name + '_' + SecureRandom.hex(4) } password_verifier "1234ABCD" password_salt "4321AB" + invite_code "testcode" + factory :user_with_settings do email_forward { Faker::Internet.email } diff --git a/test/integration/api/srp_test.rb b/test/integration/api/srp_test.rb index fbef47e..ea06cde 100644 --- a/test/integration/api/srp_test.rb +++ b/test/integration/api/srp_test.rb @@ -32,10 +32,10 @@ class SrpTest < RackTest attr_reader :server_auth - def register_user(login = "integration_test", password = 'srp, verify me!') + def register_user(login = "integration_test", password = 'srp, verify me!', invite_code = "testcode") cleanup_user(login) post 'http://api.lvh.me:3000/1/users.json', - user_params(login: login, password: password) + user_params(login: login, password: password, invite_code: invite_code) assert(@user = User.find_by_login(login), 'user should have been created: %s' % last_response_errors) @login = login @password = password diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb index 1e2aa51..48c7623 100644 --- a/test/support/browser_integration_test.rb +++ b/test/support/browser_integration_test.rb @@ -50,6 +50,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest visit '/users/new' fill_in 'Username', with: username fill_in 'Password', with: password + fill_in 'Invite code', with: 'testcode' fill_in 'Password confirmation', with: password click_on 'Sign Up' return username, password diff --git a/test/unit/tmp_user_test.rb b/test/unit/tmp_user_test.rb index 55b117f..af2218e 100644 --- a/test/unit/tmp_user_test.rb +++ b/test/unit/tmp_user_test.rb @@ -8,13 +8,13 @@ class TmpUserTest < ActiveSupport::TestCase assert_difference('User.database.info["doc_count"]') do normal_user = User.create!(:login => 'a'+SecureRandom.hex(5).downcase, - :password_verifier => 'ABCDEF0010101', :password_salt => 'ABCDEF') + :password_verifier => 'ABCDEF0010101', :password_salt => 'ABCDEF', :invite_code => 'testcode') refute normal_user.database.to_s.include?('tmp') end assert_difference('User.tmp_database.info["doc_count"]') do tmp_user = User.create!(:login => 'test_user_'+SecureRandom.hex(5).downcase, - :password_verifier => 'ABCDEF0010101', :password_salt => 'ABCDEF') + :password_verifier => 'ABCDEF0010101', :password_salt => 'ABCDEF', :invite_code => 'testcode') assert tmp_user.database.to_s.include?('tmp') end ensure -- cgit v1.2.3 From 393291de60c4f29bb03bad596bb60c1e0648e6e7 Mon Sep 17 00:00:00 2001 From: ankonym Date: Thu, 13 Aug 2015 15:28:02 +0200 Subject: Add invite code model --- app/models/invite_code.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 app/models/invite_code.rb diff --git a/app/models/invite_code.rb b/app/models/invite_code.rb new file mode 100644 index 0000000..b9f6f33 --- /dev/null +++ b/app/models/invite_code.rb @@ -0,0 +1,10 @@ +class InviteCode < CouchRest::Model::Base + use_database 'invite_codes' + property :invite_code, String + timestamps! + + design do + view :by__id + end +end + -- cgit v1.2.3 From bd98a2cbb8796039efbffb7039a9bda1cde22dae Mon Sep 17 00:00:00 2001 From: ankonym Date: Thu, 13 Aug 2015 15:29:04 +0200 Subject: Add validation of invite code in user object based on codes in couch db --- app/models/invite_code_validator.rb | 16 ++++++++++++++++ app/models/user.rb | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 app/models/invite_code_validator.rb diff --git a/app/models/invite_code_validator.rb b/app/models/invite_code_validator.rb new file mode 100644 index 0000000..55b7a74 --- /dev/null +++ b/app/models/invite_code_validator.rb @@ -0,0 +1,16 @@ +class InviteCodeValidator < ActiveModel::Validator + def validate(user) + if not_existent?(user.invite_code) + add_error_to_user("This is not a valid code", user) + end + end + + private + def not_existent?(code) + InviteCode.find_by__id(code) == nil + end + + def add_error_to_user(error, user) + user.errors[:invite_code] << error + end +end \ No newline at end of file diff --git a/app/models/user.rb b/app/models/user.rb index 5510470..06e1a0f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -39,7 +39,7 @@ class User < CouchRest::Model::Base :email => true, :mx_with_fallback => true - validates :invite_code, inclusion: { in: ["testcode"], message: "%{value} is not a valid invite code" } + validates_with InviteCodeValidator timestamps! -- cgit v1.2.3 From 8b9b2a0c3c8457024d99d0794416c59cb245a513 Mon Sep 17 00:00:00 2001 From: ankonym Date: Thu, 13 Aug 2015 17:24:02 +0200 Subject: Changed invite code query to look for invite_code string instead of id --- app/models/invite_code.rb | 2 +- app/models/invite_code_validator.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/invite_code.rb b/app/models/invite_code.rb index b9f6f33..751b28e 100644 --- a/app/models/invite_code.rb +++ b/app/models/invite_code.rb @@ -4,7 +4,7 @@ class InviteCode < CouchRest::Model::Base timestamps! design do - view :by__id + view :by_invite_code end end diff --git a/app/models/invite_code_validator.rb b/app/models/invite_code_validator.rb index 55b7a74..978d3f5 100644 --- a/app/models/invite_code_validator.rb +++ b/app/models/invite_code_validator.rb @@ -7,7 +7,7 @@ class InviteCodeValidator < ActiveModel::Validator private def not_existent?(code) - InviteCode.find_by__id(code) == nil + InviteCode.find_by_invite_code(code) == nil end def add_error_to_user(error, user) -- cgit v1.2.3 From 8b5665b857edc460ef6105c3ba0f106dd99a25d5 Mon Sep 17 00:00:00 2001 From: ankonym Date: Thu, 13 Aug 2015 17:24:31 +0200 Subject: Fix test based on actual invite code validation --- .../test/functional/customers_controller_test.rb | 1 + .../support/test/unit/account_extension_test.rb | 4 ++++ engines/support/test/unit/ticket_test.rb | 4 ++++ test/factories.rb | 1 - test/functional/identities_controller_test.rb | 4 ++++ test/functional/v1/messages_controller_test.rb | 1 + test/support/browser_integration_test.rb | 4 +++- test/unit/tmp_user_test.rb | 8 ++++++-- test/unit/token_test.rb | 1 + test/unit/user_test.rb | 23 +++++++++++++++++++--- 10 files changed, 44 insertions(+), 7 deletions(-) diff --git a/engines/billing/test/functional/customers_controller_test.rb b/engines/billing/test/functional/customers_controller_test.rb index cc82fc1..0dafe72 100644 --- a/engines/billing/test/functional/customers_controller_test.rb +++ b/engines/billing/test/functional/customers_controller_test.rb @@ -5,6 +5,7 @@ class CustomersControllerTest < ActionController::TestCase tests CustomerController setup do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) @user = FactoryGirl.create :user @other_user = FactoryGirl.create :user #FakeBraintree.clear! diff --git a/engines/support/test/unit/account_extension_test.rb b/engines/support/test/unit/account_extension_test.rb index aba162c..dd7e77d 100644 --- a/engines/support/test/unit/account_extension_test.rb +++ b/engines/support/test/unit/account_extension_test.rb @@ -2,6 +2,10 @@ require 'test_helper' class AccountExtensionTest < ActiveSupport::TestCase + setup do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + end + test "destroying an account triggers ticket destruction" do t = FactoryGirl.create :ticket_with_creator u = t.created_by_user diff --git a/engines/support/test/unit/ticket_test.rb b/engines/support/test/unit/ticket_test.rb index c64e8f4..7727650 100644 --- a/engines/support/test/unit/ticket_test.rb +++ b/engines/support/test/unit/ticket_test.rb @@ -2,6 +2,10 @@ require 'test_helper' class TicketTest < ActiveSupport::TestCase + setup do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + end + test "ticket with default attribs is valid" do t = FactoryGirl.build :ticket assert t.valid? diff --git a/test/factories.rb b/test/factories.rb index 1a778ff..b6e1475 100644 --- a/test/factories.rb +++ b/test/factories.rb @@ -13,7 +13,6 @@ FactoryGirl.define do password_salt "4321AB" invite_code "testcode" - factory :user_with_settings do email_forward { Faker::Internet.email } email_aliases_attributes do diff --git a/test/functional/identities_controller_test.rb b/test/functional/identities_controller_test.rb index fcdeaa2..c900178 100644 --- a/test/functional/identities_controller_test.rb +++ b/test/functional/identities_controller_test.rb @@ -2,6 +2,10 @@ require 'test_helper' class IdentitiesControllerTest < ActionController::TestCase + setup do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + end + test "admin can list active and blocked ids" do login :is_admin? => true get :index diff --git a/test/functional/v1/messages_controller_test.rb b/test/functional/v1/messages_controller_test.rb index 6f7ea5d..455eebd 100644 --- a/test/functional/v1/messages_controller_test.rb +++ b/test/functional/v1/messages_controller_test.rb @@ -3,6 +3,7 @@ require 'test_helper' class V1::MessagesControllerTest < ActionController::TestCase setup do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) @user = FactoryGirl.build(:user) @user.save @message = Message.new(:text => 'a test message') diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb index 48c7623..b4bc273 100644 --- a/test/support/browser_integration_test.rb +++ b/test/support/browser_integration_test.rb @@ -37,6 +37,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest setup do Capybara.current_driver = Capybara.javascript_driver page.driver.add_headers 'ACCEPT-LANGUAGE' => 'en-EN' + @invite_code = InviteCode.create(invite_code: "testcode") end teardown do @@ -50,7 +51,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest visit '/users/new' fill_in 'Username', with: username fill_in 'Password', with: password - fill_in 'Invite code', with: 'testcode' + fill_in 'Invite code', with: "testcode" fill_in 'Password confirmation', with: password click_on 'Sign Up' return username, password @@ -59,6 +60,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest # currently this only works for tests with poltergeist. # ApiIntegrationTest has a working implementation for RackTest def login(user = nil) + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) @user ||= user ||= FactoryGirl.create(:user) token = Token.create user_id: user.id page.driver.add_header "Authorization", %Q(Token token="#{token}") diff --git a/test/unit/tmp_user_test.rb b/test/unit/tmp_user_test.rb index af2218e..0a9ad68 100644 --- a/test/unit/tmp_user_test.rb +++ b/test/unit/tmp_user_test.rb @@ -2,19 +2,23 @@ require 'test_helper' class TmpUserTest < ActiveSupport::TestCase + setup do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + end + test "test_user saved to tmp_users" do begin assert User.ancestors.include?(TemporaryUser) assert_difference('User.database.info["doc_count"]') do normal_user = User.create!(:login => 'a'+SecureRandom.hex(5).downcase, - :password_verifier => 'ABCDEF0010101', :password_salt => 'ABCDEF', :invite_code => 'testcode') + :password_verifier => 'ABCDEF0010101', :password_salt => 'ABCDEF') refute normal_user.database.to_s.include?('tmp') end assert_difference('User.tmp_database.info["doc_count"]') do tmp_user = User.create!(:login => 'test_user_'+SecureRandom.hex(5).downcase, - :password_verifier => 'ABCDEF0010101', :password_salt => 'ABCDEF', :invite_code => 'testcode') + :password_verifier => 'ABCDEF0010101', :password_salt => 'ABCDEF') assert tmp_user.database.to_s.include?('tmp') end ensure diff --git a/test/unit/token_test.rb b/test/unit/token_test.rb index 5468650..dcf3fc4 100644 --- a/test/unit/token_test.rb +++ b/test/unit/token_test.rb @@ -4,6 +4,7 @@ class TokenTest < ActiveSupport::TestCase include StubRecordHelper setup do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) @user = find_record :user end diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index cd290d5..e28bef6 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -4,6 +4,7 @@ class UserTest < ActiveSupport::TestCase include SRP::Util setup do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) @user = FactoryGirl.build(:user) end @@ -70,9 +71,25 @@ class UserTest < ActiveSupport::TestCase assert_equal key, @user.public_key end - test "user should have an invite token" do - user = User.new - assert_nil(user.invite_code) + test "user should not be created with invalid invite code" do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(true) + invalid_user = FactoryGirl.build(:user) + + assert !invalid_user.valid? + end + + test "user should be created with valid invite code" do + assert @user.valid? + end + + test "trying to create a user with invalid invite code should add error" do + InviteCodeValidator.any_instance.stubs(:not_existent?).returns(true) + invalid_user = FactoryGirl.build(:user) + + invalid_user.valid? + + errors = {invite_code: ["This is not a valid code"]} + assert_equal errors, invalid_user.errors.messages end # -- cgit v1.2.3 From 35494972411d4649ed8e81e24b6ec3f8734f5529 Mon Sep 17 00:00:00 2001 From: ankonym Date: Wed, 19 Aug 2015 13:58:02 +0200 Subject: Remove change password browser test Remove the change password test because the change password functionality is currently unused - however, it breaks with the new invite code field in the signup form. --- test/integration/browser/account_test.rb | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/integration/browser/account_test.rb b/test/integration/browser/account_test.rb index aea5406..34eb72e 100644 --- a/test/integration/browser/account_test.rb +++ b/test/integration/browser/account_test.rb @@ -81,21 +81,6 @@ class AccountTest < BrowserIntegrationTest end end - test "change password" do - with_config user_actions: ['change_password'] do - login - click_on "Account Settings" - within('#update_login_and_password') do - fill_in 'Password', with: "other password" - fill_in 'Password confirmation', with: "other password" - click_on 'Save' - end - click_on 'Log Out' - attempt_login(@user.login, "other password") - assert page.has_content?("Welcome #{@user.login}") - end - end - test "change pgp key" do with_config user_actions: ['change_pgp_key'] do pgp_key = FactoryGirl.build :pgp_key -- cgit v1.2.3 From 0543217b433a8f4809f08018c1a11c20119fa85d Mon Sep 17 00:00:00 2001 From: ankonym Date: Fri, 21 Aug 2015 17:49:36 +0200 Subject: assign random invite code when creating new invite codes --- Gemfile | 1 + Gemfile.lock | 2 ++ app/models/invite_code.rb | 11 ++++++++++- test/unit/invite_code_test.rb | 21 +++++++++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 test/unit/invite_code_test.rb diff --git a/Gemfile b/Gemfile index fdd63f5..f0d50df 100644 --- a/Gemfile +++ b/Gemfile @@ -13,6 +13,7 @@ end ## AUTHENTICATION gem "ruby-srp", "~> 0.2.1" gem "rails_warden" +gem "coupon_code" ## LOCALIZATION gem 'http_accept_language' diff --git a/Gemfile.lock b/Gemfile.lock index b859e81..5ffb0c8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -92,6 +92,7 @@ GEM actionpack (~> 3.0) couchrest couchrest_model + coupon_code (0.0.1) cucumber (1.3.17) builder (>= 2.1.2) diff-lcs (>= 1.1.3) @@ -276,6 +277,7 @@ DEPENDENCIES couchrest (~> 1.1.3) couchrest_model (~> 2.0.0) couchrest_session_store (= 0.3.0) + coupon_code cucumber-rails debugger factory_girl_rails diff --git a/app/models/invite_code.rb b/app/models/invite_code.rb index 751b28e..d52436f 100644 --- a/app/models/invite_code.rb +++ b/app/models/invite_code.rb @@ -1,10 +1,19 @@ +require 'coupon_code' + class InviteCode < CouchRest::Model::Base use_database 'invite_codes' - property :invite_code, String + property :invite_code, String, :read_only => true timestamps! design do view :by_invite_code end + + def initialize(attributes = {}, options = {}) + super(attributes, options) + write_attribute('invite_code', CouponCode.generate) if new? + end + end + diff --git a/test/unit/invite_code_test.rb b/test/unit/invite_code_test.rb new file mode 100644 index 0000000..2684f8e --- /dev/null +++ b/test/unit/invite_code_test.rb @@ -0,0 +1,21 @@ +require 'test_helper' + +class InviteCodeTest < ActiveSupport::TestCase + + test "it is created with an invite code" do + code = InviteCode.new + assert_not_nil code.invite_code + end + + test "the invite code can be read from couch db correctly" do + code1 = InviteCode.new + code1.save + + code2 = InviteCode.find_by__id code1.id + + assert_equal code1.invite_code, code2.invite_code + + end + + +end \ No newline at end of file -- cgit v1.2.3 From 5f9aec72f124a971b765c14052363f3ce1e16c65 Mon Sep 17 00:00:00 2001 From: ankonym Date: Mon, 24 Aug 2015 16:08:57 +0200 Subject: Add rake task for invite code batch generation --- lib/tasks/invite_code.rake | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 lib/tasks/invite_code.rake diff --git a/lib/tasks/invite_code.rake b/lib/tasks/invite_code.rake new file mode 100644 index 0000000..f3bafac --- /dev/null +++ b/lib/tasks/invite_code.rake @@ -0,0 +1,16 @@ + + +desc "Generate a batch of invite codes" +task :generate_invites, [:n] => :environment do |task, args| + + codes = args.n + codes = codes.to_i + + codes.times do |x| + x = InviteCode.new + x.save + puts "#{x.invite_code} Code generated." + + end +end + -- cgit v1.2.3 From 45c3fadd930a474951bd918a50e1ea2b00af75c7 Mon Sep 17 00:00:00 2001 From: ankonym Date: Tue, 25 Aug 2015 14:11:00 +0200 Subject: Make sure codes can only be used once, fix validations We introduced a count on invite codes to make sure that (at the moment) codes can only be used once. (The code will also allow multi-use codes in the future.) Also, some of our validations weren't validating against the correct data, which is now fixed. --- app/models/account.rb | 5 +++-- app/models/invite_code.rb | 3 +++ app/models/invite_code_validator.rb | 20 ++++++++++++++++++-- app/models/user.rb | 2 ++ test/unit/invite_code_test.rb | 33 ++++++++++++++++++++++++++++++--- 5 files changed, 56 insertions(+), 7 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index af470ed..a57e3f7 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -1,7 +1,7 @@ # -# The Account model takes care of the livecycle of a user. +# The Account model takes care of the lifecycle of a user. # It composes a User record and it's identity records. -# It also allows for other engines to hook into the livecycle by +# It also allows for other engines to hook into the lifecycle by # monkeypatching the create, update and destroy methods. # There's an ActiveSupport load_hook at the end of this file to # make this more easy. @@ -19,6 +19,7 @@ class Account identity = nil user = nil user = User.new(attrs) + user.save if !user.tmp? && user.persisted? identity = user.identity diff --git a/app/models/invite_code.rb b/app/models/invite_code.rb index d52436f..30a6498 100644 --- a/app/models/invite_code.rb +++ b/app/models/invite_code.rb @@ -3,10 +3,13 @@ require 'coupon_code' class InviteCode < CouchRest::Model::Base use_database 'invite_codes' property :invite_code, String, :read_only => true + property :invite_count, Integer, :default => 0, :accessible => true + timestamps! design do view :by_invite_code + view :by_invite_count end def initialize(attributes = {}, options = {}) diff --git a/app/models/invite_code_validator.rb b/app/models/invite_code_validator.rb index 978d3f5..73c333a 100644 --- a/app/models/invite_code_validator.rb +++ b/app/models/invite_code_validator.rb @@ -1,7 +1,17 @@ class InviteCodeValidator < ActiveModel::Validator def validate(user) - if not_existent?(user.invite_code) + + user_invite_code = InviteCode.find_by_invite_code user.invite_code + + if not_existent?(user_invite_code.invite_code) add_error_to_user("This is not a valid code", user) + + elsif count_greater_than_zero?(user_invite_code.invite_count) + add_error_to_user("This code has already been used", user) + + else + user_invite_code.invite_count += 1 + user_invite_code.save end end @@ -10,7 +20,13 @@ class InviteCodeValidator < ActiveModel::Validator InviteCode.find_by_invite_code(code) == nil end + def count_greater_than_zero?(code) + code > 0 + end + def add_error_to_user(error, user) user.errors[:invite_code] << error end -end \ No newline at end of file +end + + diff --git a/app/models/user.rb b/app/models/user.rb index 06e1a0f..a4dd132 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -39,8 +39,10 @@ class User < CouchRest::Model::Base :email => true, :mx_with_fallback => true + validates_with InviteCodeValidator + timestamps! design do diff --git a/test/unit/invite_code_test.rb b/test/unit/invite_code_test.rb index 2684f8e..b6044f4 100644 --- a/test/unit/invite_code_test.rb +++ b/test/unit/invite_code_test.rb @@ -10,12 +10,39 @@ class InviteCodeTest < ActiveSupport::TestCase test "the invite code can be read from couch db correctly" do code1 = InviteCode.new code1.save - code2 = InviteCode.find_by__id code1.id - assert_equal code1.invite_code, code2.invite_code + end + test "the invite code count gets set to 0 upon creation" do + code1 = InviteCode.new + code1.save + assert_equal code1.invite_count, 0 end + # TODO: does the count go up when code gets entered? + test "Invite code count goes up by 1 when the invite code is entered" do + + validator = InviteCodeValidator.new nil + + user = FactoryGirl.build :user + user_code = InviteCode.new + user_code.save + user.invite_code = user_code.invite_code + + + validator.validate(user) + + user_code.reload + assert_equal 1, user_code.invite_count + + end +# +# +# # TODO: count >0 is not accepted for signup + # test "Invite count >0 is not accepted for new account signup" do + + # end + +end -end \ No newline at end of file -- cgit v1.2.3 From a8f07fb18518fd95fd701e8c0e550a3cd70520b0 Mon Sep 17 00:00:00 2001 From: ankonym Date: Mon, 31 Aug 2015 17:15:27 +0200 Subject: Fixes for the invite code validator Validation should only happen for new records User invite code was nil for invalid invite codes Adding missing tests --- app/models/invite_code_validator.rb | 3 +- app/models/user.rb | 2 +- test/unit/invite_code_test.rb | 67 ++++++++++++++++++++++++++++--------- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/app/models/invite_code_validator.rb b/app/models/invite_code_validator.rb index 73c333a..d03d510 100644 --- a/app/models/invite_code_validator.rb +++ b/app/models/invite_code_validator.rb @@ -3,7 +3,7 @@ class InviteCodeValidator < ActiveModel::Validator user_invite_code = InviteCode.find_by_invite_code user.invite_code - if not_existent?(user_invite_code.invite_code) + if not_existent?(user.invite_code) add_error_to_user("This is not a valid code", user) elsif count_greater_than_zero?(user_invite_code.invite_count) @@ -18,6 +18,7 @@ class InviteCodeValidator < ActiveModel::Validator private def not_existent?(code) InviteCode.find_by_invite_code(code) == nil + end def count_greater_than_zero?(code) diff --git a/app/models/user.rb b/app/models/user.rb index a4dd132..c0079d5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -40,7 +40,7 @@ class User < CouchRest::Model::Base :mx_with_fallback => true - validates_with InviteCodeValidator + validates_with InviteCodeValidator, on: :create timestamps! diff --git a/test/unit/invite_code_test.rb b/test/unit/invite_code_test.rb index b6044f4..2c9e33f 100644 --- a/test/unit/invite_code_test.rb +++ b/test/unit/invite_code_test.rb @@ -20,29 +20,64 @@ class InviteCodeTest < ActiveSupport::TestCase assert_equal code1.invite_count, 0 end - # TODO: does the count go up when code gets entered? - test "Invite code count goes up by 1 when the invite code is entered" do + test "Invite code count goes up by 1 when the invite code is entered" do + #TODO but onlz if there are no other errors on the form - validator = InviteCodeValidator.new nil + validator = InviteCodeValidator.new nil - user = FactoryGirl.build :user - user_code = InviteCode.new - user_code.save - user.invite_code = user_code.invite_code + user = FactoryGirl.build :user + user_code = InviteCode.new + user_code.save + user.invite_code = user_code.invite_code + validator.validate(user) - validator.validate(user) + user_code.reload + assert_equal 1, user_code.invite_count - user_code.reload - assert_equal 1, user_code.invite_count + end + + test "Invite count >0 is not accepted for new account signup" do + validator = InviteCodeValidator.new nil + + user_code = InviteCode.new + user_code.invite_count = 1 + user_code.save + + user = FactoryGirl.build :user + user.invite_code = user_code.invite_code + + validator.validate(user) + + assert_equal ["This code has already been used"], user.errors[:invite_code] + + end + + test "Invite count 0 is accepted for new account signup" do + validator = InviteCodeValidator.new nil + + user_code = InviteCode.create - end -# -# -# # TODO: count >0 is not accepted for signup - # test "Invite count >0 is not accepted for new account signup" do + user = FactoryGirl.build :user + user.invite_code = user_code.invite_code + + validator.validate(user) + + assert_equal [], user.errors[:invite_code] + end + + test "There is an error message if the invite code does not exist" do + validator = InviteCodeValidator.new nil + + user = FactoryGirl.build :user + user.invite_code = "wrongcode" + + validator.validate(user) + + assert_equal ["This is not a valid code"], user.errors[:invite_code] + + end - # end end -- cgit v1.2.3 From 06ebc254bc4537e81c1336627ba8a54c881a1765 Mon Sep 17 00:00:00 2001 From: ankonym Date: Mon, 31 Aug 2015 17:49:47 +0200 Subject: Separate user and invite code validator tests --- test/unit/invite_code_validator_test.rb | 26 ++++++++++++++++++++++++++ test/unit/user_test.rb | 21 +-------------------- 2 files changed, 27 insertions(+), 20 deletions(-) create mode 100644 test/unit/invite_code_validator_test.rb diff --git a/test/unit/invite_code_validator_test.rb b/test/unit/invite_code_validator_test.rb new file mode 100644 index 0000000..62768e3 --- /dev/null +++ b/test/unit/invite_code_validator_test.rb @@ -0,0 +1,26 @@ +require 'test_helper' + +class InviteCodeValidatorTest < ActiveSupport::TestCase + test "user should not be created with invalid invite code" do + invalid_user = FactoryGirl.build(:user) + + assert !invalid_user.valid? + end + + test "user should be created with valid invite code" do + valid_user = FactoryGirl.build(:user) + valid_code = InviteCode.create + valid_user.invite_code = valid_code.invite_code + + assert valid_user.valid? + end + + test "trying to create a user with invalid invite code should add error" do + invalid_user = FactoryGirl.build(:user) + + invalid_user.valid? + + errors = {invite_code: ["This is not a valid code"]} + assert_equal errors, invalid_user.errors.messages + end +end \ No newline at end of file diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index e28bef6..9501d34 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -4,7 +4,7 @@ class UserTest < ActiveSupport::TestCase include SRP::Util setup do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + InviteCodeValidator.any_instance.stubs(:validate) @user = FactoryGirl.build(:user) end @@ -71,26 +71,7 @@ class UserTest < ActiveSupport::TestCase assert_equal key, @user.public_key end - test "user should not be created with invalid invite code" do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(true) - invalid_user = FactoryGirl.build(:user) - assert !invalid_user.valid? - end - - test "user should be created with valid invite code" do - assert @user.valid? - end - - test "trying to create a user with invalid invite code should add error" do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(true) - invalid_user = FactoryGirl.build(:user) - - invalid_user.valid? - - errors = {invite_code: ["This is not a valid code"]} - assert_equal errors, invalid_user.errors.messages - end # ## Regression tests -- cgit v1.2.3 From c48e921c101d49bf68fa1af489b8012517b1a105 Mon Sep 17 00:00:00 2001 From: ankonym Date: Tue, 1 Sep 2015 10:48:25 +0200 Subject: Fix several test failures by stubbing invite code validation --- engines/billing/test/functional/customers_controller_test.rb | 2 +- engines/support/test/unit/account_extension_test.rb | 2 +- engines/support/test/unit/ticket_test.rb | 2 +- test/functional/identities_controller_test.rb | 2 +- test/functional/v1/messages_controller_test.rb | 2 +- test/integration/browser/account_test.rb | 3 +++ test/support/browser_integration_test.rb | 2 +- test/unit/tmp_user_test.rb | 2 +- test/unit/token_test.rb | 2 +- 9 files changed, 11 insertions(+), 8 deletions(-) diff --git a/engines/billing/test/functional/customers_controller_test.rb b/engines/billing/test/functional/customers_controller_test.rb index 0dafe72..4d84fb0 100644 --- a/engines/billing/test/functional/customers_controller_test.rb +++ b/engines/billing/test/functional/customers_controller_test.rb @@ -5,7 +5,7 @@ class CustomersControllerTest < ActionController::TestCase tests CustomerController setup do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + InviteCodeValidator.any_instance.stubs(:validate) @user = FactoryGirl.create :user @other_user = FactoryGirl.create :user #FakeBraintree.clear! diff --git a/engines/support/test/unit/account_extension_test.rb b/engines/support/test/unit/account_extension_test.rb index dd7e77d..0ecb1aa 100644 --- a/engines/support/test/unit/account_extension_test.rb +++ b/engines/support/test/unit/account_extension_test.rb @@ -3,7 +3,7 @@ require 'test_helper' class AccountExtensionTest < ActiveSupport::TestCase setup do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + InviteCodeValidator.any_instance.stubs(:validate) end test "destroying an account triggers ticket destruction" do diff --git a/engines/support/test/unit/ticket_test.rb b/engines/support/test/unit/ticket_test.rb index 7727650..7b5281f 100644 --- a/engines/support/test/unit/ticket_test.rb +++ b/engines/support/test/unit/ticket_test.rb @@ -3,7 +3,7 @@ require 'test_helper' class TicketTest < ActiveSupport::TestCase setup do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + InviteCodeValidator.any_instance.stubs(:validate) end test "ticket with default attribs is valid" do diff --git a/test/functional/identities_controller_test.rb b/test/functional/identities_controller_test.rb index c900178..e491c52 100644 --- a/test/functional/identities_controller_test.rb +++ b/test/functional/identities_controller_test.rb @@ -3,7 +3,7 @@ require 'test_helper' class IdentitiesControllerTest < ActionController::TestCase setup do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + InviteCodeValidator.any_instance.stubs(:validate) end test "admin can list active and blocked ids" do diff --git a/test/functional/v1/messages_controller_test.rb b/test/functional/v1/messages_controller_test.rb index 455eebd..720d862 100644 --- a/test/functional/v1/messages_controller_test.rb +++ b/test/functional/v1/messages_controller_test.rb @@ -3,7 +3,7 @@ require 'test_helper' class V1::MessagesControllerTest < ActionController::TestCase setup do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + InviteCodeValidator.any_instance.stubs(:validate) @user = FactoryGirl.build(:user) @user.save @message = Message.new(:text => 'a test message') diff --git a/test/integration/browser/account_test.rb b/test/integration/browser/account_test.rb index 34eb72e..6ab9eb2 100644 --- a/test/integration/browser/account_test.rb +++ b/test/integration/browser/account_test.rb @@ -47,6 +47,7 @@ class AccountTest < BrowserIntegrationTest test "account destruction" do username, password = submit_signup + click_on I18n.t('account_settings') click_on I18n.t('destroy_my_account') assert page.has_content?(I18n.t('account_destroyed')) @@ -102,6 +103,8 @@ class AccountTest < BrowserIntegrationTest # trying to seed an invalid A for srp login test "detects attempt to circumvent SRP" do + InviteCodeValidator.any_instance.stubs(:validate) + user = FactoryGirl.create :user visit '/login' fill_in 'Username', with: user.login diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb index b4bc273..f20421d 100644 --- a/test/support/browser_integration_test.rb +++ b/test/support/browser_integration_test.rb @@ -60,7 +60,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest # currently this only works for tests with poltergeist. # ApiIntegrationTest has a working implementation for RackTest def login(user = nil) - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + InviteCodeValidator.any_instance.stubs(:validate) @user ||= user ||= FactoryGirl.create(:user) token = Token.create user_id: user.id page.driver.add_header "Authorization", %Q(Token token="#{token}") diff --git a/test/unit/tmp_user_test.rb b/test/unit/tmp_user_test.rb index 0a9ad68..9494377 100644 --- a/test/unit/tmp_user_test.rb +++ b/test/unit/tmp_user_test.rb @@ -3,7 +3,7 @@ require 'test_helper' class TmpUserTest < ActiveSupport::TestCase setup do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + InviteCodeValidator.any_instance.stubs(:validate) end test "test_user saved to tmp_users" do diff --git a/test/unit/token_test.rb b/test/unit/token_test.rb index dcf3fc4..51c8d8e 100644 --- a/test/unit/token_test.rb +++ b/test/unit/token_test.rb @@ -4,7 +4,7 @@ class TokenTest < ActiveSupport::TestCase include StubRecordHelper setup do - InviteCodeValidator.any_instance.stubs(:not_existent?).returns(false) + InviteCodeValidator.any_instance.stubs(:validate) @user = find_record :user end -- cgit v1.2.3 From b3762da8d8a06e164524a20d26293a8d5a9770d8 Mon Sep 17 00:00:00 2001 From: ankonym Date: Wed, 2 Sep 2015 16:03:48 +0200 Subject: Fix three unit tests by passing Factory Girl a valid invite code The tests were failing because of a hardcoded "testcode" string so during test setup we generate a valid code and pass it to Factory Girl --- test/unit/account_test.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/unit/account_test.rb b/test/unit/account_test.rb index b2bfe27..8c66853 100644 --- a/test/unit/account_test.rb +++ b/test/unit/account_test.rb @@ -2,12 +2,17 @@ require 'test_helper' class AccountTest < ActiveSupport::TestCase + setup do + @testcode = InviteCode.new + @testcode.save! + end + teardown do Identity.destroy_all_disabled end test "create a new account" do - user = Account.create(FactoryGirl.attributes_for(:user)) + user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) assert user.valid?, "unexpected errors: #{user.errors.inspect}" assert user.persisted? assert id = user.identity @@ -20,14 +25,14 @@ class AccountTest < ActiveSupport::TestCase # We keep an identity that will block the handle from being reused. assert_difference "Identity.count" do assert_no_difference "User.count" do - user = Account.create(FactoryGirl.attributes_for(:user)) + user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) user.account.destroy end end end test "change username and create alias" do - user = Account.create(FactoryGirl.attributes_for(:user)) + user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) old_id = user.identity old_email = user.email_address user.account.update(FactoryGirl.attributes_for(:user)) -- cgit v1.2.3 From 2f7d085c5747a1a68afc0d854fa087ffbb46f8e7 Mon Sep 17 00:00:00 2001 From: ankonym Date: Wed, 2 Sep 2015 17:01:16 +0200 Subject: Fix the remaining failures/errors in our tests Handing freshly generated invite codes to Factory Girl to make the tests pass --- engines/support/test/integration/create_ticket_test.rb | 9 +++++++-- test/integration/api/cert_test.rb | 1 + test/integration/api/smtp_cert_test.rb | 12 +++++++++--- test/integration/api/srp_test.rb | 7 ++++++- test/support/api_integration_test.rb | 7 ++++++- test/support/browser_integration_test.rb | 5 +++-- 6 files changed, 32 insertions(+), 9 deletions(-) diff --git a/engines/support/test/integration/create_ticket_test.rb b/engines/support/test/integration/create_ticket_test.rb index 90e9a8a..00f9a6b 100644 --- a/engines/support/test/integration/create_ticket_test.rb +++ b/engines/support/test/integration/create_ticket_test.rb @@ -2,6 +2,11 @@ require 'test_helper' class CreateTicketTest < BrowserIntegrationTest + setup do + @testcode = InviteCode.new + @testcode.save! + end + test "can submit ticket anonymously" do visit '/' click_on 'Get Help' @@ -29,7 +34,7 @@ class CreateTicketTest < BrowserIntegrationTest end test "prefills fields" do - login FactoryGirl.create(:premium_user) + login FactoryGirl.create(:premium_user, :invite_code => @testcode.invite_code) visit '/' click_on "Support Tickets" click_on "New Ticket" @@ -48,7 +53,7 @@ class CreateTicketTest < BrowserIntegrationTest end test "cleared email field should remain clear" do - login FactoryGirl.create(:premium_user) + login FactoryGirl.create(:premium_user, :invite_code => @testcode.invite_code) visit '/' click_on "Support Tickets" click_on "New Ticket" diff --git a/test/integration/api/cert_test.rb b/test/integration/api/cert_test.rb index 118fb9f..772901d 100644 --- a/test/integration/api/cert_test.rb +++ b/test/integration/api/cert_test.rb @@ -2,6 +2,7 @@ require 'test_helper' class CertTest < ApiIntegrationTest + test "retrieve eip cert" do login get '/1/cert', {}, RACK_ENV diff --git a/test/integration/api/smtp_cert_test.rb b/test/integration/api/smtp_cert_test.rb index 2f50ef3..681d509 100644 --- a/test/integration/api/smtp_cert_test.rb +++ b/test/integration/api/smtp_cert_test.rb @@ -3,8 +3,13 @@ require 'openssl' class SmtpCertTest < ApiIntegrationTest + setup do + @testcode = InviteCode.new + @testcode.save! + end + test "retrieve smtp cert" do - @user = FactoryGirl.create :user, effective_service_level_code: 2 + @user = FactoryGirl.create :user, effective_service_level_code: 2, :invite_code => @testcode.invite_code login post '/1/smtp_cert', {}, RACK_ENV assert_text_response @@ -15,7 +20,7 @@ class SmtpCertTest < ApiIntegrationTest end test "cert and key" do - @user = FactoryGirl.create :user, effective_service_level_code: 2 + @user = FactoryGirl.create :user, effective_service_level_code: 2, :invite_code => @testcode.invite_code login post '/1/smtp_cert', {}, RACK_ENV assert_text_response @@ -27,7 +32,7 @@ class SmtpCertTest < ApiIntegrationTest end test "fingerprint is stored with identity" do - @user = FactoryGirl.create :user, effective_service_level_code: 2 + @user = FactoryGirl.create :user, effective_service_level_code: 2, :invite_code => @testcode.invite_code login post '/1/smtp_cert', {}, RACK_ENV assert_text_response @@ -41,6 +46,7 @@ class SmtpCertTest < ApiIntegrationTest end test "fetching smtp certs requires email account" do + login post '/1/smtp_cert', {}, RACK_ENV assert_access_denied diff --git a/test/integration/api/srp_test.rb b/test/integration/api/srp_test.rb index ea06cde..463abcd 100644 --- a/test/integration/api/srp_test.rb +++ b/test/integration/api/srp_test.rb @@ -1,5 +1,10 @@ class SrpTest < RackTest + setup do + @testcode = InviteCode.new + @testcode.save! + end + teardown do if @user cleanup_user @@ -32,7 +37,7 @@ class SrpTest < RackTest attr_reader :server_auth - def register_user(login = "integration_test", password = 'srp, verify me!', invite_code = "testcode") + def register_user(login = "integration_test", password = 'srp, verify me!', invite_code = @testcode.invite_code) cleanup_user(login) post 'http://api.lvh.me:3000/1/users.json', user_params(login: login, password: password, invite_code: invite_code) diff --git a/test/support/api_integration_test.rb b/test/support/api_integration_test.rb index bd10f11..4077920 100644 --- a/test/support/api_integration_test.rb +++ b/test/support/api_integration_test.rb @@ -3,8 +3,13 @@ class ApiIntegrationTest < ActionDispatch::IntegrationTest DUMMY_TOKEN = Token.new RACK_ENV = {'HTTP_AUTHORIZATION' => %Q(Token token="#{DUMMY_TOKEN.to_s}")} + setup do + @testcode = InviteCode.new + @testcode.save! + end + def login(user = nil) - @user ||= user ||= FactoryGirl.create(:user) + @user ||= user ||= FactoryGirl.create(:user, :invite_code => @testcode.invite_code) # DUMMY_TOKEN will be frozen. So let's use a dup @token ||= DUMMY_TOKEN.dup # make sure @token is up to date if it already exists diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb index f20421d..34ec9a6 100644 --- a/test/support/browser_integration_test.rb +++ b/test/support/browser_integration_test.rb @@ -37,7 +37,8 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest setup do Capybara.current_driver = Capybara.javascript_driver page.driver.add_headers 'ACCEPT-LANGUAGE' => 'en-EN' - @invite_code = InviteCode.create(invite_code: "testcode") + @testcode = InviteCode.new + @testcode.save! end teardown do @@ -51,7 +52,7 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest visit '/users/new' fill_in 'Username', with: username fill_in 'Password', with: password - fill_in 'Invite code', with: "testcode" + fill_in 'Invite code', with: @testcode.invite_code fill_in 'Password confirmation', with: password click_on 'Sign Up' return username, password -- cgit v1.2.3 From d13e2d841a632385031a0b98a15773d8b90d05e9 Mon Sep 17 00:00:00 2001 From: ankonym Date: Wed, 2 Sep 2015 17:16:37 +0200 Subject: Fix cucumber tests by passing valid invite code --- features/step_definitions/auth_steps.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/features/step_definitions/auth_steps.rb b/features/step_definitions/auth_steps.rb index e75455a..52c92ee 100644 --- a/features/step_definitions/auth_steps.rb +++ b/features/step_definitions/auth_steps.rb @@ -1,5 +1,7 @@ Given /^I authenticated$/ do - @user = FactoryGirl.create(:user) + @testcode = InviteCode.new + @testcode.save! + @user = FactoryGirl.create(:user, :invite_code => @testcode.invite_code) @my_auth_token = Token.create user_id: @user.id end -- cgit v1.2.3 From 2ce3d14cfa77f985b6849dd4431db65e9abd0226 Mon Sep 17 00:00:00 2001 From: Aya Jaff Date: Fri, 4 Sep 2015 14:01:49 +0200 Subject: Fixed the signup bug that wrongly consumes the invite code. --- app/models/account.rb | 5 ++++- app/models/invite_code_validator.rb | 4 ---- test/unit/account_test.rb | 20 ++++++++++++++++++++ test/unit/invite_code_test.rb | 16 ---------------- test/unit/invite_code_validator_test.rb | 2 +- 5 files changed, 25 insertions(+), 22 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index a57e3f7..c398d93 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -19,8 +19,8 @@ class Account identity = nil user = nil user = User.new(attrs) - user.save + if !user.tmp? && user.persisted? identity = user.identity identity.user_id = user.id @@ -28,6 +28,9 @@ class Account identity.errors.each do |attr, msg| user.errors.add(attr, msg) end + user_invite_code = InviteCode.find_by_invite_code user.invite_code + user_invite_code.invite_count += 1 + user_invite_code.save end rescue StandardError => ex user.errors.add(:base, ex.to_s) if user diff --git a/app/models/invite_code_validator.rb b/app/models/invite_code_validator.rb index d03d510..127dc57 100644 --- a/app/models/invite_code_validator.rb +++ b/app/models/invite_code_validator.rb @@ -8,10 +8,6 @@ class InviteCodeValidator < ActiveModel::Validator elsif count_greater_than_zero?(user_invite_code.invite_count) add_error_to_user("This code has already been used", user) - - else - user_invite_code.invite_count += 1 - user_invite_code.save end end diff --git a/test/unit/account_test.rb b/test/unit/account_test.rb index 8c66853..0882c43 100644 --- a/test/unit/account_test.rb +++ b/test/unit/account_test.rb @@ -49,4 +49,24 @@ class AccountTest < ActiveSupport::TestCase user.account.destroy end + test "Invite code count goes up by 1 when the invite code is entered" do + + user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user_code = InviteCode.find_by_invite_code user.invite_code + user_code.save + user.save + assert user.persisted? + assert_equal 1, user_code.invite_count + + end + + test "Invite code stays zero when invite code is not used" do + #user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + invalid_user = FactoryGirl.build(:user, :invite_code => @testcode.invite_code) + invalid_user.save + user_code = InviteCode.find_by_invite_code invalid_user.invite_code + user_code.save + + assert_equal 0, user_code.invite_count + end end diff --git a/test/unit/invite_code_test.rb b/test/unit/invite_code_test.rb index 2c9e33f..1b6a002 100644 --- a/test/unit/invite_code_test.rb +++ b/test/unit/invite_code_test.rb @@ -20,22 +20,6 @@ class InviteCodeTest < ActiveSupport::TestCase assert_equal code1.invite_count, 0 end - test "Invite code count goes up by 1 when the invite code is entered" do - #TODO but onlz if there are no other errors on the form - - validator = InviteCodeValidator.new nil - - user = FactoryGirl.build :user - user_code = InviteCode.new - user_code.save - user.invite_code = user_code.invite_code - - validator.validate(user) - - user_code.reload - assert_equal 1, user_code.invite_count - - end test "Invite count >0 is not accepted for new account signup" do validator = InviteCodeValidator.new nil diff --git a/test/unit/invite_code_validator_test.rb b/test/unit/invite_code_validator_test.rb index 62768e3..75e691d 100644 --- a/test/unit/invite_code_validator_test.rb +++ b/test/unit/invite_code_validator_test.rb @@ -16,7 +16,7 @@ class InviteCodeValidatorTest < ActiveSupport::TestCase end test "trying to create a user with invalid invite code should add error" do - invalid_user = FactoryGirl.build(:user) + invalid_user = FactoryGirl.build(:user, :invite_code => "a non-existent code") invalid_user.valid? -- cgit v1.2.3 From c1cd099089c09b79cb7945be4c3283afed9ab3b3 Mon Sep 17 00:00:00 2001 From: ankonym Date: Thu, 10 Sep 2015 18:09:49 +0200 Subject: Removed the view_by__id from invite code test --- test/unit/invite_code_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/invite_code_test.rb b/test/unit/invite_code_test.rb index 1b6a002..b17d1bc 100644 --- a/test/unit/invite_code_test.rb +++ b/test/unit/invite_code_test.rb @@ -10,7 +10,7 @@ class InviteCodeTest < ActiveSupport::TestCase test "the invite code can be read from couch db correctly" do code1 = InviteCode.new code1.save - code2 = InviteCode.find_by__id code1.id + code2 = InviteCode.find_by_invite_code code1.invite_code assert_equal code1.invite_code, code2.invite_code end -- cgit v1.2.3 From ca591b482870c93674aaf454e90f56796da7d87d Mon Sep 17 00:00:00 2001 From: ankonym Date: Thu, 10 Sep 2015 18:10:28 +0200 Subject: Cleaned up code in invite_code_validator.rb --- app/models/invite_code_validator.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/models/invite_code_validator.rb b/app/models/invite_code_validator.rb index 127dc57..f96ca4a 100644 --- a/app/models/invite_code_validator.rb +++ b/app/models/invite_code_validator.rb @@ -3,22 +3,21 @@ class InviteCodeValidator < ActiveModel::Validator user_invite_code = InviteCode.find_by_invite_code user.invite_code - if not_existent?(user.invite_code) + if not_existent?(user_invite_code) add_error_to_user("This is not a valid code", user) - elsif count_greater_than_zero?(user_invite_code.invite_count) + elsif count_greater_than_zero?(user_invite_code) add_error_to_user("This code has already been used", user) end end private def not_existent?(code) - InviteCode.find_by_invite_code(code) == nil - + code == nil end def count_greater_than_zero?(code) - code > 0 + code.invite_count > 0 end def add_error_to_user(error, user) -- cgit v1.2.3 From 9adbde13619de8b2c300056b062d12f0961cb710 Mon Sep 17 00:00:00 2001 From: ankonym Date: Mon, 21 Sep 2015 18:34:04 +0200 Subject: Make invite code configurable Through the config param 'invite_required', providers can decide whether users need to provide an invite code upon signup. The default setting is false. --- app/models/account.rb | 9 +++++--- app/models/user.rb | 2 +- app/views/users/new.html.haml | 5 ++++- config/defaults.yml | 1 + test/integration/browser/account_test.rb | 18 +++++++++++++++- test/support/browser_integration_test.rb | 35 ++++++++++++++++++++++++-------- test/unit/account_test.rb | 27 ++++++++++++++++-------- test/unit/invite_code_validator_test.rb | 4 ++++ 8 files changed, 78 insertions(+), 23 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index c398d93..a5cd833 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -28,9 +28,12 @@ class Account identity.errors.each do |attr, msg| user.errors.add(attr, msg) end - user_invite_code = InviteCode.find_by_invite_code user.invite_code - user_invite_code.invite_count += 1 - user_invite_code.save + + if APP_CONFIG[:invite_required] + user_invite_code = InviteCode.find_by_invite_code user.invite_code + user_invite_code.invite_count += 1 + user_invite_code.save + end end rescue StandardError => ex user.errors.add(:base, ex.to_s) if user diff --git a/app/models/user.rb b/app/models/user.rb index c0079d5..3daee0f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -40,7 +40,7 @@ class User < CouchRest::Model::Base :mx_with_fallback => true - validates_with InviteCodeValidator, on: :create + validates_with InviteCodeValidator, on: :create, if: -> {APP_CONFIG[:invite_required]} timestamps! diff --git a/app/views/users/new.html.haml b/app/views/users/new.html.haml index cd4bf70..b44f77c 100644 --- a/app/views/users/new.html.haml +++ b/app/views/users/new.html.haml @@ -17,8 +17,11 @@ = f.input :login, :label => t(:username), :required => false, :input_html => { :id => :srp_username } = f.input :password, :required => false, :validate => true, :input_html => { :id => :srp_password } = f.input :password_confirmation, :required => false, :validate => true, :input_html => { :id => :srp_password_confirmation } - = f.input :invite_code, :input_html => { :id => :srp_invite_code } + - if APP_CONFIG[:invite_required] + = f.input :invite_code, :input_html => { :id => :srp_invite_code } + - else + = f.input :invite_code, :as => "hidden", :input_html => { :value => " ", :id => :srp_invite_code } = f.button :wrapped, cancel: home_path -# diff --git a/config/defaults.yml b/config/defaults.yml index dfa2a9a..906b446 100644 --- a/config/defaults.yml +++ b/config/defaults.yml @@ -82,6 +82,7 @@ common: &common - support - billing allow_registration: true + invite_required: false config_file_paths: soledad-service: 'public/1/config/soledad-service.json' eip-service: 'public/1/config/eip-service.json' diff --git a/test/integration/browser/account_test.rb b/test/integration/browser/account_test.rb index 6ab9eb2..cbe7ba9 100644 --- a/test/integration/browser/account_test.rb +++ b/test/integration/browser/account_test.rb @@ -6,7 +6,7 @@ class AccountTest < BrowserIntegrationTest Identity.destroy_all_disabled end - test "signup successfully" do + test "signup successfully when invited" do username, password = submit_signup assert page.has_content?("Welcome #{username}") click_on 'Log Out' @@ -16,6 +16,22 @@ class AccountTest < BrowserIntegrationTest user.account.destroy end + test "signup successfully without invitation" do + with_config invite_required: false 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' + + assert page.has_content?("Welcome #{username}") + end + end + test "signup with username ending in dot json" do username = Faker::Internet.user_name + '.json' submit_signup username diff --git a/test/support/browser_integration_test.rb b/test/support/browser_integration_test.rb index 34ec9a6..35887cc 100644 --- a/test/support/browser_integration_test.rb +++ b/test/support/browser_integration_test.rb @@ -47,15 +47,32 @@ class BrowserIntegrationTest < ActionDispatch::IntegrationTest end def submit_signup(username = nil, password = nil) - username ||= "test_#{SecureRandom.urlsafe_base64}".downcase - password ||= SecureRandom.base64 - visit '/users/new' - fill_in 'Username', with: username - fill_in 'Password', with: password - fill_in 'Invite code', with: @testcode.invite_code - fill_in 'Password confirmation', with: password - click_on 'Sign Up' - return username, password + + with_config invite_required: true 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 'Invite code', with: @testcode.invite_code + fill_in 'Password confirmation', with: password + click_on 'Sign Up' + return username, password + end + + with_config invite_required: false 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' + return username, password + end + end # currently this only works for tests with poltergeist. diff --git a/test/unit/account_test.rb b/test/unit/account_test.rb index 0882c43..6b814b6 100644 --- a/test/unit/account_test.rb +++ b/test/unit/account_test.rb @@ -11,7 +11,7 @@ class AccountTest < ActiveSupport::TestCase Identity.destroy_all_disabled end - test "create a new account" do + test "create a new account when invited" do user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) assert user.valid?, "unexpected errors: #{user.errors.inspect}" assert user.persisted? @@ -21,6 +21,16 @@ class AccountTest < ActiveSupport::TestCase user.account.destroy end + test "create a new account" do + with_config invite_required: false do + user = Account.create(FactoryGirl.attributes_for(:user)) + assert user.valid?, "unexpected errors: #{user.errors.inspect}" + assert user.persisted? + user.account.destroy + end + end + + test "create and remove a user account" do # We keep an identity that will block the handle from being reused. assert_difference "Identity.count" do @@ -50,13 +60,14 @@ class AccountTest < ActiveSupport::TestCase end test "Invite code count goes up by 1 when the invite code is entered" do - - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) - user_code = InviteCode.find_by_invite_code user.invite_code - user_code.save - user.save - assert user.persisted? - assert_equal 1, user_code.invite_count + with_config invite_required: true do + user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user_code = InviteCode.find_by_invite_code user.invite_code + user_code.save + user.save + assert user.persisted? + assert_equal 1, user_code.invite_count + end end diff --git a/test/unit/invite_code_validator_test.rb b/test/unit/invite_code_validator_test.rb index 75e691d..ee8f1b3 100644 --- a/test/unit/invite_code_validator_test.rb +++ b/test/unit/invite_code_validator_test.rb @@ -2,9 +2,11 @@ require 'test_helper' class InviteCodeValidatorTest < ActiveSupport::TestCase test "user should not be created with invalid invite code" do + with_config invite_required: true do invalid_user = FactoryGirl.build(:user) assert !invalid_user.valid? + end end test "user should be created with valid invite code" do @@ -16,11 +18,13 @@ class InviteCodeValidatorTest < ActiveSupport::TestCase end test "trying to create a user with invalid invite code should add error" do + with_config invite_required: true do invalid_user = FactoryGirl.build(:user, :invite_code => "a non-existent code") invalid_user.valid? errors = {invite_code: ["This is not a valid code"]} assert_equal errors, invalid_user.errors.messages + end end end \ No newline at end of file -- cgit v1.2.3 From d4f10a8d47572bcab4c44878b952146732d64d2e Mon Sep 17 00:00:00 2001 From: ankonym Date: Mon, 28 Sep 2015 17:43:34 +0200 Subject: Add localization labels to signup form and user.en.yml Added the necessary labels to allow the localization of the signup form and the labels to users.en.yml for localization --- app/views/users/new.html.haml | 6 +++--- config/locales/en/users.en.yml | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/views/users/new.html.haml b/app/views/users/new.html.haml index b44f77c..bc0b1af 100644 --- a/app/views/users/new.html.haml +++ b/app/views/users/new.html.haml @@ -15,11 +15,11 @@ = render :partial => 'warnings' = simple_form_for(@user, form_options) do |f| = f.input :login, :label => t(:username), :required => false, :input_html => { :id => :srp_username } - = f.input :password, :required => false, :validate => true, :input_html => { :id => :srp_password } - = f.input :password_confirmation, :required => false, :validate => true, :input_html => { :id => :srp_password_confirmation } + = f.input :password, :label => t(:password), :required => false, :validate => true, :input_html => { :id => :srp_password } + = f.input :password_confirmation, :label => t(:password_confirmation), :required => false, :validate => true, :input_html => { :id => :srp_password_confirmation } - if APP_CONFIG[:invite_required] - = f.input :invite_code, :input_html => { :id => :srp_invite_code } + = f.input :invite_code, :label => t(:invite_code), :input_html => { :id => :srp_invite_code } - else = f.input :invite_code, :as => "hidden", :input_html => { :value => " ", :id => :srp_invite_code } diff --git a/config/locales/en/users.en.yml b/config/locales/en/users.en.yml index 89307dd..4c6bbc0 100644 --- a/config/locales/en/users.en.yml +++ b/config/locales/en/users.en.yml @@ -8,6 +8,8 @@ en: account_settings: "Account Settings" username: "Username" password: "Password" + password_confirmation: "Password confirmation" + invite_code: "Invite code" change_password: "Change Password" invalid_user_pass: "Not a valid username/password combination" invalid_ephemeral: "Invalid random key used. This looked like an attempt to hack the site to us. If it wasn't please contact support so we can look into the issue." -- cgit v1.2.3