Merge pull request #1337 from leggedrobotics/staging

Dev Version 0.41.2

Author: wp99cp

backend/package.json

@@ -1,6 +1,6 @@
 {
     "name": "kleinkram-backend",
-    "version": "0.41.1",
+    "version": "0.41.2",
     "description": "",
     "author": "",
     "private": true,

backend/src/app.module.ts

@@ -23,6 +23,7 @@ import { PrometheusModule } from '@willsoto/nestjs-prometheus';
 import { AuditLoggerMiddleware } from './routing/middlewares/audit-logger-middleware.service';
 import { appVersion } from './app-version';
 import { ActionModule } from './endpoints/action/action.module';
+import { VersionCheckerMiddlewareService } from './routing/middlewares/version-checker-middleware.service';
 
 export interface AccessGroupConfig {
     emails: [{ email: string; access_groups: string[] }];
@@ -92,8 +93,13 @@ export class AppModule implements NestModule {
      * @param consumer
      */
     configure(consumer: MiddlewareConsumer): void {
-        consumer // enable default middleware for all routes
-            .apply(APIKeyResolverMiddleware, AuditLoggerMiddleware)
+        // Apply APIKeyResolverMiddleware and AuditLoggerMiddleware to all routes
+        consumer
+            .apply(
+                APIKeyResolverMiddleware,
+                AuditLoggerMiddleware,
+                VersionCheckerMiddlewareService,
+            )
             .forRoutes('*');
     }
 }

backend/src/routing/middlewares/version-checker-middleware.service.ts

@@ -0,0 +1,117 @@
+import { Injectable, NestMiddleware } from '@nestjs/common';
+import { NextFunction, Request, Response } from 'express';
+import logger from '../../logger';
+
+/**
+ *
+ * A nest middleware that resolves the user from the API key in the request.
+ *
+ */
+@Injectable()
+export class VersionCheckerMiddlewareService implements NestMiddleware {
+    async use(
+        request: Request,
+        response: Response,
+        next: NextFunction,
+    ): Promise<void> {
+        let clientVersion = request.headers['kleinkram-client-version'] as
+            | string
+            | undefined;
+
+        const requestPath = request.originalUrl;
+
+        // ignore auth endpoints
+        if (requestPath.startsWith('/auth/')) {
+            next();
+            return;
+        }
+
+        // strip away everything after .dev
+        if (clientVersion !== undefined) {
+            clientVersion = clientVersion.split('.dev')[0];
+        }
+
+        // verify if version is of semver format
+        const validVersion = /^\d+\.\d+\.\d+$/;
+        if (clientVersion !== undefined && !validVersion.test(clientVersion)) {
+            this.rejectRequest(response, clientVersion);
+            return;
+        }
+
+        logger.debug(
+            `Check Client Version for call to endpoint: ${requestPath} is: ${clientVersion}`,
+        );
+
+        if (clientVersion === undefined) {
+            this.rejectRequest(response, 'undefined');
+            return;
+        }
+
+        // forbidden client versions: allows for the following notations
+        // - '<0.40.x': versions below 0.40.x are forbidden
+        // - '0.41.x': version 0.41.x is forbidden
+        const forbiddenClientVersions = ['<0.40.0', '0.41.0', '0.41.1'];
+
+        if (this.isVersionForbidden(clientVersion, forbiddenClientVersions)) {
+            this.rejectRequest(response, clientVersion);
+            return;
+        }
+
+        next();
+    }
+
+    private rejectRequest(response: Response, clientVersion: string): void {
+        // reject request with 426
+        response.status(426).json({
+            statusCode: 426,
+            message: `Client version ${clientVersion} is not a valid version.`,
+        });
+
+        response.send();
+    }
+
+    private isVersionForbidden(
+        clientVersion: string,
+        forbiddenVersions: string[],
+    ): boolean {
+        for (const forbiddenVersion of forbiddenVersions) {
+            if (forbiddenVersion.startsWith('<')) {
+                const versionToCompare = forbiddenVersion.slice(1);
+                if (this.isLessThan(clientVersion, versionToCompare)) {
+                    return true;
+                }
+            } else if (forbiddenVersion.endsWith('.x')) {
+                const baseVersion = forbiddenVersion.slice(
+                    0,
+                    Math.max(0, forbiddenVersion.length - 2),
+                );
+                if (clientVersion.startsWith(baseVersion)) {
+                    return true;
+                }
+            } else if (clientVersion === forbiddenVersion) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private isLessThan(version1: string, version2: string): boolean {
+        const v1Parts = version1.split('.').map(Number);
+        const v2Parts = version2.split('.').map(Number);
+
+        const maxLength = Math.max(v1Parts.length, v2Parts.length);
+
+        for (let i = 0; i < maxLength; i++) {
+            const part1 = v1Parts[i] || 0;
+            const part2 = v2Parts[i] || 0;
+
+            if (part1 < part2) {
+                return true;
+            } else if (part1 > part2) {
+                return false;
+            }
+        }
+
+        return false;
+    }
+}

cli/kleinkram/api/client.py

@@ -2,6 +2,7 @@
 
 import logging
 from collections import abc
+from pathlib import Path
 from threading import Lock
 from typing import Any
 from typing import List
@@ -13,6 +14,9 @@
 import httpx
 from httpx._types import PrimitiveData
 
+import kleinkram.errors
+from kleinkram._version import __version__
+from kleinkram.config import CONFIG_PATH
 from kleinkram.config import Config
 from kleinkram.config import Credentials
 from kleinkram.config import get_config
@@ -26,6 +30,8 @@
 COOKIE_REFRESH_TOKEN = "refreshtoken"
 COOKIE_API_KEY = "clikey"
 
+CLI_VERSION_HEADER = "Kleinkram-Client-Version"
+
 
 Data = Union[PrimitiveData, Any]
 NestedData = Mapping[str, Data]
@@ -65,10 +71,12 @@ class AuthenticatedClient(httpx.Client):
     _config: Config
     _config_lock: Lock
 
-    def __init__(self, *args: Any, **kwargs: Any) -> None:
+    def __init__(
+        self, config_path: Path = CONFIG_PATH, *args: Any, **kwargs: Any
+    ) -> None:
         super().__init__(*args, **kwargs)
 
-        self._config = get_config()
+        self._config = get_config(path=config_path)
         self._config_lock = Lock()
 
         if self._config.credentials is None:
@@ -95,9 +103,7 @@ def _refresh_token(self) -> None:
         self.cookies.set(COOKIE_REFRESH_TOKEN, refresh_token)
 
         logger.info("refreshing token...")
-        response = self.post(
-            "/auth/refresh-token",
-        )
+        response = self.post("/auth/refresh-token")
         response.raise_for_status()
         new_access_token = response.cookies[COOKIE_AUTH_TOKEN]
         creds = Credentials(auth_token=new_access_token, refresh_token=refresh_token)
@@ -110,6 +116,22 @@ def _refresh_token(self) -> None:
 
         self.cookies.set(COOKIE_AUTH_TOKEN, new_access_token)
 
+    def _send_request_with_kleinkram_headers(
+        self, *args: Any, **kwargs: Any
+    ) -> httpx.Response:
+        # add the cli version to the headers
+        headers = kwargs.get("headers") or {}
+        headers.setdefault(CLI_VERSION_HEADER, __version__)
+        kwargs["headers"] = headers
+
+        # send the request
+        response = super().request(*args, **kwargs)
+
+        # check version compatibility
+        if response.status_code == 426:
+            raise kleinkram.errors.UpdateCLIVersion
+        return response
+
     def request(
         self,
         method: str,
@@ -128,7 +150,7 @@ def request(
         logger.info(f"requesting {method} {full_url}")
 
         httpx_params = _convert_query_params_to_httpx_format(params or {})
-        response = super().request(
+        response = self._send_request_with_kleinkram_headers(
             method, full_url, params=httpx_params, *args, **kwargs
         )
 
@@ -148,10 +170,10 @@ def request(
                 raise NotAuthenticated
 
             logger.info(f"retrying request {method} {full_url}")
-            resp = super().request(
+            response = self._send_request_with_kleinkram_headers(
                 method, full_url, params=httpx_params, *args, **kwargs
             )
-            logger.info(f"got response {resp}")
-            return resp
+            logger.info(f"got response {response}")
+            return response
         else:
             return response

cli/kleinkram/core.py

@@ -210,6 +210,10 @@ def delete_files(*, client: AuthenticatedClient, file_ids: Collection[UUID]) ->
     """\
     deletes multiple files accross multiple missions
     """
+    if not file_ids:
+        return
+
+    # we need to check that file_ids is not empty, otherwise this is bad
     files = list(kleinkram.api.routes.get_files(client, FileQuery(ids=list(file_ids))))
 
     # check if all file_ids were actually found
@@ -220,6 +224,9 @@ def delete_files(*, client: AuthenticatedClient, file_ids: Collection[UUID]) ->
                 f"file {file_id} not found, did not delete any files"
             )
 
+    # to prevent catastrophic mistakes from happening *again*
+    assert set(file_ids) == set([file.id for file in files]), "unreachable"
+
     # we can only batch delete files within the same mission
     missions_to_files: Dict[UUID, List[UUID]] = {}
     for file in files:

cli/kleinkram/errors.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 LOGIN_MESSAGE = "Please login using `klein login`."
+UPDATE_MESSAGE = "Please update your CLI using `pip install --upgrade kleinkram`."
 
 
 class ParsingError(Exception): ...
@@ -33,11 +34,6 @@ class FileNotFound(Exception): ...
 class AccessDenied(Exception): ...
 
 
-class NotAuthenticated(Exception):
-    def __init__(self) -> None:
-        super().__init__(LOGIN_MESSAGE)
-
-
 class InvalidCLIVersion(Exception): ...
 
 
@@ -48,3 +44,13 @@ class FileNameNotSupported(Exception): ...
 
 
 class InvalidMissionMetadata(Exception): ...
+
+
+class NotAuthenticated(Exception):
+    def __init__(self) -> None:
+        super().__init__(LOGIN_MESSAGE)
+
+
+class UpdateCLIVersion(Exception):
+    def __init__(self) -> None:
+        super().__init__(UPDATE_MESSAGE)

cli/setup.cfg

@@ -1,6 +1,6 @@
 [metadata]
 name = kleinkram
-version = 0.41.1
+version = 0.41.2
 description = give me your bags
 long_description = file: README.md
 long_description_content_type = text/markdown

cli/tests/api/test_client.py

@@ -1,10 +1,73 @@
 from __future__ import annotations
 
+from pathlib import Path
+from tempfile import TemporaryDirectory
+
+import httpx
 import pytest
 
+import kleinkram.errors
+from kleinkram._version import __version__
+from kleinkram.api.client import CLI_VERSION_HEADER
+from kleinkram.api.client import AuthenticatedClient
 from kleinkram.api.client import _convert_list_data_query_params_values
 from kleinkram.api.client import _convert_nested_data_query_params_values
 from kleinkram.api.client import _convert_query_params_to_httpx_format
+from kleinkram.config import Config
+from kleinkram.config import Credentials
+from kleinkram.config import save_config
+
+CONFIG_FILENAME = ".kleinkram.json"
+
+
+@pytest.fixture
+def config_path():
+    with TemporaryDirectory() as tmpdir:
+        yield Path(tmpdir) / CONFIG_FILENAME
+
+
+@pytest.fixture
+def empty_config(config_path):
+    test_creds = Credentials(api_key="test")
+    config = Config(
+        endpoint_credentials={"local": test_creds}, selected_endpoint="local"
+    )
+    save_config(config, config_path)
+    return config_path
+
+
+def mock_transport(request: httpx.Request) -> httpx.Response:
+    assert CLI_VERSION_HEADER in request.headers
+    assert request.headers[CLI_VERSION_HEADER] == __version__
+    return httpx.Response(200)
+
+
+def test_client_sending_kleinkram_version_header(empty_config):
+    with AuthenticatedClient(
+        config_path=empty_config, transport=httpx.MockTransport(mock_transport)
+    ) as client:
+        resp = client.get("/example")
+        assert resp.status_code == 200
+
+
+def test_client_sending_kleinkram_version_header_with_custom_headers(empty_config):
+    with AuthenticatedClient(
+        config_path=empty_config, transport=httpx.MockTransport(mock_transport)
+    ) as client:
+        resp = client.get("/example", headers={"foo": "bar"})
+        assert resp.status_code == 200
+
+
+def test_client_response_on_426_status_code(empty_config):
+    def return_426_response(request: httpx.Request) -> httpx.Response:
+        _ = request
+        return httpx.Response(426)
+
+    with AuthenticatedClient(
+        config_path=empty_config, transport=httpx.MockTransport(return_426_response)
+    ) as client:
+        with pytest.raises(kleinkram.errors.UpdateCLIVersion):
+            client.get("/example")
 
 
 def test_convert_query_params_httpx_format():