From ad5a5b486645f6bd16a8c5d41743219fef64b111 Mon Sep 17 00:00:00 2001
From: Jesse Pollak <jpollak92@gmail.com>
Date: Thu, 14 Feb 2013 11:11:13 -0800
Subject: [PATCH] This commit adds a callback route to handle omniauth failure
 and removes a few unneccessary entries in en.yml

---
 .../users/omniauth_callbacks_controller.rb    |  7 +++-
 .../users/omniauth_callbacks/failure.html.erb | 11 ++++++
 config/initializers/omniauth.rb               | 33 ++++++++--------
 config/locales/server.en.yml                  | 38 ++++++++++---------
 config/routes.rb                              | 37 +++++++++---------
 .../failure.html.erb_spec.rb                  | 12 ++++++
 6 files changed, 84 insertions(+), 54 deletions(-)
 create mode 100644 app/views/users/omniauth_callbacks/failure.html.erb
 create mode 100644 spec/views/omniauth_callbacks/failure.html.erb_spec.rb

diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
index 63cceafa06e..d4e98a2f7a1 100644
--- a/app/controllers/users/omniauth_callbacks_controller.rb
+++ b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -22,6 +22,11 @@ class Users::OmniauthCallbacksController < ApplicationController
     end
   end
 
+  def failure
+    flash[:error] = I18n.t("login.omniauth_error", strategy: params[:strategy].titleize)
+    render :layout => 'no_js'
+  end
+
   def create_or_sign_on_user_using_twitter(auth_token)
 
     data = auth_token[:info]
@@ -62,7 +67,7 @@ class Users::OmniauthCallbacksController < ApplicationController
     email = data[:email]
     name = data["name"]
     fb_uid = auth_token["uid"]
-  
+
 
     username = User.suggest_username(name)
 
diff --git a/app/views/users/omniauth_callbacks/failure.html.erb b/app/views/users/omniauth_callbacks/failure.html.erb
new file mode 100644
index 00000000000..cbd8f8c6031
--- /dev/null
+++ b/app/views/users/omniauth_callbacks/failure.html.erb
@@ -0,0 +1,11 @@
+<div id="simple-container">
+  <%if flash[:error]%>
+    <div class='alert alert-error'>
+      <%=flash[:error]%>
+    </div>
+  <%else%>
+    <div class='alert alert-error'>
+      Something went wrong processing your log in, please try again.
+    </div>
+  <%end%>
+</div>
\ No newline at end of file
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb
index 2ab0e6f77eb..41e4c8d4d42 100644
--- a/config/initializers/omniauth.rb
+++ b/config/initializers/omniauth.rb
@@ -1,31 +1,30 @@
-require 'openid/store/filesystem'
 require 'openssl'
 require 'openid_redis_store'
 
-# if you need to test this and are having ssl issues see: 
+# if you need to test this and are having ssl issues see:
 #  http://stackoverflow.com/questions/6756460/openssl-error-using-omniauth-specified-ssl-path-but-didnt-work
 
 Rails.application.config.middleware.use OmniAuth::Builder do
 
-  provider :open_id, 
-           :store => OpenID::Store::Redis.new($redis), 
-           :name => 'google', 
-           :identifier => 'https://www.google.com/accounts/o8/id', 
-           :require => 'omniauth-openid' 
+  provider :open_id,
+           :store => OpenID::Store::Redis.new($redis),
+           :name => 'google',
+           :identifier => 'https://www.google.com/accounts/o8/id',
+           :require => 'omniauth-openid'
 
-  provider :open_id, 
-           :store => OpenID::Store::Redis.new($redis), 
-           :name => 'yahoo', 
-           :identifier => 'https://me.yahoo.com', 
-           :require => 'omniauth-openid' 
+  provider :open_id,
+           :store => OpenID::Store::Redis.new($redis),
+           :name => 'yahoo',
+           :identifier => 'https://me.yahoo.com',
+           :require => 'omniauth-openid'
 
