Don't use return_to, but instead pass state with signed return_to parameter

5 files changed, 19 insertions, 13 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 9852736..0d27134 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -1,4 +1,6 @@
 class ApplicationController < ActionController::Base
+  include UserSessionsHelper
+
   rescue_from Network::UnauthorizedError, with: :invalid_token
   before_filter :default_headers
   before_filter :check_config
@@ -39,7 +41,7 @@ class ApplicationController < ActionController::Base
   def authenticate_public_page!
     unless project.public
       unless current_user
-        redirect_to(new_user_sessions_path(return_to: request.fullpath)) and return
+        redirect_to(new_user_sessions_path(state: generate_oauth_state(request.fullpath))) and return
       end
 
       unless current_user.can_access_project?(project.gitlab_id)
diff --git a/app/controllers/user_sessions_controller.rb b/app/controllers/user_sessions_controller.rb
index e486b24..23e4182 100644
--- a/app/controllers/user_sessions_controller.rb
+++ b/app/controllers/user_sessions_controller.rb
@@ -1,6 +1,4 @@
 class UserSessionsController < ApplicationController
-  include UserSessionsHelper
-
   before_filter :authenticate_user!, except: [:new, :callback, :auth]
 
   def show
@@ -11,9 +9,14 @@ class UserSessionsController < ApplicationController
   end
 
   def auth
+    unless is_oauth_state_valid?(params[:state])
+      redirect_to new_user_sessions_path
+      return
+    end
+
     redirect_to client.auth_code.authorize_url({
       redirect_uri: callback_user_sessions_url,
-      state: generate_oauth_state(params[:return_to])
+      state: params[:state]
     })
   end
 
diff --git a/app/helpers/user_sessions_helper.rb b/app/helpers/user_sessions_helper.rb
index e5853b5..df158c6 100644
--- a/app/helpers/user_sessions_helper.rb
+++ b/app/helpers/user_sessions_helper.rb
@@ -3,17 +3,18 @@ module UserSessionsHelper
     SecureRandom.hex(16)
   end
 
-  def generate_oauth_secret(salt, return_to)
+  def generate_oauth_hmac(salt, return_to)
     return unless return_to
-    message = GitlabCi::Application.config.secret_key_base + salt + return_to
-    Digest::SHA256.hexdigest message
+    digest = OpenSSL::Digest.new('sha256')
+    key = GitlabCi::Application.config.secret_key_base + salt
+    OpenSSL::HMAC.hexdigest(digest, key, return_to)
   end
 
   def generate_oauth_state(return_to)
     return unless return_to
     salt = generate_oauth_salt
-    secret = generate_oauth_secret(salt, return_to)
-    "#{salt}:#{secret}:#{return_to}"
+    hmac = generate_oauth_hmac(salt, return_to)
+    "#{salt}:#{hmac}:#{return_to}"
   end
 
   def get_ouath_state_return_to(state)
@@ -22,8 +23,8 @@ module UserSessionsHelper
 
   def is_oauth_state_valid?(state)
     return true unless state
-    salt, secret, return_to = state.split(':', 3)
+    salt, hmac, return_to = state.split(':', 3)
     return false unless return_to
-    secret == generate_oauth_secret(salt, return_to)
+    hmac == generate_oauth_hmac(salt, return_to)
   end
 end
diff --git a/app/views/projects/_public.html.haml b/app/views/projects/_public.html.haml
index 11eccbd..9662cc9 100644
--- a/app/views/projects/_public.html.haml
+++ b/app/views/projects/_public.html.haml
@@ -3,7 +3,7 @@
     Public projects
 
 .bs-callout
-  = link_to new_user_sessions_path(return_to: request.fullpath) do
+  = link_to new_user_sessions_path(state: generate_oauth_state(request.fullpath)) do
     %strong Login with GitLab
   to see your private projects
 
diff --git a/app/views/user_sessions/new.html.haml b/app/views/user_sessions/new.html.haml
index b457e93..c5be95b 100644
--- a/app/views/user_sessions/new.html.haml
+++ b/app/views/user_sessions/new.html.haml
@@ -4,5 +4,5 @@
     Make sure you have account on GitLab server
     = link_to GitlabCi.config.gitlab_server.url, GitlabCi.config.gitlab_server.url, no_turbolink
   %hr
-  = link_to "Login with GitLab", auth_user_sessions_path(return_to: params[:return_to]), no_turbolink.merge( class: 'btn btn-login btn-success' )
+  = link_to "Login with GitLab", auth_user_sessions_path(state: params[:state]), no_turbolink.merge( class: 'btn btn-login btn-success' )