From bab1890623113c59c87f63f8c344e5effd3ecbcf Mon Sep 17 00:00:00 2001 From: Achilleas Buisman Date: Mon, 11 Sep 2023 23:07:29 +0200 Subject: [PATCH] Replace api key with token per query --- app/controllers/blazer/queries_controller.rb | 11 +++++---- app/models/blazer/query.rb | 6 +++++ lib/blazer/sharing.rb | 17 ++++++------- lib/generators/blazer/templates/install.rb.tt | 1 + test/internal/config/blazer.yml | 4 ++++ test/internal/config/routes.rb | 2 ++ test/internal/db/schema.rb | 1 + test/queries_test.rb | 24 +++++++++++++++---- 8 files changed, 47 insertions(+), 19 deletions(-) diff --git a/app/controllers/blazer/queries_controller.rb b/app/controllers/blazer/queries_controller.rb index 9536fbe1c..c81a31349 100644 --- a/app/controllers/blazer/queries_controller.rb +++ b/app/controllers/blazer/queries_controller.rb @@ -79,11 +79,12 @@ def edit end def share - if params[:token] && params[:query_id] && params[:token] == Blazer.sharing.query_token(params[:query_id]) - run - else - render_forbidden - end + return render_forbidden unless params[:token] && params[:query_id] + + @query = Query.find_by(id: params[:query_id]) if params[:query_id] + return render_forbidden unless @query.correct_token?(params[:token]) + + run end def run diff --git a/app/models/blazer/query.rb b/app/models/blazer/query.rb index f51fd8f96..c6b444a2c 100644 --- a/app/models/blazer/query.rb +++ b/app/models/blazer/query.rb @@ -1,5 +1,7 @@ module Blazer class Query < Record + has_secure_token :secret_token, length: 36 + belongs_to :creator, optional: true, class_name: Blazer.user_class.to_s if Blazer.user_class has_many :checks, dependent: :destroy has_many :dashboard_queries, dependent: :destroy @@ -15,6 +17,10 @@ def to_param [id, name].compact.join("-").gsub("'", "").parameterize end + def correct_token?(token) + ActiveSupport::SecurityUtils.secure_compare(secret_token, token) + end + def friendly_name name.to_s.sub(/\A[#\*]/, "").gsub(/\[.+\]/, "").strip end diff --git a/lib/blazer/sharing.rb b/lib/blazer/sharing.rb index b514a21a9..248a8d4f9 100644 --- a/lib/blazer/sharing.rb +++ b/lib/blazer/sharing.rb @@ -1,10 +1,10 @@ module Blazer class Sharing - attr_accessor :api_key, :path + attr_accessor :path, :enabled - def initialize(api_key: ENV.fetch('BLAZER_DOWNLOAD_API_KEY', nil), path: '/blazer_share') - @api_key = api_key + def initialize(enabled: false, path: '/blazer_share') @path = path.sub(/\/$/, '') # Strip trailing / + @enabled = enabled end def route_path @@ -15,16 +15,13 @@ def to_controller 'blazer/queries#share' end - def query_token(query_id) - Digest::SHA1.hexdigest("#{query_id}-#{api_key}") - end - def enabled? - api_key.present? + enabled end - def share_path(query_id, format: nil) - "#{path}/#{query_token(query_id)}/#{query_id}#{".#{format}" if format}" + def share_path(query_id, format: nil, token: nil) + query = Query.find(query_id) + "#{path}/#{token}/#{query_id}#{".#{format}" if format}" end def url_for(query_id, current_url, format: 'csv') diff --git a/lib/generators/blazer/templates/install.rb.tt b/lib/generators/blazer/templates/install.rb.tt index bf1999303..22220aea0 100644 --- a/lib/generators/blazer/templates/install.rb.tt +++ b/lib/generators/blazer/templates/install.rb.tt @@ -5,6 +5,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration<%= migration_version t.string :name t.text :description t.text :statement + t.text :secret_token t.string :data_source t.string :status t.timestamps null: false diff --git a/test/internal/config/blazer.yml b/test/internal/config/blazer.yml index 07c7b09d6..d3b55d652 100644 --- a/test/internal/config/blazer.yml +++ b/test/internal/config/blazer.yml @@ -161,3 +161,7 @@ uploads: url: postgres://localhost/blazer_test schema: uploads data_source: main + +sharing: + path: /blazer_share + enabled: true diff --git a/test/internal/config/routes.rb b/test/internal/config/routes.rb index fc4c62b3e..d0932527b 100644 --- a/test/internal/config/routes.rb +++ b/test/internal/config/routes.rb @@ -1,3 +1,5 @@ Rails.application.routes.draw do mount Blazer::Engine, at: "/" + + get Blazer.sharing.route_path, to: Blazer.sharing.to_controller, as: :share_query if Blazer.sharing.enabled? end diff --git a/test/internal/db/schema.rb b/test/internal/db/schema.rb index 6d6b43035..5ef6b772a 100644 --- a/test/internal/db/schema.rb +++ b/test/internal/db/schema.rb @@ -4,6 +4,7 @@ t.string :name t.text :description t.text :statement + t.text :secret_token t.string :data_source t.string :status t.timestamps null: false diff --git a/test/queries_test.rb b/test/queries_test.rb index bd78a219f..3cf99f387 100644 --- a/test/queries_test.rb +++ b/test/queries_test.rb @@ -76,6 +76,22 @@ def test_variables_time_range assert_match "daterangepicker", response.body end + def test_correct_token + query = create_query(statement: "SELECT 1") + get share_query_path(query.id, token: query.secret_token, format: 'csv') + + assert_response :success + assert_equal "text/csv", response.content_type + end + + def test_incorrect_token + query = create_query(statement: "SELECT 1") + get share_query_path(query.id, token: "x") + + assert_response :forbidden + assert_match "Access denied", response.body + end + def test_variable_defaults query = create_query(statement: "SELECT {default_var}") get blazer.query_path(query) @@ -108,12 +124,12 @@ def test_csv end def test_share - Blazer.sharing.api_key = "123" query = create_query - get blazer.query_share_path(query_id: query.id, token: Digest::SHA1.hexdigest("#{query.id}-123"), format: 'csv') + assert query.secret_token + + get blazer.query_share_path(query_id: query.id, token: query.secret_token, format: 'csv') + assert_response :success - assert_match query.name, response.body - Blazer.sharing.api_key = nil end def test_url