major refactor of auth, break up the gigantic omniauth controller into sub classes for way better extensibitily

app/controllers/users/omniauth_callbacks_controller.rb

@@ -2,8 +2,24 @@
 require_dependency 'email'
 require_dependency 'enum'
 require_dependency 'user_name_suggester'
+require_dependency 'auth/authenticator'
+require_dependency 'auth/facebook_authenticator'
+require_dependency 'auth/open_id_authenticator'
+require_dependency 'auth/github_authenticator'
+require_dependency 'auth/twitter_authenticator'
+require_dependency 'auth/persona_authenticator'
 
 class Users::OmniauthCallbacksController < ApplicationController
+
+  BUILTIN_AUTH = [
+    Auth::FacebookAuthenticator.new,
+    Auth::OpenIdAuthenticator.new("google", trusted: true),
+    Auth::OpenIdAuthenticator.new("yahoo", trusted: true),
+    Auth::GithubAuthenticator.new,
+    Auth::TwitterAuthenticator.new,
+    Auth::PersonaAuthenticator.new
+  ]
+
   skip_before_filter :redirect_to_login_if_required
 
   layout false
@@ -20,34 +36,16 @@ class Users::OmniauthCallbacksController < ApplicationController
   skip_before_filter :verify_authenticity_token, only: :complete
 
   def complete
-    provider = params[:provider]
+    auth = request.env["omniauth.auth"]
 
-    # If we are a plugin, then try to login with it
+    authenticator = self.class.find_authenticator(params[:provider])
-    found = false
-    Discourse.auth_providers.each do |p|
-      if p.name == provider && p.type == :open_id
-        create_or_sign_on_user_using_openid request.env["omniauth.auth"]
-        found = true
-        break
-      elsif p.name == provider && p.type == :oauth2
-        create_or_sign_on_user_using_oauth2 request.env["omniauth.auth"]
-        found = true
-        break
-      end
-    end
 
-    unless found
+    @data = authenticator.after_authenticate(auth)
-      # Make sure we support that provider
+    @data.authenticator_name = authenticator.name
-      raise Discourse::InvalidAccess.new unless self.class.types.keys.map(&:to_s).include?(provider)
 
-      # Check if the provider is enabled
+    user_found(@data.user) if @data.user
-      raise Discourse::InvalidAccess.new("provider is not enabled") unless SiteSetting.send("enable_#{provider}_logins?")
 
-      # Call the appropriate logic
+    session[:authentication] = @data.session_data
-      send("create_or_sign_on_user_using_#{provider}", request.env["omniauth.auth"])
-    end
-
-    @data[:awaiting_approval] = true if invite_only?
 
     respond_to do |format|
       format.html
@@ -60,340 +58,45 @@ class Users::OmniauthCallbacksController < ApplicationController
     render layout: 'no_js'
   end
 
-  def create_or_sign_on_user_using_twitter(auth_token)
 
-    data = auth_token[:info]
+  def self.find_authenticator(name)
-    screen_name = data["nickname"]
+    BUILTIN_AUTH.each do |authenticator|
-    twitter_user_id = auth_token["uid"]
+      if authenticator.name == name
+        raise Discourse::InvalidAccess.new("provider is not enabled") unless SiteSetting.send("enable_#{name}_logins?")
 
-    session[:authentication] = {
+        return authenticator
-      twitter_user_id: twitter_user_id,
+      end
-      twitter_screen_name: screen_name
+    end
-    }
 
-    user_info = TwitterUserInfo.where(twitter_user_id: twitter_user_id).first
+    Discourse.auth_providers.each do |provider|
+      if provider.name == name
 
-    @data = {
+        return provider.authenticator
-      username: screen_name,
+      end
-      auth_provider: "Twitter"
+    end
-    }
 
-    process_user_info(user_info, screen_name)
+    raise Discourse::InvalidAccess.new("provider is not found")
   end
 
-  def create_or_sign_on_user_using_facebook(auth_token)
+  protected
 
-    data = auth_token[:info]
+  def user_found(user)
-    raw_info = auth_token["extra"]["raw_info"]
+    # automatically activate any account if a provider marked the email valid
+    if !user.active && @data.email_valid
+      user.toggle(:active).save
+    end
 
