From de2fad9c1104389cafb262c6f86f03935c377b31 Mon Sep 17 00:00:00 2001 From: Caio SBA Date: Fri, 30 Sep 2011 01:24:56 -0300 Subject: [PATCH] Done --- app/controllers/public/account_controller.rb | 11 ++++++++--- app/models/user.rb | 39 ++++++++++++++++++++++++++++++++++++++- app/views/user/mailer/activation_code.rhtml | 9 +++++++++ db/migrate/20110824192153_add_activated_at_to_users.rb | 16 ++++++++++++++++ features/my_network_block.feature | 7 ++++--- features/signup.feature | 12 ++++++++++++ features/step_definitions/noosfero_steps.rb | 20 +++++++++++++++++++- test/fixtures/users.yml | 8 ++++---- test/functional/account_controller_test.rb | 28 +++++++++++++++++++++++++++- test/functional/profile_controller_test.rb | 4 ++++ test/integration/editing_person_info_test.rb | 2 ++ test/integration/enterprise_registration_test.rb | 1 + test/integration/forgot_password_test.rb | 2 +- test/integration/manage_documents_test.rb | 3 +++ test/integration/signup_test.rb | 5 ++++- test/integration/user_registers_at_the_application_test.rb | 5 +---- test/unit/user_test.rb | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 17 files changed, 217 insertions(+), 20 deletions(-) create mode 100644 app/views/user/mailer/activation_code.rhtml create mode 100644 db/migrate/20110824192153_add_activated_at_to_users.rb diff --git a/app/controllers/public/account_controller.rb b/app/controllers/public/account_controller.rb index 256442a..39e772d 100644 --- a/app/controllers/public/account_controller.rb +++ b/app/controllers/public/account_controller.rb @@ -16,6 +16,12 @@ class AccountController < ApplicationController end end + def activate + @user = User.find_by_activation_code(params[:activation_code]) if params[:activation_code] + session[:notice] = (@user and @user.activate) ? _("Your account has been activated, now you can log in!") : _("It looks like you're trying to activate an account. Perhaps have already activated this account?") + redirect_back_or_default(:controller => 'home') + end + # action to perform login to the application def login @user = User.new @@ -60,7 +66,6 @@ class AccountController < ApplicationController @person.environment = @user.environment if request.post? && params[self.icaptcha_field].blank? @user.signup! - self.current_user = @user owner_role = Role.find_by_name('owner') @user.person.affiliate(@user.person, [owner_role]) if owner_role invitation = Task.find_by_code(@invitation_code) @@ -68,8 +73,8 @@ class AccountController < ApplicationController invitation.update_attributes!({:friend => @user.person}) invitation.finish end - session[:notice] = _("Thanks for signing up!") - go_to_initial_page if redirect? + session[:notice] = _("Thanks for signing up! Now go to your e-mail and activate your account.") + redirect_back_or_default(:controller => 'home') end rescue ActiveRecord::RecordInvalid @person.valid? diff --git a/app/models/user.rb b/app/models/user.rb index d9bf605..e991924 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -21,6 +21,8 @@ class User < ActiveRecord::Base end end + before_create :make_activation_code + before_create do |user| if user.environment.nil? user.environment = Environment.default @@ -31,8 +33,10 @@ class User < ActiveRecord::Base user.person ||= Person.new user.person.attributes = user.person_data.merge(:identifier => user.login, :user_id => user.id, :environment_id => user.environment_id) user.person.name ||= user.login + user.person.visible = false unless user.activated? user.person.save! end + after_create :deliver_activation_code attr_writer :person_data def person_data @@ -55,6 +59,17 @@ class User < ActiveRecord::Base :environment => user.environment.name, :url => url_for(:host => user.environment.default_hostname, :controller => 'home') end + + def activation_code(user) + recipients user.email + + from "#{user.environment.name} <#{user.environment.contact_email}>" + subject _("[%s] Activate your account") % [user.environment.name] + body :recipient => user.name, + :activation_code => user.activation_code, + :environment => user.environment.name, + :url => user.environment.top_url + end end def signup! @@ -67,6 +82,8 @@ class User < ActiveRecord::Base has_one :person, :dependent => :destroy belongs_to :environment + attr_protected :activated_at + # Virtual attribute for the unencrypted password attr_accessor :password @@ -87,10 +104,22 @@ class User < ActiveRecord::Base # Authenticates a user by their login name and unencrypted password. Returns the user or nil. def self.authenticate(login, password, environment = nil) environment ||= Environment.default - u = find_by_login_and_environment_id(login, environment.id) # need to get the salt + u = first :conditions => ['login = ? AND environment_id = ? AND activated_at IS NOT NULL', login, environment.id] # need to get the salt u && u.authenticated?(password) ? u : nil end + # Activates the user in the database. + def activate + self.activated_at = Time.now.utc + self.activation_code = nil + self.person.visible = true + self.person.save && self.save + end + + def activated? + self.activation_code.nil? && !self.activated_at.nil? + end + class UnsupportedEncryptionType < Exception; end def self.system_encryption_method @@ -253,4 +282,12 @@ class User < ActiveRecord::Base def password_required? crypted_password.blank? || !password.blank? end + + def make_activation_code + self.activation_code = Digest::SHA1.hexdigest(Time.now.to_s.split(//).sort_by{rand}.join) + end + + def deliver_activation_code + User::Mailer.deliver_activation_code(self) unless self.activation_code.blank? + end end diff --git a/app/views/user/mailer/activation_code.rhtml b/app/views/user/mailer/activation_code.rhtml new file mode 100644 index 0000000..3f35a3c --- /dev/null +++ b/app/views/user/mailer/activation_code.rhtml @@ -0,0 +1,9 @@ +<%= _('Hi, %{recipient}!') % { :recipient => @recipient } %> + +<%= word_wrap(_('Welcome to %{environment}! To activate your account, follow the link: %{activation_url}') % { :environment => @environment, :activation_url => @url + url_for(:controller => :account, :action => :activate, :activation_code => @activation_code) }) %> + +<%= _("Greetings,") %> + +-- +<%= _('%s team.') % @environment %> +<%= url_for @url %> diff --git a/db/migrate/20110824192153_add_activated_at_to_users.rb b/db/migrate/20110824192153_add_activated_at_to_users.rb new file mode 100644 index 0000000..764a814 --- /dev/null +++ b/db/migrate/20110824192153_add_activated_at_to_users.rb @@ -0,0 +1,16 @@ +class AddActivatedAtToUsers < ActiveRecord::Migration + def self.up + add_column :users, :activation_code, :string, :limit => 40 + add_column :users, :activated_at, :datetime + if ActiveRecord::Base.connection.adapter_name == 'SQLite' + execute "update users set activated_at = datetime();" + else + execute "update users set activated_at = now();" + end + end + + def self.down + remove_column :users, :activation_code + remove_column :users, :activated_at + end +end diff --git a/features/my_network_block.feature b/features/my_network_block.feature index f9e1d16..b4b61d3 100644 --- a/features/my_network_block.feature +++ b/features/my_network_block.feature @@ -56,9 +56,10 @@ Feature: my_network_block Scenario: not display how many invisible friends I have Given the following users - | login | name | visible | - | mariasilva | Maria Silva | true | - | josesilva | Jose Silva | false | + | login | name | + | mariasilva | Maria Silva | + | josesilva | Jose Silva | + And "josesilva" is invisible And "joaosilva" is friend of "mariasilva" And I am logged in as "joaosilva" When I go to Joao Silva's homepage diff --git a/features/signup.feature b/features/signup.feature index 43e49b0..df9acb7 100644 --- a/features/signup.feature +++ b/features/signup.feature @@ -14,6 +14,18 @@ Feature: signup | Password confirmation | secret | | Name | José da Silva | And I press "Sign up" + Then I should not be logged in + And I should receive an e-mail on josesilva@example.com + When I go to login page + And I fill in "Username" with "josesilva" + And I fill in "Password" with "secret" + And I press "Log in" + Then I should not be logged in + When José da Silva's account is activated + And I go to login page + And I fill in "Username" with "josesilva" + And I fill in "Password" with "secret" + And I press "Log in" Then I should be logged in as "josesilva" Scenario: be redirected if user goes to signup page and is logged diff --git a/features/step_definitions/noosfero_steps.rb b/features/step_definitions/noosfero_steps.rb index bc9efe1..634a866 100644 --- a/features/step_definitions/noosfero_steps.rb +++ b/features/step_definitions/noosfero_steps.rb @@ -7,10 +7,14 @@ Given /^the following users?$/ do |table| table.hashes.each do |item| person_data = item.dup person_data.delete("login") - User.create!(:login => item[:login], :password => '123456', :password_confirmation => '123456', :email => item[:login] + "@example.com", :person_data => person_data) + User.create!(:login => item[:login], :password => '123456', :password_confirmation => '123456', :email => item[:login] + "@example.com", :person_data => person_data).activate end end +Given /^"(.+)" is (invisible|visible)$/ do |user, visibility| + User.find_by_login(user).person.update_attributes(:visible => (visibility == 'visible')) +end + Given /^"(.+)" is (online|offline|busy) in chat$/ do |user, status| status = {'online' => 'chat', 'offline' => '', 'busy' => 'dnd'}[status] User.find_by_login(user).update_attributes(:chat_status => status, :chat_status_at => DateTime.now) @@ -195,6 +199,7 @@ end Given /^I am logged in as admin$/ do visit('/account/logout') user = User.create!(:login => 'admin_user', :password => '123456', :password_confirmation => '123456', :email => 'admin_user@example.com') + user.activate e = Environment.default e.add_admin(user.person) visit('/account/login') @@ -345,6 +350,10 @@ Then /^I should be logged in as "(.+)"$/ do |login| User.find(session[:user]).login.should == login end +Then /^I should not be logged in$/ do + session[:user].nil? +end + Given /^the profile "(.+)" has no blocks$/ do |profile| profile = Profile[profile] profile.boxes.map do |box| @@ -416,3 +425,12 @@ end Given /^skip comments captcha$/ do Comment.any_instance.stubs(:skip_captcha?).returns(true) end + +When /^([^\']*)'s account is activated$/ do |person| + Person.find_by_name(person).user.activate +end + +Then /^I should receive an e-mail on (.*)$/ do |address| + last_mail = ActionMailer::Base.deliveries.last + last_mail['to'].to_s.should == address +end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index deac2b6..880e840 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -6,7 +6,7 @@ johndoe: crypted_password: 00742970dc9e6319f8019fd54864d3ea740f04b1 # test #crypted_password: "ce2/iFrNtQ8=\n" # johndoe, use only if you're using 2-way encryption created_at: <%= 5.days.ago.to_s :db %> - # activated_at: <%= 5.days.ago.to_s :db %> # only if you're activating new signups + activated_at: <%= 5.days.ago.to_s :db %> # only if you're activating new signups environment_id: 1 joerandomhacker: id: 2 @@ -14,7 +14,7 @@ joerandomhacker: email: joerandomhacker@localhost.localdomain salt: 7e3041ebc2fc05a40c60028e2c4901a81035d3cd crypted_password: 00742970dc9e6319f8019fd54864d3ea740f04b1 # test - # activation_code: aaronscode # only if you're activating new signups + activated_at: <%= 5.days.ago.to_s :db %> # only if you're activating new signups created_at: <%= 1.days.ago.to_s :db %> environment_id: 1 ze: @@ -23,7 +23,7 @@ ze: email: ze@localhost.localdomain salt: 7e3041ebc2fc05a40c60028e2c4901a81035d3cd crypted_password: 00742970dc9e6319f8019fd54864d3ea740f04b1 # test - # activation_code: aaronscode # only if you're activating new signups + activated_at: <%= 5.days.ago.to_s :db %> # only if you're activating new signups created_at: <%= 1.days.ago.to_s :db %> environment_id: 1 other_ze: @@ -32,6 +32,6 @@ other_ze: email: ze@localhost.localdomain salt: 7e3041ebc2fc05a40c60028e2c4901a81035d3cd crypted_password: 00742970dc9e6319f8019fd54864d3ea740f04b1 # test - # activation_code: aaronscode # only if you're activating new signups + activated_at: <%= 5.days.ago.to_s :db %> # only if you're activating new signups created_at: <%= 1.days.ago.to_s :db %> environment_id: 2 diff --git a/test/functional/account_controller_test.rb b/test/functional/account_controller_test.rb index f62e024..baa627d 100644 --- a/test/functional/account_controller_test.rb +++ b/test/functional/account_controller_test.rb @@ -643,7 +643,7 @@ class AccountControllerTest < Test::Unit::TestCase Person.any_instance.stubs(:required_fields).returns(['organization']) assert_difference User, :count do post :signup, :user => { :login => 'testuser', :password => '123456', :password_confirmation => '123456', :email => 'testuser@example.com' }, :profile_data => { :organization => 'example.com' } - assert_redirected_to :controller => 'profile_editor', :profile => 'testuser' + assert_redirected_to :controller => 'home' end assert_equal 'example.com', Person['testuser'].organization end @@ -689,6 +689,32 @@ class AccountControllerTest < Test::Unit::TestCase assert_equal User.find_by_login('ze').data_hash.merge({ 'foo' => 'bar', 'test' => 5 }), ActiveSupport::JSON.decode(@response.body) end + should 'activate user when activation code is present and correct' do + user = User.create! :login => 'testuser', :password => 'test123', :password_confirmation => 'test123', :email => 'test@test.org' + get :activate, :activation_code => user.activation_code + post :login, :user => {:login => 'testuser', :password => 'test123'} + assert_not_nil session[:user] + assert_redirected_to :controller => 'profile_editor', :profile => 'testuser' + end + + should 'not activate user when activation code is missing' do + @request.env["HTTP_REFERER"] = '/bli' + user = User.create! :login => 'testuser', :password => 'test123', :password_confirmation => 'test123', :email => 'test@test.org' + get :activate + post :login, :user => {:login => 'testuser', :password => 'test123'} + assert_nil session[:user] + assert_redirected_to '/bli' + end + + should 'not activate user when activation code is incorrect' do + @request.env["HTTP_REFERER"] = '/bli' + user = User.create! :login => 'testuser', :password => 'test123', :password_confirmation => 'test123', :email => 'test@test.org' + get :activate, :activation_code => 'wrongcode' + post :login, :user => {:login => 'testuser', :password => 'test123'} + assert_nil session[:user] + assert_redirected_to '/bli' + end + protected def new_user(options = {}, extra_options ={}) data = {:profile_data => person_data} diff --git a/test/functional/profile_controller_test.rb b/test/functional/profile_controller_test.rb index 4684391..b8f9ef1 100644 --- a/test/functional/profile_controller_test.rb +++ b/test/functional/profile_controller_test.rb @@ -189,8 +189,10 @@ class ProfileControllerTest < Test::Unit::TestCase end should 'display add friend button' do + @profile.user.activate login_as(@profile.identifier) friend = create_user_full('friendtestuser').person + friend.user.activate friend.boxes.first.blocks << block = ProfileInfoBlock.create! get :profile_info, :profile => friend.identifier, :block_id => block.id assert_match /Add friend/, @response.body @@ -318,6 +320,7 @@ class ProfileControllerTest < Test::Unit::TestCase should 'display contact button only if friends' do friend = create_user_full('friend_user').person + friend.user.activate friend.boxes.first.blocks << block = ProfileInfoBlock.create! @profile.add_friend(friend) env = Environment.default @@ -338,6 +341,7 @@ class ProfileControllerTest < Test::Unit::TestCase should 'display contact button only if friends and its enable in environment' do friend = create_user_full('friend_user').person + friend.user.activate friend.boxes.first.blocks << block = ProfileInfoBlock.create! env = Environment.default env.disable('disable_contact_person') diff --git a/test/integration/editing_person_info_test.rb b/test/integration/editing_person_info_test.rb index 3701773..9873f2a 100644 --- a/test/integration/editing_person_info_test.rb +++ b/test/integration/editing_person_info_test.rb @@ -8,6 +8,8 @@ class EditingPersonInfoTest < ActionController::IntegrationTest profile = create_user('user_ze', :password => 'test', :password_confirmation => 'test').person + profile.user.activate + login(profile.identifier, 'test') get "/myprofile/#{profile.identifier}" diff --git a/test/integration/enterprise_registration_test.rb b/test/integration/enterprise_registration_test.rb index 26d0d59..f380193 100644 --- a/test/integration/enterprise_registration_test.rb +++ b/test/integration/enterprise_registration_test.rb @@ -50,6 +50,7 @@ class EnterpriseRegistrationTest < ActionController::IntegrationTest # steps done by the validator validator = create_user_with_permission('validator', 'validate_enterprise', org) + validator.user.activate login 'validator', 'validator' get "/myprofile/myorg/enterprise_validation" diff --git a/test/integration/forgot_password_test.rb b/test/integration/forgot_password_test.rb index c227c6f..1fb5c6f 100644 --- a/test/integration/forgot_password_test.rb +++ b/test/integration/forgot_password_test.rb @@ -12,7 +12,7 @@ class ForgotPasswordTest < ActionController::IntegrationTest Profile.destroy_all ChangePassword.destroy_all - create_user('forgotten', :password => 'test', :password_confirmation => 'test', :email => 'forgotten@localhost.localdomain') + create_user('forgotten', :password => 'test', :password_confirmation => 'test', :email => 'forgotten@localhost.localdomain').activate get '/account/forgot_password' diff --git a/test/integration/manage_documents_test.rb b/test/integration/manage_documents_test.rb index f2575b3..e561596 100644 --- a/test/integration/manage_documents_test.rb +++ b/test/integration/manage_documents_test.rb @@ -6,6 +6,7 @@ class ManageDocumentsTest < ActionController::IntegrationTest def test_creation_of_a_new_article user = create_user('myuser') + user.activate login('myuser', 'myuser') assert_tag :tag => 'a', :attributes => { :href => "#{user.environment.top_url}/myprofile\/{login}" } @@ -34,6 +35,7 @@ class ManageDocumentsTest < ActionController::IntegrationTest def test_update_of_an_existing_article profile = create_user('myuser').person + profile.user.activate article = create_article(profile, :name => 'my-article') article.save! @@ -67,6 +69,7 @@ class ManageDocumentsTest < ActionController::IntegrationTest def test_removing_an_article profile = create_user('myuser').person + profile.user.activate article = create_article(profile, :name => 'my-article') article.save! diff --git a/test/integration/signup_test.rb b/test/integration/signup_test.rb index e09e56d..d8ab192 100644 --- a/test/integration/signup_test.rb +++ b/test/integration/signup_test.rb @@ -11,6 +11,7 @@ class SignupTest < ActionController::IntegrationTest Environment.default.update_attributes(:terms_of_use => 'You agree to not be annoying.') count = User.count + mail_count = ActionMailer::Base.deliveries.count get '/account/signup' assert_response :success @@ -20,13 +21,15 @@ class SignupTest < ActionController::IntegrationTest assert_response :success assert_template 'signup' assert_equal count, User.count + assert_equal mail_count, ActionMailer::Base.deliveries.count post '/account/signup', :user => { :login => 'shouldaccepterms', :password => 'test', :password_confirmation => 'test', :email => 'shouldaccepterms@example.com', :terms_accepted => '1' }, :profile_data => person_data assert_response :redirect follow_redirect! - assert_response :success + assert_response :redirect assert_equal count + 1, User.count + assert_equal mail_count + 1, ActionMailer::Base.deliveries.count end diff --git a/test/integration/user_registers_at_the_application_test.rb b/test/integration/user_registers_at_the_application_test.rb index a049f0c..04b326f 100644 --- a/test/integration/user_registers_at_the_application_test.rb +++ b/test/integration/user_registers_at_the_application_test.rb @@ -14,12 +14,9 @@ class UserRegistersAtTheApplicationTest < ActionController::IntegrationTest post '/account/signup', :user => { :login => 'mylogin', :password => 'mypassword', :password_confirmation => 'mypassword', :email => 'mylogin@example.com' } assert_response :redirect - assert_redirected_to '/myprofile/mylogin' - # user is logged in right after the registration follow_redirect! - assert_no_tag :tag => 'a', :attributes => { :href => '/account/login_popup' } - assert_tag :tag => 'a', :attributes => { :href => '/account/logout' } + assert_tag :tag => 'a', :attributes => { :href => /^\/account\/login/ } end def test_trying_an_existing_login_name diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 95b5112..10916b7 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -346,7 +346,7 @@ class UserTest < Test::Unit::TestCase Person.any_instance.stubs(:profile_custom_icon).returns('/custom_icon') expected_hash = { 'coldplayfriend' => { - 'avatar' => '/custom_icon', 'name' => 'coldplayfriend', 'jid' => 'coldplayfriend@colivre.net/coldplayfriend', 'status' => 'chat' + 'avatar' => '/custom_icon', 'name' => 'coldplayfriend', 'jid' => 'coldplayfriend@' + Environment.default.default_hostname + '/coldplayfriend', 'status' => 'chat' } } assert_equal expected_hash, person.user.data_hash['friends_list'] @@ -409,6 +409,69 @@ class UserTest < Test::Unit::TestCase assert_equal 'testuser', user.name end + should 'have activation code' do + user = create_user('testuser') + assert_respond_to user, :activation_code + end + + should 'have activated at' do + user = create_user('testuser') + assert_respond_to user, :activated_at + end + + should 'make activation code before creation' do + assert_not_nil new_user.activation_code + end + + should 'deliver e-mail with activation code after creation' do + assert_difference(ActionMailer::Base.deliveries, :size, 1) do + new_user :email => 'pending@activation.com' + end + assert_equal 'pending@activation.com', ActionMailer::Base.deliveries.last['to'].to_s + end + + should 'not mass assign activated at' do + user = User.new :activated_at => 5.days.ago + assert_nil user.activated_at + user.attributes = { :activated_at => 5.days.ago } + assert_nil user.activated_at + user.activated_at = 5.days.ago + assert_not_nil user.activated_at + end + + should 'authenticate an activated user' do + user = new_user :login => 'testuser', :password => 'test123', :password_confirmation => 'test123' + user.activate + assert_equal user, User.authenticate('testuser', 'test123') + end + + should 'not authenticate a not activated user' do + user = new_user :login => 'testuser', :password => 'test123', :password_confirmation => 'test123' + assert_nil User.authenticate('testuser', 'test123') + end + + should 'have activation code but no activated at when created' do + user = new_user + assert_not_nil user.activation_code + assert_nil user.activated_at + assert !user.person.visible + end + + should 'activate an user' do + user = new_user + assert user.activate + assert_nil user.activation_code + assert_not_nil user.activated_at + assert user.person.visible + end + + should 'return if the user is activated' do + user = new_user + assert !user.activated? + user.activate + assert user.activated? + end + protected def new_user(options = {}) user = User.new({ :login => 'quire', :email => 'quire@example.com', :password => 'quire', :password_confirmation => 'quire' }.merge(options)) -- libgit2 0.21.2