session_rpcserver+litrpc: allow custom session with all read-only perms #457
Conversation
ellemouton
force-pushed
the
customPlusReadOnlyPerms
branch
from
8a344a6 to
1b7f1ca
Compare
In this commit, a special case is added to the creation of a custom session to allow the user to specify custom URIs as well as the permissions for all read-only endpoints.
ellemouton
force-pushed
the
customPlusReadOnlyPerms
branch
from
1b7f1ca to
b9d1208
Compare
litrpc/lit-sessions.proto
Outdated
| // - a URI regex, in which case access will be granted to each URI that | ||
| // matches the regex. | ||
| // - the "***readonly***" keyword. This will result in the access being | ||
| // granted all read-only endpoints. |
There was a problem hiding this comment.
nit: granted to all read-only endpoints?
session_rpcserver.go
Outdated
| true, | ||
| ) | ||
|
|
||
| permissions = append(permissions, readPerms...) |
There was a problem hiding this comment.
Do we do a de-duplication somewhere? Because maybe the ***readonly*** overlaps with URIs matched by a regex?
There was a problem hiding this comment.
Good idea re adding de-dupping. I actually though we already did that somewhere but turns out not! So added it in a new commit here :)
I dont actually think the perms from ***readonly*** will ever clash with the regex URIs since those will all be using the special case "uri" entity. But yeah - good to have the dedup stuff for future!
There was a problem hiding this comment.
(added in the last commit so that the first 2 commits dont need to be re-reviewed. Hope that that is ok)
There was a problem hiding this comment.
I was thinking forward to when we'd convert all permissions into URIs anyway, then I can imagine there would be some overlap possible. Additional commit looks good 👍
The proto comments are updated to reflect the special case behaviour of the entity-action pairs.
ellemouton
force-pushed
the
customPlusReadOnlyPerms
branch
from
b9d1208 to
9360087
Compare
ellemouton
force-pushed
the
customPlusReadOnlyPerms
branch
from
9360087 to
00357fd
Compare
|
Thanks for the review @guggero ! Added one more commit to address your comment about de-duplicating the perms. |
There was a problem hiding this comment.
tACK. I rebased my pr #451 using this branch and was able to create a custom session using the new ***readonly*** flag
A special case is added to the creation of a custom session to
allow the user to specify custom URIs as well as the permissions
for all read-only endpoints. To use this option when creating a
custom session, a user should set the
entityof the macaroonpermission to
"uri"and theactionto"***readonly***"The reason for the using the three '*'s in the string is to ensure
that the string never collides with a valid URI or a valid regex.
litcliexample:Fixes #456