-    email = data[:email]
+    # log on any account that is active with forum access
-    name = data["name"]
+    if Guardian.new(user).can_access_forum? && user.active
-    fb_uid = auth_token["uid"]
+      log_on_user(user)
-
+      @data.authenticated = true
-
-    username = UserNameSuggester.suggest(name)
-
-    session[:authentication] = {
-      facebook: {
-        facebook_user_id: fb_uid,
-        link: raw_info["link"],
-        username: raw_info["username"],
-        first_name: raw_info["first_name"],
-        last_name: raw_info["last_name"],
-        email: raw_info["email"],
-        gender: raw_info["gender"],
-        name: raw_info["name"]
-      },
-      email: email,
-      email_valid: true
-    }
-
-    user_info = FacebookUserInfo.where(facebook_user_id: fb_uid).first
-
-    @data = {
-      username: username,
-      name: name,
-      email: email,
-      auth_provider: "Facebook",
-      email_valid: true
-    }
-
-    if user_info
-      if user = user_info.user
-        user.toggle(:active).save unless user.active?
-
-        # If we have to approve users
-        if Guardian.new(user).can_access_forum?
-          log_on_user(user)
-          @data[:authenticated] = true
-        else
-          @data[:awaiting_approval] = true
-        end
-      end
     else
-      if user = User.where(email: email).first
+      if SiteSetting.invite_only?
-        user.create_facebook_user_info! session[:authentication][:facebook]
+        @data.awaiting_approval = true
-        user.toggle(:active).save unless user.active?
-        log_on_user(user)
-        @data[:authenticated] = true
-      end
-    end
-
-  end
-
-  def create_or_sign_on_user_using_cas(auth_token)
-    logger.error "authtoken #{auth_token}"
-
-    email = auth_token[:info][:email] if auth_token[:info]
-    email ||= if SiteSetting.cas_domainname.present?
-      "#{auth_token[:extra][:user]}@#{SiteSetting.cas_domainname}"
-    else
-      auth_token[:extra][:user]
-    end
-
-    username = auth_token[:extra][:user]
-
-    name = if auth_token[:info] && auth_token[:info][:name]
-      auth_token[:info][:name]
-    else
-      auth_token["uid"]
-    end
-
-    cas_user_id = auth_token["uid"]
-
-    session[:authentication] = {
-        cas: {
-            cas_user_id: cas_user_id ,
-            username: username
-        },
-        email: email,
-        email_valid: true
-    }
-
-    user_info = CasUserInfo.where(:cas_user_id => cas_user_id ).first
-
-    @data = {
-        username: username,
-        name: name,
-        email: email,
-        auth_provider: "CAS",
-        email_valid: true
-    }
-
-    if user_info
-      if user = user_info.user
-        user.toggle(:active).save unless user.active?
-        log_on_user(user)
-        @data[:authenticated] = true
-      end
-    else
-      user = User.where(email: email).first
-      if user
-        CasUserInfo.create!(session[:authentication][:cas].merge(user_id: user.id))
-        user.toggle(:active).save unless user.active?
-        log_on_user(user)
-        @data[:authenticated] = true
-      end
-    end
-
-  end
-
-  def create_or_sign_on_user_using_oauth2(auth_token)
-    oauth2_provider = auth_token[:provider]
-    oauth2_uid = auth_token[:uid]
-    data = auth_token[:info]
-    email = data[:email]
-    name = data[:name]
-
-    oauth2_user_info = Oauth2UserInfo.where(uid: oauth2_uid, provider: oauth2_provider).first
-
-    if oauth2_user_info.blank? && user = User.find_by_email(email)
-      # TODO is only safe if we trust our oauth2 provider to return an email
-      # legitimately owned by our user
-      oauth2_user_info = Oauth2UserInfo.create(uid: oauth2_uid,
-                                               provider: oauth2_provider,
-                                               name: name,
-                                               email: email,
-                                               user: user)
-    end
-
-    authenticated = oauth2_user_info.present?
-
-    if authenticated
-      user = oauth2_user_info.user
-
-      # If we have to approve users
-      if Guardian.new(user).can_access_forum?
-        log_on_user(user)
-        @data = {authenticated: true}
       else
