feat(u2f): Enable renaming u2f device

src/sentry/api/endpoints/user_authenticator_details.py

@@ -13,6 +13,38 @@
 class UserAuthenticatorDetailsEndpoint(UserEndpoint):
     permission_classes = (OrganizationUserPermission,)
 
+    def _get_device_for_rename(self, authenticator, interface_device_id):
+        devices = authenticator.config
+        for device in devices["devices"]:
+            if device["binding"]["keyHandle"] == interface_device_id:
+                return device
+        return None
+
+    def _rename_device(self, authenticator, interface_device_id, new_name):
+        device = self._get_device_for_rename(authenticator, interface_device_id)
+        if not device:
+            return Response(status=status.HTTP_400_BAD_REQUEST)
+        device["name"] = new_name
+        authenticator.save()
+
+        return Response(status=status.HTTP_204_NO_CONTENT)
+
+    def _regenerate_recovery_code(self, authenticator, request, user):
+        interface = authenticator.interface
+
+        if interface.interface_id == "recovery":
+            interface.regenerate_codes()
+
+            capture_security_activity(
+                account=user,
+                type="recovery-codes-regenerated",
+                actor=request.user,
+                ip_address=request.META["REMOTE_ADDR"],
+                context={"authenticator": authenticator},
+                send_email=True,
+            )
+        return Response(serialize(interface))
+
     @sudo_required
     def get(self, request, user, auth_id):
         """
@@ -52,7 +84,7 @@ def get(self, request, user, auth_id):
         return Response(response)
 
     @sudo_required
-    def put(self, request, user, auth_id):
+    def put(self, request, user, auth_id, interface_device_id=None):
         """
         Modify authenticator interface
         ``````````````````````````````
