From b52aba15e072e2e6fe50e9f4049def9feafcd19f Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 23 Aug 2013 16:20:43 +1000 Subject: [PATCH] major refactor of auth, break up the gigantic omniauth controller into sub classes for way better extensibitily --- .../users/omniauth_callbacks_controller.rb | 393 +++--------------- app/controllers/users_controller.rb | 52 +-- lib/auth/authenticator.rb | 24 ++ lib/auth/cas_authenticator.rb | 42 ++ lib/auth/facebook_authenticator.rb | 56 +++ lib/auth/github_authenticator.rb | 48 +++ lib/auth/oauth2_authenticator.rb | 55 +++ lib/auth/open_id_authenticator.rb | 38 ++ lib/auth/persona_authenticator.rb | 18 + lib/auth/result.rb | 35 ++ lib/auth/twitter_authenticator.rb | 40 ++ 11 files changed, 407 insertions(+), 394 deletions(-) create mode 100644 lib/auth/authenticator.rb create mode 100644 lib/auth/cas_authenticator.rb create mode 100644 lib/auth/facebook_authenticator.rb create mode 100644 lib/auth/github_authenticator.rb create mode 100644 lib/auth/oauth2_authenticator.rb create mode 100644 lib/auth/open_id_authenticator.rb create mode 100644 lib/auth/persona_authenticator.rb create mode 100644 lib/auth/result.rb create mode 100644 lib/auth/twitter_authenticator.rb diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 5c83b20ad..895927b71 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/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 - 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 + authenticator = self.class.find_authenticator(params[:provider]) - unless found - # Make sure we support that provider - raise Discourse::InvalidAccess.new unless self.class.types.keys.map(&:to_s).include?(provider) + @data = authenticator.after_authenticate(auth) + @data.authenticator_name = authenticator.name - # Check if the provider is enabled - raise Discourse::InvalidAccess.new("provider is not enabled") unless SiteSetting.send("enable_#{provider}_logins?") + user_found(@data.user) if @data.user - # Call the appropriate logic - send("create_or_sign_on_user_using_#{provider}", request.env["omniauth.auth"]) - end - - @data[:awaiting_approval] = true if invite_only? + session[:authentication] = @data.session_data 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] - screen_name = data["nickname"] - twitter_user_id = auth_token["uid"] + def self.find_authenticator(name) + BUILTIN_AUTH.each do |authenticator| + if authenticator.name == name + raise Discourse::InvalidAccess.new("provider is not enabled") unless SiteSetting.send("enable_#{name}_logins?") - session[:authentication] = { - twitter_user_id: twitter_user_id, - twitter_screen_name: screen_name - } + return authenticator + end + end - user_info = TwitterUserInfo.where(twitter_user_id: twitter_user_id).first + Discourse.auth_providers.each do |provider| + if provider.name == name - @data = { - username: screen_name, - auth_provider: "Twitter" - } + return provider.authenticator + end + 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] - raw_info = auth_token["extra"]["raw_info"] + def user_found(user) + # 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] - name = data["name"] - fb_uid = auth_token["uid"] - - - 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 + # log on any account that is active with forum access + if Guardian.new(user).can_access_forum? && user.active + log_on_user(user) + @data.authenticated = true else - if user = User.where(email: email).first - user.create_facebook_user_info! session[:authentication][:facebook] - 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} + if SiteSetting.invite_only? + @data.awaiting_approval = 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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9bd9fb5d9..af8d1622b 100644 --- a/app/controllers/users_controller.rb +++ b/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) - TwitterUserInfo.create( - 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 + authenticator = Users::OmniauthCallbacksController.find_authenticator(auth[:authenticator_name]) + authenticator.after_create_account(user, auth) 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? diff --git a/lib/auth/authenticator.rb b/lib/auth/authenticator.rb new file mode 100644 index 000000000..351d3591e --- /dev/null +++ b/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 diff --git a/lib/auth/cas_authenticator.rb b/lib/auth/cas_authenticator.rb new file mode 100644 index 000000000..9adf9ff54 --- /dev/null +++ b/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 diff --git a/lib/auth/facebook_authenticator.rb b/lib/auth/facebook_authenticator.rb new file mode 100644 index 000000000..f8b4e895c --- /dev/null +++ b/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 diff --git a/lib/auth/github_authenticator.rb b/lib/auth/github_authenticator.rb new file mode 100644 index 000000000..c24f76322 --- /dev/null +++ b/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 diff --git a/lib/auth/oauth2_authenticator.rb b/lib/auth/oauth2_authenticator.rb new file mode 100644 index 000000000..089f006f7 --- /dev/null +++ b/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 diff --git a/lib/auth/open_id_authenticator.rb b/lib/auth/open_id_authenticator.rb new file mode 100644 index 000000000..ab249f2bd --- /dev/null +++ b/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 diff --git a/lib/auth/persona_authenticator.rb b/lib/auth/persona_authenticator.rb new file mode 100644 index 000000000..029c4ba7c --- /dev/null +++ b/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 diff --git a/lib/auth/result.rb b/lib/auth/result.rb new file mode 100644 index 000000000..d76f3e045 --- /dev/null +++ b/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 diff --git a/lib/auth/twitter_authenticator.rb b/lib/auth/twitter_authenticator.rb new file mode 100644 index 000000000..f0728c482 --- /dev/null +++ b/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