diff --git a/docs_website/docs/developer_guide/users_and_groups.mdx b/docs_website/docs/developer_guide/users_and_groups.mdx index 823ebc139..b8de00e0e 100644 --- a/docs_website/docs/developer_guide/users_and_groups.mdx +++ b/docs_website/docs/developer_guide/users_and_groups.mdx @@ -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. @@ -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. diff --git a/package.json b/package.json index b36beb417..ca11426e0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "querybook", - "version": "3.31.2", + "version": "3.32.0", "description": "A Big Data Webapp", "private": true, "scripts": { diff --git a/querybook/server/const/permissions.py b/querybook/server/const/permissions.py new file mode 100644 index 000000000..f10178818 --- /dev/null +++ b/querybook/server/const/permissions.py @@ -0,0 +1,6 @@ +from enum import Enum + + +class Permission(Enum): + READ = "read" + WRITE = "write" diff --git a/querybook/server/datasources/board.py b/querybook/server/datasources/board.py index 9e2dd8f48..8c59e8cd6 100644 --- a/querybook/server/datasources/board.py +++ b/querybook/server/datasources/board.py @@ -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 @@ -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, diff --git a/querybook/server/datasources/datadoc.py b/querybook/server/datasources/datadoc.py index 618829437..3aaa0413f 100644 --- a/querybook/server/datasources/datadoc.py +++ b/querybook/server/datasources/datadoc.py @@ -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, @@ -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, diff --git a/querybook/server/logic/board.py b/querybook/server/logic/board.py index 717c852f4..2df2dd14d 100644 --- a/querybook/server/logic/board.py +++ b/querybook/server/logic/board.py @@ -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 @@ -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) diff --git a/querybook/server/logic/board_permission.py b/querybook/server/logic/board_permission.py index 51f0443bd..fe8ff4b55 100644 --- a/querybook/server/logic/board_permission.py +++ b/querybook/server/logic/board_permission.py @@ -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): @@ -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): @@ -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): @@ -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( @@ -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, + ) diff --git a/querybook/server/logic/datadoc.py b/querybook/server/logic/datadoc.py index 37ff2ddbf..35325a513 100644 --- a/querybook/server/logic/datadoc.py +++ b/querybook/server/logic/datadoc.py @@ -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, @@ -29,7 +30,6 @@ sync_es_query_cells_by_datadoc_id, ) - """ ---------------------------------------------------------------------------------------------------------- DATA DOC @@ -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 @@ -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) diff --git a/querybook/server/logic/datadoc_permission.py b/querybook/server/logic/datadoc_permission.py index d4e6265fa..51b93a81b 100644 --- a/querybook/server/logic/datadoc_permission.py +++ b/querybook/server/logic/datadoc_permission.py @@ -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): @@ -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 @@ -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( @@ -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, + ) diff --git a/querybook/server/logic/elasticsearch.py b/querybook/server/logic/elasticsearch.py index 157ec4232..4187a5a70 100644 --- a/querybook/server/logic/elasticsearch.py +++ b/querybook/server/logic/elasticsearch.py @@ -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): diff --git a/querybook/server/logic/generic_permission.py b/querybook/server/logic/generic_permission.py new file mode 100644 index 000000000..47d50dfe1 --- /dev/null +++ b/querybook/server/logic/generic_permission.py @@ -0,0 +1,139 @@ +from typing import List, Optional, Tuple, Union +from app.db import with_session +from models import UserGroupMember, User, DataDocEditor, BoardEditor +from sqlalchemy import func, select, Integer +from sqlalchemy.orm import Session +from const.permissions import Permission + + +@with_session +def get_all_groups_and_group_members_with_access( + doc_or_board_id: int, + editor_type: Union[DataDocEditor, BoardEditor], + uid: Optional[int] = None, + session: Optional[Session] = None, +) -> List[Tuple[int, int, bool, bool]]: + """ + Get all groups and group members with access to a DataDoc or Board. + + Args: + doc_or_board_id: The ID of the DataDoc or Board. + editor_type: The type of editor (DataDocEditor or BoardEditor). + uid: The ID of the user to filter by (optional) + session: The database session to use. + + Returns: + A list of tuples containing the editor ID, the group or user ID, and the most permissive read and write + permissions. This means that if a user has write=true but inherits write=false, their write will remain true. + Likewise, if a user has write=false and inherits write=true, their write will become true. This tuple will + appear in the form "(editor_id, uid, read, write)". Editors with inherited permissions have their editor_ID set + to None. + """ + + # Begin constructing the top query, starting by selecting editors of the required type (doc or board) + topq = session.query( + editor_type.id, editor_type.uid, editor_type.read, editor_type.write + ).select_from(editor_type) + + # Filter by the doc or board ID to get the editors for the specific doc or board + if editor_type == DataDocEditor: + topq = topq.filter(editor_type.data_doc_id == doc_or_board_id) + elif editor_type == BoardEditor: + topq = topq.filter(editor_type.board_id == doc_or_board_id) + + topq = topq.cte("cte", recursive=True) + + # This bottom query determines if the user is a group or a user, and then selects the group members + bottomq = ( + select([None, UserGroupMember.uid, topq.c.read, topq.c.write]) + .select_from(topq) + .join(User, topq.c.uid == User.id) + .join(UserGroupMember, UserGroupMember.gid == User.id) + .filter(User.is_group) + ) + + # This is then applied recursively to the top query + recursive_q = topq.union(bottomq) + + editors = recursive_q.alias() + + q = select( + [ + func.max(editors.c.id), + editors.c.uid, + func.max( + editors.c.read.cast(Integer) + ), # Get the most permissive read permissions + func.max( + editors.c.write.cast(Integer) + ), # Get the most permissive write permissions + ] + ).group_by(editors.c.uid) + + # Optionally filter by uid to get only the permissions for a specific user + if uid is not None: + q = q.filter(editors.c.uid == uid) + + return session.query(q.subquery()).all() + + +@with_session +def user_has_permission( + doc_or_board_id: int, + permission_level: Permission, + editor_type: Union[DataDocEditor, BoardEditor], + uid: int, + session: Optional[Session] = None, +) -> bool: + """ + Check if the user has the specified permission for the specified datadoc or board. + + Args: + doc_or_board_id: The ID of the DataDoc or Board. + permission_level: The permission level to check for (READ or WRITE). + editor_type: The type of editor (DataDocEditor or BoardEditor). + uid: The ID of the user. + session: The database session to use. + + Returns: + True or False depending on whether the user has the specified permission. + """ + if editor_type == BoardEditor: + editor = ( + session.query(BoardEditor) + .filter_by(uid=uid, board_id=doc_or_board_id) + .first() + ) + else: + editor = ( + session.query(DataDocEditor) + .filter_by(uid=uid, data_doc_id=doc_or_board_id) + .first() + ) + + # Check the user's direct permissions + if permission_level == Permission.READ: + if editor is not None and (editor.write or editor.read): + return True + elif permission_level == Permission.WRITE: + if editor is not None and editor.write: + return True + + # Get the user's inherited permissions + inherited_editors = get_all_groups_and_group_members_with_access( + doc_or_board_id=doc_or_board_id, + editor_type=editor_type, + uid=uid, + session=session, + ) + + if permission_level == Permission.READ: + if len(inherited_editors) == 1: + return True + elif permission_level == Permission.WRITE: + if len(inherited_editors) == 1: + # Check if the editor's write privileges are true + if inherited_editors[0][3]: + return True + + return False diff --git a/querybook/server/logic/user.py b/querybook/server/logic/user.py index 94c05c26d..bbc9ad6c6 100644 --- a/querybook/server/logic/user.py +++ b/querybook/server/logic/user.py @@ -194,6 +194,8 @@ def create_or_update_user_group(user_group: UserGroup, commit=True, session=None [UserGroupMember(gid=group.id, uid=user_id) for user_id in members_to_add] ) + update_es_users_by_id(group.id) + if commit: session.commit() else: diff --git a/querybook/webapp/components/BoardViewersList/BoardViewersList.tsx b/querybook/webapp/components/BoardViewersList/BoardViewersList.tsx index 63cbe406f..4b7ec0a47 100644 --- a/querybook/webapp/components/BoardViewersList/BoardViewersList.tsx +++ b/querybook/webapp/components/BoardViewersList/BoardViewersList.tsx @@ -80,7 +80,10 @@ export const BoardViewersList: React.FunctionComponent = ({ const handleUserSelect = React.useCallback( (uid: number) => { - if (uid in editorsByUid || uid === board.owner_uid) { + if ( + uid === board.owner_uid || + (uid in editorsByUid && editorsByUid[uid].id !== null) + ) { toast.error('User already added.'); } else { const newUserPermission = board.public diff --git a/querybook/webapp/components/DataDocViewersList/DataDocViewersList.tsx b/querybook/webapp/components/DataDocViewersList/DataDocViewersList.tsx index e6facdfa2..3b624ef90 100644 --- a/querybook/webapp/components/DataDocViewersList/DataDocViewersList.tsx +++ b/querybook/webapp/components/DataDocViewersList/DataDocViewersList.tsx @@ -76,7 +76,10 @@ export const DataDocViewersList: React.FunctionComponent< const onUserSelect = React.useCallback( (uid: number) => { - if (uid in editorsByUid || uid === dataDoc.owner_uid) { + if ( + uid === dataDoc.owner_uid || + (uid in editorsByUid && editorsByUid[uid].id !== null) + ) { toast.error('User already added.'); } else { const newUserPermission = dataDoc.public diff --git a/querybook/webapp/components/DataDocViewersList/ViewerPermissionPicker.tsx b/querybook/webapp/components/DataDocViewersList/ViewerPermissionPicker.tsx index 246eb2dfd..21b95e954 100644 --- a/querybook/webapp/components/DataDocViewersList/ViewerPermissionPicker.tsx +++ b/querybook/webapp/components/DataDocViewersList/ViewerPermissionPicker.tsx @@ -82,7 +82,7 @@ export const ViewerPermissionPicker: React.FunctionComponent = ({ viewerInfo.permission === Permission.CAN_WRITE; const pickerButton = - canShowEditMenu && !readonly ? ( + canShowEditMenu && viewerInfo.editorId != null && !readonly ? (
setShowEditMenu(true)} diff --git a/querybook/webapp/lib/data-doc/datadoc-permission.ts b/querybook/webapp/lib/data-doc/datadoc-permission.ts index 21f2f9069..1355d14d0 100644 --- a/querybook/webapp/lib/data-doc/datadoc-permission.ts +++ b/querybook/webapp/lib/data-doc/datadoc-permission.ts @@ -7,6 +7,8 @@ export enum Permission { CAN_WRITE = 'edit', OWNER = 'owner', NULL = 'no access', + INHERITED_READ = '[INHERITED] read only', + INHERITED_WRITE = '[INHERITED] edit', } export interface IViewerInfo { @@ -20,18 +22,21 @@ export function readWriteToPermission( read: boolean, write: boolean, isOwner: boolean, - publicDoc: boolean + publicDoc: boolean, + id: number ): Permission { if (isOwner) { return Permission.OWNER; } + if (id < 0) { + return Permission.NULL; + } if (write) { - return Permission.CAN_WRITE; + return id == null ? Permission.INHERITED_WRITE : Permission.CAN_WRITE; } - if (read || publicDoc) { - return Permission.CAN_READ; + if (read) { + return id == null ? Permission.INHERITED_READ : Permission.CAN_READ; } - return Permission.NULL; } @@ -39,14 +44,18 @@ export function permissionToReadWrite(permission: Permission): { read: boolean; write: boolean; } { - if (permission === Permission.CAN_READ) { + if ( + permission === Permission.CAN_READ || + permission === Permission.INHERITED_READ + ) { return { read: true, write: false, }; } else if ( permission === Permission.CAN_WRITE || - permission === Permission.OWNER + permission === Permission.OWNER || + permission === Permission.INHERITED_WRITE ) { return { read: true, @@ -64,14 +73,21 @@ export function getViewerInfo( uid: number, editorsByUserId: Record, dataDoc: IDataDoc, - viewerIds: number[] + viewerIds: number[], + nonExplicitEditorPermissions: Record ): IViewerInfo { - const editor = uid in editorsByUserId ? editorsByUserId[uid] : null; + let editor = null; + if (uid in editorsByUserId) { + editor = editorsByUserId[uid]; + } else if (uid in nonExplicitEditorPermissions) { + editor = nonExplicitEditorPermissions[uid]; + } const permission = readWriteToPermission( editor ? editor.read : false, editor ? editor.write : false, dataDoc.owner_uid === uid, - dataDoc.public + dataDoc.public, + editor ? editor.id : -1 ); const online = viewerIds.includes(uid); return { diff --git a/querybook/webapp/redux/board/selector.ts b/querybook/webapp/redux/board/selector.ts index 8482cfdf3..de3b5901b 100644 --- a/querybook/webapp/redux/board/selector.ts +++ b/querybook/webapp/redux/board/selector.ts @@ -140,16 +140,24 @@ export const canCurrentUserEditSelector = createSelector( export const boardEditorInfosSelector = createSelector( currentBoardSelector, boardEditorByUidSelector, - (board, editorsByUserId) => { + (state: IStoreState) => state.user.myUserInfo.uid, + (board, editorsByUserId, uid) => { + const newEditorsByUserId = Object.fromEntries( + Object.entries(editorsByUserId).filter( + // Filter out any editors inherited from groups + // (i.e. editors with a uid but no id) + ([_userId, editor]) => editor.id !== null || editor.uid === uid + ) + ); const allUserIds = [ ...new Set( [board.owner_uid].concat( - Object.keys(editorsByUserId).map(Number) + Object.keys(newEditorsByUserId).map(Number) ) ), ]; return allUserIds.map((uid) => - getEditorInfo(uid, editorsByUserId, board) + getEditorInfo(uid, newEditorsByUserId, board) ); } ); @@ -164,9 +172,9 @@ export function getEditorInfo( editor ? editor.read : false, editor ? editor.write : false, board.owner_uid === uid, - board.public + board.public, + editor ? editor.id : -1 ); - return { editorId: editor?.id, uid, diff --git a/querybook/webapp/redux/dataDoc/selector.ts b/querybook/webapp/redux/dataDoc/selector.ts index 991fb9e74..3ae9bd7d3 100644 --- a/querybook/webapp/redux/dataDoc/selector.ts +++ b/querybook/webapp/redux/dataDoc/selector.ts @@ -165,17 +165,35 @@ export const dataDocViewerInfosSelector = createSelector( dataDocSelector, dataDocEditorByUidSelector, dataDocViewerIdsSelector, - (dataDoc, editorsByUserId, viewerIds) => { + (state: IStoreState) => state.user.myUserInfo.uid, + (dataDoc, editorsByUserId, viewerIds, uid) => { + const newEditorsByUserId = Object.fromEntries( + Object.entries(editorsByUserId).filter( + // Filter out any editors inherited from groups + // (i.e. editors with a uid but no id) + ([_userId, editor]) => editor.id !== null || editor.uid === uid + ) + ); const allUserIds = [ ...new Set( [dataDoc.owner_uid] .concat(viewerIds) - .concat(Object.keys(editorsByUserId).map(Number)) + .concat(Object.keys(newEditorsByUserId).map(Number)) ), ]; + const nonExplicitEditorPermissions = {}; + for (const viewerId of viewerIds) { + nonExplicitEditorPermissions[viewerId] = editorsByUserId[viewerId]; + } return sortViewersInfo( allUserIds.map((uid) => - getViewerInfo(uid, editorsByUserId, dataDoc, viewerIds) + getViewerInfo( + uid, + newEditorsByUserId, + dataDoc, + viewerIds, + nonExplicitEditorPermissions + ) ) ); } @@ -189,13 +207,13 @@ export const canCurrentUserEditSelector = createSelector( if (!dataDoc) { return false; } - const editor = uid in editorsByUserId ? editorsByUserId[uid] : null; const permission = readWriteToPermission( editor ? editor.read : false, editor ? editor.write : false, dataDoc.owner_uid === uid, - dataDoc.public + dataDoc.public, + editor ? editor.id : -1 ); return permissionToReadWrite(permission).write; }