From 9db36c2d5be97f68ab29d9dfc93571fe162a8e36 Mon Sep 17 00:00:00 2001 From: Daniel Dolezal Date: Mon, 27 Jul 2026 17:08:23 +0200 Subject: [PATCH] fix: allow CLI file tokens for RPC access - Accept files-scoped bearer tokens on servicelink RPC calls. - Keep mesh shared-secret auth for trusted internal callers. - Validate CLI auth scopes and reject unsupported values early. - Stop CLI browser login waiting for the full timeout after callback. - Add tests for scope normalization, RPC access, and login callback timing. --- cli/nanoshare_client/auth.py | 21 +++++++++++++---- routes/api/cli_auth.py | 20 +++++++++++++--- routes/api/link.py | 14 ++++++----- tests/test_api_cli_auth.py | 43 ++++++++++++++++++++++++++++++++++ tests/test_nanoshare_cli.py | 20 ++++++++++++++++ tests/test_servicelink_link.py | 9 ++++--- 6 files changed, 110 insertions(+), 17 deletions(-) create mode 100644 tests/test_api_cli_auth.py diff --git a/cli/nanoshare_client/auth.py b/cli/nanoshare_client/auth.py index df74af4..b91abe8 100644 --- a/cli/nanoshare_client/auth.py +++ b/cli/nanoshare_client/auth.py @@ -22,6 +22,7 @@ class _CallbackServer: self.code: str | None = None self.error: str | None = None self.expected_state = expected_state + self.done = threading.Event() outer = self class Handler(BaseHTTPRequestHandler): @@ -36,6 +37,7 @@ class _CallbackServer: self.send_response(400) self.end_headers() self.wfile.write(b'NanoShare CLI login failed: state mismatch') + outer.done.set() return outer.code = params.get('code', [''])[0] outer.error = params.get('error', [''])[0] or None @@ -45,31 +47,40 @@ class _CallbackServer: self.wfile.write(b'NanoShare CLI login complete. You can close this browser tab.') else: self.wfile.write(b'NanoShare CLI login failed.') + outer.done.set() self.httpd = ThreadingHTTPServer((host, port), Handler) self.url = f'http://{host}:{self.httpd.server_port}/callback' self.thread = threading.Thread(target=self.httpd.serve_forever, daemon=True) + self.started = False + + def start(self) -> None: + if not self.started: + self.thread.start() + self.started = True def wait_for_code(self, timeout: float) -> str: - self.thread.start() - self.thread.join(timeout) + self.start() + received_callback = self.done.wait(timeout) self.httpd.shutdown() + self.thread.join(5) if self.error: raise RuntimeError(self.error) - if not self.code: + if not received_callback or not self.code: raise TimeoutError('login timed out') return self.code def login(args) -> int: state = secrets.token_urlsafe(24) server = _CallbackServer(args.callback_host, args.callback_port, state) + server.start() authorize_url = _make_authorize_url(args.base_url, server.url, state, args.scope) - print(f'Opening browser for NanoShare login: {authorize_url}') + print(f'Opening browser for NanoShare login: {authorize_url}', flush=True) if not args.no_browser: webbrowser.open(authorize_url) else: - print(authorize_url) + print(authorize_url, flush=True) try: code = server.wait_for_code(args.login_timeout) diff --git a/routes/api/cli_auth.py b/routes/api/cli_auth.py index f7b629d..f64544b 100644 --- a/routes/api/cli_auth.py +++ b/routes/api/cli_auth.py @@ -12,13 +12,24 @@ from quart import current_app cli_auth_bp = Blueprint('cli_auth', __name__) CLI_CODE_TTL_SECONDS = 300 +CLI_AUTH_DEFAULT_SCOPE = 'files' +CLI_AUTH_ALLOWED_SCOPES = {'all', 'files'} + +def _normalize_cli_scope(scope: str | None) -> str: + requested_scope = (scope or CLI_AUTH_DEFAULT_SCOPE).strip() or CLI_AUTH_DEFAULT_SCOPE + if requested_scope not in CLI_AUTH_ALLOWED_SCOPES: + raise ValueError(f'unsupported CLI scope: {requested_scope}') + return requested_scope @cli_auth_bp.get('/auth/cli/login') @LIMITER.limit('10 per minute;60 per hour;') async def cli_login(): redirect_uri = request.args.get('redirect_uri', '').strip() state = request.args.get('state', '').strip() - scope = request.args.get('scope', 'files').strip() or 'files' + try: + scope = _normalize_cli_scope(request.args.get('scope')) + except ValueError as exc: + return jsonify({'ok': False, 'error': str(exc)}), 400 add_wide_event_context(auth={'method': 'cli_browser', 'operation': 'cli_login_start'}) if not redirect_uri or not state: @@ -44,7 +55,7 @@ async def cli_login_complete(): user['sub'], login_request['redirect_uri'], login_request['state'], - login_request.get('scope') or 'files', + _normalize_cli_scope(login_request.get('scope')), ) async def _redirect_with_cli_code(user_id: str, redirect_uri: str, state: str, scope: str): @@ -72,7 +83,10 @@ async def cli_token_exchange(): await cache.delete(f'cli:login:{code}') user_id = login_data['user_id'] - scope = login_data.get('scope') or 'files' + try: + scope = _normalize_cli_scope(login_data.get('scope')) + except ValueError as exc: + return jsonify({'ok': False, 'error': str(exc)}), 400 token_data = await current_app.convex.add_refresh_token( token_name='nanoshare-cli', user_id=user_id, diff --git a/routes/api/link.py b/routes/api/link.py index e24f81b..105b644 100644 --- a/routes/api/link.py +++ b/routes/api/link.py @@ -4,7 +4,7 @@ Lets other nodes (browser-cli, website) push files in and read file metadata over the shared servicelink envelope at POST /rpc, alongside the existing web UI and /api routes. -Every call needs a bearer token carrying the `mesh` scope; the endpoint is rate +Every call needs a bearer token carrying the `files` or `mesh` scope; the endpoint is rate limited and body-size capped. Keep /rpc on the internal node network. ''' from __future__ import annotations @@ -17,16 +17,18 @@ from quart import current_app from my_modules.app.setup import LIMITER from my_modules.expiry import ensure_utc, parse_expires from my_modules.file_meta import format_size, iso_stamp_filename -from servicelink import InvalidParams, NotFound, Router, Unauthorized, any_verifier, bearer_verifier, create_link_blueprint, shared_secret_verifier +from servicelink import Forbidden, InvalidParams, NotFound, Router, Unauthorized, any_verifier, bearer_verifier, create_link_blueprint, shared_secret_verifier MAX_RPC_BODY = 16 * 1024 * 1024 -MESH_SCOPE = 'mesh' +RPC_SCOPES = ('files', 'mesh') router = Router('picoshare') def _user_id(ctx): if ctx.principal is None: raise Unauthorized('authentication required') + if not any(ctx.principal.has_scope(scope) for scope in RPC_SCOPES): + raise Forbidden('missing required scope: files') return ctx.principal.subject @router.method('files.upload') @@ -110,11 +112,11 @@ async def _decode_access_token(token): return payload def _build_verify(): - # Accept a JWT access token with the mesh scope (public path) OR, on the + # Accept a JWT access token with a file/RPC scope (public path) OR, on the # trusted Docker network, a static shared secret from SERVICELINK_MESH_SECRET. - jwt = bearer_verifier(_decode_access_token, require_scope=MESH_SCOPE) + jwt = bearer_verifier(_decode_access_token) secret = os.getenv('SERVICELINK_MESH_SECRET') - return any_verifier(shared_secret_verifier(secret, scopes=(MESH_SCOPE,)), jwt) if secret else jwt + return any_verifier(shared_secret_verifier(secret, scopes=('mesh',)), jwt) if secret else jwt verify = _build_verify() link_bp = create_link_blueprint(router, verify=verify, limiter=LIMITER.limit('30 per minute'), max_body=MAX_RPC_BODY) diff --git a/tests/test_api_cli_auth.py b/tests/test_api_cli_auth.py new file mode 100644 index 0000000..30cc5f0 --- /dev/null +++ b/tests/test_api_cli_auth.py @@ -0,0 +1,43 @@ +import importlib.util +import sys +import types +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +class _NoOpLimiter: + def limit(self, *args, **kwargs): + return lambda fn: fn + +def _load_cli_auth_module(): + setup_stub = types.ModuleType('my_modules.app.setup') + setup_stub.LIMITER = _NoOpLimiter() + setup_stub.cache = object() + sys.modules['my_modules.app.setup'] = setup_stub + + module_path = Path(__file__).resolve().parents[1] / 'routes' / 'api' / 'cli_auth.py' + spec = importlib.util.spec_from_file_location('api_cli_auth_under_test', module_path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + +def test_normalize_cli_scope_keeps_files_scope(): + module = _load_cli_auth_module() + + assert module._normalize_cli_scope('files') == 'files' + +def test_normalize_cli_scope_defaults_to_files(): + module = _load_cli_auth_module() + + assert module._normalize_cli_scope(None) == 'files' + assert module._normalize_cli_scope('') == 'files' + +def test_normalize_cli_scope_rejects_unsupported_scope(): + module = _load_cli_auth_module() + + try: + module._normalize_cli_scope('printer') + except ValueError as exc: + assert 'unsupported CLI scope' in str(exc) + else: + raise AssertionError('expected unsupported scope to raise') diff --git a/tests/test_nanoshare_cli.py b/tests/test_nanoshare_cli.py index 9ab2ba6..fa5f542 100644 --- a/tests/test_nanoshare_cli.py +++ b/tests/test_nanoshare_cli.py @@ -1,10 +1,13 @@ import json import sys +import threading +import urllib.request from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'cli')) from nanoshare_client import cli +from nanoshare_client.auth import _CallbackServer class FakeClient: def __init__(self): @@ -52,6 +55,23 @@ def test_sync_uploads_new_local_file_and_writes_state(tmp_path, monkeypatch): state = json.loads((tmp_path / '.nanoshare-sync' / 'state.json').read_text()) assert state['files']['hello.txt']['file_id'] == 'file_1' +def test_login_callback_returns_without_waiting_for_timeout(): + server = _CallbackServer('127.0.0.1', 0, 'expected-state') + result = {} + + def wait_for_code(): + result['code'] = server.wait_for_code(30) + + thread = threading.Thread(target=wait_for_code) + thread.start() + with urllib.request.urlopen(f'{server.url}?code=login-code&state=expected-state', timeout=5) as response: + assert response.status == 200 + assert b'login complete' in response.read() + thread.join(5) + + assert not thread.is_alive() + assert result['code'] == 'login-code' + def test_sync_deletes_old_remote_when_local_file_changes(tmp_path, monkeypatch): fake = FakeClient() fake.remote_files = [{'file_id': 'old_file', 'file_name': 'hello.txt', 'file_size': '5 B'}] diff --git a/tests/test_servicelink_link.py b/tests/test_servicelink_link.py index 9923adf..3b62f09 100644 --- a/tests/test_servicelink_link.py +++ b/tests/test_servicelink_link.py @@ -56,6 +56,9 @@ class FakeConvex: async def get_file(self, file_id): return self.files.get(file_id) + async def get_files(self, user_id): + return list(self.files.values()) + def _call(envelope, token=None): # Call the handler the /rpc route delegates to, bypassing the LIMITER/size-cap # wrapper; exercises the real verify (mesh scope) + dispatch + handlers. @@ -100,7 +103,7 @@ def test_missing_token_is_unauthorized(): assert status == 401 assert body['error']['code'] == 'unauthorized' -def test_token_without_mesh_scope_is_forbidden(): +def test_token_with_files_scope_is_allowed(): status, body = _call(_request('files.list'), token='nomesh') - assert status == 403 - assert body['error']['code'] == 'forbidden' + assert status == 200 + assert body['ok'] is True