From a87b1a74dfc9b34a20e65ebf1f127d0221a654d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Wed, 6 Nov 2024 18:04:53 +0100 Subject: [PATCH 1/2] Sign the CMS token --- .../admin/cms/visit_portal_controller.rb | 28 +++++++++++++++++ app/helpers/cms/url_helper.rb | 8 ++--- app/helpers/vertical_nav_helper.rb | 2 +- app/lib/cms/signature.rb | 11 +++++++ config/routes.rb | 1 + .../access_codes_controller.rb | 2 +- .../app/views/shared/cms/_toolbar.html.slim | 2 +- lib/developer_portal/lib/cms/toolbar.rb | 30 +++++++++++++++++-- 8 files changed, 72 insertions(+), 12 deletions(-) create mode 100644 app/controllers/provider/admin/cms/visit_portal_controller.rb create mode 100644 app/lib/cms/signature.rb diff --git a/app/controllers/provider/admin/cms/visit_portal_controller.rb b/app/controllers/provider/admin/cms/visit_portal_controller.rb new file mode 100644 index 0000000000..a8371ef95a --- /dev/null +++ b/app/controllers/provider/admin/cms/visit_portal_controller.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class Provider::Admin::CMS::VisitPortalController < Provider::Admin::CMS::BaseController + # Encrypt the CMS token under a temporary SSO token and redirect to the Developer Portal + def with_token + cms_token = current_account.settings.cms_token! + expires_at = Time.now.utc.round + 1.minute + signature = CMS::Signature.generate(cms_token, expires_at) + + redirect_to access_code_url( + host: current_account.external_domain, + signature:, + expires_at: expires_at.to_i, + access_code: current_account.site_access_code, + return_to:, + cms: cms_mode) + end + + private + + def return_to + params.permit(:return_to)[:return_to] + end + + def cms_mode + session[:cms] + end +end diff --git a/app/helpers/cms/url_helper.rb b/app/helpers/cms/url_helper.rb index 3e06e06344..397e4d2f5e 100644 --- a/app/helpers/cms/url_helper.rb +++ b/app/helpers/cms/url_helper.rb @@ -21,12 +21,8 @@ def cms_published_url(page) }.freeze def cms_uri(page) - uri = URI.parse(access_code_url) - uri.host = page.provider.external_domain - uri.query = { return_to: page.path ? page.path : HARD_WIRED_PATHS[page.system_name], - access_code: current_account.site_access_code, - cms_token: page.provider.settings.cms_token! }.to_query - uri.port = request.port if Rails.env.development? + uri = URI.parse(provider_admin_cms_visit_portal_path) + uri.query = { return_to: page.path ? page.path : HARD_WIRED_PATHS[page.system_name]}.to_query uri end diff --git a/app/helpers/vertical_nav_helper.rb b/app/helpers/vertical_nav_helper.rb index 4e2dfea30d..36080e72d8 100644 --- a/app/helpers/vertical_nav_helper.rb +++ b/app/helpers/vertical_nav_helper.rb @@ -150,7 +150,7 @@ def audience_portal_items # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticC end items << {id: 'separator 0'} # Separator - items << {title: 'Visit Portal', path: access_code_url(host: current_account.external_domain, cms_token: current_account.settings.cms_token!, access_code: current_account.site_access_code).html_safe, target: '_blank'} + items << {title: 'Visit Portal', path: provider_admin_cms_visit_portal_path.html_safe, target: '_blank'} items << {id: 'separator 1'} # Separator if can?(:manage, :portal) diff --git a/app/lib/cms/signature.rb b/app/lib/cms/signature.rb new file mode 100644 index 0000000000..14d7c5cfbe --- /dev/null +++ b/app/lib/cms/signature.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module CMS + class Signature + def self.generate(token, expires_at) + verifier = Rails.application.message_verifier(:cms_token) + signing_options = { purpose: :cms_edit_mode, expires_at: expires_at.utc.floor } + verifier.generate(token.b, **signing_options).split("--").last + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 4631ed3176..f7cf8f538f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -354,6 +354,7 @@ end end + get 'visit_portal' => 'visit_portal#with_token' end namespace :user do diff --git a/lib/developer_portal/app/controllers/developer_portal/access_codes_controller.rb b/lib/developer_portal/app/controllers/developer_portal/access_codes_controller.rb index f27dded8cc..2b21a5c1f3 100644 --- a/lib/developer_portal/app/controllers/developer_portal/access_codes_controller.rb +++ b/lib/developer_portal/app/controllers/developer_portal/access_codes_controller.rb @@ -30,7 +30,7 @@ def show private def cms_params - params.permit(:cms_token, :cms) + params.permit(:signature, :expires_at, :cms) end def return_url diff --git a/lib/developer_portal/app/views/shared/cms/_toolbar.html.slim b/lib/developer_portal/app/views/shared/cms/_toolbar.html.slim index fa51dcf1b3..0537fe686f 100644 --- a/lib/developer_portal/app/views/shared/cms/_toolbar.html.slim +++ b/lib/developer_portal/app/views/shared/cms/_toolbar.html.slim @@ -34,7 +34,7 @@ div class="pf-c-drawer pf-m-inline #{'pf-m-expanded' unless hidden}" div class="pf-c-drawer__actions" id="cms-toolbar-menu-right" div class="pf-c-drawer__close" - unless draft - a class="pf-c-button pf-m-plain" type="button" href=url_for(request.query_parameters.merge(cms_token: "")) title="Close the CMS toolbar" + a class="pf-c-button pf-m-plain" type="button" href=url_for(request.query_parameters.merge(signature: "")) title="Close the CMS toolbar" i class="fa fa-times-circle" aria-hidden="true" div class="pf-c-drawer__body" diff --git a/lib/developer_portal/lib/cms/toolbar.rb b/lib/developer_portal/lib/cms/toolbar.rb index fe041d3ee1..a51a4e8573 100644 --- a/lib/developer_portal/lib/cms/toolbar.rb +++ b/lib/developer_portal/lib/cms/toolbar.rb @@ -17,15 +17,15 @@ def cms_toolbar protected def handle_cms_token - token = params.delete(:cms_token) + token = validate_and_extract_cms_token if cms.valid_token?(token) session[:cms_token] = token - Rails.logger.info "CMS edit mode enabled" + Rails.logger.info "CMS edit mode enabled for portal #{site_account.external_domain}" elsif token session[:cms_token] = nil session[:cms] = nil - Rails.logger.info "Invalid CMS edit mode token." + Rails.logger.info "Invalid CMS edit mode signature for portal #{site_account.external_domain}" end if (mode = params.delete(:cms).presence) @@ -39,6 +39,30 @@ def draft? private + def validate_and_extract_cms_token + signature = params.delete(:signature) + return signature if signature.blank? + + expires_at = Time.at(params.delete(:expires_at).to_i).utc + raise ActiveSupport::MessageVerifier::InvalidSignature unless expires_at > Time.now.utc + + cms_token = site_account.settings.cms_token! + valid_signature = signature == CMS::Signature.generate(cms_token, expires_at) + + raise ActiveSupport::MessageVerifier::InvalidSignature unless valid_signature + + # We don't need the signature after processing, better remove it to avoid resending it with future redirections + request.query_parameters.delete(:expires_at) + request.query_parameters.delete(:signature) + + cms_token + rescue StandardError + # In the case the signature is invalid or any other problem, I don't think we have to bother the client and + # BugSnag with an exception. Better return an empty token which means "Disable CMS edit mode (hide toolbar)" + flash[:error] = 'Disabling CMS edit mode due to an invalid or expired signature' + '' + end + def cms_toolbar_enabled? return false if @_exception_handled From 80abd96ff1a8f7d3900c5ea4725cfbe6ff1b1251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Lled=C3=B3?= Date: Wed, 6 Nov 2024 18:05:01 +0100 Subject: [PATCH 2/2] CMS Token signature: Update tests --- features/developer_portal/cms_toolbar.feature | 11 ++++++++--- .../developer_portal/cms_toolbar_steps.rb | 10 ++++++++-- test/integration/cms/toolbar_test.rb | 5 ++++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/features/developer_portal/cms_toolbar.feature b/features/developer_portal/cms_toolbar.feature index e016178b20..aaf634b387 100644 --- a/features/developer_portal/cms_toolbar.feature +++ b/features/developer_portal/cms_toolbar.feature @@ -14,7 +14,7 @@ Feature: CMS Toolbar And go to the homepage Then there should not be a CMS toolbar - Scenario: Hide the toolbar + Scenario: Hide the toolbar when seeing drafts When they visit the developer portal in CMS mode And follow "Draft" And press "Toggle toolbar" @@ -27,17 +27,22 @@ Feature: CMS Toolbar But they press "Toggle toolbar" And the cms toolbar should be visible - Scenario: + Scenario: Hide the toolbar when seeing published pages When they visit the developer portal in CMS mode And follow "Published" And follow "Close the CMS toolbar" Then there should not be a CMS toolbar + Scenario: Hide the toolbar when providing an expired signature + When they visit the developer portal in CMS mode with an expired signature + Then there should not be a CMS toolbar + And should see "Invalid or expired signature" + Rule: There is a John Doe admin user Background: When the admin user is John Doe - Scenario: An admin visist de dev portal + Scenario: An admin visits the dev portal When they visit the developer portal in CMS mode Then the cms toolbar should be visible And should see "Templates used on this page" diff --git a/features/step_definitions/developer_portal/cms_toolbar_steps.rb b/features/step_definitions/developer_portal/cms_toolbar_steps.rb index 1d9182d8d1..1d753a549c 100644 --- a/features/step_definitions/developer_portal/cms_toolbar_steps.rb +++ b/features/step_definitions/developer_portal/cms_toolbar_steps.rb @@ -1,9 +1,15 @@ # frozen_string_literal: true -Given "they visit the developer portal in CMS mode" do +Given /^they visit the developer portal in CMS mode(\swith an expired signature)?/ do |token_is_expired| + cms_token = @provider.settings.cms_token! + expires_at = Time.now.utc.round - 30.seconds + expires_at += 1.minute unless token_is_expired + signature = CMS::Signature.generate(cms_token, expires_at) + visit access_code_url(host: @provider.external_domain, cms: 'draft', - cms_token: @provider.settings.cms_token!, + expires_at: expires_at.to_i, + signature:, access_code: @provider.site_access_code) end diff --git a/test/integration/cms/toolbar_test.rb b/test/integration/cms/toolbar_test.rb index 825ece40c9..9848b95ac3 100644 --- a/test/integration/cms/toolbar_test.rb +++ b/test/integration/cms/toolbar_test.rb @@ -8,9 +8,12 @@ def setup end test 'CMS toolbar rendering' do + cms_token = @provider.settings.cms_token! + expires_at = Time.now.utc.round + 1.minute + signature = CMS::Signature.generate(cms_token, expires_at) host! @provider.internal_domain - get "/?cms_token=#{@provider.settings.cms_token!}" + get "/", params: { expires_at: expires_at.to_i, signature: } assert_response :success get '/api_docs/login'