From f1e833d5b7834cec21f76a70269c2d5b81318813 Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Mon, 23 Oct 2023 12:47:41 +0200 Subject: [PATCH] Broken test on delete cascade --- .../requests/GroupRequestsApproveTests.java | 4 +-- .../requests/GroupRequestsRejectTests.java | 6 ++-- .../ClientRegistrationTests.java | 2 +- .../repository/IamTokenRepositoryTests.java | 33 +++++++++++++++++++ .../db/migration/h2/V96__add_foreign_keys.sql | 18 +++++++--- .../migration/mysql/V96__add_foreign_keys.sql | 18 +++++++--- 6 files changed, 65 insertions(+), 16 deletions(-) diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/requests/GroupRequestsApproveTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/requests/GroupRequestsApproveTests.java index 094375f65..f51dab525 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/requests/GroupRequestsApproveTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/requests/GroupRequestsApproveTests.java @@ -18,7 +18,7 @@ import static java.lang.String.format; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.MatcherAssert.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -89,7 +89,7 @@ public void approveGroupRequestAsAdmin() throws Exception { // @formatter:on GroupRequestDto result = mapper.readValue(response, GroupRequestDto.class); - assertThat(result.getLastUpdateTime(), greaterThan(result.getCreationTime())); + assertThat(result.getLastUpdateTime(), greaterThanOrEqualTo(result.getCreationTime())); int mailCount = notificationService.countPendingNotifications(); assertThat(mailCount, equalTo(1)); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/requests/GroupRequestsRejectTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/requests/GroupRequestsRejectTests.java index e1e1be042..78fd83feb 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/requests/GroupRequestsRejectTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/requests/GroupRequestsRejectTests.java @@ -16,11 +16,11 @@ package it.infn.mw.iam.test.api.requests; import static java.lang.String.format; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.MatcherAssert.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -95,7 +95,7 @@ public void rejectGroupRequestAsAdmin() throws Exception { .getContentAsString(); // @formatter:on GroupRequestDto result = mapper.readValue(response, GroupRequestDto.class); - assertThat(result.getLastUpdateTime(), greaterThan(result.getCreationTime())); + assertThat(result.getLastUpdateTime(), greaterThanOrEqualTo(result.getCreationTime())); int mailCount = notificationService.countPendingNotifications(); assertThat(mailCount, equalTo(1)); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTests.java index 13752e9aa..e7f624966 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTests.java @@ -127,7 +127,7 @@ public void testClientRegistrationAccessTokenWorks() throws Exception { mvc .perform(get(registrationUri).contentType(APPLICATION_JSON) .header("Authorization", "Bearer " + rat)) - .andExpect(status().isNotFound()); + .andExpect(status().isUnauthorized()); } @Test diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/repository/IamTokenRepositoryTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/repository/IamTokenRepositoryTests.java index 94c82d439..f3c3c7a04 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/repository/IamTokenRepositoryTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/repository/IamTokenRepositoryTests.java @@ -15,6 +15,7 @@ */ package it.infn.mw.iam.test.repository; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasSize; @@ -24,8 +25,11 @@ import org.apache.commons.lang.time.DateUtils; import org.junit.Test; import org.junit.runner.RunWith; +import org.mitre.oauth2.model.AuthenticationHolderEntity; import org.mitre.oauth2.model.ClientDetailsEntity; import org.mitre.oauth2.model.OAuth2AccessTokenEntity; +import org.mitre.oauth2.model.OAuth2RefreshTokenEntity; +import org.mitre.oauth2.repository.AuthenticationHolderRepository; import org.mitre.oauth2.service.ClientDetailsEntityService; import org.mitre.oauth2.service.impl.DefaultOAuth2ProviderTokenService; import org.springframework.beans.factory.annotation.Autowired; @@ -59,6 +63,9 @@ public class IamTokenRepositoryTests { @Autowired private IamOAuthRefreshTokenRepository refreshTokenRepo; + @Autowired + private AuthenticationHolderRepository authenticationHolderRepo; + @Autowired private ClientDetailsEntityService clientDetailsService; @@ -164,4 +171,30 @@ public void testRepositoryDoesntRelyOnDbTime() { assertThat(refreshTokenRepo.findValidRefreshTokensForUser(TEST_347_USER, now), hasSize(1)); } + @Test + public void testTokenNoCascadeDeletion() { + OAuth2AccessTokenEntity at = buildAccessToken(loadTestClient(), TEST_347_USER); + OAuth2RefreshTokenEntity rt = at.getRefreshToken(); + AuthenticationHolderEntity ah = at.getAuthenticationHolder(); + accessTokenRepo.delete(at); + assertThat(refreshTokenRepo.findById(rt.getId()).isEmpty(), is(false)); + assertThat(authenticationHolderRepo.getById(ah.getId()) != null, is(true)); + refreshTokenRepo.delete(rt); + assertThat(refreshTokenRepo.findById(rt.getId()).isEmpty(), is(true)); + assertThat(authenticationHolderRepo.getById(ah.getId()) != null, is(true)); + authenticationHolderRepo.remove(ah); + assertThat(authenticationHolderRepo.getById(ah.getId()) != null, is(false)); + } + + @Test + public void testTokenCascadeDeletion() { + OAuth2AccessTokenEntity at = buildAccessToken(loadTestClient(), TEST_347_USER); + OAuth2RefreshTokenEntity rt = at.getRefreshToken(); + AuthenticationHolderEntity ah = at.getAuthenticationHolder(); + authenticationHolderRepo.remove(ah); + assertThat(accessTokenRepo.findById(at.getId()).isEmpty(), is(true)); + assertThat(refreshTokenRepo.findById(rt.getId()).isEmpty(), is(true)); + assertThat(authenticationHolderRepo.getById(ah.getId()) != null, is(false)); + } + } diff --git a/iam-persistence/src/main/resources/db/migration/h2/V96__add_foreign_keys.sql b/iam-persistence/src/main/resources/db/migration/h2/V96__add_foreign_keys.sql index d05a177a4..9eceda90d 100644 --- a/iam-persistence/src/main/resources/db/migration/h2/V96__add_foreign_keys.sql +++ b/iam-persistence/src/main/resources/db/migration/h2/V96__add_foreign_keys.sql @@ -71,19 +71,27 @@ DELETE FROM access_token WHERE refresh_token_id NOT IN (SELECT id FROM refresh_t DELETE FROM access_token WHERE client_id NOT IN (SELECT id FROM client_details); DELETE FROM access_token WHERE auth_holder_id NOT IN (SELECT id FROM authentication_holder); -ALTER TABLE access_token ADD CONSTRAINT FK_access_token_refresh_token_id FOREIGN KEY (refresh_token_id) REFERENCES refresh_token (id) ON DELETE SET NULL; -ALTER TABLE access_token ADD CONSTRAINT FK_access_token_client_id FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE SET NULL; -ALTER TABLE access_token ADD CONSTRAINT FK_access_token_auth_holder_id FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE SET NULL; +ALTER TABLE access_token ADD CONSTRAINT FK_access_token_refresh_token_id FOREIGN KEY (refresh_token_id) REFERENCES refresh_token (id) ON DELETE CASCADE; +ALTER TABLE access_token ADD CONSTRAINT FK_access_token_client_id FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE CASCADE; +ALTER TABLE access_token ADD CONSTRAINT FK_access_token_auth_holder_id FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; -- REFRESH TOKEN DELETE FROM refresh_token WHERE client_id NOT IN (SELECT id FROM client_details); -ALTER TABLE refresh_token ADD CONSTRAINT FK_refresh_token_client_id FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE SET NULL; +ALTER TABLE refresh_token ADD CONSTRAINT FK_refresh_token_client_id FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM refresh_token WHERE auth_holder_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE refresh_token ADD CONSTRAINT FK_refresh_token_auth_holder_id FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +-- AUTHORIZATION CODE + +DELETE FROM authorization_code WHERE auth_holder_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authorization_code ADD CONSTRAINT FK_authorization_code_auth_holder_id FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; -- APPROVED SITE DELETE FROM approved_site WHERE client_id NOT IN (SELECT id FROM client_details); -ALTER TABLE approved_site ADD CONSTRAINT FK_approved_site_client_id FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE SET NULL; +ALTER TABLE approved_site ADD CONSTRAINT FK_approved_site_client_id FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE CASCADE; DELETE FROM approved_site_scope WHERE owner_id NOT IN (SELECT id FROM approved_site); ALTER TABLE approved_site_scope ADD CONSTRAINT FK_approved_site_scope_owner_id FOREIGN KEY (owner_id) REFERENCES approved_site (id) ON DELETE CASCADE; diff --git a/iam-persistence/src/main/resources/db/migration/mysql/V96__add_foreign_keys.sql b/iam-persistence/src/main/resources/db/migration/mysql/V96__add_foreign_keys.sql index 386d20565..7604a7fe2 100644 --- a/iam-persistence/src/main/resources/db/migration/mysql/V96__add_foreign_keys.sql +++ b/iam-persistence/src/main/resources/db/migration/mysql/V96__add_foreign_keys.sql @@ -71,19 +71,27 @@ DELETE FROM access_token WHERE refresh_token_id NOT IN (SELECT id FROM refresh_t DELETE FROM access_token WHERE client_id NOT IN (SELECT id FROM client_details); DELETE FROM access_token WHERE auth_holder_id NOT IN (SELECT id FROM authentication_holder); -ALTER TABLE access_token ADD CONSTRAINT FK_access_token_refresh_token_id FOREIGN KEY (refresh_token_id) REFERENCES refresh_token (id) ON DELETE SET NULL; -ALTER TABLE access_token ADD CONSTRAINT FK_access_token_client_id FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE SET NULL; -ALTER TABLE access_token ADD CONSTRAINT FK_access_token_auth_holder_id FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE SET NULL; +ALTER TABLE access_token ADD CONSTRAINT FK_access_token_refresh_token_id FOREIGN KEY (refresh_token_id) REFERENCES refresh_token (id) ON DELETE CASCADE; +ALTER TABLE access_token ADD CONSTRAINT FK_access_token_client_id FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE CASCADE; +ALTER TABLE access_token ADD CONSTRAINT FK_access_token_auth_holder_id FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; -- REFRESH TOKEN DELETE FROM refresh_token WHERE client_id NOT IN (SELECT id FROM client_details); -ALTER TABLE refresh_token ADD CONSTRAINT FK_refresh_token_client_id FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE SET NULL; +ALTER TABLE refresh_token ADD CONSTRAINT FK_refresh_token_client_id FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM refresh_token WHERE auth_holder_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE refresh_token ADD CONSTRAINT FK_refresh_token_auth_holder_id FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +-- AUTHORIZATION CODE + +DELETE FROM authorization_code WHERE auth_holder_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authorization_code ADD CONSTRAINT FK_authorization_code_auth_holder_id FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; -- APPROVED SITE DELETE FROM approved_site WHERE client_id NOT IN (SELECT id FROM client_details); -ALTER TABLE approved_site ADD CONSTRAINT FK_approved_site_client_id FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE SET NULL; +ALTER TABLE approved_site ADD CONSTRAINT FK_approved_site_client_id FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE CASCADE; DELETE FROM approved_site_scope WHERE owner_id NOT IN (SELECT id FROM approved_site); ALTER TABLE approved_site_scope ADD CONSTRAINT FK_approved_site_scope_owner_id FOREIGN KEY (owner_id) REFERENCES approved_site (id) ON DELETE CASCADE;