From ea66bcdc75ff260702ae59c1c36af8386d0a348d Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Fri, 21 Jan 2022 07:15:04 +0300 Subject: [PATCH] FEATURE: Add an API scope for running queries (#154) --- .../data_explorer/query_controller.rb | 7 ++ config/locales/client.en.yml | 6 ++ plugin.rb | 11 +- .../integration/custom_api_key_scopes_spec.rb | 102 ++++++++++++++++++ 4 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 spec/integration/custom_api_key_scopes_spec.rb diff --git a/app/controllers/data_explorer/query_controller.rb b/app/controllers/data_explorer/query_controller.rb index 644e46d..74bff9d 100644 --- a/app/controllers/data_explorer/query_controller.rb +++ b/app/controllers/data_explorer/query_controller.rb @@ -5,7 +5,14 @@ class DataExplorer::QueryController < ::ApplicationController before_action :set_group, only: %i(group_reports_index group_reports_show group_reports_run) before_action :set_query, only: %i(group_reports_show group_reports_run show update) + before_action :ensure_admin + skip_before_action :check_xhr, only: %i(show group_reports_run run) + skip_before_action :ensure_admin, only: %i( + group_reports_index + group_reports_show + group_reports_run + ) def index queries = DataExplorer::Query.where(hidden: false).order(:last_run_at, :name).includes(:groups).to_a diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5b73d30..9ff757b 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -90,3 +90,9 @@ en: no_search_results: "Sorry, we couldn't find any results matching your text." group: reports: "Reports" + admin: + api: + scopes: + descriptions: + data_explorer: + run_queries: "Run Data Explorer queries. Restrict the API key to a set of queries by specifying queries IDs." diff --git a/plugin.rb b/plugin.rb index 7517ebe..ac60f41 100644 --- a/plugin.rb +++ b/plugin.rb @@ -886,7 +886,7 @@ SQL get 'queries/:id' => "query#show" put 'queries/:id' => "query#update" delete 'queries/:id' => "query#destroy" - post 'queries/:id/run' => "query#run" + post 'queries/:id/run' => "query#run", constraints: { format: /(json|csv)/ } end Discourse::Application.routes.append do @@ -894,6 +894,13 @@ SQL get '/g/:group_name/reports/:id' => 'data_explorer/query#group_reports_show' post '/g/:group_name/reports/:id/run' => 'data_explorer/query#group_reports_run' - mount ::DataExplorer::Engine, at: '/admin/plugins/explorer', constraints: AdminConstraint.new + mount ::DataExplorer::Engine, at: '/admin/plugins/explorer' end + + add_api_key_scope(:data_explorer, { + run_queries: { + actions: %w[data_explorer/query#run], + params: %i[id] + } + }) end diff --git a/spec/integration/custom_api_key_scopes_spec.rb b/spec/integration/custom_api_key_scopes_spec.rb new file mode 100644 index 0000000..6cbc8a6 --- /dev/null +++ b/spec/integration/custom_api_key_scopes_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'API keys scoped to query#run' do + before do + SiteSetting.data_explorer_enabled = true + end + + fab!(:query1) { DataExplorer::Query.create!(name: "Query 1", sql: "SELECT 1 AS query1_res") } + fab!(:query2) { DataExplorer::Query.create!(name: "Query 2", sql: "SELECT 1 AS query2_res") } + fab!(:admin) { Fabricate(:admin) } + + let(:all_queries_api_key) do + key = ApiKey.create! + ApiKeyScope.create!( + resource: "data_explorer", + action: "run_queries", + api_key_id: key.id + ) + key + end + + let(:single_query_api_key) do + key = ApiKey.create! + ApiKeyScope.create!( + resource: "data_explorer", + action: "run_queries", + api_key_id: key.id, + allowed_parameters: { "id" => [query1.id.to_s] } + ) + key + end + + it 'cannot hit any other endpoints' do + get "/latest.json", headers: { + "Api-Key" => all_queries_api_key.key, + "Api-Username" => admin.username + } + expect(response.status).to eq(403) + + get "/latest.json", headers: { + "Api-Key" => single_query_api_key.key, + "Api-Username" => admin.username + } + expect(response.status).to eq(403) + + get "/u/#{admin.username}.json", headers: { + "Api-Key" => all_queries_api_key.key, + "Api-Username" => admin.username + } + expect(response.status).to eq(403) + + get "/u/#{admin.username}.json", headers: { + "Api-Key" => single_query_api_key.key, + "Api-Username" => admin.username + } + expect(response.status).to eq(403) + end + + it "can only run the queries they're allowed to run" do + expect { + post "/admin/plugins/explorer/queries/#{query1.id}/run.json", headers: { + "Api-Key" => single_query_api_key.key, + "Api-Username" => admin.username + } + }.to change { query1.reload.last_run_at } + expect(response.status).to eq(200) + expect(response.parsed_body["success"]).to eq(true) + expect(response.parsed_body["columns"]).to eq(["query1_res"]) + + expect { + post "/admin/plugins/explorer/queries/#{query2.id}/run.json", headers: { + "Api-Key" => single_query_api_key.key, + "Api-Username" => admin.username + } + }.not_to change { query2.reload.last_run_at } + expect(response.status).to eq(403) + end + + it "can run all queries if they're not restricted to any queries" do + expect { + post "/admin/plugins/explorer/queries/#{query1.id}/run.json", headers: { + "Api-Key" => all_queries_api_key.key, + "Api-Username" => admin.username + } + }.to change { query1.reload.last_run_at } + expect(response.status).to eq(200) + expect(response.parsed_body["success"]).to eq(true) + expect(response.parsed_body["columns"]).to eq(["query1_res"]) + + expect { + post "/admin/plugins/explorer/queries/#{query2.id}/run.json", headers: { + "Api-Key" => all_queries_api_key.key, + "Api-Username" => admin.username + } + }.to change { query2.reload.last_run_at } + expect(response.status).to eq(200) + expect(response.parsed_body["success"]).to eq(true) + expect(response.parsed_body["columns"]).to eq(["query2_res"]) + end +end