bigbluebutton / greenlight

A really simple end-user interface for your BigBlueButton server.
GNU Lesser General Public License v3.0
781 stars 3.81k forks source link

Greenlight 3.3.5 - Shibboleth integration auth enabled and tested - It works ! #5843

Closed aguerson closed 3 weeks ago

aguerson commented 4 weeks ago

Hi all, I did it ! I hope it could be help some people who don't want to use openid and keycloak. I have already a Shibboleth IDP. One more tool is one more tool to maintain... No way.

@farhatahmad ;)

My code.

Same way as https://github.com/bigbluebutton/greenlight/issues/1859. I assume you already have nginx, a SP shibboleth and a PostgreSQL database ;) ( No docker, all in OpenVZ container ;) )

I use my gem "omniauth-v2-shibboleth-provider" ( juste to be sure for my test, it more easy to install with "Gemfile" ), but you can use "omniauth-shibboleth-redux" https://github.com/omniauth/omniauth-shibboleth-redux ( it is the same )

diff --git a/Gemfile b/Gemfile
index 2e3ec8ba..64e7e6f2 100644
--- a/Gemfile
+++ b/Gemfile
@@ -27,6 +27,7 @@ gem 'mini_magick', '>= 4.9.5'
 gem 'omniauth', '~> 2.1.2'
 gem 'omniauth_openid_connect', '>= 0.6.1'
 gem 'omniauth-rails_csrf_protection', '~> 1.0.1'
+gem 'omniauth-v2-shibboleth-provider', '~> 2.0.0'
 gem 'pagy', '~> 6.0', '>= 6.0.0'
 gem 'pg'
 gem 'puma', '~> 5.6'
diff --git a/Gemfile.lock b/Gemfile.lock
index f5ea4a2a..dd425f4d 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -289,6 +289,8 @@ GEM
     omniauth-rails_csrf_protection (1.0.1)
       actionpack (>= 4.2)
       omniauth (~> 2.0)
+    omniauth-v2-shibboleth-provider (2.0.0)
+      omniauth (~> 2.0, >= 2.0.0)
     omniauth_openid_connect (0.7.1)
       omniauth (>= 1.9, < 3)
       openid_connect (~> 2.2)
diff --git a/app/controllers/api/v1/api_controller.rb b/app/controllers/api/v1/api_controller.rb
index 0ebbe53f..c970caa1 100644
--- a/app/controllers/api/v1/api_controller.rb
+++ b/app/controllers/api/v1/api_controller.rb
@@ -91,7 +91,7 @@ module Api

       # Checks if external authentication is enabled (currently only OIDC is implemented)
       def external_auth?
-        return ENV['OPENID_CONNECT_ISSUER'].present? if ENV['LOADBALANCER_ENDPOINT'].blank?
+        return ENV['SHIBBOLETH'].present? if ENV['LOADBALANCER_ENDPOINT'].blank?

         !Tenant.exists?(name: current_provider, client_secret: 'local')
       end
diff --git a/app/controllers/api/v1/migrations/external_controller.rb b/app/controllers/api/v1/migrations/external_controller.rb
index 39ab2970..e4e84290 100644
--- a/app/controllers/api/v1/migrations/external_controller.rb
+++ b/app/controllers/api/v1/migrations/external_controller.rb
@@ -82,7 +82,7 @@ module Api
           user_hash = user_params.to_h

           # Re-write LDAP and Google to greenlight
-          user_hash[:provider] = %w[greenlight ldap google openid_connect].include?(user_hash[:provider]) ? 'greenlight' : user_hash[:provider]
+          user_hash[:provider] = %w[greenlight ldap google shibboleth].include?(user_hash[:provider]) ? 'greenlight' : user_hash[:provider]

           # Returns an error if the provider does not exist
           unless user_hash[:provider] == 'greenlight' || Tenant.exists?(name: user_hash[:provider])
@@ -119,7 +119,7 @@ module Api
           room_hash = room_params.to_h

           # Re-write LDAP and Google to greenlight
-          room_hash[:provider] = %w[greenlight ldap google openid_connect].include?(room_hash[:provider]) ? 'greenlight' : room_hash[:provider]
+          room_hash[:provider] = %w[greenlight ldap google shibboleth].include?(room_hash[:provider]) ? 'greenlight' : room_hash[:provider]

           unless room_hash[:provider] == 'greenlight' || Tenant.exists?(name: room_hash[:provider])
             return render_error(status: :bad_request, errors: 'Provider does not exist')
