From 6116ab546db09eb6c338566cc95ae4b91c324dcb Mon Sep 17 00:00:00 2001 From: Calvin Walton Date: Thu, 16 May 2024 09:35:53 -0400 Subject: [PATCH 1/7] Merge pull request from GHSA-p3q9-qff5-97p7 * Implement checksums for POST requests Previously, the checksum logic always looked at the query string, even on POST requests where the parameters are in the body. Add checksum support for POST parameters (this is somewhat complicated by the fact that scalelite's internal API uses Rails nested parameters), and match BigBlueButton's behaviour of rejecting a request if parameters are present in both the query string and POST request body. * Validate BigBlueButton API request content-type * Check 'GET' checksum on 'POST' request with json content type The checksum helper is also used on the Scalelite administration API, which is designed to be used with POST requests with json request body. The checker designed for the BigBlueButton APIs rejected this as an unsupported content type. The expected behaviour of these requests is to have the checksum in the query string, and the body isn't covered by the checksum. Adapt the code to support this behaviour. * Use right content type in Admin API tests, reject form data * Remove checksum validation workaround for admin api * Update supported request methods to match BigBlueButton * Remove incorrect POST req checksumming * Update tests for supported methods and content types * Remove unused GET_CHECKSUM_MIME_TYPES * Ensure create call does not pass parameters in POST body as-is to BBB * Fix formatting issues reported by rubocop * Correct list of endpoints supporting GET for content type check --- .../api/scalelite_api_controller.rb | 18 +++ app/controllers/api/servers_controller.rb | 7 +- app/controllers/api/tenants_controller.rb | 7 +- .../bigbluebutton_api_controller.rb | 13 +- app/controllers/concerns/api_helper.rb | 66 ++++++---- app/controllers/concerns/bbb_errors.rb | 6 + config/routes.rb | 19 +-- spec/controllers/concerns/api_helper_spec.rb | 27 ++-- .../bigbluebutton_api_controller_spec.rb | 115 +++++++++++------- spec/requests/servers_controller_spec.rb | 56 ++++++--- spec/requests/tenants_controller_spec.rb | 46 ++++--- .../bigbluebutton_api_controller_test.rb | 68 +++++++---- 12 files changed, 286 insertions(+), 162 deletions(-) create mode 100644 app/controllers/api/scalelite_api_controller.rb diff --git a/app/controllers/api/scalelite_api_controller.rb b/app/controllers/api/scalelite_api_controller.rb new file mode 100644 index 00000000..648518a6 --- /dev/null +++ b/app/controllers/api/scalelite_api_controller.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Api + class ScaleliteApiController < ApplicationController + include ApiHelper + + skip_before_action :verify_authenticity_token + + before_action :verify_content_type + before_action -> { verify_checksum(true) } + + def verify_content_type + return unless request.post? && request.content_length.positive? + + raise UnsupportedContentType unless request.content_mime_type == Mime[:json] + end + end +end diff --git a/app/controllers/api/servers_controller.rb b/app/controllers/api/servers_controller.rb index 6cace03d..ac2614a3 100644 --- a/app/controllers/api/servers_controller.rb +++ b/app/controllers/api/servers_controller.rb @@ -1,12 +1,7 @@ # frozen_string_literal: true module Api - class ServersController < ApplicationController - include ApiHelper - - skip_before_action :verify_authenticity_token - - before_action -> { verify_checksum(true) } + class ServersController < ScaleliteApiController before_action :set_server, only: [:get_server_info, :update_server, :delete_server, :panic_server] # Return a list of the configured BigBlueButton servers diff --git a/app/controllers/api/tenants_controller.rb b/app/controllers/api/tenants_controller.rb index 567fe77b..c63d7265 100644 --- a/app/controllers/api/tenants_controller.rb +++ b/app/controllers/api/tenants_controller.rb @@ -1,12 +1,7 @@ # frozen_string_literal: true module Api - class TenantsController < ApplicationController - include ApiHelper - - skip_before_action :verify_authenticity_token - - before_action -> { verify_checksum(true) } + class TenantsController < ScaleliteApiController before_action :check_multitenancy before_action :set_tenant, only: [:get_tenant_info, :update_tenant, :delete_tenant] diff --git a/app/controllers/bigbluebutton_api_controller.rb b/app/controllers/bigbluebutton_api_controller.rb index 93c22837..371dbf60 100644 --- a/app/controllers/bigbluebutton_api_controller.rb +++ b/app/controllers/bigbluebutton_api_controller.rb @@ -5,6 +5,13 @@ class BigBlueButtonApiController < ApplicationController skip_before_action :verify_authenticity_token + # Check content types on endpoints that accept POST requests. For most endpoints, form data is permitted. + before_action :verify_content_type, except: [:create, :insert_document, :join, :publish_recordings, :delete_recordings] + # create allows either form data or XML + before_action :verify_create_content_type, only: [:create] + # insertDocument only allows XML + before_action :verify_insert_document_content_type, only: [:insert_document] + before_action :verify_checksum, except: [:index, :get_recordings_disabled, :recordings_disabled, :get_meetings_disabled, :analytics_callback,] @@ -198,6 +205,8 @@ def create params[:voiceBridge] = meeting.voice_bridge + have_preuploaded_slide = request.post? && request.content_mime_type == Mime[:xml] + logger.debug("Creating meeting #{params[:meetingID]} on BigBlueButton server #{server.id}") params_hash = params @@ -209,8 +218,8 @@ def create uri = encode_bbb_uri('create', server.url, server.secret, pass_through_params(excluded_params)) begin - # Read the body if POST - body = request.post? ? request.body.read : '' + # Read the body if preuploaded slide XML is present + body = have_preuploaded_slide ? request.raw_post : '' # Send a GET/POST request to the server response = get_post_req(uri, body, **bbb_req_timeout(server)) diff --git a/app/controllers/concerns/api_helper.rb b/app/controllers/concerns/api_helper.rb index 10f24314..2db996f0 100644 --- a/app/controllers/concerns/api_helper.rb +++ b/app/controllers/concerns/api_helper.rb @@ -19,38 +19,39 @@ module ApiHelper # which should be accessed only by superadmins. def verify_checksum(force_loadbalancer_secret = false) secrets = fetch_secrets(force_loadbalancer_secret: force_loadbalancer_secret) - - raise ChecksumError if params[:checksum].blank? raise ChecksumError if secrets.empty? - algorithm = case params[:checksum].length - when CHECKSUM_LENGTH_SHA1 - 'SHA1' - when CHECKSUM_LENGTH_SHA256 - 'SHA256' - when CHECKSUM_LENGTH_SHA512 - 'SHA512' - else - raise ChecksumError - end - - # Camel case (ex) get_meetings to getMeetings to match BBB server - check_string = action_name.camelcase(:lower) - check_string += request.query_string.gsub( - /&checksum=#{params[:checksum]}|checksum=#{params[:checksum]}&|checksum=#{params[:checksum]}/, '' - ) + checksum = request.params[:checksum] + raise ChecksumError if checksum.blank? + algorithm = guess_checksum_digest_algorithm(checksum) allowed_checksum_algorithms = Rails.configuration.x.loadbalancer_checksum_algorithms - raise ChecksumError unless allowed_checksum_algorithms.include? algorithm + raise ChecksumError unless allowed_checksum_algorithms.include?(algorithm) - secrets.each do |secret| - return true if ActiveSupport::SecurityUtils.secure_compare(get_checksum(check_string + secret, algorithm), - params[:checksum]) + check_string = action_name.camelcase(:lower) + query_string_remove_checksum(checksum) + return true if secrets.any? do |secret| + ActiveSupport::SecurityUtils.secure_compare(get_checksum(check_string + secret, algorithm), checksum) end raise ChecksumError end + # Remove the checksum from the request query string. This is done as string manipulation, rather than decoding then re-encoding the parameters, + # since there's multiple possible valid encodings for query parameters, and the one used by Ruby might not match. + def query_string_remove_checksum(checksum) + checksum = Regexp.escape(checksum) + request.query_string.gsub(/&checksum=#{checksum}|checksum=#{checksum}&|checksum=#{checksum}/, '') + end + + def guess_checksum_digest_algorithm(checksum) + case checksum.length + when CHECKSUM_LENGTH_SHA1 then 'SHA1' + when CHECKSUM_LENGTH_SHA256 then 'SHA256' + when CHECKSUM_LENGTH_SHA512 then 'SHA512' + else raise ChecksumError + end + end + def fetch_secrets(tenant_name: nil, force_loadbalancer_secret: false) return Rails.configuration.x.loadbalancer_secrets if force_loadbalancer_secret || !Rails.configuration.x.multitenancy_enabled @@ -98,6 +99,27 @@ def checksum_algorithm end end + # Verify that the Content-Type of POST requests is a "form data" type (applies to most APIs) + def verify_content_type + return unless request.post? && request.content_length.positive? + raise UnsupportedContentType unless request.form_data? + end + + # Verify that the Content-Type of a POST request is a format permitted by the create API. + # This can either be form data containing params, or an XML document for pre-uploaded slides + CREATE_PERMITTED_CONTENT_TYPES = Set.new([Mime[:url_encoded_form], Mime[:multipart_form], Mime[:xml]]).freeze + def verify_create_content_type + return unless request.post? && request.content_length.positive? + raise UnsupportedContentType unless CREATE_PERMITTED_CONTENT_TYPES.include?(request.content_mime_type) + end + + # Verify that the Content-Type of a POST request is a format permitted by the insertDocument API. + # Only an XML document for pre-uploaded slides (same format as create) is permitted. + def verify_insert_document_content_type + return unless request.post? && request.content_length.positive? + raise UnsupportedContentType unless request.content_mime_type == Mime[:xml] + end + # Encode URI and append checksum def encode_bbb_uri(action, base_uri, secret, bbb_params = {}) # Add slash at the end if its not there diff --git a/app/controllers/concerns/bbb_errors.rb b/app/controllers/concerns/bbb_errors.rb index 6a7676cc..04fbeb5e 100644 --- a/app/controllers/concerns/bbb_errors.rb +++ b/app/controllers/concerns/bbb_errors.rb @@ -40,6 +40,12 @@ def initialize end end + class UnsupportedContentType < BBBErrors::BBBError + def initialize + super('unsupportedContentType', 'POST request Content-Type is missing or unsupported') + end + end + class InternalError < BBBError def initialize(error) super('internalError', error) diff --git a/config/routes.rb b/config/routes.rb index 05733813..f4cb4df5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,7 +3,10 @@ Rails.application.routes.draw do # For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html - scope 'bigbluebutton/api', as: 'bigbluebutton_api', format: false, defaults: { format: 'xml' } do + scope 'bigbluebutton/api', as: :bigbluebutton_api, format: false, defaults: { format: 'xml' } do + # See https://github.com/bigbluebutton/bigbluebutton/blob/main/bigbluebutton-web/grails-app/controllers/org/bigbluebutton/web/UrlMappings.groovy + # for the definitions of the routes in BigBlueButton itself. Note that both private (BBB internal) and public APIs are in that file. + match '/', to: 'bigbluebutton_api#index', via: [:get, :post] match 'isMeetingRunning', to: 'bigbluebutton_api#is_meeting_running', as: :is_meeting_running, via: [:get, :post] match 'getMeetingInfo', to: 'bigbluebutton_api#get_meeting_info', as: :get_meeting_info, via: [:get, :post] @@ -14,19 +17,19 @@ end match 'create', to: 'bigbluebutton_api#create', via: [:get, :post] match 'end', to: 'bigbluebutton_api#end', via: [:get, :post] - match 'join', to: 'bigbluebutton_api#join', via: [:get, :post] - post 'analytics_callback', to: 'bigbluebutton_api#analytics_callback', as: :analytics_callback - post 'insertDocument', to: 'bigbluebutton_api#insert_document' + get 'join', to: 'bigbluebutton_api#join' + post 'analytics_callback', to: 'bigbluebutton_api#analytics_callback' + post 'insertDocument', to: 'bigbluebutton_api#insert_document', as: :insert_document if Rails.configuration.x.recording_disabled match('getRecordings', to: 'bigbluebutton_api#get_recordings_disabled', as: :get_recordings, via: [:get, :post]) - match('publishRecordings', to: 'bigbluebutton_api#recordings_disabled', as: :publish_recordings, via: [:get, :post]) + get('publishRecordings', to: 'bigbluebutton_api#recordings_disabled', as: :publish_recordings) match('updateRecordings', to: 'bigbluebutton_api#recordings_disabled', as: :update_recordings, via: [:get, :post]) - match('deleteRecordings', to: 'bigbluebutton_api#recordings_disabled', as: :delete_recordings, via: [:get, :post]) + get('deleteRecordings', to: 'bigbluebutton_api#recordings_disabled', as: :delete_recordings) else match('getRecordings', to: 'bigbluebutton_api#get_recordings', as: :get_recordings, via: [:get, :post]) - match('publishRecordings', to: 'bigbluebutton_api#publish_recordings', as: :publish_recordings, via: [:get, :post]) + get('publishRecordings', to: 'bigbluebutton_api#publish_recordings', as: :publish_recordings) match('updateRecordings', to: 'bigbluebutton_api#update_recordings', as: :update_recordings, via: [:get, :post]) - match('deleteRecordings', to: 'bigbluebutton_api#delete_recordings', as: :delete_recordings, via: [:get, :post]) + get('deleteRecordings', to: 'bigbluebutton_api#delete_recordings', as: :delete_recordings) end end diff --git a/spec/controllers/concerns/api_helper_spec.rb b/spec/controllers/concerns/api_helper_spec.rb index 18958fa1..03c145b6 100644 --- a/spec/controllers/concerns/api_helper_spec.rb +++ b/spec/controllers/concerns/api_helper_spec.rb @@ -55,6 +55,7 @@ describe '.verify_checksum' do let(:query_string) { 'querystring' } let(:action_name) { 'index' } + let(:method) { :get } let(:check_string) { action_name + query_string } let(:checksum_algo) { nil } # To be defined down the scope let(:secret) { 'IAmSecret' } @@ -63,10 +64,19 @@ before do controller.action_name = action_name allow(request).to receive(:query_string).and_return(query_string) + allow(request).to receive(:request_method_symbol).and_return(method) Rails.configuration.x.loadbalancer_secrets = [secret] + if hash.present? + case method + when :get then request.query_parameters[:checksum] = hash + when :post then request.request_parameters[:checksum] = hash + end + end end context 'without params[:checksum]' do + let(:hash) { nil } + it 'throws an error' do expect { verify_checksum @@ -78,39 +88,24 @@ context 'with SHA1' do let(:checksum_algo) { 'SHA1' } - before do - params[:checksum] = hash - end - include_examples 'proper verify_checksum behavior' end context 'with SHA256' do let(:checksum_algo) { 'SHA256' } - before do - params[:checksum] = hash - end - include_examples 'proper verify_checksum behavior' end context 'with SHA512' do let(:checksum_algo) { 'SHA512' } - before do - params[:checksum] = hash - end - include_examples 'proper verify_checksum behavior' end context 'with incorrect checksum' do let(:checksum_algo) { 'MD5' } - - before do - params[:checksum] = 'totallyNotAHash' - end + let(:hash) { 'totallyNotAHash' } it 'throws an error' do expect { diff --git a/spec/requests/bigbluebutton_api_controller_spec.rb b/spec/requests/bigbluebutton_api_controller_spec.rb index 6f2ae5fa..a0da8be2 100644 --- a/spec/requests/bigbluebutton_api_controller_spec.rb +++ b/spec/requests/bigbluebutton_api_controller_spec.rb @@ -312,7 +312,9 @@ allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['test-2']) - post bigbluebutton_api_get_meeting_info_url, params: { meetingID: meeting.id, checksum: "cbf00ea96fae6ff06c2cb311bbde8b26ad66d765" } + check_params = { meetingID: meeting.id } + check_params[:checksum] = Digest::SHA1.hexdigest("getMeetingInfotest-2") + post(bigbluebutton_api_get_meeting_info_url, params: URI.encode_www_form(check_params)) response_xml = Nokogiri::XML(response.body) expect(response_xml.at_xpath('/response/returncode').text).to eq('SUCCESS') @@ -327,8 +329,9 @@ allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['test-1']) - post bigbluebutton_api_get_meeting_info_url, - params: { meetingID: "SHA256_meeting", checksum: "217da05b692320353e17a1b11c24e9e715caeee51ab5af35231ee5becc350d1e" } + check_params = { meetingID: "SHA256_meeting" } + check_params[:checksum] = Digest::SHA256.hexdigest("getMeetingInfotest-1") + post(bigbluebutton_api_get_meeting_info_url, params: URI.encode_www_form(check_params)) response_xml = Nokogiri::XML(response.body) expect(response_xml.at_xpath('/response/returncode').text).to eq('SUCCESS') @@ -799,7 +802,7 @@ voiceBridge: "123" } - stub_create = stub_request(:post, encode_bbb_uri("create", server.url, server.secret, params)) + stub_create = stub_request(:get, encode_bbb_uri("create", server.url, server.secret, params)) .to_return(body: "SUCCESStest-meeting-1 apmp") @@ -816,6 +819,38 @@ expect(meeting.server.id).to eq(server.id) expect(new_server.load).to eq(1) end + + it 'passes through preuploaded slides xml' do + params = { + meetingID: 'test-meeting-1', + moderatorPW: 'mp', + voiceBridge: "123" + } + + body = '' + url = URI(bigbluebutton_api_create_url) + url.query = params.to_param + + stub_create = + stub_request(:post, encode_bbb_uri("create", server.url, server.secret, params)) \ + .with(body: body, headers: { 'Content-Type' => 'application/xml' }) \ + .to_return(body: "SUCCESStest-meeting-1 + apmp") + + # The Moodle integration uses text/xml instead of application/xml, so check that the matching handles that. + post url.to_s, params: body, headers: { 'Content-Type' => 'text/xml' } + + # Reload + new_server = Server.find(server.id) + meeting = Meeting.find(params[:meetingID]) + + response_xml = Nokogiri::XML(response.body) + expect(stub_create).to have_been_requested + expect(response_xml.at_xpath('/response/returncode').text).to eq('SUCCESS') + expect(meeting.id).to eq(params[:meetingID]) + expect(meeting.server.id).to eq(server.id) + expect(new_server.load).to eq(1) + end end context 'multitenancy' do @@ -1237,13 +1272,17 @@ end context 'POST request' do - it "redirects user to the correct join url for a post request" do + it 'is not supported' do meeting = create(:meeting, server: server) params = { meetingID: meeting.id, password: "test-password", fullName: "test-name" } post bigbluebutton_api_join_url, params: params - expect(response).to redirect_to(encode_bbb_uri("join", server.url, server.secret, params).to_s) + response_xml = Nokogiri.XML(response.body) + expected_error = BBBErrors::UnsupportedRequestError.new + expect(response_xml.at_xpath("/response/returncode").text).to(eq("FAILED")) + expect(response_xml.at_xpath("/response/messageKey").text).to(eq(expected_error.message_key)) + expect(response_xml.at_xpath("/response/message").text).to(eq(expected_error.message)) end end @@ -1356,7 +1395,7 @@ describe '#insert_document' do it "responds with MissingMeetingIDError if meeting ID is not passed" do - post bigbluebutton_api_insertDocument_url + post bigbluebutton_api_insert_document_url, as: :xml response_xml = Nokogiri.XML(response.body) expected_error = BBBErrors::MissingMeetingIDError.new @@ -1366,7 +1405,9 @@ end it "responds with MeetingNotFoundError if meeting is not found in database for join" do - post bigbluebutton_api_insertDocument_url, params: { meetingID: "test-meeting-1" } + url = URI(bigbluebutton_api_insert_document_url) + url.query = { meetingID: 'test-meeting-1' }.to_param + post url.to_s, as: :xml response_xml = Nokogiri.XML(response.body) expected_error = BBBErrors::MeetingNotFoundError.new @@ -1379,10 +1420,17 @@ server = create(:server) meeting = create(:meeting, server: server) - stub_insert = stub_request(:post, encode_bbb_uri("insertDocument", server.url, server.secret, { meetingID: meeting.id })) - .to_return(body: "SUCCESSPresentation is being uploaded") + body = '' + url = URI(bigbluebutton_api_insert_document_url) + url.query = { meetingID: meeting.id }.to_param + + stub_insert = + stub_request(:post, encode_bbb_uri("insertDocument", server.url, server.secret, { meetingID: meeting.id })) \ + .with(body: body, headers: { 'Content-Type' => 'application/xml' }) \ + .to_return(body: "SUCCESSPresentation is being uploaded") - post bigbluebutton_api_insertDocument_url, params: { meetingID: meeting.id } + # The Moodle integration uses text/xml instead of application/xml, so check that the matching handles that. + post url.to_s, params: body, headers: { 'Content-Type' => 'text/xml' } response_xml = Nokogiri.XML(response.body) expect(stub_insert).to have_been_requested @@ -1933,38 +1981,18 @@ end context 'POST request' do - it "updates published property to true for a post request" do + it 'is not supported' do r = create(:recording, :unpublished) params = encode_bbb_params("publishRecordings", { recordID: r.record_id, publish: "true" }.to_query) post bigbluebutton_api_publish_recordings_url, params: params - expect(response).to be_successful - response_xml = Nokogiri::XML(response.body) - expect(response_xml.at_xpath('/response/returncode').text).to eq('SUCCESS') - expect(response_xml.at_xpath('/response/published').text).to eq('true') - - r.reload - expect(r.published).to be(true) - end - - it "returns notFound if RECORDING_DISABLED flag is set to true for a post request" do - params = encode_bbb_params("publishRecordings", { publish: "true" }.to_query) - - allow(Rails.configuration.x).to receive(:recording_disabled).and_return(true) - reload_routes! - - post bigbluebutton_api_publish_recordings_url, params: params - - expect(response).to be_successful - response_xml = Nokogiri::XML(response.body) - expect(response_xml.at_xpath('/response/returncode').text).to eq('FAILED') - expect(response_xml.at_xpath('/response/messageKey').text).to eq('notFound') - expect(response_xml.at_xpath('/response/message').text).to eq('We could not find recordings') - - allow(Rails.configuration.x).to receive(:recording_disabled).and_return(false) - reload_routes! + response_xml = Nokogiri.XML(response.body) + expected_error = BBBErrors::UnsupportedRequestError.new + expect(response_xml.at_xpath("/response/returncode").text).to(eq("FAILED")) + expect(response_xml.at_xpath("/response/messageKey").text).to(eq(expected_error.message_key)) + expect(response_xml.at_xpath("/response/message").text).to(eq(expected_error.message)) end end @@ -2329,18 +2357,17 @@ end context 'POST request' do - it 'deletes the recording from the database if passed recordID for a post request' do + it 'is not supported' do r = create(:recording) params = encode_bbb_params('deleteRecordings', "recordID=#{r.record_id}") post bigbluebutton_api_delete_recordings_url, params: params - expect(response).to be_successful - response_xml = Nokogiri::XML(response.body) - expect(response_xml.at_xpath('/response/returncode').text).to eq('SUCCESS') - expect(response_xml.at_xpath('/response/deleted').text).to eq('true') - - expect(r.reload.state).to eq('deleted') + response_xml = Nokogiri.XML(response.body) + expected_error = BBBErrors::UnsupportedRequestError.new + expect(response_xml.at_xpath("/response/returncode").text).to(eq("FAILED")) + expect(response_xml.at_xpath("/response/messageKey").text).to(eq(expected_error.message_key)) + expect(response_xml.at_xpath("/response/message").text).to(eq(expected_error.message)) end end diff --git a/spec/requests/servers_controller_spec.rb b/spec/requests/servers_controller_spec.rb index 6311d1ae..85f479ec 100644 --- a/spec/requests/servers_controller_spec.rb +++ b/spec/requests/servers_controller_spec.rb @@ -52,7 +52,7 @@ } it 'creates a new BigBlueButton server' do - expect { post scalelite_api_add_server_url, params: { server: valid_params } }.to change { Server.all.count }.by(1) + expect { post scalelite_api_add_server_url, params: { server: valid_params }, as: :json }.to change { Server.all.count }.by(1) expect(response).to have_http_status(:created) response_data = response.parsed_body server = Server.find(response_data['id']) @@ -62,7 +62,7 @@ end it 'defaults load_multiplier to 1.0 if not provided' do - post scalelite_api_add_server_url, params: { server: valid_params.except(:load_multiplier) } + post scalelite_api_add_server_url, params: { server: valid_params.except(:load_multiplier) }, as: :json expect(response).to have_http_status(:created) server = Server.find(response.parsed_body['id']) expect(server.load_multiplier.to_d).to eq(1.0) @@ -71,13 +71,13 @@ context 'with invalid parameters' do it 'renders an error message if URL is missing' do - post scalelite_api_add_server_url, params: { server: { url: 'https://example.com/bigbluebutton' } } + post scalelite_api_add_server_url, params: { server: { url: 'https://example.com/bigbluebutton' } }, as: :json expect(response).to have_http_status(:bad_request) expect(response.parsed_body['error']).to eq('Server needs a URL and a secret') end it 'renders an error message if secret is missing' do - post scalelite_api_add_server_url, params: { server: { secret: 'supersecret' } } + post scalelite_api_add_server_url, params: { server: { secret: 'supersecret' } }, as: :json expect(response).to have_http_status(:bad_request) expect(response.parsed_body['error']).to eq('Server needs a URL and a secret') end @@ -88,7 +88,7 @@ context 'when updating state' do it 'updates the server state to "enabled"' do server = create(:server) - post scalelite_api_update_server_url, params: { id: server.id, server: { state: 'enable' } } + post scalelite_api_update_server_url, params: { id: server.id, server: { state: 'enable' } }, as: :json updated_server = Server.find(server.id) # Reload expect(updated_server.state).to eq('enabled') expect(response).to have_http_status(:ok) @@ -98,7 +98,7 @@ it 'updates the server state to "cordoned"' do server = create(:server) - post scalelite_api_update_server_url, params: { id: server.id, server: { state: 'cordon' } } + post scalelite_api_update_server_url, params: { id: server.id, server: { state: 'cordon' } }, as: :json updated_server = Server.find(server.id) # Reload expect(updated_server.state).to eq('cordoned') expect(response).to have_http_status(:ok) @@ -108,7 +108,7 @@ it 'updates the server state to "disabled"' do server = create(:server) - post scalelite_api_update_server_url, params: { id: server.id, server: { state: 'disable' } } + post scalelite_api_update_server_url, params: { id: server.id, server: { state: 'disable' } }, as: :json updated_server = Server.find(server.id) # Reload expect(updated_server.state).to eq('disabled') expect(response).to have_http_status(:ok) @@ -118,7 +118,7 @@ it 'returns an error for an invalid state parameter' do server = create(:server) - post scalelite_api_update_server_url, params: { id: server.id, server: { state: 'invalid_state' } } + post scalelite_api_update_server_url, params: { id: server.id, server: { state: 'invalid_state' } }, as: :json expect(response).to have_http_status(:bad_request) expect(response.parsed_body['error']).to eq("Invalid state parameter: invalid_state") end @@ -127,7 +127,7 @@ context 'when updating load_multiplier' do it 'updates the server load_multiplier' do server = create(:server) - post scalelite_api_update_server_url, params: { id: server.id, server: { load_multiplier: '2.5' } } + post scalelite_api_update_server_url, params: { id: server.id, server: { load_multiplier: '2.5' } }, as: :json updated_server = Server.find(server.id) # Reload expect(updated_server.load_multiplier).to eq("2.5") expect(response).to have_http_status(:ok) @@ -137,7 +137,7 @@ it 'returns an error for an invalid load_multiplier parameter' do server = create(:server) - post scalelite_api_update_server_url, params: { id: server.id, server: { load_multiplier: 0 } } + post scalelite_api_update_server_url, params: { id: server.id, server: { load_multiplier: 0 } }, as: :json expect(response).to have_http_status(:bad_request) expect(response.parsed_body['error']).to eq("Load-multiplier must be a non-zero number") end @@ -148,7 +148,7 @@ context 'with an existing server' do it 'deletes the server' do server = create(:server) - expect { post scalelite_api_delete_server_url, params: { id: server.id } }.to change { Server.all.count }.by(-1) + expect { post scalelite_api_delete_server_url, params: { id: server.id }, as: :json }.to change { Server.all.count }.by(-1) expect(response).to have_http_status(:ok) expect(response.parsed_body['success']).to eq("Server id=#{server.id} was destroyed") end @@ -156,7 +156,7 @@ context 'with a non-existent server' do it 'does not delete any server' do - post scalelite_api_delete_server_url, params: { id: 'nonexistent-id' } + post scalelite_api_delete_server_url, params: { id: 'nonexistent-id' }, as: :json expect(response).to have_http_status(:not_found) expect(response.parsed_body['error']).to eq("Couldn't find server with id=nonexistent-id") end @@ -189,7 +189,7 @@ .to_return(body: "SUCCESSOK The meeting was ended successfully.") - post scalelite_api_panic_server_url, params: { id: server.id } + post scalelite_api_panic_server_url, params: { id: server.id }, as: :json expect(response).to have_http_status(:ok) json = response.parsed_body @@ -202,7 +202,7 @@ it 'keeps server state if keep_state is true' do server = create(:server, state: 'enabled') - post scalelite_api_panic_server_url, params: { id: server.id, keep_state: true } + post scalelite_api_panic_server_url, params: { id: server.id, keep_state: true }, as: :json expect(response).to have_http_status(:ok) json = response.parsed_body @@ -213,7 +213,7 @@ end it 'returns an error message if the server is not found' do - post scalelite_api_panic_server_url, params: { id: 'nonexistent_id' } + post scalelite_api_panic_server_url, params: { id: 'nonexistent_id' }, as: :json expect(response).to have_http_status(:not_found) json = response.parsed_body @@ -234,26 +234,42 @@ it 'successfully creates a server with checksum value computed using SHA1' do allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['sha1-secret']) - post scalelite_api_add_server_url, params: { server: valid_params, checksum: 'a347310666f712cc1d5e860969aaf39bda37298d' } + url = URI(scalelite_api_add_server_url) + url.query = "checksum=#{Digest::SHA1.hexdigest('addServersha1-secret')}" + post(url.to_s, params: { server: valid_params }, as: :json) expect(response).to have_http_status(:created) end it 'successfully creates a server with checksum value computed using SHA256' do allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['sha256-secret']) - post scalelite_api_add_server_url, -params: { server: valid_params, checksum: 'b6a4fd61f463c63ba8ac0696c02f87ee3f8de6d5e9f3e94cedeaded63161c73a' } + url = URI(scalelite_api_add_server_url) + url.query = "checksum=#{Digest::SHA256.hexdigest('addServersha256-secret')}" + post(url.to_s, params: { server: valid_params }, as: :json) expect(response).to have_http_status(:created) end it 'returns a checksum error if the wrong secret is used' do allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['sha1-secret']) - # a random secret was used to generate the checksum here - post scalelite_api_add_tenant_url, params: { server: valid_params, checksum: 'e636000e010c2effcabdfcf78bb59d0971bfb8eb' } + url = URI(scalelite_api_add_server_url) + url.query = "checksum=#{Digest::SHA1.hexdigest('addServersha1-secret-incorrect')}" + post(url.to_s, params: { server: valid_params }, as: :json) xml_response = Nokogiri::XML(response.body) expect(xml_response.at_xpath("//response/returncode").text).to eq("FAILED") expect(xml_response.at_xpath("//response/messageKey").text).to eq("checksumError") end + + it 'returns an error if the wrong content type is used' do + allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['sha1-secret']) + + url = URI(scalelite_api_add_server_url) + url.query = "checksum=#{Digest::SHA1.hexdigest('addServersha1-secret')}" + post(url.to_s, params: { server: valid_params }, as: :url_encoded_form) + + xml_response = Nokogiri::XML(response.body) + expect(xml_response.at_xpath('/response/returncode').content).to eq('FAILED') + expect(xml_response.at_xpath('/response/messageKey').content).to eq('unsupportedContentType') + end end end diff --git a/spec/requests/tenants_controller_spec.rb b/spec/requests/tenants_controller_spec.rb index 45c44179..a1481aa4 100644 --- a/spec/requests/tenants_controller_spec.rb +++ b/spec/requests/tenants_controller_spec.rb @@ -82,7 +82,7 @@ } it 'creates a new tenant' do - expect { post scalelite_api_add_tenant_url, params: { tenant: valid_params } }.to change { Tenant.all.count }.by(1) + expect { post scalelite_api_add_tenant_url, params: { tenant: valid_params }, as: :json }.to change { Tenant.all.count }.by(1) expect(response).to have_http_status(:created) response_data = response.parsed_body expect(response_data['tenant']['id']).to be_present @@ -91,13 +91,13 @@ context 'with invalid parameters' do it 'renders an error message if name is missing' do - post scalelite_api_add_tenant_url, params: { tenant: { secrets: 'test-secret' } } + post scalelite_api_add_tenant_url, params: { tenant: { secrets: 'test-secret' } }, as: :json expect(response).to have_http_status(:bad_request) expect(response.parsed_body['message']).to eq('Error: both name and secrets are required to create a Tenant') end it 'renders an error message if secret is missing' do - post scalelite_api_add_tenant_url, params: { tenant: { name: 'test-name' } } + post scalelite_api_add_tenant_url, params: { tenant: { name: 'test-name' } }, as: :json expect(response).to have_http_status(:bad_request) expect(response.parsed_body['message']).to eq('Error: both name and secrets are required to create a Tenant') end @@ -112,7 +112,7 @@ it 'updates a tenant name' do tenant = create(:tenant) new_tenant_params = { name: 'new-name' } - post scalelite_api_update_tenant_url, params: { id: tenant.id, tenant: new_tenant_params } + post scalelite_api_update_tenant_url, params: { id: tenant.id, tenant: new_tenant_params }, as: :json expect(response).to have_http_status(:ok) expect(Tenant.find(tenant.id).name).to eq('new-name') # check that the id-> name index was updated expect(Tenant.find_by_name('new-name')).not_to be_nil # check that the new name->id index has been added @@ -122,7 +122,7 @@ it 'updates a tenant secret' do tenant = create(:tenant) new_tenant_params = { secrets: 'new-secret' } - post scalelite_api_update_tenant_url, params: { id: tenant.id, tenant: new_tenant_params } + post scalelite_api_update_tenant_url, params: { id: tenant.id, tenant: new_tenant_params }, as: :json expect(response).to have_http_status(:ok) expect(Tenant.find(tenant.id).secrets).to eq('new-secret') # check that the id-> secret index was updated end @@ -136,7 +136,7 @@ context 'with an existing tenant' do it 'deletes the tenant' do tenant = create(:tenant) - expect { post scalelite_api_delete_tenant_url, params: { id: tenant.id } }.to change { Tenant.all.count }.by(-1) + expect { post scalelite_api_delete_tenant_url, params: { id: tenant.id }, as: :json }.to change { Tenant.all.count }.by(-1) expect(response).to have_http_status(:ok) expect(response.parsed_body['success']).to eq("Tenant id=#{tenant.id} was destroyed") end @@ -144,7 +144,7 @@ context 'with a non-existent tenant' do it 'does not delete any tenant' do - post scalelite_api_delete_tenant_url, params: { id: 'nonexistent-id' } + post scalelite_api_delete_tenant_url, params: { id: 'nonexistent-id' }, as: :json expect(response).to have_http_status(:not_found) expect(response.parsed_body['error']).to eq("Couldn't find Tenant with id=nonexistent-id") end @@ -158,28 +158,46 @@ it 'successfully creates a tenant with checksum value computed using SHA1' do allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['sha1-secret']) - post scalelite_api_add_tenant_url, -params: { tenant: { name: "SHA1-tenant", secrets: "SHA1-secret" }, checksum: '8adf3a77cafa6e72c3ac7ada5e42d92f8ea8f4f0' } + url = URI(scalelite_api_add_tenant_url) + url.query = "checksum=#{Digest::SHA1.hexdigest('addTenantsha1-secret')}" + check_params = { tenant: { name: "SHA1-tenant", secrets: "SHA1-secret" } } + post(url.to_s, params: check_params, as: :json) expect(response).to have_http_status(:created) end it 'successfully creates a tenant with checksum value computed using SHA256' do allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['sha256-secret']) - post scalelite_api_add_tenant_url, -params: { tenant: { name: "SHA1-tenant", secrets: "SHA256-secret" }, checksum: 'a8a7e0b3ab8fe65283628e7032177ab3696e41f7c1bc8aec86fd8c60ffb44cdd' } + url = URI(scalelite_api_add_tenant_url) + url.query = "checksum=#{Digest::SHA1.hexdigest('addTenantsha256-secret')}" + check_params = { tenant: { name: "SHA1-tenant", secrets: "SHA256-secret" } } + post(url.to_s, params: check_params, as: :json) expect(response).to have_http_status(:created) end it 'returns a checksum error if the wrong secret is used' do allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['sha1-secret']) - # a random secret was used to generate the checksum here - post scalelite_api_add_tenant_url, -params: { tenant: { name: "SHA1-tenant", secrets: "SHA1-secret" }, checksum: 'e636000e010c2effcabdfcf78bb59d0971bfb8eb' } + url = URI(scalelite_api_add_tenant_url) + url.query = "checksum=#{Digest::SHA1.hexdigest('addTenantsha1-secret-incorrect')}" + check_params = { tenant: { name: "SHA1-tenant", secrets: "SHA1-secret" } } + post(url.to_s, params: check_params, as: :json) xml_response = Nokogiri::XML(response.body) expect(xml_response.at_xpath("//response/returncode").text).to eq("FAILED") expect(xml_response.at_xpath("//response/messageKey").text).to eq("checksumError") end + + it 'returns an error if the wrong content type is used' do + allow(Rails.configuration.x).to receive(:loadbalancer_secrets).and_return(['sha1-secret']) + + url = URI(scalelite_api_add_tenant_url) + url.query = "checksum=#{Digest::SHA1.hexdigest('addServersha1-secret')}" + check_params = { tenant: { name: "SHA1-tenant", secrets: "SHA1-secret" } } + post(url.to_s, params: check_params, as: :url_encoded_form) + + xml_response = Nokogiri::XML(response.body) + expect(xml_response.at_xpath('/response/returncode').content).to eq('FAILED') + expect(xml_response.at_xpath('/response/messageKey').content).to eq('unsupportedContentType') + end end end diff --git a/test/controllers/bigbluebutton_api_controller_test.rb b/test/controllers/bigbluebutton_api_controller_test.rb index 82caa601..8a943f61 100644 --- a/test/controllers/bigbluebutton_api_controller_test.rb +++ b/test/controllers/bigbluebutton_api_controller_test.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'test_helper' + class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest include BBBErrors include ApiHelper @@ -83,9 +85,10 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest stub_request(:get, url) .to_return(body: 'SUCCESSSHA1_meeting') + check_params = { meetingID: 'SHA1_meeting' } + check_params[:checksum] = Digest::SHA256.hexdigest("getMeetingInfotest-2") Rails.configuration.x.stub(:loadbalancer_secrets, ['test-2']) do - post bigbluebutton_api_get_meeting_info_url, params: { meetingID: 'SHA1_meeting', - checksum: 'cbf00ea96fae6ff06c2cb311bbde8b26ad66d765', } + post(bigbluebutton_api_get_meeting_info_url, params: URI.encode_www_form(check_params)) end response_xml = Nokogiri::XML(@response.body) @@ -101,10 +104,10 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest stub_request(:get, url) .to_return(body: 'SUCCESSSHA256_meeting') + check_params = { meetingID: 'SHA256_meeting' } + check_params[:checksum] = Digest::SHA256.hexdigest("getMeetingInfotest-1") Rails.configuration.x.stub(:loadbalancer_secrets, ['test-1']) do - post bigbluebutton_api_get_meeting_info_url, - params: { meetingID: 'SHA256_meeting', - checksum: '217da05b692320353e17a1b11c24e9e715caeee51ab5af35231ee5becc350d1e', } + post(bigbluebutton_api_get_meeting_info_url, params: URI.encode_www_form(check_params)) end response_xml = Nokogiri::XML(@response.body) assert_equal 'SUCCESS', response_xml.at_xpath('/response/returncode').content @@ -1085,7 +1088,7 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest assert_redirected_to encode_bbb_uri('join', server1.url, server1.secret, params).to_s end - test 'join redirects user to the corrent join url for a post request' do + test 'join does not support POST requests' do server1 = Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', secret: 'test-1-secret', enabled: true, load: 0, online: true) meeting = Meeting.find_or_create_with_server('test-meeting-1', server1, 'mp') @@ -1096,7 +1099,12 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest post bigbluebutton_api_join_url, params: params end - assert_redirected_to encode_bbb_uri('join', server1.url, server1.secret, params).to_s + assert_response :success + response_xml = Nokogiri.XML(response.body) + expected_error = BBBErrors::UnsupportedRequestError.new + assert_equal(response_xml.at_xpath('/response/returncode').text, 'FAILED') + assert_equal(response_xml.at_xpath('/response/messageKey').text, expected_error.message_key) + assert_equal(response_xml.at_xpath('/response/message').text, expected_error.message) end test 'join redirects user to the current join url with only permitted params for join' do @@ -1624,7 +1632,7 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest assert_equal r.published, true end - test 'publishRecordings updates published property to true for a post request' do + test 'publishRecordings does not support POST requests' do r = create(:recording, :unpublished) assert_equal r.published, false @@ -1633,12 +1641,14 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest post bigbluebutton_api_publish_recordings_url, params: params end - assert_response :success - assert_select 'response>returncode', 'SUCCESS' - assert_select 'response>published', 'true' + response_xml = Nokogiri.XML(response.body) + expected_error = BBBErrors::UnsupportedRequestError.new + assert_equal('FAILED', response_xml.at_xpath('/response/returncode').text) + assert_equal(expected_error.message_key, response_xml.at_xpath('/response/messageKey').text) + assert_equal(expected_error.message, response_xml.at_xpath('/response/message').text) r.reload - assert_equal r.published, true + assert_equal r.published, false end test 'publishRecordings returns error if no recording found' do @@ -1837,17 +1847,22 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest assert r.reload.state.eql?('deleted') end - test 'deleteRecordings deletes the recording from the database if passed recordID for a post request' do + test 'deleteRecordings does not support POST requests' do r = create(:recording) params = encode_bbb_params('deleteRecordings', "recordID=#{r.record_id}") BigBlueButtonApiController.stub_any_instance(:verify_checksum, nil) do post bigbluebutton_api_delete_recordings_url, params: params end + assert_response :success - assert_select 'response>returncode', 'SUCCESS' - assert_select 'response>deleted', 'true' - assert r.reload.state.eql?('deleted') + response_xml = Nokogiri.XML(response.body) + expected_error = BBBErrors::UnsupportedRequestError.new + assert_equal('FAILED', response_xml.at_xpath('/response/returncode').text) + assert_equal(expected_error.message_key, response_xml.at_xpath('/response/messageKey').text) + assert_equal(expected_error.message, response_xml.at_xpath('/response/message').text) + r.reload + assert_not_equal('deleted', r.state) end test 'deleteRecordings handles multiple recording IDs passed' do @@ -1910,15 +1925,18 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest reload_routes! end - test 'publishRecordings returns notFound if RECORDING_DISABLED flag is set to true for a post request' do + test 'publishRecordings does not support POST requests if RECORDING_DISABLED flag is set to true' do params = encode_bbb_params('publishRecordings', { publish: 'true' }.to_query) Rails.configuration.x.recording_disabled = true reload_routes! post 'http://www.example.com/bigbluebutton/api/publishRecordings', params: params + assert_response :success - assert_select 'response>returncode', 'FAILED' - assert_select 'response>messageKey', 'notFound' - assert_select 'response>message', 'We could not find recordings' + response_xml = Nokogiri.XML(response.body) + expected_error = BBBErrors::UnsupportedRequestError.new + assert_equal('FAILED', response_xml.at_xpath('/response/returncode').text) + assert_equal(expected_error.message_key, response_xml.at_xpath('/response/messageKey').text) + assert_equal(expected_error.message, response_xml.at_xpath('/response/message').text) Rails.configuration.x.recording_disabled = false reload_routes! end @@ -1963,16 +1981,18 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest reload_routes! end - test 'deleteRecordings returns notFound if RECORDING_DISABLED flag is set to TRUE for a post request' do + test 'deleteRecordings does not support POST requests if RECORDING_DISABLED flag is set to TRUE' do r = create(:recording) params = encode_bbb_params('deleteRecordings', "recordID=#{r.record_id}") Rails.configuration.x.recording_disabled = true reload_routes! post 'http://www.example.com/bigbluebutton/api/deleteRecordings', params: params assert_response :success - assert_select 'response>returncode', 'FAILED' - assert_select 'response>messageKey', 'notFound' - assert_select 'response>message', 'We could not find recordings' + response_xml = Nokogiri.XML(response.body) + expected_error = BBBErrors::UnsupportedRequestError.new + assert_equal('FAILED', response_xml.at_xpath('/response/returncode').text) + assert_equal(expected_error.message_key, response_xml.at_xpath('/response/messageKey').text) + assert_equal(expected_error.message, response_xml.at_xpath('/response/message').text) Rails.configuration.x.recording_disabled = false reload_routes! end From 5e361832ba1fd180b0da1a5a123b5a22c206c80f Mon Sep 17 00:00:00 2001 From: Ahmad Farhat Date: Thu, 9 Nov 2023 12:13:23 -0500 Subject: [PATCH 2/7] Initial work for custom lrs integration (#1035) * Initial work for custom lrs integration * Added tests --- .rubocop.yml | 2 + .../bigbluebutton_api_controller.rb | 5 ++ app/models/tenant.rb | 16 ++++- app/services/lrs_payload_service.rb | 63 +++++++++++++++++++ lib/tasks/tenants.rake | 38 +++++++++++ spec/factories/tenant.rb | 6 ++ .../bigbluebutton_api_controller_spec.rb | 43 +++++++++++++ spec/services/lrs_payload_service_spec.rb | 44 +++++++++++++ 8 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 app/services/lrs_payload_service.rb create mode 100644 spec/services/lrs_payload_service_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index b17ad0f7..d939fb4b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -135,6 +135,7 @@ Metrics/PerceivedComplexity: Exclude: - app/models/recording.rb - app/models/server.rb + - app/models/tenant.rb - lib/server_sync.rb # Avoid classes longer than 100 lines of code. @@ -164,6 +165,7 @@ Metrics/CyclomaticComplexity: Exclude: - app/models/recording.rb - app/models/server.rb + - app/models/tenant.rb - lib/server_sync.rb # Checks for method parameter names that contain capital letters, end in numbers, or do not meet a minimal length. diff --git a/app/controllers/bigbluebutton_api_controller.rb b/app/controllers/bigbluebutton_api_controller.rb index 371dbf60..ced8605c 100644 --- a/app/controllers/bigbluebutton_api_controller.rb +++ b/app/controllers/bigbluebutton_api_controller.rb @@ -205,6 +205,11 @@ def create params[:voiceBridge] = meeting.voice_bridge + if @tenant&.lrs_endpoint.present? + lrs_payload = LrsPayloadService.new(tenant: @tenant, secret: server.secret).call + params[:'meta_secret-lrs-payload'] = lrs_payload if lrs_payload.present? + end + have_preuploaded_slide = request.post? && request.content_mime_type == Mime[:xml] logger.debug("Creating meeting #{params[:meetingID]} on BigBlueButton server #{server.id}") diff --git a/app/models/tenant.rb b/app/models/tenant.rb index 1fce1188..8ee40cdf 100644 --- a/app/models/tenant.rb +++ b/app/models/tenant.rb @@ -3,7 +3,7 @@ class Tenant < ApplicationRedisRecord SECRETS_SEPARATOR = ':' - define_attribute_methods :id, :name, :secrets + define_attribute_methods :id, :name, :secrets, :lrs_endpoint, :kc_token_url, :kc_client_id, :kc_client_secret, :kc_username, :kc_password # Unique ID for this tenant application_redis_attr :id @@ -14,6 +14,14 @@ class Tenant < ApplicationRedisRecord # Shared secrets for making API requests for this tenant (: separated) application_redis_attr :secrets + # Custom LRS work + application_redis_attr :lrs_endpoint + application_redis_attr :kc_token_url + application_redis_attr :kc_client_id + application_redis_attr :kc_client_secret + application_redis_attr :kc_username + application_redis_attr :kc_password + def save! with_connection do |redis| raise RecordNotSaved.new('Cannot update id field', self) if id_changed? && !@new_record @@ -34,6 +42,12 @@ def save! pipeline.del(old_names_key) if !id_changed? && name_changed? # Delete the old name key if it's not a new record and the name was updated pipeline.hset(id_key, 'name', name) if name_changed? pipeline.hset(id_key, 'secrets', secrets) if secrets_changed? + pipeline.hset(id_key, 'lrs_endpoint', lrs_endpoint) if lrs_endpoint_changed? + pipeline.hset(id_key, 'kc_token_url', kc_token_url) if kc_token_url_changed? + pipeline.hset(id_key, 'kc_client_id', kc_client_id) if kc_client_id_changed? + pipeline.hset(id_key, 'kc_client_secret', kc_client_secret) if kc_client_secret_changed? + pipeline.hset(id_key, 'kc_username', kc_username) if kc_username_changed? + pipeline.hset(id_key, 'kc_password', kc_password) if kc_password_changed? pipeline.sadd?('tenants', id) if id_changed? end end diff --git a/app/services/lrs_payload_service.rb b/app/services/lrs_payload_service.rb new file mode 100644 index 00000000..202e8099 --- /dev/null +++ b/app/services/lrs_payload_service.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +class LrsPayloadService + def initialize(tenant:, secret:) + @tenant = tenant + @secret = secret + end + + def call + Rails.logger.debug { "Fetching LRS token from #{@tenant.kc_token_url}" } + + url = URI.parse(@tenant.kc_token_url) + http = Net::HTTP.new(url.host, url.port) + http.use_ssl = (url.scheme == 'https') + + payload = { + client_id: @tenant.kc_client_id, + client_secret: @tenant.kc_client_secret, + username: @tenant.kc_username, + password: @tenant.kc_password, + grant_type: 'password' + } + + request = Net::HTTP::Post.new(url.path) + request.set_form_data(payload) + + response = http.request(request) + + if response.code.to_i != 200 + Rails.logger.warn("Error #{response.message} when trying to fetch LRS Access Token") + return nil + end + + parsed_response = JSON.parse(response.body) + kc_access_token = parsed_response['access_token'] + + lrs_payload = { + lrs_endpoint: @tenant.lrs_endpoint, + lrs_token: kc_access_token + } + + # Generate a random salt + salt = SecureRandom.random_bytes(8) + + # Generate a key and initialization vector (IV) using PBKDF2 with SHA-256 + key_iv = OpenSSL::PKCS5.pbkdf2_hmac(@secret, salt, 10_000, 48, OpenSSL::Digest.new('SHA256')) + key = key_iv[0, 32] # 32 bytes for the key + iv = key_iv[32, 16] # 16 bytes for the IV + + # Encrypt the data using AES-256-CBC + cipher = OpenSSL::Cipher.new('AES-256-CBC') + cipher.encrypt + cipher.key = key + cipher.iv = iv + + # Encrypt and Base64 encode the data + Base64.strict_encode64(Random.random_bytes(8) + salt + cipher.update(lrs_payload.to_json) + cipher.final) + rescue StandardError => e + Rails.logger.warn("Error #{e} when trying to compute LRS Payload") + + nil + end +end diff --git a/lib/tasks/tenants.rake b/lib/tasks/tenants.rake index 6a10a277..5af6167b 100644 --- a/lib/tasks/tenants.rake +++ b/lib/tasks/tenants.rake @@ -14,6 +14,12 @@ task tenants: :environment do |_t, _args| puts("id: #{tenant.id}") puts("\tname: #{tenant.name}") puts("\tsecrets: #{tenant.secrets}") + puts("\tlrs_endpoint: #{tenant.lrs_endpoint}") if tenant.lrs_endpoint.present? + puts("\tkc_token_url: #{tenant.kc_token_url}") if tenant.kc_token_url.present? + puts("\tkc_client_id: #{tenant.kc_client_id}") if tenant.kc_client_id.present? + puts("\tkc_client_secret: #{tenant.kc_client_secret}") if tenant.kc_client_secret.present? + puts("\tkc_username: #{tenant.kc_username}") if tenant.kc_username.present? + puts("\tkc_password: #{tenant.kc_password}") if tenant.kc_password.present? end end @@ -53,6 +59,38 @@ namespace :tenants do tenant = Tenant.find(id) tenant.name = name if name.present? tenant.secrets = secrets if secrets.present? + + tenant.save! + + puts('OK') + puts("Updated Tenant id: #{tenant.id}") + end + + desc 'Update an existing Tenants LRS credentials' + task :update_lrs, [:id, :lrs_endpoint, :kc_token_url, :kc_client_id, :kc_client_secret, :kc_username, :kc_password] => :environment do |_t, args| + check_multitenancy + id = args[:id] + lrs_endpoint = args[:lrs_endpoint] + kc_token_url = args[:kc_token_url] + kc_client_id = args[:kc_client_id] + kc_client_secret = args[:kc_client_secret] + kc_username = args[:kc_username] + kc_password = args[:kc_password] + + if id.blank? || lrs_endpoint.blank? || kc_token_url.blank? || kc_client_id.blank? || + kc_client_secret.blank? || kc_username.blank? || kc_password.blank? + puts('Error: id and either name or secrets are required to update a Tenant') + exit(1) + end + + tenant = Tenant.find(id) + tenant.lrs_endpoint = lrs_endpoint + tenant.kc_token_url = kc_token_url + tenant.kc_client_id = kc_client_id + tenant.kc_client_secret = kc_client_secret + tenant.kc_username = kc_username + tenant.kc_password = kc_password + tenant.save! puts('OK') diff --git a/spec/factories/tenant.rb b/spec/factories/tenant.rb index dc795fea..15f43012 100644 --- a/spec/factories/tenant.rb +++ b/spec/factories/tenant.rb @@ -4,5 +4,11 @@ factory :tenant do name { Faker::Creature::Animal.name } secrets { "#{Faker::Crypto.sha256}:#{Faker::Crypto.sha512}" } + lrs_endpoint { nil } + kc_token_url { nil } + kc_client_id { nil } + kc_client_secret { nil } + kc_username { nil } + kc_password { nil } end end diff --git a/spec/requests/bigbluebutton_api_controller_spec.rb b/spec/requests/bigbluebutton_api_controller_spec.rb index a0da8be2..2e9412f5 100644 --- a/spec/requests/bigbluebutton_api_controller_spec.rb +++ b/spec/requests/bigbluebutton_api_controller_spec.rb @@ -957,6 +957,49 @@ end end end + + context 'secret-lrs-payload' do + before do + tenant.lrs_endpoint = 'https://test.com' + + tenant.save! + end + + it 'makes a call to the LrsPayloadService and sets meta_secret-lrs-payload' do + allow_any_instance_of(LrsPayloadService).to receive(:call).and_return('test-token') + + create_params = { meetingID: "test-meeting-1", moderatorPW: "test-password", voiceBridge: "1234567" } + stub_params = { meetingID: "test-meeting-1", moderatorPW: "test-password", voiceBridge: "1234567", + 'meta_tenant-id': tenant.id, 'meta_secret-lrs-payload': 'test-token' } + + stub_create = stub_request(:get, encode_bbb_uri("create", server.url, server.secret, stub_params)) + .to_return(body: "SUCCESStest-meeting-1 + apmp") + + get bigbluebutton_api_create_url, params: create_params + + response_xml = Nokogiri.XML(response.body) + expect(stub_create).to have_been_requested + expect(response_xml.at_xpath("/response/returncode").text).to eq("SUCCESS") + end + + it 'does not set meta_secret-lrs-payload if the value is nil' do + allow_any_instance_of(LrsPayloadService).to receive(:call).and_return(nil) + + create_params = { meetingID: "test-meeting-1", moderatorPW: "test-password", voiceBridge: "1234567" } + stub_params = { meetingID: "test-meeting-1", moderatorPW: "test-password", voiceBridge: "1234567", 'meta_tenant-id': tenant.id } + + stub_create = stub_request(:get, encode_bbb_uri("create", server.url, server.secret, stub_params)) + .to_return(body: "SUCCESStest-meeting-1 + apmp") + + get bigbluebutton_api_create_url, params: create_params + + response_xml = Nokogiri.XML(response.body) + expect(stub_create).to have_been_requested + expect(response_xml.at_xpath("/response/returncode").text).to eq("SUCCESS") + end + end end end diff --git a/spec/services/lrs_payload_service_spec.rb b/spec/services/lrs_payload_service_spec.rb new file mode 100644 index 00000000..841f47a5 --- /dev/null +++ b/spec/services/lrs_payload_service_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe LrsPayloadService, type: :service do + let!(:tenant) do + create(:tenant, + name: 'bn', + lrs_endpoint: 'https://lrs_endpoint.com', + kc_token_url: 'https://token_url.com/auth/token', + kc_client_id: 'client_id', + kc_client_secret: 'client_secret', + kc_username: 'kc_username', + kc_password: 'kc_password') + end + + describe '#call' do + it 'makes a call to kc_token_url with the correct payload' do + payload = { + client_id: tenant.kc_client_id, + client_secret: tenant.kc_client_secret, + username: tenant.kc_username, + password: tenant.kc_password, + grant_type: 'password' + } + + stub_create = stub_request(:post, tenant.kc_token_url) + .with(body: payload).to_return(body: "kc_access_token") + + described_class.new(tenant: tenant, secret: 'server-secret').call + + expect(stub_create).to have_been_requested + end + + it 'logs a warning and returns nil if kc_token_url returns an error' do + stub_request(:post, tenant.kc_token_url) + .to_return(status: 500, body: 'Internal Server Error', headers: {}) + + expect(Rails.logger).to receive(:warn) + + expect(described_class.new(tenant: tenant, secret: 'server-secret').call).to be_nil + end + end +end From ed3559abaca35d6eb32ff16d4f9c504a8c68d6a7 Mon Sep 17 00:00:00 2001 From: Ahmad Farhat Date: Thu, 14 Dec 2023 14:19:49 -0500 Subject: [PATCH 3/7] Added basic authentication for LRS endpoint (#1038) * Added basic authentication for LRS endpoint * rubo --- app/models/tenant.rb | 5 +- app/services/lrs_payload_service.rb | 59 ++++++++------ lib/tasks/tenants.rake | 29 ++++++- spec/factories/tenant.rb | 1 + spec/services/lrs_payload_service_spec.rb | 96 ++++++++++++++++------- 5 files changed, 134 insertions(+), 56 deletions(-) diff --git a/app/models/tenant.rb b/app/models/tenant.rb index 8ee40cdf..4244b8cf 100644 --- a/app/models/tenant.rb +++ b/app/models/tenant.rb @@ -3,7 +3,8 @@ class Tenant < ApplicationRedisRecord SECRETS_SEPARATOR = ':' - define_attribute_methods :id, :name, :secrets, :lrs_endpoint, :kc_token_url, :kc_client_id, :kc_client_secret, :kc_username, :kc_password + define_attribute_methods :id, :name, :secrets, :lrs_endpoint, :lrs_basic_token, :kc_token_url, :kc_client_id, :kc_client_secret, :kc_username, +:kc_password # Unique ID for this tenant application_redis_attr :id @@ -16,6 +17,7 @@ class Tenant < ApplicationRedisRecord # Custom LRS work application_redis_attr :lrs_endpoint + application_redis_attr :lrs_basic_token application_redis_attr :kc_token_url application_redis_attr :kc_client_id application_redis_attr :kc_client_secret @@ -43,6 +45,7 @@ def save! pipeline.hset(id_key, 'name', name) if name_changed? pipeline.hset(id_key, 'secrets', secrets) if secrets_changed? pipeline.hset(id_key, 'lrs_endpoint', lrs_endpoint) if lrs_endpoint_changed? + pipeline.hset(id_key, 'lrs_basic_token', lrs_basic_token) if lrs_basic_token_changed? pipeline.hset(id_key, 'kc_token_url', kc_token_url) if kc_token_url_changed? pipeline.hset(id_key, 'kc_client_id', kc_client_id) if kc_client_id_changed? pipeline.hset(id_key, 'kc_client_secret', kc_client_secret) if kc_client_secret_changed? diff --git a/app/services/lrs_payload_service.rb b/app/services/lrs_payload_service.rb index 202e8099..1233dded 100644 --- a/app/services/lrs_payload_service.rb +++ b/app/services/lrs_payload_service.rb @@ -7,36 +7,16 @@ def initialize(tenant:, secret:) end def call - Rails.logger.debug { "Fetching LRS token from #{@tenant.kc_token_url}" } - - url = URI.parse(@tenant.kc_token_url) - http = Net::HTTP.new(url.host, url.port) - http.use_ssl = (url.scheme == 'https') - - payload = { - client_id: @tenant.kc_client_id, - client_secret: @tenant.kc_client_secret, - username: @tenant.kc_username, - password: @tenant.kc_password, - grant_type: 'password' - } + token = @tenant.kc_token_url.present? ? fetch_token_from_keycloak : @tenant.lrs_basic_token - request = Net::HTTP::Post.new(url.path) - request.set_form_data(payload) - - response = http.request(request) - - if response.code.to_i != 200 - Rails.logger.warn("Error #{response.message} when trying to fetch LRS Access Token") + if token.nil? + Rails.logger.warn("LRS Token not found") return nil end - parsed_response = JSON.parse(response.body) - kc_access_token = parsed_response['access_token'] - lrs_payload = { lrs_endpoint: @tenant.lrs_endpoint, - lrs_token: kc_access_token + lrs_token: token } # Generate a random salt @@ -60,4 +40,35 @@ def call nil end + + private + + def fetch_token_from_keycloak + Rails.logger.debug { "Fetching LRS token from #{@tenant.kc_token_url}" } + + url = URI.parse(@tenant.kc_token_url) + http = Net::HTTP.new(url.host, url.port) + http.use_ssl = (url.scheme == 'https') + + payload = { + client_id: @tenant.kc_client_id, + client_secret: @tenant.kc_client_secret, + username: @tenant.kc_username, + password: @tenant.kc_password, + grant_type: 'password' + } + + request = Net::HTTP::Post.new(url.path) + request.set_form_data(payload) + + response = http.request(request) + + if response.code.to_i != 200 + Rails.logger.warn("Error #{response.message} when trying to fetch LRS Access Token") + return nil + end + + parsed_response = JSON.parse(response.body) + parsed_response['access_token'] + end end diff --git a/lib/tasks/tenants.rake b/lib/tasks/tenants.rake index 5af6167b..91563b66 100644 --- a/lib/tasks/tenants.rake +++ b/lib/tasks/tenants.rake @@ -15,6 +15,7 @@ task tenants: :environment do |_t, _args| puts("\tname: #{tenant.name}") puts("\tsecrets: #{tenant.secrets}") puts("\tlrs_endpoint: #{tenant.lrs_endpoint}") if tenant.lrs_endpoint.present? + puts("\tlrs_basic_token: #{tenant.lrs_basic_token}") if tenant.lrs_basic_token.present? puts("\tkc_token_url: #{tenant.kc_token_url}") if tenant.kc_token_url.present? puts("\tkc_client_id: #{tenant.kc_client_id}") if tenant.kc_client_id.present? puts("\tkc_client_secret: #{tenant.kc_client_secret}") if tenant.kc_client_secret.present? @@ -66,8 +67,30 @@ namespace :tenants do puts("Updated Tenant id: #{tenant.id}") end - desc 'Update an existing Tenants LRS credentials' - task :update_lrs, [:id, :lrs_endpoint, :kc_token_url, :kc_client_id, :kc_client_secret, :kc_username, :kc_password] => :environment do |_t, args| + desc 'Update an existing Tenants LRS credentials with basic authentication' + task :update_lrs_basic, [:id, :lrs_endpoint, :lrs_basic_token] => :environment do |_t, args| + check_multitenancy + id = args[:id] + lrs_endpoint = args[:lrs_endpoint] + lrs_basic_token = args[:lrs_basic_token] + + if id.blank? || lrs_endpoint.blank? || lrs_basic_token.blank? + puts('Error: id, LRS_ENDPOINT, LRS_BASIC_TOKEN are required to update a Tenant') + exit(1) + end + + tenant = Tenant.find(id) + tenant.lrs_endpoint = lrs_endpoint + tenant.lrs_basic_token = lrs_basic_token + + tenant.save! + + puts('OK') + puts("Updated Tenant id: #{tenant.id}") + end + + desc 'Update an existing Tenants LRS credentials with Keycloak' + task :update_lrs_kc, [:id, :lrs_endpoint, :kc_token_url, :kc_client_id, :kc_client_secret, :kc_username, :kc_password] => :environment do |_t, args| check_multitenancy id = args[:id] lrs_endpoint = args[:lrs_endpoint] @@ -79,7 +102,7 @@ namespace :tenants do if id.blank? || lrs_endpoint.blank? || kc_token_url.blank? || kc_client_id.blank? || kc_client_secret.blank? || kc_username.blank? || kc_password.blank? - puts('Error: id and either name or secrets are required to update a Tenant') + puts('Error: LRS_ENDPOINT, KC_TOKEN_URL, KC_CLIENT_ID, KC_CLIENT_SECRET, KC_USERNAME, KC_PASSWORD are required to update a Tenant') exit(1) end diff --git a/spec/factories/tenant.rb b/spec/factories/tenant.rb index 15f43012..7af50c8e 100644 --- a/spec/factories/tenant.rb +++ b/spec/factories/tenant.rb @@ -5,6 +5,7 @@ name { Faker::Creature::Animal.name } secrets { "#{Faker::Crypto.sha256}:#{Faker::Crypto.sha512}" } lrs_endpoint { nil } + lrs_basic_token { nil } kc_token_url { nil } kc_client_id { nil } kc_client_secret { nil } diff --git a/spec/services/lrs_payload_service_spec.rb b/spec/services/lrs_payload_service_spec.rb index 841f47a5..bbf53cf2 100644 --- a/spec/services/lrs_payload_service_spec.rb +++ b/spec/services/lrs_payload_service_spec.rb @@ -3,42 +3,82 @@ require 'rails_helper' RSpec.describe LrsPayloadService, type: :service do - let!(:tenant) do - create(:tenant, - name: 'bn', - lrs_endpoint: 'https://lrs_endpoint.com', - kc_token_url: 'https://token_url.com/auth/token', - kc_client_id: 'client_id', - kc_client_secret: 'client_secret', - kc_username: 'kc_username', - kc_password: 'kc_password') - end - describe '#call' do - it 'makes a call to kc_token_url with the correct payload' do - payload = { - client_id: tenant.kc_client_id, - client_secret: tenant.kc_client_secret, - username: tenant.kc_username, - password: tenant.kc_password, - grant_type: 'password' - } + context 'Basic Auth' do + it 'uses the lrs_basic_token if set' do + tenant = create(:tenant, name: 'bn', lrs_endpoint: 'https://lrs_endpoint.com', lrs_basic_token: 'basic_token') + + encrypted_value = described_class.new(tenant: tenant, secret: 'server-secret').call + + expect(JSON.parse(decrypt(encrypted_value, 'server-secret'))["lrs_token"]).to eq(tenant.lrs_basic_token) + end - stub_create = stub_request(:post, tenant.kc_token_url) - .with(body: payload).to_return(body: "kc_access_token") + it 'logs a warning and returns nil if lrs_basic_token is not set' do + tenant = create(:tenant, name: 'bn', lrs_endpoint: 'https://lrs_endpoint.com') - described_class.new(tenant: tenant, secret: 'server-secret').call + expect(Rails.logger).to receive(:warn) - expect(stub_create).to have_been_requested + expect(described_class.new(tenant: tenant, secret: 'server-secret').call).to be_nil + end end - it 'logs a warning and returns nil if kc_token_url returns an error' do - stub_request(:post, tenant.kc_token_url) - .to_return(status: 500, body: 'Internal Server Error', headers: {}) + context 'Keycloak' do + let!(:tenant) do + create(:tenant, + name: 'bn', + lrs_endpoint: 'https://lrs_endpoint.com', + kc_token_url: 'https://token_url.com/auth/token', + kc_client_id: 'client_id', + kc_client_secret: 'client_secret', + kc_username: 'kc_username', + kc_password: 'kc_password') + end + + it 'makes a call to kc_token_url with the correct payload' do + payload = { + client_id: tenant.kc_client_id, + client_secret: tenant.kc_client_secret, + username: tenant.kc_username, + password: tenant.kc_password, + grant_type: 'password' + } + + stub_create = stub_request(:post, tenant.kc_token_url) + .with(body: payload).to_return(body: "kc_access_token") + + described_class.new(tenant: tenant, secret: 'server-secret').call + + expect(stub_create).to have_been_requested + end - expect(Rails.logger).to receive(:warn) + it 'logs a warning and returns nil if kc_token_url returns an error' do + stub_request(:post, tenant.kc_token_url) + .to_return(status: 500, body: 'Internal Server Error', headers: {}) - expect(described_class.new(tenant: tenant, secret: 'server-secret').call).to be_nil + expect(Rails.logger).to receive(:warn).twice + + expect(described_class.new(tenant: tenant, secret: 'server-secret').call).to be_nil + end end end + + private + + def decrypt(encrypted_text, secret) + decoded_text = Base64.strict_decode64(encrypted_text) + + salt = decoded_text[8, 8] + ciphertext = decoded_text[16..] + + key_iv_bytes = OpenSSL::PKCS5.pbkdf2_hmac(secret, salt, 10_000, 48, 'sha256') + key = key_iv_bytes[0, 32] + iv = key_iv_bytes[32..] + + decipher = OpenSSL::Cipher.new('aes-256-cbc') + decipher.decrypt + decipher.key = key + decipher.iv = iv + + decipher.update(ciphertext) + decipher.final + end end From 40a673a2552f8fdaac69d7f56bd66687a35718d8 Mon Sep 17 00:00:00 2001 From: Ahmad Farhat Date: Fri, 7 Jun 2024 15:16:36 -0400 Subject: [PATCH 4/7] Changes to LRS Basic Authentication (#1080) --- app/models/tenant.rb | 10 ++++++---- app/services/lrs_payload_service.rb | 22 ++++++++++++++-------- lib/tasks/tenants.rake | 15 +++++++++------ spec/factories/tenant.rb | 3 ++- spec/services/lrs_payload_service_spec.rb | 15 ++++----------- 5 files changed, 35 insertions(+), 30 deletions(-) diff --git a/app/models/tenant.rb b/app/models/tenant.rb index 4244b8cf..39bbf86e 100644 --- a/app/models/tenant.rb +++ b/app/models/tenant.rb @@ -3,8 +3,8 @@ class Tenant < ApplicationRedisRecord SECRETS_SEPARATOR = ':' - define_attribute_methods :id, :name, :secrets, :lrs_endpoint, :lrs_basic_token, :kc_token_url, :kc_client_id, :kc_client_secret, :kc_username, -:kc_password + define_attribute_methods :id, :name, :secrets, :lrs_endpoint, :lrs_username, :lrs_password, + :kc_token_url, :kc_client_id, :kc_client_secret, :kc_username, :kc_password # Unique ID for this tenant application_redis_attr :id @@ -17,7 +17,8 @@ class Tenant < ApplicationRedisRecord # Custom LRS work application_redis_attr :lrs_endpoint - application_redis_attr :lrs_basic_token + application_redis_attr :lrs_username + application_redis_attr :lrs_password application_redis_attr :kc_token_url application_redis_attr :kc_client_id application_redis_attr :kc_client_secret @@ -45,7 +46,8 @@ def save! pipeline.hset(id_key, 'name', name) if name_changed? pipeline.hset(id_key, 'secrets', secrets) if secrets_changed? pipeline.hset(id_key, 'lrs_endpoint', lrs_endpoint) if lrs_endpoint_changed? - pipeline.hset(id_key, 'lrs_basic_token', lrs_basic_token) if lrs_basic_token_changed? + pipeline.hset(id_key, 'lrs_username', lrs_username) if lrs_username_changed? + pipeline.hset(id_key, 'lrs_password', lrs_password) if lrs_password_changed? pipeline.hset(id_key, 'kc_token_url', kc_token_url) if kc_token_url_changed? pipeline.hset(id_key, 'kc_client_id', kc_client_id) if kc_client_id_changed? pipeline.hset(id_key, 'kc_client_secret', kc_client_secret) if kc_client_secret_changed? diff --git a/app/services/lrs_payload_service.rb b/app/services/lrs_payload_service.rb index 1233dded..97954a43 100644 --- a/app/services/lrs_payload_service.rb +++ b/app/services/lrs_payload_service.rb @@ -7,18 +7,24 @@ def initialize(tenant:, secret:) end def call - token = @tenant.kc_token_url.present? ? fetch_token_from_keycloak : @tenant.lrs_basic_token - - if token.nil? - Rails.logger.warn("LRS Token not found") - return nil - end - lrs_payload = { lrs_endpoint: @tenant.lrs_endpoint, - lrs_token: token } + if @tenant.lrs_username.present? + lrs_payload[:lrs_username] = @tenant.lrs_username + lrs_payload[:lrs_password] = @tenant.lrs_password + else + token = fetch_token_from_keycloak + + if token.nil? + Rails.logger.warn("LRS Token not found") + return nil + end + + lrs_payload[:lrs_token] = token + end + # Generate a random salt salt = SecureRandom.random_bytes(8) diff --git a/lib/tasks/tenants.rake b/lib/tasks/tenants.rake index 91563b66..523edec6 100644 --- a/lib/tasks/tenants.rake +++ b/lib/tasks/tenants.rake @@ -15,7 +15,8 @@ task tenants: :environment do |_t, _args| puts("\tname: #{tenant.name}") puts("\tsecrets: #{tenant.secrets}") puts("\tlrs_endpoint: #{tenant.lrs_endpoint}") if tenant.lrs_endpoint.present? - puts("\tlrs_basic_token: #{tenant.lrs_basic_token}") if tenant.lrs_basic_token.present? + puts("\tlrs_username: #{tenant.lrs_username}") if tenant.lrs_username.present? + puts("\tlrs_password: #{tenant.lrs_password}") if tenant.lrs_password.present? puts("\tkc_token_url: #{tenant.kc_token_url}") if tenant.kc_token_url.present? puts("\tkc_client_id: #{tenant.kc_client_id}") if tenant.kc_client_id.present? puts("\tkc_client_secret: #{tenant.kc_client_secret}") if tenant.kc_client_secret.present? @@ -68,20 +69,22 @@ namespace :tenants do end desc 'Update an existing Tenants LRS credentials with basic authentication' - task :update_lrs_basic, [:id, :lrs_endpoint, :lrs_basic_token] => :environment do |_t, args| + task :update_lrs_basic, [:id, :lrs_endpoint, :lrs_username, :lrs_password] => :environment do |_t, args| check_multitenancy id = args[:id] lrs_endpoint = args[:lrs_endpoint] - lrs_basic_token = args[:lrs_basic_token] + lrs_username = args[:lrs_username] + lrs_password = args[:lrs_password] - if id.blank? || lrs_endpoint.blank? || lrs_basic_token.blank? - puts('Error: id, LRS_ENDPOINT, LRS_BASIC_TOKEN are required to update a Tenant') + if id.blank? || lrs_endpoint.blank? || lrs_username.blank? || lrs_password.blank? + puts('Error: id, LRS_ENDPOINT, LRS_USERNAME, LRS_PASSWORD are required to update a Tenant') exit(1) end tenant = Tenant.find(id) tenant.lrs_endpoint = lrs_endpoint - tenant.lrs_basic_token = lrs_basic_token + tenant.lrs_username = lrs_username + tenant.lrs_password = lrs_password tenant.save! diff --git a/spec/factories/tenant.rb b/spec/factories/tenant.rb index 7af50c8e..dbacd596 100644 --- a/spec/factories/tenant.rb +++ b/spec/factories/tenant.rb @@ -5,7 +5,8 @@ name { Faker::Creature::Animal.name } secrets { "#{Faker::Crypto.sha256}:#{Faker::Crypto.sha512}" } lrs_endpoint { nil } - lrs_basic_token { nil } + lrs_username { nil } + lrs_password { nil } kc_token_url { nil } kc_client_id { nil } kc_client_secret { nil } diff --git a/spec/services/lrs_payload_service_spec.rb b/spec/services/lrs_payload_service_spec.rb index bbf53cf2..2be525aa 100644 --- a/spec/services/lrs_payload_service_spec.rb +++ b/spec/services/lrs_payload_service_spec.rb @@ -5,20 +5,13 @@ RSpec.describe LrsPayloadService, type: :service do describe '#call' do context 'Basic Auth' do - it 'uses the lrs_basic_token if set' do - tenant = create(:tenant, name: 'bn', lrs_endpoint: 'https://lrs_endpoint.com', lrs_basic_token: 'basic_token') + it 'uses the lrs_username and lrs_password if set' do + tenant = create(:tenant, name: 'bn', lrs_endpoint: 'https://lrs_endpoint.com', lrs_username: 'basic_username', lrs_password: 'basic_password') encrypted_value = described_class.new(tenant: tenant, secret: 'server-secret').call - expect(JSON.parse(decrypt(encrypted_value, 'server-secret'))["lrs_token"]).to eq(tenant.lrs_basic_token) - end - - it 'logs a warning and returns nil if lrs_basic_token is not set' do - tenant = create(:tenant, name: 'bn', lrs_endpoint: 'https://lrs_endpoint.com') - - expect(Rails.logger).to receive(:warn) - - expect(described_class.new(tenant: tenant, secret: 'server-secret').call).to be_nil + expect(JSON.parse(decrypt(encrypted_value, 'server-secret'))["lrs_username"]).to eq(tenant.lrs_username) + expect(JSON.parse(decrypt(encrypted_value, 'server-secret'))["lrs_password"]).to eq(tenant.lrs_password) end end From b3aa5e6376ca83968e887bea729c4b0c0617a697 Mon Sep 17 00:00:00 2001 From: Ahmad Farhat Date: Tue, 25 Jun 2024 17:08:09 -0400 Subject: [PATCH 5/7] Added LRS details to docs (#1092) --- docs/rake-README.md | 224 +++++++++++++++++++++++++------------------- 1 file changed, 127 insertions(+), 97 deletions(-) diff --git a/docs/rake-README.md b/docs/rake-README.md index 344c6b9e..033dd088 100644 --- a/docs/rake-README.md +++ b/docs/rake-README.md @@ -4,103 +4,6 @@ Scalelite comes with a set of rake tasks that allow for the management of tenant In a Docker deployment, these should be run from in the Docker container. You can enter the Docker container using a command like `docker exec -it scalelite-api /bin/sh` -## Rake Tasks for Tenant Management - -### Show Tenants -```sh -./bin/rake tenants -``` - -When you run this command, Scalelite will return a list of all tenants, along with their IDs, names, and secrets. For example: - -``` -id: 9a870f45-ec23-4d29-828b-4673f3536d7b - name: tenant1 - secrets: secret1 -id: 4f3e4bb8-2a4e-41a6-9af8-0678c651777f - name: tenant2 - secrets: secret2:secret2a:secret2b -``` - -### Add Tenant -```sh -./bin/rake tenants:add[id,secrets] -``` - -If you need to add multiple secrets for a tenant, you can provide a colon-separated (`:`) list of secrets when creating the tenant in Scalelite. - -When you run this command, Scalelite will print out the ID of the newly created tenant, followed by `OK` if the operation was successful. - -### Update Tenant -```sh -./bin/rake tenants:update[id,name,secrets] -``` - -You can update an existing tenants name or secrets using this rake command. - -When you run this command, Scalelite will print out the ID of the updated tenant, followed by `OK` if the operation was successful. - -### Remove Tenant -```sh -./bin/rake tenants:remove[id] -``` - -Warning: Removing a tenant with data still in the database may cause some inconsistencies. - -### Associate Old Recordings with a Tenant -```sh -./bin/rake recordings:addToTenant[tenant-id] -``` - -If you are switching over from single-tenancy to multitenancy, the existing recordings will have to be transferred to the new tenant. The above task updates the recordings' metadata with the tenant id. - -## Rake Tasks for Tenant Settings Management -If you have enabled multitenancy for your Scalelite deployment, you gain the ability to customize the parameters passed into the `create` and `join` calls on a per-tenant basis. This functionality empowers you to tailor the user experience according to the specific needs and preferences of each tenant. - -By customizing these parameters for each tenant, you can modify various aspects of the meeting experience, such as recording settings, welcome messages, and lock settings, among others. This level of customization ensures that each tenant receives a unique and tailored experience within the Scalelite platform. - -### Show Tenant Settings -```sh -./bin/rake tenantSettings -``` - -When you run this command, Scalelite will return a list of all settings for all tenants. For example: - -``` -Tenant: tenant1 - id: 18dcd4eb-769b-4c59-a441-5a9f7c0bf209 - param: param1 - value: value1 - override: true -Tenant: tenant2 - id: 9867dd51-9065-4486-9216-afb238a04748 - param: param2 - value: value2 - override: false - id: ac7d7443-3515-4b02-bdcf-6f6452a3e00a - param: param3 - value: value3 - override: true -``` - -### Add Tenant Setting -```sh -./bin/rake tenantSettings:add[tenant_id,param,value,override] -``` - -To add a new TenantSetting, Scalelite requires 4 values: -1. `tenant_id`: This is the unique identifier of the tenant to which you want to add the setting. -2. `param`: Specify the name of the parameter you wish to set. For example, you can use values like record, welcome, or lockSettingsLockOnJoin. To view a comprehensive list of available options, refer to the [create](https://docs.bigbluebutton.org/development/api#create) and [join](https://docs.bigbluebutton.org/development/api#join) documentation. -3. `value` -> Assign the desired value to the parameter you specified. It can be a boolean value like 'true' or 'false', a numeric value like '5' or a string like 'Welcome to BigBlueButton'. -4. `override` -> This field should be set to either 'true' or 'false'. If set to 'true', the provided value will override any value passed by the person making the create/join call. If set to 'false', the value will only be applied if the user making the create/join call does not provide any value for the specified parameter. - -When you run this command, Scalelite will print out the ID of the newly created setting, followed by `OK` if the operation was successful. - -### Remove Tenant Setting -```sh -./bin/rake tenantSettings:remove[id] -``` - ## Rake Tasks for Server Management ### Show configured server details @@ -523,3 +426,130 @@ Tenant: tenant2 value: value3 override: true ``` + + +## Rake Tasks for Tenant Management + +### Show Tenants +```sh +./bin/rake tenants +``` + +When you run this command, Scalelite will return a list of all tenants, along with their IDs, names, and secrets. For example: + +``` +id: 9a870f45-ec23-4d29-828b-4673f3536d7b + name: tenant1 + secrets: secret1 +id: 4f3e4bb8-2a4e-41a6-9af8-0678c651777f + name: tenant2 + secrets: secret2:secret2a:secret2b +``` + +### Add Tenant +```sh +./bin/rake tenants:add[id,secrets] +``` + +If you need to add multiple secrets for a tenant, you can provide a colon-separated (`:`) list of secrets when creating the tenant in Scalelite. + +When you run this command, Scalelite will print out the ID of the newly created tenant, followed by `OK` if the operation was successful. + +### Update Tenant +```sh +./bin/rake tenants:update[id,name,secrets] +``` + +You can update an existing tenants name or secrets using this rake command. + +When you run this command, Scalelite will print out the ID of the updated tenant, followed by `OK` if the operation was successful. + +### Update Tenant xAPI + +Scalelite supports setting xAPI credentials on a per-tenant basis, which is passed to BigBlueButton. Scalelite currently supports 2 options: Basic Auth and Dynamic Bearer Auth. + +**IMPORTANT NOTE:** xAPI functionality requires bbb-webhooks version [3.2.0](https://github.com/bigbluebutton/bbb-webhooks/releases/tag/v3.2.0) or greater to be installed on your BigBlueButton Server + +#### LRS Credentials (Basic Authentication) +```sh +./bin/rake tenants:update_lrs_basic[id,lrs_endpoint,lrs_username,lrs_password] +``` + +To enable xAPI using Basic Authentication, set the following variables: +1. `lrs_endpoint`: The endpoint of the LRS system +2. `lrs_username`: The username to be used to authenticate against the LRS system +3. `lrs_password`: The password to be used to authenticate against the LRS system + +#### Keycloak Credentials (Bearer Authentication) +```sh +./bin/rake tenants:update_lrs_kc[id,lrs_endpoint,kc_token_url,kc_client_id,kc_client_secret,kc_username,kc_password] +``` +To dynamically fetch the token from any OIDC Provider (ex: Keycloak), you can enable xAPI support by setting the following variables: +1. `lrs_endpoint`: The endpoint of the LRS system +1. `kc_token_url`: The full url to fetch the token from Keycloak (should be in the format https://{KEYCLOAK_URL}/realms/{REALM_NAME}/protocol/openid-connect/token) +1. `kc_client_id`: The keycloak client id +1. `kc_client_secret`: The keycloak client secret +1. `kc_client_username`: The keycloak username that has access to fetch the token +1. `kc_client_password`: The keycloak password for the above user + + +### Remove Tenant +```sh +./bin/rake tenants:remove[id] +``` + +Warning: Removing a tenant with data still in the database may cause some inconsistencies. + +### Associate Old Recordings with a Tenant +```sh +./bin/rake recordings:addToTenant[tenant-id] +``` + +If you are switching over from single-tenancy to multitenancy, the existing recordings will have to be transferred to the new tenant. The above task updates the recordings' metadata with the tenant id. + +## Rake Tasks for Tenant Settings Management +If you have enabled multitenancy for your Scalelite deployment, you gain the ability to customize the parameters passed into the `create` and `join` calls on a per-tenant basis. This functionality empowers you to tailor the user experience according to the specific needs and preferences of each tenant. + +By customizing these parameters for each tenant, you can modify various aspects of the meeting experience, such as recording settings, welcome messages, and lock settings, among others. This level of customization ensures that each tenant receives a unique and tailored experience within the Scalelite platform. + +### Show Tenant Settings +```sh +./bin/rake tenantSettings +``` + +When you run this command, Scalelite will return a list of all settings for all tenants. For example: + +``` +Tenant: tenant1 + id: 18dcd4eb-769b-4c59-a441-5a9f7c0bf209 + param: param1 + value: value1 + override: true +Tenant: tenant2 + id: 9867dd51-9065-4486-9216-afb238a04748 + param: param2 + value: value2 + override: false + id: ac7d7443-3515-4b02-bdcf-6f6452a3e00a + param: param3 + value: value3 + override: true +``` + +### Add Tenant Setting +```sh +./bin/rake tenantSettings:add[tenant_id,param,value,override] +``` + +To add a new TenantSetting, Scalelite requires 4 values: +1. `tenant_id`: This is the unique identifier of the tenant to which you want to add the setting. +2. `param`: Specify the name of the parameter you wish to set. For example, you can use values like record, welcome, or lockSettingsLockOnJoin. To view a comprehensive list of available options, refer to the [create](https://docs.bigbluebutton.org/development/api#create) and [join](https://docs.bigbluebutton.org/development/api#join) documentation. +3. `value` -> Assign the desired value to the parameter you specified. It can be a boolean value like 'true' or 'false', a numeric value like '5' or a string like 'Welcome to BigBlueButton'. +4. `override` -> This field should be set to either 'true' or 'false'. If set to 'true', the provided value will override any value passed by the person making the create/join call. If set to 'false', the value will only be applied if the user making the create/join call does not provide any value for the specified parameter. + +When you run this command, Scalelite will print out the ID of the newly created setting, followed by `OK` if the operation was successful. + +### Remove Tenant Setting +```sh +./bin/rake tenantSettings:remove[id] +``` \ No newline at end of file From bd501cd32b86b8060c3576a50b0f89f1cde061db Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Thu, 27 Jun 2024 21:25:58 +0200 Subject: [PATCH 6/7] Prepare RSpec and rubocop.yml for merge of v1.5.2 (#1071) * add ContentType for POST requests in server-tag related RSpec * increase rubocop PerceivedComplexity limit 20 * Update .rubocop.yml --------- Co-authored-by: Ahmad Farhat Co-authored-by: Ahmad Farhat --- .rubocop.yml | 2 +- spec/requests/servers_controller_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d939fb4b..8e4b23bd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -131,7 +131,7 @@ Metrics/BlockLength: # A complexity metric geared towards measuring complexity for a human reader. Metrics/PerceivedComplexity: - Max: 17 + Max: 21 Exclude: - app/models/recording.rb - app/models/server.rb diff --git a/spec/requests/servers_controller_spec.rb b/spec/requests/servers_controller_spec.rb index 1babd4a1..af402550 100644 --- a/spec/requests/servers_controller_spec.rb +++ b/spec/requests/servers_controller_spec.rb @@ -149,7 +149,7 @@ context 'when updating tag' do it 'updates the server tag' do server = create(:server) - post scalelite_api_update_server_url, params: { id: server.id, server: { tag: 'test-tag' } } + post scalelite_api_update_server_url, params: { id: server.id, server: { tag: 'test-tag' } }, as: :json updated_server = Server.find(server.id) # Reload expect(updated_server.tag).to eq("test-tag") expect(response).to have_http_status(:ok) @@ -159,10 +159,10 @@ it 'updates the server tag back to nil' do server = create(:server) - post scalelite_api_update_server_url, params: { id: server.id, server: { tag: 'test-tag' } } + post scalelite_api_update_server_url, params: { id: server.id, server: { tag: 'test-tag' } }, as: :json updated_server = Server.find(server.id) # Reload expect(updated_server.tag).to eq("test-tag") - post scalelite_api_update_server_url, params: { id: server.id, server: { tag: '' } } + post scalelite_api_update_server_url, params: { id: server.id, server: { tag: '' } }, as: :json updated_server = Server.find(server.id) # Reload expect(updated_server.tag).to be_nil expect(response).to have_http_status(:ok) From 2a30c7f9e930d2f685066665daffc79eeeaa6383 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:28:42 -0400 Subject: [PATCH 7/7] Bump rexml from 3.2.5 to 3.2.8 (#1068) Bumps [rexml](https://github.com/ruby/rexml) from 3.2.5 to 3.2.8. - [Release notes](https://github.com/ruby/rexml/releases) - [Changelog](https://github.com/ruby/rexml/blob/master/NEWS.md) - [Commits](https://github.com/ruby/rexml/compare/v3.2.5...v3.2.8) --- updated-dependencies: - dependency-name: rexml dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ahmad Farhat --- Gemfile.lock | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 9d7cf039..bf332f88 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -177,7 +177,8 @@ GEM redis-namespace (1.11.0) redis (>= 4) regexp_parser (2.7.0) - rexml (3.2.5) + rexml (3.2.8) + strscan (>= 3.0.9) rspec-core (3.12.1) rspec-support (~> 3.12.0) rspec-expectations (3.12.2) @@ -233,6 +234,7 @@ GEM sprockets (>= 3.0.0) sqlite3 (1.6.2) mini_portile2 (~> 2.8.0) + strscan (3.1.0) tabulo (2.8.2) tty-screen (= 0.8.1) unicode-display_width (~> 2.2)