Skip to content
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(u2f): disabled enrolling the same device for u2f #29276

Merged
merged 6 commits into from
Oct 18, 2021
Merged

Conversation

maxiuyuan
Copy link
Contributor

@maxiuyuan maxiuyuan commented Oct 13, 2021

A user will no longer be able to enrol the same u2f device twice.

u2f.register() with registeredKeys passed in will error if the device is been registered already. If so, we want to display that it has been registered and not allow the user to continue with adding a u2f key.

Solves: https://getsentry.atlassian.net/browse/ER-640
https://getsentry.atlassian.net/browse/ER-642

@maxiuyuan maxiuyuan requested a review from a team as a code owner October 13, 2021 01:31
@maxiuyuan maxiuyuan marked this pull request as draft October 13, 2021 01:32
@github-actions
Copy link
Contributor

github-actions bot commented Oct 13, 2021

size-limit report

Path Base Size (ae7d0c4) Current Size Change
src/sentry/static/sentry/dist/entrypoints/app.js 52.78 KB 52.78 KB -0.01% 🔽
src/sentry/static/sentry/dist/entrypoints/sentry.css 70.9 KB 70.9 KB 0%

promise = u2f.sign(this.props.challengeData.authenticateRequests);
} else if (this.props.flowMode === 'enroll') {
const {registerRequests, registeredKeys} = this.props.challengeData;
promise = u2f.register(registerRequests as any, registeredKeys as any);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need the as any?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i believe so, i think it failed the linting checks without it since register doesn't take registeredKeys in the api function

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

Copy link
Member

@JoshFerge JoshFerge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@maxiuyuan maxiuyuan merged commit 4cded5e into master Oct 18, 2021
@maxiuyuan maxiuyuan deleted the ER-640 branch October 18, 2021 18:29
@maxiuyuan maxiuyuan linked an issue Oct 18, 2021 that may be closed by this pull request
@github-actions github-actions bot locked and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to identify an already enrolled U2F device.
2 participants