macos presence detection #1867
macos presence detection #1867
Conversation
| @@ -182,9 +184,10 @@ func runDesktop(_ *multislogger.MultiSlogger, args []string) error { | |||
| } | |||
| }() | |||
|
|
|||
| // block until a send on showDesktopChan | |||
| <-showDesktopChan | |||
There was a problem hiding this comment.
I was torn between this route and just passing a flag when we spin up desktop. We are able to show the desktop on demand, but as far as I can tell, there is no way to hide the desktop on demand. When you tell systray to shutdown, it exits the program completely. So if we ever want to hide it, we have to kill the process and let a new one spin up hidden.
I chose this route because I think it's rare that we would turn off the desktop and I don't want to be starting / killing processes on a first install where a user is trying auth for the first time.
There was a problem hiding this comment.
I think it's rare that we would turn off the desktop
I agree, I think that's a reasonable assumption -- this route makes sense to me
There was a problem hiding this comment.
I think users will, but I think your reasoning is sound.
| "unsafe" | ||
| ) | ||
|
|
||
| func Detect(reason string) (bool, error) { |
There was a problem hiding this comment.
For Windows, I think we're going to want something like
| func Detect(reason string) (bool, error) { | |
| func Detect(credentialName string, reason string) (bool, string, error) { |
where the string returned is the new credential name, if the credential didn't exist yet and had to be provisioned. I think we'd also want knapsack/store access in this package, so that we can store the credential name.
I can add both of those changes in a follow-up PR, but wanted to mention it in case it affected anything about what you're doing in this PR!
James-Pickett
changed the title
| @@ -182,9 +184,10 @@ func runDesktop(_ *multislogger.MultiSlogger, args []string) error { | |||
| } | |||
| }() | |||
|
|
|||
| // block until a send on showDesktopChan | |||
| <-showDesktopChan | |||
There was a problem hiding this comment.
I think users will, but I think your reasoning is sound.
| const DetectionFailedDurationValue = -1 * time.Second | ||
|
|
||
| type PresenceDetector struct { | ||
| lastDetectionUTC time.Time |
There was a problem hiding this comment.
if this is time.Time maybe drop the UTC?
There was a problem hiding this comment.
will drop it, but we still want UTC instead of local time I'm assuming
| @@ -334,6 +344,14 @@ func (e *kryptoEcMiddleware) Wrap(next http.Handler) http.Handler { | |||
|
|
|||
| w.Header().Add(kolideKryptoHeaderKey, kolideKryptoEccHeader20230130Value) | |||
|
|
|||
| for k, v := range bhr.Header() { | |||
There was a problem hiding this comment.
These headers are outside the krypto box, is that okay?
There was a problem hiding this comment.
It would be better if they were signed, but also not fatal. I thought if we put them into the CmdReq they'd be inside?
There was a problem hiding this comment.
The headers of the request are inside the CmdReq (in krypto box). We don't have a mechanism currently to add headers from the response to the krypto box. Currently we just grab the body from the response and put it in the krypto box. We could come up with some kind of base response that we imbed in each of the local server responses.
There was a problem hiding this comment.
I'm confused. We have too many layers
I think it's important for the response from launcher to k2, to be in the box. Otherwise it's a bit too trivial to slap whatever you want in flight.
There was a problem hiding this comment.
But I'm not sure I'm concerned about what's happening between localserver and the desktop.
| @@ -334,6 +344,14 @@ func (e *kryptoEcMiddleware) Wrap(next http.Handler) http.Handler { | |||
|
|
|||
| w.Header().Add(kolideKryptoHeaderKey, kolideKryptoEccHeader20230130Value) | |||
|
|
|||
| for k, v := range bhr.Header() { | |||
There was a problem hiding this comment.
Do we want to blindly copy all headers or cherry pick?
This PR adds a presence detection endpoint to the DesktopUserServer which will call the OS specific implementation of presence detection and return the result. The call to DesktopUserServer is made by middleware in the LocalServer. The LocalServer looks for values in cmdRequest (krypto) headers to determine when it needs to detect presence.
More docs here: https://github.com/kolide/monorepo/pull/240