diff --git a/app/javascript/components/home/HomePage.jsx b/app/javascript/components/home/HomePage.jsx
index f4f520b8..45e8caf0 100644
--- a/app/javascript/components/home/HomePage.jsx
+++ b/app/javascript/components/home/HomePage.jsx
@@ -75,7 +75,7 @@ export default function HomePage() {
       }

       if (inviteToken && env?.EXTERNAL_AUTH) {
-        const signInForm = document.querySelector('form[action="/auth/openid_connect"]');
+        const signInForm = document.querySelector('form[action="/auth/shibboleth"]');
         signInForm.submit();
       } else if (inviteToken && !env?.EXTERNAL_AUTH) {
         const buttons = document.querySelectorAll('.btn');
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb
index 7d0a0cb7..df678283 100644
--- a/config/initializers/omniauth.rb
+++ b/config/initializers/omniauth.rb
@@ -16,42 +16,25 @@

 # frozen_string_literal: true

-Rails.application.config.middleware.use OmniAuth::Builder do
-  issuer = ENV.fetch('OPENID_CONNECT_ISSUER', '')
-  lb = ENV.fetch('LOADBALANCER_ENDPOINT', '')
+#OmniAuth.config.logger = Rails.logger
+
+#Rails.application.config.omniauth_shibboleth = ENV['SHIBBOLETH'].present?

-  if lb.present?
-    provider :openid_connect, setup: lambda { |env|
-      request = Rack::Request.new(env)
-      current_provider = request.params['current_provider'] || request.host&.split('.')&.first
-      secret = Tenant.find_by(name: current_provider)&.client_secret
-      issuer_url = File.join issuer.to_s, "/#{current_provider}"
+Rails.application.config.middleware.use OmniAuth::Builder do
+  issuer = ENV.fetch('SHIBBOLETH', '')

-      env['omniauth.strategy'].options[:issuer] = issuer_url
-      env['omniauth.strategy'].options[:scope] = %i[openid email profile]
-      env['omniauth.strategy'].options[:uid_field] = ENV.fetch('OPENID_CONNECT_UID_FIELD', 'sub')
-      env['omniauth.strategy'].options[:discovery] = true
-      env['omniauth.strategy'].options[:client_options].identifier = ENV.fetch('OPENID_CONNECT_CLIENT_ID')
-      env['omniauth.strategy'].options[:client_options].secret = secret
-      env['omniauth.strategy'].options[:client_options].redirect_uri = File.join(
-        File.join('https://', "#{current_provider}.#{ENV.fetch('OPENID_CONNECT_REDIRECT', '')}", 'auth', 'openid_connect', 'callback')
-      )
-      env['omniauth.strategy'].options[:client_options].authorization_endpoint = File.join(issuer_url, 'protocol', 'openid-connect', 'auth')
-      env['omniauth.strategy'].options[:client_options].token_endpoint = File.join(issuer_url, 'protocol', 'openid-connect', 'token')
-      env['omniauth.strategy'].options[:client_options].userinfo_endpoint = File.join(issuer_url, 'protocol', 'openid-connect', 'userinfo')
-      env['omniauth.strategy'].options[:client_options].jwks_uri = File.join(issuer_url, 'protocol', 'openid-connect', 'certs')
-      env['omniauth.strategy'].options[:client_options].end_session_endpoint = File.join(issuer_url, 'protocol', 'openid-connect', 'logout')
+  if issuer.present?
+    provider :shibboleth, {
+      :debug => true,
+      :request_type => :header,
+      :uid_field  => "eppn",
+      :name_field => "displayName",
+      :shib_session_id_field     => "Shib-Session-ID",
+      :shib_application_id_field => "Shib-Application-ID",
+      :info_fields => {
+        :email => "mail",
+      },
     }
-  elsif issuer.present?
-    provider :openid_connect,
-             issuer:,
-             scope: %i[openid email profile],
-             uid_field: ENV.fetch('OPENID_CONNECT_UID_FIELD', 'sub'),
-             discovery: true,
-             client_options: {
-               identifier: ENV.fetch('OPENID_CONNECT_CLIENT_ID'),
-               secret: ENV.fetch('OPENID_CONNECT_CLIENT_SECRET'),
-               redirect_uri: File.join(ENV.fetch('OPENID_CONNECT_REDIRECT', ''), 'auth', 'openid_connect', 'callback')
-             }
   end
 end
+
diff --git a/esbuild.dev.mjs b/esbuild.dev.mjs
index ded76ccc..55c28c2e 100644
--- a/esbuild.dev.mjs
+++ b/esbuild.dev.mjs
@@ -14,7 +14,7 @@ esbuild.context({
   },
   define: {
     'process.env.RELATIVE_URL_ROOT': `"${relativeUrlRoot}"`,
-    'process.env.OMNIAUTH_PATH': `"${relativeUrlRoot}/auth/openid_connect"`, // currently, only OIDC is implemented
+    'process.env.OMNIAUTH_PATH': `"${relativeUrlRoot}/auth/shibboleth"`, // currently, only OIDC is implemented
   },
 }).then(context => {
   if (process.argv.includes("--watch")) {
diff --git a/esbuild.mjs b/esbuild.mjs
index e9aa8a45..0707de1a 100644
--- a/esbuild.mjs
+++ b/esbuild.mjs
@@ -14,7 +14,7 @@ await esbuild.build({
   },
   define: {
     'process.env.RELATIVE_URL_ROOT': `"${relativeUrlRoot}"`,
-    'process.env.OMNIAUTH_PATH': `"${relativeUrlRoot}/auth/openid_connect"`, // currently, only OIDC is implemented
+    'process.env.OMNIAUTH_PATH': `"${relativeUrlRoot}/auth/shibboleth"`, // currently, only OIDC is implemented
   },
 });

diff --git a/spec/controllers/external_controller_spec.rb b/spec/controllers/external_controller_spec.rb
index 607dac43..95deac5c 100644
--- a/spec/controllers/external_controller_spec.rb
+++ b/spec/controllers/external_controller_spec.rb
@@ -25,7 +25,7 @@ RSpec.describe ExternalController, type: :controller do
     before do
       OmniAuth.config.test_mode = true

-      OmniAuth.config.mock_auth[:openid_connect] = OmniAuth::AuthHash.new(
+      OmniAuth.config.mock_auth[:shibboleth] = OmniAuth::AuthHash.new(
         uid: Faker::Internet.uuid,
         info: {
           email: Faker::Internet.email,
@@ -41,164 +41,164 @@ RSpec.describe ExternalController, type: :controller do
     let!(:role) { create(:role, name: 'User') }

     it 'creates the user if the info returned is valid' do
-      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

       expect do
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }
       end.to change(User, :count).by(1)
     end

     it 'logs the user in and redirects to their rooms page' do
-      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

-      get :create_user, params: { provider: 'openid_connect' }
+      get :create_user, params: { provider: 'shibboleth' }

-      expect(session[:session_token]).to eq(User.find_by(email: OmniAuth.config.mock_auth[:openid_connect][:info][:email]).session_token)
+      expect(session[:session_token]).to eq(User.find_by(email: OmniAuth.config.mock_auth[:shibboleth][:info][:email]).session_token)
       expect(response).to redirect_to(root_path)
     end

     it 'assigns the User role to the user' do
-      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

-      get :create_user, params: { provider: 'openid_connect' }
+      get :create_user, params: { provider: 'shibboleth' }

-      expect(User.find_by(email: OmniAuth.config.mock_auth[:openid_connect][:info][:email]).role).to eq(role)
+      expect(User.find_by(email: OmniAuth.config.mock_auth[:shibboleth][:info][:email]).role).to eq(role)
     end

     it 'marks the user as verified' do
-      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

-      get :create_user, params: { provider: 'openid_connect' }
+      get :create_user, params: { provider: 'shibboleth' }

-      expect(User.find_by(email: OmniAuth.config.mock_auth[:openid_connect][:info][:email]).verified?).to be true
+      expect(User.find_by(email: OmniAuth.config.mock_auth[:shibboleth][:info][:email]).verified?).to be true
     end

     it 'looks the user up based on external id' do
-      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

       create(:user, external_id: request.env['omniauth.auth']['uid'])

       expect do
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }
       end.to change(User, :count).by(0)
     end

     it 'looks the user up based on email' do
-      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+      request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

       create(:user, email: request.env['omniauth.auth']['info']['email'])

       expect do
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }
       end.to change(User, :count).by(0)
     end

     context 'redirect' do
       it 'redirects to the location cookie if a relative redirection 1' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         cookies[:location] = {
           value: '/rooms/o5g-hvb-s44-p5t/join',
           path: '/'
         }
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(response).to redirect_to('/rooms/o5g-hvb-s44-p5t/join')
       end

       it 'redirects to the location cookie if its a legacy url (3 sections in uid)' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         cookies[:location] = {
           value: '/rooms/o5g-hvb-s44/join',
           path: '/'
         }
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(response).to redirect_to('/rooms/o5g-hvb-s44/join')
       end

       it 'redirects to the location cookie if a relative redirection 2' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         cookies[:location] = {
           value: '/a/b/c/d/rooms/o5g-hvb-s44-p5t/join',
           path: '/'
         }
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(response).to redirect_to('/a/b/c/d/rooms/o5g-hvb-s44-p5t/join')
       end

       it 'doesnt redirect if NOT a relative redirection check 1' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         cookies[:location] = {
           value: Faker::Internet.url,
           path: '/'
         }
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(response).to redirect_to(root_path)
       end

       it 'doesnt redirect if it NOT a relative redirection format check 2' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         cookies[:location] = {
           value: 'https://www.google.com?ignore=/rooms/iam-abl-eto-pas/join',
           path: '/'
         }
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(response).to redirect_to(root_path)
       end

       it 'doesnt redirect if it NOT a relative redirection format check 3' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         cookies[:location] = {
           value: "http://example.com/?ignore=\n/rooms/abc-def-ghi-jkl/join",
           path: '/'
         }
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(response).to redirect_to(root_path)
       end

       it 'doesnt redirect if NOT a relative redirection check 4' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         cookies[:location] = {
           value: Faker::Internet.url(path: '/rooms/o5g-hvb-s44-p5t/join'),
           path: '/'
         }
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(response).to redirect_to(root_path)
       end

       it 'doesnt redirect if NOT a valid room join link check 5' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         cookies[:location] = {
           value: '/romios/o5g-hvb-s44-p5t/join',
           path: '/'
         }
-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(response).to redirect_to(root_path)
       end

       it 'deletes the cookie after reading' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         cookies[:location] = {
           value: '/rooms/o5g-hvb-s44-p5t/join',
           path: '/'
         }

-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(cookies[:location]).to be_nil
       end
@@ -207,7 +207,7 @@ RSpec.describe ExternalController, type: :controller do
     context 'ResyncOnLogin' do
       let!(:user) do
         create(:user,
-               external_id: OmniAuth.config.mock_auth[:openid_connect]['uid'],
+               external_id: OmniAuth.config.mock_auth[:shibboleth]['uid'],
                name: 'Example Name',
                email: 'email@example.com')
       end
@@ -215,21 +215,21 @@ RSpec.describe ExternalController, type: :controller do
       it 'overwrites the saved values with the values from the authentication provider if true' do
         allow_any_instance_of(SettingGetter).to receive(:call).and_return(true)

-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         user.reload
-        expect(user.name).to eq(OmniAuth.config.mock_auth[:openid_connect]['info']['name'])
-        expect(user.email).to eq(OmniAuth.config.mock_auth[:openid_connect]['info']['email'])
+        expect(user.name).to eq(OmniAuth.config.mock_auth[:shibboleth]['info']['name'])
+        expect(user.email).to eq(OmniAuth.config.mock_auth[:shibboleth]['info']['email'])
       end

       it 'does not overwrite the saved values with the values from the authentication provider if false' do
         allow_any_instance_of(SettingGetter).to receive(:call).and_return(false)

-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         user.reload
         expect(user.name).to eq('Example Name')
@@ -238,12 +238,12 @@ RSpec.describe ExternalController, type: :controller do

       it 'does not overwrite the role even if true' do
         allow_any_instance_of(SettingGetter).to receive(:call).and_return(true)
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

         new_role = create(:role)
         user.update(role: new_role)

-        get :create_user, params: { provider: 'openid_connect' }
+        get :create_user, params: { provider: 'shibboleth' }

         expect(user.reload.role).to eq(new_role)
       end
@@ -258,50 +258,50 @@ RSpec.describe ExternalController, type: :controller do
         end

         it 'creates a user account if they have a valid invitation' do
-          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
-          invite = create(:invitation, email: OmniAuth.config.mock_auth[:openid_connect][:info][:email])
+          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]
+          invite = create(:invitation, email: OmniAuth.config.mock_auth[:shibboleth][:info][:email])
           cookies[:inviteToken] = {
             value: invite.token
           }

-          expect { get :create_user, params: { provider: 'openid_connect' } }.to change(User, :count).by(1)
+          expect { get :create_user, params: { provider: 'shibboleth' } }.to change(User, :count).by(1)
         end

         it 'deletes an invitation after using it' do
-          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
-          invite = create(:invitation, email: OmniAuth.config.mock_auth[:openid_connect][:info][:email])
+          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]
+          invite = create(:invitation, email: OmniAuth.config.mock_auth[:shibboleth][:info][:email])
           cookies[:inviteToken] = {
             value: invite.token
           }

-          expect { get :create_user, params: { provider: 'openid_connect' } }.to change(Invitation, :count).by(-1)
+          expect { get :create_user, params: { provider: 'shibboleth' } }.to change(Invitation, :count).by(-1)
         end

         it 'allows a user with an existing account to sign in without a token' do
-          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

-          create(:user, external_id: OmniAuth.config.mock_auth[:openid_connect][:uid])
+          create(:user, external_id: OmniAuth.config.mock_auth[:shibboleth][:uid])

-          expect { get :create_user, params: { provider: 'openid_connect' } }.not_to raise_error
+          expect { get :create_user, params: { provider: 'shibboleth' } }.not_to raise_error
           expect(response).to redirect_to(root_path)
         end

         it 'returns an InviteInvalid error if no invite is passed' do
-          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

-          get :create_user, params: { provider: 'openid_connect' }
+          get :create_user, params: { provider: 'shibboleth' }

           expect(response).to redirect_to(root_path(error: Rails.configuration.custom_error_msgs[:invite_token_invalid]))
         end

         it 'returns an InviteInvalid error if the token is wrong' do
-          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
-          create(:invitation, email: OmniAuth.config.mock_auth[:openid_connect][:info][:email])
+          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]
+          create(:invitation, email: OmniAuth.config.mock_auth[:shibboleth][:info][:email])
           cookies[:inviteToken] = {
             value: '123'
           }

