Fix: allow users with view only acces to use the queries in Query Results (#4112)

* Fix: allow users with view only acces to access the queries

* Add tests

* Update error message

* Update error message. Take 2
This commit is contained in:
Arik Fraimovich 2019-09-01 22:17:53 +03:00 committed by GitHub
parent 98e33b7780
commit df3da82afd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 131 additions and 40 deletions

View File

@ -3,7 +3,7 @@ import re
import sqlite3
from redash import models
from redash.permissions import has_access, not_view_only
from redash.permissions import has_access, view_only
from redash.query_runner import BaseQueryRunner, TYPE_STRING, guess_type, register
from redash.utils import json_dumps, json_loads
@ -24,7 +24,8 @@ def extract_query_ids(query):
def extract_cached_query_ids(query):
queries = re.findall(r'(?:join|from)\s+cached_query_(\d+)', query, re.IGNORECASE)
queries = re.findall(r'(?:join|from)\s+cached_query_(\d+)', query,
re.IGNORECASE)
return [int(q) for q in queries]
@ -34,9 +35,11 @@ def _load_query(user, query_id):
if user.org_id != query.org_id:
raise PermissionError("Query id {} not found.".format(query.id))
if not has_access(query.data_source, user, not_view_only):
raise PermissionError(u"You are not allowed to execute queries on {} data source (used for query id {}).".format(
query.data_source.name, query.id))
# TODO: this duplicates some of the logic we already have in the redash.handlers.query_results.
# We should merge it so it's consistent.
if not has_access(query.data_source, user, view_only):
raise PermissionError(u"You do not have access to query id {}.".format(
query.id))
return query
@ -47,16 +50,22 @@ def get_query_results(user, query_id, bring_from_cache):
if query.latest_query_data_id is not None:
results = query.latest_query_data.data
else:
raise Exception("No cached result available for query {}.".format(query.id))
raise Exception("No cached result available for query {}.".format(
query.id))
else:
results, error = query.data_source.query_runner.run_query(query.query_text, user)
results, error = query.data_source.query_runner.run_query(
query.query_text, user)
if error:
raise Exception("Failed loading results for query id {}.".format(query.id))
raise Exception("Failed loading results for query id {}.".format(
query.id))
return json_loads(results)
def create_tables_from_query_ids(user, connection, query_ids, cached_query_ids=[]):
def create_tables_from_query_ids(user,
connection,
query_ids,
cached_query_ids=[]):
for query_id in set(cached_query_ids):
results = get_query_results(user, query_id, True)
table_name = 'cached_query_{query_id}'.format(query_id=query_id)
@ -81,8 +90,7 @@ def flatten(value):
def create_table(connection, table_name, query_results):
try:
columns = [column['name']
for column in query_results['columns']]
columns = [column['name'] for column in query_results['columns']]
safe_columns = [fix_column_name(column) for column in columns]
column_list = ", ".join(safe_columns)
@ -91,7 +99,8 @@ def create_table(connection, table_name, query_results):
logger.debug("CREATE TABLE query: %s", create_table)
connection.execute(create_table)
except sqlite3.OperationalError as exc:
raise CreateTableError(u"Error creating table {}: {}".format(table_name, exc.message))
raise CreateTableError(u"Error creating table {}: {}".format(
table_name, exc.message))
insert_template = u"insert into {table_name} ({column_list}) values ({place_holders})".format(
table_name=table_name,
@ -108,11 +117,7 @@ class Results(BaseQueryRunner):
@classmethod
def configuration_schema(cls):
return {
"type": "object",
"properties": {
}
}
return {"type": "object", "properties": {}}
@classmethod
def annotate_query(cls):
@ -127,7 +132,8 @@ class Results(BaseQueryRunner):
query_ids = extract_query_ids(query)
cached_query_ids = extract_cached_query_ids(query)
create_tables_from_query_ids(user, connection, query_ids, cached_query_ids)
create_tables_from_query_ids(user, connection, query_ids,
cached_query_ids)
cursor = connection.cursor()
@ -135,8 +141,8 @@ class Results(BaseQueryRunner):
cursor.execute(query)
if cursor.description is not None:
columns = self.fetch_columns(
[(i[0], None) for i in cursor.description])
columns = self.fetch_columns([(i[0], None)
for i in cursor.description])
rows = []
column_names = [c['name'] for c in columns]

View File

@ -3,7 +3,9 @@ from unittest import TestCase
import pytest
from redash.query_runner.query_results import (CreateTableError, PermissionError, _load_query, create_table, extract_cached_query_ids, extract_query_ids, fix_column_name)
from redash.query_runner.query_results import (
CreateTableError, PermissionError, _load_query, create_table,
extract_cached_query_ids, extract_query_ids, fix_column_name)
from tests import BaseTestCase
@ -28,40 +30,86 @@ class TestExtractQueryIds(TestCase):
class TestCreateTable(TestCase):
def test_creates_table_with_colons_in_column_name(self):
connection = sqlite3.connect(':memory:')
results = {'columns': [{'name': 'ga:newUsers'}, {
'name': 'test2'}], 'rows': [{'ga:newUsers': 123, 'test2': 2}]}
results = {
'columns': [{
'name': 'ga:newUsers'
}, {
'name': 'test2'
}],
'rows': [{
'ga:newUsers': 123,
'test2': 2
}]
}
table_name = 'query_123'
create_table(connection, table_name, results)
connection.execute('SELECT 1 FROM query_123')
def test_creates_table_with_double_quotes_in_column_name(self):
connection = sqlite3.connect(':memory:')
results = {'columns': [{'name': 'ga:newUsers'}, {
'name': '"test2"'}], 'rows': [{'ga:newUsers': 123, '"test2"': 2}]}
results = {
'columns': [{
'name': 'ga:newUsers'
}, {
'name': '"test2"'
}],
'rows': [{
'ga:newUsers': 123,
'"test2"': 2
}]
}
table_name = 'query_123'
create_table(connection, table_name, results)
connection.execute('SELECT 1 FROM query_123')
def test_creates_table(self):
connection = sqlite3.connect(':memory:')
results = {'columns': [{'name': 'test1'},
{'name': 'test2'}], 'rows': []}
results = {
'columns': [{
'name': 'test1'
}, {
'name': 'test2'
}],
'rows': []
}
table_name = 'query_123'
create_table(connection, table_name, results)
connection.execute('SELECT 1 FROM query_123')
def test_creates_table_with_missing_columns(self):
connection = sqlite3.connect(':memory:')
results = {'columns': [{'name': 'test1'}, {'name': 'test2'}], 'rows': [
{'test1': 1, 'test2': 2}, {'test1': 3}]}
results = {
'columns': [{
'name': 'test1'
}, {
'name': 'test2'
}],
'rows': [{
'test1': 1,
'test2': 2
}, {
'test1': 3
}]
}
table_name = 'query_123'
create_table(connection, table_name, results)
connection.execute('SELECT 1 FROM query_123')
def test_creates_table_with_spaces_in_column_name(self):
connection = sqlite3.connect(':memory:')
results = {'columns': [{'name': 'two words'}, {'name': 'test2'}], 'rows': [
{'two words': 1, 'test2': 2}, {'test1': 3}]}
results = {
'columns': [{
'name': 'two words'
}, {
'name': 'test2'
}],
'rows': [{
'two words': 1,
'test2': 2
}, {
'test1': 3
}]
}
table_name = 'query_123'
create_table(connection, table_name, results)
connection.execute('SELECT 1 FROM query_123')
@ -69,8 +117,15 @@ class TestCreateTable(TestCase):
def test_creates_table_with_dashes_in_column_name(self):
connection = sqlite3.connect(':memory:')
results = {
'columns': [{'name': 'two-words'}, {'name': 'test2'}],
'rows': [{'two-words': 1, 'test2': 2}]
'columns': [{
'name': 'two-words'
}, {
'name': 'test2'
}],
'rows': [{
'two-words': 1,
'test2': 2
}]
}
table_name = 'query_123'
create_table(connection, table_name, results)
@ -79,8 +134,17 @@ class TestCreateTable(TestCase):
def test_creates_table_with_non_ascii_in_column_name(self):
connection = sqlite3.connect(':memory:')
results = {'columns': [{'name': u'\xe4'}, {'name': 'test2'}], 'rows': [
{u'\xe4': 1, 'test2': 2}]}
results = {
'columns': [{
'name': u'\xe4'
}, {
'name': 'test2'
}],
'rows': [{
u'\xe4': 1,
'test2': 2
}]
}
table_name = 'query_123'
create_table(connection, table_name, results)
connection.execute('SELECT 1 FROM query_123')
@ -95,8 +159,14 @@ class TestCreateTable(TestCase):
def test_loads_results(self):
connection = sqlite3.connect(':memory:')
rows = [{'test1': 1, 'test2': 'test'}, {'test1': 2, 'test2': 'test2'}]
results = {'columns': [{'name': 'test1'},
{'name': 'test2'}], 'rows': rows}
results = {
'columns': [{
'name': 'test1'
}, {
'name': 'test2'
}],
'rows': rows
}
table_name = 'query_123'
create_table(connection, table_name, results)
self.assertEquals(
@ -104,9 +174,15 @@ class TestCreateTable(TestCase):
def test_loads_list_and_dict_results(self):
connection = sqlite3.connect(':memory:')
rows = [{'test1': [1,2,3]}, {'test2': {'a': 'b'}}]
results = {'columns': [{'name': 'test1'},
{'name': 'test2'}], 'rows': rows}
rows = [{'test1': [1, 2, 3]}, {'test2': {'a': 'b'}}]
results = {
'columns': [{
'name': 'test1'
}, {
'name': 'test2'
}],
'rows': rows
}
table_name = 'query_123'
create_table(connection, table_name, results)
self.assertEquals(
@ -135,6 +211,15 @@ class TestGetQuery(BaseTestCase):
loaded = _load_query(user, query.id)
self.assertEquals(query, loaded)
def test_returns_query_when_user_has_view_only_access(self):
ds = self.factory.create_data_source(
group=self.factory.org.default_group, view_only=True)
query = self.factory.create_query(data_source=ds)
user = self.factory.create_user()
loaded = _load_query(user, query.id)
self.assertEquals(query, loaded)
class TestExtractCachedQueryIds(TestCase):
def test_works_with_simple_query(self):