-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(auth): add a default deviceName when remembering device #13022
Conversation
|
fad7499
to
7ffe114
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!!!!!
* 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:109.0) Gecko/20100101 Firefox/115.0' | ||
*/ | ||
export const getDeviceName = async (): Promise<string> => { | ||
const { userAgentData } = navigator as NavigatorUA; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
userAgentData is experimental and hence isn't typed but is officially recommended by google.
The support for userAgentData
is still limited and so we have a fall back to option to use userAgent
Since
userAgentData
is experimental can we just useuserAgent
across all browsers ?No, using only
userAgent
would result in chrome triggeringAudit usage of navigator.userAgent, navigator.appVersion, and navigator.platform
and reopening an old issue.
const attributes = DeviceAttributes.reduce( | ||
(attrs: any, { Name, Value }) => { | ||
if (Name && Value) { | ||
if (Name === 'device_name') deviceName = Value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it needed to assert for this specific device name ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deviceName is returned by the service call in DeviceAttributes
on the device_name
attribute. We were doing the same in V5 as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not opposed to creating our own types, but I wonder if there's a pre-built @types
package we could use if it's well-maintained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a reference of the types from this package user-agent-data-types but since it was less widely used, added the types directly
Description of changes
navigator.userAgent
API causes a warning in Chrome DevTools (Issues tab) starting with version 92navigator.userAgentData
(when available) instead ofnavigator.userAgent
Note:
The
navigator.userAgentData
API involves introducing randomness by adding a stringNot_A_Brand | Not_A Brand | Not A(Brand | ...
to prevent user fingerprinting. The deviceName passed to theConfirmDevice
service API is called every time the user signs-in until therememberDevice
API is called, after which it just updates the status. It isn't a concern if the deviceName changes later on.Device name output / platform
Verified the following devices names on cognito console
Issue #, if available
#12854
Description of how you validated changes
Tested the following flows
Verified
fetchDevices
returnsChecklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.