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

Update two-factor #1214

Merged
merged 1 commit into from
Apr 23, 2019
Merged

Update two-factor #1214

merged 1 commit into from
Apr 23, 2019

Conversation

joshbetz
Copy link
Contributor

Allows two-factor to work with Jetpack SSO

WordPress/two-factor#276

Allows two-factor to work with Jetpack SSO

WordPress/two-factor#276
@mjangda mjangda mentioned this pull request Apr 15, 2019
1 task
@joshbetz joshbetz mentioned this pull request Apr 16, 2019
@nickdaugherty
Copy link
Contributor

Just noting for visibility that the latest version of the 2FA plugin enforces it on API requests (#)

@nickdaugherty
Copy link
Contributor

Correction - it adds the ability to enforce it on API requests - the filter used defaults to false.

@mjangda
Copy link
Member

mjangda commented Apr 23, 2019

Correction - it adds the ability to enforce it on API requests - the filter used defaults to false.

Good catch.

I think we probably should look at enforcing it. However, we'll want to double-check that it doesn't block Jetpack and other legitimate requests. Maybe, in a separate PR, worth adding some tracking to see how many and which requests this would block?

@joshbetz joshbetz merged commit 34173bb into master Apr 23, 2019
@joshbetz joshbetz deleted the update/two-factor branch April 23, 2019 14:28
@joshbetz
Copy link
Contributor Author

Correction - it adds the ability to enforce it on API requests - the filter used defaults to false.

Just noticed this is enabled by default. Note the name of the filter is two_factor_user_api_login_enable, which defaults to false. So, API login is disabled.

I will flip it back to true before deploying and we can follow up with the testing @mjangda mentioned.

@joshbetz
Copy link
Contributor Author

Noting this is not deployed. We need to merge #1222 first.

@joshbetz
Copy link
Contributor Author

r137564-deploy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants