From 03ebf669775287ebeb348c7b5f9163d821547253 Mon Sep 17 00:00:00 2001 From: Sidharth Bansal Date: Wed, 13 Jun 2018 22:55:56 +0530 Subject: [PATCH] Google login (#2826) * Routes modified * Set the views * Modified error messages and user_tag * Add User_Tag as identity and find user_tag * Reformat range stats page to table (#2792) * Index page listing for UserTags per issue #2741. (#2753) * Index page listing for UserTags per issue #2741. * Changes per pull request checks. * Removed debugging line. * Added routes to reflect 'groups' naming convention for user_tags per pull request spec. * Tests rewritten after rebase. * Index page listing for UserTags per issue #2741. * Changes per pull request checks. * Removed debugging line. * Added routes to reflect 'groups' naming convention for user_tags per pull request spec. * Fix for test of sort by value. * Update routes.rb for wiki update (#2804) * Update routes.rb * fixes * User_sessions_controller modified * User_tag modified * user session controller modified * Checkpoint 1 * checkpoint1 codeclimate errors fixed * integration tests for assets (#2806) * integration tests for assets * full assets included * Update assets.rb * Checkpoint 2 * Added routes to notes/edit (#2808) * Added routes to notes/edit * Removed redundant notes#edit and added get instead of post in edit actions * Update routes.rb (#2810) * Create OPENID.md * fixing error in home_controller related to `group by` (#2794) * Added group by note.nid * correction * Openid fix with post route and better alert texts (#2815) * Added group by note.nid * correction * reworded openid requests and added post method for 2nd step * adjust message * Update openid_test.rb * Update openid_test.rb * Update openid_test.rb * Update openid_test.rb * Update Dangerfile (#2816) * Update routes.rb to fix embeddable features (#2818) * Update routes.rb * Removed the apostrophe mismatch * Added tests for embed in feature * Delete route for notes (#2820) * checkpoint 2 * User is created from sign up process successfully * create a usertag_with_omniauth * Search a usertag for oauth * search user_tag existing in db * create a user with omniauth test * LOGIN WORKS * current_user=(user) method removed * sign up correction * routing tests for google oauth * Google auth details saved * Google should return omniauth hash test * sign up and login via provider * sign up and login via provider alternative flow * flash message test added * login user with an email and then connect google provider * . --- .gitignore | 1 + Dangerfile | 2 - app/controllers/admin_controller.rb | 3 + app/controllers/application_controller.rb | 5 + app/controllers/home_controller.rb | 2 +- app/controllers/openid_controller.rb | 2 +- app/controllers/user_sessions_controller.rb | 179 ++++++++++++------ app/controllers/user_tags_controller.rb | 39 +++- app/controllers/users_controller.rb | 2 +- app/models/node.rb | 8 +- app/models/user.rb | 18 ++ app/models/user_tag.rb | 11 +- app/views/admin/assets.html.erb | 48 +++++ app/views/layouts/_footer.html.erb | 1 + app/views/layouts/_header.html.erb | 7 +- app/views/openid/decide.html.erb | 4 +- app/views/stats/range.html.erb | 47 +++-- app/views/user_sessions/new.html.erb | 10 +- app/views/user_tags/index.html.erb | 38 ++++ app/views/users/edit.html.erb | 5 + app/views/users/new.html.erb | 6 + config/environments/test.rb | 20 ++ config/initializers/assets.rb | 52 ++++- config/initializers/omniauth.rb | 2 +- config/routes.rb | 17 +- doc/OPENID.md | 48 +++++ test/fixtures/user_tags.yml | 40 ++++ test/functional/features_controller_test.rb | 7 + test/functional/notes_controller_test.rb | 8 +- .../user_sessions_controller_test.rb | 54 ++++++ test/functional/user_tags_controller_test.rb | 23 +++ test/integration/login_flow_test.rb | 17 ++ test/integration/openid_test.rb | 90 +++++++++ test/integration/public_pages_test.rb | 5 + test/integration/wiki_creation_test.rb | 14 ++ test/unit/user_tag_test.rb | 109 +++++++++++ test/unit/user_test.rb | 21 ++ 37 files changed, 858 insertions(+), 107 deletions(-) create mode 100644 app/views/admin/assets.html.erb create mode 100644 app/views/user_tags/index.html.erb create mode 100644 doc/OPENID.md create mode 100644 test/integration/openid_test.rb diff --git a/.gitignore b/.gitignore index f9efa92dac1..862f1d099d0 100644 --- a/.gitignore +++ b/.gitignore @@ -26,6 +26,7 @@ config/initializers/recaptcha.rb config/config.yml config/initializers/site_keys.rb config/initializers/secret_token.rb +config/application.yml public/system public/lib db/openid-store/ diff --git a/Dangerfile b/Dangerfile index 0b19b2c96f6..44deab270e8 100644 --- a/Dangerfile +++ b/Dangerfile @@ -51,8 +51,6 @@ begin if !source_path.nil? && !line.nil? f = f.gsub(source_path + ':' + line, "#{source_path}:#{line}") .gsub('`', "'") # remove ` as these cause Markdown formatting - #.gsub('\’', "`") # also remove ’ -- this causes the script to hang! omitting this line. - #.gsub('’', "`") # alternative (untested): replace ’ with ` for proper Markdown `code` formatting end fail("There was a test error at: #{f}") end diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 7be9fd1a433..ec6946a649a 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -1,6 +1,9 @@ class AdminController < ApplicationController before_action :require_user, only: %i(spam spam_revisions mark_comment_spam publish_comment) + # intended to provide integration tests for assets + def assets + end def promote_admin @user = User.find params[:id] diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7a8e2a612dd..547d905abf3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -186,4 +186,9 @@ def redirect_old_urls redirect_to @node.path, status: :moved_permanently end end + + def signed_in? + !!current_user + end + end diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index b1b7c4542dd..f6678fa9b91 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -121,7 +121,7 @@ def activity .where('node_revisions.status = 1') .where('timestamp - node.created > ?', 300) # don't report edits within 5 mins of page creation .limit(10) - .group('node.title') + .group(['node.title', 'node.nid']) # group by day: http://stackoverflow.com/questions/5970938/group-by-day-from-timestamp revisions = revisions.group('DATE(FROM_UNIXTIME(timestamp))') if Rails.env == 'production' revisions = revisions.to_a # ensure it can be serialized for caching diff --git a/app/controllers/openid_controller.rb b/app/controllers/openid_controller.rb index 74bf14e02e4..c09819cb227 100644 --- a/app/controllers/openid_controller.rb +++ b/app/controllers/openid_controller.rb @@ -114,7 +114,7 @@ def resume end end - def show_decision_page(oidreq, message = 'Do you trust this site with your identity?') + def show_decision_page(oidreq, message = 'The site shown below is asking to use your PublicLab.org account to log you in. Do you trust this site?') session[:last_oidreq] = oidreq @oidreq = oidreq diff --git a/app/controllers/user_sessions_controller.rb b/app/controllers/user_sessions_controller.rb index fc1ca35d64a..a47c957a432 100644 --- a/app/controllers/user_sessions_controller.rb +++ b/app/controllers/user_sessions_controller.rb @@ -6,75 +6,132 @@ def new end def create - params[:user_session][:username] = params[:openid] if params[:openid] # second runthrough must preserve username - username = params[:user_session][:username] if params[:user_session] - @user = User.find_by(username: username) + auth = request.env['omniauth.auth'] + if auth + # Find an identity here + @identity = UserTag.find_with_omniauth(auth) - # try finding by email, if that exists - if @user.nil? && !User.where(email: username).empty? - @user = User.find_by(email: username) - params[:user_session][:username] = @user.username - end - - if @user.nil? - flash[:warning] = "There is nobody in our system by that name, are you sure you have the right username?" - redirect_to '/login' - elsif params[:user_session].nil? || @user&.drupal_user&.status == 1 - # an existing Rails user - if params[:user_session].nil? || @user - if @user&.crypted_password.nil? # the user has not created a pwd in the new site - params[:user_session][:openid_identifier] = 'https://old.publiclab.org/people/' + username + '/identity' if username - params[:user_session].delete(:password) - params[:user_session].delete(:username) - params[:openid] = username # pack up username for second runthrough + if signed_in? + if @identity.nil? + # If no identity was found, create a brand new one here + @identity = UserTag.create_with_omniauth(auth, current_user.id) + # The identity is not associated with the current_user so lets + # associate the identity + @identity.user = current_user + @identity.save + redirect_to root_url, notice: "Successfully linked to your account!" + elsif @identity.user == current_user + # User is signed in so they are trying to link an identity with their + # account. But we found the identity and the user associated with it + # is the current user. So the identity is already associated with + # this user. So let's display an error message. + redirect_to root_url, notice: "Already linked to your account!" + else + # User is signed in so they are trying to link an identity with their + # account. But we found the identity and a different user associated with it + # ,which is not the current user. So the identity is already associated with + # that user. So let's display an error message. + redirect_to root_url, notice: "Already linked to another account!" end - @user_session = UserSession.new(username: params[:user_session][:username], - password: params[:user_session][:password], - remember_me: params[:user_session][:remember_me]) - saved = @user_session.save do |result| - if result - # replace this with temporarily saving pwd in session, - # and automatically saving it in the user record after login is completed - if current_user.crypted_password.nil? # the user has not created a pwd in the new site - flash[:warning] = I18n.t('user_sessions_controller.create_password_for_new_site') - redirect_to '/profile/edit' - else - flash[:notice] = I18n.t('user_sessions_controller.logged_in') - if session[:openid_return_to] # for openid login, redirects back to openid auth process - return_to = session[:openid_return_to] - session[:openid_return_to] = nil - redirect_to return_to - elsif session[:return_to] - return_to = session[:return_to] - session[:return_to] = nil - redirect_to return_to - elsif params[:return_to] - redirect_to params[:return_to] + else # not signed in + if @identity&.user.present? + # The identity we found had a user associated with it so let's + # just log them in here + UserSession.create( @identity.user) + redirect_to root_url, notice: "Signed in!" + else #identity does not exist so we need to either create a user with identity OR link identity to existing user + if User.where(email: auth["info"]["email"]).empty? + #Create a new user as email provided is not present in PL database + user = User.create_with_omniauth(auth) + @identity = UserTag.create_with_omniauth(auth, user.id) + key = user.generate_reset_key + # send key to user email + PasswordResetMailer.reset_notify(user, key).deliver_now unless user.nil? # respond the same to both successes and failures; security + redirect_to root_url, notice: "You have successfully signed in. Please change your password via a link sent to you via a mail" + else #email exists so link the identity with existing user and log in the user + user = User.where(email: auth["info"]["email"]) + # If no identity was found, create a brand new one here + @identity = UserTag.create_with_omniauth(auth, user.ids.first) + # The identity is not associated with the current_user so lets + # associate the identity + @identity.save + #log in them + UserSession.create( @identity.user) + redirect_to root_url, notice: "Successfully linked to your account!" + end + end + end + else + params[:user_session][:username] = params[:openid] if params[:openid] # second runthrough must preserve username + username = params[:user_session][:username] if params[:user_session] + @user = User.find_by(username: username) + + # try finding by email, if that exists + if @user.nil? && !User.where(email: username).empty? + @user = User.find_by(email: username) + params[:user_session][:username] = @user.username + end + + if @user.nil? + flash[:warning] = "There is nobody in our system by that name, are you sure you have the right username?" + redirect_to '/login' + elsif params[:user_session].nil? || @user&.drupal_user&.status == 1 + # an existing Rails user + if params[:user_session].nil? || @user + if @user&.crypted_password.nil? # the user has not created a pwd in the new site + params[:user_session][:openid_identifier] = 'https://old.publiclab.org/people/' + username + '/identity' if username + params[:user_session].delete(:password) + params[:user_session].delete(:username) + params[:openid] = username # pack up username for second runthrough + end + @user_session = UserSession.new(username: params[:user_session][:username], + password: params[:user_session][:password], + remember_me: params[:user_session][:remember_me]) + saved = @user_session.save do |result| + if result + # replace this with temporarily saving pwd in session, + # and automatically saving it in the user record after login is completed + if current_user.crypted_password.nil? # the user has not created a pwd in the new site + flash[:warning] = I18n.t('user_sessions_controller.create_password_for_new_site') + redirect_to '/profile/edit' else - redirect_to '/dashboard' + flash[:notice] = I18n.t('user_sessions_controller.logged_in') + if session[:openid_return_to] # for openid login, redirects back to openid auth process + return_to = session[:openid_return_to] + session[:openid_return_to] = nil + redirect_to return_to + elsif session[:return_to] + return_to = session[:return_to] + session[:return_to] = nil + redirect_to return_to + elsif params[:return_to] + redirect_to params[:return_to] + else + redirect_to '/dashboard' + end end + else + # Login failed; probably bad password. + # Errors will display on login form: + render action: 'new' end - else - # Login failed; probably bad password. - # Errors will display on login form: - render action: 'new' + end + else # not a native user + if !DrupalUser.find_by(name: username).nil? + # this is a user from the old site who hasn't registered on the new site + redirect_to controller: :users, action: :create, user: { openid_identifier: username } + else # totally new user! + flash[:warning] = I18n.t('user_sessions_controller.sign_up_to_join') + redirect_to '/signup' end end - else # not a native user - if !DrupalUser.find_by(name: username).nil? - # this is a user from the old site who hasn't registered on the new site - redirect_to controller: :users, action: :create, user: { openid_identifier: username } - else # totally new user! - flash[:warning] = I18n.t('user_sessions_controller.sign_up_to_join') - redirect_to '/signup' - end + elsif params[:user_session].nil? || @user&.drupal_user&.status == 5 + flash[:error] = I18n.t('user_sessions_controller.user_has_been_moderated', username: @user.username).html_safe + redirect_to '/' + else + flash[:error] = I18n.t('user_sessions_controller.user_has_been_banned', username: @user.username).html_safe + redirect_to '/' end - elsif params[:user_session].nil? || @user&.drupal_user&.status == 5 - flash[:error] = I18n.t('user_sessions_controller.user_has_been_moderated', username: @user.username).html_safe - redirect_to '/' - else - flash[:error] = I18n.t('user_sessions_controller.user_has_been_banned', username: @user.username).html_safe - redirect_to '/' end end diff --git a/app/controllers/user_tags_controller.rb b/app/controllers/user_tags_controller.rb index 006ba96221b..40b9282c550 100644 --- a/app/controllers/user_tags_controller.rb +++ b/app/controllers/user_tags_controller.rb @@ -1,6 +1,37 @@ class UserTagsController < ApplicationController respond_to :html, :xml, :json, :js + require 'will_paginate/array' + + def index + @toggle = params[:sort] || "uses" + + @title = I18n.t('tag_controller.tags') + @paginated = true + if params[:search] + keyword = params[:search] + @user_tags = UserTag + .select('value') + .where("value LIKE :keyword", keyword: "%#{keyword}%") + .group(:value) + .order('value ASC') + .count('value').to_a + .paginate(page: params[:page], per_page: 24) + elsif @toggle == "value" + @user_tags = UserTag.group(:value) + .select('value') + .order('value ASC') + .count('value').to_a + .paginate(page: params[:page], per_page: 24) + else # @toggle == "uses" + @user_tags = UserTag.group(:value) + .select('value') + .order('count_value DESC') + .count('value').to_a + .paginate(page: params[:page], per_page: 24) + end + end + def create @output = { @@ -23,13 +54,13 @@ def create unless exist user_tag = user.user_tags.build(value: name) - if tagname.split(':')[0] == "oauth-facebook" + if tagname.split(':')[1] == "facebook" @output[:errors] << "This tag is used for associating a Facebook account. Click here to read more " - elsif tagname.split(':')[0] == "oauth-github" + elsif tagname.split(':')[1] == "github" @output[:errors] << "This tag is used for associating a Github account. Click here to read more " - elsif tagname.split(':')[0] == "oauth-google" + elsif tagname.split(':')[1] == "google_oauth2" @output[:errors] << "This tag is used for associating a Google account. Click here to read more " - elsif tagname.split(':')[0] == "oauth-twitter" + elsif tagname.split(':')[1] == "twitter" @output[:errors] << "This tag is used for associating a Twitter account. Click here to read more " elsif user_tag.save @output[:saved] << [name, user_tag.id] diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f5299089b7d..4d3001739f7 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -98,7 +98,7 @@ def list .where('rusers.role = ?', params[:id]) .where('rusers.status = 1') .page(params[:page]) - + elsif @tagname_param @users = User.where(id: UserTag.where(value: @tagname_param).collect(&:uid)) .page(params[:page]) diff --git a/app/models/node.rb b/app/models/node.rb index 98b2a9af814..3c14fe08ac4 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -861,13 +861,13 @@ def can_tag(tagname, user, errors = false) errors ? I18n.t('node.only_admins_can_lock') : false elsif tagname.split(':')[0] == 'redirect' && Node.where(slug: tagname.split(':')[1]).length <= 0 errors ? I18n.t('node.page_does_not_exist') : false - elsif tagname.split(':')[0] == "oauth-facebook" + elsif tagname.split(':')[1] == "facebook" errors ? "This tag is used for associating a Facebook account. Click here to read more " : false - elsif tagname.split(':')[0] == "oauth-github" + elsif tagname.split(':')[1] == "github" errors ? "This tag is used for associating a Github account. Click here to read more " : false - elsif tagname.split(':')[0] == "oauth-google" + elsif tagname.split(':')[1] == "google_oauth2" errors ? "This tag is used for associating a Google account. Click here to read more " : false - elsif tagname.split(':')[0] == "oauth-twitter" + elsif tagname.split(':')[1] == "twitter" errors ? "This tag is used for associating a Twitter account. Click here to read more " : false else true diff --git a/app/models/user.rb b/app/models/user.rb index 01552a668fd..617ee8f5e20 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -414,4 +414,22 @@ def self.contributor_count_for(start_time,end_time) contributors end + def self.create_with_omniauth(auth) + #email prefix is part of email before @ with periods replaced with underscores + #generate a 2 digit alphanumeric number and append it at the end of email-prefix + charset = Array('A'..'Z') + Array('a'..'z') + Array(0..9) + email_prefix = auth["info"]["email"].gsub('.','_').split('@')[0] + while(!User.where(username: email_prefix).empty?) + email_prefix = auth["info"]["email"].gsub('.','_').split('@')[0] + Array.new(2) { charset.sample }.join + end + puts(auth) + create! do |user| + user.username = email_prefix + user.email = auth["info"]["email"] + user.password = auth["uid"] + user.password_confirmation = auth["uid"] + user.save! + end + end + end diff --git a/app/models/user_tag.rb b/app/models/user_tag.rb index ef429f010e1..9b95ff872c0 100644 --- a/app/models/user_tag.rb +++ b/app/models/user_tag.rb @@ -3,7 +3,7 @@ class UserTag < ApplicationRecord validates :value, presence: :true validates :value, format: { with: /\A[\w\.:-]*\z/, message: 'can only include letters, numbers, and dashes' } - + validates_uniqueness_of :value, :scope => :uid before_save :preprocess def preprocess @@ -18,4 +18,13 @@ def name self.value end + def self.find_with_omniauth(auth) + find_by(value: "oauth:" + auth['provider'] + ":" + auth['uid']) + end + + def self.create_with_omniauth(auth, uid) + create(value: "oauth:" + auth['provider'] + ":" + auth['uid'], + uid: uid) + end + end diff --git a/app/views/admin/assets.html.erb b/app/views/admin/assets.html.erb new file mode 100644 index 00000000000..dde7f755974 --- /dev/null +++ b/app/views/admin/assets.html.erb @@ -0,0 +1,48 @@ +<%= javascript_include_tag 'leaflet-blurred-location/dist/Leaflet.BlurredLocation' %> + +<%= javascript_include_tag 'application' %> +<%= javascript_include_tag 'advanced_search' %> +<%= javascript_include_tag 'comment_expand' %> +<%= javascript_include_tag 'dashboard' %> +<%= javascript_include_tag 'dragdrop' %> +<%= javascript_include_tag 'dynamic_search' %> +<%= javascript_include_tag 'editor' %> +<%= javascript_include_tag 'graph' %> +<%= javascript_include_tag 'grids' %> +<%= javascript_include_tag 'header_footer' %> +<%= javascript_include_tag 'ics.deps.min' %> +<%= javascript_include_tag 'ics.min' %> +<%= javascript_include_tag 'jsdiff' %> +<%= javascript_include_tag 'leaflet_helper' %> +<%= javascript_include_tag 'like' %> +<%= javascript_include_tag 'locationForm' %> +<%= javascript_include_tag 'main_image' %> +<%= javascript_include_tag 'methods' %> +<%= javascript_include_tag 'notes' %> +<%= javascript_include_tag 'post' %> +<%= javascript_include_tag 'question' %> +<%= javascript_include_tag 'restful_typeahead' %> +<%= javascript_include_tag 'searchform' %> +<%= javascript_include_tag 'setup' %> +<%= javascript_include_tag 'sidebar' %> +<%= javascript_include_tag 'tagging' %> +<%= javascript_include_tag 'textbox_expand' %> +<%= javascript_include_tag 'users' %> +<%= javascript_include_tag 'wikis' %> + +<%= stylesheet_link_tag 'blog' %> +<%= stylesheet_link_tag 'comments' %> +<%= stylesheet_link_tag 'dashboard' %> +<%= stylesheet_link_tag 'editor' %> +<%= stylesheet_link_tag 'fancy' %> +<%= stylesheet_link_tag 'feature' %> +<%= stylesheet_link_tag 'I18n' %> +<%= stylesheet_link_tag 'location_tags' %> +<%= stylesheet_link_tag 'map' %> +<%= stylesheet_link_tag 'print' %> +<%= stylesheet_link_tag 'question' %> +<%= stylesheet_link_tag 'search' %> +<%= stylesheet_link_tag 'style' %> +<%= stylesheet_link_tag 'tags' %> +<%= stylesheet_link_tag 'user_tags.' %> +<%= stylesheet_link_tag 'wiki' %> diff --git a/app/views/layouts/_footer.html.erb b/app/views/layouts/_footer.html.erb index 37a21eb9a19..530ca5ad821 100644 --- a/app/views/layouts/_footer.html.erb +++ b/app/views/layouts/_footer.html.erb @@ -2,4 +2,5 @@ <% cache('feature_footer-notice', skip_digest: true) do %> <%= feature('footer-notice') %> <% end %> + diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb index af7133774d5..3cf20422d56 100644 --- a/app/views/layouts/_header.html.erb +++ b/app/views/layouts/_header.html.erb @@ -48,13 +48,13 @@