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

Remove recovery codes for MFA #712

Closed
wants to merge 99 commits into from
Closed

Remove recovery codes for MFA #712

wants to merge 99 commits into from

Conversation

rmiccoli
Copy link
Contributor

No description provided.

Sam Glendenning and others added 30 commits November 16, 2021 16:57
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
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
…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 #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 #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 #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: #432

commit 195c2d7
Merge: 7f90144 5b8d9d8
Author: Andrea Ceccanti <[email protected]>
Date:   Thu Sep 23 15:23:34 2021 +0200

    Merge pull request #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: #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 #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
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
Mfa settings menu now updates to show status of user's multi-factor settings that are enabled (i.e. button will be green or red).

Toaster notification also displays upon successful operation

Authenticator app disabling now possible through GET request. Still working on POST request and code validation
Sam Glendenning and others added 28 commits February 3, 2022 13:57
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).
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.
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.
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.
CodeDTO objects were not being verified properly

MFA was not being properly checked for in verification stage
@rmiccoli
Copy link
Contributor Author

Superseded by PR #733

@rmiccoli rmiccoli closed this Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants