WIP: Support for multi-factor authentication through authenticator app #418 #441
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Release v1.7.1
Files are not complete, work still to be done on creating and displaying the angular components
Currently not displaying secret and QR code
Also adding Maven dependency for TOTP library
Also slight amendments to model logic for IamTotpMfa and IamTotpRecoveryCode
…iam/iam into iam-spring-update-oct-2021
The test client app has been modified to allow downscoping the authorization request to only include a subset of the configured scopes.
Files are not complete, work still to be done on creating and displaying the angular components
Currently not displaying secret and QR code
Also adding Maven dependency for TOTP library
Also slight amendments to model logic for IamTotpMfa and IamTotpRecoveryCode
Previously, a separate controller existed for enabling and disabling the authenticator app. Since these do similar things, they are being unified into one controller. Also creating a service for enabling and disabling auth app. This currently contains two functions (enabling and disabling) but both are extremely similar so will likely unify later. Added a TODO for this to create the foundations of step up authentication from this
…nning/iam into issue-418-mfa-support
…ommit of the following: commit dccc0b6 Merge: 0c674fb cc3b4d5 Author: Sam Glendenning <[email protected]> Date: Mon Nov 22 14:55:04 2021 +0000 Merge branch 'iam-spring-update-oct-2021' of git://github.com/indigo-iam/iam into iam-spring-update-oct-2021 commit cc3b4d5 Author: Andrea Ceccanti <[email protected]> Date: Mon Nov 15 08:33:55 2021 +0100 More fixes for SonarCloud warnings commit 5ff5e3b Author: Andrea Ceccanti <[email protected]> Date: Sun Nov 14 16:38:07 2021 +0100 Fixes for Sonar warnings/errors and other minor improvements commit 91d0533 Author: Andrea Ceccanti <[email protected]> Date: Sat Nov 13 16:02:51 2021 +0100 Tests green (locally) commit d575a47 Author: Andrea Ceccanti <[email protected]> Date: Sat Nov 13 08:59:09 2021 +0100 More warning and test fixes commit 0b62963 Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 12 18:38:35 2021 +0100 More test fixes commit 922b464 Author: Andrea Ceccanti <[email protected]> Date: Tue Nov 9 11:00:34 2021 +0100 Test errors -> 0, Test failures -> ~12% commit f7f8513 Author: Andrea Ceccanti <[email protected]> Date: Sat Nov 6 09:50:08 2021 +0100 Silence deprecation warnings Only if coming from the latest spring-security-oauth2 (for which we do not and won't have a replacement for some time). commit 8f27bd2 Author: Andrea Ceccanti <[email protected]> Date: Sat Nov 6 08:44:07 2021 +0100 Use H2 datasource for the tests This prevents issues with the hikari connection pool being closed. commit 45c7b4e Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 19:44:15 2021 +0100 Service starts up! commit 4f984ee Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 19:27:01 2021 +0100 Use a keystore with key size 2048 commit 3efc9f9 Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 18:37:34 2021 +0100 Flyway migration refactoring to avoid naming errors commit 3f5e741 Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 18:22:42 2021 +0100 Moved source/target compatibility to Java 11 commit 7e1f1d6 Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 18:14:18 2021 +0100 License updates commit b63ce93 Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 18:13:32 2021 +0100 Config files changes commit 66d28d2 Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 17:59:07 2021 +0100 Builds against spring boot 2.5.6 commit 0d9167a Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 10:26:35 2021 +0100 Fixed flyway migrations compilation problems commit 1ca9d73 Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 10:09:10 2021 +0100 wip commit acd7e4f Author: Andrea Ceccanti <[email protected]> Date: Fri Nov 5 08:07:33 2021 +0100 WIP: maven clean succeeds commit 2b9835e Author: Andrea Ceccanti <[email protected]> Date: Thu Nov 4 18:18:33 2021 +0100 wip commit 8dbf1cf Author: Andrea Ceccanti <[email protected]> Date: Wed Nov 3 19:35:18 2021 +0100 Fixed code smells reported by Sonar commit 56b570e Author: Andrea Ceccanti <[email protected]> Date: Wed Nov 3 19:16:59 2021 +0100 Just build on Java 11 Still not there for Java 17... commit 1529049 Author: Andrea Ceccanti <[email protected]> Date: Wed Nov 3 18:41:18 2021 +0100 Restore sonar analysis commit ed52207 Author: Andrea Ceccanti <[email protected]> Date: Wed Nov 3 18:40:23 2021 +0100 Use openjdk:11 docker images commit cc382c2 Author: Andrea Ceccanti <[email protected]> Date: Wed Nov 3 18:26:14 2021 +0100 Drop java 8 commit 9dc729c Author: Andrea Ceccanti <[email protected]> Date: Wed Nov 3 17:59:29 2021 +0100 Build on Jenkins with Java 11 commit 7c090dd Author: Andrea Ceccanti <[email protected]> Date: Wed Nov 3 17:52:18 2021 +0100 First attempt at java version matrix build commit d77d860 Author: Andrea Ceccanti <[email protected]> Date: Wed Nov 3 17:43:21 2021 +0100 Dropped validator-collections dependency commit 47cf69b Author: Andrea Ceccanti <[email protected]> Date: Mon Nov 1 16:46:22 2021 +0100 Fix test fixture initialization commit 59406e0 Author: Andrea Ceccanti <[email protected]> Date: Mon Nov 1 16:45:23 2021 +0100 Drop DevToolsDataSourceAutoConfiguration Which breaks h2 tests. commit 6c18f35 Author: Andrea Ceccanti <[email protected]> Date: Mon Nov 1 16:44:52 2021 +0100 Add flyway debug log handle commit 2f433ad Author: Andrea Ceccanti <[email protected]> Date: Mon Nov 1 16:44:28 2021 +0100 Streamlined h2 db test configuration commit c9eaa16 Author: Andrea Ceccanti <[email protected]> Date: Mon Nov 1 16:41:39 2021 +0100 Upgrade surefire plugin to the latest version commit 3ae9b7f Author: Andrea Ceccanti <[email protected]> Date: Fri Oct 29 08:18:12 2021 +0200 Archive JUnit reports commit 9626982 Author: Andrea Ceccanti <[email protected]> Date: Fri Oct 29 07:45:04 2021 +0200 Removed ununsed dependency commit de574c8 Author: Andrea Ceccanti <[email protected]> Date: Fri Oct 29 07:44:30 2021 +0200 More test fixes commit b3620ac Author: Andrea Ceccanti <[email protected]> Date: Fri Oct 29 07:43:58 2021 +0200 Control how many test contexts are cached during builds commit a853f94 Author: Andrea Ceccanti <[email protected]> Date: Thu Oct 28 12:32:08 2021 +0200 More test fixes commit 481a456 Author: Andrea Ceccanti <[email protected]> Date: Thu Oct 28 11:13:39 2021 +0200 More test fixes commit 003a486 Author: Andrea Ceccanti <[email protected]> Date: Thu Oct 28 08:49:19 2021 +0200 More test fixes commit 0417ad1 Author: Andrea Ceccanti <[email protected]> Date: Wed Oct 27 19:25:19 2021 +0200 More test porting commit ed30322 Author: Andrea Ceccanti <[email protected]> Date: Wed Oct 27 18:29:58 2021 +0200 Fixed Velocity initialization And moved email templates from the /templates folder to the /email-templates folder in the classpath. commit b35bf83 Author: Andrea Ceccanti <[email protected]> Date: Wed Oct 27 08:52:18 2021 +0200 More test fixing work commit c2b205b Author: Andrea Ceccanti <[email protected]> Date: Tue Oct 26 17:48:16 2021 +0200 More test fixes commit b310d4c Author: Andrea Ceccanti <[email protected]> Date: Tue Oct 26 17:23:47 2021 +0200 All api tests green commit 4a70982 Author: Andrea Ceccanti <[email protected]> Date: Tue Oct 26 12:17:49 2021 +0200 Cors configuration & actuator test fixes commit 4ed75ff Author: Andrea Ceccanti <[email protected]> Date: Tue Oct 26 00:38:23 2021 +0200 Remove cors filter configuration commit a948741 Author: Andrea Ceccanti <[email protected]> Date: Mon Oct 25 19:37:51 2021 +0200 Added license commit e025b7d Author: Andrea Ceccanti <[email protected]> Date: Mon Oct 25 19:37:25 2021 +0200 Started migration of integration tests commit ee7fc54 Author: Andrea Ceccanti <[email protected]> Date: Mon Oct 25 19:36:54 2021 +0200 New unified test annotation commit 149d9d1 Author: Andrea Ceccanti <[email protected]> Date: Mon Oct 25 19:14:54 2021 +0200 Project compiles commit 84ed532 Author: Andrea Ceccanti <[email protected]> Date: Mon Oct 25 19:12:59 2021 +0200 Run update-tests script commit ce93f59 Author: Andrea Ceccanti <[email protected]> Date: Mon Oct 25 19:07:21 2021 +0200 Fix compilation problems on main code Fix renamed classes and changed JPAConfig commit 93b80c6 Author: Andrea Ceccanti <[email protected]> Date: Mon Oct 25 19:06:47 2021 +0200 First migrate to spring boot 1.5.22 commit de5f1b1 Author: Andrea Ceccanti <[email protected]> Date: Mon Oct 25 18:30:00 2021 +0200 Updated Spring and mitreid deps commit e9e5408 Merge: 8c9b8bc 4bfc271 Author: Andrea Ceccanti <[email protected]> Date: Sun Oct 24 17:47:19 2021 +0200 Merge pull request indigo-iam#433 from indigo-iam/issue-432-include-groups-in-userinfo-response-wlcg Include wlcg.groups in userinfo response commit 8c9b8bc Merge: ec31232 8ffed21 Author: Andrea Ceccanti <[email protected]> Date: Sun Oct 24 17:47:05 2021 +0200 Merge pull request indigo-iam#431 from indigo-iam/issue-430-improved-jwk-configuration Improved support for JWK configuration commit ec31232 Merge: 767e86e 88bb278 Author: Andrea Ceccanti <[email protected]> Date: Sun Oct 24 17:46:45 2021 +0200 Merge pull request indigo-iam#427 from indigo-iam/issue-426-jwt-based-client-auth First attempt at JWT-based client-auth commit 4bfc271 Author: Andrea Ceccanti <[email protected]> Date: Sun Oct 24 17:23:36 2021 +0200 Include wlcg.groups information in userinfo response Even though the IAM access token is a JWT and even though groups are included in the access token when requested, as mandated by the WLCG JWT profile, there are still apps treating the access token as an opaque string. To support those apps, and be more consistent with the traditional IAM profile behaviour, IAM should include group information in the userinfo endpoint response also for the WLCG profile. Issue: indigo-iam#432 commit 195c2d7 Merge: 7f90144 5b8d9d8 Author: Andrea Ceccanti <[email protected]> Date: Thu Sep 23 15:23:34 2021 +0200 Merge pull request indigo-iam#425 from indigo-iam/issue-424-IAM-does-not-encode-group-names-correctly-aarc-g002 Fix for issue-422: iam does not encode group names correctly according to AARC G002 commit 8ffed21 Author: Andrea Ceccanti <[email protected]> Date: Sun Oct 24 09:25:36 2021 +0200 Improved support for JWT configuration It's now possible to specify the default key id and algorithm used for signing tokens. Issue: indigo-iam#430 commit 88bb278 Author: Andrea Ceccanti <[email protected]> Date: Sat Oct 23 09:56:08 2021 +0200 More integration tests commit cd8ef61 Author: Andrea Ceccanti <[email protected]> Date: Sat Oct 23 08:39:43 2021 +0200 More tests for JWTAuthenticationProvider commit fc7148d Author: Andrea Ceccanti <[email protected]> Date: Sun Oct 17 23:03:37 2021 +0200 First attempt at JWT-based client-auth commit 767e86e Merge: 7f90144 5b8d9d8 Author: Andrea Ceccanti <[email protected]> Date: Thu Sep 23 15:23:34 2021 +0200 Merge pull request indigo-iam#425 from indigo-iam/issue-424-IAM-does-not-encode-group-names-correctly-aarc-g002 Fix for issue-422: iam does not encode group names correctly according to AARC G002 commit 5b8d9d8 Author: Andrea Ceccanti <[email protected]> Date: Thu Sep 23 14:50:32 2021 +0200 Fix wrong AARC G002 group name encoding commit 7f90144 Author: Andrea Ceccanti <[email protected]> Date: Tue Sep 14 07:56:12 2021 +0200 Version bumped back to 1.8.0-SNAPSHOT commit 1828bf0 Author: Andrea Ceccanti <[email protected]> Date: Tue Sep 14 07:55:41 2021 +0200 Test custom logging conf
This ensures tables are created properly and contain the appropriate test data. Currently, no test data for multi-factor secrets and recovery codes exist.
This is currently done through a GET request because I couldn't get a POST request to work. Will investigate this as a TOTP needs to be passed for verification anyway. Secrets and recovery codes are generated through the user account service and then added in plaintext. Later on, will add functionality for encrypting them
Conflicts: iam-login-service/src/main/java/it/infn/mw/iam/actuator/endpoint/IamMailHealthIndicator.java iam-login-service/src/main/java/it/infn/mw/iam/api/account/multi_factor_authentication/authenticator_app/CodeDTO.java iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/jwk/JWKTestSupport.java
…iam/iam into iam-spring-update-oct-2021
These were old event classes that are no longer used/have been replaced
Still work to be done on autowiring QR generator instead of creating a new object each time
added 3 commits
January 27, 2022 16:23
This was only a placeholder for if/when the functionality for YubiKey was ever implemented
The TokenEndpoint class is responsible for calling createTokenRequest. It passes parameters from the request to that endpoint and stores them inside the OAuth2AccessToken itself in the request field. This request is passed throughout the rest of the OAuth procedure, including to the IdTokenCustomizer. Here, its parameters can be read and so the amr claim can be retrieved here. This is much simpler than trying to extend or modify the authentication object present at this stage of the OAuth process. What we need to do from here is try and pass the amr set as part of the request parameters to the token endpoint from the authorize endpoint. At the authorize endpoint, we are still authenticated with the ExtendedAuthenticationToken, which contains the amr set which we will access.
…token for non-MFA users This required extending the HttpServletRequest to include additional information and a filter to determine when to do this. This filter has to execute after authentication has taken place so that the ExtendedAuthenticationToken can be retrieved from the security context. This currently only works for non-MFA users, as the current implementation redirects all MFA users to the dashboard after authenticating (demo purposes).
enricovianello
changed the base branch from
iam-spring-update-oct-2021
to
v1.8.0
added 7 commits
February 24, 2022 15:19
This involved refactoring the way TOTPs were verified. Previously, this was done through the controller for the verification page itself. This logic is being moved into a dedicated authentication provider, executed by a dedicated authentication processing filter on a new authentication entry point. Still to test failed TOTP entries and account for recovery codes as well
Bad credentials for the initial password-based login were failing due to a failure handler not being attached to the login filter. This was because the default UsernamePasswordAuthenticationFilter has been replaced by the ExtendedAuthenticationFilter
Incorrect setting of failure handlers and handling of errors in general meant incorrect TOTPs would cause a crash. Incorrect codes now redirect back to the /verify endpoint in the same way as a failed credential check would redirect back to the /login endpoint
Conflicts: iam-login-service/src/main/java/it/infn/mw/iam/api/account/AccountUtils.java iam-login-service/src/main/java/it/infn/mw/iam/core/user/DefaultIamAccountService.java iam-login-service/src/main/java/it/infn/mw/iam/core/web/util/IamViewInfoInterceptor.java iam-login-service/src/main/webapp/WEB-INF/views/iam/dashboard.jsp iam-login-service/src/test/java/it/infn/mw/iam/test/service/IamAccountServiceTests.java iam-test-client/docker/Dockerfile iam-test-client/docker/Dockerfile.prod
This currently allows the user, having entered a code, to choose whether or not to reset their recovery codes.
Includes latest updates to database migrations and repositories, etc. Now making use of separate repository and service for IamTotpMfa secret objects and their respective operations. This separates them in a more logical manner from IamAccount objects and removes the need to go through an IamAccount to access its MFA secret. Also adding a service specifically for resetting recovery codes. Also fixing a couple of bugs with resetting recovery codes.
added 5 commits
March 8, 2022 15:49
Fixing a problem with granted authorities. Earlier changes to code meant that certain parts of the website could still be visited after only one step of authentication. This was because ROLE_USER was accidentally assigned alongside ROLE_PRE_AUTHENTICATED. This is being fixed by assigning the user's fully authenticated authorities to a separate field in the ExtendedAuthenticationToken, while the token's official authorities only contains ROLE_PRE_AUTHENTICATED. Once full authentication occurs, the full set of authorities are granted to the user.
…nning/iam into issue-418-mfa-support
All existing tests relating to MFA are being fixed. These tests all relate to MFA work prior to OAuth2 integration, IamAuthenticationMethodReference, ExtendedAuthenticationToken, etc. Some tests are still broken and these seem to relate to the inclusion of ExtendedAuthenticationToken being adopted as the default global Authentication object. Because of this, it is coming into conflict with some OAuth functionality I haven't touched.
|
@enricovianello based on the time I have left, I'm going to mark this as Ready for review now. This will allow us to move my work from my IAM fork into the IAM repo proper. I think we should make a new branch on the IAM repo and change the base branch of this PR to that new branch. Here's some preliminary info about where I'm at:
Tomorrow, I will endeavour to finish off any remaining documentation and send it to you and Francesco. I will also add links to it in this PR and will include a wider amount of information relating to all the features that have been tackled and what is remaining. EDIT: there is one final test in one of my controllers that is still failing. If I have time tomorrow, I'll look at it but it's not a big task. |
CodeDTO objects were not being verified properly MFA was not being properly checked for in verification stage
Feature list as of 11/03/2022Implemented
Not yet implemented
|
|
Thanks, Sam, for your nice work and all the best for your next challenge. |
Thanks Francesco, just working on one final document to add to this PR and then I'll forward all the documentation in an email to you and Enrico when I'm done. |
|
Superseded by #662 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This adds the option of enabling multi-factor authentication through an authenticator app on an IAM account. This is designed to be flexible so that the user can enable or disable it at their will. The additional verification step kicks in after username and password authentication has taken place. At this stage, the user is "pre-authenticated" - they still cannot access the rest of the application. They are redirected to a verification page, whereby they enter a TOTP or a recovery code to be granted full authentication.
Documentation
Screenshots
Verify page
Multi-factor settings button
Multi-factor settings menu
Enabling multi-factor via a QR code and authenticator app
Display of recovery codes