-        @data = {awaiting_approval: true}
+        @data.awaiting_activation = true
       end
-    else
-      @data = {
-        email: email,
-        name: User.suggest_name(name),
-        username: UserNameSuggester.suggest(email),
-        email_valid: true ,
-        auth_provider: oauth2_provider
-      }
-
-      session[:authentication] = {
-        oauth2: {
-          provider: oauth2_provider,
-          uid: oauth2_uid,
-        },
-        name: name,
-        email: @data[:email],
-        email_valid: @data[:email_valid]
-      }
     end
   end
 
-
-  def create_or_sign_on_user_using_openid(auth_token)
-
-    data = auth_token[:info]
-    identity_url = auth_token[:extra][:identity_url]
-
-    email = data[:email]
-
-    # If the auth supplies a name / username, use those. Otherwise start with email.
-    name = data[:name] || data[:email]
-    username = data[:nickname] || data[:email]
-
-    user_open_id = UserOpenId.find_by_url(identity_url)
-
-    if user_open_id.blank? && user = User.find_by_email(email)
-      # we trust so do an email lookup
-      # TODO some openid providers may not be trust worthy, allow for that
-      #  for now we are good (google, yahoo are trust worthy)
-      user_open_id = UserOpenId.create(url: identity_url , user_id: user.id, email: email, active: true)
-    end
-
-    authenticated = user_open_id # if authed before
-
-    if authenticated
-      user = user_open_id.user
-
-      # If we have to approve users
-      if Guardian.new(user).can_access_forum?
-        log_on_user(user)
-        @data = {authenticated: true}
-      else
-        @data = {awaiting_approval: true}
-      end
-
-    else
-      @data = {
-        email: email,
-        name: User.suggest_name(name),
-        username: UserNameSuggester.suggest(username),
-        email_valid: true ,
-        auth_provider: data[:provider] || params[:provider].try(:capitalize)
-      }
-      session[:authentication] = {
-        email: @data[:email],
-        email_valid: @data[:email_valid],
-        openid_url: identity_url
-      }
-    end
-
-  end
-
-  alias_method :create_or_sign_on_user_using_yahoo, :create_or_sign_on_user_using_openid
-  alias_method :create_or_sign_on_user_using_google, :create_or_sign_on_user_using_openid
-
-  def create_or_sign_on_user_using_github(auth_token)
-
-    data = auth_token[:info]
-    screen_name = data["nickname"]
-    email = data["email"]
-    github_user_id = auth_token["uid"]
-
-    session[:authentication] = {
-      github_user_id: github_user_id,
-      github_screen_name: screen_name,
-      email: email,
-      email_valid: true
-    }
-
-    user_info = GithubUserInfo.where(github_user_id: github_user_id).first
-
-    if !user_info && user = User.find_by_email(email)
-      # we trust so do an email lookup
-      user_info = GithubUserInfo.create(
-          user_id: user.id,
-          screen_name: screen_name,
-          github_user_id: github_user_id
-      )
-    end
-
-    @data = {
-      username: screen_name,
-      auth_provider: "Github",
-      email: email,
-      email_valid: true
-    }
-
-    process_user_info(user_info, screen_name)
-  end
-
-  def create_or_sign_on_user_using_persona(auth_token)
-
-    email = auth_token[:info][:email]
-
-    user = User.find_by_email(email)
-
-    if user
-
-      if Guardian.new(user).can_access_forum?
-        log_on_user(user)
-        @data = {authenticated: true}
-      else
-        @data = {awaiting_approval: true}
-      end
-
-    else
-      @data = {
-        email: email,
-        email_valid: true,
-        name: User.suggest_name(email),
-        username: UserNameSuggester.suggest(email),
-        auth_provider: params[:provider].try(:capitalize)
-      }
-
-      session[:authentication] = {
-        email: email,
-        email_valid: true,
-      }
-    end
-
-  end
-
-  private
-
-  def process_user_info(user_info, screen_name)
-    if user_info
-      if user_info.user.active?
-
-        if Guardian.new(user_info.user).can_access_forum?
-          log_on_user(user_info.user)
-          @data[:authenticated] = true
-        else
-          @data[:awaiting_approval] = true
-        end
-
-      else
-        @data[:awaiting_activation] = true
-        # send another email ?
-      end
-    else
-      @data[:name] = screen_name
-    end
-  end
-
-  def invite_only?
-    SiteSetting.invite_only? && !@data[:authenticated]
-  end
 end

