From: Tom Hughes Date: Sat, 21 Feb 2015 12:41:42 +0000 (+0000) Subject: Convert OpenID authentication to use OmniAuth X-Git-Tag: live~4753 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/b0150caee632110eb5b305f9937b94473101645a?ds=inline Convert OpenID authentication to use OmniAuth --- diff --git a/Gemfile b/Gemfile index 6a1bbc66e..a459fabc4 100644 --- a/Gemfile +++ b/Gemfile @@ -42,7 +42,6 @@ gem "rails-i18n", "~> 4.0.0" gem "dynamic_form" gem "rinku", ">= 1.2.2", :require => "rails_rinku" gem "oauth-plugin", ">= 0.5.1" -gem "open_id_authentication", ">= 1.1.0" gem "validates_email_format_of", ">= 1.5.1" gem "composite_primary_keys", "~> 8.0.0" gem "http_accept_language", "~> 2.0.0" @@ -52,8 +51,9 @@ gem "openstreetmap-i18n-js", ">= 3.0.0.rc5.3", :require => "i18n-js" gem "rack-cors" gem "actionpack-page_caching" -# We need ruby-openid 2.2.0 or later for ruby 1.9 support -gem "ruby-openid", ">= 2.2.0" +# Omniauth for authentication +gem "omniauth" +gem "omniauth-openid" # Markdown formatting support gem "redcarpet" diff --git a/Gemfile.lock b/Gemfile.lock index f40dc2b72..7c8691a22 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -79,6 +79,7 @@ GEM multipart-post (>= 1.2, < 3) globalid (0.3.3) activesupport (>= 4.1.0) + hashie (3.4.0) hike (1.2.3) htmlentities (4.3.3) http_accept_language (2.0.5) @@ -134,8 +135,12 @@ GEM multi_json (~> 1.3) multi_xml (~> 0.5) rack (~> 1.2) - open_id_authentication (1.2.0) - rack-openid (~> 1.3) + omniauth (1.2.2) + hashie (>= 1.2, < 4) + rack (~> 1.0) + omniauth-openid (1.0.1) + omniauth (~> 1.0) + rack-openid (~> 1.3.1) openstreetmap-i18n-js (3.0.0.rc5.3) i18n paperclip (4.2.1) @@ -156,7 +161,7 @@ GEM r2 (0.2.5) rack (1.6.0) rack-cors (0.3.1) - rack-openid (1.4.2) + rack-openid (1.3.1) rack (>= 1.1.0) ruby-openid (>= 2.1.8) rack-test (0.6.3) @@ -268,7 +273,8 @@ DEPENDENCIES libxml-ruby (>= 2.0.5) minitest (~> 5.1) oauth-plugin (>= 0.5.1) - open_id_authentication (>= 1.1.0) + omniauth + omniauth-openid openstreetmap-i18n-js (>= 3.0.0.rc5.3) paperclip (~> 4.0) pg @@ -281,7 +287,6 @@ DEPENDENCIES redcarpet rinku (>= 1.2.2) rubocop - ruby-openid (>= 2.2.0) sanitize sass-rails (~> 5.0) soap4r-ruby1.9 diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 6cb0b60db..b1d6c86f7 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -1,7 +1,7 @@ class UserController < ApplicationController layout "site", :except => [:api_details] - skip_before_filter :verify_authenticity_token, :only => [:api_read, :api_details, :api_gpx_files] + skip_before_filter :verify_authenticity_token, :only => [:api_read, :api_details, :api_gpx_files, :auth_success] before_filter :disable_terms_redirect, :only => [:terms, :save, :logout, :api_details] before_filter :authorize, :only => [:api_details, :api_gpx_files] before_filter :authorize_web, :except => [:api_read, :api_details, :api_gpx_files] @@ -126,18 +126,11 @@ class UserController < ApplicationController # valid OpenID and one the user has control over before saving # it as a password equivalent for the user. session[:new_user_settings] = params - openid_verify(params[:user][:openid_url], @user) + openid_url = openid_expand_url(params[:user][:openid_url]) + redirect_to auth_path(:provider => "openid", :openid_url => openid_url, :origin => request.path) else update_user(@user, params) end - elsif using_open_id? - # The redirect from the OpenID provider reenters here - # again and we need to pass the parameters through to - # the open_id_authentication function - settings = session.delete(:new_user_settings) - openid_verify(nil, @user) do |user| - update_user(user, settings) - end end end @@ -205,23 +198,7 @@ class UserController < ApplicationController @title = t "user.new.title" @referer = params[:referer] || session[:referer] - if using_open_id? - # The redirect from the OpenID provider reenters here - # again and we need to pass the parameters through to - # the open_id_authentication function - @user = session.delete(:new_user) - - openid_verify(nil, @user) do |user, verified_email| - user.status = "active" if user.email == verified_email - end - - if @user.openid_url.nil? || @user.invalid? - render :action => "new" - else - session[:new_user] = @user - redirect_to :action => "terms" - end - elsif @user + if @user # The user is logged in already, so don't show them the signup # page, instead send them to the home page if @referer @@ -262,7 +239,8 @@ class UserController < ApplicationController elsif @user.openid_url.present? # Verify OpenID before moving on session[:new_user] = @user - openid_verify(@user.openid_url, @user) + openid_url = openid_expand_url(@user.openid_url) + redirect_to auth_path(:provider => "openid", :openid_url => openid_url, :origin => request.path) else # Save the user record session[:new_user] = @user @@ -272,12 +250,13 @@ class UserController < ApplicationController end def login - if params[:username] || using_open_id? + if params[:username] || params[:openid_url] session[:referer] ||= params[:referer] - if using_open_id? + if params[:openid_url].present? session[:remember_me] ||= params[:remember_me_openid] - openid_authentication(params[:openid_url]) + openid_url = openid_expand_url(params[:openid_url]) + redirect_to auth_path(:provider => "openid", :openid_url => openid_url, :origin => request.path) else session[:remember_me] ||= params[:remember_me] password_authentication(params[:username], params[:password]) @@ -498,6 +477,52 @@ class UserController < ApplicationController end end + ## + # omniauth success callback + def auth_success + auth_info = env["omniauth.auth"] + + openid_url = auth_info[:uid] + name = auth_info[:info][:name] + email = auth_info[:info][:email] + + if user = User.find_by_openid_url(openid_url) + case user.status + when "pending" then + unconfirmed_login(user) + when "active", "confirmed" then + successful_login(user) + when "suspended" then + failed_login t("user.login.account is suspended", :webmaster => "mailto:webmaster@openstreetmap.org") + else + failed_login t("user.login.auth failure") + end + elsif settings = session.delete(:new_user_settings) + @user.openid_url = openid_url + + update_user(@user, settings) + + redirect_to :action => "account", :display_name => @user.display_name + elsif session[:new_user] + session[:new_user].openid_url = openid_url + + if email == session[:new_user].email && openid_email_verified(email) + session[:new_user].status = "active" + end + + redirect_to :action => "terms" + else + redirect_to :action => "new", :nickname => name, :email => email, :openid => openid_url + end + end + + ## + # omniauth failure callback + def auth_failure + flash[:error] = t("user.auth_failure." + params[:message]) + redirect_to params[:origin] + end + private ## @@ -514,96 +539,6 @@ class UserController < ApplicationController end end - ## - # handle OpenID authentication - def openid_authentication(openid_url) - # If we don't appear to have a user for this URL then ask the - # provider for some extra information to help with signup - if openid_url && User.find_by_openid_url(openid_url) - required = nil - else - required = [:nickname, :email, "http://axschema.org/namePerson/friendly", "http://axschema.org/contact/email"] - end - - # Start the authentication - authenticate_with_open_id(openid_expand_url(openid_url), :method => :get, :required => required) do |result, identity_url, sreg, ax| - if result.successful? - # We need to use the openid url passed back from the OpenID provider - # rather than the one supplied by the user, as these can be different. - # - # For example, you can simply enter yahoo.com in the login box rather - # than a user specific url. Only once it comes back from the provider - # provider do we know the unique address for the user. - if user = User.find_by_openid_url(identity_url) - case user.status - when "pending" then - unconfirmed_login(user) - when "active", "confirmed" then - successful_login(user) - when "suspended" then - failed_login t("user.login.account is suspended", :webmaster => "mailto:webmaster@openstreetmap.org") - else - failed_login t("user.login.auth failure") - end - else - # Guard against not getting any extension data - sreg = {} if sreg.nil? - ax = {} if ax.nil? - - # We don't have a user registered to this OpenID, so redirect - # to the create account page with username and email filled - # in if they have been given by the OpenID provider through - # the simple registration protocol. - nickname = sreg["nickname"] || ax["http://axschema.org/namePerson/friendly"].first - email = sreg["email"] || ax["http://axschema.org/contact/email"].first - - redirect_to :controller => "user", :action => "new", :nickname => nickname, :email => email, :openid => identity_url - end - elsif result.missing? - failed_login t("user.login.openid missing provider") - elsif result.invalid? - failed_login t("user.login.openid invalid") - else - failed_login t("user.login.auth failure") - end - end - end - - ## - # verify an OpenID URL - def openid_verify(openid_url, user) - user.openid_url = openid_url - - authenticate_with_open_id(openid_expand_url(openid_url), :method => :get, :required => [:email, "http://axschema.org/contact/email"]) do |result, identity_url, sreg, ax| - if result.successful? - # Do we trust the emails this provider returns? - if openid_email_verified(identity_url) - # Guard against not getting any extension data - sreg = {} if sreg.nil? - ax = {} if ax.nil? - - # Get the verified email - verified_email = sreg["email"] || ax["http://axschema.org/contact/email"].first - end - - # We need to use the openid url passed back from the OpenID provider - # rather than the one supplied by the user, as these can be different. - # - # For example, you can simply enter yahoo.com in the login box rather - # than a user specific url. Only once it comes back from the provider - # provider do we know the unique address for the user. - user.openid_url = identity_url - yield user, verified_email - elsif result.missing? - flash.now[:error] = t "user.login.openid missing provider" - elsif result.invalid? - flash.now[:error] = t "user.login.openid invalid" - else - flash.now[:error] = t "user.login.auth failure" - end - end - end - ## # special case some common OpenID providers by applying heuristics to # try and come up with the correct URL based on what the user entered diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index a3d598086..d48837166 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -51,8 +51,8 @@ module UserHelper def openid_button(name, url) link_to( image_tag("#{name}.png", :alt => t("user.login.openid_providers.#{name}.alt")), - "#", - :class => "openid_button", :data => { :url => url }, + auth_path(:provider => "openid", :openid_url => url), + :class => "openid_button", :title => t("user.login.openid_providers.#{name}.title") ) end diff --git a/app/views/user/login.html.erb b/app/views/user/login.html.erb index 2b667c725..9fdeb3ab3 100644 --- a/app/views/user/login.html.erb +++ b/app/views/user/login.html.erb @@ -42,8 +42,8 @@ @@ -83,11 +83,6 @@ $(document).ready(function() { $("#login_openid_submit").show(); }); - $(".openid_button").click(function() { - $("#openid_url").val($(this).attr("data-url")); - $("#login_form").submit(); - }); - $("#login_openid_url").hide(); $("#login_openid_submit").hide(); }); diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb new file mode 100644 index 000000000..2f2245419 --- /dev/null +++ b/config/initializers/omniauth.rb @@ -0,0 +1,16 @@ +OmniAuth.config.logger = Rails.logger +OmniAuth.config.failure_raise_out_environments = [] + +if defined?(MEMCACHE_SERVERS) + require "openid/store/memcache" + + openid_store = OpenID::Store::Memcache.new(Dalli::Client.new(MEMCACHE_SERVERS, :namespace => "rails")) +else + require "openid/store/filesystem" + + openid_store = OpenID::Store::Filesystem.new(Rails.root.join("tmp/openids")) +end + +Rails.application.config.middleware.use OmniAuth::Builder do + provider :openid, :name => "openid", :store => openid_store +end diff --git a/config/initializers/openid.rb b/config/initializers/openid.rb deleted file mode 100644 index 2a6de16b2..000000000 --- a/config/initializers/openid.rb +++ /dev/null @@ -1,7 +0,0 @@ -if defined?(MEMCACHE_SERVERS) - require "openid/store/memcache" - - OpenIdAuthentication.store = OpenID::Store::Memcache.new(Dalli::Client.new(MEMCACHE_SERVERS, :namespace => "rails")) -else - OpenIdAuthentication.store = :file -end diff --git a/config/locales/en.yml b/config/locales/en.yml index 3e3905298..c58ee0db5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1689,8 +1689,6 @@ en: account not active: "Sorry, your account is not active yet.
Please use the link in the account confirmation email to activate your account, or request a new confirmation email." account is suspended: Sorry, your account has been suspended due to suspicious activity.
Please contact the webmaster if you wish to discuss this. auth failure: "Sorry, could not log in with those details." - openid missing provider: "Sorry, could not contact your OpenID provider" - openid invalid: "Sorry, your OpenID seems to be malformed" openid_logo_alt: "Log in with an OpenID" openid_providers: openid: @@ -1965,6 +1963,9 @@ en: This decision will be reviewed by an administrator shortly, or you may contact the %{webmaster} if you wish to discuss this.

+ auth_failure: + connection_failed: Connection to authentication provider failed + invalid_credentials: Invalid authentication credentials user_role: filter: not_an_administrator: "Only administrators can perform user role management, and you are not an administrator." diff --git a/config/routes.rb b/config/routes.rb index c9563f8d4..f67121d25 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -175,6 +175,11 @@ OpenStreetMap::Application.routes.draw do get "/create-account.html", :to => redirect(:path => "/user/new") get "/forgot-password.html", :to => redirect(:path => "/user/forgot-password") + # omniauth + match "/auth/failure" => "user#auth_failure", :via => :get + match "/auth/:provider/callback" => "user#auth_success", :via => [:get, :post], :as => :auth_success + match "/auth/:provider" => "user#auth", :via => [:get, :post], :as => :auth + # permalink match "/go/:code" => "site#permalink", :via => :get, :code => /[a-zA-Z0-9_@~]+[=-]*/