Persistence layer migrations for MFA support #419

[sam-glendenning]

Creating a fresh pull request based off of #452, which got confused with the branch it was merging to and the branch it was originally based off of. As such, there were merge conflicts which weren't being registered, which complicated the entire PR. This PR should be considered the most up to date one.

---

This PR contains the necessary database migrations and Java model changes to support the inclusion of multi-factor authentication into the IAM. Broadly speaking, the changes encompass:

• new tables for authenticator app secrets and their respective recovery codes (iam_totp_mfa and iam_totp_recovery_code)
• an additional iam_authority called ROLE_PRE_AUTHENTICATED which is used by Spring to grant a user access to webpages where they can continue the authentication process after signing in with their basic login credentials but does not grant them full access to the IAM. This was necessary to simulate the "two-step" login process associated with multi-factor authentication.

This PR is related to #441 which focuses on the higher-level implementation of MFA in the IAM.

Closes #419

[enricovianello]

As expected, adding this one-to-one table to IamAccount without adding stuff to IamAccountService and other related classes, as you've done later on this PR (https://github.com/indigo-iam/iam/pull/441/files#diff-b61d0dbfd42c33b0632f9868c83d2caaaa6588fa508458c436116dde3234bf5a) makes the deletion of your added user "test-with-mfa" not working for a "Referential integrity constraint violation"

[EL Warning]: 2022-02-25 19:14:14.22--UnitOfWork(2096969069)--Exception [EclipseLink-4002] (Eclipse Persistence Services - 2.7.9.v20210604-2c549e2208): org.eclipse.persistence.exceptions.DatabaseException
Internal Exception: org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException: Referential integrity constraint violation: "FK_IAM_TOTP_MFA_ACCOUNT_ID: PUBLIC.IAM_TOTP_MFA FOREIGN KEY(ACCOUNT_ID) REFERENCES PUBLIC.IAM_ACCOUNT(ID) (1000)"; SQL statement:
DELETE FROM iam_account WHERE (ID = ?) [23503-200]
Error Code: 23503
Call: DELETE FROM iam_account WHERE (ID = ?)
	bind => [1 parameter bound]
Query: DeleteObjectQuery(IamAccount [id=1000, uuid=467c882e-90da-11ec-b909-0242ac120002, username=test-with-mfa, active=true])
2022-02-25 19:14:14.238 ERROR 25919 --- [io-8080-exec-10] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is org.springframework.transaction.TransactionSystemException: Could not commit JPA transaction; nested exception is javax.persistence.RollbackException: Exception [EclipseLink-4002] (Eclipse Persistence Services - 2.7.9.v20210604-2c549e2208): org.eclipse.persistence.exceptions.DatabaseException
Internal Exception: org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException: Referential integrity constraint violation: "FK_IAM_TOTP_MFA_ACCOUNT_ID: PUBLIC.IAM_TOTP_MFA FOREIGN KEY(ACCOUNT_ID) REFERENCES PUBLIC.IAM_ACCOUNT(ID) (1000)"; SQL statement:
DELETE FROM iam_account WHERE (ID = ?) [23503-200]
Error Code: 23503
Call: DELETE FROM iam_account WHERE (ID = ?)
	bind => [1 parameter bound]
Query: DeleteObjectQuery(IamAccount [id=1000, uuid=467c882e-90da-11ec-b909-0242ac120002, username=test-with-mfa, active=true])] with root cause

org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException: Referential integrity constraint violation: "FK_IAM_TOTP_MFA_ACCOUNT_ID: PUBLIC.IAM_TOTP_MFA FOREIGN KEY(ACCOUNT_ID) REFERENCES PUBLIC.IAM_ACCOUNT(ID) (1000)"; SQL statement:
DELETE FROM iam_account WHERE (ID = ?) [23503-200]
	at org.h2.message.DbException.getJdbcSQLException(DbException.java:459)
	at org.h2.message.DbException.getJdbcSQLException(DbException.java:429)
[...]

I got this by running the spring boot application locally. But we were expecting this behavior after the decision to not touch IamAccount. This is the natural consequence.
But, for sure, users have no chance to add stuff into the new tables. So this behavior won't happen in any case. So I think we could ignore this problem at the moment and merge all.
We can discuss this all together on our Monday weekly meeting and then definitively approve and merge this PR. 👍

[enricovianello]

I was about to merge then I took another look and saw a little thing that could be fixed.

[enricovianello]

The code in "else" works also in case recoveryCodes is empty. My suggestion is to replace all the if-else with:

this.recoveryCodes.clear();
this.recoveryCodes.addAll(recoveryCodes);

[sam-glendenning]

Fixed this in the latest commit. Also added some additional setting of creation and update time to the constructors

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[enricovianello]

Perfect! It's time to merge all!