diff --git a/plugins/clnrest/utilities/rpc_routes.py b/plugins/clnrest/utilities/rpc_routes.py index 7b7d8333d..78a8dd9c2 100644 --- a/plugins/clnrest/utilities/rpc_routes.py +++ b/plugins/clnrest/utilities/rpc_routes.py @@ -1,7 +1,8 @@ +from pyln.client import RpcError import json5 from flask import request, make_response from flask_restx import Namespace, Resource -from .shared import call_rpc_method, verify_rune, process_help_response +from .shared import call_rpc_method, verify_rune, process_help_response, RuneError from .rpc_plugin import plugin methods_list = [] @@ -42,20 +43,19 @@ class RpcMethodResource(Resource): rpc_params = request.form.to_dict() if not request.is_json else request.get_json() if len(request.data) != 0 else {} try: - is_valid_rune = verify_rune(plugin, rune, rpc_method, rpc_params) - if "error" in is_valid_rune: - plugin.log(f"Error: {is_valid_rune}", "error") - raise Exception(is_valid_rune) - - except Exception as err: - return json5.loads(str(err)), 401 + verify_rune(plugin, rune, rpc_method, rpc_params) + except RpcError as rpc_err: + plugin.log(f"RPC Error: {str(rpc_err.error)}", "info") + return rpc_err.error, 401 + except RuneError as rune_err: + plugin.log(f"Rune Error: {str(rune_err)}", "info") + return rune_err.error, 403 try: return call_rpc_method(plugin, rpc_method, rpc_params), 201 - - except Exception as err: - plugin.log(f"Error: {err}", "info") - return json5.loads(str(err)), 500 + except RpcError as rpc_err: + plugin.log(f"RPC Error: {str(rpc_err.error)}", "info") + return (rpc_err.error, 404) if rpc_err.error["code"] == -32601 else (rpc_err.error, 500) except Exception as err: return f"Unable to parse request: {err}", 500 diff --git a/plugins/clnrest/utilities/shared.py b/plugins/clnrest/utilities/shared.py index eab00af3f..3b51369e1 100644 --- a/plugins/clnrest/utilities/shared.py +++ b/plugins/clnrest/utilities/shared.py @@ -1,12 +1,16 @@ import json5 -import re -import json import ipaddress CERTS_PATH, REST_PROTOCOL, REST_HOST, REST_PORT, REST_CSP, REST_CORS_ORIGINS = "", "", "", "", "", [] +class RuneError(Exception): + def __init__(self, error=str({"code": 1501, "message": "Not authorized: Missing or invalid rune"})): + self.error = error + super().__init__(self.error) + + def validate_ip4(ip_str): try: # Create an IPv4 address object. @@ -61,34 +65,12 @@ def set_config(options): def call_rpc_method(plugin, rpc_method, payload): - try: - response = plugin.rpc.call(rpc_method, payload) - if '"error":' in str(response).lower(): - raise Exception(response) - else: - plugin.log(f"{response}", "debug") - if '"result":' in str(response).lower(): - # Use json5.loads ONLY when necessary, as it increases processing time - return json.loads(response)["result"] - else: - return response - - except Exception as err: - plugin.log(f"Error: {err}", "info") - if "error" in str(err).lower(): - match_err_obj = re.search(r'"error":\{.*?\}', str(err)) - if match_err_obj is not None: - err = "{" + match_err_obj.group() + "}" - else: - match_err_str = re.search(r"error: \{.*?\}", str(err)) - if match_err_str is not None: - err = "{" + match_err_str.group() + "}" - raise Exception(err) + return plugin.rpc.call(rpc_method, payload) def verify_rune(plugin, rune, rpc_method, rpc_params): if rune is None: - raise Exception('{ "error": {"code": 403, "message": "Not authorized: Missing rune"} }') + raise RuneError({"code": 1501, "message": "Not authorized: Missing rune"}) return call_rpc_method(plugin, "checkrune", {"rune": rune, diff --git a/tests/test_clnrest.py b/tests/test_clnrest.py index 388b82f7f..da9ee654e 100644 --- a/tests/test_clnrest.py +++ b/tests/test_clnrest.py @@ -1,6 +1,7 @@ from ephemeral_port_reserve import reserve from fixtures import * # noqa: F401,F403 from pyln.testing.utils import env, TEST_NETWORK +from pyln.client import Millisatoshi import unittest import requests from pathlib import Path @@ -164,9 +165,9 @@ def test_clnrest_unknown_method(node_factory): """Test POST request error on `/v1/unknown-post` end point.""" rune = l1.rpc.createrune()['rune'] response = http_session.post(base_url + '/v1/unknown-post', headers={'Rune': rune}, verify=ca_cert) - assert response.status_code == 500 - assert response.json()['error']['code'] == -32601 - assert response.json()['error']['message'] == "Unknown command 'unknown-post'" + assert response.status_code == 404 + assert response.json()['code'] == -32601 + assert response.json()['message'] == "Unknown command 'unknown-post'" def test_clnrest_rpc_method(node_factory): @@ -177,17 +178,17 @@ def test_clnrest_rpc_method(node_factory): # /v1/getinfo no rune provided in header of the request response = http_session.post(base_url + '/v1/getinfo', verify=ca_cert) - assert response.status_code == 401 - assert response.json()['error']['code'] == 403 - assert response.json()['error']['message'] == 'Not authorized: Missing rune' + assert response.status_code == 403 + assert response.json()['code'] == 1501 + assert response.json()['message'] == 'Not authorized: Missing rune' # /v1/getinfo with a rune which doesn't authorized getinfo method rune_no_getinfo = l1.rpc.createrune(restrictions=[["method/getinfo"]])['rune'] response = http_session.post(base_url + '/v1/getinfo', headers={'Rune': rune_no_getinfo}, verify=ca_cert) assert response.status_code == 401 - assert response.json()['error']['code'] == 1502 - assert response.json()['error']['message'] == 'Not permitted: method is equal to getinfo' + assert response.json()['code'] == 1502 + assert response.json()['message'] == 'Not permitted: method is equal to getinfo' # /v1/getinfo with a correct rune rune_getinfo = l1.rpc.createrune(restrictions=[["method=getinfo"]])['rune'] @@ -201,7 +202,7 @@ def test_clnrest_rpc_method(node_factory): response = http_session.post(base_url + '/v1/invoice', headers={'Rune': rune_invoice}, verify=ca_cert) assert response.status_code == 500 - assert response.json()['error']['code'] == -32602 + assert response.json()['code'] == -32602 # /v1/invoice with a correct rune but wrong parameters rune_invoice = l1.rpc.createrune(restrictions=[["method=invoice"]])['rune'] @@ -211,7 +212,7 @@ def test_clnrest_rpc_method(node_factory): 'description': 'description'}, verify=ca_cert) assert response.status_code == 500 - assert response.json()['error']['code'] == -32602 + assert response.json()['code'] == -32602 # l2 pays l1's invoice where the invoice is created with /v1/invoice rune_invoice = l1.rpc.createrune(restrictions=[["method=invoice"]])['rune'] @@ -282,7 +283,7 @@ def test_clnrest_websocket_wrong_rune(node_factory): notifications = notifications_received_via_websocket(l1, base_url, http_session) l1.daemon.logsearch_start = 0 - assert l1.daemon.is_in_log(r"plugin-clnrest.py: {error: {'code': 1501, 'message': 'Not authorized: Not derived from master'}}") + assert l1.daemon.is_in_log(r"error: {'code': 1501, 'message': 'Not authorized: Not derived from master'}") assert len(notifications) == 0