Clean up desktop access alerts (part 1) (#46201)

Rename the TDP notification message to "alert" to avoid confusing
them with the notifications introduced with Teleport 16.

Update the desktop session storybook to allow adding alerts of
different severity levels, which uncovered a bug where multiple
alerts stack horizontally instead of vertically (this has been fixed).

lib/srv/desktop/rdp/rdpclient/client.go

@@ -268,7 +268,7 @@ func (c *Client) readClientSize() error {
 				"screen size of %d x %d is greater than the maximum allowed by RDP (%d x %d)",
 				s.Width, s.Height, types.MaxRDPScreenWidth, types.MaxRDPScreenHeight,
 			)
-			if err := c.sendTDPNotification(err.Error(), tdp.SeverityError); err != nil {
+			if err := c.sendTDPAlert(err.Error(), tdp.SeverityError); err != nil {
 				return trace.Wrap(err)
 			}
 			return trace.Wrap(err)
@@ -278,8 +278,8 @@ func (c *Client) readClientSize() error {
 	}
 }
 
-func (c *Client) sendTDPNotification(message string, severity tdp.Severity) error {
-	return c.cfg.Conn.WriteMessage(tdp.Notification{Message: message, Severity: severity})
+func (c *Client) sendTDPAlert(message string, severity tdp.Severity) error {
+	return c.cfg.Conn.WriteMessage(tdp.Alert{Message: message, Severity: severity})
 }
 
 func (c *Client) startRustRDP(ctx context.Context) error {
@@ -350,7 +350,7 @@ func (c *Client) startRustRDP(ctx context.Context) error {
 		defer C.free_string(res.message)
 	}
 
-	// If the client exited with an error, send a tdp error notification and return it.
+	// If the client exited with an error, send a TDP notification and return it.
 	if res.err_code != C.ErrCodeSuccess {
 		var err error
 
@@ -360,7 +360,7 @@ func (c *Client) startRustRDP(ctx context.Context) error {
 			err = trace.Errorf("RDP client exited with an unknown error")
 		}
 
-		c.sendTDPNotification(err.Error(), tdp.SeverityError)
+		c.sendTDPAlert(err.Error(), tdp.SeverityError)
 		return err
 	}
 
@@ -371,7 +371,9 @@ func (c *Client) startRustRDP(ctx context.Context) error {
 	}
 
 	c.cfg.Logger.InfoContext(ctx, message)
-	c.sendTDPNotification(message, tdp.SeverityInfo)
+
+	// TODO(zmb3): convert this to severity error and ensure it renders in the UI
+	c.sendTDPAlert(message, tdp.SeverityInfo)
 
 	return nil
 }

lib/srv/desktop/tdp/conn.go

@@ -128,7 +128,7 @@ func (c *Conn) ReadClientScreenSpec() (*ClientScreenSpec, error) {
 
 // SendNotification is a convenience function for sending a Notification message.
 func (c *Conn) SendNotification(message string, severity Severity) error {
-	return c.WriteMessage(Notification{Message: message, Severity: severity})
+	return c.WriteMessage(Alert{Message: message, Severity: severity})
 }
 
 // LocalAddr returns local address

lib/srv/desktop/tdp/proto.go

@@ -75,7 +75,7 @@ const (
 	TypeSharedDirectoryListRequest      = MessageType(25)
 	TypeSharedDirectoryListResponse     = MessageType(26)
 	TypePNG2Frame                       = MessageType(27)
-	TypeNotification                    = MessageType(28)
+	TypeAlert                           = MessageType(28)
 	TypeRDPFastPathPDU                  = MessageType(29)
 	TypeRDPResponsePDU                  = MessageType(30)
 	TypeRDPConnectionInitialized        = MessageType(31)
@@ -142,8 +142,8 @@ func decodeMessage(firstByte byte, in byteReader) (Message, error) {
 		return decodeClipboardData(in, maxClipboardDataLength)
 	case TypeError:
 		return decodeError(in)
-	case TypeNotification:
-		return decodeNotification(in)
+	case TypeAlert:
+		return decodeAlert(in)
 	case TypeMFA:
 		return DecodeMFA(in)
 	case TypeSharedDirectoryAnnounce:
@@ -571,7 +571,7 @@ func (m Error) Encode() ([]byte, error) {
 }
 
 func decodeError(in io.Reader) (Error, error) {
-	message, err := decodeString(in, tdpMaxNotificationMessageLength)
+	message, err := decodeString(in, tdpMaxAlertMessageLength)
 	if err != nil {
 		return Error{}, trace.Wrap(err)
 	}
@@ -586,35 +586,36 @@ const (
 	SeverityError   Severity = 2
 )
 
-// Notification is an informational message sent from Teleport
+// Alert is an informational message sent from Teleport
 // to the Web UI. It can be used for fatal errors or non-fatal
 // warnings.
+//
 // | message type (28) | message_length uint32 | message []byte | severity byte |
-type Notification struct {
+type Alert struct {
 	Message  string
 	Severity Severity
 }
 
-func (m Notification) Encode() ([]byte, error) {
+func (m Alert) Encode() ([]byte, error) {
 	buf := new(bytes.Buffer)
-	buf.WriteByte(byte(TypeNotification))
+	buf.WriteByte(byte(TypeAlert))
 	if err := encodeString(buf, m.Message); err != nil {
 		return nil, trace.Wrap(err)
 	}
 	buf.WriteByte(byte(m.Severity))
 	return buf.Bytes(), nil
 }
 
-func decodeNotification(in byteReader) (Notification, error) {
-	message, err := decodeString(in, tdpMaxNotificationMessageLength)
+func decodeAlert(in byteReader) (Alert, error) {
+	message, err := decodeString(in, tdpMaxAlertMessageLength)
 	if err != nil {
-		return Notification{}, trace.Wrap(err)
+		return Alert{}, trace.Wrap(err)
 	}
 	severity, err := in.ReadByte()
 	if err != nil {
-		return Notification{}, trace.Wrap(err)
+		return Alert{}, trace.Wrap(err)
 	}
-	return Notification{Message: message, Severity: Severity(severity)}, nil
+	return Alert{Message: message, Severity: Severity(severity)}, nil
 }
 
 // MouseWheelAxis identifies a scroll axis on the mouse wheel.
@@ -1688,9 +1689,9 @@ func writeUint64(b *bytes.Buffer, v uint64) {
 }
 
 const (
-	// tdpMaxNotificationMessageLength is somewhat arbitrary, as it is only sent *to*
+	// tdpMaxAlertMessageLength is somewhat arbitrary, as it is only sent *to*
 	// the browser (Teleport never receives this message, so won't be decoding it)
-	tdpMaxNotificationMessageLength = 10240
+	tdpMaxAlertMessageLength = 10240
 
 	// tdpMaxPathLength is somewhat arbitrary because we weren't able to determine
 	// a precise value to set it to: https://github.com/gravitational/teleport/issues/14950#issuecomment-1341632465

lib/srv/desktop/tdp/proto_test.go

@@ -223,14 +223,14 @@ func TestSizeLimitsAreNonFatal(t *testing.T) {
 		{
 			name: "rejects long Error",
 			msg: Error{
-				Message: string(bytes.Repeat([]byte("a"), tdpMaxNotificationMessageLength+1)),
+				Message: string(bytes.Repeat([]byte("a"), tdpMaxAlertMessageLength+1)),
 			},
 			fatal: false,
 		},
 		{
-			name: "rejects long Notification",
-			msg: Notification{
-				Message: string(bytes.Repeat([]byte("a"), tdpMaxNotificationMessageLength+1)),
+			name: "rejects long Alert",
+			msg: Alert{
+				Message: string(bytes.Repeat([]byte("a"), tdpMaxAlertMessageLength+1)),
 			},
 			fatal: false,
 		},

lib/srv/desktop/windows_server.go

@@ -981,7 +981,7 @@ func (s *WindowsService) makeTDPSendHandler(
 	return func(m tdp.Message, b []byte) {
 		switch b[0] {
 		case byte(tdp.TypeRDPConnectionInitialized), byte(tdp.TypeRDPFastPathPDU), byte(tdp.TypePNG2Frame),
-			byte(tdp.TypePNGFrame), byte(tdp.TypeError), byte(tdp.TypeNotification):
+			byte(tdp.TypePNGFrame), byte(tdp.TypeError), byte(tdp.TypeAlert):
 			e := &events.DesktopRecording{
 				Metadata: events.Metadata{
 					Type: libevents.DesktopRecordingEvent,

lib/web/apiserver_test.go

@@ -2290,7 +2290,7 @@ func (w *windowsDesktopServiceMock) handleConn(t *testing.T, conn *tls.Conn) {
 	require.NoError(t, err)
 	require.IsType(t, tdp.ClientScreenSpec{}, msg)
 
-	err = tdpConn.WriteMessage(tdp.Notification{Message: "test", Severity: tdp.SeverityWarning})
+	err = tdpConn.WriteMessage(tdp.Alert{Message: "test", Severity: tdp.SeverityWarning})
 	require.NoError(t, err)
 }
 
@@ -2364,7 +2364,7 @@ func TestDesktopAccessMFARequiresMfa(t *testing.T) {
 
 			msg, err := tdpClient.ReadMessage()
 			require.NoError(t, err)
-			require.IsType(t, tdp.Notification{}, msg)
+			require.IsType(t, tdp.Alert{}, msg)
 		})
 	}
 }

lib/web/desktop.go

@@ -99,7 +99,7 @@ func (h *Handler) createDesktopConnection(
 	ctx := r.Context()
 
 	sendTDPError := func(err error) error {
-		sendErr := sendTDPNotification(ws, err, tdp.SeverityError)
+		sendErr := sendTDPAlert(ws, err, tdp.SeverityError)
 		if sendErr != nil {
 			return sendErr
 		}
@@ -560,7 +560,7 @@ func proxyWebsocketConn(ws *websocket.Conn, wds net.Conn) error {
 				if !isFatal {
 					severity = tdp.SeverityWarning
 				}
-				sendErr := sendTDPNotification(ws, err, severity)
+				sendErr := sendTDPAlert(ws, err, severity)
 
 				// If the error wasn't fatal and we successfully
 				// sent it back to the client, continue.
@@ -753,10 +753,10 @@ func (h *Handler) desktopAccessScriptInstallADCSHandle(w http.ResponseWriter, r
 	return nil, trace.Wrap(err)
 }
 
-// sendTDPNotification sends a tdp Notification over the supplied websocket with the
+// sendTDPAlert sends a TDP alert over the supplied websocket with the
 // error message of err.
-func sendTDPNotification(ws *websocket.Conn, err error, severity tdp.Severity) error {
-	msg := tdp.Notification{Message: err.Error(), Severity: severity}
+func sendTDPAlert(ws *websocket.Conn, err error, severity tdp.Severity) error {
+	msg := tdp.Alert{Message: err.Error(), Severity: severity}
 	b, err := msg.Encode()
 	if err != nil {
 		return trace.Wrap(err)

rfd/0037-desktop-access-protocol.md

@@ -222,13 +222,13 @@ This message contains a mouse wheel update. Sent from client to server.
 
 This message indicates an error has occurred.
 
-#### 28 - notification
+#### 28 - alert
 
 ```
 | message type (28) | message_length uint32 | message []byte | severity byte |
 ```
 
-This message sends a notification message with a severity level. Sent from server to client.
+This message sends an alert message along with a severity level. Sent from server to client.
 
 `message_length` denotes the length of the `message` byte array. It doesn't include the `severity` byte.
 
@@ -239,7 +239,7 @@ This message sends a notification message with a severity level. Sent from serve
 - `2` is for an error
 
 An error (`2`) means that some fatal problem was encountered and the TDP connection is ending imminently.
-A notification with `severity == 2` should be preferred to the `error` message above.
+An alert with `severity == 2` should be preferred to the `error` message above.
 
 A warning (`1`) means some non-fatal problem was encountered but the TDP connection can still continue.
 

web/packages/teleport/src/DesktopSession/WarningDropdown.tsx → web/packages/teleport/src/DesktopSession/AlertDropdown.tsx

@@ -25,19 +25,19 @@ import { useClickOutside } from 'shared/hooks/useClickOutside';
 
 import type { NotificationItem } from 'shared/components/Notification';
 
-export function WarningDropdown({ warnings, onRemoveWarning }: Props) {
+export function AlertDropdown({ alerts, onRemoveAlert }: Props) {
   const [showDropdown, setShowDropdown] = useState(false);
   const ref = useRef(null);
   const theme = useTheme();
 
   const toggleDropdown = () => {
-    if (warnings.length > 0) {
+    if (alerts.length > 0) {
       setShowDropdown(prevState => !prevState);
     }
   };
 
   // Dropdown is always closed if there are no errors to show
-  if (warnings.length === 0 && showDropdown) setShowDropdown(false);
+  if (alerts.length === 0 && showDropdown) setShowDropdown(false);
 
   // Close the dropdown if it's open and the user clicks outside of it
   useClickOutside(ref, () => {
@@ -51,45 +51,43 @@ export function WarningDropdown({ warnings, onRemoveWarning }: Props) {
   return (
     <>
       <StyledButton
-        title={'Warnings'}
-        hasWarnings={warnings.length > 0}
+        title={'Alerts'}
+        hasAlerts={alerts.length > 0}
         px={2}
         onClick={toggleDropdown}
       >
         <Flex
           alignItems="center"
           justifyContent="space-between"
           color={
-            warnings.length
-              ? theme.colors.text.main
-              : theme.colors.text.disabled
+            alerts.length ? theme.colors.text.main : theme.colors.text.disabled
           }
         >
-          <Warning size={20} mr={2} /> {warnings.length}
+          <Warning size={20} mr={2} /> {alerts.length}
         </Flex>
       </StyledButton>
       {showDropdown && (
         <StyledCard
           mt={3}
           p={2}
           style={{
-            maxHeight: window.innerHeight / 4,
+            maxHeight: window.innerHeight / 3,
           }}
         >
           <Flex alignItems="center" justifyContent="space-between">
             <H3 px={3} style={{ overflow: 'visible' }}>
-              {warnings.length} {warnings.length > 1 ? 'Warnings' : 'Warning'}
+              {alerts.length} {alerts.length > 1 ? 'Alerts' : 'Alert'}
             </H3>
             <ButtonIcon size={1} ml={1} mr={2} onClick={toggleDropdown}>
               <Cross size="medium" />
             </ButtonIcon>
           </Flex>
-          <StyledOverflow flexWrap="wrap" gap={2}>
-            {warnings.map(warning => (
+          <StyledOverflow flexDirection="column" gap={2}>
+            {alerts.map(alert => (
               <StyledNotification
-                key={warning.id}
-                item={warning}
-                onRemove={() => onRemoveWarning(warning.id)}
+                key={alert.id}
+                item={alert}
+                onRemove={() => onRemoveAlert(alert.id)}
                 Icon={Warning}
                 getColor={theme => theme.colors.warning.main}
                 isAutoRemovable={false}
@@ -107,13 +105,13 @@ const StyledButton = styled(Button)`
   min-height: 0;
   height: ${({ theme }) => theme.fontSizes[7] + 'px'};
   background-color: ${props =>
-    props.hasWarnings
+    props.hasAlerts
       ? props.theme.colors.warning.main
       : props.theme.colors.spotBackground[1]};
   &:hover,
   &:focus {
     background-color: ${props =>
-      props.hasWarnings
+      props.hasAlerts
         ? props.theme.colors.warning.hover
         : props.theme.colors.spotBackground[2]};
   }
@@ -141,6 +139,6 @@ const StyledOverflow = styled(Flex)`
 `;
 
 type Props = {
-  warnings: NotificationItem[];
-  onRemoveWarning: (id: string) => void;
+  alerts: NotificationItem[];
+  onRemoveAlert: (id: string) => void;
 };