-          get :create_user, params: { provider: 'openid_connect' }
+          get :create_user, params: { provider: 'shibboleth' }

           expect(response).to redirect_to(root_path(error: Rails.configuration.custom_error_msgs[:invite_token_invalid]))
         end
@@ -315,11 +315,11 @@ RSpec.describe ExternalController, type: :controller do
         end

         it 'sets a user to pending when registering' do
-          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+          request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

-          expect { get :create_user, params: { provider: 'openid_connect' } }.to change(User, :count).by(1)
+          expect { get :create_user, params: { provider: 'shibboleth' } }.to change(User, :count).by(1)

-          expect(User.find_by(email: OmniAuth.config.mock_auth[:openid_connect][:info][:email])).to be_pending
+          expect(User.find_by(email: OmniAuth.config.mock_auth[:shibboleth][:info][:email])).to be_pending
           expect(response).to redirect_to(controller.pending_path)
         end
       end
@@ -332,15 +332,15 @@ RSpec.describe ExternalController, type: :controller do
         role_map = instance_double(SettingGetter)
         allow(SettingGetter).to receive(:new).with(setting_name: 'RoleMapping', provider: 'greenlight').and_return(role_map)
         allow(role_map).to receive(:call).and_return(
-          "role1=#{OmniAuth.config.mock_auth[:openid_connect][:info][:email].split('@')[1]}"
+          "role1=#{OmniAuth.config.mock_auth[:shibboleth][:info][:email].split('@')[1]}"
         )
       end

       it 'Creates a User and assign a role if a rule matches their email' do