app/controllers/users_controller.rb

@@ -448,58 +448,12 @@ class UsersController < ApplicationController
     end
 
     def create_third_party_auth_records(user, auth)
-      return unless auth.present?
+      return unless auth && auth[:authenticator_name]
 
-      if twitter_auth?(auth)
+      authenticator = Users::OmniauthCallbacksController.find_authenticator(auth[:authenticator_name])
-        TwitterUserInfo.create(
+      authenticator.after_create_account(user, auth)
-          user_id: user.id,
-          screen_name: auth[:twitter_screen_name],
-          twitter_user_id: auth[:twitter_user_id]
-        )
-      end
-
-      if facebook_auth?(auth)
-        FacebookUserInfo.create!(auth[:facebook].merge(user_id: user.id))
-      end
-
-      if github_auth?(auth)
-        GithubUserInfo.create(
-          user_id: user.id,
-          screen_name: auth[:github_screen_name],
-          github_user_id: auth[:github_user_id]
-        )
-      end
-
-      if oauth2_auth?(auth)
-        Oauth2UserInfo.create(
-          uid: auth[:oauth2][:uid],
-          provider: auth[:oauth2][:provider],
-          name: auth[:name],
-          email: auth[:email],
-          user_id: user.id
-        )
-      end
     end
 
-    def twitter_auth?(auth)
-      auth[:twitter_user_id] && auth[:twitter_screen_name] &&
-      TwitterUserInfo.find_by_twitter_user_id(auth[:twitter_user_id]).nil?
-    end
-
-    def facebook_auth?(auth)
-      auth[:facebook].present? &&
-      FacebookUserInfo.find_by_facebook_user_id(auth[:facebook][:facebook_user_id]).nil?
-    end
-
-    def github_auth?(auth)
-      auth[:github_user_id] && auth[:github_screen_name] &&
-      GithubUserInfo.find_by_github_user_id(auth[:github_user_id]).nil?
-    end
-
-    def oauth2_auth?(auth)
-      auth[:oauth2].is_a?(Hash) && auth[:oauth2][:provider] && auth[:oauth2][:uid] &&
-      Oauth2UserInfo.where(provider: auth[:oauth2][:provider], uid: auth[:oauth2][:uid]).empty?
-    end
 
     def register_nickname(user)
       if user.valid? && SiteSetting.call_discourse_hub?

lib/auth/authenticator.rb

@@ -0,0 +1,24 @@
+# this class is used by the user and omniauth controllers, it controls how
+#  an authentication system interacts with our database
+
+module Auth; end
+
+require 'auth/result'
+
+class Auth::Authenticator
+  def after_authenticate(auth_options)
+    raise NotImplementedError
+  end
+
+  # can be used to hook in afete the authentication process
+  #  to ensure records exist for the provider in the db
+  #  this MUST be implemented for authenticators that do not
+  #  trust email
+  def after_create_account(user, auth)
+    # not required
+  end
+
+  def lookup_user(user_info, email)
+    user_info.try(:user) || User.where(email: email).first
+  end
+end

lib/auth/cas_authenticator.rb

@@ -0,0 +1,42 @@
+class Auth::CasAuthenticator < Auth::Authenticator
+
+  def name
+    'cas'
+  end
+
+  def after_authenticate(auth_token)
+    result = Auth::Result.new
+
+    email = auth_token[:info][:email] if auth_token[:info]
+    email ||= if SiteSetting.cas_domainname.present?
+      "#{auth_token[:extra][:user]}@#{SiteSetting.cas_domainname}"
+    else
+      auth_token[:extra][:user]
+    end
+
+    result.email = email
+    result.email_valid = true
+
+    result.username = username = auth_token[:extra][:user]
+
+    result.name = name = if auth_token[:info] && auth_token[:info][:name]
+      auth_token[:info][:name]
+    else
+      auth_token["uid"]
+    end
+
+    cas_user_id = auth_token["uid"]
+
+    result.extra_data = {
+      cas_user_id: cas_user_id
+    }
+
+    user_info = CasUserInfo.where(:cas_user_id => cas_user_id ).first
+
+    result.user = user_info.try(:user)
+    result.user ||= User.where(email: email).first
+    # TODO, create CAS record ?
+
+    result
+  end
+end

