From 5b16f5485739c333a00891f168f396e85757ac2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?dni=20=E2=9A=A1?= Date: Mon, 25 Sep 2023 15:06:00 +0200 Subject: [PATCH] [FEAT] cleanup GET /wallet endpoint, add wallet api routes (#1932) * [FEAT] cleanup GET /wallet endpoint, add wallet api route this removes the functionalitiy to create accounts and wallets via the GET /wallet endpoint in generic.py it add endpoints inside the api.py for it and the frontend is modified to use the api endpoints this also simplifies for the `feat/login` for the route. * remove stale generic tests and add api tests * bug wrong endpoint create account * vlad nitpick * added checkif deleted is 404 * reload after renaming wallet * another iteration with vlad * create new wallet if it none exist * fix delete refresh * formatting --- lnbits/core/crud.py | 2 +- lnbits/core/models.py | 4 + lnbits/core/static/js/index.js | 4 +- lnbits/core/static/js/wallet.js | 26 ++++-- lnbits/core/templates/core/wallet.html | 2 +- lnbits/core/views/api.py | 28 ++++++ lnbits/core/views/generic.py | 118 +++++++------------------ lnbits/static/js/base.js | 38 +++++--- lnbits/static/js/components.js | 2 +- tests/core/views/test_api.py | 46 +++++++++- tests/core/views/test_generic.py | 52 +---------- 11 files changed, 158 insertions(+), 164 deletions(-) diff --git a/lnbits/core/crud.py b/lnbits/core/crud.py index 24cd47690..7c9ca0f7e 100644 --- a/lnbits/core/crud.py +++ b/lnbits/core/crud.py @@ -77,7 +77,7 @@ async def get_user(user_id: str, conn: Optional[Connection] = None) -> Optional[ SELECT balance FROM balances WHERE wallet = wallets.id ), 0) AS balance_msat FROM wallets - WHERE "user" = ? + WHERE "user" = ? and wallets.deleted = false """, (user_id,), ) diff --git a/lnbits/core/models.py b/lnbits/core/models.py index 3668aef01..d768b9202 100644 --- a/lnbits/core/models.py +++ b/lnbits/core/models.py @@ -339,6 +339,10 @@ class CreateLnurlAuth(BaseModel): callback: str +class CreateWallet(BaseModel): + name: Optional[str] = None + + class CreateWebPushSubscription(BaseModel): subscription: str diff --git a/lnbits/core/static/js/index.js b/lnbits/core/static/js/index.js index 0a930e1ed..052b4e891 100644 --- a/lnbits/core/static/js/index.js +++ b/lnbits/core/static/js/index.js @@ -12,7 +12,9 @@ new Vue({ }, methods: { createWallet: function () { - LNbits.href.createWallet(this.walletName) + LNbits.api.createAccount(this.walletName).then(res => { + window.location = '/wallet?usr=' + res.data.user + '&wal=' + res.data.id + }) }, processing: function () { this.$q.notify({ diff --git a/lnbits/core/static/js/wallet.js b/lnbits/core/static/js/wallet.js index b2b7cba35..2f2120497 100644 --- a/lnbits/core/static/js/wallet.js +++ b/lnbits/core/static/js/wallet.js @@ -702,7 +702,7 @@ new Vue({ LNbits.api .authLnurl(this.g.wallet, this.parse.lnurlauth.callback) - .then(response => { + .then(_ => { dismissAuthMsg() this.$q.notify({ message: `Authentication successful.`, @@ -728,27 +728,35 @@ new Vue({ updateWallet: function (data) { LNbits.api .request('PATCH', '/api/v1/wallet', this.g.wallet.adminkey, data) - .then(res => { + .then(_ => { this.$q.notify({ message: `Wallet updated.`, type: 'positive', timeout: 3500 }) - LNbits.href.updateWallet( - res.data.name, - this.user.id, - this.g.wallet.id - ) + window.location.reload() }) .catch(err => { LNbits.utils.notifyApiError(err) }) }, - deleteWallet: function (walletId, user) { + deleteWallet: function () { LNbits.utils .confirmDialog('Are you sure you want to delete this wallet?') .onOk(() => { - LNbits.href.deleteWallet(walletId, user) + LNbits.api + .deleteWallet(this.g.wallet) + .then(_ => { + this.$q.notify({ + timeout: 3000, + message: `Wallet deleted!`, + spinner: true + }) + }) + .catch(err => { + this.paymentsTable.loading = false + LNbits.utils.notifyApiError(err) + }) }) }, fetchPayments: function (props) { diff --git a/lnbits/core/templates/core/wallet.html b/lnbits/core/templates/core/wallet.html index 534fe52ec..6349272bc 100644 --- a/lnbits/core/templates/core/wallet.html +++ b/lnbits/core/templates/core/wallet.html @@ -428,7 +428,7 @@ diff --git a/lnbits/core/views/api.py b/lnbits/core/views/api.py index 5b9a3ffb3..6df09d39c 100644 --- a/lnbits/core/views/api.py +++ b/lnbits/core/views/api.py @@ -36,6 +36,7 @@ from lnbits.core.models import ( CreateInvoice, CreateLnurl, CreateLnurlAuth, + CreateWallet, CreateWebPushSubscription, DecodePayment, Payment, @@ -75,11 +76,14 @@ from lnbits.utils.exchange_rates import ( from ..crud import ( DateTrunc, add_installed_extension, + create_account, create_tinyurl, + create_wallet, create_webpush_subscription, delete_dbversion, delete_installed_extension, delete_tinyurl, + delete_wallet, delete_webpush_subscription, drop_extension_db, get_dbversions, @@ -148,6 +152,30 @@ async def api_update_wallet( return await update_wallet(wallet.wallet.id, name, currency) +@api_router.delete("/api/v1/wallet") +async def api_delete_wallet( + wallet: WalletTypeInfo = Depends(require_admin_key), +) -> None: + await delete_wallet( + user_id=wallet.wallet.user, + wallet_id=wallet.wallet.id, + ) + + +@api_router.post("/api/v1/wallet", response_model=Wallet) +async def api_create_wallet( + data: CreateWallet, + wallet: WalletTypeInfo = Depends(require_admin_key), +) -> Wallet: + return await create_wallet(user_id=wallet.wallet.user, wallet_name=data.name) + + +@api_router.post("/api/v1/account", response_model=Wallet) +async def api_create_account(data: CreateWallet) -> Wallet: + account = await create_account() + return await create_wallet(user_id=account.id, wallet_name=data.name) + + @api_router.get( "/api/v1/payments", name="Payment List", diff --git a/lnbits/core/views/generic.py b/lnbits/core/views/generic.py index ce109ca6e..06657ad7e 100644 --- a/lnbits/core/views/generic.py +++ b/lnbits/core/views/generic.py @@ -23,7 +23,6 @@ from ...utils.exchange_rates import currencies from ..crud import ( create_account, create_wallet, - delete_wallet, get_balance_check, get_dbversions, get_inactive_extensions, @@ -161,77 +160,52 @@ async def extensions_install( @generic_router.get( "/wallet", response_class=HTMLResponse, - description=""" -just **wallet_name**: create a new user, then create a new wallet - for user with wallet_name -just **user_id**: return the first user wallet or create one if none found - (with default wallet_name) -**user_id** and **wallet_name**: create a new wallet for user with wallet_name -**user_id** and **wallet_id**: return that wallet if user is the owner -nothing: create everything -""", + description="show wallet page", ) async def wallet( request: Request, - nme: Optional[str] = Query(None), - usr: Optional[UUID4] = Query(None), + usr: UUID4 = Query(...), wal: Optional[UUID4] = Query(None), ): - user_id = usr.hex if usr else None - wallet_id = wal.hex if wal else None - wallet_name = nme + user_id = usr.hex + user = await get_user(user_id) - if not user_id: - new_user = await create_account() - user = await get_user(new_user.id) - assert user, "Newly created user has to exist." - logger.info(f"Create user {user.id}") - else: - user = await get_user(user_id) - if not user: - return template_renderer().TemplateResponse( - "error.html", {"request": request, "err": "User does not exist."} - ) - if ( - len(settings.lnbits_allowed_users) > 0 - and user_id not in settings.lnbits_allowed_users - and user_id not in settings.lnbits_admin_users - and user_id != settings.super_user - ): - return template_renderer().TemplateResponse( - "error.html", {"request": request, "err": "User not authorized."} - ) - if user_id == settings.super_user or user_id in settings.lnbits_admin_users: - user.admin = True - if user_id == settings.super_user: - user.super_user = True - - if not wallet_id: - if user.wallets and not wallet_name: - wallet = user.wallets[0] - else: - wallet = await create_wallet(user_id=user.id, wallet_name=wallet_name) - logger.info( - f"Created new wallet {wallet_name if wallet_name else '(no name)'} for" - f" user {user.id}" - ) - - return RedirectResponse( - f"/wallet?usr={user.id}&wal={wallet.id}", - status_code=status.HTTP_307_TEMPORARY_REDIRECT, + if not user: + return template_renderer().TemplateResponse( + "error.html", {"request": request, "err": "User does not exist."} ) - logger.debug( - "Access " - f"{'user '+ user.id + ' ' if user else ''} " - f"{'wallet ' + wallet_name if wallet_name else ''}" - ) + if not wal: + if len(user.wallets) == 0: + wallet = await create_wallet(user_id=user.id) + return RedirectResponse(url=f"/wallet?usr={user_id}&wal={wallet.id}") + return RedirectResponse(url=f"/wallet?usr={user_id}&wal={user.wallets[0].id}") + else: + wallet_id = wal.hex + userwallet = user.get_wallet(wallet_id) - if not userwallet: + if not userwallet or userwallet.deleted: return template_renderer().TemplateResponse( "error.html", {"request": request, "err": "Wallet not found"} ) + if ( + len(settings.lnbits_allowed_users) > 0 + and user_id not in settings.lnbits_allowed_users + and user_id not in settings.lnbits_admin_users + and user_id != settings.super_user + ): + return template_renderer().TemplateResponse( + "error.html", {"request": request, "err": "User not authorized."} + ) + + if user_id == settings.super_user or user_id in settings.lnbits_admin_users: + user.admin = True + if user_id == settings.super_user: + user.super_user = True + + logger.debug(f"Access user {user.id} wallet {userwallet.name}") + return template_renderer().TemplateResponse( "core/wallet.html", { @@ -312,32 +286,6 @@ async def lnurl_full_withdraw_callback(request: Request): return {"status": "OK"} -@generic_router.get("/deletewallet", response_class=RedirectResponse) -async def deletewallet(wal: str = Query(...), usr: str = Query(...)): - user = await get_user(usr) - if not user: - raise HTTPException(HTTPStatus.FORBIDDEN, "User not found.") - - user_wallet_ids = [u.id for u in user.wallets] - - if wal not in user_wallet_ids: - raise HTTPException(HTTPStatus.FORBIDDEN, "Not your wallet.") - else: - await delete_wallet(user_id=user.id, wallet_id=wal) - user_wallet_ids.remove(wal) - logger.debug("Deleted wallet {wal} of user {user.id}") - - if user_wallet_ids: - return RedirectResponse( - url_for("/wallet", usr=user.id, wal=user_wallet_ids[0]), - status_code=status.HTTP_307_TEMPORARY_REDIRECT, - ) - - return RedirectResponse( - url_for("/"), status_code=status.HTTP_307_TEMPORARY_REDIRECT - ) - - @generic_router.get("/withdraw/notify/{service}") async def lnurl_balance_notify(request: Request, service: str): wal_param = request.query_params.get("wal") diff --git a/lnbits/static/js/base.js b/lnbits/static/js/base.js index e475bd682..1d3180a6f 100644 --- a/lnbits/static/js/base.js +++ b/lnbits/static/js/base.js @@ -64,9 +64,35 @@ window.LNbits = { callback }) }, + createAccount: function (name) { + return this.request('post', '/api/v1/account', null, { + name: name + }) + }, getWallet: function (wallet) { return this.request('get', '/api/v1/wallet', wallet.inkey) }, + createWallet: function (wallet, name) { + return this.request('post', '/api/v1/wallet', wallet.adminkey, { + name: name + }).then(res => { + window.location = '/wallet?usr=' + res.data.user + '&wal=' + res.data.id + }) + }, + updateWallet: function (name, wallet) { + return this.request('patch', '/api/v1/wallet', wallet.adminkey, { + name: name + }) + }, + deleteWallet: function (wallet) { + return this.request('delete', '/api/v1/wallet', wallet.adminkey).then( + _ => { + let url = new URL(window.location.href) + url.searchParams.delete('wal') + window.location = url + } + ) + }, getPayments: function (wallet, query) { const params = new URLSearchParams(query) return this.request( @@ -118,18 +144,6 @@ window.LNbits = { } } }, - href: { - createWallet: function (walletName, userId) { - window.location.href = - '/wallet?' + (userId ? 'usr=' + userId + '&' : '') + 'nme=' + walletName - }, - updateWallet: function (walletName, userId, walletId) { - window.location.href = `/wallet?usr=${userId}&wal=${walletId}&nme=${walletName}` - }, - deleteWallet: function (walletId, userId) { - window.location.href = '/deletewallet?usr=' + userId + '&wal=' + walletId - } - }, map: { extension: function (data) { var obj = _.object( diff --git a/lnbits/static/js/components.js b/lnbits/static/js/components.js index 476b5396e..8439e6b65 100644 --- a/lnbits/static/js/components.js +++ b/lnbits/static/js/components.js @@ -86,7 +86,7 @@ Vue.component('lnbits-wallet-list', { }, methods: { createWallet: function () { - LNbits.href.createWallet(this.walletName, this.user.id) + LNbits.api.createWallet(this.user.wallets[0], this.walletName) }, updateWalletBalance: function (payload) { this.activeBalance = payload diff --git a/tests/core/views/test_api.py b/tests/core/views/test_api.py index d839a225c..b90818fb7 100644 --- a/tests/core/views/test_api.py +++ b/tests/core/views/test_api.py @@ -23,11 +23,51 @@ from ...helpers import ( WALLET = get_wallet_class() -# check if the client is working +# create account POST /api/v1/account @pytest.mark.asyncio -async def test_core_views_generic(client): - response = await client.get("/") +async def test_create_account(client): + response = await client.post("/api/v1/account", json={"name": "test"}) assert response.status_code == 200 + result = response.json() + assert "name" in result + assert result["name"] == "test" + assert "balance_msat" in result + assert "id" in result + assert "user" in result + + +# check POST and DELETE /api/v1/wallet with adminkey: +# create additional wallet and delete it +@pytest.mark.asyncio +async def test_create_wallet_and_delete(client, adminkey_headers_to): + response = await client.post( + "/api/v1/wallet", json={"name": "test"}, headers=adminkey_headers_to + ) + assert response.status_code == 200 + result = response.json() + assert "name" in result + assert result["name"] == "test" + assert "balance_msat" in result + assert "id" in result + assert "adminkey" in result + response = await client.delete( + "/api/v1/wallet", + headers={ + "X-Api-Key": result["adminkey"], + "Content-type": "application/json", + }, + ) + assert response.status_code == 200 + + # get deleted wallet + response = await client.get( + "/api/v1/wallet", + headers={ + "X-Api-Key": result["adminkey"], + "Content-type": "application/json", + }, + ) + assert response.status_code == 404 # check GET /api/v1/wallet with inkey: wallet info, no balance diff --git a/tests/core/views/test_generic.py b/tests/core/views/test_generic.py index 81dd538e9..5dd1025a0 100644 --- a/tests/core/views/test_generic.py +++ b/tests/core/views/test_generic.py @@ -7,35 +7,6 @@ async def test_core_views_generic(client): assert response.status_code == 200, f"{response.url} {response.status_code}" -# check GET /wallet: wallet info -@pytest.mark.asyncio -async def test_get_wallet(client): - response = await client.get("wallet") - # redirect not modified - assert response.status_code == 307, f"{response.url} {response.status_code}" - - -# check GET /wallet: do not allow redirects, expect code 307 -@pytest.mark.asyncio -async def test_get_wallet_no_redirect(client): - response = await client.get("wallet", follow_redirects=False) - assert response.status_code == 307, f"{response.url} {response.status_code}" - - # determine the next redirect location - request = client.build_request("GET", "wallet") - i = 0 - while request is not None: - response = await client.send(request) - request = response.next_request - if i == 0: - # first redirect - assert response.status_code == 307, f"{response.url} {response.status_code}" - elif i == 1: - # then get the actual page - assert response.status_code == 200, f"{response.url} {response.status_code}" - i += 1 - - # check GET /wallet: wrong user, expect 400 @pytest.mark.asyncio async def test_get_wallet_with_nonexistent_user(client): @@ -43,27 +14,6 @@ async def test_get_wallet_with_nonexistent_user(client): assert response.status_code == 400, f"{response.url} {response.status_code}" -# check GET /wallet: with user -@pytest.mark.asyncio -async def test_get_wallet_with_user(client, to_user): - response = await client.get("wallet", params={"usr": to_user.id}) - assert response.status_code == 307, f"{response.url} {response.status_code}" - - # determine the next redirect location - request = client.build_request("GET", "wallet", params={"usr": to_user.id}) - i = 0 - while request is not None: - response = await client.send(request) - request = response.next_request - if i == 0: - # first redirect - assert response.status_code == 307, f"{response.url} {response.status_code}" - elif i == 1: - # then get the actual page - assert response.status_code == 200, f"{response.url} {response.status_code}" - i += 1 - - # check GET /wallet: wallet and user @pytest.mark.asyncio async def test_get_wallet_with_user_and_wallet(client, to_user, to_wallet): @@ -89,7 +39,7 @@ async def test_get_extensions(client, to_user): # check GET /extensions: extensions list wrong user, expect 400 @pytest.mark.asyncio -async def test_get_extensions_wrong_user(client, to_user): +async def test_get_extensions_wrong_user(client): response = await client.get("extensions", params={"usr": "1"}) assert response.status_code == 400, f"{response.url} {response.status_code}"