From 37b491e8bf4afd310cbefbd8cba87dab23f4d6c4 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 1 Apr 2024 18:09:11 -0700 Subject: [PATCH] feat: Slack Avatar integration Our current Avatars are a little dry, using a limited variety of colors and showing the user's initials. The app could use a little more personality. Now given we already have a slack integration and administrators can configure their `SLACK_API_TOKEN`, I was thinking we can use this integration to get Slack's avatars integrated in Superset. Given the popularity of Slack and users familiarity with the avatars there, I thought it'd be neat to bring a bit more color in Superset. The approach here is centered around a new endpoint `/api/v1/user/{id}/avatar.png`, which fetches from a new column in UserAttribute (our one-to-one extension to FAB's User model) for avatar_url, and redirects to the destination. When loading for the first time, if information is empty and Slack is configured, we ask Slack for the URL, which is secret, but serves the image from outside the authentication part of their API. --- .gitignore | 1 + UPDATING.md | 4 + docker-compose-image-tag.yml | 30 +++++-- docker-compose-non-dev.yml | 30 +++++-- docker-compose.yml | 42 ++++++++-- docker/pythonpath_dev/superset_config.py | 1 + .../src/components/FacePile/index.tsx | 17 ++-- .../src/components/TableCollection/index.tsx | 1 + .../src/pages/DashboardList/index.tsx | 1 + superset/config.py | 4 + superset/dashboards/api.py | 3 + superset/initialization/__init__.py | 3 +- ...22-44_c22cb5c2e546_user_attr_avatar_url.py | 39 +++++++++ superset/models/user.py | 21 +++++ superset/models/user_attributes.py | 4 +- superset/reports/notifications/slack.py | 7 +- superset/security/manager.py | 1 + superset/utils/slack.py | 9 +++ superset/views/users/api.py | 81 ++++++++++++++++++- 19 files changed, 267 insertions(+), 32 deletions(-) create mode 100644 superset/migrations/versions/2024-04-01_22-44_c22cb5c2e546_user_attr_avatar_url.py create mode 100644 superset/models/user.py create mode 100644 superset/utils/slack.py diff --git a/.gitignore b/.gitignore index c36f24283b182..5ed980f208156 100644 --- a/.gitignore +++ b/.gitignore @@ -110,5 +110,6 @@ release.json messages.mo docker/requirements-local.txt +docker/.env-local cache/ diff --git a/UPDATING.md b/UPDATING.md index 459cc4e452754..104c842ee1caa 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -38,6 +38,10 @@ assists people when migrating to a new version. - [27697](https://github.com/apache/superset/pull/27697) [minor] flask-session bump leads to them deprecating `SESSION_USE_SIGNER`, check your configs as this flag won't do anything moving forward. +- [27849](https://github.com/apache/superset/pull/27849/) New config flag `SLACK_ENABLE_AVATARS` + that is `True` by default, meaning if you have + set `SLACK_API_TOKEN` already for other purposes, Superset will start using Slack + avatars throughout the app. ### Breaking Changes diff --git a/docker-compose-image-tag.yml b/docker-compose-image-tag.yml index 07f0d0dcb14b7..6c017ef75e757 100644 --- a/docker-compose-image-tag.yml +++ b/docker-compose-image-tag.yml @@ -33,7 +33,11 @@ services: - redis:/data db: - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local image: postgres:15 container_name: superset_db restart: unless-stopped @@ -42,7 +46,11 @@ services: - ./docker/docker-entrypoint-initdb.d:/docker-entrypoint-initdb.d superset: - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local image: *superset-image container_name: superset_app command: ["/app/docker/docker-bootstrap.sh", "app-gunicorn"] @@ -57,7 +65,11 @@ services: image: *superset-image container_name: superset_init command: ["/app/docker/docker-init.sh"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local depends_on: *superset-depends-on user: "root" volumes: *superset-volumes @@ -68,7 +80,11 @@ services: image: *superset-image container_name: superset_worker command: ["/app/docker/docker-bootstrap.sh", "worker"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local restart: unless-stopped depends_on: *superset-depends-on user: "root" @@ -84,7 +100,11 @@ services: image: *superset-image container_name: superset_worker_beat command: ["/app/docker/docker-bootstrap.sh", "beat"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local restart: unless-stopped depends_on: *superset-depends-on user: "root" diff --git a/docker-compose-non-dev.yml b/docker-compose-non-dev.yml index b49d070118bcb..c6c93f706bb21 100644 --- a/docker-compose-non-dev.yml +++ b/docker-compose-non-dev.yml @@ -38,7 +38,11 @@ services: - redis:/data db: - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local image: postgres:15 container_name: superset_db restart: unless-stopped @@ -47,7 +51,11 @@ services: - ./docker/docker-entrypoint-initdb.d:/docker-entrypoint-initdb.d superset: - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local build: <<: *common-build container_name: superset_app @@ -64,7 +72,11 @@ services: build: <<: *common-build command: ["/app/docker/docker-init.sh"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local depends_on: *superset-depends-on user: "root" volumes: *superset-volumes @@ -76,7 +88,11 @@ services: <<: *common-build container_name: superset_worker command: ["/app/docker/docker-bootstrap.sh", "worker"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local restart: unless-stopped depends_on: *superset-depends-on user: "root" @@ -93,7 +109,11 @@ services: <<: *common-build container_name: superset_worker_beat command: ["/app/docker/docker-bootstrap.sh", "beat"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local restart: unless-stopped depends_on: *superset-depends-on user: "root" diff --git a/docker-compose.yml b/docker-compose.yml index 23c040b2f6ec0..de5f7e0afd701 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -54,7 +54,11 @@ services: - redis:/data db: - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local image: postgres:15 container_name: superset_db restart: unless-stopped @@ -65,7 +69,11 @@ services: - ./docker/docker-entrypoint-initdb.d:/docker-entrypoint-initdb.d superset: - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local build: <<: *common-build container_name: superset_app @@ -116,7 +124,11 @@ services: <<: *common-build container_name: superset_init command: ["/app/docker/docker-init.sh"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local depends_on: *superset-depends-on user: *superset-user volumes: *superset-volumes @@ -135,7 +147,11 @@ services: PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: ${BUILD_SUPERSET_FRONTEND_IN_DOCKER:-false} container_name: superset_node command: ["/app/docker/docker-frontend.sh"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local depends_on: *superset-depends-on volumes: *superset-volumes @@ -144,7 +160,11 @@ services: <<: *common-build container_name: superset_worker command: ["/app/docker/docker-bootstrap.sh", "worker"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local restart: unless-stopped depends_on: *superset-depends-on user: *superset-user @@ -162,7 +182,11 @@ services: <<: *common-build container_name: superset_worker_beat command: ["/app/docker/docker-bootstrap.sh", "beat"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local restart: unless-stopped depends_on: *superset-depends-on user: *superset-user @@ -175,7 +199,11 @@ services: <<: *common-build container_name: superset_tests_worker command: ["/app/docker/docker-bootstrap.sh", "worker"] - env_file: docker/.env + env_file: + # defaults + - docker/.env + # gitignored overrides + - docker/.env-local environment: DATABASE_HOST: localhost DATABASE_DB: test diff --git a/docker/pythonpath_dev/superset_config.py b/docker/pythonpath_dev/superset_config.py index 005cc600ae1bf..af5d230f0f705 100644 --- a/docker/pythonpath_dev/superset_config.py +++ b/docker/pythonpath_dev/superset_config.py @@ -99,6 +99,7 @@ class CeleryConfig: WEBDRIVER_BASEURL_USER_FRIENDLY = WEBDRIVER_BASEURL SQLLAB_CTAS_NO_LIMIT = True +SLACK_API_TOKEN = os.getenv("SLACK_API_TOKEN") # # Optionally import superset_config_docker.py (which will have been included on diff --git a/superset-frontend/src/components/FacePile/index.tsx b/superset-frontend/src/components/FacePile/index.tsx index 44cc62ce1d624..77795113e314b 100644 --- a/superset-frontend/src/components/FacePile/index.tsx +++ b/superset-frontend/src/components/FacePile/index.tsx @@ -33,12 +33,14 @@ interface FacePileProps { const colorList = getCategoricalSchemeRegistry().get()?.colors ?? []; -const customAvatarStyler = (theme: SupersetTheme) => ` - width: ${theme.gridUnit * 6}px; - height: ${theme.gridUnit * 6}px; - line-height: ${theme.gridUnit * 6}px; - font-size: ${theme.typography.sizes.m}px; -`; +const customAvatarStyler = (theme: SupersetTheme) => { + const size = theme.gridUnit * 8; + return ` + width: ${size}px; + height: ${size}px; + line-height: ${size}px; + font-size: ${theme.typography.sizes.m}px;`; +}; const StyledAvatar = styled(Avatar)` ${({ theme }) => customAvatarStyler(theme)} @@ -58,6 +60,7 @@ export default function FacePile({ users, maxCount = 4 }: FacePileProps) { const name = `${first_name} ${last_name}`; const uniqueKey = `${id}-${first_name}-${last_name}`; const color = getRandomColor(uniqueKey, colorList); + const avatarUrl = `/api/v1/user/${id}/avatar.png`; return ( {first_name?.[0]?.toLocaleUpperCase()} {last_name?.[0]?.toLocaleUpperCase()} ); + return ; })} ); diff --git a/superset-frontend/src/components/TableCollection/index.tsx b/superset-frontend/src/components/TableCollection/index.tsx index bcda5139eb141..fa56cd1b7c3d7 100644 --- a/superset-frontend/src/components/TableCollection/index.tsx +++ b/superset-frontend/src/components/TableCollection/index.tsx @@ -307,6 +307,7 @@ export default React.memo( [cell.column.size || '']: cell.column.size, }, )} + style={cell.column.tdStyle} {...cell.getCellProps()} {...columnCellProps} > diff --git a/superset-frontend/src/pages/DashboardList/index.tsx b/superset-frontend/src/pages/DashboardList/index.tsx index 23261aff84a67..81ded6c7eb413 100644 --- a/superset-frontend/src/pages/DashboardList/index.tsx +++ b/superset-frontend/src/pages/DashboardList/index.tsx @@ -357,6 +357,7 @@ function DashboardList(props: DashboardListProps) { Header: t('Owners'), accessor: 'owners', disableSortBy: true, + tdStyle: { padding: '0px' }, size: 'xl', }, { diff --git a/superset/config.py b/superset/config.py index 0d00fedfbb9e3..506a31e39a7ac 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1316,6 +1316,8 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument # Slack API token for the superset reports, either string or callable SLACK_API_TOKEN: Callable[[], str] | str | None = None SLACK_PROXY = None +# Whether Superset should use Slack avatars for users +SLACK_ENABLE_AVATARS = True # The webdriver to use for generating reports. Use one of the following # firefox @@ -1412,6 +1414,7 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument "data:", "https://apachesuperset.gateway.scarf.sh", "https://static.scarf.sh/", + "https://avatars.slack-edge.com", ], "worker-src": ["'self'", "blob:"], "connect-src": [ @@ -1441,6 +1444,7 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument "data:", "https://apachesuperset.gateway.scarf.sh", "https://static.scarf.sh/", + "https://avatars.slack-edge.com", ], "worker-src": ["'self'", "blob:"], "connect-src": [ diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index ee8f1f73aeddb..a708ca3971360 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -179,6 +179,9 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "owners.id", "owners.first_name", "owners.last_name", + "owners.email", + "owners.id", + # "owners.user_attributes.avatar_url", "roles.id", "roles.name", "is_managed_externally", diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index f39a1ec99dce6..8a3197b4e009e 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -190,7 +190,7 @@ def init_views(self) -> None: ) from superset.views.sqllab import SqllabView from superset.views.tags import TagModelView, TagView - from superset.views.users.api import CurrentUserRestApi + from superset.views.users.api import CurrentUserRestApi, UserRestApi # # Setup API views @@ -205,6 +205,7 @@ def init_views(self) -> None: appbuilder.add_api(ChartDataRestApi) appbuilder.add_api(CssTemplateRestApi) appbuilder.add_api(CurrentUserRestApi) + appbuilder.add_api(UserRestApi) appbuilder.add_api(DashboardFilterStateRestApi) appbuilder.add_api(DashboardPermalinkRestApi) appbuilder.add_api(DashboardRestApi) diff --git a/superset/migrations/versions/2024-04-01_22-44_c22cb5c2e546_user_attr_avatar_url.py b/superset/migrations/versions/2024-04-01_22-44_c22cb5c2e546_user_attr_avatar_url.py new file mode 100644 index 0000000000000..0a5430c684edb --- /dev/null +++ b/superset/migrations/versions/2024-04-01_22-44_c22cb5c2e546_user_attr_avatar_url.py @@ -0,0 +1,39 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# # Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""empty message + +Revision ID: c22cb5c2e546 +Revises: be1b217cd8cd +Create Date: 2024-04-01 22:44:40.386543 + +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "c22cb5c2e546" +down_revision = "be1b217cd8cd" + + +def upgrade(): + op.add_column( + "user_attribute", sa.Column("avatar_url", sa.String(length=100), nullable=True) + ) + + +def downgrade(): + op.drop_column("user_attribute", "avatar_url") diff --git a/superset/models/user.py b/superset/models/user.py new file mode 100644 index 0000000000000..0ea5d1a33276c --- /dev/null +++ b/superset/models/user.py @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from flask_appbuilder.security.sqla.models import User as FABUser + + +class User(FABUser): + pass diff --git a/superset/models/user_attributes.py b/superset/models/user_attributes.py index b2af44a188945..5a124ba4c707f 100644 --- a/superset/models/user_attributes.py +++ b/superset/models/user_attributes.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. from flask_appbuilder import Model -from sqlalchemy import Column, ForeignKey, Integer +from sqlalchemy import Column, ForeignKey, Integer, String from sqlalchemy.orm import relationship from superset import security_manager @@ -39,6 +39,6 @@ class UserAttribute(Model, AuditMixinNullable): user = relationship( security_manager.user_model, backref="extra_attributes", foreign_keys=[user_id] ) - welcome_dashboard_id = Column(Integer, ForeignKey("dashboards.id")) welcome_dashboard = relationship("Dashboard") + avatar_url = Column(String(100)) diff --git a/superset/reports/notifications/slack.py b/superset/reports/notifications/slack.py index 345e9d5b26373..02f5120f685d1 100644 --- a/superset/reports/notifications/slack.py +++ b/superset/reports/notifications/slack.py @@ -24,7 +24,6 @@ import pandas as pd from flask import g from flask_babel import gettext as __ -from slack_sdk import WebClient from slack_sdk.errors import ( BotUserAccessError, SlackApiError, @@ -47,6 +46,7 @@ ) from superset.utils.core import get_email_address_list from superset.utils.decorators import statsd_gauge +from superset.utils.slack import get_slack_client logger = logging.getLogger(__name__) @@ -181,10 +181,7 @@ def send(self) -> None: body = self._get_body() global_logs_context = getattr(g, "logs_context", {}) or {} try: - token = app.config["SLACK_API_TOKEN"] - if callable(token): - token = token() - client = WebClient(token=token, proxy=app.config["SLACK_PROXY"]) + client = get_slack_client() # files_upload returns SlackResponse as we run it in sync mode. if files: for file in files: diff --git a/superset/security/manager.py b/superset/security/manager.py index e5a32e97a711c..4ac773bb77229 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -200,6 +200,7 @@ def query_context_modified(query_context: "QueryContext") -> bool: class SupersetSecurityManager( # pylint: disable=too-many-public-methods SecurityManager ): + user_model = User userstatschartview = None READ_ONLY_MODEL_VIEWS = {"Database", "DynamicPlugin"} diff --git a/superset/utils/slack.py b/superset/utils/slack.py new file mode 100644 index 0000000000000..bf07653a4019e --- /dev/null +++ b/superset/utils/slack.py @@ -0,0 +1,9 @@ +from flask import current_app +from slack_sdk import WebClient + + +def get_slack_client() -> WebClient: + token: str = current_app.config["SLACK_API_TOKEN"] + if callable(token): + token = token() + return WebClient(token=token, proxy=current_app.config["SLACK_PROXY"]) diff --git a/superset/views/users/api.py b/superset/views/users/api.py index 5324975637d36..f26d78757c902 100644 --- a/superset/views/users/api.py +++ b/superset/views/users/api.py @@ -14,10 +14,14 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from flask import g, Response +from flask import g, redirect, Response from flask_appbuilder.api import expose, safe from flask_jwt_extended.exceptions import NoAuthorizationError +from superset import app, security_manager +from superset.extensions import db +from superset.models.user_attributes import UserAttribute +from superset.utils.slack import get_slack_client from superset.views.base_api import BaseSupersetApi from superset.views.users.schemas import UserResponseSchema from superset.views.utils import bootstrap_user_data @@ -93,3 +97,78 @@ def get_my_roles(self) -> Response: return self.response_401() user = bootstrap_user_data(g.user, include_perms=True) return self.response(200, result=user) + + +class UserRestApi(BaseSupersetApi): + """An API to get information about users""" + + resource_name = "user" + openapi_spec_tag = "User" + openapi_spec_component_schemas = (UserResponseSchema,) + + @expose("//avatar.png", methods=("GET",)) + @safe + def avatar(self, user_id: str) -> Response: + """Get the user roles corresponding to the agent making the request. + --- + get: + summary: Get the user roles + description: >- + Gets the user roles corresponding to the agent making the request, + or returns a 401 error if the user is unauthenticated. + responses: + 200: + description: The current user + content: + application/json: + schema: + type: object + properties: + result: + $ref: '#/components/schemas/UserResponseSchema' + 401: + $ref: '#/components/responses/401' + """ + try: + avatar_url = None + if not user_id: + return self.response_401() + + # Fetching the user's avatar from the database + user_attrs = ( + db.session.query(UserAttribute).filter_by(user_id=user_id).first() + ) + + if not user_attrs or not user_attrs.avatar_url: + if app.config.get("SLACK_ENABLE_AVATARS"): + user = ( + db.session.query(security_manager.user_model) + .filter_by(id=user_id) + .first() + ) + # Fetching the user's avatar from slack + client = get_slack_client() + response = client.users_lookupByEmail(email=user.email) + avatar_url = ( + response.data.get("user").get("profile").get("image_192") + ) + + # Saving the avatar url to the database + user_attrs = UserAttribute() + user_attrs.user_id = user_id + user_attrs.avatar_url = avatar_url + db.session.add(user_attrs) + db.session.commit() + elif user_attrs: + avatar_url = user_attrs.avatar_url + + if avatar_url: + return redirect(avatar_url, code=301) + else: + return self.response_404() + + except NoAuthorizationError: + return self.response_401() + return self.response( + 200, result=response.data.get("user").get("profile").get("image_192") + )