lib/auth/facebook_authenticator.rb

@@ -0,0 +1,56 @@
+class Auth::FacebookAuthenticator < Auth::Authenticator
+
+  def name
+    "facebook"
+  end
+
+  def after_authenticate(auth_token)
+
+    result = Auth::Result.new
+
+    session_info = parse_auth_token(auth_token)
+    facebook_hash = session_info[:facebook]
+
+    result.email = email = session_info[:email]
+    result.name = name = facebook_hash[:name]
+
+    result.extra_info = facebook_hash
+
+    user_info = FacebookUserInfo.where(facebook_user_id: facebook_hash[:facebook_user_id]).first
+
+    if result.user = lookup_user(user_info, email) && !user_info
+      user.create_facebook_user_info! facebook_hash
+    end
+
+    result
+  end
+
+  def after_create_account(user, auth)
+    data = auth[:extra_data]
+    FacebookUserInfo.create({user_id: user.id}.merge(data))
+  end
+
+  protected
+
+  def parse_auth_token(auth_token)
+
+    raw_info = auth_token["extra"]["raw_info"]
+    email = auth_token["info"][:email]
+
+    {
+      facebook: {
+        facebook_user_id: auth_token["uid"],
+        link: raw_info["link"],
+        username: raw_info["username"],
+        first_name: raw_info["first_name"],
+        last_name: raw_info["last_name"],
+        email: email,
+        gender: raw_info["gender"],
+        name: raw_info["name"]
+      },
+      email: email,
+      email_valid: true
+    }
+
+  end
+end

lib/auth/github_authenticator.rb

@@ -0,0 +1,48 @@
+class Auth::GithubAuthenticator < Auth::Authenticator
+
+  def name
+    "github"
+  end
+
+  def after_authenticate(auth_token)
+    result = Auth::Result.new
+
+    data = auth_token[:info]
+
+    result.username = screen_name = data["nickname"]
+    result.email = email = data["email"]
+
+    github_user_id = auth_token["uid"]
+
+    result.extra_data = {
+      github_user_id: github_user_id,
+      github_screen_name: screen_name,
+    }
+
+    user_info = GithubUserInfo.where(github_user_id: github_user_id).first
+
+    if user_info
+      user = user_info.user
+    elsif user = User.find_by_email(email)
+      user_info = GithubUserInfo.create(
+          user_id: user.id,
+          screen_name: screen_name,
+          github_user_id: github_user_id
+      )
+    end
+
+    result.user = user
+    result.email_valid = true
+
+    result
+  end
+
+  def after_create_account(user, auth)
+    data = auth[:extra_data]
+    GithubUserInfo.create(
+      user_id: user.id,
+      screen_name: data[:github_screen_name],
+      github_user_id: data[:github_user_id]
+    )
+  end
+end

lib/auth/oauth2_authenticator.rb

@@ -0,0 +1,55 @@
+class Auth::OAuth2Authenticator < Auth::Authenticator
+
+  def name
+    @name
+  end
+
+  # only option at the moment is :trusted
+  def initialize(name, opts={})
+    @name = name
+    @opts = opts
+  end
+
+  def after_authenticate(auth_token)
+
+    result = Auth::Result.new
+
+    oauth2_provider = auth_token[:provider]
+    oauth2_uid = auth_token[:uid]
+    data = auth_token[:info]
+    result.email = email = data[:email]
+    result.name = name = data[:name]
+
+    oauth2_user_info = Oauth2UserInfo.where(uid: oauth2_uid, provider: oauth2_provider).first
+
+    if !oauth2_user_info && @opts[:trusted] && user = User.find_by_email(email)
+      oauth2_user_info = Oauth2UserInfo.create(uid: oauth2_uid,
+                                               provider: oauth2_provider,
+                                               name: name,
+                                               email: email,
+                                               user: user)
+    end
+
+    result.user = oauth2_user_info.try(:user)
+    result.email_valid = @opts[:trusted]
+
+    result.extra_data = {
+      uid: oauth2_uid,
+      provider: oauth2_provider
+    }
+
+    result
+  end
+
+  def after_create_account(user, auth)
+    data = auth[:extra_data]
+    Oauth2UserInfo.create(
+      uid: data[:uid],
+      provider: data[:provider],
+      name: auth[:name],
+      email: auth[:email],
+      user_id: user.id
+    )
+  end
+
+end

