diff --git a/README.md b/README.md index 5e841baae..7d3f6842a 100644 --- a/README.md +++ b/README.md @@ -61,4 +61,4 @@ Code published under AFFERO GPL v3 (see [LICENSE-AGPLv3.txt](LICENSE-AGPLv3.txt) ## Contributions -See [CONTRIBUTING_EN.md](CONTRIBUTING_EN.md) +See [CONTRIBUTING_EN.md](CONTRIBUTING_EN.md) \ No newline at end of file diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 346f6aa34..3fa634898 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -82,10 +82,9 @@ class ApplicationController < ActionController::Base end def ensure_signup_complete - # Ensure we don't go into an infinite loop - return if action_name.in? %w(finish_signup do_finish_signup) - - if user_signed_in? && !current_user.email_provided? + if user_signed_in? && + current_user.registering_with_oauth && + %w(finish_signup do_finish_signup).exclude?(action_name) redirect_to finish_signup_path end end diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 423ecedad..0bdd2a801 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -1,31 +1,49 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController - def self.provides_callback_for(provider) - class_eval %Q{ - def #{provider} - @user = User.find_for_oauth(env["omniauth.auth"], current_user) - - if @user.persisted? - sign_in_and_redirect @user, event: :authentication - set_flash_message(:notice, :success, kind: "#{provider}".capitalize) if is_navigational_format? - else - session["devise.#{provider}_data"] = env["omniauth.auth"] - redirect_to new_user_registration_url - end - end - } + def twitter + sign_in_with :twitter_login, :twitter end - [:twitter, :facebook, :google_oauth2].each do |provider| - provides_callback_for provider + def facebook + sign_in_with :facebook_login, :facebook + end + + def google_oauth2 + sign_in_with :google_login, :google_oauth2 end def after_sign_in_path_for(resource) - if resource.email_provided? - super(resource) - else + if resource.registering_with_oauth finish_signup_path + else + super(resource) end end + private + + def sign_in_with(feature, provider) + raise ActionController::RoutingError.new('Not Found') unless Setting["feature.#{feature}"] + + auth = env["omniauth.auth"] + + identity = Identity.first_or_create_from_oauth(auth) + @user = current_user || identity.user || User.first_or_initialize_for_oauth(auth) + + if save_user(@user) + identity.update(user: @user) + sign_in_and_redirect @user, event: :authentication + set_flash_message(:notice, :success, kind: "#{provider}".capitalize) if is_navigational_format? + else + session["devise.#{provider}_data"] = auth + redirect_to new_user_registration_url + end + end + + def save_user(user) + @user.save || + @user.save_requiring_finish_signup || + @user.save_requiring_finish_signup_without_email + end + end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index b25418db4..8a4aca2a8 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -24,13 +24,16 @@ class Users::RegistrationsController < Devise::RegistrationsController end def finish_signup + current_user.registering_with_oauth = false + current_user.email = current_user.oauth_email if current_user.email.blank? + current_user.validate end def do_finish_signup + current_user.registering_with_oauth = false if current_user.update(sign_up_params) - current_user.skip_reconfirmation! - sign_in(current_user, bypass: true) - redirect_to root_url + current_user.send_oauth_confirmation_instructions + sign_in_and_redirect current_user, event: :authentication else render :finish_signup end diff --git a/app/models/identity.rb b/app/models/identity.rb index 3ba19e3fa..e3704da01 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -4,14 +4,7 @@ class Identity < ActiveRecord::Base validates :provider, presence: true validates :uid, presence: true, uniqueness: { scope: :provider } - def self.find_for_oauth(auth) + def self.first_or_create_from_oauth(auth) where(uid: auth.uid, provider: auth.provider).first_or_create end - - def update_user(new_user) - return unless user != new_user - - self.user = new_user - save! - end end diff --git a/app/models/user.rb b/app/models/user.rb index 8deb423f4..a79e8acc2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,4 @@ class User < ActiveRecord::Base - OMNIAUTH_EMAIL_PREFIX = 'omniauth@participacion' - OMNIAUTH_EMAIL_REGEX = /\A#{OMNIAUTH_EMAIL_PREFIX}/ include Verification @@ -31,7 +29,6 @@ class User < ActiveRecord::Base validate :validate_username_length validates :official_level, inclusion: {in: 0..5} - validates_format_of :email, without: OMNIAUTH_EMAIL_REGEX, on: :update validates :terms_of_service, acceptance: { allow_nil: false }, on: :create validates :locale, inclusion: {in: I18n.available_locales.map(&:to_s), @@ -52,42 +49,20 @@ class User < ActiveRecord::Base before_validation :clean_document_number - def self.find_for_oauth(auth, signed_in_resource = nil) - # Get the identity and user if they exist - identity = Identity.find_for_oauth(auth) - - # If a signed_in_resource is provided it always overrides the existing user - # to prevent the identity being locked with accidentally created accounts. - # Note that this may leave zombie accounts (with no associated identity) which - # can be cleaned up at a later date. - user = signed_in_resource ? signed_in_resource : identity.user - user ||= first_or_create_for_oauth(auth) - - # Associate the identity with the user if needed - identity.update_user(user) - user - end - # Get the existing user by email if the provider gives us a verified email. - # If no verified email was provided we assign a temporary email and ask the - # user to verify it on the next step via RegistrationsController.finish_signup - def self.first_or_create_for_oauth(auth) - email = auth.info.email if auth.info.verified || auth.info.verified_email - user = User.where(email: email).first if email + def self.first_or_initialize_for_oauth(auth) + oauth_email = auth.info.email + oauth_email_confirmed = auth.info.verified || auth.info.verified_email + oauth_user = User.find_by(email: oauth_email) if oauth_email_confirmed - # Create the user if it's a new registration - if user.nil? - user = User.new( - username: auth.info.nickname || auth.extra.raw_info.name.parameterize('-') || auth.uid, - email: email ? email : "#{OMNIAUTH_EMAIL_PREFIX}-#{auth.uid}-#{auth.provider}.com", - password: Devise.friendly_token[0,20], - terms_of_service: '1' - ) - user.skip_confirmation! - user.save! - end - - user + oauth_user || User.new( + username: auth.info.name || auth.uid, + email: oauth_email, + oauth_email: oauth_email, + password: Devise.friendly_token[0,20], + terms_of_service: '1', + confirmed_at: oauth_email_confirmed ? DateTime.now : nil + ) end def name @@ -170,11 +145,6 @@ class User < ActiveRecord::Base erased_at.present? end - def email_provided? - !!(email && email !~ OMNIAUTH_EMAIL_REGEX) || - !!(unconfirmed_email && unconfirmed_email !~ OMNIAUTH_EMAIL_REGEX) - end - def locked? Lock.find_or_create_by(user: self).locked? end @@ -197,11 +167,11 @@ class User < ActiveRecord::Base end def username_required? - !organization? && !erased? + !organization? && !erased? && !registering_with_oauth end def email_required? - !erased? + !erased? && !registering_with_oauth end def has_official_email? @@ -213,6 +183,26 @@ class User < ActiveRecord::Base self[:locale] ||= I18n.default_locale.to_s end + def confirmation_required? + super && !registering_with_oauth + end + + def send_oauth_confirmation_instructions + if oauth_email != email + self.update(confirmed_at: nil) + self.send_confirmation_instructions + end + self.update(oauth_email: nil) if oauth_email.present? + end + + def save_requiring_finish_signup + self.update(registering_with_oauth: true) + end + + def save_requiring_finish_signup_without_email + self.update(registering_with_oauth: true, email: nil) + end + private def clean_document_number self.document_number = self.document_number.gsub(/[^a-z0-9]+/i, "").upcase unless self.document_number.blank? diff --git a/app/views/devise/_omniauth_form.html.erb b/app/views/devise/_omniauth_form.html.erb index 343743ff3..3167cc1d6 100644 --- a/app/views/devise/_omniauth_form.html.erb +++ b/app/views/devise/_omniauth_form.html.erb @@ -1,12 +1,35 @@ -<% if current_page?(new_user_session_path) %> - <%= link_to t("omniauth.twitter.sign_in"), user_omniauth_authorize_path(:twitter), class: "button-twitter button radius expand" %> - <%= link_to t("omniauth.facebook.sign_in"), user_omniauth_authorize_path(:facebook), class: "button-facebook button radius expand" %> - <%= link_to t("omniauth.google_oauth2.sign_in"), user_omniauth_authorize_path(:google_oauth2), class: "button-google button radius expand" %> -
-<% elsif current_page?(new_user_registration_path) %> - <%= link_to t("omniauth.twitter.sign_up"), user_omniauth_authorize_path(:twitter), class: "button-twitter button radius expand" %> - <%= link_to t("omniauth.facebook.sign_up"), user_omniauth_authorize_path(:facebook), class: "button-facebook button radius expand" %> - <%= link_to t("omniauth.google_oauth2.sign_up"), user_omniauth_authorize_path(:google_oauth2), class: "button-google button radius expand" %> -

Al hacer login con una red social, usted está aceptando los términos legales.

-
-<% end %> \ No newline at end of file +<% if feature?(:twitter_login) || feature?(:facebook_login) || feature?(:google_login) %> + + <% if current_page?(new_user_session_path) %> + + <% if feature? :twitter_login %> + <%= link_to t("omniauth.twitter.sign_in"), user_omniauth_authorize_path(:twitter), class: "button-twitter button radius expand" %> + <% end %> + + <% if feature? :facebook_login %> + <%= link_to t("omniauth.facebook.sign_in"), user_omniauth_authorize_path(:facebook), class: "button-facebook button radius expand" %> + <% end %> + + <% if feature? :google_login %> + <%= link_to t("omniauth.google_oauth2.sign_in"), user_omniauth_authorize_path(:google_oauth2), class: "button-google button radius expand" %> + <% end %> + +
+ <% elsif current_page?(new_user_registration_path) %> + + <% if feature? :twitter_login %> + <%= link_to t("omniauth.twitter.sign_up"), user_omniauth_authorize_path(:twitter), class: "button-twitter button radius expand" %> + <% end %> + + <% if feature? :facebook_login %> + <%= link_to t("omniauth.facebook.sign_up"), user_omniauth_authorize_path(:facebook), class: "button-facebook button radius expand" %> + <% end %> + + <% if feature? :google_login %> + <%= link_to t("omniauth.google_oauth2.sign_up"), user_omniauth_authorize_path(:google_oauth2), class: "button-google button radius expand" %> + <% end %> + +
+ <% end %> + +<% end %> diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index e3cfc1201..56783bcb5 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -1,7 +1,7 @@ <% provide :title do %><%= t("devise_views.sessions.new.title") %><% end %>

<%= t("devise_views.sessions.new.title") %>

-<%# render 'devise/omniauth_form' %> +<%= render 'devise/omniauth_form' %>

<%= t("devise_views.shared.links.signup", diff --git a/app/views/users/registrations/finish_signup.html.erb b/app/views/users/registrations/finish_signup.html.erb index c57f01a8e..5542ed6c3 100644 --- a/app/views/users/registrations/finish_signup.html.erb +++ b/app/views/users/registrations/finish_signup.html.erb @@ -1,13 +1,19 @@ -

-
-
-

<%= t('omniauth.finish_signup.title') %>

+

<%= t('omniauth.finish_signup.title') %>

- <%= form_for current_user, as: :user, url: do_finish_signup_path, html: { role: 'form'} do |f| %> - <%= render 'shared/errors', resource: current_user %> - <%= f.email_field :email, placeholder: t("devise_views.users.registrations.new.email_label"), value: nil %> - <%= f.submit t("devise_views.users.registrations.new.submit"), class: 'button radius' %> - <% end %> -
-
-
+<%= form_for current_user, as: :user, url: do_finish_signup_path, html: { role: 'form'} do |f| %> + <%= render 'shared/errors', resource: current_user %> + + <% if current_user.errors.include? :username %> + <%= f.text_field :username, placeholder: t("devise_views.users.registrations.new.username_label") %> + <% else %> + <%= f.hidden_field :username %> + <% end %> + + <% if current_user.errors.include? :email %> + <%= f.email_field :email, placeholder: t("devise_views.users.registrations.new.email_label") %> + <% else %> + <%= f.hidden_field :email %> + <% end %> + + <%= f.submit t("devise_views.users.registrations.new.submit"), class: 'button radius expand' %> +<% end %> diff --git a/app/views/users/registrations/new.html.erb b/app/views/users/registrations/new.html.erb index faeff384a..61ac4031b 100644 --- a/app/views/users/registrations/new.html.erb +++ b/app/views/users/registrations/new.html.erb @@ -1,7 +1,7 @@ <% provide :title do %><%= t("devise_views.users.registrations.new.title") %><% end %>

<%= t("devise_views.users.registrations.new.title") %>

-<%# render 'devise/omniauth_form' %> +<%= render 'devise/omniauth_form' %> <%= form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f| %> <%= render 'shared/errors', resource: resource %> diff --git a/config/deploy/preproduction.rb b/config/deploy/preproduction.rb index a514f84eb..16d1c5bb7 100644 --- a/config/deploy/preproduction.rb +++ b/config/deploy/preproduction.rb @@ -1,7 +1,7 @@ set :deploy_to, deploysecret(:deploy_to) set :server_name, deploysecret(:server_name) set :db_server, deploysecret(:db_server) -set :branch, :master +set :branch, ENV['branch'] || :master set :ssh_options, port: deploysecret(:ssh_port) set :stage, :preproduction set :rails_env, :preproduction diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index ad5f76fb6..31623e4cf 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -240,7 +240,7 @@ Devise.setup do |config| # up on your models and hooks. # config.omniauth :github, 'APP_ID', 'APP_SECRET', scope: 'user,public_repo' config.omniauth :twitter, Rails.application.secrets.twitter_key, Rails.application.secrets.twitter_secret - config.omniauth :facebook, Rails.application.secrets.facebook_key, Rails.application.secrets.facebook_secret + config.omniauth :facebook, Rails.application.secrets.facebook_key, Rails.application.secrets.facebook_secret, scope: 'email', info_fields: 'email' config.omniauth :google_oauth2, Rails.application.secrets.google_oauth2_key, Rails.application.secrets.google_oauth2_secret # ==> Warden configuration diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 473181f02..a7504ff3d 100755 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -15,7 +15,7 @@ en: not_found_in_database: "Invalid %{authentication_keys} or password." timeout: "Your session has expired. Please sign in again to continue." unauthenticated: "You must sign in or register to continue." - unconfirmed: "You must confirm your account to continue." + unconfirmed: "To continue, please click on the confirmation link that we have sent you via email" mailer: confirmation_instructions: subject: "Confirmation instructions" diff --git a/config/locales/devise.es.yml b/config/locales/devise.es.yml index bf81e4199..9e5ad1f36 100644 --- a/config/locales/devise.es.yml +++ b/config/locales/devise.es.yml @@ -1,7 +1,7 @@ es: devise: confirmations: - confirmed: "Tu cuenta ha sido confirmada." + confirmed: "Tu cuenta ha sido confirmada. Por favor autentifícate con tu red social o tu usuario y contraseña" send_instructions: "Recibirás un correo electrónico en unos minutos con instrucciones sobre cómo restablecer tu contraseña." send_paranoid_instructions: "Si tu correo electrónico existe en nuestra base de datos recibirás un correo electrónico en unos minutos con instrucciones sobre cómo restablecer tu contraseña." failure: @@ -13,7 +13,7 @@ es: not_found_in_database: "%{authentication_keys} o contraseña inválidos." timeout: "Tu sesión ha expirado, por favor inicia sesión nuevamente para continuar." unauthenticated: "Necesitas iniciar sesión o registrarte para continuar." - unconfirmed: "Debes confirmar tu cuenta para continuar." + unconfirmed: "Para continuar, por favor pulsa en el enlace de confirmación que hemos enviado a tu cuenta de correo." mailer: confirmation_instructions: subject: "Instrucciones de confirmación" @@ -22,8 +22,8 @@ es: unlock_instructions: subject: "Instrucciones de desbloqueo" omniauth_callbacks: - failure: "No se te ha podido autorizar de %{kind} debido a \"%{reason}\"." - success: "Identificado correctamente de %{kind}." + failure: "No se te ha podido identificar via %{kind} por el siguiente motivo: \"%{reason}\"." + success: "Identificado correctamente via %{kind}." passwords: no_token: "No puedes acceder a esta página si no es a través de un enlace para restablecer la contraseña. Si has accedido desde el enlace para restablecer la contraseña, asegúrate de que la URL esté completa." send_instructions: "Recibirás un correo electrónico con instrucciones sobre cómo restablecer tu contraseña en unos minutos." diff --git a/config/locales/en.yml b/config/locales/en.yml index 7e0bd1b46..555b47413 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -468,3 +468,22 @@ en: user_permission_verify_info: "* Only for users on Madrid Census." user_permission_verify_url: verify your account user_permission_votes: Participate on final voting + omniauth: + finish_signup: + title: "Additional details" + twitter: + sign_in: "Sign in with Twitter" + sign_up: "Sign up with Twitter" + facebook: + sign_in: "Sign in with Facebook" + sign_up: "Sign up with Facebook" + google_oauth2: + sign_in: "Sign in with Google" + sign_up: "Sign up with Google" + legislation: + help: + title: "How I can comment this document?" + text: "To comment this document you must %{sign_in} or %{sign_up}. Then select the text you want to comment and press the button with the pencil." + text_sign_in: "login" + text_sign_up: "sign up" + alt: "Select the text you want to comment and press the button with the pencil." diff --git a/config/locales/es.yml b/config/locales/es.yml index 848a14bf9..87de46def 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -468,3 +468,22 @@ es: user_permission_verify_info: "* Sólo usuarios empadronados en el municipio de Madrid." user_permission_verify_url: verifica tu cuenta user_permission_votes: Participar en las votaciones finales* + omniauth: + finish_signup: + title: "Detalles adicionales de tu cuenta" + twitter: + sign_in: "Entra con Twitter" + sign_up: "Regístrate con Twitter" + facebook: + sign_in: "Entra con Facebook" + sign_up: "Regístrate con Facebook" + google_oauth2: + sign_in: "Entra con Google" + sign_up: "Regístrate con Google" + legislation: + help: + title: "¿Cómo puedo comentar este documento?" + text: "Para comentar este documento debes %{sign_in} o %{sign_up}. Después selecciona el texto que quieres comentar y pulsa en el botón con el lápiz." + text_sign_in: "iniciar sesión" + text_sign_up: "registrarte" + alt: "Selecciona el texto que quieres comentar y pulsa en el botón con el lápiz." diff --git a/db/dev_seeds.rb b/db/dev_seeds.rb index 4f638758a..0bee91cb2 100644 --- a/db/dev_seeds.rb +++ b/db/dev_seeds.rb @@ -20,6 +20,9 @@ Setting.create(key: 'org_name', value: 'Consul') Setting.create(key: 'place_name', value: 'City') Setting.create(key: 'feature.debates', value: "true") Setting.create(key: 'feature.spending_proposals', value: "true") +Setting.create(key: 'feature.twitter_login', value: "true") +Setting.create(key: 'feature.facebook_login', value: "true") +Setting.create(key: 'feature.google_login', value: "true") puts "Creating Geozones" ('A'..'Z').each{ |i| Geozone.create(name: "District #{i}") } diff --git a/db/migrate/20160119164320_add_registering_with_oauth_to_users.rb b/db/migrate/20160119164320_add_registering_with_oauth_to_users.rb new file mode 100644 index 000000000..a29d310ba --- /dev/null +++ b/db/migrate/20160119164320_add_registering_with_oauth_to_users.rb @@ -0,0 +1,5 @@ +class AddRegisteringWithOauthToUsers < ActiveRecord::Migration + def change + add_column :users, :registering_with_oauth, :bool, default: false + end +end diff --git a/db/migrate/20160125100637_add_confirmed_oauth_email_to_user.rb b/db/migrate/20160125100637_add_confirmed_oauth_email_to_user.rb new file mode 100644 index 000000000..660735ab2 --- /dev/null +++ b/db/migrate/20160125100637_add_confirmed_oauth_email_to_user.rb @@ -0,0 +1,5 @@ +class AddConfirmedOauthEmailToUser < ActiveRecord::Migration + def change + add_column :users, :confirmed_oauth_email, :string + end +end diff --git a/db/migrate/20160126090634_rename_confirmed_oauth_email_to_oauth_email.rb b/db/migrate/20160126090634_rename_confirmed_oauth_email_to_oauth_email.rb new file mode 100644 index 000000000..3b3f027fe --- /dev/null +++ b/db/migrate/20160126090634_rename_confirmed_oauth_email_to_oauth_email.rb @@ -0,0 +1,5 @@ +class RenameConfirmedOauthEmailToOauthEmail < ActiveRecord::Migration + def change + rename_column :users, :confirmed_oauth_email, :oauth_email + end +end diff --git a/db/schema.rb b/db/schema.rb index df5bbc3a9..a9cce3582 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160122153329) do +ActiveRecord::Schema.define(version: 20160126090634) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -397,6 +397,8 @@ ActiveRecord::Schema.define(version: 20160122153329) do t.boolean "newsletter", default: false t.integer "notifications_count", default: 0 t.string "locale" + t.boolean "registering_with_oauth", default: false + t.string "oauth_email" end add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree diff --git a/db/seeds.rb b/db/seeds.rb index 6a6ae6671..659b3b2c0 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -53,3 +53,6 @@ Setting["place_name"] = "Consul-land" # Feature flags Setting['feature.debates'] = true Setting['feature.spending_proposals'] = true +Setting['feature.twitter_login'] = true +Setting['feature.facebook_login'] = true +Setting['feature.google_login'] = true diff --git a/spec/features/users_auth_spec.rb b/spec/features/users_auth_spec.rb index b23d956ac..3526ce97c 100644 --- a/spec/features/users_auth_spec.rb +++ b/spec/features/users_auth_spec.rb @@ -18,8 +18,7 @@ feature 'Users' do expect(page).to have_content "You have been sent a message containing a verification link. Please click on this link to activate your account." - sent_token = /.*confirmation_token=(.*)".*/.match(ActionMailer::Base.deliveries.last.body.to_s)[1] - visit user_confirmation_path(confirmation_token: sent_token) + confirm_email expect(page).to have_content "Your account has been confirmed." end @@ -45,115 +44,192 @@ feature 'Users' do end end - xcontext 'OAuth authentication' do + context 'OAuth authentication' do context 'Twitter' do - background do - #request.env["devise.mapping"] = Devise.mappings[:user] - end - scenario 'Sign up, when email was provided by OAuth provider' do - omniauth_twitter_hash = { 'provider' => 'twitter', - 'uid' => '12345', - 'info' => { - 'name' => 'manuela', - 'email' => 'manuelacarmena@example.com', - 'nickname' => 'ManuelaRocks', - 'verified' => '1' - }, - 'extra' => { 'raw_info' => - { 'location' => 'Madrid', - 'name' => 'Manuela de las Carmenas' - } - } - } + let(:twitter_hash){ {provider: 'twitter', uid: '12345', info: {name: 'manuela'}} } + let(:twitter_hash_with_email){ {provider: 'twitter', uid: '12345', info: {name: 'manuela', email: 'manuelacarmena@example.com'}} } + let(:twitter_hash_with_verified_email){ {provider: 'twitter', + uid: '12345', + info: {name: 'manuela', email: 'manuelacarmena@example.com', verified: '1'}} } - OmniAuth.config.add_mock(:twitter, omniauth_twitter_hash) + + scenario 'Sign up when Oauth provider has a verified email' do + OmniAuth.config.add_mock(:twitter, twitter_hash_with_verified_email) visit '/' click_link 'Register' - expect do - expect do - expect do - click_link 'Sign up with Twitter' - end.not_to change { ActionMailer::Base.deliveries.size } - end.to change { Identity.count }.by(1) - end.to change { User.count }.by(1) + click_link 'Sign up with Twitter' - expect(current_path).to eq(root_path) expect_to_be_signed_in - user = User.last - expect(user.username).to eq('ManuelaRocks') - expect(user.email).to eq('manuelacarmena@example.com') - expect(user.confirmed?).to eq(true) + click_link 'My account' + expect(page).to have_field('account_username', with: 'manuela') + + visit edit_user_registration_path + expect(page).to have_field('user_email', with: 'manuelacarmena@example.com') end - scenario 'Sign up, when neither email nor nickname were provided by OAuth provider' do - omniauth_twitter_hash = { 'provider' => 'twitter', - 'uid' => '12345', - 'info' => { - 'name' => 'manuela' - }, - 'extra' => { 'raw_info' => - { 'location' => 'Madrid', - 'name' => 'Manuela de las Carmenas' - } - } - } - - OmniAuth.config.add_mock(:twitter, omniauth_twitter_hash) + scenario 'Sign up when Oauth provider has an unverified email' do + OmniAuth.config.add_mock(:twitter, twitter_hash_with_email) visit '/' click_link 'Register' - expect do - expect do - expect do - click_link 'Sign up with Twitter' - end.not_to change { ActionMailer::Base.deliveries.size } - end.to change { Identity.count }.by(1) - end.to change { User.count }.by(1) + click_link 'Sign up with Twitter' + + expect(current_path).to eq(new_user_session_path) + expect(page).to have_content "To continue, please click on the confirmation link that we have sent you via email" + + confirm_email + expect(page).to have_content "Your account has been confirmed" + + visit '/' + click_link 'Sign in' + click_link 'Sign in with Twitter' + expect_to_be_signed_in + + click_link 'My account' + expect(page).to have_field('account_username', with: 'manuela') + + visit edit_user_registration_path + expect(page).to have_field('user_email', with: 'manuelacarmena@example.com') + end + + scenario 'Sign up, when no email was provided by OAuth provider' do + OmniAuth.config.add_mock(:twitter, twitter_hash) + + visit '/' + click_link 'Register' + click_link 'Sign up with Twitter' expect(current_path).to eq(finish_signup_path) - - user = User.last - expect(user.username).to eq('manuela-de-las-carmenas') - expect(user.email).to eq("omniauth@participacion-12345-twitter.com") - fill_in 'user_email', with: 'manueladelascarmenas@example.com' click_button 'Register' - sent_token = /.*confirmation_token=(.*)".*/.match(ActionMailer::Base.deliveries.last.body.to_s)[1] - visit user_confirmation_path(confirmation_token: sent_token) + expect(page).to have_content "To continue, please click on the confirmation link that we have sent you via email" - expect(page).to have_content "Your email address has been successfully confirmed" + confirm_email + expect(page).to have_content "Your account has been confirmed" - expect(user.reload.email).to eq('manueladelascarmenas@example.com') + visit '/' + click_link 'Sign in' + click_link 'Sign in with Twitter' + expect_to_be_signed_in + + click_link 'My account' + expect(page).to have_field('account_username', with: 'manuela') + + visit edit_user_registration_path + expect(page).to have_field('user_email', with: 'manueladelascarmenas@example.com') end scenario 'Sign in, user was already signed up with OAuth' do user = create(:user, email: 'manuela@madrid.es', password: 'judgementday') - identity = create(:identity, uid: '12345', provider: 'twitter', user: user) - omniauth_twitter_hash = { 'provider' => 'twitter', - 'uid' => '12345', - 'info' => { - 'name' => 'manuela' - } - } - - OmniAuth.config.add_mock(:twitter, omniauth_twitter_hash) + create(:identity, uid: '12345', provider: 'twitter', user: user) + OmniAuth.config.add_mock(:twitter, twitter_hash) visit '/' click_link 'Sign in' - - expect do - expect do - click_link 'Sign in with Twitter' - end.not_to change { Identity.count } - end.not_to change { User.count } + click_link 'Sign in with Twitter' expect_to_be_signed_in + + click_link 'My account' + expect(page).to have_field('account_username', with: user.username) + + visit edit_user_registration_path + expect(page).to have_field('user_email', with: user.email) + + end + + scenario 'Try to register with the username of an already existing user' do + create(:user, username: 'manuela', email: 'manuela@madrid.es', password: 'judgementday') + OmniAuth.config.add_mock(:twitter, twitter_hash_with_verified_email) + + visit '/' + click_link 'Register' + click_link 'Sign up with Twitter' + + expect(current_path).to eq(finish_signup_path) + + fill_in 'user_username', with: 'manuela2' + click_button 'Register' + + expect_to_be_signed_in + + click_link 'My account' + expect(page).to have_field('account_username', with: 'manuela2') + + visit edit_user_registration_path + expect(page).to have_field('user_email', with: 'manuelacarmena@example.com') + end + + scenario 'Try to register with the email of an already existing user, when no email was provided by oauth' do + create(:user, username: 'peter', email: 'manuela@example.com') + OmniAuth.config.add_mock(:twitter, twitter_hash) + + visit '/' + click_link 'Register' + click_link 'Sign up with Twitter' + + expect(current_path).to eq(finish_signup_path) + + fill_in 'user_email', with: 'manuela@example.com' + click_button 'Register' + + expect(current_path).to eq(do_finish_signup_path) + + fill_in 'user_email', with: 'somethingelse@example.com' + click_button 'Register' + + expect(page).to have_content "To continue, please click on the confirmation link that we have sent you via email" + + confirm_email + expect(page).to have_content "Your account has been confirmed" + + visit '/' + click_link 'Sign in' + click_link 'Sign in with Twitter' + expect_to_be_signed_in + + click_link 'My account' + expect(page).to have_field('account_username', with: 'manuela') + + visit edit_user_registration_path + expect(page).to have_field('user_email', with: 'somethingelse@example.com') + end + + scenario 'Try to register with the email of an already existing user, when an unconfirmed email was provided by oauth' do + create(:user, username: 'peter', email: 'manuelacarmena@example.com') + OmniAuth.config.add_mock(:twitter, twitter_hash_with_email) + + visit '/' + click_link 'Register' + click_link 'Sign up with Twitter' + + expect(current_path).to eq(finish_signup_path) + + expect(page).to have_field('user_email', with: 'manuelacarmena@example.com') + fill_in 'user_email', with: 'somethingelse@example.com' + click_button 'Register' + + expect(page).to have_content "To continue, please click on the confirmation link that we have sent you via email" + + confirm_email + expect(page).to have_content "Your account has been confirmed" + + visit '/' + click_link 'Sign in' + click_link 'Sign in with Twitter' + expect_to_be_signed_in + + click_link 'My account' + expect(page).to have_field('account_username', with: 'manuela') + + visit edit_user_registration_path + expect(page).to have_field('user_email', with: 'somethingelse@example.com') end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7d44a202b..6b65fcb0e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -85,26 +85,6 @@ describe User do end end - describe 'OmniAuth' do - describe '#email_provided?' do - it "is false if the email matchs was temporarely assigned by the OmniAuth process" do - subject.email = 'omniauth@participacion-ABCD-twitter.com' - expect(subject.email_provided?).to eq(false) - end - - it "is true if the email is not omniauth-like" do - subject.email = 'manuelacarmena@example.com' - expect(subject.email_provided?).to eq(true) - end - - it "is true if the user's real email is pending to be confirmed" do - subject.email = 'omniauth@participacion-ABCD-twitter.com' - subject.unconfirmed_email = 'manuelacarmena@example.com' - expect(subject.email_provided?).to eq(true) - end - end - end - describe "administrator?" do it "is false when the user is not an admin" do expect(subject.administrator?).to be false @@ -245,23 +225,23 @@ describe User do end end end - + describe "has_official_email" do it "checks if the mail address has the officials domain" do # We will use empleados.madrid.es as the officials' domain # Subdomains are also accepted + Setting['email_domain_for_officials'] = 'officials.madrid.es' - user1 = create(:user, email: "john@officials.madrid.es", confirmed_at: Time.now) user2 = create(:user, email: "john@yes.officials.madrid.es", confirmed_at: Time.now) user3 = create(:user, email: "john@unofficials.madrid.es", confirmed_at: Time.now) user4 = create(:user, email: "john@example.org", confirmed_at: Time.now) - + expect(user1.has_official_email?).to eq(true) expect(user2.has_official_email?).to eq(true) expect(user3.has_official_email?).to eq(false) expect(user4.has_official_email?).to eq(false) - + # We reset the officials' domain setting Setting.find_by(key: 'email_domain_for_officials').update(value: '') end diff --git a/spec/support/common_actions.rb b/spec/support/common_actions.rb index 4521a1e9f..437a3c7ca 100644 --- a/spec/support/common_actions.rb +++ b/spec/support/common_actions.rb @@ -36,12 +36,13 @@ module CommonActions end def confirm_email - expect(page).to have_content "A message with a confirmation link has been sent to your email address." + body = ActionMailer::Base.deliveries.last.try(:body) + expect(body).to be_present - sent_token = /.*confirmation_token=(.*)".*/.match(ActionMailer::Base.deliveries.last.body.to_s)[1] + sent_token = /.*confirmation_token=(.*)".*/.match(body.to_s)[1] visit user_confirmation_path(confirmation_token: sent_token) - expect(page).to have_content "Your email address has been successfully confirmed" + expect(page).to have_content "Your account has been confirmed" end def reset_password