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

✨ Error pages when published studies dispatcher fails, access rights fixes and plugin refactoring #3962

Merged
merged 66 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from 65 commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
ceca6ba
constants
pcrespov Mar 10, 2023
bc93c4d
error handling as error page
pcrespov Mar 10, 2023
e1753c5
updates tests
pcrespov Mar 10, 2023
92be628
some cleanup
pcrespov Mar 10, 2023
58ad06a
adds expiration of guest user in settings
pcrespov Mar 14, 2023
8182306
adds expiration to guest users
pcrespov Mar 14, 2023
5cc042c
updates TODOs
pcrespov Mar 14, 2023
55fa5b0
wip
pcrespov Mar 14, 2023
8e4d6cc
updates integration catalog_subsystem_mock
pcrespov Mar 14, 2023
6a8a0f6
annotate catalog_subsystem_mock
pcrespov Mar 14, 2023
55fc35b
test settings
pcrespov Mar 14, 2023
b253d7b
fixes error
pcrespov Mar 14, 2023
28216c1
minor
pcrespov Mar 14, 2023
27bd709
fixes expires_at
pcrespov Mar 14, 2023
b64aa33
fixes in expired_at
pcrespov Mar 14, 2023
0d150e1
fixes in expired_at
pcrespov Mar 14, 2023
c89e6b7
cleanup fixtures
pcrespov Mar 14, 2023
fe7a68e
minor
pcrespov Mar 14, 2023
b7a13a9
cleanup
pcrespov Mar 15, 2023
6f5828a
cleanup validator
pcrespov Mar 15, 2023
1a1d114
cleanup msg
pcrespov Mar 15, 2023
30383d9
cleanup
pcrespov Mar 15, 2023
6ba27d5
space missing?
pcrespov Mar 15, 2023
1852f5f
constants
pcrespov Mar 10, 2023
423e1de
error handling as error page
pcrespov Mar 10, 2023
c957289
updates tests
pcrespov Mar 10, 2023
339fb39
some cleanup
pcrespov Mar 10, 2023
6417d2d
adds expiration of guest user in settings
pcrespov Mar 14, 2023
9f01266
adds expiration to guest users
pcrespov Mar 14, 2023
51873ab
updates TODOs
pcrespov Mar 14, 2023
9dd0fc5
wip
pcrespov Mar 14, 2023
12f10ab
updates integration catalog_subsystem_mock
pcrespov Mar 14, 2023
80ba615
annotate catalog_subsystem_mock
pcrespov Mar 14, 2023
4e8f66a
test settings
pcrespov Mar 14, 2023
44a749c
fixes error
pcrespov Mar 14, 2023
89a4fba
minor
pcrespov Mar 14, 2023
84c7009
fixes expires_at
pcrespov Mar 14, 2023
e1d614d
fixes in expired_at
pcrespov Mar 14, 2023
118ab7e
fixes in expired_at
pcrespov Mar 14, 2023
8f5e4b4
cleanup fixtures
pcrespov Mar 14, 2023
2a916b0
minor
pcrespov Mar 14, 2023
c8e40c4
cleanup
pcrespov Mar 15, 2023
59d11f8
cleanup validator
pcrespov Mar 15, 2023
317feb9
cleanup msg
pcrespov Mar 15, 2023
df495bd
cleanup
pcrespov Mar 15, 2023
4534e27
space missing?
pcrespov Mar 15, 2023
85cfb80
Merge branch 'is829/study-dispatcher-errors' of github.com:pcrespov/o…
pcrespov Mar 16, 2023
ffec1ce
Remove debugger for GC service
pcrespov Mar 16, 2023
8c2b50c
Improves errors
pcrespov Mar 16, 2023
f3b38d6
improves definition of published
pcrespov Mar 16, 2023
ec456f5
minor
pcrespov Mar 16, 2023
fd87bbc
minor
pcrespov Mar 16, 2023
2398b54
Merge branch 'master' into is829/study-dispatcher-errors
pcrespov Mar 16, 2023
61f91cf
minor
pcrespov Mar 16, 2023
9c113c1
fixes missing await in catalog
pcrespov Mar 16, 2023
01c3688
adds any user permissions
pcrespov Mar 16, 2023
b7dc3f3
fix
pcrespov Mar 16, 2023
df99e81
fixes
pcrespov Mar 16, 2023
3e16bbe
Merge branch 'master' into is829/study-dispatcher-errors
pcrespov Mar 17, 2023
3ebdeb7
undo wrong fix
pcrespov Mar 17, 2023
aece420
cleanup and extra fixes
pcrespov Mar 17, 2023
fcbb642
@GitHK review: renaming
pcrespov Mar 17, 2023
460de69
Merge branch 'master' into is829/study-dispatcher-errors
pcrespov Mar 17, 2023
fea28f1
Update services/web/server/src/simcore_service_webserver/projects/pro…
pcrespov Mar 20, 2023
7482f6e
Update services/web/server/src/simcore_service_webserver/studies_disp…
pcrespov Mar 20, 2023
77210b0
Merge branch 'master' into is829/study-dispatcher-errors
pcrespov Mar 20, 2023
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
13 changes: 13 additions & 0 deletions .vscode/launch.template.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,19 @@
}
]
},
{
"name": "Python: Remote Attach webserver-garbage-collector",
"type": "python",
"request": "attach",
"port": 3011,
"host": "127.0.0.1",
"pathMappings": [
{
"localRoot": "${workspaceFolder}",
"remoteRoot": "/devel"
}
]
},
{
"name": "Python: Remote Attach storage",
"type": "python",
Expand Down
2 changes: 1 addition & 1 deletion scripts/docker/docker-compose-config.bash
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ docker-compose \
--env-file ${env_file}"
for compose_file_path in "$@"
do
docker_command+=" --file=${compose_file_path}"
docker_command+=" --file=${compose_file_path} "
pcrespov marked this conversation as resolved.
Show resolved Hide resolved
done
docker_command+=" \
config \
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Dict, List, Optional, Set
from typing import Optional

import sqlalchemy as sa
from pydantic.networks import EmailStr
Expand All @@ -11,7 +11,7 @@


class GroupsRepository(BaseRepository):
async def list_user_groups(self, user_id: int) -> List[GroupAtDB]:
async def list_user_groups(self, user_id: int) -> list[GroupAtDB]:
groups_in_db = []
async with self.db_engine.connect() as conn:
async for row in await conn.stream(
Expand Down Expand Up @@ -55,8 +55,8 @@ async def get_user_email_from_gid(self, gid: PositiveInt) -> Optional[EmailStr]:
)

async def list_user_emails_from_gids(
self, gids: Set[PositiveInt]
) -> Dict[PositiveInt, Optional[EmailStr]]:
self, gids: set[PositiveInt]
) -> dict[PositiveInt, Optional[EmailStr]]:
service_owners = {}
async with self.db_engine.connect() as conn:
async for row in await conn.stream(
Expand Down
6 changes: 4 additions & 2 deletions services/storage/src/simcore_service_storage/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ class FileMetaDataNotFoundError(DatabaseAccessError):

class FileAccessRightError(DatabaseAccessError):
code = "file.access_right_error"
msg_template: str = "Insufficient access rights to {access_right} {file_id}"
msg_template: str = "Insufficient access rights to {access_right} data {file_id}"


class ProjectAccessRightError(DatabaseAccessError):
code = "file.access_right_error"
msg_template: str = "Insufficient access rights to {access_right} {project_id}"
msg_template: str = (
"Insufficient access rights to {access_right} project {project_id}"
)


class ProjectNotFoundError(DatabaseAccessError):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ async def register(request: web.Request):

await check_other_registrations(email=registration.email, db=db, cfg=cfg)

expires_at = None # = does not expire
expires_at: Optional[datetime] = None # = does not expire
if settings.LOGIN_REGISTRATION_INVITATION_REQUIRED:
# Only requests with INVITATION can register user
# to either a permanent or to a trial account
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@
from ..utils import now_str
from .project_models import ProjectDict
from .projects_db_utils import (
ANY_USER_ID_SENTINEL,
BaseProjectDB,
Permission,
PermissionStr,
ProjectAccessRights,
assemble_array_groups,
check_project_permissions,
Expand All @@ -55,7 +56,7 @@
log = logging.getLogger(__name__)

APP_PROJECT_DBAPI = __name__ + ".ProjectDBAPI"

ANY_USER = ANY_USER_ID_SENTINEL

# pylint: disable=too-many-public-methods
# NOTE: https://github.com/ITISFoundation/osparc-simcore/issues/3516
Expand Down Expand Up @@ -316,7 +317,7 @@ async def get_project(
*,
only_published: bool = False,
only_templates: bool = False,
check_permissions: Permission = "read",
check_permissions: PermissionStr = "read",
) -> tuple[ProjectDict, ProjectType]:
"""Returns all projects *owned* by the user

Expand All @@ -325,6 +326,7 @@ async def get_project(
- Notice that a user can have access to a project where he/she has read access

:raises ProjectNotFoundError: project is not assigned to user
raises ProjectInvalidRightsError: if user has no access rights to do check_permissions
"""
async with self.engine.acquire() as conn:
project = await self._get_project(
Expand Down Expand Up @@ -634,7 +636,7 @@ async def list_node_ids_in_project(self, project_uuid: str) -> set[str]:
#

async def has_permission(
self, user_id: UserID, project_uuid: str, permission: Permission
self, user_id: UserID, project_uuid: str, permission: PermissionStr
) -> bool:
"""
NOTE: this function should never raise
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
""" Database utisl
""" Database utils

"""
import asyncio
Expand Down Expand Up @@ -32,7 +32,9 @@
DB_EXCLUSIVE_COLUMNS = ["type", "id", "published", "hidden"]
SCHEMA_NON_NULL_KEYS = ["thumbnail"]

Permission = Literal["read", "write", "delete"]
PermissionStr = Literal["read", "write", "delete"]

ANY_USER_ID_SENTINEL = -1


class ProjectAccessRights(Enum):
Expand All @@ -46,51 +48,75 @@ def check_project_permissions(
project: Union[ProjectProxy, ProjectDict],
user_id: int,
user_groups: list[RowProxy],
permission: Permission,
permission: str,
) -> None:
"""
:raises ProjectInvalidRightsError if check fails
"""

if not permission:
return

needed_permissions = permission.split("|")
operations_on_project = set(permission.split("|"))
assert set(operations_on_project).issubset(set(PermissionStr.__args__)) # nosec

# compute access rights by order of priority all group > organizations > primary
primary_group = next(
filter(lambda x: x.get("type") == GroupType.PRIMARY, user_groups), None
)
standard_groups = filter(lambda x: x.get("type") == GroupType.STANDARD, user_groups)
#
# Get primary_gid, standard_gids and everyone_gid for user_id
#
all_group = next(
filter(lambda x: x.get("type") == GroupType.EVERYONE, user_groups), None
)
if primary_group is None or all_group is None:
# the user groups is missing entries
if all_group is None:
raise ProjectInvalidRightsError(user_id, project.get("uuid"))

everyone_gid = str(all_group["gid"])

if user_id == ANY_USER_ID_SENTINEL:
primary_gid = None
standard_gids = []

else:
primary_group = next(
filter(lambda x: x.get("type") == GroupType.PRIMARY, user_groups), None
)
if primary_group is None:
# the user groups is missing entries
raise ProjectInvalidRightsError(user_id, project.get("uuid"))

standard_groups = filter(
lambda x: x.get("type") == GroupType.STANDARD, user_groups
)

primary_gid = str(primary_group["gid"])
standard_gids = [str(group["gid"]) for group in standard_groups]

#
# Composes access rights by order of priority all group > organizations > primary
#
project_access_rights = deepcopy(project.get("access_rights", {}))

# compute access rights
no_access_rights = {"read": False, "write": False, "delete": False}
computed_permissions = project_access_rights.get(
str(all_group["gid"]), no_access_rights
# access rights for everyone
user_can = project_access_rights.get(
everyone_gid, {"read": False, "write": False, "delete": False}
)

# get the standard groups
for group in standard_groups:
# access rights for standard groups
for group_id in standard_gids:
standard_project_access = project_access_rights.get(
str(group["gid"]), no_access_rights
group_id, {"read": False, "write": False, "delete": False}
)
for k in computed_permissions.keys():
computed_permissions[k] = (
computed_permissions[k] or standard_project_access[k]
for operation in user_can.keys():
user_can[operation] = (
user_can[operation] or standard_project_access[operation]
)

# get the primary group access
# access rights for primary group
primary_access_right = project_access_rights.get(
str(primary_group["gid"]), no_access_rights
primary_gid, {"read": False, "write": False, "delete": False}
)
for k in computed_permissions.keys():
computed_permissions[k] = computed_permissions[k] or primary_access_right[k]
for operation in user_can.keys():
user_can[operation] = user_can[operation] or primary_access_right[operation]

if any(not computed_permissions[p] for p in needed_permissions):
if any(not user_can[operation] for operation in operations_on_project):
raise ProjectInvalidRightsError(user_id, project.get("uuid"))


Expand Down Expand Up @@ -146,16 +172,31 @@ def assemble_array_groups(user_groups: list[RowProxy]) -> str:


class BaseProjectDB:
@staticmethod
async def _list_user_groups(conn: SAConnection, user_id: int) -> list[RowProxy]:
user_groups: list[RowProxy] = []
query = (
select([groups])
.select_from(groups.join(user_to_groups))
.where(user_to_groups.c.uid == user_id)
@classmethod
async def _get_everyone_group(cls, conn: SAConnection) -> RowProxy:
result = await conn.execute(
sa.select([groups]).where(groups.c.type == GroupType.EVERYONE)
)
async for row in conn.execute(query):
user_groups.append(row)
row = await result.first()
return row

@classmethod
async def _list_user_groups(
cls, conn: SAConnection, user_id: int
) -> list[RowProxy]:
user_groups: list[RowProxy] = []

if user_id == ANY_USER_ID_SENTINEL:
everyone_group = await cls._get_everyone_group(conn)
assert everyone_group # nosec
user_groups.append(everyone_group)
else:
result = await conn.execute(
select([groups])
.select_from(groups.join(user_to_groups))
.where(user_to_groups.c.uid == user_id)
)
user_groups = await result.fetchall()
return user_groups

@staticmethod
Expand Down Expand Up @@ -268,10 +309,11 @@ async def _get_project(
for_update: bool = False,
only_templates: bool = False,
only_published: bool = False,
check_permissions: Permission = "read",
check_permissions: PermissionStr = "read",
) -> dict:
"""
raises: ProjectNotFoundError
raises ProjectNotFoundError if project does not exists
raises ProjectInvalidRightsError if user_id does not have at 'check_permissions' access rights
"""
exclude_foreign = exclude_foreign or []

Expand Down Expand Up @@ -308,14 +350,14 @@ async def _get_project(
project_row = await result.first()

if not project_row:
raise ProjectNotFoundError(project_uuid)

# now carefuly check the access rights
if only_published is False:
check_project_permissions(
project_row, user_id, user_groups, check_permissions
raise ProjectNotFoundError(
project_uuid=project_uuid,
search_context=f"{user_id=}, {only_templates=}, {only_published=}, {check_permissions=}",
pcrespov marked this conversation as resolved.
Show resolved Hide resolved
)

# check the access rights
check_project_permissions(project_row, user_id, user_groups, check_permissions)

project: dict[str, Any] = dict(project_row.items())

if "tags" not in exclude_foreign:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
"""Defines the different exceptions that may arise in the projects subpackage"""
from typing import Any, Optional

import redis.exceptions
from models_library.projects import ProjectID
from models_library.users import UserID
Expand All @@ -10,6 +12,10 @@ class ProjectsException(Exception):
def __init__(self, msg=None):
super().__init__(msg or "Unexpected error occured in projects submodule")

def detailed_message(self):
# Override in subclass
return f"{type(self)}: {self}"


class ProjectInvalidRightsError(ProjectsException):
"""Invalid rights to access project"""
Expand All @@ -33,9 +39,17 @@ def __init__(self, project_uuid):
class ProjectNotFoundError(ProjectsException):
"""Project was not found in DB"""

def __init__(self, project_uuid):
super().__init__(f"Project with uuid {project_uuid} not found")
def __init__(self, project_uuid, *, search_context: Optional[Any] = None):
super().__init__(f"Project with uuid {project_uuid} not found.")
self.project_uuid = project_uuid
self.search_context_msg = f"{search_context}"

def detailed_message(self):
msg = f"Project with uuid {self.project_uuid}"
if self.search_context_msg:
msg += f" and {self.search_context_msg}"
msg += " was not found"
return msg


class ProjectDeleteError(ProjectsException):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from typing import Final

# NOTE: MSG_* strings MUST be human readable messages

MSG_PROJECT_NOT_FOUND: Final[str] = "Cannot find any study with ID '{project_id}'."


MSG_PROJECT_NOT_PUBLISHED: Final[
str
] = "Cannot find any published study with ID '{project_id}'"


MSG_UNEXPECTED_ERROR: Final[
str
] = "Opps this is embarrasing! Something went really wrong {hint}"
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ async def add_new_project(
app: web.Application, project: Project, user: UserInfo, *, product_name: str
):
# TODO: move this to projects_api
# TODO: this piece was taking fromt the end of projects.projects_handlers.create_projects
# TODO: this piece was taken from the end of projects.projects_handlers.create_projects

from ..director_v2_api import create_or_update_pipeline
from ..projects.projects_db import APP_PROJECT_DBAPI
Expand Down
Loading