-        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
+        request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:shibboleth]

-        expect { get :create_user, params: { provider: 'openid_connect' } }.to change(User, :count).by(1)
-        expect(User.find_by(email: OmniAuth.config.mock_auth[:openid_connect][:info][:email]).role).to eq(role1)
+        expect { get :create_user, params: { provider: 'shibboleth' } }.to change(User, :count).by(1)
+        expect(User.find_by(email: OmniAuth.config.mock_auth[:shibboleth][:info][:email]).role).to eq(role1)
       end
     end
   end
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 81a9d594..dc6f2651 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -487,9 +487,9 @@ RSpec.describe Api::V1::UsersController, type: :controller do
         allow(controller).to receive(:external_auth?).and_call_original
       end

-      context 'OPENID_CONNECT_ISSUER is present?' do
+      context 'SHIBBOLETH is present?' do
         before do
-          ENV['OPENID_CONNECT_ISSUER'] = 'issuer'
+          ENV['SHIBBOLETH'] = 'issuer'
         end

         it 'returns true' do
@@ -497,9 +497,9 @@ RSpec.describe Api::V1::UsersController, type: :controller do
         end
       end

-      context 'OPENID_CONNECT_ISSUER is NOT present?' do
+      context 'SHIBBOLETH is NOT present?' do
         before do
-          ENV['OPENID_CONNECT_ISSUER'] = ''
+          ENV['SHIBBOLETH'] = ''
         end

         it 'returns false' do
aguerson commented 4 weeks ago

I forgot to precise : It is on Ubuntu 22.04 LTS ;)

Ithanil commented 3 weeks ago

Good job! I'd suggest creating a PR though, just like https://github.com/bigbluebutton/greenlight/pull/5483 or https://github.com/bigbluebutton/greenlight/pull/5476, so the changes are more clear and it's easier to see&fix when conflicts with newer code arise. Also, it would increase the long-term visibility.

farhatahmad commented 3 weeks ago

^ Agreed