Added random salt and hashing to oauth state parameter

This ensures that content of state is generated by CI, but doesn't prevent replay attacks on state parameter.

2 files changed, 37 insertions, 2 deletions

diff --git a/app/controllers/user_sessions_controller.rb b/app/controllers/user_sessions_controller.rb
index 2f972bc..e486b24 100644
--- a/app/controllers/user_sessions_controller.rb
+++ b/app/controllers/user_sessions_controller.rb
@@ -1,4 +1,6 @@
 class UserSessionsController < ApplicationController
+  include UserSessionsHelper
+
   before_filter :authenticate_user!, except: [:new, :callback, :auth]
 
   def show
@@ -11,18 +13,24 @@ class UserSessionsController < ApplicationController
   def auth
     redirect_to client.auth_code.authorize_url({
       redirect_uri: callback_user_sessions_url,
-      state: params[:return_to]
+      state: generate_oauth_state(params[:return_to])
     })
   end
 
   def callback
+    unless is_oauth_state_valid?(params[:state])
+      redirect_to new_user_sessions_path
+      return
+    end
+
     token = client.auth_code.get_token(params[:code], redirect_uri: callback_user_sessions_url).token
     
     @user_session = UserSession.new
     user = @user_session.authenticate(access_token: token)
 
     if user && sign_in(user)
-      redirect_to(params[:state] || root_path)
+      return_to = get_ouath_state_return_to(params[:state])
+      redirect_to(return_to || root_path)
     else
       @error = 'Invalid credentials'
       render :new
diff --git a/app/helpers/user_sessions_helper.rb b/app/helpers/user_sessions_helper.rb
index 2018402..e5853b5 100644
--- a/app/helpers/user_sessions_helper.rb
+++ b/app/helpers/user_sessions_helper.rb
@@ -1,2 +1,29 @@
 module UserSessionsHelper
+  def generate_oauth_salt
+    SecureRandom.hex(16)
+  end
+
+  def generate_oauth_secret(salt, return_to)
+    return unless return_to
+    message = GitlabCi::Application.config.secret_key_base + salt + return_to
+    Digest::SHA256.hexdigest message
+  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}"
+  end
+
+  def get_ouath_state_return_to(state)
+    state.split(':', 3)[2] if state
+  end
+
+  def is_oauth_state_valid?(state)
+    return true unless state
+    salt, secret, return_to = state.split(':', 3)
+    return false unless return_to
+    secret == generate_oauth_secret(salt, return_to)
+  end
 end