@@ -64,26 +96,16 @@ def put(self, request, user, auth_id):
 
         :auth required:
         """
-
+        # TODO temporary solution for both renaming and regenerating recovery code. Need to find new home for regenerating recovery codes as it doesn't really do what put is supposed to do
         try:
             authenticator = Authenticator.objects.get(user=user, id=auth_id)
         except (ValueError, Authenticator.DoesNotExist):
             return Response(status=status.HTTP_404_NOT_FOUND)
 
-        interface = authenticator.interface
-
-        if interface.interface_id == "recovery":
-            interface.regenerate_codes()
-
-            capture_security_activity(
-                account=user,
-                type="recovery-codes-regenerated",
-                actor=request.user,
-                ip_address=request.META["REMOTE_ADDR"],
-                context={"authenticator": authenticator},
-                send_email=True,
-            )
-        return Response(serialize(interface))
+        if request.data.get("name"):
+            return self._rename_device(authenticator, interface_device_id, request.data.get("name"))
+        else:
+            return self._regenerate_recovery_code(authenticator, request, user)
 
     @sudo_required
     def delete(self, request, user, auth_id, interface_device_id=None):

static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx

@@ -72,6 +72,7 @@ class AccountSecurityDetails extends AsyncView<Props, State> {
     }
 
     // if the device is defined, it means that U2f is being removed
+    // reason for adding a trailing slash is a result of the endpoint on line 109 needing it but it can't be set there as if deviceId is None, the route will end with '//'
     const deviceId = device ? `${device.key_handle}/` : '';
     const deviceName = device ? device.name : t('Authenticator');
 
@@ -90,6 +91,34 @@ class AccountSecurityDetails extends AsyncView<Props, State> {
     }
   };
 
+  handleRename = async (device: AuthenticatorDevice, deviceName: string) => {
+    const {authenticator} = this.state;
+
+    if (!authenticator?.authId) {
+      return;
+    }
+    // if the device is defined, it means that U2f is being renamed
+    // reason for adding a trailing slash is a result of the endpoint on line 109 needing it but it can't be set there as if deviceId is None, the route will end with '//'
+    const deviceId = device ? `${device.key_handle}/` : '';
+
+    this.setState({loading: true});
+    const data = {
+      name: deviceName,
+    };
+
+    try {
+      await this.api.requestPromise(`${ENDPOINT}${authenticator.authId}/${deviceId}`, {
+        method: 'PUT',
+        data,
+      });
+      this.props.router.push(`/settings/account/security/mfa/${authenticator.authId}`);
+      addSuccessMessage(t('Device was renamed'));
+    } catch {
+      this.setState({loading: false});
+      addErrorMessage(t('Error renaming the device'));
+    }
+  };
+
   renderBody() {
     const {authenticator} = this.state;
 
@@ -143,6 +172,7 @@ class AccountSecurityDetails extends AsyncView<Props, State> {
           id={authenticator.id}
           devices={authenticator.devices}
           onRemoveU2fDevice={this.handleRemove}
+          onRenameU2fDevice={this.handleRename}
         />
 
         {authenticator.isEnrolled && authenticator.phone && (

static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx

@@ -1,45 +1,30 @@
-import {Fragment} from 'react';
+import React, {Fragment, useState} from 'react';
 import styled from '@emotion/styled';
 
 import Button from 'app/components/button';
 import Confirm from 'app/components/confirm';
 import DateTime from 'app/components/dateTime';
 import {Panel, PanelBody, PanelHeader, PanelItem} from 'app/components/panels';
 import Tooltip from 'app/components/tooltip';
-import {IconDelete} from 'app/icons';
+import {IconClose, IconDelete} from 'app/icons';
 import {t} from 'app/locale';
 import space from 'app/styles/space';
-import {AuthenticatorDevice} from 'app/types';
 import ConfirmHeader from 'app/views/settings/account/accountSecurity/components/confirmHeader';
 import EmptyMessage from 'app/views/settings/components/emptyMessage';
+import Input from 'app/views/settings/components/forms/controls/input';
 import TextBlock from 'app/views/settings/components/text/textBlock';
 
-type Props = {
-  isEnrolled: boolean;
-  id: string;
-  onRemoveU2fDevice: (device: AuthenticatorDevice) => void;
-  devices?: AuthenticatorDevice[];
-  className?: string;
-};
+const U2fEnrolledDetails = props => {
+  const {className, isEnrolled, devices, id, onRemoveU2fDevice, onRenameU2fDevice} =
+    props;
 
-/**
- * List u2f devices w/ ability to remove a single device
- */
-function U2fEnrolledDetails({
-  className,
-  isEnrolled,
-  devices,
-  id,
-  onRemoveU2fDevice,
-}: Props) {
   if (id !== 'u2f' || !isEnrolled) {
     return null;
   }
 
   const hasDevices = devices?.length;
   // Note Tooltip doesn't work because of bootstrap(?) pointer events for disabled buttons
   const isLastDevice = hasDevices === 1;
-
   return (
     <Panel className={className}>
       <PanelHeader>{t('Device name')}</PanelHeader>
@@ -49,41 +34,14 @@ function U2fEnrolledDetails({
           <EmptyMessage>{t('You have not added any U2F devices')}</EmptyMessage>
         )}
         {hasDevices &&
-          devices?.map(device => (
-            <DevicePanelItem key={device.name}>
-              <DeviceInformation>
-                <Name>{device.name}</Name>
-                <FadedDateTime date={device.timestamp} />
-              </DeviceInformation>
-
-              <Actions>
-                <Confirm
-                  onConfirm={() => onRemoveU2fDevice(device)}
-                  disabled={isLastDevice}
-                  message={
-                    <Fragment>
-                      <ConfirmHeader>
-                        {t('Do you want to remove U2F device?')}
-                      </ConfirmHeader>
-                      <TextBlock>
-                        {t(
-                          `Are you sure you want to remove the U2F device "${device.name}"?`
-                        )}
-                      </TextBlock>
-                    </Fragment>
-                  }
-                >
-                  <Button size="small" priority="danger">
-                    <Tooltip
-                      disabled={!isLastDevice}
-                      title={t('Can not remove last U2F device')}
-                    >
-                      <IconDelete size="xs" />
-                    </Tooltip>
-                  </Button>
-                </Confirm>
-              </Actions>
-            </DevicePanelItem>
+          devices?.map((device, i) => (
+            <Device
+              key={i}
+              device={device}
+              isLastDevice={isLastDevice}
+              onRenameU2fDevice={onRenameU2fDevice}
+              onRemoveU2fDevice={onRemoveU2fDevice}
+            />
           ))}
         <AddAnotherPanelItem>
           <Button
@@ -97,7 +55,97 @@ function U2fEnrolledDetails({
       </PanelBody>
     </Panel>
   );
-}
+};
+
+const Device = props => {
+  const {device, isLastDevice, onRenameU2fDevice, onRemoveU2fDevice} = props;
+  const [deviceName, setDeviceName] = useState(device.name);
+  const [isEditing, setEditting] = useState(false);
+
+  if (!isEditing) {
+    return (
+      <DevicePanelItem key={device.name}>
+        <DeviceInformation>
+          <Name>{device.name}</Name>
+          <FadedDateTime date={device.timestamp} />
+        </DeviceInformation>
+        <Actions>
+          <Button size="small" onClick={() => setEditting(true)}>
+            {t('Rename Device')}
+          </Button>
+        </Actions>
+        <Actions>
+          <Confirm
+            onConfirm={() => onRemoveU2fDevice(device)}
+            disabled={isLastDevice}
+            message={
+              <Fragment>
+                <ConfirmHeader>{t('Do you want to remove U2F device?')}</ConfirmHeader>
+                <TextBlock>
+                  {t(`Are you sure you want to remove the U2F device "${device.name}"?`)}
+                </TextBlock>
+              </Fragment>
+            }
+          >
+            <Button size="small" priority="danger">
+              <Tooltip
+                disabled={!isLastDevice}
+                title={t('Can not remove last U2F device')}
+              >
+                <IconDelete size="xs" />
+              </Tooltip>
+            </Button>
+          </Confirm>
+        </Actions>
+      </DevicePanelItem>
+    );
+  }
+
+  return (
+    <DevicePanelItem key={device.name}>
+      <DeviceInformation>
+        <DeviceNameInput
+          type="text"
+          value={deviceName}
+          onChange={e => {
+            setDeviceName(e.target.value);
+          }}
+        />
+        <FadedDateTime date={device.timestamp} />
+      </DeviceInformation>
+      <Actions>
+        <Button
+          priority="primary"
+          size="small"
+          onClick={() => {
+            onRenameU2fDevice(device, deviceName);
+            setEditting(false);
+          }}
+        >
+          Rename Device
+        </Button>
+      </Actions>
+
+      <Actions>
+        <Button
+          size="small"
+          title="Cancel rename"
+          onClick={() => {
+            setDeviceName(device.name);
+            setEditting(false);
+          }}
+        >
+          <IconClose size="xs" />
+        </Button>
+      </Actions>
+    </DevicePanelItem>
+  );
+};
+
+const DeviceNameInput = styled(Input)`
+  width: 50%;
+  margin-right: ${space(2)};
+`;
 
 const DevicePanelItem = styled(PanelItem)`
   padding: 0;
@@ -106,7 +154,9 @@ const DevicePanelItem = styled(PanelItem)`
 const DeviceInformation = styled('div')`
   display: flex;
   align-items: center;
-  flex: 1;
+  justify-content: space-between;
+  flex: 1 1;
+  height: 72px;
 
   padding: ${space(2)};
   padding-right: 0;

tests/sentry/api/endpoints/test_user_authenticator_details.py

@@ -87,6 +87,19 @@ def test_require_2fa__delete_device__ok(self):
         self._require_2fa_for_organization()
         self.test_u2f_remove_device()
 
+    def test_rename_device(self):
+        data = {"name": "for testing"}
+        auth = get_auth(self.user)
+        self.get_success_response(self.user.id, auth.id, "devicekeyhandle", **data, method="put")
+
+        authenticator = Authenticator.objects.get(id=auth.id)
+        assert authenticator.interface.get_device_name("devicekeyhandle") == "for testing"
+
+    def test_rename_device_not_found(self):
+        data = {"name": "for testing"}
+        auth = get_auth(self.user)
+        self.get_error_response(self.user.id, auth.id, "not_a_real_device", **data, method="put")
+
 
 class UserAuthenticatorDetailsTest(UserAuthenticatorDetailsTestBase):
     endpoint = "sentry-api-0-user-authenticator-details"