Merge branch 'gitlab-geo-oauth-auth' into 'master'

Gitlab Geo:  Authenticate with OAuth

This changes the way Geo (#76) nodes authenticates, to remove the requirement of session sharing between nodes, as described here: #366

# Checklist

 - [x] When a secondary Geo node is created an OAuth Application is created associated to it
 - [x] When a secondary Geo node is removed the associated OAuth application is also removed
 - [x] When a user tries to login it must be redirected to the primary node using the OAuth workflow
 - [x] The user must be redirected to the "first accessed url before authentication" (as expected)
 - [x] User should be able to logout from a secondary node
 - [x] It displays authentication errors when OAuth application is not present
 - [x] It handles authentication errors when OAuth flow fails
 - [x] Solution for what to do when/if OAuth application is removed

See merge request !237

app/controllers/admin/geo_nodes_controller.rb

@@ -22,6 +22,22 @@ class Admin::GeoNodesController < Admin::ApplicationController
     redirect_to admin_geo_nodes_path, notice: 'Node was successfully removed.'
   end
 
+  def repair
+    @node = GeoNode.find(params[:id])
+
+    if @node.primary? || !@node.missing_oauth_application?
+      flash[:notice] = "This node doesn't need to be repaired."
+    elsif @node.save
+      flash[:notice] = 'Node Authentication was successfully repaired.'
+    else
+      flash[:alert] = 'There was a problem repairing Node Authentication.'
+    end
+
+    redirect_to admin_geo_nodes_path
+  end
+
+  private
+
   def geo_node_params
     params.require(:geo_node).permit(:url, :primary, geo_node_key_attributes: [:key])
   end

app/controllers/application_controller.rb

@@ -102,7 +102,7 @@ class ApplicationController < ActionController::Base
       flash[:alert] = "Your account is blocked. Retry when an admin has unblocked it."
       new_user_session_path
     else
-      stored_location_for(:geo_node) || stored_location_for(:redirect) || stored_location_for(resource) || root_path
+      stored_location_for(:redirect) || stored_location_for(resource) || root_path
     end
   end
 

app/controllers/oauth/geo_auth_controller.rb

+class Oauth::GeoAuthController < ActionController::Base
+  rescue_from Gitlab::Geo::OauthApplicationUndefinedError, with: :undefined_oauth_application
+  rescue_from OAuth2::Error, with: :auth
+
+  def auth
+    oauth = Gitlab::Geo::OauthSession.new(state: params[:state])
+    unless oauth.is_oauth_state_valid?
+      redirect_to root_url
+      return
+    end
+
+    redirect_to oauth.authorize_url(redirect_uri: oauth_geo_callback_url, state: params[:state])
+  end
+
+  def callback
+    oauth = Gitlab::Geo::OauthSession.new(state: params[:state])
+    unless oauth.is_oauth_state_valid?
+      redirect_to new_user_sessions_path
+      return
+    end
+
+    token = oauth.get_token(params[:code], redirect_uri: oauth_geo_callback_url)
+    remote_user = oauth.authenticate_with_gitlab(token)
+
+    user = User.find(remote_user['id'])
+
+    if user && sign_in(user, bypass: true)
+      return_to = oauth.get_oauth_state_return_to
+      redirect_to(return_to || root_path)
+    else
+      invalid_credentials
+    end
+  end
+
+  private
+
+  def undefined_oauth_application
+    @error = 'There are no OAuth application defined for this Geo node. Please ask your administrator to visit "Geo Nodes" on admin screen and click on "Repair authentication".'
+    render :error, layout: 'errors'
+  end
+end

app/controllers/sessions_controller.rb

@@ -109,12 +109,13 @@ class SessionsController < Devise::SessionsController
 
   def gitlab_geo_login
     if !signed_in? && Gitlab::Geo.enabled? && Gitlab::Geo.secondary?
+      oauth = Gitlab::Geo::OauthSession.new
+
       # share full url with primary node by shared session
-      user_return_to = URI.join(root_url, session[:user_return_to]).to_s
-      session[:geo_node_return_to] = @redirect_to || user_return_to
+      user_return_to = URI.join(root_url, session[:user_return_to].to_s).to_s
+      oauth.return_to = @redirect_to || user_return_to
 
-      login_uri =  URI.join(Gitlab::Geo.primary_node.url, new_session_path(:user)).to_s
-      redirect_to login_uri
+      redirect_to oauth_geo_auth_url(state: oauth.generate_oauth_state)
     end
   end
 

app/models/geo_node.rb

@@ -12,6 +12,7 @@
 
 class GeoNode < ActiveRecord::Base
   belongs_to :geo_node_key, dependent: :destroy
+  belongs_to :oauth_application, class_name: 'Doorkeeper::Application', dependent: :destroy
 
   default_values schema: 'http',
                  host: lambda { Gitlab.config.gitlab.host },
@@ -22,14 +23,14 @@ class GeoNode < ActiveRecord::Base
   accepts_nested_attributes_for :geo_node_key
 
   validates :host, host: true, presence: true, uniqueness: { case_sensitive: false, scope: :port }
-  validates :primary, uniqueness: { message: 'primary node already exists' }, if: :primary
+  validates :primary, uniqueness: { message: 'node already exists' }, if: :primary
   validates :schema, inclusion: %w(http https)
   validates :relative_url_root, length: { minimum: 0, allow_nil: false }
 
-  after_initialize :check_geo_node_key
+  after_initialize :build_dependents
   after_save :refresh_bulk_notify_worker_status
   after_destroy :refresh_bulk_notify_worker_status
-  before_validation :change_geo_node_key_title
+  before_validation :update_dependents_attributes
 
   def uri
     if relative_url_root
@@ -55,6 +56,14 @@ class GeoNode < ActiveRecord::Base
     URI.join(uri, "#{uri.path}/", "api/#{API::API.version}/geo/refresh_projects").to_s
   end
 
+  def oauth_callback_url
+    URI.join(uri, "#{uri.path}/", 'oauth/geo/callback').to_s
+  end
+
+  def missing_oauth_application?
+    self.primary? ? false : !oauth_application.present?
+  end
+
   private
 
   def refresh_bulk_notify_worker_status
@@ -65,19 +74,27 @@ class GeoNode < ActiveRecord::Base
     end
   end
 
-  def check_geo_node_key
+  def build_dependents
     self.build_geo_node_key if geo_node_key.nil?
   end
 
-  def change_geo_node_key_title
+  def update_dependents_attributes
     self.geo_node_key.title = "Geo node: #{self.url}" if self.geo_node_key
+
+    if self.primary?
+      self.oauth_application = nil
+    else
+      self.build_oauth_application if oauth_application.nil?
+      self.oauth_application.name = "Geo node: #{self.url}"
+      self.oauth_application.redirect_uri = oauth_callback_url
+    end
   end
 
   def validate(record)
     # Prevent locking yourself out
     if record.host == Gitlab.config.gitlab.host &&
-       record.port == Gitlab.config.gitlab.port &&
-       record.relative_url_root == Gitlab.config.gitlab.relative_url_root && !record.primary
+      record.port == Gitlab.config.gitlab.port &&
+      record.relative_url_root == Gitlab.config.gitlab.relative_url_root && !record.primary
       record.errors[:base] << 'Current node must be the primary node or you will be locking yourself out'
     end
   end

app/views/admin/geo_nodes/index.html.haml

@@ -52,4 +52,8 @@
               %span.help-block #{node.primary ? 'Primary node' : 'Secondary node'}
 
           .pull-right
+            - if node.missing_oauth_application?
+              = link_to repair_admin_geo_node_path(node), method: :post, title: 'OAuth application is missing', class: 'btn btn-default, btn-sm' do
+                = icon('exclamation-triangle fw')
+                Repair authentication
             = link_to 'Remove', admin_geo_node_path(node), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm'

app/views/oauth/geo_auth/error.html.haml

+%h3.error Error
+%p
+  = @error

config/application.rb

@@ -81,7 +81,7 @@ module Gitlab
     ENV['GITLAB_PATH_OUTSIDE_HOOK'] = ENV['PATH']
 
     # Gitlab Geo Middleware support
-    config.middleware.use 'Gitlab::Middleware::ReadonlyGeo'
+    config.middleware.insert_after ActionDispatch::Flash, 'Gitlab::Middleware::ReadonlyGeo'
 
     config.generators do |g|
       g.factory_girl false

config/routes.rb

@@ -39,6 +39,11 @@ Rails.application.routes.draw do
                 authorizations: 'oauth/authorizations'
   end
 
+  namespace :oauth do
+    get 'geo/auth' => 'geo_auth#auth'
+    get 'geo/callback' => 'geo_auth#callback'
+  end
+
   # Autocomplete
   get '/autocomplete/users' => 'autocomplete#users'
   get '/autocomplete/users/:id' => 'autocomplete#user'
@@ -277,7 +282,11 @@ Rails.application.routes.draw do
       get :download, on: :member
     end
 
-    resources :geo_nodes, only: [:index, :create, :destroy]
+    resources :geo_nodes, only: [:index, :create, :destroy] do
+      member do
+        post :repair
+      end
+    end
 
     resources :labels
 

db/migrate/20160229034258_add_doorkeeper_application_to_geo_node.rb

+class AddDoorkeeperApplicationToGeoNode < ActiveRecord::Migration
+  def change
+    change_table :geo_nodes do |t|
+      t.belongs_to :oauth_application
+    end
+  end
+end

db/schema.rb

@@ -414,6 +414,7 @@ ActiveRecord::Schema.define(version: 20160309140734) do
     t.string  "relative_url_root"
     t.boolean "primary"
     t.integer "geo_node_key_id"
+    t.integer "oauth_application_id"
   end
 
   add_index "geo_nodes", ["geo_node_key_id"], name: "index_geo_nodes_on_geo_node_key_id", using: :btree

lib/gitlab/geo.rb

 module Gitlab
   module Geo
+    class OauthApplicationUndefinedError < StandardError; end
+
     def self.current_node
       RequestStore.store[:geo_node_current] ||= begin
         GeoNode.find_by(host: Gitlab.config.gitlab.host,
@@ -39,5 +41,12 @@ module Gitlab
     def self.bulk_notify_job
       Sidekiq::Cron::Job.find('geo_bulk_notify_worker')
     end
+
+    def self.oauth_authentication
+      return false unless Gitlab::Geo.secondary?
+
+      RequestStore.store[:geo_oauth_application] ||= Gitlab::Geo.current_node.oauth_application or
+                                                     raise OauthApplicationUndefinedError
+    end
   end
 end

lib/gitlab/geo/oauth_session.rb

+module Gitlab
+  module Geo
+    class OauthSession
+      include ActiveModel::Model
+
+      attr_accessor :state
+      attr_accessor :return_to
+
+      def is_oauth_state_valid?
+        return false unless state
+        salt, hmac, return_to = state.split(':', 3)
+
+        return false unless return_to
+        hmac == generate_oauth_hmac(salt, return_to)
+      end
+
+      def generate_oauth_state
+        return unless return_to
+        hmac = generate_oauth_hmac(oauth_salt, return_to)
+        "#{oauth_salt}:#{hmac}:#{return_to}"
+      end
+
+      def get_oauth_state_return_to
+        state.split(':', 3)[2] if state
+      end
+
+      def authorize_url(params = {})
+        oauth_client.auth_code.authorize_url(params)
+      end
+
+      def get_token(code, params = {}, opts = {})
+        oauth_client.auth_code.get_token(code, params, opts).token
+      end
+
+      def authenticate_with_gitlab(access_token)
+        return false unless access_token
+
+        api = OAuth2::AccessToken.from_hash(oauth_client, access_token: access_token)
+        api.get('/api/v3/user').parsed
+      end
+
+      private
+
+      def generate_oauth_hmac(salt, return_to)
+        return false unless return_to
+        digest = OpenSSL::Digest.new('sha256')
+        key = Gitlab::Application.secrets.secret_key_base + salt
+        OpenSSL::HMAC.hexdigest(digest, key, return_to)
+      end
+
+      def oauth_salt
+        @salt ||= SecureRandom.hex(16)
+      end
+
+      def oauth_client
+        @client ||= begin
+          ::OAuth2::Client.new(
+            oauth_app.uid,
+            oauth_app.secret,
+            {
+              site: primary_node_url,
+              authorize_url: 'oauth/authorize',
+              token_url: 'oauth/token'
+            }
+          )
+        end
+      end
+
+      def oauth_app
+        Gitlab::Geo.oauth_authentication
+      end
+
+      def primary_node_url
+        Gitlab::Geo.primary_node.url
+      end
+    end
+  end
+end

lib/gitlab/middleware/readonly_geo.rb

@@ -2,6 +2,7 @@ module Gitlab
   module Middleware
     class ReadonlyGeo
       DISALLOWED_METHODS = %w(POST PATCH PUT DELETE)
+      WHITELISTED = %w(api/v3/internal api/v3/geo/refresh_projects)
 
       def initialize(app)
         @app = app
@@ -49,8 +50,7 @@ module Gitlab
       end
 
       def whitelisted_routes
-        whitelisted = %w(api/v3/internal api/v3/geo/refresh_projects)
-        logout_route || whitelisted.any? { |path| @request.path.include?(path) }
+        logout_route || WHITELISTED.any? { |path| @request.path.include?(path) }
       end
 
       def logout_route

spec/factories/doorkeeper.rb

+FactoryGirl.define do
+  factory :doorkeeper_access_grant, class: Doorkeeper::AccessGrant do
+    sequence(:resource_owner_id) { |n| n }
+    association :application, factory: :doorkeeper_application
+    redirect_uri 'https://app.com/callback'
+    expires_in 100
+    scopes 'public write'
+  end
+
+  factory :doorkeeper_access_token, class: Doorkeeper::AccessToken do
+    sequence(:resource_owner_id) { |n| n }
+    association :application, factory: :doorkeeper_application
+    expires_in 2.hours
+
+    factory :clientless_access_token do
+      application nil
+    end
+  end
+
+  factory :doorkeeper_application, class: Doorkeeper::Application do
+    sequence(:name) { |n| "Application #{n}" }
+    redirect_uri 'https://app.com/callback'
+  end
+end

spec/lib/gitlab/geo/oauth_session_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Geo::OauthSession do
+  subject { described_class.new }
+  let(:oauth_app) { FactoryGirl.create(:doorkeeper_application) }
+  let(:oauth_return_to) { 'http://localhost:3000/oath/geo/callback' }
+  let(:dummy_state) { 'salt:hmac:return_to' }
+  let(:valid_state) { described_class.new(return_to: oauth_return_to).generate_oauth_state }
+  let(:access_token) { FactoryGirl.create(:doorkeeper_access_token).token }
+  let(:user) { FactoryGirl.build(:user) }
+
+  before(:each) do
+    allow(subject).to receive(:oauth_app) { oauth_app }
+    allow(subject).to receive(:primary_node_url) { 'http://localhost:3001/' }
+  end
+
+  describe '#is_oauth_state_valid?' do
+    it 'returns false when state is not present' do
+      expect(subject.is_oauth_state_valid?).to be_falsey
+    end
+
+    it 'returns false when return_to cannot be retrieved' do
+      subject.state = 'invalidstate'
+      expect(subject.is_oauth_state_valid?).to be_falsey
+    end
+
+    it 'returns false when hmac does not match' do
+      subject.state = dummy_state
+      expect(subject.is_oauth_state_valid?).to be_falsey
+    end
+
+    it 'returns true when hmac matches generated one' do
+      subject.state = valid_state
+      expect(subject.is_oauth_state_valid?).to be_truthy
+    end
+  end
+
+  describe '#generate_oauth_state' do
+    it 'returns nil when return_to is not present' do
+      state = subject.generate_oauth_state
+      expect(state).to be_nil
+    end
+
+    context 'when return_to is present' do
+      it 'returns a string' do
+        expect(valid_state).to be_a String
+        expect(valid_state).not_to be_empty
+      end
+
+      it 'includes return_to value' do
+        expect(valid_state).to include(oauth_return_to)
+      end
+    end
+  end
+
+  describe '#get_oauth_state_return_to' do
+    subject { described_class.new(state: valid_state) }
+
+    it 'returns return_to value' do
+      expect(subject.get_oauth_state_return_to).to eq(oauth_return_to)
+    end
+  end
+
+  describe '#authorized_url' do
+    subject { described_class.new(return_to: oauth_return_to) }
+
+    it 'returns a valid url' do
+      expect(subject.authorize_url).to be_a String
+      expect(subject.authorize_url).to include('http://localhost:3001/')
+    end
+  end
+
+  describe '#authenticate_with_gitlab' do
+    let(:response) { double }
+    before(:each) { allow_any_instance_of(OAuth2::AccessToken).to receive(:get) { response } }
+
+    context 'on success' do
+      it 'returns hashed user data' do
+        allow(response).to receive(:status) { 200 }
+        allow(response).to receive(:parsed) { user.to_json }
+
+        subject.authenticate_with_gitlab(access_token)
+      end
+    end
+
+    context 'on invalid token' do
+      it 'raises exception' do
+        allow(response).to receive(:status) { 401 }
+
+        expect { subject.authenticate_with_gitlab(access_token) }.to raise_error
+      end
+    end
+  end
+end

spec/lib/gitlab/geo/update_queue_spec.rb

+require 'spec_helper'
+
 describe Gitlab::Geo::UpdateQueue do
   subject { described_class.new }
   let(:dummy_data) { { 'id' => 1, 'clone_url' => 'git@localhost:repo/path.git' } }

spec/models/geo_node_spec.rb

 require 'spec_helper'
 
 describe GeoNode, type: :model do
+  subject(:new_node) { described_class.new(schema: 'https', host: 'localhost', port: 3000, relative_url_root: 'gitlab') }
+  subject(:new_primary_node) { described_class.new(schema: 'https', host: 'localhost', port: 3000, relative_url_root: 'gitlab', primary: true) }
+  subject(:empty_node) { described_class.new(schema: nil, host: nil, port: nil, relative_url_root: nil) }
+  subject(:primary_node) { FactoryGirl.create(:geo_node, :primary) }
+  subject(:node) { FactoryGirl.create(:geo_node) }
+
   let(:dummy_url) { 'https://localhost:3000/gitlab' }
 
   context 'associations' do
     it { is_expected.to belong_to(:geo_node_key).dependent(:destroy) }
+    it { is_expected.to belong_to(:oauth_application).dependent(:destroy) }
   end
 
   context 'default values' do
     let(:gitlab_host) { 'gitlabhost' }
     before(:each) { allow(Gitlab.config.gitlab).to receive(:host) { gitlab_host } }
+    subject { described_class.new }
 
     it 'defines a default schema' do
       expect(subject.schema).to eq('http')
@@ -44,39 +52,65 @@ describe GeoNode, type: :model do
     end
   end
 
+  context 'dependent models for GeoNode' do
+    let(:geo_node_key_attributes) { FactoryGirl.build(:geo_node_key).attributes }
+
+    context 'on initialize' do
+      before(:each) do
+        new_node.geo_node_key_attributes = geo_node_key_attributes
+      end
+
+      it 'initializes a corresponding key' do
+        expect(new_node.geo_node_key).to be_present
+      end
+
+      it 'is valid' do
+        expect(new_node).to be_valid
+      end
+    end
+
+    context 'on create' do
+      it 'saves a corresponding key' do
+        expect(node.geo_node_key).to be_persisted
+      end
+
+      it 'saves a corresponding oauth application if it is a secondary node' do
+        expect(node.oauth_application).to be_persisted
+      end
+
+      it 'has no oauth_application if it is a primary node' do
+        expect(primary_node.oauth_application).not_to be_present
+      end
+    end
+  end
+
   describe '#uri' do
     context 'when all fields are filled' do
-      subject { GeoNode.new(schema: 'https', host: 'localhost', port: 3000, relative_url_root: 'gitlab') }
-
       it 'returns an URI object' do
-        expect(subject.uri).to be_a URI
+        expect(new_node.uri).to be_a URI
       end
 
       it 'includes schema home port and relative_url' do
         expected_uri = URI.parse(dummy_url)
-        expect(subject.uri).to eq(expected_uri)
+        expect(new_node.uri).to eq(expected_uri)
       end
     end
 
     context 'when required fields are not filled' do
-      subject { GeoNode.new(schema: nil, host: nil, port: nil, relative_url_root: nil) }
-
       it 'returns an URI object' do
-        expect(subject.uri).to be_a URI
+        expect(empty_node.uri).to be_a URI
       end
     end
   end
 
   describe '#url' do
-    subject { GeoNode.new(schema: 'https', host: 'localhost', port: 3000, relative_url_root: 'gitlab') }
-
     it 'returns a string' do
-      expect(subject.url).to be_a String
+      expect(new_node.url).to be_a String
     end
 
     it 'includes schema home port and relative_url' do
       expected_url = 'https://localhost:3000/gitlab'
-      expect(subject.url).to eq(expected_url)
+      expect(new_node.url).to eq(expected_url)
     end
   end
 
@@ -114,11 +148,37 @@ describe GeoNode, type: :model do
   end
 
   describe '#notify_url' do
-    subject { GeoNode.new(schema: 'https', host: 'localhost', port: 3000, relative_url_root: 'gitlab') }
     let(:refresh_url) { 'https://localhost:3000/gitlab/api/v3/geo/refresh_projects' }
 
     it 'returns api url based on node uri' do
-      expect(subject.notify_url).to eq(refresh_url)
+      expect(new_node.notify_url).to eq(refresh_url)
+    end
+  end
+
+
+  describe '#oauth_callback_url' do
+    let(:oauth_callback_url) { 'https://localhost:3000/gitlab/oauth/geo/callback' }
+
+    it 'returns oauth callback url based on node uri' do
+      expect(new_node.oauth_callback_url).to eq(oauth_callback_url)
+    end
+  end
+
+  describe '#missing_oauth_application?' do
+    context 'on a primary node' do
+      it 'returns false' do
+        expect(primary_node).not_to be_missing_oauth_application
+      end
+    end
+
+    it 'returns false when present' do
+      expect(node).not_to be_missing_oauth_application
+    end
+
+    it 'returns true when it is not present' do
+      node.oauth_application.destroy!
+      node.reload
+      expect(node).to be_missing_oauth_application
     end
   end
 end