-  provider :facebook, 
-           SiteSetting.facebook_app_id, 
-           SiteSetting.facebook_app_secret, 
+  provider :facebook,
+           SiteSetting.facebook_app_id,
+           SiteSetting.facebook_app_secret,
            :scope => "email"
 
-  provider :twitter, 
-           SiteSetting.twitter_consumer_key, 
+  provider :twitter,
+           SiteSetting.twitter_consumer_key,
            SiteSetting.twitter_consumer_secret
 
 end
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 27321387309..64e39f82cb7 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -13,7 +13,7 @@ en:
 
   education:
     until_posts:
-      one: "post" 
+      one: "post"
       other: "%{count} posts"
 
     'new-topic': |
@@ -66,7 +66,7 @@ en:
   trust_levels:
     new:
       title: "new user"
-    basic: 
+    basic:
       title: "basic user"
     regular:
       title: "regular user"
@@ -91,7 +91,7 @@ en:
 
   datetime:
     distance_in_words:
-      half_a_minute: "< 1m"   
+      half_a_minute: "< 1m"
       less_than_x_seconds:
         one:   "< 1s"
         other: "< %{count}s"
@@ -127,7 +127,7 @@ en:
         other: "%{count}y"
 
     distance_in_words_verbose:
-      half_a_minute: "just now"   
+      half_a_minute: "just now"
       less_than_x_seconds:
         one:   "just now"
         other: "just now"
@@ -212,7 +212,7 @@ en:
     user_must_edit: '<p>Flagged content hidden.</p>'
 
   archetypes:
-    regular: 
+    regular:
       title: "Regular Topic"
 
   unsubscribed:
@@ -363,7 +363,7 @@ en:
     types:
       category: 'Categories'
       topic: 'Topics'
-      user: 'Users'   
+      user: 'Users'
 
   youve_posted: "You've Posted"
   original_poster: "Original Poster"
@@ -397,7 +397,7 @@ en:
     edited: "edited"
 
   move_posts:
-    moderator_post: 
+    moderator_post:
       one: "I moved a post to a new topic: %{topic_link}"
       other: "I moved %{count} posts to a new topic: %{topic_link}"
 
@@ -411,7 +411,7 @@ en:
     visible_enabled: "This topic is now visible. It will be displayed in topic lists."
     visible_disabled: "This topic is now invisible. It will no longer be displayed in any topic lists. The only way to access this topic is via direct link."
 
-  login: 
+  login:
     not_approved: "Your account hasn't been approved yet. You will be notified by email when you are ready to log in."
     incorrect_username_email_or_password: "Incorrect username, email or password"
     wait_approval: "Thanks for signing up. We will notify you when your account has been approved."
@@ -420,6 +420,8 @@ en:
     not_activated: "You can't log in yet. We sent an activation email to you. Please follow the instructions in the email to activate your account."
     errors: "%{errors}"
     not_available: "Not available. Try %{suggestion}?"
+    omniauth_error: "Sorry, there was an error authorizing your %{strategy} account. Perhaps you did not approve authorization?"
+
 
   user:
     username:
@@ -494,7 +496,7 @@ en:
       subject_template: "%{site_name} Notice: Posting Hidden due to Community Flagging"
       text_body_template: |
         Hello,
-        
+
         This is an automated message from %{site_name} to inform you that the following post was hidden as a result of community flagging.
 
         %{base_url}%{url}
@@ -608,7 +610,7 @@ en:
   unsubscribe_link: "If you'd like to unsubscribe from these emails, visit your [user preferences](%{user_preferences_url})."
 
   user_notifications:
-    unsubscribe: 
+    unsubscribe:
       title: "Unsubscribe"
       description: "Not interested in getting these emails? No problem! Click below to unsubscribe instantly:"
 
@@ -627,7 +629,7 @@ en:
         ---
         %{message}
 
-        --- 
+        ---
         Please visit this link to respond: %{base_url}%{url}
 
     user_quoted:
