fix: allow CLI file tokens for RPC access
Build and Push Docker Container / build-and-push (push) Successful in 1m2s
Build and Push Docker Container / build-and-push (push) Successful in 1m2s
- 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.
This commit is contained in:
@@ -22,6 +22,7 @@ class _CallbackServer:
|
|||||||
self.code: str | None = None
|
self.code: str | None = None
|
||||||
self.error: str | None = None
|
self.error: str | None = None
|
||||||
self.expected_state = expected_state
|
self.expected_state = expected_state
|
||||||
|
self.done = threading.Event()
|
||||||
outer = self
|
outer = self
|
||||||
|
|
||||||
class Handler(BaseHTTPRequestHandler):
|
class Handler(BaseHTTPRequestHandler):
|
||||||
@@ -36,6 +37,7 @@ class _CallbackServer:
|
|||||||
self.send_response(400)
|
self.send_response(400)
|
||||||
self.end_headers()
|
self.end_headers()
|
||||||
self.wfile.write(b'NanoShare CLI login failed: state mismatch')
|
self.wfile.write(b'NanoShare CLI login failed: state mismatch')
|
||||||
|
outer.done.set()
|
||||||
return
|
return
|
||||||
outer.code = params.get('code', [''])[0]
|
outer.code = params.get('code', [''])[0]
|
||||||
outer.error = params.get('error', [''])[0] or None
|
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.')
|
self.wfile.write(b'NanoShare CLI login complete. You can close this browser tab.')
|
||||||
else:
|
else:
|
||||||
self.wfile.write(b'NanoShare CLI login failed.')
|
self.wfile.write(b'NanoShare CLI login failed.')
|
||||||
|
outer.done.set()
|
||||||
|
|
||||||
self.httpd = ThreadingHTTPServer((host, port), Handler)
|
self.httpd = ThreadingHTTPServer((host, port), Handler)
|
||||||
self.url = f'http://{host}:{self.httpd.server_port}/callback'
|
self.url = f'http://{host}:{self.httpd.server_port}/callback'
|
||||||
self.thread = threading.Thread(target=self.httpd.serve_forever, daemon=True)
|
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:
|
def wait_for_code(self, timeout: float) -> str:
|
||||||
self.thread.start()
|
self.start()
|
||||||
self.thread.join(timeout)
|
received_callback = self.done.wait(timeout)
|
||||||
self.httpd.shutdown()
|
self.httpd.shutdown()
|
||||||
|
self.thread.join(5)
|
||||||
if self.error:
|
if self.error:
|
||||||
raise RuntimeError(self.error)
|
raise RuntimeError(self.error)
|
||||||
if not self.code:
|
if not received_callback or not self.code:
|
||||||
raise TimeoutError('login timed out')
|
raise TimeoutError('login timed out')
|
||||||
return self.code
|
return self.code
|
||||||
|
|
||||||
def login(args) -> int:
|
def login(args) -> int:
|
||||||
state = secrets.token_urlsafe(24)
|
state = secrets.token_urlsafe(24)
|
||||||
server = _CallbackServer(args.callback_host, args.callback_port, state)
|
server = _CallbackServer(args.callback_host, args.callback_port, state)
|
||||||
|
server.start()
|
||||||
authorize_url = _make_authorize_url(args.base_url, server.url, state, args.scope)
|
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:
|
if not args.no_browser:
|
||||||
webbrowser.open(authorize_url)
|
webbrowser.open(authorize_url)
|
||||||
else:
|
else:
|
||||||
print(authorize_url)
|
print(authorize_url, flush=True)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
code = server.wait_for_code(args.login_timeout)
|
code = server.wait_for_code(args.login_timeout)
|
||||||
|
|||||||
+17
-3
@@ -12,13 +12,24 @@ from quart import current_app
|
|||||||
cli_auth_bp = Blueprint('cli_auth', __name__)
|
cli_auth_bp = Blueprint('cli_auth', __name__)
|
||||||
|
|
||||||
CLI_CODE_TTL_SECONDS = 300
|
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')
|
@cli_auth_bp.get('/auth/cli/login')
|
||||||
@LIMITER.limit('10 per minute;60 per hour;')
|
@LIMITER.limit('10 per minute;60 per hour;')
|
||||||
async def cli_login():
|
async def cli_login():
|
||||||
redirect_uri = request.args.get('redirect_uri', '').strip()
|
redirect_uri = request.args.get('redirect_uri', '').strip()
|
||||||
state = request.args.get('state', '').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'})
|
add_wide_event_context(auth={'method': 'cli_browser', 'operation': 'cli_login_start'})
|
||||||
|
|
||||||
if not redirect_uri or not state:
|
if not redirect_uri or not state:
|
||||||
@@ -44,7 +55,7 @@ async def cli_login_complete():
|
|||||||
user['sub'],
|
user['sub'],
|
||||||
login_request['redirect_uri'],
|
login_request['redirect_uri'],
|
||||||
login_request['state'],
|
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):
|
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}')
|
await cache.delete(f'cli:login:{code}')
|
||||||
|
|
||||||
user_id = login_data['user_id']
|
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_data = await current_app.convex.add_refresh_token(
|
||||||
token_name='nanoshare-cli',
|
token_name='nanoshare-cli',
|
||||||
user_id=user_id,
|
user_id=user_id,
|
||||||
|
|||||||
+8
-6
@@ -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
|
over the shared servicelink envelope at POST /rpc, alongside the existing web
|
||||||
UI and /api routes.
|
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.
|
limited and body-size capped. Keep /rpc on the internal node network.
|
||||||
'''
|
'''
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
@@ -17,16 +17,18 @@ from quart import current_app
|
|||||||
from my_modules.app.setup import LIMITER
|
from my_modules.app.setup import LIMITER
|
||||||
from my_modules.expiry import ensure_utc, parse_expires
|
from my_modules.expiry import ensure_utc, parse_expires
|
||||||
from my_modules.file_meta import format_size, iso_stamp_filename
|
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
|
MAX_RPC_BODY = 16 * 1024 * 1024
|
||||||
MESH_SCOPE = 'mesh'
|
RPC_SCOPES = ('files', 'mesh')
|
||||||
|
|
||||||
router = Router('picoshare')
|
router = Router('picoshare')
|
||||||
|
|
||||||
def _user_id(ctx):
|
def _user_id(ctx):
|
||||||
if ctx.principal is None:
|
if ctx.principal is None:
|
||||||
raise Unauthorized('authentication required')
|
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
|
return ctx.principal.subject
|
||||||
|
|
||||||
@router.method('files.upload')
|
@router.method('files.upload')
|
||||||
@@ -110,11 +112,11 @@ async def _decode_access_token(token):
|
|||||||
return payload
|
return payload
|
||||||
|
|
||||||
def _build_verify():
|
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.
|
# 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')
|
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()
|
verify = _build_verify()
|
||||||
link_bp = create_link_blueprint(router, verify=verify, limiter=LIMITER.limit('30 per minute'), max_body=MAX_RPC_BODY)
|
link_bp = create_link_blueprint(router, verify=verify, limiter=LIMITER.limit('30 per minute'), max_body=MAX_RPC_BODY)
|
||||||
|
|||||||
@@ -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')
|
||||||
@@ -1,10 +1,13 @@
|
|||||||
import json
|
import json
|
||||||
import sys
|
import sys
|
||||||
|
import threading
|
||||||
|
import urllib.request
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'cli'))
|
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'cli'))
|
||||||
|
|
||||||
from nanoshare_client import cli
|
from nanoshare_client import cli
|
||||||
|
from nanoshare_client.auth import _CallbackServer
|
||||||
|
|
||||||
class FakeClient:
|
class FakeClient:
|
||||||
def __init__(self):
|
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())
|
state = json.loads((tmp_path / '.nanoshare-sync' / 'state.json').read_text())
|
||||||
assert state['files']['hello.txt']['file_id'] == 'file_1'
|
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):
|
def test_sync_deletes_old_remote_when_local_file_changes(tmp_path, monkeypatch):
|
||||||
fake = FakeClient()
|
fake = FakeClient()
|
||||||
fake.remote_files = [{'file_id': 'old_file', 'file_name': 'hello.txt', 'file_size': '5 B'}]
|
fake.remote_files = [{'file_id': 'old_file', 'file_name': 'hello.txt', 'file_size': '5 B'}]
|
||||||
|
|||||||
@@ -56,6 +56,9 @@ class FakeConvex:
|
|||||||
async def get_file(self, file_id):
|
async def get_file(self, file_id):
|
||||||
return self.files.get(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):
|
def _call(envelope, token=None):
|
||||||
# Call the handler the /rpc route delegates to, bypassing the LIMITER/size-cap
|
# Call the handler the /rpc route delegates to, bypassing the LIMITER/size-cap
|
||||||
# wrapper; exercises the real verify (mesh scope) + dispatch + handlers.
|
# wrapper; exercises the real verify (mesh scope) + dispatch + handlers.
|
||||||
@@ -100,7 +103,7 @@ def test_missing_token_is_unauthorized():
|
|||||||
assert status == 401
|
assert status == 401
|
||||||
assert body['error']['code'] == 'unauthorized'
|
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')
|
status, body = _call(_request('files.list'), token='nomesh')
|
||||||
assert status == 403
|
assert status == 200
|
||||||
assert body['error']['code'] == 'forbidden'
|
assert body['ok'] is True
|
||||||
|
|||||||
Reference in New Issue
Block a user