Convert OpenID authentication to use OmniAuth
authorTom Hughes <tom@compton.nu>
Sat, 21 Feb 2015 12:41:42 +0000 (12:41 +0000)
committerTom Hughes <tom@compton.nu>
Wed, 25 Feb 2015 01:07:23 +0000 (01:07 +0000)
Gemfile
Gemfile.lock
app/controllers/user_controller.rb
app/helpers/user_helper.rb
app/views/user/login.html.erb
config/initializers/omniauth.rb [new file with mode: 0644]
config/initializers/openid.rb [deleted file]
config/locales/en.yml
config/routes.rb

diff --git a/Gemfile b/Gemfile
index 6a1bbc66ed5154b23a24c90c41010158f3095b7e..a459fabc41fdfe78a666054271a25d9168f30899 100644 (file)
--- 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"
index f40dc2b72aaef4c0abf54569f7d7478fbd743d40..7c8691a22378fcc7f6723533e2c12d1f4a1f8f8b 100644 (file)
@@ -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
index 6cb0b60dba9f64c8f0d86cefcc61798b69c39330..b1d6c86f7a9f0d894fed11a2c9018dd00f5732ab 100644 (file)
@@ -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
index a3d5980862f3a6eae01a3eb2037f13941efc7280..d488371666f5147f9428ec2613ff07df569b5465 100644 (file)
@@ -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
index 2b667c7250ecd9d2d0bc1606f2f80f06b45dc2c0..9fdeb3ab3bc1892502445675de456997f9933950 100644 (file)
@@ -42,8 +42,8 @@
 
         <ul class='clearfix' id="login_openid_buttons">
           <li><%= link_to image_tag("openid.png", :alt => t("user.login.openid_providers.openid.title")), "#", :id => "openid_open_url", :title => t("user.login.openid_providers.openid.title") %></li>
-          <li><%= openid_button "google", "gmail.com" %></li>
-          <li><%= openid_button "yahoo", "me.yahoo.com" %></li>
+          <li><%= openid_button "google", "https://www.google.com/accounts/o8/id" %></li>
+          <li><%= openid_button "yahoo", "yahoo.com" %></li>
           <li><%= openid_button "wordpress", "wordpress.com" %></li>
           <li><%= openid_button "aol", "aol.com" %></li>
         </ul>
@@ -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 (file)
index 0000000..2f22454
--- /dev/null
@@ -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 (file)
index 2a6de16..0000000
+++ /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
index 3e3905298a9056a40a241aa686b6172242df235c..c58ee0db52cbf8ff3f4bb95c89df9cbfe1b160e1 100644 (file)
@@ -1689,8 +1689,6 @@ en:
       account not active: "Sorry, your account is not active yet.<br />Please use the link in the account confirmation email to activate your account, or <a href=\"%{reconfirm}\">request a new confirmation email</a>."
       account is suspended: Sorry, your account has been suspended due to suspicious activity.<br />Please contact the <a href="%{webmaster}">webmaster</a> 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.
         </p>
+    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."
index c9563f8d43d5afeaf4d1fa8c6319f409bcb4d93d..f67121d2536912f5f15add7ea9c337ebbbd749ca 100644 (file)
@@ -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_@~]+[=-]*/