Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reporting][help needed]New feature request for reporting and alert #13954

Closed
junlincc opened this issue Apr 5, 2021 · 22 comments
Closed

[Reporting][help needed]New feature request for reporting and alert #13954

junlincc opened this issue Apr 5, 2021 · 22 comments
Labels
alert-reports Namespace | Anything related to the Alert & Reports feature enhancement:request Enhancement request submitted by anyone from the community

Comments

@junlincc
Copy link
Member

junlincc commented Apr 5, 2021

Superset Roadmap item: apache-superset/superset-roadmap#51

Request: Users, instead of going to dashboards, would like to receive data via email: PDF,
Screenshot, Link to dashboard or info “your data is available”. Based on alerts or Anomaly detection or data availability.

Gaps in current Superset reporting:

Setting up email / report schedules

  • [Onboarding]Add tooltips to the UI - even with the improved UI/UX it’s unclear what some of the fields in the setup mean
  • [Timezone settings] - currently only (default) option is UTC, ideally user can select their timezone (e.g select PST in a dropdown) and then setup time in PST

Management of email schedules

  • [Configuration] Admin can configure whether anyone can add anyone to email schedules or limited to above points.

  • [Subscription]User can sign up for themselves and others to receive dashboard emails (setup dashboard to receive via email)

  • [Schedule management]Ability for a user (consumers) to see all their scheduled charts / dashboards (somewhat possible if they filter; maybe filter on their own schedules should be default in the UI)

  • [View subscription]Ability for chart / dashboard owners to easily see who is subscribed to their dashboard

When a change to subscription is made, the recipient & dashboard owner could be notified about the change.

Security

  • [RBAC] Security: All emails should be generated on the recipients permissions to access the data within the dashboard / chart.

Solution proposal: Check access to the section of dashboard which is being email; if user has access to all charts - include them in email; otherwise - just send them link to dashboard. → this addresses some of the perf. Concerns about each email being generated separately for permissions reason.

  • [RBAC] Dashboard owner can restrict “my dashboard is never emailable” → this addresses part of the Security issue.

Defining what to send - Tabs, Filters, Recipient

  • [Tabs]Handling dashboards with multiple tabs: The email subscription should be per tab - user can select which tab to email or even select multiple tabs (?combination of nested tabs?)

  • [Filters] Being able to send dashboard tab with defined filters - not just default filters

  • [Recipient] Ability to send email to “Dashboard owner” - in case owners change, recipients also change

  • [Text] Ability to define short text/content to be included at the top of the email - currently it includes link to dashboard, users would like to include a short message, link to documentation, contact info, etc.

Format of email

  • [PDF] Send in PDF not screenshot (PDF is standard, Looker, Tableau both send PDF)

  • [Resolution] Increase the resolution of alert email screenshot

  • [Mobile] friendly views - needed for executive reporting

Tooltips / visibility into values: Lot of charts are useful only with tooltips, since values are not displayed on the chart by default. If the chart has description it can be toggled on charts being sent via email.

Core functionality

  • [Triggering]Trigger not based on specified time or alert, but based on “last week’s data landed”

Meaning: instead of setting "send this dashboard every Monday 10am", set up as "as soon as the data for week lands (week ending sunday) send this dashboard" (it could be monday 10 am or 1pm)

  • [cleanup] mechanism to prevent email schedules which are not longer used running

Concerns:
Performance, especially if each request is processed separately to apply user’s specific permissions. Can be executed on batch cluster (instead of dashboard cluster)
Caching / data freshness - by default it uses data from cache - currently expected behavior but based on the time of the report data might be a day old (should be addressed longer term)

@junlincc junlincc added enhancement:request Enhancement request submitted by anyone from the community alert-reports Namespace | Anything related to the Alert & Reports feature labels Apr 5, 2021
@junlincc
Copy link
Member Author

junlincc commented Apr 5, 2021

^^ @daniel10012 @zuzana-vej @amitmiran137 @EBoisseauSierra @willbarrett

Do we see any above items that are aligned with our own org's goals? Can we collaborate on this project? thanks!

@junlincc junlincc changed the title [Reporting] New feature request for reporting and alert [Reporting][help needed]New feature request for reporting and alert Apr 5, 2021
@EBoisseauSierra
Copy link
Contributor

Wow! Thanks for summing that up, this sounds very exciting! Your summary covers already a lot (and more than what we could have imagined).