@@ -638,7 +640,7 @@ en:
         ---
         %{message}
 
-        --- 
+        ---
         Please visit this link to respond: %{base_url}%{url}
 
     user_mentioned:
@@ -649,7 +651,7 @@ en:
         ---
         %{message}
 
-        --- 
+        ---
         Please visit this link to respond: %{base_url}%{url}
 
 
@@ -675,9 +677,9 @@ en:
       subject_template: "[%{site_name}] Password reset"
       text_body_template: |
         Somebody asked to reset your password on [%{site_name}](%{base_url}).
-        
-        If it was not you, you can safely ignore this email. 
-        
+
+        If it was not you, you can safely ignore this email.
+
         Click the following link to choose a new password:
         %{base_url}/users/password-reset/%{email_token}
 
@@ -685,14 +687,14 @@ en:
       subject_template: "[%{site_name}] Confirm your new email address"
       text_body_template: |
         Confirm your new email address for %{site_name} by clicking on the following link:
-        
+
         %{base_url}/users/authorize-email/%{email_token}
 
     signup:
       subject_template: "[%{site_name}] Activate your new account"
       text_body_template: |
         Welcome to %{site_name}!
-        
+
         Click the following link to confirm and activate your new account:
         %{base_url}/users/activate-account/%{email_token}
 
diff --git a/config/routes.rb b/config/routes.rb
index fc9c63cdf93..0e0bf55e875 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -3,7 +3,7 @@ require 'sidekiq/web'
 require_dependency 'admin_constraint'
 
 # This used to be User#username_format, but that causes a preload of the User object
-# and makes Guard not work properly. 
+# and makes Guard not work properly.
 USERNAME_ROUTE_FORMAT = /[A-Za-z0-9\._]+/
 
 Discourse::Application.routes.draw do
@@ -12,8 +12,8 @@ Discourse::Application.routes.draw do
 
   mount Sidekiq::Web => '/sidekiq', constraints: AdminConstraint.new
 
-  resources :forums do 
-    collection do 
+  resources :forums do
+    collection do
       get 'request_access'
       post 'request_access_submit'
     end
@@ -60,24 +60,24 @@ Discourse::Application.routes.draw do
   post 'email/resubscribe/:key' => 'email#resubscribe', as: 'email_resubscribe'
 
 
-  resources :session, id: USERNAME_ROUTE_FORMAT do 
-    collection do 
+  resources :session, id: USERNAME_ROUTE_FORMAT do
+    collection do
       post 'forgot_password'
     end
-  end  
+  end
 
-  resources :users, :except => [:show, :update] do 
-    collection do 
+  resources :users, :except => [:show, :update] do
+    collection do
       get 'check_username'
       get 'is_local_username'
     end
   end
 
   resources :static
-  get 'faq' => 'static#show', id: 'faq'  
+  get 'faq' => 'static#show', id: 'faq'
   get 'tos' => 'static#show', id: 'tos'
   get 'privacy' => 'static#show', id: 'privacy'
-  
+
   get 'users/search/users' => 'users#search_users'
   get 'users/password-reset/:token' => 'users#password_reset'
   put 'users/password-reset/:token' => 'users#password_reset'
@@ -88,10 +88,10 @@ Discourse::Application.routes.draw do
   get 'user_preferences' => 'users#user_preferences_redirect'
   get 'users/:username/private-messages' => 'user_actions#private_messages', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}
   get 'users/:username' => 'users#show', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}
-  put 'users/:username' => 'users#update', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}  
+  put 'users/:username' => 'users#update', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}
   get 'users/:username/preferences' => 'users#preferences', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}, :as => :email_preferences
   get 'users/:username/preferences/email' => 'users#preferences', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}
