From 06f7af6fe588c8dbc542f3c50e47d1c6f47bdf14 Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Mon, 11 Oct 2021 15:11:08 -0700 Subject: [PATCH 01/11] enable renaming yubikey --- .../endpoints/user_authenticator_details.py | 42 +++-- .../accountSecurityDetails.tsx | 29 +++ .../components/u2fEnrolledDetails.tsx | 166 ++++++++++-------- 3 files changed, 149 insertions(+), 88 deletions(-) diff --git a/src/sentry/api/endpoints/user_authenticator_details.py b/src/sentry/api/endpoints/user_authenticator_details.py index 09ac7458b41514..dabe2197e7ce22 100644 --- a/src/sentry/api/endpoints/user_authenticator_details.py +++ b/src/sentry/api/endpoints/user_authenticator_details.py @@ -51,7 +51,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 `````````````````````````````` @@ -63,26 +63,36 @@ def put(self, request, user, auth_id): :auth required: """ - + # temporary solution for both renaming and regenerating recovery code 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"): + devices = authenticator.config + for device in devices["devices"]: + if device["binding"]["keyHandle"] == interface_device_id: + device["name"] = request.data.get("name") + authenticator.save() + + return Response(status=status.HTTP_201_CREATED) + + else: + 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 delete(self, request, user, auth_id, interface_device_id=None): diff --git a/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx b/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx index 9dfd0d7ee31d19..726a02ab704a36 100644 --- a/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx +++ b/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx @@ -90,6 +90,34 @@ class AccountSecurityDetails extends AsyncView { } }; + handleRename = async (device: AuthenticatorDevice, deviceName: string) => { + const {authenticator} = this.state; + + if (!authenticator || !authenticator.authId) { + return; + } + // if the device is defined, it means that U2f is being removed + 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('%s has been renamed', deviceName)); + } catch { + // Error deleting authenticator + this.setState({loading: false}); + addErrorMessage(t('Error Renaming to %s', deviceName)); + } + }; + renderBody() { const {authenticator} = this.state; @@ -143,6 +171,7 @@ class AccountSecurityDetails extends AsyncView { id={authenticator.id} devices={authenticator.devices} onRemoveU2fDevice={this.handleRemove} + onRenameU2fDevice={this.handleRename} /> {authenticator.isEnrolled && authenticator.phone && ( diff --git a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx index 215563ddb5735a..1b35f0f9ddb8f2 100644 --- a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx +++ b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx @@ -1,4 +1,4 @@ -import {Fragment} from 'react'; +import React, {Fragment} from 'react'; import styled from '@emotion/styled'; import Button from 'app/components/button'; @@ -12,91 +12,113 @@ 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; + onRenameU2fDevice: (device: AuthenticatorDevice, deviceName: string) => void; devices?: AuthenticatorDevice[]; className?: string; }; +type State = Record; + /** * List u2f devices w/ ability to remove a single device */ -function U2fEnrolledDetails({ - className, - isEnrolled, - devices, - id, - onRemoveU2fDevice, -}: Props) { - if (id !== 'u2f' || !isEnrolled) { - return null; - } +class U2fEnrolledDetails extends React.Component { + render() { + const {className, isEnrolled, devices, id, onRemoveU2fDevice, onRenameU2fDevice} = + this.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 ( + + {t('Device name')} - const hasDevices = devices?.length; - // Note Tooltip doesn't work because of bootstrap(?) pointer events for disabled buttons - const isLastDevice = hasDevices === 1; - - return ( - - {t('Device name')} - - - {!hasDevices && ( - {t('You have not added any U2F devices')} - )} - {hasDevices && - devices?.map(device => ( - - - {device.name} - - - - - onRemoveU2fDevice(device)} - disabled={isLastDevice} - message={ - - - {t('Do you want to remove U2F device?')} - - - {t( - `Are you sure you want to remove the U2F device "${device.name}"?` - )} - - - } - > - - - - - ))} - - - - - - ); + + {!hasDevices && ( + {t('You have not added any U2F devices')} + )} + {hasDevices && + devices?.map(device => ( + + + {device.name} + + + + + { + const deviceName = this.state[device.name]; + onRenameU2fDevice(device, deviceName); + }} + message={ + + this.setState({[device.name]: e.target.value})} + /> + + } + > + + + + + + onRemoveU2fDevice(device)} + disabled={isLastDevice} + message={ + + + {t('Do you want to remove U2F device?')} + + + {t( + `Are you sure you want to remove the U2F device "${device.name}"?` + )} + + + } + > + + + + + ))} + + + + + + ); + } } const DevicePanelItem = styled(PanelItem)` From 37d4863ad19c59ffaa7f99a2eb556752585566a1 Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Mon, 11 Oct 2021 15:55:45 -0700 Subject: [PATCH 02/11] changed msgs --- .../account/accountSecurity/accountSecurityDetails.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx b/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx index 726a02ab704a36..4f58b241b9cf22 100644 --- a/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx +++ b/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx @@ -110,11 +110,11 @@ class AccountSecurityDetails extends AsyncView { data, }); this.props.router.push(`/settings/account/security/mfa/${authenticator.authId}`); - addSuccessMessage(t('%s has been renamed', deviceName)); + addSuccessMessage(t('Device was renamed')); } catch { // Error deleting authenticator this.setState({loading: false}); - addErrorMessage(t('Error Renaming to %s', deviceName)); + addErrorMessage(t('Error renaming the device')); } }; From d5074e6a42db5d0c31ee8389d91b291668bcab85 Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Tue, 12 Oct 2021 10:54:49 -0700 Subject: [PATCH 03/11] removed comment and changed to using null traversal operator --- .../account/accountSecurity/accountSecurityDetails.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx b/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx index 4f58b241b9cf22..6bff321970bc7c 100644 --- a/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx +++ b/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx @@ -93,10 +93,10 @@ class AccountSecurityDetails extends AsyncView { handleRename = async (device: AuthenticatorDevice, deviceName: string) => { const {authenticator} = this.state; - if (!authenticator || !authenticator.authId) { + if (!authenticator?.authId) { return; } - // if the device is defined, it means that U2f is being removed + // if the device is defined, it means that U2f is being renamed const deviceId = device ? `${device.key_handle}/` : ''; this.setState({loading: true}); @@ -112,7 +112,6 @@ class AccountSecurityDetails extends AsyncView { this.props.router.push(`/settings/account/security/mfa/${authenticator.authId}`); addSuccessMessage(t('Device was renamed')); } catch { - // Error deleting authenticator this.setState({loading: false}); addErrorMessage(t('Error renaming the device')); } From df8e69798aa72c64f98811a09a1fe2658ed5ce91 Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Thu, 14 Oct 2021 15:21:39 -0700 Subject: [PATCH 04/11] test for renaming --- .../api/endpoints/test_user_authenticator_details.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/sentry/api/endpoints/test_user_authenticator_details.py b/tests/sentry/api/endpoints/test_user_authenticator_details.py index 2c3874a4d08195..66843e97bc075b 100644 --- a/tests/sentry/api/endpoints/test_user_authenticator_details.py +++ b/tests/sentry/api/endpoints/test_user_authenticator_details.py @@ -87,6 +87,14 @@ 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" + class UserAuthenticatorDetailsTest(UserAuthenticatorDetailsTestBase): endpoint = "sentry-api-0-user-authenticator-details" From f4cbf8c855b870399cb87fbd814271d3aa1f596d Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Fri, 15 Oct 2021 11:22:02 -0700 Subject: [PATCH 05/11] rename modal title --- .../account/accountSecurity/components/u2fEnrolledDetails.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx index 1b35f0f9ddb8f2..cc18fdb0bfc211 100644 --- a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx +++ b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx @@ -65,6 +65,7 @@ class U2fEnrolledDetails extends React.Component { }} message={ + {t('Rename this U2F device')} Date: Mon, 18 Oct 2021 11:06:42 -0700 Subject: [PATCH 06/11] added todo for regen recover codes --- src/sentry/api/endpoints/user_authenticator_details.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/api/endpoints/user_authenticator_details.py b/src/sentry/api/endpoints/user_authenticator_details.py index dabe2197e7ce22..554f2becfb9159 100644 --- a/src/sentry/api/endpoints/user_authenticator_details.py +++ b/src/sentry/api/endpoints/user_authenticator_details.py @@ -63,7 +63,7 @@ def put(self, request, user, auth_id, interface_device_id=None): :auth required: """ - # temporary solution for both renaming and regenerating recovery code + # 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): From f5bab908e4b7a877799a6b7520dc54d29f9fb71d Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Mon, 18 Oct 2021 13:35:45 -0700 Subject: [PATCH 07/11] return 404 if device does not exist in registered devices, and added test coverage --- .../api/endpoints/user_authenticator_details.py | 17 ++++++++++++----- .../test_user_authenticator_details.py | 5 +++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/sentry/api/endpoints/user_authenticator_details.py b/src/sentry/api/endpoints/user_authenticator_details.py index 554f2becfb9159..19c83135992289 100644 --- a/src/sentry/api/endpoints/user_authenticator_details.py +++ b/src/sentry/api/endpoints/user_authenticator_details.py @@ -12,6 +12,13 @@ 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 + @sudo_required def get(self, request, user, auth_id): """ @@ -70,11 +77,11 @@ def put(self, request, user, auth_id, interface_device_id=None): return Response(status=status.HTTP_404_NOT_FOUND) if request.data.get("name"): - devices = authenticator.config - for device in devices["devices"]: - if device["binding"]["keyHandle"] == interface_device_id: - device["name"] = request.data.get("name") - authenticator.save() + device = self._get_device_for_rename(authenticator, interface_device_id) + if not device: + return Response(status=status.HTTP_404_NOT_FOUND) + device["name"] = request.data.get("name") + authenticator.save() return Response(status=status.HTTP_201_CREATED) diff --git a/tests/sentry/api/endpoints/test_user_authenticator_details.py b/tests/sentry/api/endpoints/test_user_authenticator_details.py index 66843e97bc075b..6dea7703c507e6 100644 --- a/tests/sentry/api/endpoints/test_user_authenticator_details.py +++ b/tests/sentry/api/endpoints/test_user_authenticator_details.py @@ -95,6 +95,11 @@ def test_rename_device(self): 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" From 63f23b3f29e25cc17d484e660001fe8ba4883a8e Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Mon, 18 Oct 2021 15:03:26 -0700 Subject: [PATCH 08/11] moved rename and regen recovery codes to their own helper methods and changed response codes --- .../endpoints/user_authenticator_details.py | 49 ++++++++++--------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/src/sentry/api/endpoints/user_authenticator_details.py b/src/sentry/api/endpoints/user_authenticator_details.py index 19c83135992289..37dc255409d264 100644 --- a/src/sentry/api/endpoints/user_authenticator_details.py +++ b/src/sentry/api/endpoints/user_authenticator_details.py @@ -19,6 +19,31 @@ def _get_device_for_rename(self, authenticator, 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): """ @@ -77,29 +102,9 @@ def put(self, request, user, auth_id, interface_device_id=None): return Response(status=status.HTTP_404_NOT_FOUND) if request.data.get("name"): - device = self._get_device_for_rename(authenticator, interface_device_id) - if not device: - return Response(status=status.HTTP_404_NOT_FOUND) - device["name"] = request.data.get("name") - authenticator.save() - - return Response(status=status.HTTP_201_CREATED) - + return self._rename_device(authenticator, interface_device_id, request.data.get("name")) else: - 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)) + return self._regenerate_recovery_code(authenticator, request, user) @sudo_required def delete(self, request, user, auth_id, interface_device_id=None): From ae87616b52ec78a33173177c6d6e066482b9458d Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Fri, 22 Oct 2021 14:39:51 -0700 Subject: [PATCH 09/11] changed class to use react hooks --- .../components/u2fEnrolledDetails.tsx | 229 +++++++++++------- 1 file changed, 136 insertions(+), 93 deletions(-) diff --git a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx index cc18fdb0bfc211..d806c6b3f99c09 100644 --- a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx +++ b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx @@ -1,4 +1,4 @@ -import React, {Fragment} from 'react'; +import React, {Fragment, useState} from 'react'; import styled from '@emotion/styled'; import Button from 'app/components/button'; @@ -6,7 +6,7 @@ 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'; @@ -15,6 +15,7 @@ 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'; +// eslint-disable-next-line @typescript-eslint/no-unused-vars type Props = { isEnrolled: boolean; id: string; @@ -24,103 +25,143 @@ type Props = { className?: string; }; -type State = Record; +const U2fEnrolledDetails = props => { + const {className, isEnrolled, devices, id, onRemoveU2fDevice, onRenameU2fDevice} = + props; -/** - * List u2f devices w/ ability to remove a single device - */ -class U2fEnrolledDetails extends React.Component { - render() { - const {className, isEnrolled, devices, id, onRemoveU2fDevice, onRenameU2fDevice} = - this.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 ( + + {t('Device name')} + + + {!hasDevices && ( + {t('You have not added any U2F devices')} + )} + {hasDevices && + devices?.map((device, i) => ( + + ))} + + + + + + ); +}; - if (id !== 'u2f' || !isEnrolled) { - return null; - } +const Device = props => { + const {onRenameU2fDevice, onRemoveU2fDevice, device, isLastDevice} = props; + const [deviceName, setDeviceName] = useState(device.name); + const [isEditting, setEditting] = useState(false); - const hasDevices = devices?.length; - // Note Tooltip doesn't work because of bootstrap(?) pointer events for disabled buttons - const isLastDevice = hasDevices === 1; + if (!isEditting) { return ( - - {t('Device name')} - - - {!hasDevices && ( - {t('You have not added any U2F devices')} - )} - {hasDevices && - devices?.map(device => ( - - - {device.name} - - - - - { - const deviceName = this.state[device.name]; - onRenameU2fDevice(device, deviceName); - }} - message={ - - {t('Rename this U2F device')} - this.setState({[device.name]: e.target.value})} - /> - - } - > - - - - - - onRemoveU2fDevice(device)} - disabled={isLastDevice} - message={ - - - {t('Do you want to remove U2F device?')} - - - {t( - `Are you sure you want to remove the U2F device "${device.name}"?` - )} - - - } - > - - - - - ))} - - + + + onRemoveU2fDevice(device)} + disabled={isLastDevice} + message={ + + {t('Do you want to remove U2F device?')} + + {t(`Are you sure you want to remove the U2F device "${device.name}"?`)} + + + } + > + - - - + + + ); } -} + return ( + + + { + setDeviceName(e.target.value); + }} + /> + + + + + + + + + + + ); +}; + +const DeviceNameInput = styled(Input)` + width: 50%; + margin-right: ${space(2)}; +`; const DevicePanelItem = styled(PanelItem)` padding: 0; @@ -129,7 +170,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; From a6dc09224804b6c6a4b4ee78042ee0174e0a3293 Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Fri, 22 Oct 2021 15:38:13 -0700 Subject: [PATCH 10/11] removed unused component --- .../accountSecurity/components/u2fEnrolledDetails.tsx | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx index d806c6b3f99c09..18134c2826dd4e 100644 --- a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx +++ b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx @@ -9,22 +9,11 @@ import Tooltip from 'app/components/tooltip'; 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'; -// eslint-disable-next-line @typescript-eslint/no-unused-vars -type Props = { - isEnrolled: boolean; - id: string; - onRemoveU2fDevice: (device: AuthenticatorDevice) => void; - onRenameU2fDevice: (device: AuthenticatorDevice, deviceName: string) => void; - devices?: AuthenticatorDevice[]; - className?: string; -}; - const U2fEnrolledDetails = props => { const {className, isEnrolled, devices, id, onRemoveU2fDevice, onRenameU2fDevice} = props; From 2e38c0d550ac6bf88be26463b45bee22e52ba07a Mon Sep 17 00:00:00 2001 From: Richard Ma Date: Tue, 26 Oct 2021 15:58:34 -0700 Subject: [PATCH 11/11] changes based on pr reviews --- .../accountSecurityDetails.tsx | 2 ++ .../components/u2fEnrolledDetails.tsx | 21 +++++++------------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx b/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx index 6bff321970bc7c..eb176f628a1f73 100644 --- a/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx +++ b/static/app/views/settings/account/accountSecurity/accountSecurityDetails.tsx @@ -72,6 +72,7 @@ class AccountSecurityDetails extends AsyncView { } // 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'); @@ -97,6 +98,7 @@ class AccountSecurityDetails extends AsyncView { 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}); diff --git a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx index 18134c2826dd4e..0ebd42ec7cb044 100644 --- a/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx +++ b/static/app/views/settings/account/accountSecurity/components/u2fEnrolledDetails.tsx @@ -37,10 +37,10 @@ const U2fEnrolledDetails = props => { devices?.map((device, i) => ( ))} @@ -58,11 +58,11 @@ const U2fEnrolledDetails = props => { }; const Device = props => { - const {onRenameU2fDevice, onRemoveU2fDevice, device, isLastDevice} = props; + const {device, isLastDevice, onRenameU2fDevice, onRemoveU2fDevice} = props; const [deviceName, setDeviceName] = useState(device.name); - const [isEditting, setEditting] = useState(false); + const [isEditing, setEditting] = useState(false); - if (!isEditting) { + if (!isEditing) { return ( @@ -70,13 +70,8 @@ const Device = props => { - @@ -105,12 +100,12 @@ const Device = props => { ); } + return ( { setDeviceName(e.target.value);