-
Notifications
You must be signed in to change notification settings - Fork 827
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
Remove: deprecated native MFA feature #2717
Conversation
We have created an issue in Pivotal Tracker to manage this: https://www.pivotaltracker.com/story/show/186996979 The labels on this github issue will be updated when the story is started. |
- the MFA feature has long been deprecated and will soon be removed in the next release, see: cloudfoundry/uaa#2717 - see breaking change planning: #739 [#186854489]
@peterhaochen47 I think you can remove |
- the MFA feature has long been deprecated and will soon be removed in the next release, see: cloudfoundry/uaa#2717 - see breaking change planning: #739 [#186854489]
efb7c59
to
9e43344
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.
ok for us, see sonar comment
...main/java/org/cloudfoundry/identity/uaa/audit/JdbcUnsuccessfulLoginCountingAuditService.java
Outdated
Show resolved
Hide resolved
cd6a805
to
60f3d1b
Compare
- Context about its deprecation: - This feature is under-utilized, and requires further maintenance for which our team lacks the resource. (For example, this feature is potentially vulnerable because a secure Content-Security-Policy cannot be applied to its pages without breaking them.) The feature has also been marked as "not ready for production" for a few years now. So we opt to remove the feature and instead recommend using the external IDPs's own MFA features. See more context in #2196. - This commit removes all MFA-specific codes, except for the following, on which we will make follow-up commits: - README's deprecation notice - database operations - Content-Security-Policy's exemption toward MFA endpoint (https://github.com/cloudfoundry/uaa/blob/72565fb56cd1f90af499119d32c891937f3c5a76/server/src/main/java/org/cloudfoundry/identity/uaa/security/web/ContentSecurityPolicyFilter.java#L29) - breaking changes planning: cloudfoundry/uaa-release#739 - Further notes about specific changes in tests: - For PasscodeMockMvcTests.testLoginUsingPasscodeWithUnknownToken(), the assertion on response code is changed from 401 to 403. This is because 403 was the original asserted value before MFA was added (see: 92abee6). The 403 response also makes sense in the context of the test (authentication present but has insufficient access). [#186854489]
- the MFA feature has been removed, hence adding DB migrations to drop the feature's related DB tables. - also clean up README and tests [#186854489]
- the MFA feature has been removed (aka the "/login/mfa/" endpoint) hence the code that creates an exemption for the MFA endpoint when enforcing "Content-Security-Policy" is no longer needed [#186854489]
- the MFA feature has been removed, so no longer need the deprecation notice [#186854489]
- the beans are for MFA feature, which has been removed [#186854489]
- use a more standard way to intialize a list, instead of using com.beust.jcommander.internal.Lists.newArrayList - motivation: this com.beust.jcommander.internal.Lists.newArrayList dep was added to our dependencies indirectly via another dependency I'm trying to remove [#186854489]
- these deps are for MFA feature (which has been removed) - also: before, the code was using org.apache.httpcomponents:httpclient but getting it indirectly via the MFA-related deps (which this commit aims to remove); hence, now need to directly declare org.apache.httpcomponents:httpclient as a dependency. [#186854489]
- by merging cases
60f3d1b
to
1181e06
Compare
[#186854489]