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

FEATURE: impersonation now requires second factor authorization #12662

Closed
wants to merge 1 commit into from

Conversation

arpitjalan
Copy link
Member

Screenshot 2021-04-09 at 14 37 54

Screenshot 2021-04-09 at 14 38 04

@ZogStriP
Copy link
Member

ZogStriP commented Apr 9, 2021

Quick question: why is the @irish_kozey "mention" styled differently in the 1st modal?

second_factor_token:
this.securityKeyCredential || this.secondFactorToken,
second_factor_method: this.secondFactorMethod,
timezone: moment.tz.guess(),
Copy link
Member

Choose a reason for hiding this comment

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

Why is the timezone required?

}).then(
(result) => {
// Successful login
if (result && result.error) {
Copy link
Member

Choose a reason for hiding this comment

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

Is it a successful login if there's an error?

Comment on lines +76 to +79
document
.getElementById("second-factor")
.querySelector("input")
.focus()
Copy link
Member

Choose a reason for hiding this comment

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

You could shorten that a bit.

Suggested change
document
.getElementById("second-factor")
.querySelector("input")
.focus()
document.querySelector("#second-factor input").focus()

@SamSaffron
Copy link
Member

sorry @techapj I am closing this, when you free up you can spend some time with @martin-brennan doing the "generic" reusable 2fa screen. Then it should be a breeze plugging this in anywhere we need it.

@SamSaffron SamSaffron closed this May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants