From 94a5be8468d231458fd0d7016145d5f8b0efe809 Mon Sep 17 00:00:00 2001 From: MoritzWeber Date: Wed, 14 Dec 2022 11:22:30 +0100 Subject: [PATCH 1/6] fix: Use routes for session termination TeamForCapella users don't get deleted otherwise --- backend/capellacollab/sessions/idletimeout.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/capellacollab/sessions/idletimeout.py b/backend/capellacollab/sessions/idletimeout.py index b76b5148a..ade6bb7af 100644 --- a/backend/capellacollab/sessions/idletimeout.py +++ b/backend/capellacollab/sessions/idletimeout.py @@ -11,7 +11,8 @@ from capellacollab.config import config from capellacollab.core import database -from capellacollab.sessions.database import delete_session, get_session_by_id +from capellacollab.sessions import routes +from capellacollab.sessions.database import get_session_by_id from capellacollab.sessions.operators import OPERATOR log = logging.getLogger(__name__) @@ -31,10 +32,9 @@ def terminate_idle_session(): for metric in response.json()["data"]["result"]: if session_id := metric.get("metric", {}).get("app"): log.info("Terminating idle session %s", session_id) - OPERATOR.kill_session(session_id) with database.SessionLocal() as db: if session := get_session_by_id(db, session_id): - delete_session(db, session) + routes.end_session(session, db, OPERATOR) async def terminate_idle_sessions_in_background(interval=60): From d9d14dc96174c29fe7ccc9bc8010565c1e8c14ec Mon Sep 17 00:00:00 2001 From: MoritzWeber Date: Wed, 14 Dec 2022 11:25:06 +0100 Subject: [PATCH 2/6] fix: Kill sessions without database reference --- backend/capellacollab/sessions/idletimeout.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/backend/capellacollab/sessions/idletimeout.py b/backend/capellacollab/sessions/idletimeout.py index ade6bb7af..95a714aa7 100644 --- a/backend/capellacollab/sessions/idletimeout.py +++ b/backend/capellacollab/sessions/idletimeout.py @@ -35,6 +35,12 @@ def terminate_idle_session(): with database.SessionLocal() as db: if session := get_session_by_id(db, session_id): routes.end_session(session, db, OPERATOR) + else: + log.error( + "Session was not found in our database. Terminating idle session %s", + session_id, + ) + OPERATOR.kill_session(session_id) async def terminate_idle_sessions_in_background(interval=60): From 8b45d3b814bb2618ec6b22746737f14fd2ef2331 Mon Sep 17 00:00:00 2001 From: MoritzWeber Date: Wed, 14 Dec 2022 11:30:27 +0100 Subject: [PATCH 3/6] fix: Rename duplicate test --- backend/tests/test_sessions_idletimeout.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/tests/test_sessions_idletimeout.py b/backend/tests/test_sessions_idletimeout.py index a5579eda8..714f69bde 100644 --- a/backend/tests/test_sessions_idletimeout.py +++ b/backend/tests/test_sessions_idletimeout.py @@ -43,7 +43,9 @@ def test_no_idle_sessions(monkeypatch): terminate_idle_session() -def test_no_idle_sessions(monkeypatch, db): +def test_idle_sessions(monkeypatch, db): + # db fixture is needed to start the database + operator = MockOperator() monkeypatch.setattr( requests, From a48f5c9effed8840129accce80c2d554219fb535 Mon Sep 17 00:00:00 2001 From: MoritzWeber Date: Wed, 14 Dec 2022 17:30:42 +0100 Subject: [PATCH 4/6] refactor: Rename sessions.database to sessions.crud --- .../core/authentication/database/__init__.py | 2 +- .../capellacollab/sessions/{database.py => crud.py} | 0 backend/capellacollab/sessions/files/routes.py | 2 +- backend/capellacollab/sessions/idletimeout.py | 2 +- backend/capellacollab/sessions/injectables.py | 4 ++-- backend/capellacollab/sessions/routes.py | 12 ++++++------ 6 files changed, 11 insertions(+), 11 deletions(-) rename backend/capellacollab/sessions/{database.py => crud.py} (100%) diff --git a/backend/capellacollab/core/authentication/database/__init__.py b/backend/capellacollab/core/authentication/database/__init__.py index 532df6242..da2a0b990 100644 --- a/backend/capellacollab/core/authentication/database/__init__.py +++ b/backend/capellacollab/core/authentication/database/__init__.py @@ -19,7 +19,7 @@ ProjectUserPermission, ProjectUserRole, ) -from capellacollab.sessions.database import get_session_by_id +from capellacollab.sessions.crud import get_session_by_id from capellacollab.settings.modelsources.git import crud from capellacollab.users.crud import get_user_by_name from capellacollab.users.models import Role diff --git a/backend/capellacollab/sessions/database.py b/backend/capellacollab/sessions/crud.py similarity index 100% rename from backend/capellacollab/sessions/database.py rename to backend/capellacollab/sessions/crud.py diff --git a/backend/capellacollab/sessions/files/routes.py b/backend/capellacollab/sessions/files/routes.py index ce3223756..5090e9302 100644 --- a/backend/capellacollab/sessions/files/routes.py +++ b/backend/capellacollab/sessions/files/routes.py @@ -16,7 +16,7 @@ from capellacollab.core.authentication.helper import get_username from capellacollab.core.authentication.jwt_bearer import JWTBearer from capellacollab.core.database import get_db -from capellacollab.sessions.database import get_session_by_id +from capellacollab.sessions.crud import get_session_by_id from capellacollab.sessions.operators import OPERATOR from capellacollab.sessions.schema import FileTree diff --git a/backend/capellacollab/sessions/idletimeout.py b/backend/capellacollab/sessions/idletimeout.py index 95a714aa7..8c9fabf69 100644 --- a/backend/capellacollab/sessions/idletimeout.py +++ b/backend/capellacollab/sessions/idletimeout.py @@ -12,7 +12,7 @@ from capellacollab.config import config from capellacollab.core import database from capellacollab.sessions import routes -from capellacollab.sessions.database import get_session_by_id +from capellacollab.sessions.crud import get_session_by_id from capellacollab.sessions.operators import OPERATOR log = logging.getLogger(__name__) diff --git a/backend/capellacollab/sessions/injectables.py b/backend/capellacollab/sessions/injectables.py index fe4b7c25c..670ae8f74 100644 --- a/backend/capellacollab/sessions/injectables.py +++ b/backend/capellacollab/sessions/injectables.py @@ -11,7 +11,7 @@ from capellacollab.core.database import get_db from capellacollab.users.models import Role -from . import database +from . import crud from .models import DatabaseSession @@ -21,7 +21,7 @@ def get_existing_session( token: JWTBearer = Depends(JWTBearer()), ) -> DatabaseSession: try: - session: DatabaseSession = database.get_session_by_id(db, session_id) + session: DatabaseSession = crud.get_session_by_id(db, session_id) except NoResultFound as e: raise HTTPException( 404, diff --git a/backend/capellacollab/sessions/routes.py b/backend/capellacollab/sessions/routes.py index 2e3a36c3a..309e25a44 100644 --- a/backend/capellacollab/sessions/routes.py +++ b/backend/capellacollab/sessions/routes.py @@ -34,7 +34,7 @@ DatabaseGitModel, ) from capellacollab.projects.users.crud import ProjectUserRole -from capellacollab.sessions import database, guacamole +from capellacollab.sessions import crud, guacamole from capellacollab.sessions.files import routes as files from capellacollab.sessions.models import DatabaseSession from capellacollab.sessions.operators import get_operator @@ -87,7 +87,7 @@ def get_current_sessions( token=Depends(JWTBearer()), ): if RoleVerification(required_role=Role.ADMIN, verify=False)(token, db): - return inject_attrs_in_sessions(database.get_all_sessions(db)) + return inject_attrs_in_sessions(crud.get_all_sessions(db)) if not any( project_user.role == ProjectUserRole.MANAGER @@ -103,7 +103,7 @@ def get_current_sessions( list( itertools.chain.from_iterable( [ - database.get_sessions_for_repository(db, project) + crud.get_sessions_for_repository(db, project) for project in [ p.name for p in db_user.projects @@ -212,7 +212,7 @@ def request_persistent_session( log.info("Starting persistent session for user %s", owner) - existing_user_sessions = database.get_sessions_for_user(db, owner) + existing_user_sessions = crud.get_sessions_for_user(db, owner) if WorkspaceType.PERSISTENT in [ session.type for session in existing_user_sessions @@ -350,7 +350,7 @@ def create_database_and_guacamole_session( version=version, **session, ) - response = database.create_session(db=db, session=database_model) + response = crud.create_session(db=db, session=database_model) response.state = "New" response.last_seen = "UNKNOWN" return response @@ -394,7 +394,7 @@ def create_guacamole_token( db: Session = Depends(get_db), token=Depends(JWTBearer()), ): - session = database.get_session_by_id(db, id) + session = crud.get_session_by_id(db, id) if session.owner_name != get_username(token): raise HTTPException( status_code=403, From e32464b65c8aa2796de1d49ca9b64622a6c84329 Mon Sep 17 00:00:00 2001 From: MoritzWeber Date: Wed, 14 Dec 2022 17:31:10 +0100 Subject: [PATCH 5/6] refactor: Move session termination into core module --- backend/capellacollab/sessions/core.py | 51 +++++++++++++++++++ backend/capellacollab/sessions/idletimeout.py | 4 +- backend/capellacollab/sessions/routes.py | 27 +--------- backend/tests/test_sessions_routes.py | 2 +- 4 files changed, 56 insertions(+), 28 deletions(-) create mode 100644 backend/capellacollab/sessions/core.py diff --git a/backend/capellacollab/sessions/core.py b/backend/capellacollab/sessions/core.py new file mode 100644 index 000000000..6e76e52bb --- /dev/null +++ b/backend/capellacollab/sessions/core.py @@ -0,0 +1,51 @@ +# SPDX-FileCopyrightText: Copyright DB Netz AG and the capella-collab-manager contributors +# SPDX-License-Identifier: Apache-2.0 + +import logging + +from requests.exceptions import RequestException +from sqlalchemy.orm import Session + +from capellacollab.sessions.models import DatabaseSession +from capellacollab.sessions.operators.k8s import KubernetesOperator +from capellacollab.sessions.schema import WorkspaceType +from capellacollab.settings.modelsources.t4c.repositories.crud import ( + get_user_t4c_repositories, +) +from capellacollab.settings.modelsources.t4c.repositories.interface import ( + remove_user_from_repository, +) + +from . import crud + +log = logging.getLogger(__name__) + + +def terminate_session( + db_session: Session, session: DatabaseSession, operator: KubernetesOperator +): + if ( + session.tool.name == "Capella" + and session.type == WorkspaceType.PERSISTENT + ): + revoke_session_tokens(db_session, session) + + crud.delete_session(db_session, session) + operator.kill_session(session.id) + + +def revoke_session_tokens(db_session: Session, session: DatabaseSession): + for repository in get_user_t4c_repositories( + db_session, session.tool, session.version, session.owner + ): + try: + remove_user_from_repository( + repository.instance, + repository.name, + username=session.owner.name, + ) + except RequestException: + log.exception( + "Could not delete user from repository '%s' of instance '%s'. Please delete the user manually.", + exc_info=True, + ) diff --git a/backend/capellacollab/sessions/idletimeout.py b/backend/capellacollab/sessions/idletimeout.py index 8c9fabf69..7dde35b2f 100644 --- a/backend/capellacollab/sessions/idletimeout.py +++ b/backend/capellacollab/sessions/idletimeout.py @@ -11,7 +11,7 @@ from capellacollab.config import config from capellacollab.core import database -from capellacollab.sessions import routes +from capellacollab.sessions import core from capellacollab.sessions.crud import get_session_by_id from capellacollab.sessions.operators import OPERATOR @@ -34,7 +34,7 @@ def terminate_idle_session(): log.info("Terminating idle session %s", session_id) with database.SessionLocal() as db: if session := get_session_by_id(db, session_id): - routes.end_session(session, db, OPERATOR) + core.terminate_session(session, db, OPERATOR) else: log.error( "Session was not found in our database. Terminating idle session %s", diff --git a/backend/capellacollab/sessions/routes.py b/backend/capellacollab/sessions/routes.py index 309e25a44..c1da80818 100644 --- a/backend/capellacollab/sessions/routes.py +++ b/backend/capellacollab/sessions/routes.py @@ -12,12 +12,10 @@ from requests.exceptions import RequestException from sqlalchemy.orm import Session -import capellacollab.projects.toolmodels.modelsources.git.crud as git_models_crud from capellacollab.config import config from capellacollab.core.authentication.database import ( ProjectRoleVerification, RoleVerification, - verify_project_role, ) from capellacollab.core.authentication.helper import get_username from capellacollab.core.authentication.jwt_bearer import JWTBearer @@ -40,7 +38,6 @@ from capellacollab.sessions.operators import get_operator from capellacollab.sessions.operators.k8s import KubernetesOperator from capellacollab.sessions.schema import ( - DepthType, GetSessionsResponse, GuacamoleAuthentication, PostPersistentSessionRequest, @@ -53,7 +50,6 @@ ) from capellacollab.settings.modelsources.t4c.repositories.interface import ( add_user_to_repository, - remove_user_from_repository, ) from capellacollab.tools.crud import ( get_image_for_tool_version, @@ -67,6 +63,7 @@ from capellacollab.users.injectables import get_own_user from capellacollab.users.models import DatabaseUser, Role +from . import core from .injectables import get_existing_session router = APIRouter( @@ -362,27 +359,7 @@ def end_session( db: Session = Depends(get_db), operator: KubernetesOperator = Depends(get_operator), ): - if ( - session.tool.name == "Capella" - and session.type == WorkspaceType.PERSISTENT - ): - for repository in get_user_t4c_repositories( - db, session.tool, session.version, session.owner - ): - try: - remove_user_from_repository( - repository.instance, - repository.name, - username=session.owner.name, - ) - except RequestException: - log.exception( - "Could not delete user from repository '%s' of instance '%s'. Please delete the user manually.", - exc_info=True, - ) - - database.delete_session(db, session) - operator.kill_session(session.id) + core.terminate_session(db, session, operator) @router.post( diff --git a/backend/tests/test_sessions_routes.py b/backend/tests/test_sessions_routes.py index 46ea45f68..6dc0e00e7 100644 --- a/backend/tests/test_sessions_routes.py +++ b/backend/tests/test_sessions_routes.py @@ -25,7 +25,7 @@ ProjectUserPermission, ProjectUserRole, ) -from capellacollab.sessions.database import ( +from capellacollab.sessions.crud import ( get_session_by_id, get_sessions_for_user, ) From e093887b747acadf967bb9aa8750d2b781da2c8c Mon Sep 17 00:00:00 2001 From: MoritzWeber Date: Thu, 15 Dec 2022 19:02:58 +0100 Subject: [PATCH 6/6] refactor(sessions): Rename code to util --- backend/capellacollab/sessions/idletimeout.py | 4 ++-- backend/capellacollab/sessions/routes.py | 4 ++-- backend/capellacollab/sessions/{core.py => util.py} | 0 3 files changed, 4 insertions(+), 4 deletions(-) rename backend/capellacollab/sessions/{core.py => util.py} (100%) diff --git a/backend/capellacollab/sessions/idletimeout.py b/backend/capellacollab/sessions/idletimeout.py index 7dde35b2f..35ed51880 100644 --- a/backend/capellacollab/sessions/idletimeout.py +++ b/backend/capellacollab/sessions/idletimeout.py @@ -11,7 +11,7 @@ from capellacollab.config import config from capellacollab.core import database -from capellacollab.sessions import core +from capellacollab.sessions import util from capellacollab.sessions.crud import get_session_by_id from capellacollab.sessions.operators import OPERATOR @@ -34,7 +34,7 @@ def terminate_idle_session(): log.info("Terminating idle session %s", session_id) with database.SessionLocal() as db: if session := get_session_by_id(db, session_id): - core.terminate_session(session, db, OPERATOR) + util.terminate_session(session, db, OPERATOR) else: log.error( "Session was not found in our database. Terminating idle session %s", diff --git a/backend/capellacollab/sessions/routes.py b/backend/capellacollab/sessions/routes.py index c1da80818..c5c636ae4 100644 --- a/backend/capellacollab/sessions/routes.py +++ b/backend/capellacollab/sessions/routes.py @@ -63,7 +63,7 @@ from capellacollab.users.injectables import get_own_user from capellacollab.users.models import DatabaseUser, Role -from . import core +from . import util from .injectables import get_existing_session router = APIRouter( @@ -359,7 +359,7 @@ def end_session( db: Session = Depends(get_db), operator: KubernetesOperator = Depends(get_operator), ): - core.terminate_session(db, session, operator) + util.terminate_session(db, session, operator) @router.post( diff --git a/backend/capellacollab/sessions/core.py b/backend/capellacollab/sessions/util.py similarity index 100% rename from backend/capellacollab/sessions/core.py rename to backend/capellacollab/sessions/util.py