-  put 'users/:username/preferences/email' => 'users#change_email', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}  
+  put 'users/:username/preferences/email' => 'users#change_email', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}
   get 'users/:username/preferences/username' => 'users#preferences', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}
   put 'users/:username/preferences/username' => 'users#username', :format => false, :constraints => {:username => USERNAME_ROUTE_FORMAT}
   get 'users/:username/avatar(/:size)' => 'users#avatar', :constraints => {:username => USERNAME_ROUTE_FORMAT}
@@ -115,13 +115,14 @@ Discourse::Application.routes.draw do
   resources :categories
 
   match "/auth/:provider/callback", to: "users/omniauth_callbacks#complete"
+  match "/auth/failure", to: "users/omniauth_callbacks#failure"
 
   get 'twitter/frame' => 'twitter#frame'
   get 'twitter/complete' => 'twitter#complete'
 
   get 'facebook/frame' => 'facebook#frame'
   get 'facebook/complete' => 'facebook#complete'
-  
+
   resources :clicks do
     collection do
       get 'track' => 'clicks#track'
@@ -162,13 +163,13 @@ Discourse::Application.routes.draw do
   get 't/:id' => 'topics#show'
   delete 't/:id' => 'topics#destroy'
   put 't/:id' => 'topics#update'
-  post 't' => 'topics#create'  
+  post 't' => 'topics#create'
   post 'topics/timings' => 'topics#timings'
 
   # Legacy route for old avatars
   get 'threads/:topic_id/:post_number/avatar' => 'topics#avatar', :constraints => {:topic_id => /\d+/, :post_number => /\d+/}
 
-  # Topic routes  
+  # Topic routes
   get 't/:slug/:topic_id/best_of' => 'topics#show', :constraints => {:topic_id => /\d+/, :post_number => /\d+/}
   get 't/:topic_id/best_of' => 'topics#show', :constraints => {:topic_id => /\d+/, :post_number => /\d+/}
   put 't/:slug/:topic_id' => 'topics#update', :constraints => {:topic_id => /\d+/}
@@ -181,18 +182,18 @@ Discourse::Application.routes.draw do
 
   get 't/:topic_id/:post_number' => 'topics#show', :constraints => {:topic_id => /\d+/, :post_number => /\d+/}
   get 't/:slug/:topic_id' => 'topics#show', :constraints => {:topic_id => /\d+/}
-  get 't/:slug/:topic_id/:post_number' => 'topics#show', :constraints => {:topic_id => /\d+/, :post_number => /\d+/}  
+  get 't/:slug/:topic_id/:post_number' => 'topics#show', :constraints => {:topic_id => /\d+/, :post_number => /\d+/}
   post 't/:topic_id/timings' => 'topics#timings', :constraints => {:topic_id => /\d+/}
   post 't/:topic_id/invite' => 'topics#invite', :constraints => {:topic_id => /\d+/}
   post 't/:topic_id/move-posts' => 'topics#move_posts', :constraints => {:topic_id => /\d+/}
   delete 't/:topic_id/timings' => 'topics#destroy_timings', :constraints => {:topic_id => /\d+/}
 
   post 't/:topic_id/notifications' => 'topics#set_notifications' , :constraints => {:topic_id => /\d+/}
-  
+
 
   resources :invites
   delete 'invites' => 'invites#destroy'
-  
+
   get 'request_access' => 'request_access#new'
   post 'request_access' => 'request_access#create'
 
diff --git a/spec/views/omniauth_callbacks/failure.html.erb_spec.rb b/spec/views/omniauth_callbacks/failure.html.erb_spec.rb
new file mode 100644
index 00000000000..d6e8eadf21a
--- /dev/null
+++ b/spec/views/omniauth_callbacks/failure.html.erb_spec.rb
@@ -0,0 +1,12 @@
+require "spec_helper"
+
+describe "users/omniauth_callbacks/failure.html.erb" do
+
+    it "renders the failure page" do
+        flash[:error] = I18n.t("login.omniauth_error", strategy: 'test')
+        render
+
+        rendered.match(I18n.t("login.omniauth_error", strategy: 'test')).should be_true
+    end
+
+end
\ No newline at end of file