Show device trust status in Connect #49508
Conversation
…TS definitions for it Generating TS definitions for the entire legacy/types/types.proto would result in 50K lines of generated code, and the bundle size would increase by ~20% (around 1 MB).
…ed both by the server and the client Important: it now uses `GetEnforcementMode` instead of `GetEffectiveMode`. The check for OSS module is performed on the server-side anyway.
|
Can we add a little bit of color? The colors and icon are the same in both cases, so you have to read the text to determine if your device is trusted or not. Making the shield green or yellow might make it easier to tell at a glance. |
Good idea, I will do it tomorrow. |
|
Not sure if you were able to use it or not, but we have a |
At first I wanted to use it, but then I realized that it would require some changes to make it work Connect (customizable size, getting rid of the container). I believe there's not much benefit to using the shared icon, and it's not worth the effort of making it shared. On top of that, in Connect I used more vibrant colors: This is how it looks now: 
|
| user types.User | ||
| roles []types.Role | ||
| ) | ||
|
|
||
| group, groupCtx := errgroup.WithContext(ctx) |
There was a problem hiding this comment.
At some point we might want to put some kind of limit here. I know that the code responsible for things like tsh ls -R sets that limit to 10 active goroutines:
teleport/tool/tsh/common/tsh.go
Lines 2469 to 2471 in 864eca5
Though in that case, each goroutine uses a separate connection and that's why it's limited (I assume). Here from what I can tell we have three separate clients making three separate connections and it's just 8 goroutines total. So we should be safe for now.
There was a problem hiding this comment.
+1 for setting a limit to the group.
There was a problem hiding this comment.
@codingllama But at this point what we should set it to?
There was a problem hiding this comment.
The number itself doesn't matter a whole lot, what is more important IMO is making sure we have an upper bound on the number of goroutines. I usually consider 8 to 10 fairly big numbers for this kind of work. Also note that you probably don't need to increase the number every time you add a new group.Go call - at limit=8 this should be pretty fast anyway.
For example:
group, groupCtx := errgroup.WithContext(ctx)
const groupLimit = 8 // Arbitrary. No need to increase for every new goroutine.
group.SetLimit(groupLimit)There was a problem hiding this comment.
(If this is really critical you can always measure for a better number.)
There was a problem hiding this comment.
I set it to 8 for now.
web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityList.tsx
Show resolved
Hide resolved
| top: 1.5px; | ||
| right: 1.5px; |
There was a problem hiding this comment.
I wanted to check if this is consistent with the other indicators in the top bar and… ggs we'll get them next time.
There was a problem hiding this comment.
I placed it to fit in the border of the Profiles button :/
But in general, we should replace this icon, it's too detailed for its size and transparent, making it difficult to place over other elements. Maybe I will find time for this when redesigning the profile switcher.
# Conflicts: # api/types/types.pb.go
* Check if device is trusted * Move `TrustedDeviceRequirement` enum to a separate file and generate TS definitions for it Generating TS definitions for the entire legacy/types/types.proto would result in 50K lines of generated code, and the bundle size would increase by ~20% (around 1 MB). * Extract function to calculate device trust requirement that can be used both by the server and the client Important: it now uses `GetEnforcementMode` instead of `GetEffectiveMode`. The check for OSS module is performed on the server-side anyway. * Check what is device trust requirement based on cluster config and user roles * Show device trust status in UI * Remove `types.` prefix * Improve godocs * Bring back `getRoles func() ([]types.Role, error)` * Simplify `TestHasDeviceTrustExtensions` * Clean up and improve `TestCalculateTrustedDeviceRequirement` * Move `CalculateTrustedDeviceRequirement` and tests to a separate file * Add colors to shield icons * Declare variables where they are used * Extract a component to display device trust status for better readability * Correctly reference `lib/client.ProfileStatus.Extensions` * Set group limit to 8 * Make `switch` exhaustive * Regenerate protos * `make fix-license` (cherry picked from commit 095b0a3)
* Check if device is trusted * Move `TrustedDeviceRequirement` enum to a separate file and generate TS definitions for it Generating TS definitions for the entire legacy/types/types.proto would result in 50K lines of generated code, and the bundle size would increase by ~20% (around 1 MB). * Extract function to calculate device trust requirement that can be used both by the server and the client Important: it now uses `GetEnforcementMode` instead of `GetEffectiveMode`. The check for OSS module is performed on the server-side anyway. * Check what is device trust requirement based on cluster config and user roles * Show device trust status in UI * Remove `types.` prefix * Improve godocs * Bring back `getRoles func() ([]types.Role, error)` * Simplify `TestHasDeviceTrustExtensions` * Clean up and improve `TestCalculateTrustedDeviceRequirement` * Move `CalculateTrustedDeviceRequirement` and tests to a separate file * Add colors to shield icons * Declare variables where they are used * Extract a component to display device trust status for better readability * Correctly reference `lib/client.ProfileStatus.Extensions` * Set group limit to 8 * Make `switch` exhaustive * Regenerate protos * `make fix-license` (cherry picked from commit 095b0a3)
* Check if device is trusted * Move `TrustedDeviceRequirement` enum to a separate file and generate TS definitions for it Generating TS definitions for the entire legacy/types/types.proto would result in 50K lines of generated code, and the bundle size would increase by ~20% (around 1 MB). * Extract function to calculate device trust requirement that can be used both by the server and the client Important: it now uses `GetEnforcementMode` instead of `GetEffectiveMode`. The check for OSS module is performed on the server-side anyway. * Check what is device trust requirement based on cluster config and user roles * Show device trust status in UI * Remove `types.` prefix * Improve godocs * Bring back `getRoles func() ([]types.Role, error)` * Simplify `TestHasDeviceTrustExtensions` * Clean up and improve `TestCalculateTrustedDeviceRequirement` * Move `CalculateTrustedDeviceRequirement` and tests to a separate file * Add colors to shield icons * Declare variables where they are used * Extract a component to display device trust status for better readability * Correctly reference `lib/client.ProfileStatus.Extensions` * Set group limit to 8 * Make `switch` exhaustive * Regenerate protos * `make fix-license` (cherry picked from commit 095b0a3)
* Show device trust status in Connect (#49508) * Check if device is trusted * Move `TrustedDeviceRequirement` enum to a separate file and generate TS definitions for it Generating TS definitions for the entire legacy/types/types.proto would result in 50K lines of generated code, and the bundle size would increase by ~20% (around 1 MB). * Extract function to calculate device trust requirement that can be used both by the server and the client Important: it now uses `GetEnforcementMode` instead of `GetEffectiveMode`. The check for OSS module is performed on the server-side anyway. * Check what is device trust requirement based on cluster config and user roles * Show device trust status in UI * Remove `types.` prefix * Improve godocs * Bring back `getRoles func() ([]types.Role, error)` * Simplify `TestHasDeviceTrustExtensions` * Clean up and improve `TestCalculateTrustedDeviceRequirement` * Move `CalculateTrustedDeviceRequirement` and tests to a separate file * Add colors to shield icons * Declare variables where they are used * Extract a component to display device trust status for better readability * Correctly reference `lib/client.ProfileStatus.Extensions` * Set group limit to 8 * Make `switch` exhaustive * Regenerate protos * `make fix-license` (cherry picked from commit 095b0a3) * Remove unsupported property from stories
* Check if device is trusted * Move `TrustedDeviceRequirement` enum to a separate file and generate TS definitions for it Generating TS definitions for the entire legacy/types/types.proto would result in 50K lines of generated code, and the bundle size would increase by ~20% (around 1 MB). * Extract function to calculate device trust requirement that can be used both by the server and the client Important: it now uses `GetEnforcementMode` instead of `GetEffectiveMode`. The check for OSS module is performed on the server-side anyway. * Check what is device trust requirement based on cluster config and user roles * Show device trust status in UI * Remove `types.` prefix * Improve godocs * Bring back `getRoles func() ([]types.Role, error)` * Simplify `TestHasDeviceTrustExtensions` * Clean up and improve `TestCalculateTrustedDeviceRequirement` * Move `CalculateTrustedDeviceRequirement` and tests to a separate file * Add colors to shield icons * Declare variables where they are used * Extract a component to display device trust status for better readability * Correctly reference `lib/client.ProfileStatus.Extensions` * Set group limit to 8 * Make `switch` exhaustive * Regenerate protos * `make fix-license` (cherry picked from commit 095b0a3)
Closes #47249
The original issue is about indicating if the device is enrolled, which is simply done by checking if the profile extensions include all the necessary device trust extensions.
While working on this, I realized it's quite simple to add a warning when device trust is required but the device isn't enrolled, similar to the Web UI. To implement this, I had to:
This partially addresses #43020.
The messages shown in Connect are slightly different than in Web UI (for reference we say there
Session authorized with device trust.orSession in not authorized with Device Trust. Access is restricted.).My reasoning was that device enrollment is persistent, you don't need to authorize every session, as in Web UI. Because of that, I dropped the "session" term.
I'm curious what are your thoughts on this :)
A confirmation that device is enrolled:
A warning that access is restricted without enrolling the device:
Changelog: Teleport Connect now shows whether it is being used on a trusted device or if enrollment is required for full access