-
Notifications
You must be signed in to change notification settings - Fork 105
session_rpcserver+litrpc: allow custom session with all read-only perms #457
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,14 @@ import ( | |
"gopkg.in/macaroon.v2" | ||
) | ||
|
||
// readOnlyAction defines the keyword that a permission action should be set to | ||
// when the entity is set to "uri" in order to activate the special case that | ||
// will result in all read-only permissions known to lit to be added to a | ||
// session's macaroon. The purpose of the three '*'s is to make this keyword | ||
// an invalid URI and an invalid regex so that it does not ever clash with the | ||
// other special cases. | ||
const readOnlyAction = "***readonly***" | ||
|
||
// sessionRpcServer is the gRPC server for the Session RPC interface. | ||
type sessionRpcServer struct { | ||
litrpc.UnimplementedSessionsServer | ||
|
@@ -152,6 +160,20 @@ func (s *sessionRpcServer) AddSession(_ context.Context, | |
continue | ||
} | ||
|
||
// If the action specified was equal to the | ||
// readOnlyAction keyword, then this is taken to mean | ||
// that the permissions for all read-only URIs should be | ||
// granted. | ||
if op.Action == readOnlyAction { | ||
readPerms := s.cfg.permMgr.ActivePermissions( | ||
true, | ||
) | ||
|
||
permissions = append(permissions, readPerms...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we do a de-duplication somewhere? Because maybe the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 👍 |
||
|
||
continue | ||
} | ||
|
||
// First check if this is a regex URI. | ||
uris, isRegex := s.cfg.permMgr.MatchRegexURI(op.Action) | ||
if isRegex { | ||
|
Uh oh!
There was an error while loading. Please reload this page.