From b059506afad33611664aa62b3ef43175828e6764 Mon Sep 17 00:00:00 2001 From: Alejandro Fernandez Date: Mon, 6 Nov 2017 10:20:38 -0800 Subject: [PATCH 1/5] DI-1113. ADDENDUM. Authentication: Enable user impersonation for Superset to HiveServer2 using hive.server2.proxy.user (a.fernandez) (#3697) --- superset/db_engine_specs.py | 34 ++++++++++++++++++---------------- superset/db_engines/hive.py | 21 --------------------- superset/models/core.py | 20 ++++++++++++++++---- superset/views/core.py | 25 +++++++++++++++++++------ 4 files changed, 53 insertions(+), 47 deletions(-) diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py index c5363af2b2c8a..d37096ac374b0 100644 --- a/superset/db_engine_specs.py +++ b/superset/db_engine_specs.py @@ -198,15 +198,16 @@ def modify_url_for_impersonation(cls, url, impersonate_user, username): url.username = username @classmethod - def get_uri_for_impersonation(cls, uri, impersonate_user, username): + def get_configuration_for_impersonation(cls, uri, impersonate_user, username): """ - Return a new URI string that allows for user impersonation. + Return a configuration dictionary that can be merged with other configs + that can set the correct properties for impersonating users :param uri: URI string - :param impersonate_user: Bool indicating if impersonation is enabled + :param impersonate_user: Bool indicating if impersonation is enabled :param username: Effective username - :return: New URI string + :return: Dictionary with configs required for impersonation """ - return uri + return {} class PostgresEngineSpec(BaseEngineSpec): @@ -701,7 +702,6 @@ def patch(cls): hive.constants = patched_constants hive.ttypes = patched_ttypes hive.Cursor.fetch_logs = patched_hive.fetch_logs - hive.Connection = patched_hive.ConnectionProxyUser @classmethod @cache_util.memoized_func( @@ -863,27 +863,29 @@ def modify_url_for_impersonation(cls, url, impersonate_user, username): :param impersonate_user: Bool indicating if impersonation is enabled :param username: Effective username """ - if impersonate_user is not None and "auth" in url.query.keys() and username is not None: - url.query["hive_server2_proxy_user"] = username + # Do nothing in the URL object since instead this should modify + # the configuraiton dictionary. See get_configuration_for_impersonation + pass @classmethod - def get_uri_for_impersonation(cls, uri, impersonate_user, username): + def get_configuration_for_impersonation(cls, uri, impersonate_user, username): """ - Return a new URI string that allows for user impersonation. + Return a configuration dictionary that can be merged with other configs + that can set the correct properties for impersonating users :param uri: URI string - :param impersonate_user: Bool indicating if impersonation is enabled + :param impersonate_user: Bool indicating if impersonation is enabled :param username: Effective username - :return: New URI string + :return: Dictionary with configs required for impersonation """ - new_uri = uri + configuration = {} url = make_url(uri) backend_name = url.get_backend_name() # Must be Hive connection, enable impersonation, and set param auth=LDAP|KERBEROS - if backend_name == "hive" and "auth" in url.query.keys() and\ + if backend_name == "hive" and "auth" in url.query.keys() and \ impersonate_user is True and username is not None: - new_uri += "&hive_server2_proxy_user={0}".format(username) - return new_uri + configuration["hive.server2.proxy.user"] = username + return configuration class MssqlEngineSpec(BaseEngineSpec): engine = 'mssql' diff --git a/superset/db_engines/hive.py b/superset/db_engines/hive.py index 334ae0a4b2d87..bf3566fac11ec 100644 --- a/superset/db_engines/hive.py +++ b/superset/db_engines/hive.py @@ -3,27 +3,6 @@ from thrift import Thrift -old_Connection = hive.Connection - -# TODO -# Monkey-patch of PyHive project's pyhive/hive.py which needed to change the constructor. -# Submitted a pull request on October 13, 2017 and waiting for it to be merged. -# https://github.com/dropbox/PyHive/pull/165 -class ConnectionProxyUser(hive.Connection): - - def __init__(self, host=None, port=None, username=None, database='default', auth=None, - configuration=None, kerberos_service_name=None, password=None, - thrift_transport=None, hive_server2_proxy_user=None): - configuration = configuration or {} - if auth is not None and auth in ('LDAP', 'KERBEROS'): - if hive_server2_proxy_user is not None: - configuration["hive.server2.proxy.user"] = hive_server2_proxy_user - # restore the old connection class, otherwise, will recurse on its own __init__ method - hive.Connection = old_Connection - hive.Connection.__init__(self, host=host, port=port, username=username, database=database, auth=auth, - configuration=configuration, kerberos_service_name=kerberos_service_name, password=password, - thrift_transport=thrift_transport) - # TODO: contribute back to pyhive. def fetch_logs(self, max_rows=1024, diff --git a/superset/models/core.py b/superset/models/core.py index aa2484fdb48b3..78a72a7487612 100644 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -620,23 +620,35 @@ def get_effective_user(self, url, user_name=None): effective_username = url.username if user_name: effective_username = user_name - elif hasattr(g, 'user') and g.user.username: + elif hasattr(g, 'user') and hasattr(g.user, 'username') and g.user.username is not None: effective_username = g.user.username return effective_username def get_sqla_engine(self, schema=None, nullpool=False, user_name=None): extra = self.get_extra() url = make_url(self.sqlalchemy_uri_decrypted) - params = extra.get('engine_params', {}) - if nullpool: - params['poolclass'] = NullPool url = self.db_engine_spec.adjust_database_uri(url, schema) effective_username = self.get_effective_user(url, user_name) + # If using MySQL or Presto for example, will set url.username + # If using Hive, will not do anything yet since that relies on a configuration parameter instead. self.db_engine_spec.modify_url_for_impersonation(url, self.impersonate_user, effective_username) masked_url = self.get_password_masked_url(url) logging.info("Database.get_sqla_engine(). Masked URL: {0}".format(masked_url)) + params = extra.get('engine_params', {}) + if nullpool: + params['poolclass'] = NullPool + + # If using Hive, this will set hive.server2.proxy.user=$effective_username + configuration = {} + configuration.update( + self.db_engine_spec.get_configuration_for_impersonation(str(url), + self.impersonate_user, + effective_username)) + if configuration: + params["connect_args"] = {"configuration": configuration} + return create_engine(url, **params) def get_reserved_words(self): diff --git a/superset/views/core.py b/superset/views/core.py index bd4d4e52ac361..9e1d6ec364923 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1433,6 +1433,7 @@ def testconn(self): uri = request.json.get('uri') db_name = request.json.get('name') impersonate_user = request.json.get('impersonate_user') + database = None if db_name: database = ( db.session @@ -1444,20 +1445,32 @@ def testconn(self): # the password-masked uri was passed # use the URI associated with this database uri = database.sqlalchemy_uri_decrypted + + configuration = {} + + if database and uri: + url = make_url(uri) + db_engine = models.Database.get_db_engine_spec_for_backend(url.get_backend_name()) + db_engine.patch() - url = make_url(uri) - db_engine = models.Database.get_db_engine_spec_for_backend(url.get_backend_name()) - db_engine.patch() - uri = db_engine.get_uri_for_impersonation(uri, impersonate_user, username) - masked_url = database.get_password_masked_url_from_uri(uri) + masked_url = database.get_password_masked_url_from_uri(uri) + logging.info("Superset.testconn(). Masked URL: {0}".format(masked_url)) - logging.info("Superset.testconn(). Masked URL: {0}".format(masked_url)) + configuration.update( + db_engine.get_configuration_for_impersonation(uri, + impersonate_user, + username) + ) connect_args = ( request.json .get('extras', {}) .get('engine_params', {}) .get('connect_args', {})) + + if configuration: + connect_args["configuration"] = configuration + engine = create_engine(uri, connect_args=connect_args) engine.connect() return json_success(json.dumps(engine.table_names(), indent=4)) From 9a49b1c41dca13b9f5095e7cc8f3b931d01accf9 Mon Sep 17 00:00:00 2001 From: Jeff Niu Date: Mon, 6 Nov 2017 15:20:13 -0800 Subject: [PATCH 2/5] [Performance] VirtualizedSelect for SelectControl and FilterBox (#3654) * Added virtualized select to SelectControl, allow onPaste to create new options * Added unit tests * Added virtualized/paste select to filterbox --- .../javascripts/components/OnPasteSelect.jsx | 87 ++++++++++++++ .../components/VirtualizedRendererWrap.jsx | 56 +++++++++ .../components/controls/SelectControl.jsx | 65 +---------- .../components/OnPasteSelect_spec.jsx | 105 +++++++++++++++++ .../VirtualizedRendererWrap_spec.jsx | 106 ++++++++++++++++++ .../explore/components/SelectControl_spec.jsx | 33 +++++- superset/assets/visualizations/filter_box.jsx | 12 +- 7 files changed, 397 insertions(+), 67 deletions(-) create mode 100644 superset/assets/javascripts/components/OnPasteSelect.jsx create mode 100644 superset/assets/javascripts/components/VirtualizedRendererWrap.jsx create mode 100644 superset/assets/spec/javascripts/components/OnPasteSelect_spec.jsx create mode 100644 superset/assets/spec/javascripts/components/VirtualizedRendererWrap_spec.jsx diff --git a/superset/assets/javascripts/components/OnPasteSelect.jsx b/superset/assets/javascripts/components/OnPasteSelect.jsx new file mode 100644 index 0000000000000..b043bf317d175 --- /dev/null +++ b/superset/assets/javascripts/components/OnPasteSelect.jsx @@ -0,0 +1,87 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import Select from 'react-select'; + +export default class OnPasteSelect extends React.Component { + onPaste(evt) { + if (!this.props.multi) { + return; + } + evt.preventDefault(); + const clipboard = evt.clipboardData.getData('Text'); + if (!clipboard) { + return; + } + const regex = `[${this.props.separator}]+`; + const values = clipboard.split(new RegExp(regex)).map(v => v.trim()); + const validator = this.props.isValidNewOption; + const selected = this.props.value || []; + const existingOptions = {}; + const existing = {}; + this.props.options.forEach((v) => { + existingOptions[v[this.props.valueKey]] = 1; + }); + let options = []; + selected.forEach((v) => { + options.push({ [this.props.labelKey]: v, [this.props.valueKey]: v }); + existing[v] = 1; + }); + options = options.concat(values + .filter((v) => { + const notExists = !existing[v]; + existing[v] = 1; + return notExists && (validator ? validator({ [this.props.labelKey]: v }) : !!v); + }) + .map((v) => { + const opt = { [this.props.labelKey]: v, [this.props.valueKey]: v }; + if (!existingOptions[v]) { + this.props.options.unshift(opt); + } + return opt; + }), + ); + if (options.length) { + if (this.props.onChange) { + this.props.onChange(options); + } + } + } + render() { + const SelectComponent = this.props.selectWrap; + const refFunc = (ref) => { + if (this.props.ref) { + this.props.ref(ref); + } + this.pasteInput = ref; + }; + const inputProps = { onPaste: this.onPaste.bind(this) }; + return ( + + ); + } +} + +OnPasteSelect.propTypes = { + separator: PropTypes.string.isRequired, + selectWrap: PropTypes.func.isRequired, + ref: PropTypes.func, + onChange: PropTypes.func.isRequired, + valueKey: PropTypes.string.isRequired, + labelKey: PropTypes.string.isRequired, + options: PropTypes.array, + multi: PropTypes.bool.isRequired, + value: PropTypes.any, + isValidNewOption: PropTypes.func, +}; +OnPasteSelect.defaultProps = { + separator: ',', + selectWrap: Select, + valueKey: 'value', + labelKey: 'label', + options: [], + multi: false, +}; diff --git a/superset/assets/javascripts/components/VirtualizedRendererWrap.jsx b/superset/assets/javascripts/components/VirtualizedRendererWrap.jsx new file mode 100644 index 0000000000000..32821124bd19b --- /dev/null +++ b/superset/assets/javascripts/components/VirtualizedRendererWrap.jsx @@ -0,0 +1,56 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +export default function VirtualizedRendererWrap(renderer) { + function WrapperRenderer({ + focusedOption, + focusOption, + key, + option, + selectValue, + style, + valueArray, + }) { + if (!option) { + return null; + } + const className = ['VirtualizedSelectOption']; + if (option === focusedOption) { + className.push('VirtualizedSelectFocusedOption'); + } + if (option.disabled) { + className.push('VirtualizedSelectDisabledOption'); + } + if (valueArray && valueArray.indexOf(option) >= 0) { + className.push('VirtualizedSelectSelectedOption'); + } + if (option.className) { + className.push(option.className); + } + const events = option.disabled ? {} : { + onClick: () => selectValue(option), + onMouseEnter: () => focusOption(option), + }; + return ( +
+ {renderer(option)} +
+ ); + } + WrapperRenderer.propTypes = { + focusedOption: PropTypes.object.isRequired, + focusOption: PropTypes.func.isRequired, + key: PropTypes.string, + option: PropTypes.object, + selectValue: PropTypes.func.isRequired, + style: PropTypes.object, + valueArray: PropTypes.array, + }; + return WrapperRenderer; +} diff --git a/superset/assets/javascripts/explore/components/controls/SelectControl.jsx b/superset/assets/javascripts/explore/components/controls/SelectControl.jsx index 51381d64d63dd..a82995d70e4fa 100644 --- a/superset/assets/javascripts/explore/components/controls/SelectControl.jsx +++ b/superset/assets/javascripts/explore/components/controls/SelectControl.jsx @@ -1,8 +1,11 @@ import React from 'react'; import PropTypes from 'prop-types'; +import VirtualizedSelect from 'react-virtualized-select'; import Select, { Creatable } from 'react-select'; import ControlHeader from '../ControlHeader'; import { t } from '../../../locales'; +import VirtualizedRendererWrap from '../../../components/VirtualizedRendererWrap'; +import OnPasteSelect from '../../../components/OnPasteSelect'; const propTypes = { choices: PropTypes.array, @@ -37,55 +40,6 @@ const defaultProps = { valueKey: 'value', }; -// Handle `onPaste` so that users may paste in -// options as comma-delimited, slightly modified from -// https://github.com/JedWatson/react-select/issues/1672 -function pasteSelect(props) { - let pasteInput; - return ( -