-
Notifications
You must be signed in to change notification settings - Fork 353
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: email-based authentication codes for MFA/2FA #19008
Conversation
Signed-off-by: Morrten Svanaes <[email protected]>
# Conflicts: # dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/SystemSettings.java
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
...ice-core/src/main/java/org/hisp/dhis/security/spring2fa/TwoFactorAuthenticationProvider.java
Fixed
Show fixed
Hide fixed
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
# Conflicts: # dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/SystemSettings.java # dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/SystemUser.java # dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserDetails.java # dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserDetailsImpl.java # dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/oidc/DhisOidcUser.java # dhis-2/dhis-services/dhis-service-setting/src/test/java/org/hisp/dhis/setting/SystemSettingsTest.java
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
Signed-off-by: Morrten Svanaes <[email protected]>
dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java
Outdated
Show resolved
Hide resolved
dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java
Outdated
Show resolved
Hide resolved
...vices/dhis-service-core/src/main/java/org/hisp/dhis/security/twofa/TwoFactorAuthService.java
Show resolved
Hide resolved
dhis-2/dhis-services/dhis-service-core/src/main/resources/i18n_global.properties
Outdated
Show resolved
Hide resolved
dhis-2/dhis-services/dhis-service-core/src/main/resources/twofa_email_body_template_v1.vm
Outdated
Show resolved
Hide resolved
dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/AccountController.java
Show resolved
Hide resolved
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.
some small comments left, good job 👍
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.
I saw you moved more (internal) APIs from using a user's UID to the username which I think is a good thing. As we discussed it seems to me that the username is our identifier when it comes to questions of authentication. 👍
The are a handful or so details to correct and some suggestions about the REST API endpoints.
dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/CodeGenerator.java
Outdated
Show resolved
Hide resolved
dhis-2/dhis-api/src/main/java/org/hisp/dhis/security/twofa/TwoFactorType.java
Outdated
Show resolved
Hide resolved
...vices/dhis-service-core/src/main/java/org/hisp/dhis/security/twofa/TwoFactorAuthService.java
Show resolved
Hide resolved
...vices/dhis-service-core/src/main/java/org/hisp/dhis/security/twofa/TwoFactorAuthService.java
Outdated
Show resolved
Hide resolved
...2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/security/SessionController.java
Outdated
Show resolved
Hide resolved
...2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/security/SessionController.java
Show resolved
Hide resolved
...dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/security/TwoFactorController.java
Show resolved
Hide resolved
dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/UserController.java
Outdated
Show resolved
Hide resolved
dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/servlet/DhisWebApiWebAppInitializer.java
Show resolved
Hide resolved
Signed-off-by: Morten Svanaes <[email protected]>
Signed-off-by: Morten Svanaes <[email protected]>
Signed-off-by: Morten Svanaes <[email protected]>
Signed-off-by: Morten Svanaes <[email protected]>
dhis-2/dhis-api/src/main/java/org/hisp/dhis/security/twofa/TwoFactorType.java
Outdated
Show resolved
Hide resolved
...vices/dhis-service-core/src/main/java/org/hisp/dhis/security/twofa/TwoFactorAuthService.java
Show resolved
Hide resolved
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.
There a some comments left
- on visibility and TX-annotations in services
- on which HTTP method and response code to use for the new endpoints (session controller)
...vices/dhis-service-core/src/main/java/org/hisp/dhis/security/twofa/TwoFactorAuthService.java
Outdated
Show resolved
Hide resolved
...ation/src/main/resources/org/hisp/dhis/db/migration/2.42/V2_42_29__add_2FA_type_userinfo.sql
Outdated
Show resolved
Hide resolved
Signed-off-by: Morten Svanaes <[email protected]>
Signed-off-by: Morten Svanaes <[email protected]>
Quality Gate passedIssues Measures |
Summary
This adds 2FA login with a code sent via email as a new feature.
NOTE: This PR removes the enforced 2FA enrollment with TOTP functionality and does not address this for email 2FA either. The requirements for enforced enrollment needs to be addressed in a separate JIRA ticket. Enforced enrollment for TOTP was never used, so it's safe to remove.
TODO:
Automatic tests
Manual tests
Jira:
DHIS2-13334