Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpc: Bake and validate macaroons with external permissions #5304

Merged
merged 6 commits into from Sep 15, 2021

Conversation

orbitalturtle
Copy link
Contributor

Following step two of these directions for stateless-remote mode (lightninglabs/lightning-terminal#160), this PR:

  • Adds an option to the BakeMacaroon rpc "allow-external-permissions," which makes it possible to bake a macaroon with external permissions. That way, the baked macaroons can be used for services beyond LND.
  • Adds a new CheckMacaroonPermissions rpc that checks that the macaroon permissions and other restrictions are being followed. It can also check permissions not native to LND.

Use case: With the new BakeMacaroon option, users of lightning terminal can bake a "super" macaroon permitting a user to use LND calls, as well as calls to the daemons Loop, Pool, and Faraday. The daemons incorporated into lightning terminal can use CheckMacaroonPermissions to validate the macaroon

@orbitalturtle orbitalturtle force-pushed the check-macaroon-rpcs branch 2 times, most recently from 4c463a1 to 8a05ff5 Compare May 17, 2021 08:04
@guggero guggero self-requested a review May 17, 2021 10:13
@orbitalturtle orbitalturtle force-pushed the check-macaroon-rpcs branch 3 times, most recently from 7cedca4 to 56651aa Compare May 17, 2021 23:16
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great feature, thanks a lot for the PR!
Also nice commit structure, was very easy to review.

Did a high-level pass and left some comments. Just a heads up: As we're currently in the RC phase for v0.13.0-beta this won't make it into that version and further reviews might be somewhat delayed. Hope to get this in soon in the v0.14.0 cycle though!

lnrpc/rpc.proto Outdated Show resolved Hide resolved
macaroons/service.go Outdated Show resolved Hide resolved
macaroons/service.go Outdated Show resolved Hide resolved
macaroons/service.go Outdated Show resolved Hide resolved
lnrpc/rpc.proto Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
cmd/lncli/cmd_macaroon.go Outdated Show resolved Hide resolved
cmd/lncli/cmd_macaroon.go Outdated Show resolved Hide resolved
lntest/itest/lnd_macaroons_test.go Outdated Show resolved Hide resolved
@guggero guggero added enhancement Improvements to existing features / behaviour macaroons rpc Related to the RPC interface labels May 18, 2021
@guggero guggero added this to the v0.14.0 milestone May 18, 2021
@Roasbeef Roasbeef requested a review from ellemouton July 1, 2021 01:41
@Roasbeef Roasbeef added this to In progress in v0.14.0-beta via automation Jul 1, 2021
@Roasbeef Roasbeef moved this from In progress to Review in progress in v0.14.0-beta Jul 1, 2021
@orbitalturtle orbitalturtle force-pushed the check-macaroon-rpcs branch 3 times, most recently from b7fc717 to 51dacf5 Compare July 6, 2021 04:13
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment about returning the error, otherwise LGTM 🎉

macaroons/service.go Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I learned a lot while reviewing this :) Great commit structure.

I left some nit comments (mostly just about updating rest-annotations.yml and re-running make rpc) and one question about the arguments passed to CheckMacAuth

lnrpc/rpc.pb.go Outdated Show resolved Hide resolved
lnrpc/rpc.proto Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
rpcserver.go Show resolved Hide resolved
@Roasbeef
Copy link
Member

Just needs an addition to the release notes now for 0.14!

orbitalturtle added a commit to orbitalturtle/lnd that referenced this pull request Aug 1, 2021
@orbitalturtle
Copy link
Contributor Author

orbitalturtle commented Aug 1, 2021

Great, rebased that and added some release notes @Roasbeef !

Also thank you for the reviews @guggero and @ellemouton !

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there, found 3 more small issues 😅

lnrpc/lightning.yaml Show resolved Hide resolved
lnrpc/rest-annotations.yaml Outdated Show resolved Hide resolved
rpcserver.go Show resolved Hide resolved
@orijbot
Copy link

orijbot commented Sep 14, 2021

@orbitalturtle
Copy link
Contributor Author

Sorry for taking so long - made those changes @guggero

@guggero guggero merged commit 08c9d3f into lightningnetwork:master Sep 15, 2021
v0.14.0-beta automation moved this from Reviewer approved to Done Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features / behaviour macaroons rpc Related to the RPC interface
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants