From b7af085c8bd24dc71f055a3fb6d0e416e9cd46bb Mon Sep 17 00:00:00 2001
From: Bogdan Denkovych <bdenkovych@gitlab.com>
Date: Wed, 27 Apr 2022 16:27:08 +0300
Subject: [PATCH] Remove `omniauth-kerberos` gem

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/2908

Changelog: removed
EE: true
---
 Gemfile                                       |  2 +-
 Gemfile.lock                                  |  7 +---
 .../omniauth_callbacks_controller.rb          |  2 +-
 doc/integration/kerberos.md                   |  8 ++--
 ee/lib/ee/gitlab/auth.rb                      | 19 ----------
 ee/lib/gitlab/kerberos/authentication.rb      | 35 -----------------
 ee/spec/lib/gitlab/auth_spec.rb               | 28 --------------
 .../gitlab/kerberos/authentication_spec.rb    | 38 +------------------
 lib/gitlab/omniauth_initializer.rb            |  2 -
 9 files changed, 9 insertions(+), 132 deletions(-)
 delete mode 100644 ee/lib/ee/gitlab/auth.rb
 delete mode 100644 ee/spec/lib/gitlab/auth_spec.rb

diff --git a/Gemfile b/Gemfile
index 5c231f8178ece..1dd8fd325fd0b 100644
--- a/Gemfile
+++ b/Gemfile
@@ -46,7 +46,6 @@ gem 'omniauth-facebook', '~> 4.0.0'
 gem 'omniauth-github', '~> 1.4'
 gem 'omniauth-gitlab', '~> 1.0.2'
 gem 'omniauth-google-oauth2', '~> 0.6.0'
-gem 'omniauth-kerberos', '~> 0.3.0', group: :kerberos
 gem 'omniauth-oauth2-generic', '~> 0.2.2'
 gem 'omniauth-saml', '~> 1.10'
 gem 'omniauth-shibboleth', '~> 1.3.0'
@@ -61,6 +60,7 @@ gem 'jwt', '~> 2.1.0'
 
 # Kerberos authentication. EE-only
 gem 'gssapi', group: :kerberos
+gem 'timfel-krb5-auth', '~> 0.8', group: :kerberos
 
 # Spam and anti-bot protection
 gem 'recaptcha', '~> 4.11', require: 'recaptcha/rails'
diff --git a/Gemfile.lock b/Gemfile.lock
index 5f2e4f18ca1fb..3344feae54b8c 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -873,11 +873,6 @@ GEM
       jwt (>= 2.0)
       omniauth (>= 1.1.1)
       omniauth-oauth2 (>= 1.5)
-    omniauth-kerberos (0.3.0)
-      omniauth-multipassword
-      timfel-krb5-auth (~> 0.8)
-    omniauth-multipassword (0.4.2)
-      omniauth (~> 1.0)
     omniauth-oauth (1.1.0)
       oauth
       omniauth (~> 1.0)
@@ -1594,7 +1589,6 @@ DEPENDENCIES
   omniauth-github (~> 1.4)
   omniauth-gitlab (~> 1.0.2)
   omniauth-google-oauth2 (~> 0.6.0)
-  omniauth-kerberos (~> 0.3.0)
   omniauth-oauth2-generic (~> 0.2.2)
   omniauth-salesforce (~> 1.0.5)
   omniauth-saml (~> 1.10)
@@ -1690,6 +1684,7 @@ DEPENDENCIES
   thin (~> 1.8.0)
   thrift (>= 0.14.0)
   timecop (~> 0.9.1)
+  timfel-krb5-auth (~> 0.8)
   toml-rb (~> 2.0)
   truncato (~> 0.7.11)
   typhoeus (~> 1.4.0)
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index dc5b22e16067f..c1c4a110aae4e 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -9,7 +9,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
 
   after_action :verify_known_sign_in
 
-  protect_from_forgery except: [:kerberos, :saml, :cas3, :failure] + AuthHelper.saml_providers, with: :exception, prepend: true
+  protect_from_forgery except: [:saml, :cas3, :failure] + AuthHelper.saml_providers, with: :exception, prepend: true
 
   feature_category :authentication_and_authorization
 
diff --git a/doc/integration/kerberos.md b/doc/integration/kerberos.md
index 17a81419ad0a6..73674b66f05db 100644
--- a/doc/integration/kerberos.md
+++ b/doc/integration/kerberos.md
@@ -299,9 +299,11 @@ Kerberos ticket-based authentication.
 ## Upgrading from password-based to ticket-based Kerberos sign-ins
 
 In previous versions of GitLab users had to submit their
