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

feat: Added datadoc and list access permissions for user groups #1297

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
533448b
feat: Added datadoc and list permissions for user groups
Jun 15, 2023
e63a465
Merge branch 'pinterest:master' into external/group-permissions
jij1949 Jul 19, 2023
a85dca6
feat: changed nested ternary to if else
jij1949 Aug 15, 2023
2c435af
feat: created unit test class, empty
jij1949 Dec 5, 2023
aef40bc
feat: combined permission checking into a single generic function
jij1949 Dec 7, 2023
afe20ac
feat: resolved PR comments
jij1949 Dec 8, 2023
84ee6ca
feat: fixed permission enum import
jij1949 Dec 8, 2023
d8c90ef
fix: fixed inherited permissions not converting to proper read/write …
jij1949 Jan 4, 2024
94f6916
fix: added [inherited] to board permissions
jij1949 Jan 5, 2024
e51374b
feat: fixed doc_id -> data_doc_id
jij1949 Dec 7, 2023
be9ac2b
Merge branch 'external/group-permissions' of https://github.com/jij19…
jij1949 Feb 6, 2024
e746826
fix: cast booleans to integer in cte
jij1949 Feb 6, 2024
4504953
fix: added more description of cte returned value
jij1949 Feb 6, 2024
0af40d8
feat: added more comments in cte function
jij1949 Feb 6, 2024
748eea0
feat: added docs for group permissions
jij1949 Feb 6, 2024
13d7874
feat: resolving merge conflicts
jij1949 Feb 6, 2024
bbbf5fe
Merge branch 'master' of https://github.com/jij1949/querybook into ex…
jij1949 Feb 6, 2024
930c24e
Delete querybook/tests/test_lib/test_permission/test_permission.py
jij1949 Feb 6, 2024
2be50d2
Delete querybook/server/scripts/update_users.py
jij1949 Feb 6, 2024
d982443
fix: changed the way to cast boolean to int in cte
jij1949 Feb 6, 2024
952feff
Merge branch 'external/group-permissions' of https://github.com/jij19…
jij1949 Feb 6, 2024
43fd016
fix: added check for is_group is null so that older users can be assi…
jij1949 Feb 9, 2024
788542b
feat: resolved PR comments
jij1949 Feb 28, 2024
06f7bb8
feat: updated package.json version
jij1949 Feb 28, 2024
d7fd3a2
feat: resolved merge conflicts and updated package.json
jij1949 Mar 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion docs_website/docs/developer_guide/users_and_groups.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ A user or a group can have below properties
- profile_img: Profile image url of a user or a group.
- deleted: To indicate if this user has been deleted/deactivated or not.
- is_group: To indicate it is a user group if it's true.
- properties: Any addiontal properties are stored in this column. It is a freeform JSON object, you can basically add any properties inside it. By default all the properties inside `properties` are private, which are invisiable to end users.
- properties: Any additional properties are stored in this column. It is a freeform JSON object, you can basically add any properties inside it. By default all the properties inside `properties` are private, which are invisiable to end users.
- public_info: Only properties stored inside `properties.public_info` are visible to end users.
- description: [optional] Description of a user or a group.

Expand All @@ -31,3 +31,11 @@ For users
- oauth/ldap authentication: they both support auto-creation of users.

For user groups, they can only be sync'ed now and can not be created on UI. You can have a scheduled task for your organization to sync them from either ldap, metastore or any other system which contains user groups.


## Group Permissions
Querybook users can inherit permissions from groups to gain access to boards and datadocs alongside their explicit permissions. Likewise, user groups can also inherit permissions from other user groups. Whether a user or group is a member of two distinct groups or inherits permissions from multiple nested groups, the most permissive permissions are applied.

For example, if a user is a member of two groups, one with read-only access and the other with write access, the user will have write access. If a user is a member of a group with read-only access and a group with no access, the user will have read-only access.

