diff --git a/assets/javascripts/discourse/components/group-reports-nav-item.js.es6 b/assets/javascripts/discourse/components/group-reports-nav-item.js.es6 new file mode 100644 index 0000000..6280436 --- /dev/null +++ b/assets/javascripts/discourse/components/group-reports-nav-item.js.es6 @@ -0,0 +1,21 @@ +import { ajax } from "discourse/lib/ajax"; + +export default Ember.Component.extend({ + group: null, + showReportsTab: false, + + checkForReports() { + return ajax(`/g/${this.group.name}/reports`).then(response => { + return this.set("showReportsTab", response.queries.length > 0); + }); + }, + + init(args) { + this.set("group", args.group); + if (this.currentUser.groups.some(g => g.id === this.group.id)) { + // User is a part of the group. Now check if the group has reports + this.checkForReports(); + } + this._super(args); + } +}); diff --git a/assets/javascripts/discourse/components/query-result.js.es6 b/assets/javascripts/discourse/components/query-result.js.es6 index 87d4ff8..d359c00 100644 --- a/assets/javascripts/discourse/components/query-result.js.es6 +++ b/assets/javascripts/discourse/components/query-result.js.es6 @@ -146,6 +146,12 @@ const QueryResultComponent = Ember.Component.extend({ return this.site.get("categoriesById")[id]; }, + download_url() { + return this.group + ? `/g/${this.group.name}/reports/` + : "/admin/plugins/explorer/queries/"; + }, + downloadResult(format) { // Create a frame to submit the form in (?) // to avoid leaving an about:blank behind @@ -161,7 +167,7 @@ const QueryResultComponent = Ember.Component.extend({ form.setAttribute( "action", Discourse.getURL( - "/admin/plugins/explorer/queries/" + + this.download_url() + this.get("query.id") + "/run." + format + diff --git a/assets/javascripts/discourse/controllers/admin-plugins-explorer.js.es6 b/assets/javascripts/discourse/controllers/admin-plugins-explorer.js.es6 index d6bc55f..73cb79b 100644 --- a/assets/javascripts/discourse/controllers/admin-plugins-explorer.js.es6 +++ b/assets/javascripts/discourse/controllers/admin-plugins-explorer.js.es6 @@ -54,6 +54,22 @@ export default Ember.Controller.extend({ return item || NoQuery; }, + @computed("selectedItem", "editing") + selectedGroupNames(selectedItem) { + const groupIds = this.selectedItem.group_ids || []; + const groupNames = groupIds.map(id => { + return this.groupOptions.find(groupOption => groupOption.id == id).name; + }); + return groupNames.join(", "); + }, + + @computed("groups") + groupOptions(groups) { + return groups.arrangedContent.map(g => { + return { id: g.id.toString(), name: g.name }; + }); + }, + @computed("selectedItem", "selectedItem.dirty") othersDirty(selectedItem) { return !!this.model.find(q => q !== selectedItem && q.dirty); @@ -81,6 +97,7 @@ export default Ember.Controller.extend({ this.set("loading", true); if (this.get("selectedItem.description") === "") this.set("selectedItem.description", ""); + return this.selectedItem .save() .then(() => { @@ -183,6 +200,8 @@ export default Ember.Controller.extend({ .then(result => { const query = this.get("selectedItem"); query.setProperties(result.getProperties(Query.updatePropertyNames)); + if (!query.group_ids || !Array.isArray(query.group_ids)) + query.set("group_ids", []); query.markNotDirty(); this.set("editing", false); }) diff --git a/assets/javascripts/discourse/controllers/group-reports-index.js.es6 b/assets/javascripts/discourse/controllers/group-reports-index.js.es6 new file mode 100644 index 0000000..b2573e3 --- /dev/null +++ b/assets/javascripts/discourse/controllers/group-reports-index.js.es6 @@ -0,0 +1,10 @@ +import Query from "discourse/plugins/discourse-data-explorer/discourse/models/query"; +import { ajax } from "discourse/lib/ajax"; +import { + default as computed, + observes +} from "ember-addons/ember-computed-decorators"; + +export default Ember.Controller.extend({ + queries: Ember.computed.alias("model.queries") +}); diff --git a/assets/javascripts/discourse/controllers/group-reports-show.js.es6 b/assets/javascripts/discourse/controllers/group-reports-show.js.es6 new file mode 100644 index 0000000..02f7030 --- /dev/null +++ b/assets/javascripts/discourse/controllers/group-reports-show.js.es6 @@ -0,0 +1,44 @@ +import Query from "discourse/plugins/discourse-data-explorer/discourse/models/query"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import { ajax } from "discourse/lib/ajax"; +import { + default as computed, + observes +} from "ember-addons/ember-computed-decorators"; + +export default Ember.Controller.extend({ + showResults: false, + explain: false, + loading: false, + results: Ember.computed.alias("model.results"), + hasParams: Ember.computed.gt("model.param_info.length", 0), + + actions: { + run() { + this.setProperties({ loading: true, showResults: false }); + ajax(`/g/${this.get("group.name")}/reports/${this.model.id}/run`, { + type: "POST", + data: { + params: JSON.stringify(this.model.params), + explain: this.explain + } + }) + .then(result => { + this.set("results", result); + if (!result.success) { + return; + } + + this.set("showResults", true); + }) + .catch(err => { + if (err.jqXHR && err.jqXHR.status === 422 && err.jqXHR.responseJSON) { + this.set("results", err.jqXHR.responseJSON); + } else { + popupAjaxError(err); + } + }) + .finally(() => this.set("loading", false)); + } + } +}); diff --git a/assets/javascripts/discourse/group-reports-route-map.js.es6 b/assets/javascripts/discourse/group-reports-route-map.js.es6 new file mode 100644 index 0000000..f354996 --- /dev/null +++ b/assets/javascripts/discourse/group-reports-route-map.js.es6 @@ -0,0 +1,9 @@ +export default { + resource: "group", + + map() { + this.route("reports", function() { + this.route("show", { path: "/:query_id" }); + }); + } +}; diff --git a/assets/javascripts/discourse/models/query.js.es6 b/assets/javascripts/discourse/models/query.js.es6 index 3575630..501db9a 100644 --- a/assets/javascripts/discourse/models/query.js.es6 +++ b/assets/javascripts/discourse/models/query.js.es6 @@ -23,7 +23,7 @@ const Query = RestModel.extend({ this.resetParams(); }, - @observes("name", "description", "sql") + @observes("name", "description", "sql", "group_ids") markDirty() { this.set("dirty", true); }, @@ -85,6 +85,7 @@ Query.reopenClass({ "sql", "created_by", "created_at", + "group_ids", "last_run_at" ] }); diff --git a/assets/javascripts/discourse/routes/admin-plugins-explorer.js.es6 b/assets/javascripts/discourse/routes/admin-plugins-explorer.js.es6 index 6b21191..f6db09c 100644 --- a/assets/javascripts/discourse/routes/admin-plugins-explorer.js.es6 +++ b/assets/javascripts/discourse/routes/admin-plugins-explorer.js.es6 @@ -4,19 +4,36 @@ export default Discourse.Route.extend({ controllerName: "admin-plugins-explorer", model() { - const p1 = this.store.findAll("query"); - const p2 = ajax("/admin/plugins/explorer/schema.json", { cache: true }); - return p1 - .then(model => { - model.forEach(query => query.markNotDirty()); + const groupPromise = this.store.findAll("group"); + const schemaPromise = ajax("/admin/plugins/explorer/schema.json", { cache: true }); + const queryPromise = this.store.findAll("query"); - return p2.then(schema => { - return { model, schema }; + return groupPromise + .then(groups => { + let groupNames = {}; + groups.forEach(g => { + groupNames[g.id] = g.name; + }); + return schemaPromise.then(schema => { + return queryPromise.then(model => { + model.forEach(query => { + query.markNotDirty(); + query.set( + "group_names", + query.group_ids + .map(id => groupNames[id]) + .filter(n => n) + .join(", ") + ); + }); + return { model, schema, groups }; + }); }); }) .catch(() => { - p2.catch(() => {}); - return { model: null, schema: null, disallow: true }; + schemaPromise.catch(() => {}); + queryPromise.catch(() => {}); + return { model: null, schema: null, disallow: true, groups: null }; }); }, diff --git a/assets/javascripts/discourse/routes/group-reports-index.js.es6 b/assets/javascripts/discourse/routes/group-reports-index.js.es6 new file mode 100644 index 0000000..2bf6a40 --- /dev/null +++ b/assets/javascripts/discourse/routes/group-reports-index.js.es6 @@ -0,0 +1,38 @@ +import { ajax } from "discourse/lib/ajax"; + +export default Discourse.Route.extend({ + controllerName: "group-reports-index", + + model() { + const group = this.modelFor("group"); + return ajax(`/g/${group.name}/reports`) + .then(queries => { + return { + model: queries, + group: group + }; + }) + .catch(() => { + this.transitionTo("group.members", group); + }); + }, + afterModel(model) { + if ( + !model.group.get("is_group_user") && + !(this.currentUser && this.currentUser.admin) + ) { + this.transitionTo("group.members", group); + } + }, + + setupController(controller, model) { + controller.setProperties(model); + }, + + actions: { + refreshModel() { + this.refresh(); + return false; + } + } +}); diff --git a/assets/javascripts/discourse/routes/group-reports-show.js.es6 b/assets/javascripts/discourse/routes/group-reports-show.js.es6 new file mode 100644 index 0000000..caf8e2b --- /dev/null +++ b/assets/javascripts/discourse/routes/group-reports-show.js.es6 @@ -0,0 +1,30 @@ +import { ajax } from "discourse/lib/ajax"; + +export default Discourse.Route.extend({ + controllerName: "group-reports-show", + + model(params) { + const group = this.modelFor("group"); + return ajax(`/g/${group.name}/reports/${params.query_id}`) + .then(response => { + return { + model: Object.assign({ params: {} }, response.query), + group: group + }; + }) + .catch(err => { + this.transitionTo("group.members", group); + }); + }, + + setupController(controller, model) { + controller.setProperties(model); + }, + + actions: { + refreshModel() { + this.refresh(); + return false; + } + } +}); diff --git a/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs b/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs index 240f317..9a00a54 100644 --- a/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs +++ b/assets/javascripts/discourse/templates/admin/plugins-explorer.hbs @@ -38,6 +38,7 @@
+ {{i18n "explorer.report_name"}} + | ++ {{i18n "explorer.query_description"}} + | ++ {{i18n "explorer.query_time"}} + | + +
---|---|---|
+ {{#link-to 'group.reports.show' group.name query.id}}{{query.name}}{{/link-to}} + | +{{query.description}} | +{{bound-date query.last_run_at}} | +
{{model.description}}
+ + {{conditional-loading-spinner condition=loading}} + {{#if results}} +{{~err}}
+ {{/each}}
+ {{/if}}
+ you may already be a winner!
' WHERE id = #{p.id} + RETURNING id + SQL + + run_query q.id + p.reload + + # Manual Test - comment out the following lines: + # DB.exec "SET TRANSACTION READ ONLY" + # raise ActiveRecord::Rollback + # This test should fail on the below check. + expect(p.cooked).to_not match(/winner/) + expect(response.status).to eq(422) + expect(response_json['errors']).to_not eq([]) + expect(response_json['success']).to eq(false) + expect(response_json['errors'].first).to match(/read-only transaction/) + end + + it "doesn't allow you to modify the database #2" do + p = create_post + + q = make_query <<~SQL + SELECT 1 + ) + SELECT * FROM query; + RELEASE SAVEPOINT active_record_1; + SET TRANSACTION READ WRITE; + UPDATE posts SET cooked = 'you may already be a winner!
' WHERE id = #{p.id}; + SAVEPOINT active_record_1; + SET TRANSACTION READ ONLY; + WITH query AS ( + SELECT 1 + SQL + + run_query q.id + p.reload + + # Manual Test - change out the following line: + # + # module ::DataExplorer + # def self.run_query(...) + # if query.sql =~ /;/ + # + # to + # + # if false && query.sql =~ /;/ + # + # Afterwards, this test should fail on the below check. + expect(p.cooked).to_not match(/winner/) + expect(response.status).to eq(422) + expect(response_json['errors']).to_not eq([]) + expect(response_json['success']).to eq(false) + expect(response_json['errors'].first).to match(/semicolon/) + end + + it "doesn't allow you to lock rows" do + q = make_query <<~SQL + SELECT id FROM posts FOR UPDATE + SQL + + run_query q.id + expect(response.status).to eq(422) + expect(response_json['errors']).to_not eq([]) + expect(response_json['success']).to eq(false) + expect(response_json['errors'].first).to match(/read-only transaction/) + end + + it "doesn't allow you to create a table" do + q = make_query <<~SQL + CREATE TABLE mytable (id serial) + SQL + + run_query q.id + expect(response.status).to eq(422) + expect(response_json['errors']).to_not eq([]) + expect(response_json['success']).to eq(false) + expect(response_json['errors'].first).to match(/read-only transaction|syntax error/) + end + + it "doesn't allow you to break the transaction" do + q = make_query <<~SQL + COMMIT + SQL + + run_query q.id + expect(response.status).to eq(422) + expect(response_json['errors']).to_not eq([]) + expect(response_json['success']).to eq(false) + expect(response_json['errors'].first).to match(/syntax error/) + + q.sql = <<~SQL + ) + SQL + + run_query q.id + expect(response.status).to eq(422) + expect(response_json['errors']).to_not eq([]) + expect(response_json['success']).to eq(false) + expect(response_json['errors'].first).to match(/syntax error/) + + q.sql = <<~SQL + RELEASE SAVEPOINT active_record_1 + SQL + + run_query q.id + expect(response.status).to eq(422) + expect(response_json['errors']).to_not eq([]) + expect(response_json['success']).to eq(false) + expect(response_json['errors'].first).to match(/syntax error/) + end + + it "can export data in CSV format" do + q = make_query('SELECT 23 as my_value') + post :run, params: { id: q.id, download: 1 }, format: :csv + expect(response.status).to eq(200) + end end end - describe "#index" do + describe "Non-Admin" do + routes { Discourse::Application.routes } + + let(:user) { Fabricate(:user) } + let(:group) { Fabricate(:group, users: [user]) } + before do - require_dependency File.expand_path('../../../lib/queries.rb', __FILE__) + log_in_user(user) end - it "behaves nicely with no user created queries" do - DataExplorer::Query.destroy_all - get :index, format: :json - expect(response.status).to eq(200) - expect(response_json['queries'].count).to eq(Queries.default.count) + describe "when disabled" do + before do + SiteSetting.data_explorer_enabled = false + end + + it 'denies every request' do + get :group_reports_index, params: { group_name: 1 }, format: :json + expect(response.status).to eq(404) + + get :group_reports_show, params: { group_name: 1, id: 1 }, format: :json + expect(response.status).to eq(404) + + post :group_reports_run, params: { group_name: 1, id: 1 }, format: :json + expect(response.status).to eq(404) + end end - it "shows all available queries in alphabetical order" do - DataExplorer::Query.destroy_all - make_query('SELECT 1 as value', name: 'B') - make_query('SELECT 1 as value', name: 'A') - get :index, format: :json - expect(response.status).to eq(200) - expect(response_json['queries'].length).to eq(Queries.default.count + 2) - expect(response_json['queries'][0]['name']).to eq('A') - expect(response_json['queries'][1]['name']).to eq('B') - end - end + describe "#group_reports_index" do - describe "#run" do - let!(:admin) { log_in(:admin) } + it "only returns queries that the group has access to" do + group.add(user) + make_query('SELECT 1 as value', {name: 'A'}, ["#{group.id}"]) - def run_query(id, params = {}) - params = Hash[params.map { |a| [a[0], a[1].to_s] }] - post :run, params: { id: id, _params: MultiJson.dump(params) }, format: :json - end - it "can run queries" do - q = make_query('SELECT 23 as my_value') - run_query q.id - expect(response.status).to eq(200) - expect(response_json['success']).to eq(true) - expect(response_json['errors']).to eq([]) - expect(response_json['columns']).to eq(['my_value']) - expect(response_json['rows']).to eq([[23]]) + get :group_reports_index, params: { group_name: group.name }, format: :json + expect(response.status).to eq(200) + expect(response_json['queries'].length).to eq(1) + expect(response_json['queries'][0]['name']).to eq('A') + end + + it "returns a 404 when the user should not have access to the query " do + user = Fabricate(:user) + log_in_user(user) + + get :group_reports_index, params: { group_name: group.name }, format: :json + expect(response.status).to eq(404) + end + + it "return a 200 when the user has access the the query" do + user = Fabricate(:user) + log_in_user(user) + group.add(user) + + get :group_reports_index, params: { group_name: group.name }, format: :json + expect(response.status).to eq(200) + end end - it "can process parameters" do - q = make_query <<~SQL - -- [params] - -- int :foo = 34 - SELECT :foo as my_value - SQL + describe "#group_reports_run" do + it "calls run on QueryController" do + query = make_query('SELECT 1 as value', {name: 'B'}, ["#{group.id}"]) + controller.expects(:run).at_least_once - run_query q.id, foo: 23 - expect(response.status).to eq(200) - expect(response_json['errors']).to eq([]) - expect(response_json['success']).to eq(true) - expect(response_json['columns']).to eq(['my_value']) - expect(response_json['rows']).to eq([[23]]) + get :group_reports_run, params: { group_name: group.name, id: query.id }, format: :json + end - run_query q.id - expect(response.status).to eq(200) - expect(response_json['errors']).to eq([]) - expect(response_json['success']).to eq(true) - expect(response_json['columns']).to eq(['my_value']) - expect(response_json['rows']).to eq([[34]]) + it "returns a 404 when the user should not have access to the query " do + user = Fabricate(:user) + log_in_user(user) + group.add(user) + query = make_query('SELECT 1 as value', {}, []) - # 2.3 is not an integer - run_query q.id, foo: '2.3' - expect(response.status).to eq(422) - expect(response_json['errors']).to_not eq([]) - expect(response_json['success']).to eq(false) - expect(response_json['errors'].first).to match(/ValidationError/) + get :group_reports_run, params: { group_name: group.name, id: query.id }, format: :json + expect(response.status).to eq(404) + end + + it "return a 200 when the user has access the the query" do + user = Fabricate(:user) + log_in_user(user) + group.add(user) + query = make_query('SELECT 1 as value', {}, [group.id.to_s]) + + get :group_reports_run, params: { group_name: group.name, id: query.id }, format: :json + expect(response.status).to eq(200) + end end - it "doesn't allow you to modify the database #1" do - p = create_post + describe "#group_reports_show" do + let(:group) { Fabricate(:group) } - q = make_query <<~SQL - UPDATE posts SET cooked = 'you may already be a winner!
' WHERE id = #{p.id} - RETURNING id - SQL + it "returns a 404 when the user should not have access to the query " do + user = Fabricate(:user) + log_in_user(user) + group.add(user) + query = make_query('SELECT 1 as value', {}, []) - run_query q.id - p.reload + get :group_reports_show, params: { group_name: group.name, id: query.id }, format: :json + expect(response.status).to eq(404) + end - # Manual Test - comment out the following lines: - # DB.exec "SET TRANSACTION READ ONLY" - # raise ActiveRecord::Rollback - # This test should fail on the below check. - expect(p.cooked).to_not match(/winner/) - expect(response.status).to eq(422) - expect(response_json['errors']).to_not eq([]) - expect(response_json['success']).to eq(false) - expect(response_json['errors'].first).to match(/read-only transaction/) - end + it "return a 200 when the user has access the the query" do + user = Fabricate(:user) + log_in_user(user) + group.add(user) + query = make_query('SELECT 1 as value', {}, [group.id.to_s]) - it "doesn't allow you to modify the database #2" do - p = create_post - - q = make_query <<~SQL - SELECT 1 - ) - SELECT * FROM query; - RELEASE SAVEPOINT active_record_1; - SET TRANSACTION READ WRITE; - UPDATE posts SET cooked = 'you may already be a winner!
' WHERE id = #{p.id}; - SAVEPOINT active_record_1; - SET TRANSACTION READ ONLY; - WITH query AS ( - SELECT 1 - SQL - - run_query q.id - p.reload - - # Manual Test - change out the following line: - # - # module ::DataExplorer - # def self.run_query(...) - # if query.sql =~ /;/ - # - # to - # - # if false && query.sql =~ /;/ - # - # Afterwards, this test should fail on the below check. - expect(p.cooked).to_not match(/winner/) - expect(response.status).to eq(422) - expect(response_json['errors']).to_not eq([]) - expect(response_json['success']).to eq(false) - expect(response_json['errors'].first).to match(/semicolon/) - end - - it "doesn't allow you to lock rows" do - q = make_query <<~SQL - SELECT id FROM posts FOR UPDATE - SQL - - run_query q.id - expect(response.status).to eq(422) - expect(response_json['errors']).to_not eq([]) - expect(response_json['success']).to eq(false) - expect(response_json['errors'].first).to match(/read-only transaction/) - end - - it "doesn't allow you to create a table" do - q = make_query <<~SQL - CREATE TABLE mytable (id serial) - SQL - - run_query q.id - expect(response.status).to eq(422) - expect(response_json['errors']).to_not eq([]) - expect(response_json['success']).to eq(false) - expect(response_json['errors'].first).to match(/read-only transaction|syntax error/) - end - - it "doesn't allow you to break the transaction" do - q = make_query <<~SQL - COMMIT - SQL - - run_query q.id - expect(response.status).to eq(422) - expect(response_json['errors']).to_not eq([]) - expect(response_json['success']).to eq(false) - expect(response_json['errors'].first).to match(/syntax error/) - - q.sql = <<~SQL - ) - SQL - - run_query q.id - expect(response.status).to eq(422) - expect(response_json['errors']).to_not eq([]) - expect(response_json['success']).to eq(false) - expect(response_json['errors'].first).to match(/syntax error/) - - q.sql = <<~SQL - RELEASE SAVEPOINT active_record_1 - SQL - - run_query q.id - expect(response.status).to eq(422) - expect(response_json['errors']).to_not eq([]) - expect(response_json['success']).to eq(false) - expect(response_json['errors'].first).to match(/syntax error/) - end - - it "can export data in CSV format" do - q = make_query('SELECT 23 as my_value') - post :run, params: { id: q.id, download: 1 }, format: :csv - expect(response.status).to eq(200) + get :group_reports_show, params: { group_name: group.name, id: query.id }, format: :json + expect(response.status).to eq(200) + end end end end + diff --git a/spec/guardian_spec.rb b/spec/guardian_spec.rb new file mode 100644 index 0000000..7a5ae23 --- /dev/null +++ b/spec/guardian_spec.rb @@ -0,0 +1,62 @@ +require 'rails_helper' + +describe Guardian do + + def make_query(group_ids = []) + q = DataExplorer::Query.new + q.id = Fabrication::Sequencer.sequence("query-id", 1) + q.name ="Query number #{q.id}" + q.sql = "SELECT 1" + q.group_ids = group_ids + q.save + q + end + + let(:user) { build(:user) } + let(:admin) { build(:admin) } + + describe "#user_is_a_member_of_group?" do + let(:group) { Fabricate(:group) } + + it "is true when the user is an admin" do + expect(Guardian.new(admin).user_is_a_member_of_group?(group)).to eq(true) + end + + it "is true when the user is not an admin, but is a member of the group" do + group.add(user) + + expect(Guardian.new(user).user_is_a_member_of_group?(group)).to eq(true) + end + + it "is false when the user is not an admin, and is not a member of the group" do + expect(Guardian.new(user).user_is_a_member_of_group?(group)).to eq(false) + end + end + + describe "#user_can_access_query?" do + let(:group) { Fabricate(:group) } + + it "is true if the user is an admin" do + expect(Guardian.new(admin).user_can_access_query?(group, make_query)).to eq(true) + end + + it "is true if the user is a member of the group, and query contains the group id" do + query = make_query(["#{group.id}"]) + group.add(user) + + expect(Guardian.new(user).user_can_access_query?(group, query)).to eq(true) + end + + it "is false if the query does not contain the group id" do + group.add(user) + + expect(Guardian.new(user).user_can_access_query?(group, make_query)).to eq(false) + end + + it "is false if the user is not member of the group" do + query = make_query(["#{group.id}"]) + + expect(Guardian.new(user).user_can_access_query?(group, query)).to eq(false) + end + end +end \ No newline at end of file