I quite like the idea to be able to send one single report, and use RBAC to automagically customise it per recipient. This suppose that each addressee is also a user of the Superset instance. This isn't a bad thing from a security perspective, yet might make it overcomplicated if people need to setup an account just to be able to receive an email. (Fall back to the Public role?)
A possibility could be to enable sending report to non-Superset-users. In that case, a nice to have would be for the dashboard owner to whitelist which domain name(s) the report can be sent to (e.g. only to *@mycorp.com and *@cooldivision.io).
(And, in general, it could be interesting to be able to configure Superset so that one can restrict outbound email to certain domain name(s).)

Re caching, a “force refresh graphs” could be added as an option.

As for attachment, we might want to be able to add PDF and/or CSV.

I'm very much looking forward to it! 🚀

@krsnik93
Copy link
Contributor

krsnik93 commented Apr 6, 2021

Email should contain an "Unsubscribe" link.

What was useful to me in some other BI tools was a "Send now" button right where the schedule/content is being defined, so that the user can immediately test out the report. Workaround is for the user to create a schedule sending in a minute or similar, but then it needs to be canceled as well, so a one time event is nicer.

@kamalkeshavani-aiinside
Copy link
Contributor

One concern I have in current implementation is regarding RLS.
When I have a dashboard/chart with RLS control enabled, I can't schedule email/alerts to different users, since they are generated by admin rights showing all the data to all the users. But if we try to control this, then we have to limit the recipients to only Superset users as mentioned by @EBoisseauSierra, which can be troublesome in some cases.

@john-bodley
Copy link
Member

john-bodley commented Apr 8, 2021

TL;DR Feature request changes to remedy security vulnerabilities

Though the reporting and alerting feature is great and I can understand the desire for Slack integrations for alerting etc., I and others sense this exposes a fairly major data security vulnerability, i.e., users could be exposed (via email or Slack) to information which they do not have permission to via the Superset UI.

Ensuring Superset adheres to the defined security policy should trump any desired functionality. Superset needs to have a sound concise and do the right thing, though any downstream actions taken by individuals, i.e., forwarding of emails etc., is outside of Superset's jurisdiction.

I propose the following changes should be made with the understanding that functionality will be impacted:

  1. The Slack and generic email notification methods be disabled. This exposes a fairly major security vulnerability as Superset has no a priori knowledge which individuals are members of said Slack or email group.
  2. Recipients should be Superset users (see screenshot). Users are emailed content individually according to the email registered in their profile.
  3. The screenshots are captured on behalf of each user. This ensures what they receive is identical to their interactive Superset experience. Though this requires more requests, if run sequentially the chart payloads will be cached and thus the overhead should be manageable. Note this is a more desirable user experience than checking whether said user can access the entire dashboard in question and emailing the image only if this is the case.

These changes should be fairly minor in terms of engineering work and do remedy the security issues. Down the road we can explore how we could integrate with Slack et al. in a more secure way.

Screenshots

Before

Screen Shot 2021-04-09 at 9 07 52 AM

After

Screen Shot 2021-04-09 at 9 12 17 AM

cc: @graceguo-supercat @junlincc @nytai @willbarrett @zuzana-vej

@junlincc
Copy link
Member Author

junlincc commented Apr 9, 2021

@EBoisseauSierra @krsnik93
I'm wondering if any of those interests you and your org, lmk if you wanna work on those. PDF seems to be a small size project.
What was useful to me in some other BI tools was a "Send now" button right where the schedule/content is being defined, so that the user can immediately test out the report.
This should also be a small item :)

@zuzana-vej
Copy link
Contributor

This is also small project and very high value based on user feedback (currently the email already includes link to the dashboard):

[Text] Ability to define short text/content to be included at the top of the email - currently it includes link to dashboard, users would like to include a short message, link to documentation, contact info, etc.

@nytai
Copy link
Member

nytai commented Apr 9, 2021

@zuzana-vej we have a PR open for including the description in the alert #13827

@zuzana-vej
Copy link
Contributor

Discussion Issue for how to handle Dashboards with multiple tabs: #14056

@john-bodley
Copy link
Member

@amitmiran137 @bkyryliuk @eschutho @nytai @willbarrett we discussed the security concerns I raised above during today's Superset Meetup (notes). I realize the proposed solution limits functionality and adds some complexity if you wanted to send a group email, i.e., you would need to create a user (with the appropriate permissions) with said email address, but it does close a fairly large security concern.

Does anyone have concerns with the proposed approach and/or ideas for an alternative formulation?

@nytai
Copy link
Member

nytai commented Apr 16, 2021

Can we make this optional? It would be a significant change to the current implementation and would break various use cases that are currently supported, and have been for a few released versions.

@john-bodley
Copy link
Member

I think the challenge with making it optional is it muddies the waters in terms of security, i.e., ideally the security manager should serve as the sole overlord for managing all aspects of security (including cases of relinquishing).

@kamalkeshavani-aiinside
Copy link
Contributor

How about we keep this behavior(new proposed) as default, but allow the user to enable less-secure(current implementation) reporting features from config file if they want to use the current behavior even with security-vulnerabilities.

The config file can include a comment giving a brief of security concerns along with a warning that these features may be removed in future versions.

@bkyryliuk
Copy link
Member

notes

Superset users currently can screenshot / download dashboards and send them to email / slack. That behavior doesn't change. Different organizations have different levels of trust for the employees to handle data.
I agree that screenshots rendering should be impersonated and superset permissions applied in alerts view.

Controlling recipients seems to be an overkill to me. However we could definitely benefit from the pluggable middle layer that could analyze requested dashboard / slice & recipients and make a call if company security practices permit it.

@fancyfreeman
Copy link

The new features are very exciting.
I have a suggestion that in alter and reporting, different dashboard contents should be sent according to the permissions and roles of subscribers. In my application scenario, users view different content according to RLS filter.

@john-bodley
Copy link
Member

john-bodley commented May 12, 2021

@bkyryliuk @kamalkeshavani-aiinside @nytai: @etr2460, @graceguo-supercat and myself discussed this in more detail and we feel at Airbnb we can close the current security vulnerability with custom overrides in our security manager. The TL;DR is we would perform a check on behalf of the members of the email or Slack group and deny access to the chart if any member does not have access. We'll leverage the Google Directory and Slack APIs to enumerate the group members from the information provided in the modal.

In order to achieve this there would be two minor changes:

  1. Add a config option to define valid email domains and add frontend validation in the modal, i.e., we want to ensure that emails can only be sent to the @airbnb.com domain.
  2. Add the email or Slack group to the Flask.g context which the security manager can access. This approach feels cleaner than having to pass the information through a litany of function calls.

@younsb
Copy link

younsb commented Jun 4, 2021

Can you check this bug #14407 when you are developing this feature ?

@ReninPrince
Copy link

ReninPrince commented Aug 23, 2021

Would really love to see these updates in my superset meanwhile are there any current workarounds to get the Dashboard as PDF. I have used JS to get the dashboard as PDF but it's having many setbacks. And also is there any date mentioned for updated version release.

@wulfuric
Copy link

Are there any updates on this, specifically the RBAC portion that would allow reports to be generated with the users scope that it is being sent to?

Our use case is that we are using OIDC, so all users in the org can login, but all of our dashboards are locked down with row level security like username LIKE '%{{current_username()}}%', we would like to be able to email reports showing only the relevant data to users. In our case we would use the same email associated with the superset user, but any filtering or parameterization in the reports functionality would be helpful.

We would also consider doing this ourselves, by hitting the superset API with a WHERE clause to get an image of the chart, but this doesn't seem supported either. Are there any workarounds or options to support this currently?

@TheHassi
Copy link

I wrote for my company a little python tool, because we need to store and send PDF's.
You can download Charts via API from superset and design your PDF report via LaTeX.

For those, who want to use it:
https://pypi.org/search/?q=superset-pdf-report

@rvsoni
Copy link

rvsoni commented Jan 17, 2022

Can we set Report Filters value before exporting data, like we can use Dashboard filter,

@eschutho
Copy link
Member

eschutho commented Jan 21, 2022

I added a few PRs that have addressed some of the points in the ticket description for reference

Can we set Report Filters value before exporting data, like we can use Dashboard filter,

@rvsoni Yes, I believe someone is actively working on this.

@apache apache locked and limited conversation to collaborators Feb 2, 2022
@geido geido converted this issue into discussion #18472 Feb 2, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
alert-reports Namespace | Anything related to the Alert & Reports feature enhancement:request Enhancement request submitted by anyone from the community
Projects
None yet
Development

No branches or pull requests