-Kerberos username and password to GitLab when signing in. We plan to
-remove support for password-based Kerberos sign-ins in a future
-release, so we recommend that you upgrade to ticket-based sign-ins.
+Kerberos username and password to GitLab when signing in.
+
+We [deprecated](../update/deprecations.md#omniauth-kerberos-gem) password-based
+Kerberos sign-ins in GitLab 14.3 and [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/2908)
+it in GitLab 15.0. You must switch to ticket-based sign in.
 
 Depending on your existing GitLab configuration, the 'Sign in with:
 Kerberos SPNEGO' button may already be visible on your GitLab sign-in
diff --git a/ee/lib/ee/gitlab/auth.rb b/ee/lib/ee/gitlab/auth.rb
deleted file mode 100644
index 9e8a79e9057e4..0000000000000
--- a/ee/lib/ee/gitlab/auth.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-# frozen_string_literal: true
-
-module EE
-  module Gitlab
-    module Auth
-      extend ::Gitlab::Utils::Override
-
-      override :find_with_user_password
-      def find_with_user_password(login, password, increment_failed_attempts: false)
-        if Devise.omniauth_providers.include?(:kerberos)
-          kerberos_user = ::Gitlab::Kerberos::Authentication.login(login, password)
-          return kerberos_user if kerberos_user
-        end
-
-        super
-      end
-    end
-  end
-end
diff --git a/ee/lib/gitlab/kerberos/authentication.rb b/ee/lib/gitlab/kerberos/authentication.rb
index 2c66b70a8c569..ae5554c7236d9 100644
--- a/ee/lib/gitlab/kerberos/authentication.rb
+++ b/ee/lib/gitlab/kerberos/authentication.rb
@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-# This calls helps to authenticate to Kerberos by providing username and password
 module Gitlab
   module Kerberos
     class Authentication
@@ -11,46 +10,12 @@ def self.kerberos_default_realm
         default_realm
       end
 
-      def self.login(login, password)
-        return unless Devise.omniauth_providers.include?(:kerberos)
-        return unless login.present? && password.present?
-
-        auth = new(login, password)
-        auth.login
-      end
-
       def self.krb5_class
         @krb5_class ||= begin
           require "krb5_auth"
           Krb5Auth::Krb5
         end
       end
-
-      def initialize(login, password)
-        @login = login
-        @password = password
-        @krb5 = self.class.krb5_class.new
-      end
-
-      def valid?
-        @krb5.get_init_creds_password(@login, @password)
-      rescue self.class.krb5_class::Exception
-        false
-      end
-
-      def login
-        # get_default_principal consistently returns the canonical Kerberos principal name, with realm
-        valid? && find_by_login(@krb5.get_default_principal)
-      end
-
-      private
-
-      # rubocop: disable CodeReuse/ActiveRecord
-      def find_by_login(login)
-        identity = ::Identity.with_extern_uid(:kerberos, login).take
-        identity && identity.user
-      end
-      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end
diff --git a/ee/spec/lib/gitlab/auth_spec.rb b/ee/spec/lib/gitlab/auth_spec.rb
deleted file mode 100644
index 427e3f02ac86d..0000000000000
--- a/ee/spec/lib/gitlab/auth_spec.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Auth do
-  let(:gl_auth) { described_class }
-  let!(:user) do
-    create(:user,
-           username: username,
-           password: password,
-           password_confirmation: password)
-  end
-
-  let(:username) { 'John' } # username isn't lowercase, test this
-  let(:password) { 'my-secret' }
-
-  context 'with kerberos' do
-    before do
-      allow(Devise).to receive_messages(omniauth_providers: [:kerberos])
-    end
-
-    it 'finds user' do
-      expect(::Gitlab::Kerberos::Authentication).to receive_messages(login: user)
-
-      expect( gl_auth.find_with_user_password(username, password) ).to eql user
-    end
-  end
-end
diff --git a/ee/spec/lib/gitlab/kerberos/authentication_spec.rb b/ee/spec/lib/gitlab/kerberos/authentication_spec.rb
index 5e5d6adee1337..37e8407c9e34c 100644
--- a/ee/spec/lib/gitlab/kerberos/authentication_spec.rb
+++ b/ee/spec/lib/gitlab/kerberos/authentication_spec.rb
@@ -3,12 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Kerberos::Authentication do
-  let(:user) { create(:omniauth_user, provider: :kerberos, extern_uid: 'gitlab@FOO.COM') }
-  let(:login) { 'john' }
-  let(:password) { 'password' }
-
   before do
-    described_class.krb5_class # eager load the krb5_auth gem
+    described_class.krb5_class # eager load Krb5Auth::Krb5
   end
 
   describe '.kerberos_default_realm' do
@@ -20,36 +16,4 @@
       expect(described_class.kerberos_default_realm).to eq("FOO.COM")
     end
   end
-
-  describe '.login' do
-    before do
-      allow(Devise).to receive_messages(omniauth_providers: [:kerberos])
-      user # make sure user is instanciated
-    end
-
-    it "finds the user if authentication is successful (login without kerberos realm)" do
-      allow_next_instance_of(::Krb5Auth::Krb5) do |instance|
-        allow(instance).to receive_messages(get_init_creds_password: true, get_default_principal: 'gitlab@FOO.COM')
-      end
-
-      expect(described_class.login('gitlab', password)).to be_truthy
-    end
-
-    it "finds the user if authentication is successful (login with a kerberos realm)" do
-      allow_next_instance_of(::Krb5Auth::Krb5) do |instance|
-        allow(instance).to receive_messages(get_init_creds_password: true, get_default_principal: 'gitlab@FOO.COM')
-      end
-
-      expect(described_class.login('gitlab@FOO.COM', password)).to be_truthy
-    end
-
-    it "returns false if there is no such user in kerberos" do
-      kerberos_login = "some-login"
-      allow_next_instance_of(::Krb5Auth::Krb5) do |instance|
-        allow(instance).to receive_messages(get_init_creds_password: true, get_default_principal: 'some-login@FOO.COM')
-      end
-
-      expect(described_class.login(kerberos_login, password)).to be_falsy
-    end
-  end
 end
diff --git a/lib/gitlab/omniauth_initializer.rb b/lib/gitlab/omniauth_initializer.rb
index 51277497c999c..b78cd2a6b9572 100644
--- a/lib/gitlab/omniauth_initializer.rb
+++ b/lib/gitlab/omniauth_initializer.rb
@@ -136,8 +136,6 @@ def build_omniauth_customized_providers
 
     def setup_provider(provider)
       case provider
-      when :kerberos
-        require 'omniauth-kerberos'
       when *omniauth_customized_providers
         require_dependency "omni_auth/strategies/#{provider}"
       end
-- 
GitLab