User groups can be used to manage permissions for a large number of users. For example, you can create a group for all users in a team and assign permissions to the group. When a new user joins the team, you can add them to the group and they will inherit all of the permissions of the group. However, user groups cannot be assigned as the owners of a board or datadoc.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "querybook",
"version": "3.31.2",
"version": "3.32.0",
"description": "A Big Data Webapp",
"private": true,
"scripts": {
Expand Down
6 changes: 6 additions & 0 deletions querybook/server/const/permissions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from enum import Enum


class Permission(Enum):
READ = "read"
WRITE = "write"
8 changes: 7 additions & 1 deletion querybook/server/datasources/board.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@
from const.datasources import RESOURCE_NOT_FOUND_STATUS_CODE
from logic import board as logic
from logic import user as user_logic
from logic.board_permission import assert_can_read, assert_can_edit, assert_is_owner
from logic.board_permission import (
assert_can_read,
assert_can_edit,
assert_is_owner,
assert_is_not_group,
)
from logic.query_execution import get_environments_by_execution_id

from models.board import Board, BoardItem
Expand Down Expand Up @@ -293,6 +298,7 @@ def update_board_owner(board_id, next_owner_id, originator=None):
with DBSession() as session:
# Add previous owner as an editor to the doc
assert_is_owner(board_id, session=session)
assert_is_not_group(next_owner_id, session=session)
current_owner_editor = logic.create_board_editor(
board_id=board_id,
uid=current_user.id,
Expand Down
8 changes: 7 additions & 1 deletion querybook/server/datasources/datadoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@
schedule as schedule_logic,
user as user_logic,
)
from logic.datadoc_permission import assert_can_read, assert_can_write, assert_is_owner
from logic.datadoc_permission import (
assert_can_read,
assert_can_write,
assert_is_owner,
assert_is_not_group,
)
from logic.query_execution import get_query_execution_by_id
from logic.schedule import (
run_and_log_scheduled_task,
Expand Down Expand Up @@ -622,6 +627,7 @@ def update_datadoc_owner(doc_id, next_owner_id, originator=None):
with DBSession() as session:
# Add previous owner as an editor to the doc
assert_is_owner(doc_id, session=session)
assert_is_not_group(next_owner_id, session=session)
current_owner_editor = logic.create_data_doc_editor(
data_doc_id=doc_id,
uid=current_user.id,
Expand Down
27 changes: 25 additions & 2 deletions querybook/server/logic/board.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import datetime

from sqlalchemy import or_
from app.db import with_session
from const.elasticsearch import ElasticsearchItem
from models.board import Board, BoardItem, BoardEditor
from models.access_request import AccessRequest
from lib.sqlalchemy import update_model_fields
from tasks.sync_elasticsearch import sync_elasticsearch
from sqlalchemy import or_
from logic.generic_permission import get_all_groups_and_group_members_with_access


@with_session
Expand Down Expand Up @@ -232,13 +233,35 @@ def get_board_editor_by_id(id, session=None):

@with_session
def get_board_editors_by_board_id(board_id, session=None):
return session.query(BoardEditor).filter_by(board_id=board_id).all()
editors = get_all_groups_and_group_members_with_access(
doc_or_board_id=board_id,
editor_type=BoardEditor,
session=session,
)

return [
BoardEditor(
# [0] is id, [1] is uid, [2] is read, [3] is write
board_id=board_id,
id=editor[0],
uid=editor[1],
read=editor[2],
write=editor[3],
)
for editor in editors
]


@with_session
def create_board_editor(
board_id, uid, read=False, write=False, commit=True, session=None
):
existing_editor = (
session.query(BoardEditor).filter_by(board_id=board_id, uid=uid).first()
)
if existing_editor is not None:
return existing_editor

editor = BoardEditor(board_id=board_id, uid=uid, read=read, write=write)

session.add(editor)
Expand Down
44 changes: 25 additions & 19 deletions querybook/server/logic/board_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
ACCESS_RESTRICTED_STATUS_CODE,
RESOURCE_NOT_FOUND_STATUS_CODE,
)
from const.permissions import Permission

from models import User
from logic.generic_permission import user_has_permission


class BoardDoesNotExist(Exception):
Expand All @@ -23,15 +27,10 @@ def user_can_edit(board_id, uid, session=None):
if board.owner_uid == uid:
return True

editor = (
session.query(BoardEditor)
.filter(BoardEditor.board_id == board_id)
.filter(BoardEditor.uid == uid)
.first()
return user_has_permission(
board_id, Permission.WRITE, BoardEditor, uid, session=session
)

return editor is not None and editor.write


@with_session
def user_can_read(board_id, uid, session=None):
Expand All @@ -40,21 +39,13 @@ def user_can_read(board_id, uid, session=None):
if board is None:
raise BoardDoesNotExist()

if board.public:
return True

if board.owner_uid == uid:
if board.public or board.owner_uid == uid:
return True

editor = (
session.query(BoardEditor)
.filter(BoardEditor.board_id == board_id)
.filter(BoardEditor.uid == uid)
.first()
return user_has_permission(
board_id, Permission.READ, BoardEditor, uid, session=session
)

return editor is not None and (editor.read or editor.write)


@with_session
def assert_can_edit(board_id, session=None):
Expand Down Expand Up @@ -83,7 +74,7 @@ def assert_can_read(board_id, session=None):
@with_session
def assert_is_owner(board_id, session=None):
try:
board = session.query(Board).filter(Board.id == board_id).first()
board = session.query(Board).get(board_id)
if board is None:
raise BoardDoesNotExist
api_assert(
Expand All @@ -93,3 +84,18 @@ def assert_is_owner(board_id, session=None):
)
except BoardDoesNotExist:
api_assert(False, "BOARD_DNE", RESOURCE_NOT_FOUND_STATUS_CODE)


@with_session
def assert_is_not_group(id, session=None):
editor = session.query(BoardEditor).get(id)
if editor is None:
api_assert(False, "EDITOR_DNE", RESOURCE_NOT_FOUND_STATUS_CODE)
user = session.query(User).filter_by(id=editor.uid).first()
if user is None:
api_assert(False, "USER_DNE", RESOURCE_NOT_FOUND_STATUS_CODE)
api_assert(
user.is_group is False or user.is_group is None,
"Group cannot be assigned as owner",
ACCESS_RESTRICTED_STATUS_CODE,
)
26 changes: 24 additions & 2 deletions querybook/server/logic/datadoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from lib.sqlalchemy import update_model_fields
from lib.data_doc.data_cell import cell_types, sanitize_data_cell_meta
from logic.query_execution import get_last_query_execution_from_cell
from logic.generic_permission import get_all_groups_and_group_members_with_access
from models.datadoc import (
DataDoc,
DataDocDataCell,
Expand All @@ -29,7 +30,6 @@
sync_es_query_cells_by_datadoc_id,
)


"""
----------------------------------------------------------------------------------------------------------
DATA DOC
Expand Down Expand Up @@ -811,7 +811,23 @@ def get_data_doc_editor_by_id(id, session=None):

@with_session
def get_data_doc_editors_by_doc_id(data_doc_id, session=None):
return session.query(DataDocEditor).filter_by(data_doc_id=data_doc_id).all()
editors = get_all_groups_and_group_members_with_access(
doc_or_board_id=data_doc_id,
editor_type=DataDocEditor,
session=session,
)

return [
DataDocEditor(
# [0] is id, [1] is uid, [2] is read, [3] is write
data_doc_id=data_doc_id,
id=editor[0],
uid=editor[1],
read=editor[2],
write=editor[3],
)
for editor in editors
]


@with_session
Expand All @@ -823,6 +839,12 @@ def get_data_doc_writers_by_doc_id(doc_id, session=None):
def create_data_doc_editor(
data_doc_id, uid, read=False, write=False, commit=True, session=None
):
existing_editor = (
session.query(DataDocEditor).filter_by(data_doc_id=data_doc_id, uid=uid).first()
)
if existing_editor is not None:
return existing_editor

editor = DataDocEditor(data_doc_id=data_doc_id, uid=uid, read=read, write=write)

session.add(editor)
Expand Down
55 changes: 38 additions & 17 deletions querybook/server/logic/datadoc_permission.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
from flask_login import current_user
from sqlalchemy import and_

from app.datasource import api_assert
from app.db import with_session
from models.datadoc import DataDoc, DataDocEditor
from const.datasources import UNAUTHORIZED_STATUS_CODE, RESOURCE_NOT_FOUND_STATUS_CODE
from const.datasources import (
UNAUTHORIZED_STATUS_CODE,
RESOURCE_NOT_FOUND_STATUS_CODE,
ACCESS_RESTRICTED_STATUS_CODE,
)
from const.permissions import Permission

from models import User
from logic.generic_permission import user_has_permission


class DocDoesNotExist(Exception):
Expand All @@ -13,34 +20,33 @@ class DocDoesNotExist(Exception):

@with_session
def user_can_write(doc_id, uid, session=None):
doc, editor = session.query(DataDoc, DataDocEditor).outerjoin(
DataDocEditor,
and_(DataDoc.id == DataDocEditor.data_doc_id, DataDocEditor.uid == uid),
).filter(DataDoc.id == doc_id).first() or (None, None)
datadoc = session.query(DataDoc).get(doc_id)

if doc is None:
if datadoc is None:
raise DocDoesNotExist()

if doc.owner_uid == uid:
if datadoc.owner_uid == uid:
return True

return editor is not None and editor.write
return user_has_permission(
doc_id, Permission.WRITE, DataDocEditor, uid, session=session
)


@with_session
def user_can_read(doc_id, uid, session=None):
doc, editor = session.query(DataDoc, DataDocEditor).outerjoin(
DataDocEditor,
and_(DataDoc.id == DataDocEditor.data_doc_id, DataDocEditor.uid == uid),
).filter(DataDoc.id == doc_id).first() or (None, None)
# Check if the doc is public or if the user is the owner
datadoc = session.query(DataDoc).get(doc_id)

if doc is None:
if datadoc is None:
raise DocDoesNotExist()

if doc.public or doc.owner_uid == uid:
if datadoc.public or datadoc.owner_uid == uid:
return True

return editor is not None and (editor.read or editor.write)
return user_has_permission(
doc_id, Permission.READ, DataDocEditor, uid, session=session
)


@with_session
Expand Down Expand Up @@ -70,7 +76,7 @@ def assert_can_write(doc_id, session=None):
@with_session
def assert_is_owner(doc_id, session=None):
try:
doc = session.query(DataDoc).filter(DataDoc.id == doc_id).first()
doc = session.query(DataDoc).get(doc_id)
if doc is None:
raise DocDoesNotExist
api_assert(
Expand All @@ -80,3 +86,18 @@ def assert_is_owner(doc_id, session=None):
)
except DocDoesNotExist:
api_assert(False, "DOC_DNE", RESOURCE_NOT_FOUND_STATUS_CODE)


@with_session
def assert_is_not_group(id, session=None):
editor = session.query(DataDocEditor).get(id)
if editor is None:
api_assert(False, "EDITOR_DNE", RESOURCE_NOT_FOUND_STATUS_CODE)
user = session.query(User).filter_by(id=editor.uid).first()
if user is None:
api_assert(False, "USER_DNE", RESOURCE_NOT_FOUND_STATUS_CODE)
api_assert(
user.is_group is False or user.is_group is None,
"Group cannot be assigned as owner",
ACCESS_RESTRICTED_STATUS_CODE,
)
6 changes: 2 additions & 4 deletions querybook/server/logic/elasticsearch.py
Original file line number Diff line number Diff line change
Expand Up @@ -740,10 +740,8 @@ def _bulk_insert_users():
index_name = ES_CONFIG["users"]["index_name"]

for user in get_users_iter():
# skip indexing user groups before having the correct permission setup for it.
if not user.is_group:
expanded_user = user_to_es(user, fields=None)
_insert(index_name, expanded_user["id"], expanded_user)
expanded_user = user_to_es(user, fields=None)
_insert(index_name, expanded_user["id"], expanded_user)


def _bulk_update_users(fields: Set[str] = None):
Expand Down
Loading
Loading