Skip to content
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

[READY] [feature] Added dashboard read only with new Rho role #5845

Closed
wants to merge 13 commits into from

Conversation

datinho
Copy link
Contributor

@datinho datinho commented Sep 10, 2018

I needed a user who could only access the read-only dashboards, so I've added a new built-in role Rho (ρ) as logged user but less powerful than Gamma.
As Gamma user this role need a complementary role to have access to view the dashboards.

@datinho datinho changed the title Added dashboard only Rho user Added dashboard only Rho role Sep 10, 2018
@codecov-io
Copy link

codecov-io commented Sep 10, 2018

Codecov Report

Merging #5845 into master will increase coverage by 8.22%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5845      +/-   ##
==========================================
+ Coverage   65.89%   74.11%   +8.22%     
==========================================
  Files         459      106     -353     
  Lines       22014    11522   -10492     
  Branches     2417        0    -2417     
==========================================
- Hits        14507     8540    -5967     
+ Misses       7386     2982    -4404     
+ Partials      121        0     -121
Impacted Files Coverage Δ
superset/config.py 94.04% <100%> (+0.03%) ⬆️
superset/cli.py 36.91% <100%> (+0.89%) ⬆️
superset/security.py 74.8% <91.66%> (+0.83%) ⬆️
superset/db_engine_specs/sqlite.py 45.83% <0%> (-20.84%) ⬇️
superset/utils/core.py 87.93% <0%> (-0.18%) ⬇️
superset/assets/src/components/Checkbox.jsx
...ations/deckgl/layers/Polygon/PolygonChartPlugin.js
...ets/src/dashboard/components/dnd/DragDroppable.jsx
...c/visualizations/deckgl/layers/Polygon/Polygon.jsx
superset/assets/src/components/EditableTitle.jsx
... and 348 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56eac68...d50f21f. Read the comment docs.

@datinho
Copy link
Contributor Author

datinho commented Sep 11, 2018

I've just modified the doc 'security.rst' but Travis CI returned a timeout during the step 'Creating a dashboard':

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

If this status persists I'll try to commit again in order to activate te CI.

@datinho datinho closed this Sep 12, 2018
@datinho datinho reopened this Sep 12, 2018
@datinho datinho closed this Sep 12, 2018
@datinho datinho reopened this Sep 12, 2018
@datinho datinho closed this Sep 13, 2018
@datinho datinho reopened this Sep 13, 2018
@datinho datinho force-pushed the dashboard-only-rho-user branch from 4d03160 to f0df9f8 Compare October 24, 2018 07:50
@datinho datinho changed the title Added dashboard only Rho role [feature] Added dashboard only Rho role Oct 24, 2018
@datinho
Copy link
Contributor Author

datinho commented Oct 24, 2018

Hi all,
I think a built-in read only role/user is missing and I've tried to be complaint with the new code review process.
I've re-based the dashboard-only-rho-user branch to the master and I hope that PR cuold be merged soon.
Thanks in advance.

@datinho datinho changed the title [feature] Added dashboard only Rho role [READY] [feature] Added dashboard only Rho role Oct 29, 2018
@mistercrunch
Copy link
Member

We'll need unit tests making sure that a users with this role can access the things we except them to be able to access and not other things.

@datinho
Copy link
Contributor Author

datinho commented Oct 30, 2018

Hi @mistercrunch ,
I never did an unit test but I'll try. Just a tip, do I modify only the tests/security_tests.py or is there other files to handle?
Thanks in advance.

@mistercrunch
Copy link
Member

@datinho datinho changed the title [READY] [feature] Added dashboard only Rho role [WIP] [feature] Added dashboard only Rho role Nov 7, 2018
@datinho datinho force-pushed the dashboard-only-rho-user branch from 6cb7753 to 26bfb7c Compare November 8, 2018 15:36
@datinho datinho changed the title [WIP] [feature] Added dashboard only Rho role [READY] [feature] Added dashboard only Rho role Nov 8, 2018
@datinho
Copy link
Contributor Author

datinho commented Nov 9, 2018

I've added the unit test, someone can kindly give me a feedback please?
@mistercrunch @ryw @jeremi

@datinho
Copy link
Contributor Author

datinho commented Nov 21, 2018

Hi @mistercrunch @ryw @jeremi
could you kindly check my PR?
Thanks.

@datinho
Copy link
Contributor Author

datinho commented Nov 30, 2018

@hughhhh @kristw could you kindly check my PR?
Thanks.

@kristw kristw added enhancement:request Enhancement request submitted by anyone from the community .security labels Jan 23, 2019
@datinho datinho changed the title [READY] [feature] Added dashboard only Rho role [READY] [feature] Added dashboard read only with new Rho role Mar 11, 2019
@datinho
Copy link
Contributor Author

datinho commented Apr 23, 2019

@timifasubaa @mistercrunch
I'm waiting a review from 3 months during I'm still work to avoid conflict.
Do you plan to review thi PR soon?
Thanks in advance.
CC: @kristw

@datinho
Copy link
Contributor Author

datinho commented Jul 4, 2019

@mistercrunch @timifasubaa
could you kindly review this PR please?
Thanks in advance.

@kalimuthu123
Copy link

Not worked for me i added code in all 5 files

@datinho datinho force-pushed the dashboard-only-rho-user branch from 3bd0402 to 87914e8 Compare September 10, 2019 10:45
@datinho
Copy link
Contributor Author

datinho commented Sep 12, 2019

The check fails on TOXENV=cypress-dashboard, it seems a timeout problem (Travis CI side?) that
I don't have on my local environment.

  1. Dashboard top-level controls should allow chart level refresh:
    CypressError: Timed out retrying: cy.wait() timed out waiting 5000ms for the 1st request to the route: 'postJson_8_force'. No request ever occurred.

Copy link

@kalimuthu123 kalimuthu123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good

@amitmiran137
Copy link
Member

#10408

@junlincc junlincc removed the review label Jan 3, 2021
@junlincc
Copy link
Member

Closing inactive PR in favor of #12680 @amitmiran137, thank you for your contribution! @datinho

@junlincc junlincc closed this Jan 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement:request Enhancement request submitted by anyone from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants