-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
Implement permission request/approve flow. #1095
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
"""Add access_request table to manage requests to access datastores. | ||
|
||
Revision ID: 5e4a03ef0bf0 | ||
Revises: 41f6a59a61f2 | ||
Create Date: 2016-09-09 17:39:57.846309 | ||
|
||
""" | ||
from alembic import op | ||
import sqlalchemy as sa | ||
|
||
# revision identifiers, used by Alembic. | ||
revision = '5e4a03ef0bf0' | ||
down_revision = 'b347b202819b' | ||
|
||
|
||
def upgrade(): | ||
op.create_table( | ||
'access_request', | ||
sa.Column('created_on', sa.DateTime(), nullable=True), | ||
sa.Column('changed_on', sa.DateTime(), nullable=True), | ||
sa.Column('id', sa.Integer(), nullable=False), | ||
sa.Column('datasource_type', sa.String(length=200), nullable=True), | ||
sa.Column('datasource_id', sa.Integer(), nullable=True), | ||
sa.Column('changed_by_fk', sa.Integer(), nullable=True), | ||
sa.Column('created_by_fk', sa.Integer(), nullable=True), | ||
sa.ForeignKeyConstraint(['changed_by_fk'], ['ab_user.id'], ), | ||
sa.ForeignKeyConstraint(['created_by_fk'], ['ab_user.id'], ), | ||
sa.PrimaryKeyConstraint('id') | ||
) | ||
|
||
|
||
def downgrade(): | ||
op.drop_table('access_request') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
{% extends "caravel/basic.html" %} | ||
{% block title %}{{ _("No Access!") }}{% endblock %} | ||
{% block body %} | ||
{% include "caravel/flash_wrapper.html" %} | ||
<div class="container"> | ||
<h4> | ||
{{ _("You do not have permissions to access the datasource %(name)s.", | ||
name=datasource_name) | ||
}} | ||
</h4> | ||
<div id="buttons"> | ||
<button onclick="window.location.href = '{{ request_access_url }}';" | ||
id="request" | ||
> | ||
{{ _("Request Permissions") }} | ||
</button> | ||
<button onclick="window.location.href = '{{ slicemodelview_link }}';" | ||
id="cancel" | ||
> | ||
{{ _("Cancel") }} | ||
</button> | ||
</div> | ||
</div> | ||
{% endblock %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -212,6 +212,32 @@ def process_result_value(self, value, dialect): | |
|
||
def init(caravel): | ||
"""Inits the Caravel application with security roles and such""" | ||
ADMIN_ONLY_VIEW_MENUES = set([ | ||
'ResetPasswordView', | ||
'RoleModelView', | ||
'Security', | ||
'UserDBModelView', | ||
'SQL Lab <span class="label label-danger">alpha</span>', | ||
'AccessRequestsModelView', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably need a new role for people who can grant rights to others that are not admin. It doesn't have to be part of this PR, your call. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree, separate PR |
||
]) | ||
|
||
ADMIN_ONLY_PERMISSIONS = set([ | ||
'can_sync_druid_source', | ||
'can_approve', | ||
]) | ||
|
||
ALPHA_ONLY_PERMISSIONS = set([ | ||
'all_datasource_access', | ||
'can_add', | ||
'can_download', | ||
'can_delete', | ||
'can_edit', | ||
'can_save', | ||
'datasource_access', | ||
'database_access', | ||
'muldelete', | ||
]) | ||
|
||
db = caravel.db | ||
models = caravel.models | ||
config = caravel.app.config | ||
|
@@ -223,44 +249,34 @@ def init(caravel): | |
merge_perm(sm, 'all_datasource_access', 'all_datasource_access') | ||
|
||
perms = db.session.query(ab_models.PermissionView).all() | ||
# set alpha and admin permissions | ||
for perm in perms: | ||
if ( | ||
perm.permission and | ||
perm.permission.name in ('datasource_access', 'database_access')): | ||
continue | ||
if perm.view_menu and perm.view_menu.name not in ( | ||
'ResetPasswordView', | ||
'RoleModelView', | ||
'Security', | ||
'UserDBModelView', | ||
'SQL Lab'): | ||
if ( | ||
perm.view_menu and | ||
perm.view_menu.name not in ADMIN_ONLY_VIEW_MENUES and | ||
perm.permission and | ||
perm.permission.name not in ADMIN_ONLY_PERMISSIONS): | ||
|
||
sm.add_permission_role(alpha, perm) | ||
sm.add_permission_role(admin, perm) | ||
|
||
gamma = sm.add_role("Gamma") | ||
public_role = sm.find_role("Public") | ||
public_role_like_gamma = \ | ||
public_role and config.get('PUBLIC_ROLE_LIKE_GAMMA', False) | ||
|
||
# set gamma permissions | ||
for perm in perms: | ||
if ( | ||
perm.view_menu and perm.view_menu.name not in ( | ||
'ResetPasswordView', | ||
'RoleModelView', | ||
'UserDBModelView', | ||
'SQL Lab', | ||
'Security') and | ||
perm.view_menu and | ||
perm.view_menu.name not in ADMIN_ONLY_VIEW_MENUES and | ||
perm.permission and | ||
perm.permission.name not in ( | ||
'all_datasource_access', | ||
'can_add', | ||
'can_download', | ||
'can_delete', | ||
'can_edit', | ||
'can_save', | ||
'datasource_access', | ||
'database_access', | ||
'muldelete', | ||
)): | ||
perm.permission.name not in ADMIN_ONLY_PERMISSIONS and | ||
perm.permission.name not in ALPHA_ONLY_PERMISSIONS): | ||
sm.add_permission_role(gamma, perm) | ||
if public_role_like_gamma: | ||
sm.add_permission_role(public_role, perm) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related to my other comment about auditing who granted what to who, I'd add:
state as ('requested', 'granted' and maybe eventually 'denied')
processed_by_userid Integer
processed_on Datetime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've refactored the code in a way that
@log_this
will have all needed information