lib/auth/open_id_authenticator.rb

@@ -0,0 +1,38 @@
+class Auth::OpenIdAuthenticator < Auth::Authenticator
+
+  def initialize(name, opts = {})
+    @name = name
+    @opts = opts
+  end
+
+  def name
+    @name
+  end
+
+  def after_authenticate(auth_token)
+
+    result = Auth::Result.new
+
+    data = auth_token[:info]
+    identity_url = auth_token[:extra][:identity_url]
+    result.email = email = data[:email]
+
+    # If the auth supplies a name / username, use those. Otherwise start with email.
+    result.name = name = data[:name] || data[:email]
+    result.username = username = data[:nickname] || data[:email]
+
+    user_open_id = UserOpenId.find_by_url(identity_url)
+
+    if !user_open_id && @opts[:trusted] && user = User.find_by_email(email)
+      user_open_id = UserOpenId.create(url: identity_url , user_id: user.id, email: email, active: true)
+    end
+
+    result.user = user_open_id.try(:user)
+    result.extra_data = {
+      openid_url: identity_url
+    }
+    result.email_valid = @opts[:trusted]
+
+    result
+  end
+end

lib/auth/persona_authenticator.rb

@@ -0,0 +1,18 @@
+class Auth::PersonaAuthenticator < Auth::Authenticator
+
+  def name
+    "persona"
+  end
+
+  # TODO twitter provides all sorts of extra info, like website/bio etc.
+  #  it may be worth considering pulling some of it in.
+  def after_authenticate(auth_token)
+    result = Auth::Result.new
+
+    result.email = email = auth_token[:info][:email]
+    result.email_valid = true
+
+    result.user = User.find_by_email(email)
+    result
+  end
+end

lib/auth/result.rb

@@ -0,0 +1,35 @@
+class Auth::Result
+  attr_accessor :user, :name, :username, :email, :user,
+                :email_valid, :extra_data, :awaiting_activation,
+                :awaiting_approval, :authenticated, :authenticator_name
+
+  def session_data
+    {
+      email: email,
+      username: username,
+      email_valid: email_valid,
+      name: name,
+      authenticator_name: authenticator_name,
+      extra_data: extra_data
+    }
+  end
+
+  def to_client_hash
+    if user
+      {
+        authenticated: !!authenticated,
+        awaiting_activation: !!awaiting_activation,
+        awaiting_approval: !!awaiting_approval
+      }
+    else
+      {
+        email: email,
+        name:  User.suggest_name(name || username || email),
+        username: UserNameSuggester.suggest(username || name || email),
+        # this feels a tad wrong
+        auth_provider: authenticator_name.capitalize,
+        email_valid: !!email_valid
+      }
+    end
+  end
+end

lib/auth/twitter_authenticator.rb

@@ -0,0 +1,40 @@
+class Auth::TwitterAuthenticator < Auth::Authenticator
+
+  def name
+    "twitter"
+  end
+
+  # TODO twitter provides all sorts of extra info, like website/bio etc.
+  #  it may be worth considering pulling some of it in.
+  def after_authenticate(auth_token)
+
+    result = Auth::Result.new
+
+    data = auth_token[:info]
+
+    result.username = screen_name = data["nickname"]
+    result.name = name = data["name"]
+    twitter_user_id = auth_token["uid"]
+
+    result.extra_data = {
+      twitter_user_id: twitter_user_id,
+      twitter_screen_name: screen_name
+    }
+
+    user_info = TwitterUserInfo.where(twitter_user_id: twitter_user_id).first
+
+    result.user = user_info.try(:user)
+
+    result
+  end
+
+  def after_create_account(user, auth)
+    data = auth[:extra_data]
+    TwitterUserInfo.create(
+      user_id: user.id,
+      screen_name: data[:twitter_screen_name],
+      twitter_user_id: data[:twitter_user_id]
+    )
+  end
+
+end