Add FileTransferRequests to session for moderated scp#23875
Conversation
mdwn
left a comment
mdwn
left a comment
There was a problem hiding this comment.
Just two small things. Feel free to push back!
| // File transfers in a moderated session require these two variablesto check for | ||
| // approval on the ssh server. If they exist in the context, set them in our env vars | ||
| if moderatedSessionID, ok := ctx.Value(scp.ModeratedSession).(string); ok { | ||
| s.Setenv(ctx, scp.ModeratedSession, moderatedSessionID) | ||
| } | ||
| if fileTransferRequestID, ok := ctx.Value(scp.FileTransferRequest).(string); ok { | ||
| s.Setenv(ctx, scp.FileTransferRequest, fileTransferRequestID) | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: Should we add an error here if only one of these two vars is supplied?
There was a problem hiding this comment.
Good call. I won't do it here tho, I think in the api handler is better to catch it early on. Thanks!
| return sess.term.GetWinSize() | ||
| } | ||
|
|
||
| func (s *SessionRegistry) isApprovedFileTransfer(ctx context.Context, scx *ServerContext) (bool, error) { |
There was a problem hiding this comment.
Let's add some tests for this function since it seems complex enough to warrant it.
There was a problem hiding this comment.
Gave the test a shot although the test seemed to be a bit complex (maybe unavoidable?). Happy for input
fspmarshall
left a comment
fspmarshall
left a comment
There was a problem hiding this comment.
lgmt once existing feedback is addressed.
There was a problem hiding this comment.
I think better name would be FileTransferRequestID to more accurately show what it is and be consistent with the rest of the naming. (Same for ModeratedSession)
Part of #23546 This will add a fileTransferRequest to a session and allow environment variables to be passed from the webUI in order to validate a request that happens "outside" the moderated session (via HTTP request).
avatus
force-pushed
the
michaelmyers/moderated_session_scp_check
branch
from
5ffe88e to
a5e8a18
Compare
5 participants
Part of #23546
This will add a fileTransferRequest to a session and allow environment variables to be passed from the webUI in order to validate a request that happens "outside" the moderated session (via HTTP request).
The next PR to follow will be the webui/terminal updates that include the approval process.
Note: The
fileTransferRequeststruct varies slightly from the RFD as I found most of the fields unused outside of the approval process. This will most likely be expanded to closer match in the RFD in the subsequent PR.