From e6eec641cd7378132c034aa93c2993cac7e9dbfe Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Mon, 17 Jul 2023 18:51:22 +0200 Subject: [PATCH 01/22] Bump mitre version to 1.3.6.cnaf-20230717 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 19d0a9174..1f57b4e46 100644 --- a/pom.xml +++ b/pom.xml @@ -29,7 +29,7 @@ 1.16.2 - 1.3.5.cnaf-spring-update-080122 + 1.3.6.cnaf-20230717 2.5.1.RELEASE 3.3.2 From 8423edc4a991d081d6e645b66479a02a03b410b8 Mon Sep 17 00:00:00 2001 From: Roberta Miccoli <85555840+rmiccoli@users.noreply.github.com> Date: Mon, 17 Jul 2023 19:34:43 +0200 Subject: [PATCH 02/22] Align codebase with latest mitre updates to access token table (#613) * Add db migrations - drop UNIQUE constraint on the first 766 characters of the `token_value` - add `token_value_hash` column to `access_token` table - create UNIQUE index on `token_value_hash` column * Change access token search query in order to search by the hash of the value instead of the value --- .../repository/IamOAuthAccessTokenRepository.java | 6 ++---- .../src/main/resources/db/migration/h2/V93__add_at_hash.sql | 3 +++ .../main/resources/db/migration/mysql/V93__add_at_hash.sql | 3 +++ 3 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 iam-persistence/src/main/resources/db/migration/h2/V93__add_at_hash.sql create mode 100644 iam-persistence/src/main/resources/db/migration/mysql/V93__add_at_hash.sql diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/persistence/repository/IamOAuthAccessTokenRepository.java b/iam-login-service/src/main/java/it/infn/mw/iam/persistence/repository/IamOAuthAccessTokenRepository.java index b0733232f..5d93d7e31 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/persistence/repository/IamOAuthAccessTokenRepository.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/persistence/repository/IamOAuthAccessTokenRepository.java @@ -26,13 +26,11 @@ import org.springframework.data.repository.PagingAndSortingRepository; import org.springframework.data.repository.query.Param; -import com.nimbusds.jwt.JWT; - public interface IamOAuthAccessTokenRepository extends PagingAndSortingRepository { - @Query("select t from OAuth2AccessTokenEntity t where t.jwt = :tokenValue") - Optional findByTokenValue(@Param("tokenValue") JWT tokenValue); + @Query("select t from OAuth2AccessTokenEntity t where t.tokenValueHash = :atHash") + Optional findByTokenValue(@Param("atHash") String atHash); @Query("select t from OAuth2AccessTokenEntity t where t.authenticationHolder.userAuth.name = :userId " + "and (t.expiration is NOT NULL and t.expiration > :timestamp)") diff --git a/iam-persistence/src/main/resources/db/migration/h2/V93__add_at_hash.sql b/iam-persistence/src/main/resources/db/migration/h2/V93__add_at_hash.sql new file mode 100644 index 000000000..b2bf521fd --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/h2/V93__add_at_hash.sql @@ -0,0 +1,3 @@ +ALTER TABLE access_token DROP CONSTRAINT at_unique_token_value; +ALTER TABLE access_token ADD COLUMN token_value_hash CHAR(64) AS HASH('SHA256', token_value); +ALTER TABLE access_token ADD CONSTRAINT at_tvh_idx UNIQUE (token_value_hash); \ No newline at end of file diff --git a/iam-persistence/src/main/resources/db/migration/mysql/V93__add_at_hash.sql b/iam-persistence/src/main/resources/db/migration/mysql/V93__add_at_hash.sql new file mode 100644 index 000000000..8c839a374 --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/mysql/V93__add_at_hash.sql @@ -0,0 +1,3 @@ +ALTER TABLE access_token DROP INDEX token_value; +ALTER TABLE access_token ADD COLUMN token_value_hash CHAR(64) AS (SHA2(token_value, 256)); +ALTER TABLE access_token ADD CONSTRAINT at_tvh_idx UNIQUE (token_value_hash); \ No newline at end of file From 7a97229a429b7f4fc01d6046605af5af684d6ce4 Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Tue, 18 Jul 2023 16:17:44 +0200 Subject: [PATCH 03/22] Improved Multi-Module Maven Application structure Bump bouncycastle version to 1.75 Bump Spring Boot version to 2.6.15 Bump jQuery version to 1.13.2 --- iam-common/pom.xml | 11 ++++++----- iam-login-service/pom.xml | 22 ++++++++++++++-------- iam-persistence/pom.xml | 9 +++++---- iam-test-client/pom.xml | 16 +++++++++++----- iam-voms-aa/pom.xml | 22 ++++++++++++++-------- pom.xml | 31 ++++++++++++++++++------------- 6 files changed, 68 insertions(+), 43 deletions(-) diff --git a/iam-common/pom.xml b/iam-common/pom.xml index 4e624817e..80ad43735 100644 --- a/iam-common/pom.xml +++ b/iam-common/pom.xml @@ -1,14 +1,15 @@ - + 4.0.0 - it.infn.mw + it.infn.mw.iam-parent iam-parent 1.8.3 + it.infn.mw.iam-common iam-common jar @@ -35,7 +36,7 @@ - ${project.groupId} + it.infn.mw.iam-persistence iam-persistence ${project.version} diff --git a/iam-login-service/pom.xml b/iam-login-service/pom.xml index ae7224bf5..fe3041d79 100644 --- a/iam-login-service/pom.xml +++ b/iam-login-service/pom.xml @@ -16,16 +16,17 @@ limitations under the License. --> - + 4.0.0 - it.infn.mw + it.infn.mw.iam-parent iam-parent 1.8.3 + it.infn.mw.iam-login-service iam-login-service war @@ -44,7 +45,7 @@ - ${project.groupId} + it.infn.mw.iam-persistence iam-persistence ${project.version} @@ -56,7 +57,7 @@ - ${project.groupId} + it.infn.mw.iam-common iam-common ${project.version} @@ -279,12 +280,17 @@ org.bouncycastle - bcpkix-jdk15on + bcpkix-jdk18on org.bouncycastle - bcprov-jdk15on + bcprov-jdk18on + + + + org.bouncycastle + bcutil-jdk18on diff --git a/iam-persistence/pom.xml b/iam-persistence/pom.xml index 571c1a2fa..dac27367e 100644 --- a/iam-persistence/pom.xml +++ b/iam-persistence/pom.xml @@ -16,15 +16,16 @@ limitations under the License. --> - + 4.0.0 - it.infn.mw + it.infn.mw.iam-parent iam-parent 1.8.3 + it.infn.mw.iam-persistence iam-persistence jar diff --git a/iam-test-client/pom.xml b/iam-test-client/pom.xml index 8a98b2ca4..6946086c2 100644 --- a/iam-test-client/pom.xml +++ b/iam-test-client/pom.xml @@ -1,15 +1,16 @@ + xmlns:ns="https://maven.apache.org/POM/4.0.0" + xmlns:xsi="https://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="https://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 - it.infn.mw + it.infn.mw.iam-parent iam-parent 1.8.3 + it.infn.mw.iam-test-client iam-test-client jar @@ -39,7 +40,12 @@ org.bouncycastle - bcpkix-jdk15on + bcpkix-jdk18on + + + + org.bouncycastle + bcutil-jdk18on diff --git a/iam-voms-aa/pom.xml b/iam-voms-aa/pom.xml index 76278dee7..f611e50a8 100644 --- a/iam-voms-aa/pom.xml +++ b/iam-voms-aa/pom.xml @@ -16,17 +16,18 @@ limitations under the License. --> - + 4.0.0 - it.infn.mw + it.infn.mw.iam-parent iam-parent 1.8.3 + it.infn.mw.iam-voms-aa iam-voms-aa jar @@ -42,7 +43,7 @@ - ${project.groupId} + it.infn.mw.iam-common iam-common ${project.version} @@ -149,16 +150,21 @@ org.bouncycastle - bcprov-jdk15on + bcprov-jdk18on org.bouncycastle - bcpkix-jdk15on + bcpkix-jdk18on - it.infn.mw + org.bouncycastle + bcutil-jdk18on + + + + it.infn.mw.iam-persistence iam-persistence ${project.version} diff --git a/pom.xml b/pom.xml index 1f57b4e46..8133b3203 100644 --- a/pom.xml +++ b/pom.xml @@ -1,19 +1,18 @@ - - + 4.0.0 org.springframework.boot spring-boot-starter-parent - 2.6.14 + 2.6.15 - it.infn.mw + it.infn.mw.iam-parent iam-parent 1.8.3 pom @@ -30,13 +29,13 @@ 1.16.2 1.3.6.cnaf-20230717 - 2.5.1.RELEASE + 2.5.2.RELEASE 3.3.2 1.0.10.RELEASE - 2.6.14 + 2.6.15 1.6.1 2.5.6 @@ -46,7 +45,7 @@ 4.7.0 3.6.0 3.4.1 - 1.13.1 + 1.13.2 4.4.0 2.2.0 @@ -204,14 +203,20 @@ org.bouncycastle - bcpkix-jdk15on - 1.57 + bcpkix-jdk18on + 1.75 + + + + org.bouncycastle + bcprov-jdk18on + 1.75 org.bouncycastle - bcprov-jdk15on - 1.57 + bcutil-jdk18on + 1.75 From b28850178fd94507d00d4e658a848d30e88cd6fd Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Tue, 18 Jul 2023 17:53:49 +0200 Subject: [PATCH 04/22] Add foreign keys to several tables - access_token - access_token_permissions - refresh_token - approved_site - approved_site_scope - authentication_holder - authentication_holder_authority - authentication_holder_extension - authentication_holder_request_parameter - authentication_holder_resource_id - authentication_holder_response_type - authentication_holder_scope - client_scope - client_claims_redirect_uri - client_redirect_uri - client_contact - client_default_acr_value - client_post_logout_redirect_uri - client_request_uri - token_scope --- .../scim/updater/UsernameUpdaterTests.java | 1 + .../db/migration/h2/V96__add_foreign_keys.sql | 87 +++++++++++++++++++ .../migration/mysql/V96__add_foreign_keys.sql | 87 +++++++++++++++++++ .../V100000_2___test_data_orphan_tokens.sql | 27 ------ 4 files changed, 175 insertions(+), 27 deletions(-) create mode 100644 iam-persistence/src/main/resources/db/migration/h2/V96__add_foreign_keys.sql create mode 100644 iam-persistence/src/main/resources/db/migration/mysql/V96__add_foreign_keys.sql delete mode 100644 iam-persistence/src/main/resources/db/migration/test/V100000_2___test_data_orphan_tokens.sql diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/updater/UsernameUpdaterTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/updater/UsernameUpdaterTests.java index 5db3d72ba..da07fba5f 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/updater/UsernameUpdaterTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/scim/updater/UsernameUpdaterTests.java @@ -71,6 +71,7 @@ public class UsernameUpdaterTests extends TestTokensUtils { public void setup() { clearAllTokens(); mockOAuth2Filter.cleanupSecurityContext(); + accessTokenRepository.deleteAll(); } @After 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 new file mode 100644 index 000000000..bb5853474 --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/h2/V96__add_foreign_keys.sql @@ -0,0 +1,87 @@ +-- TOKEN_SCOPE + +DELETE from token_scope where owner_id not in (select id from access_token); +ALTER TABLE token_scope ALTER COLUMN owner_id SET NOT NULL; +ALTER TABLE token_scope ALTER COLUMN scope SET NOT NULL; +ALTER TABLE token_scope ADD FOREIGN KEY (owner_id) REFERENCES access_token (id) ON DELETE CASCADE; + +-- CLIENT_DETAILS related TABLES + +DELETE FROM client_request_uri WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_request_uri ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_post_logout_redirect_uri WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_post_logout_redirect_uri ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_default_acr_value WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_default_acr_value ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_contact WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_contact ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_redirect_uri WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_redirect_uri ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_claims_redirect_uri WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_claims_redirect_uri ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_scope WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_scope ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +-- AUTHENTICATION HOLDER and related + +DELETE FROM authentication_holder_scope WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_scope ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_response_type WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_response_type ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_resource_id WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_resource_id ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_request_parameter WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_request_parameter ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_extension WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_extension ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_authority WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_authority ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder +WHERE id NOT IN (SELECT auth_holder_id FROM access_token) +AND id NOT IN (SELECT auth_holder_id FROM refresh_token) +AND id NOT IN (SELECT auth_holder_id FROM authorization_code); + +ALTER TABLE authentication_holder ADD FOREIGN KEY (user_auth_id) REFERENCES saved_user_auth (id) ON DELETE CASCADE; +ALTER TABLE authentication_holder ADD FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE CASCADE; + +-- ACCESS TOKEN TABLE and related + +DELETE FROM access_token_permissions WHERE access_token_id NOT IN (SELECT id FROM access_token); +DELETE FROM access_token_permissions WHERE permission_id NOT IN (SELECT id FROM permission); + +ALTER TABLE access_token_permissions ADD PRIMARY KEY (access_token_id, permission_id); +ALTER TABLE access_token_permissions ADD FOREIGN KEY (access_token_id) REFERENCES access_token (id) ON DELETE CASCADE; +ALTER TABLE access_token_permissions ADD FOREIGN KEY (permission_id) REFERENCES permission (id) ON DELETE CASCADE; + +DELETE FROM access_token WHERE refresh_token_id NOT IN (SELECT id FROM refresh_token); +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 FOREIGN KEY (refresh_token_id) REFERENCES refresh_token (id) ON DELETE SET NULL; +ALTER TABLE access_token ADD FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE SET NULL; +ALTER TABLE access_token ADD FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE SET NULL; + +-- REFRESH TOKEN + +DELETE FROM refresh_token WHERE client_id NOT IN (SELECT id FROM client_details); +ALTER TABLE refresh_token ADD FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE SET NULL; + +-- APPROVED SITE + +DELETE FROM approved_site WHERE client_id NOT IN (SELECT id FROM client_details); +ALTER TABLE approved_site ADD FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE SET NULL; + +DELETE FROM approved_site_scope WHERE owner_id NOT IN (SELECT id FROM approved_site); +ALTER TABLE approved_site_scope ADD 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 new file mode 100644 index 000000000..ea0e804b7 --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/mysql/V96__add_foreign_keys.sql @@ -0,0 +1,87 @@ +-- TOKEN_SCOPE + +DELETE from token_scope where owner_id not in (select id from access_token); +ALTER TABLE token_scope MODIFY COLUMN owner_id bigint(20) NOT NULL; +ALTER TABLE token_scope MODIFY COLUMN scope varchar(2048) NOT NULL; +ALTER TABLE token_scope ADD FOREIGN KEY (owner_id) REFERENCES access_token (id) ON DELETE CASCADE; + +-- CLIENT_DETAILS related TABLES + +DELETE FROM client_request_uri WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_request_uri ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_post_logout_redirect_uri WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_post_logout_redirect_uri ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_default_acr_value WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_default_acr_value ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_contact WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_contact ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_redirect_uri WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_redirect_uri ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_claims_redirect_uri WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_claims_redirect_uri ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +DELETE FROM client_scope WHERE owner_id NOT IN (SELECT id FROM client_details); +ALTER TABLE client_scope ADD FOREIGN KEY (owner_id) REFERENCES client_details (id) ON DELETE CASCADE; + +-- AUTHENTICATION HOLDER and related + +DELETE FROM authentication_holder_scope WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_scope ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_response_type WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_response_type ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_resource_id WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_resource_id ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_request_parameter WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_request_parameter ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_extension WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_extension ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder_authority WHERE owner_id NOT IN (SELECT id FROM authentication_holder); +ALTER TABLE authentication_holder_authority ADD FOREIGN KEY (owner_id) REFERENCES authentication_holder (id) ON DELETE CASCADE; + +DELETE FROM authentication_holder +WHERE id NOT IN (SELECT auth_holder_id FROM access_token) +AND id NOT IN (SELECT auth_holder_id FROM refresh_token) +AND id NOT IN (SELECT auth_holder_id FROM authorization_code); + +ALTER TABLE authentication_holder ADD FOREIGN KEY (user_auth_id) REFERENCES saved_user_auth (id) ON DELETE CASCADE; +ALTER TABLE authentication_holder ADD FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE CASCADE; + +-- ACCESS TOKEN TABLE and related + +DELETE FROM access_token_permissions WHERE access_token_id NOT IN (SELECT id FROM access_token); +DELETE FROM access_token_permissions WHERE permission_id NOT IN (SELECT id FROM permission); + +ALTER TABLE access_token_permissions ADD PRIMARY KEY (access_token_id, permission_id); +ALTER TABLE access_token_permissions ADD FOREIGN KEY (access_token_id) REFERENCES access_token (id) ON DELETE CASCADE; +ALTER TABLE access_token_permissions ADD FOREIGN KEY (permission_id) REFERENCES permission (id) ON DELETE CASCADE; + +DELETE FROM access_token WHERE refresh_token_id NOT IN (SELECT id FROM refresh_token); +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 FOREIGN KEY (refresh_token_id) REFERENCES refresh_token (id) ON DELETE SET NULL; +ALTER TABLE access_token ADD FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE SET NULL; +ALTER TABLE access_token ADD FOREIGN KEY (auth_holder_id) REFERENCES authentication_holder (id) ON DELETE SET NULL; + +-- REFRESH TOKEN + +DELETE FROM refresh_token WHERE client_id NOT IN (SELECT id FROM client_details); +ALTER TABLE refresh_token ADD FOREIGN KEY (client_id) REFERENCES client_details (id) ON DELETE SET NULL; + +-- APPROVED SITE + +DELETE FROM approved_site WHERE client_id NOT IN (SELECT id FROM client_details); +ALTER TABLE approved_site ADD FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE SET NULL; + +DELETE FROM approved_site_scope WHERE owner_id NOT IN (SELECT id FROM approved_site); +ALTER TABLE approved_site_scope ADD FOREIGN KEY (owner_id) REFERENCES approved_site (id) ON DELETE CASCADE; diff --git a/iam-persistence/src/main/resources/db/migration/test/V100000_2___test_data_orphan_tokens.sql b/iam-persistence/src/main/resources/db/migration/test/V100000_2___test_data_orphan_tokens.sql deleted file mode 100644 index 6db4a1f75..000000000 --- a/iam-persistence/src/main/resources/db/migration/test/V100000_2___test_data_orphan_tokens.sql +++ /dev/null @@ -1,27 +0,0 @@ -INSERT INTO authentication_holder (id, user_auth_id, approved, redirect_uri, client_id) VALUES -(1, 1, TRUE, NULL, 'token-exchange-subject'); - -INSERT INTO saved_user_auth_authority (owner_id, authority) VALUES -(1, 'ROLE_USER'); - -INSERT INTO authentication_holder_scope (owner_id, scope) VALUES -(1, 'openid'), -(1, 'offline_access'), -(1, 'profile'); - -INSERT INTO authentication_holder_extension (owner_id, extension, VAL) VALUES -(1, 'AUTH_TIMESTAMP', TIMESTAMPADD(DAY, 1, CURRENT_TIMESTAMP())); - -INSERT INTO authentication_holder_request_parameter (owner_id, param, val) VALUES -(1, 'scope', 'openid profile offline_access'), -(1, 'grant_type', 'password'), -(1, 'username', 'vianello'); - -INSERT INTO saved_user_auth (id, name, authenticated, source_class) VALUES -(1, 'vianello', TRUE, 'org.springframework.security.authentication.UsernamePasswordAuthenticationToken'); - -INSERT INTO access_token (id, token_value, expiration, token_type, refresh_token_id, client_id, auth_holder_id, id_token_id, approved_site_id) VALUES -(1, 'eyJraWQiOiJyc2ExIiwiYWxnIjoiUlMyNTYifQ.eyJzdWIiOiI4MGU1ZmI4ZC1iN2M4LTQ1MWEtODliYS0zNDZhZTI3OGE2NmYiLCJpc3MiOiJodHRwOlwvXC9sb2NhbGhvc3Q6ODA4MFwvIiwiZXhwIjoxNTQyODEwNzYwLCJpYXQiOjE1NDI4MDcxNjAsImp0aSI6ImEyNTJjMmE5LWFhZTEtNDNmZi04ZjNlLTAwY2JkODU0MTUwYSJ9.hm5GhHd1FOeeUkndGFKL8r8rNcpcmS_XFDyB6a4LFUO9uLqhC08-d1qDkpesg6MKTeTBuygA4ihX6khc8PGdfZRAtfQiYQJNqGJ72nOKZ-MNKBNqn0ztVEXvu9QTeGMKSyhFSOc2sScclZtCwUCIQfFXJsa_XjzpRdE_DuYOP0w', TIMESTAMPADD(DAY, 1, CURRENT_TIMESTAMP()), 'Bearer', 1, 9, 1, NULL, NULL); - -INSERT INTO refresh_token (id, token_value, expiration, auth_holder_id, client_id) VALUES -(1, 'eyJhbGciOiJub25lIn0.eyJqdGkiOiIwNDE4YWNkMi1hNWY3LTQ3YWItYTljZS1mZDVlZWYzNjI0MjcifQ.', NULL, 1, 9); From 5957954b780b347b6676616ecf27cf83da464aec Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Fri, 21 Jul 2023 14:24:38 +0200 Subject: [PATCH 05/22] Apply tidy maven plugin to avoid annoying cvc-elt.1.a warning --- iam-common/pom.xml | 23 ++++----- iam-login-service/pom.xml | 103 +++++++++++++++++++------------------- iam-persistence/pom.xml | 6 +-- iam-test-client/pom.xml | 6 +-- iam-voms-aa/pom.xml | 5 +- pom.xml | 56 ++++++++++----------- 6 files changed, 95 insertions(+), 104 deletions(-) diff --git a/iam-common/pom.xml b/iam-common/pom.xml index 80ad43735..3abfe3255 100644 --- a/iam-common/pom.xml +++ b/iam-common/pom.xml @@ -1,8 +1,7 @@ - + 4.0.0 + it.infn.mw.iam-parent iam-parent @@ -19,8 +18,16 @@ true - + + + it.infn.mw.iam-persistence + iam-persistence + ${project.version} + + + + @@ -33,12 +40,4 @@ - - - - it.infn.mw.iam-persistence - iam-persistence - ${project.version} - - diff --git a/iam-login-service/pom.xml b/iam-login-service/pom.xml index fe3041d79..1ab52bc39 100644 --- a/iam-login-service/pom.xml +++ b/iam-login-service/pom.xml @@ -16,10 +16,9 @@ limitations under the License. --> - + 4.0.0 + it.infn.mw.iam-parent iam-parent @@ -398,6 +397,7 @@ + iam-login-service @@ -412,6 +412,54 @@ src/main/webapp + + + + com.mycila + license-maven-plugin + +
${project.parent.basedir}/LICENSE
+ + .jslintrc + **/*.vm + **/*.tag + **/Dockerfile + **/Dockerfile.prod + **/resources/iam/js/datepicker/*.js + **/resources/js/lib/*.js + **/resources/js/*.js + **/resources/template/*.html + **/resources/iam-banner.txt + **/resources/css/*.css + **/resources/iam/css/ionicons/ionicons.min.css + **/*.template.html + **/WEB-INF/views/*.jsp + **/*.jks + **/*.pem + **/*.p12 + **/*.crt + **/*.key + **/*.jwks + **/*.factorypath + **/*.ftl + **/*.woff + **/*.woff2 + **/ddl.sql + + true + + + + check-headers + validate + + check + + + + + + org.apache.maven.plugins @@ -487,54 +535,5 @@ - - - - - com.mycila - license-maven-plugin - -
${project.parent.basedir}/LICENSE
- - .jslintrc - **/*.vm - **/*.tag - **/Dockerfile - **/Dockerfile.prod - **/resources/iam/js/datepicker/*.js - **/resources/js/lib/*.js - **/resources/js/*.js - **/resources/template/*.html - **/resources/iam-banner.txt - **/resources/css/*.css - **/resources/iam/css/ionicons/ionicons.min.css - **/*.template.html - **/WEB-INF/views/*.jsp - **/*.jks - **/*.pem - **/*.p12 - **/*.crt - **/*.key - **/*.jwks - **/*.factorypath - **/*.ftl - **/*.woff - **/*.woff2 - **/ddl.sql - - true - - - - check-headers - validate - - check - - - - - - diff --git a/iam-persistence/pom.xml b/iam-persistence/pom.xml index dac27367e..482df8699 100644 --- a/iam-persistence/pom.xml +++ b/iam-persistence/pom.xml @@ -16,15 +16,15 @@ limitations under the License. --> - + 4.0.0 + it.infn.mw.iam-parent iam-parent 1.8.3 + it.infn.mw.iam-persistence iam-persistence jar diff --git a/iam-test-client/pom.xml b/iam-test-client/pom.xml index 6946086c2..61fa5037c 100644 --- a/iam-test-client/pom.xml +++ b/iam-test-client/pom.xml @@ -1,9 +1,7 @@ - + 4.0.0 + it.infn.mw.iam-parent iam-parent diff --git a/iam-voms-aa/pom.xml b/iam-voms-aa/pom.xml index f611e50a8..a3d851159 100644 --- a/iam-voms-aa/pom.xml +++ b/iam-voms-aa/pom.xml @@ -16,9 +16,7 @@ limitations under the License. --> - + 4.0.0 @@ -238,7 +236,6 @@ - org.springframework.boot diff --git a/pom.xml b/pom.xml index 8133b3203..ea2cfa729 100644 --- a/pom.xml +++ b/pom.xml @@ -1,23 +1,45 @@ - + 4.0.0 + org.springframework.boot spring-boot-starter-parent - 2.6.15 - + + it.infn.mw.iam-parent iam-parent 1.8.3 pom + INDIGO Identity and Access Manager (IAM) - Parent POM + + iam-common + iam-persistence + iam-voms-aa + iam-login-service + iam-test-client + + + + + cnaf-releases + CNAF releases + https://repo.cloud.cnaf.infn.it/repository/cnaf-releases/ + + + + cnaf-snapshots + CNAF snapshots + https://repo.cloud.cnaf.infn.it/repository/cnaf-snapshots/ + + + ${project.version}-${git.commit.id.abbrev} @@ -67,14 +89,6 @@ iam-persistence/**/*,iam-test-client/**/*,iam-test-protected-resource/**/*,iam-common/** - - iam-common - iam-persistence - iam-voms-aa - iam-login-service - iam-test-client - - @@ -277,7 +291,6 @@ - @@ -292,7 +305,6 @@ - org.apache.maven.plugins @@ -415,18 +427,4 @@ - - - - cnaf-releases - CNAF releases - https://repo.cloud.cnaf.infn.it/repository/cnaf-releases/ - - - - cnaf-snapshots - CNAF snapshots - https://repo.cloud.cnaf.infn.it/repository/cnaf-snapshots/ - - From f148e9917066c4a3d012734fab9796be0bdd21f7 Mon Sep 17 00:00:00 2001 From: Stefano Date: Fri, 21 Jul 2023 14:41:26 +0200 Subject: [PATCH 06/22] Avoid upper case characters into VO names (#616) --- .../mw/voms/properties/VomsProperties.java | 2 + .../VomsPropertiesValidatorTests.java | 73 +++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 iam-voms-aa/src/test/java/it/infn/mw/voms/properties/VomsPropertiesValidatorTests.java diff --git a/iam-voms-aa/src/main/java/it/infn/mw/voms/properties/VomsProperties.java b/iam-voms-aa/src/main/java/it/infn/mw/voms/properties/VomsProperties.java index f1be2a41d..8835249e7 100644 --- a/iam-voms-aa/src/main/java/it/infn/mw/voms/properties/VomsProperties.java +++ b/iam-voms-aa/src/main/java/it/infn/mw/voms/properties/VomsProperties.java @@ -19,6 +19,7 @@ import javax.validation.Valid; import javax.validation.constraints.NotBlank; +import javax.validation.constraints.Pattern; import javax.validation.constraints.Positive; import org.springframework.boot.context.properties.ConfigurationProperties; @@ -103,6 +104,7 @@ public void setRefreshIntervalSec(long refreshIntervalSec) { public static class VOMSAAProperties { @NotBlank + @Pattern(regexp = "^[a-z][a-z0-9\\-]*(\\.[a-z][a-z0-9\\-]*)*$") private String voName; @NotBlank diff --git a/iam-voms-aa/src/test/java/it/infn/mw/voms/properties/VomsPropertiesValidatorTests.java b/iam-voms-aa/src/test/java/it/infn/mw/voms/properties/VomsPropertiesValidatorTests.java new file mode 100644 index 000000000..cf09d474a --- /dev/null +++ b/iam-voms-aa/src/test/java/it/infn/mw/voms/properties/VomsPropertiesValidatorTests.java @@ -0,0 +1,73 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.voms.properties; + +import static org.junit.Assert.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Set; + +import javax.validation.ConstraintViolation; +import javax.validation.Validation; +import javax.validation.Validator; +import javax.validation.ValidatorFactory; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import it.infn.mw.voms.properties.VomsProperties.VOMSAAProperties; + +public class VomsPropertiesValidatorTests { + private static ValidatorFactory validatorFactory; + private static Validator validator; + private static final List invalidExampleVONames = List.of("VO-name.test", "vo_name.test", "1vo-name.test", + "_vo-name.test", "vo_name.test", "vo_name.test1"); + private static final String validVOName = "vo1-name.test"; + private static final String validVOHost = "vo-host"; + + @BeforeClass + public static void createValidator() { + validatorFactory = Validation.buildDefaultValidatorFactory(); + validator = validatorFactory.getValidator(); + } + + @AfterClass + public static void close() { + validatorFactory.close(); + } + + @Test + public void invalidVoName() { + VOMSAAProperties vomsaa = new VOMSAAProperties(); + vomsaa.setHost(validVOHost); + for (String voName : invalidExampleVONames) { + vomsaa.setVoName(voName); + Set> violations = validator.validate(vomsaa); + assertFalse(violations.isEmpty()); + } + } + + @Test + public void validVoName() { + VOMSAAProperties vomsaa = new VOMSAAProperties(); + vomsaa.setHost(validVOHost); + vomsaa.setVoName(validVOName); + Set> violations = validator.validate(vomsaa); + assertTrue(violations.isEmpty()); + } +} From 665bc7fd63d41b92b82c78fd4c4338a15b0555d0 Mon Sep 17 00:00:00 2001 From: Federica Agostini Date: Fri, 21 Jul 2023 15:14:47 +0200 Subject: [PATCH 07/22] Add scim_endpoint entry to well-known endpoint (#631) as per "OpenID Connect Profile for SCIM Services" specification: https://openid.net/specs/openid-connect-scim-profile-1_0.html#discoveryMetadata --- .../wellknown/IamWellKnownInfoProvider.java | 11 +++++++--- .../WellKnownConfigurationEndpointTests.java | 20 ++++++++++--------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/wellknown/IamWellKnownInfoProvider.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/wellknown/IamWellKnownInfoProvider.java index 81a604dfb..92d6c73e3 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/wellknown/IamWellKnownInfoProvider.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/wellknown/IamWellKnownInfoProvider.java @@ -57,6 +57,7 @@ public class IamWellKnownInfoProvider implements WellKnownInfoProvider { public static final String AUTHORIZE_ENDPOINT = "authorize"; public static final String TOKEN_ENDPOINT = "token"; public static final String ABOUT_ENDPOINT = "about"; + public static final String SCIM_ENDPOINT = "scim"; private static final List TOKEN_ENDPOINT_AUTH_METHODS = newArrayList( "client_secret_basic", "client_secret_post", "client_secret_jwt", "private_key_jwt", "none"); @@ -111,8 +112,9 @@ public class IamWellKnownInfoProvider implements WellKnownInfoProvider { private final String revocationEndpoint; private final String deviceAuthorizationEndpoint; private final String aboutEndpoint; + private final String scimEndpoint; private Set supportedScopes; - + public IamWellKnownInfoProvider(IamProperties properties, JWTEncryptionAndDecryptionService encService, SystemScopeService scopeService) { @@ -138,6 +140,7 @@ public IamWellKnownInfoProvider(IamProperties properties, revocationEndpoint = buildEndpointUrl(RevocationEndpoint.URL); deviceAuthorizationEndpoint = buildEndpointUrl(DeviceEndpoint.URL); aboutEndpoint = buildEndpointUrl(ABOUT_ENDPOINT); + scimEndpoint = buildEndpointUrl(SCIM_ENDPOINT); updateSupportedScopes(); } @@ -182,13 +185,15 @@ public Map getWellKnownInfo() { result.put("jwks_uri", jwkEndpoint); result.put("registration_endpoint", clientRegistrationEndpoint); - result.put("introspection_endpoint", introspectionEndpoint ); + result.put("introspection_endpoint", introspectionEndpoint); result.put("revocation_endpoint", revocationEndpoint); result.put("device_authorization_endpoint", deviceAuthorizationEndpoint); result.put("op_policy_uri", aboutEndpoint); result.put("op_tos_uri", aboutEndpoint); + result.put("scim_endpoint", scimEndpoint); + result.put("response_types_supported", RESPONSE_TYPES); result.put("grant_types_supported", GRANT_TYPES); @@ -221,7 +226,7 @@ public Map getWellKnownInfo() { result.put("code_challenge_methods_supported", CODE_CHALLENGE_METHODS); result.put("scopes_supported", supportedScopes); - + return result; } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/WellKnownConfigurationEndpointTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/WellKnownConfigurationEndpointTests.java index a306ceadb..83c1b0f65 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/WellKnownConfigurationEndpointTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/WellKnownConfigurationEndpointTests.java @@ -121,7 +121,8 @@ public void testEndpoints() throws Exception { .andExpect(jsonPath("$.introspection_endpoint", is("http://localhost:8080/introspect"))) .andExpect(jsonPath("$.revocation_endpoint", is("http://localhost:8080/revoke"))) .andExpect(jsonPath("$.userinfo_endpoint", is("http://localhost:8080/userinfo"))) - .andExpect(jsonPath("$.jwks_uri", is("http://localhost:8080/jwk"))); + .andExpect(jsonPath("$.jwks_uri", is("http://localhost:8080/jwk"))) + .andExpect(jsonPath("$.scim_endpoint", is("http://localhost:8080/scim"))); } @Test @@ -132,23 +133,24 @@ public void testScopes() throws Exception { .map(SystemScope::getValue) .collect(Collectors.toSet()); - - String responseJson = - mvc.perform(get(endpoint)) + + String responseJson = mvc.perform(get(endpoint)) .andExpect(status().isOk()) .andExpect(jsonPath("$.scopes_supported", notNullValue())) - .andReturn().getResponse().getContentAsString(); + .andReturn() + .getResponse() + .getContentAsString(); + + + ArrayNode scopesSupported = (ArrayNode) mapper.readTree(responseJson).get("scopes_supported"); - - ArrayNode scopesSupported = (ArrayNode) mapper.readTree(responseJson).get("scopes_supported"); - Set returnedScopes = Sets.newHashSet(); Iterator scopesIterator = scopesSupported.iterator(); while (scopesIterator.hasNext()) { returnedScopes.add(scopesIterator.next().asText()); } - + assertTrue(returnedScopes.containsAll(unrestrictedScopes)); } From 6c8bb4fae0fe7b9a4f054f4131701a82002271b5 Mon Sep 17 00:00:00 2001 From: Roberta Miccoli <85555840+rmiccoli@users.noreply.github.com> Date: Fri, 21 Jul 2023 15:25:22 +0200 Subject: [PATCH 08/22] Allow to add certificates with the same subjectDn (#624) Removing unique constraint on issuer_dn column and making subject_dn and issuer_dn case sensitive columns in iam_x509_cert db table, allow to add certificates with the same subject. If the certificate to be added has the same issuer as an already linked certificate, they are considered the same and then the certificate is only updated. Previously, this happened with certificates having the same subject. Tests updated as a consequence. --- .../DefaultAccountLinkingService.java | 4 +- .../X509AuthenticationIntegrationTests.java | 85 ++++++++++ .../test/ext_authn/x509/X509TestSupport.java | 146 +++++++++++++++++- .../persistence/model/IamX509Certificate.java | 17 +- .../migration/mysql/V94__alter_x509_table.sql | 2 + 5 files changed, 239 insertions(+), 15 deletions(-) create mode 100644 iam-persistence/src/main/resources/db/migration/mysql/V94__alter_x509_table.sql diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/DefaultAccountLinkingService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/DefaultAccountLinkingService.java index 1820f5519..15c06826b 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/DefaultAccountLinkingService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/account_linking/DefaultAccountLinkingService.java @@ -151,14 +151,12 @@ public void linkX509Certificate(Principal authenticatedUser, Optional linkedCert = userAccount.getX509Certificates() .stream() - .filter(c -> c.getSubjectDn().equals(x509Credential.getSubject())) + .filter(c -> c.getSubjectDn().equals(x509Credential.getSubject()) && c.getIssuerDn().equals(x509Credential.getIssuer())) .findAny(); if (linkedCert.isPresent()) { linkedCert.ifPresent(c -> { - c.setSubjectDn(x509Credential.getSubject()); - c.setIssuerDn(x509Credential.getIssuer()); c.setCertificate(x509Credential.getCertificateChainPemString()); c.setLastUpdateTime(new Date()); }); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509AuthenticationIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509AuthenticationIntegrationTests.java index 0993b77a1..82ed97a05 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509AuthenticationIntegrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509AuthenticationIntegrationTests.java @@ -27,6 +27,8 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.authenticated; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; @@ -39,7 +41,9 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.time.Instant; +import java.util.Arrays; import java.util.Date; +import java.util.HashSet; import org.junit.Test; import org.junit.runner.RunWith; @@ -55,6 +59,7 @@ import it.infn.mw.iam.IamLoginService; import it.infn.mw.iam.authn.x509.IamX509AuthenticationCredential; import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.model.IamX509Certificate; import it.infn.mw.iam.persistence.repository.IamAccountRepository; import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; import junit.framework.AssertionFailedError; @@ -195,8 +200,71 @@ public void testx509AccountLinking() throws Exception { .orElseThrow(() -> new AssertionFailedError("Expected user linked to certificate not found")); assertThat(linkedAccount.getLastUpdateTime().after(lastUpdateTime), is(true)); + assertThat(linkedAccount.getX509Certificates().size(), is(1)); + + MockHttpSession session1 = loginAsTestUserWithTest1Cert(mvc); + + IamX509AuthenticationCredential credential1 = + (IamX509AuthenticationCredential) session1.getAttribute(X509_CREDENTIAL_SESSION_KEY); + + assertThat(credential1.getSubject(), equalTo(TEST_0_SUBJECT)); + assertThat(credential1.getIssuer(), equalTo(TEST_NEW_ISSUER)); + + String confirmationMsg = + String.format("Certificate '%s' linked succesfully", credential1.getSubject()); + + mvc.perform(post("/iam/account-linking/X509").session(session1).with(csrf().asHeader())) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/dashboard")) + .andExpect( + flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMsg))); + + assertThat(linkedAccount.getX509Certificates().size(), is(2)); + } + @Test + public void testx509AccountLinkingWithDifferentSubjectAndIssuer() throws Exception { + + MockHttpSession session = loginAsTestUserWithTest0Cert(mvc); + IamX509AuthenticationCredential credential = + (IamX509AuthenticationCredential) session.getAttribute(X509_CREDENTIAL_SESSION_KEY); + + assertThat(credential.getSubject(), equalTo(TEST_0_SUBJECT)); + + String confirmationMessage = + String.format("Certificate '%s' linked succesfully", credential.getSubject()); + + mvc.perform(post("/iam/account-linking/X509").session(session).with(csrf().asHeader())) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/dashboard")) + .andExpect( + flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMessage))); + + IamAccount linkedAccount = iamAccountRepo.findByCertificateSubject(TEST_0_SUBJECT) + .orElseThrow(() -> new AssertionFailedError("Expected user linked to certificate not found")); + + assertThat(linkedAccount.getUsername(), equalTo("test")); + + MockHttpSession session1 = loginAsTestUserWithTest2Cert(mvc); + + IamX509AuthenticationCredential credential1 = + (IamX509AuthenticationCredential) session1.getAttribute(X509_CREDENTIAL_SESSION_KEY); + + assertThat(credential1.getSubject(), equalTo(TEST_1_SUBJECT)); + assertThat(credential1.getIssuer(), equalTo(TEST_NEW_ISSUER)); + + String confirmationMsg = + String.format("Certificate '%s' linked succesfully", credential1.getSubject()); + + mvc.perform(post("/iam/account-linking/X509").session(session1).with(csrf().asHeader())) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/dashboard")) + .andExpect( + flash().attribute(ACCOUNT_LINKING_DASHBOARD_MESSAGE_KEY, equalTo(confirmationMsg))); + + assertThat(linkedAccount.getX509Certificates().size(), is(2)); + } @Test @WithMockUser(username = "test") @@ -284,4 +352,21 @@ public void testx509AuthNFailsIfDisabledUser() throws Exception { } + @Test + public void testHashAndEqualsMethods() { + + HashSet set1 = new HashSet(Arrays.asList(TEST_0_IAM_X509_CERT, TEST_1_IAM_X509_CERT)); + assertThat(set1.size(), is(2)); + assertNotEquals(TEST_0_IAM_X509_CERT.hashCode(), TEST_1_IAM_X509_CERT.hashCode()); + assertEquals(set1.hashCode(), TEST_0_IAM_X509_CERT.hashCode()+TEST_1_IAM_X509_CERT.hashCode()); + assertNotEquals(TEST_0_IAM_X509_CERT, TEST_1_IAM_X509_CERT); + + HashSet set2 = new HashSet(Arrays.asList(TEST_0_IAM_X509_CERT, TEST_2_IAM_X509_CERT)); + assertThat(set2.size(), is(1)); + assertEquals(TEST_0_IAM_X509_CERT.hashCode(), TEST_2_IAM_X509_CERT.hashCode()); + assertEquals(set2.hashCode(), TEST_0_IAM_X509_CERT.hashCode()); + assertEquals(TEST_0_IAM_X509_CERT, TEST_2_IAM_X509_CERT); + + } + } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509TestSupport.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509TestSupport.java index 7f1c04380..da9cc9704 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509TestSupport.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/x509/X509TestSupport.java @@ -67,6 +67,8 @@ public class X509TestSupport { public static final String TEST_1_SERIAL = "10"; public static final String TEST_1_V_START = "Sep 26 15:39:36 2012 GMT"; public static final String TEST_1_V_END = "Sep 24 15:39:36 2022 GMT"; + + public static final String TEST_NEW_ISSUER = "CN=Test1 CA,O=IGI,C=IT"; public static final String RCAUTH_CA_CERT_PATH = "src/test/resources/x509/rcauth-mock-ca.p12"; public static final String RCAUTH_CA_CERT_PASSWORD = "pass123"; @@ -83,6 +85,7 @@ public class X509TestSupport { protected IamX509Certificate TEST_0_IAM_X509_CERT; protected IamX509Certificate TEST_1_IAM_X509_CERT; + protected IamX509Certificate TEST_2_IAM_X509_CERT; protected PEMCredential TEST_0_PEM_CREDENTIAL; @@ -122,7 +125,14 @@ protected X509TestSupport() { TEST_1_IAM_X509_CERT.setLabel(TEST_1_CERT_LABEL); TEST_1_IAM_X509_CERT.setPrimary(false); - // This is how NGINX encodes certficate in the header + TEST_2_IAM_X509_CERT = new IamX509Certificate(); + TEST_2_IAM_X509_CERT.setCertificate(TEST_1_CERT_STRING); + TEST_2_IAM_X509_CERT.setSubjectDn(TEST_0_SUBJECT); + TEST_2_IAM_X509_CERT.setIssuerDn(TEST_1_ISSUER); + TEST_2_IAM_X509_CERT.setLabel(TEST_1_CERT_LABEL); + TEST_2_IAM_X509_CERT.setPrimary(false); + + // This is how NGINX encodes certificate in the header TEST_0_CERT_STRING_NGINX = TEST_0_CERT_STRING.replace('\n', '\t'); TEST_1_CERT_STRING_NGINX = TEST_1_CERT_STRING.replace('\n', '\t'); @@ -173,6 +183,60 @@ protected MockHttpSession loginAsTestUserWithTest0Cert(MockMvc mvc) throws Excep return session; } + protected MockHttpSession loginAsTestUserWithTest1Cert(MockMvc mvc) throws Exception { + + MockHttpSession session = + (MockHttpSession) mvc.perform(get("/").headers(test1SSLHeadersVerificationSuccess())) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("http://localhost/login")) + .andExpect(MockMvcResultMatchers.request() + .sessionAttribute(X509_CREDENTIAL_SESSION_KEY, notNullValue())) + .andReturn() + .getRequest() + .getSession(); + + session = (MockHttpSession) mvc + .perform(post("/login").session(session) + .param("username", TEST_USERNAME) + .param("password", TEST_PASSWORD) + .param("submit", "Login")) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("http://localhost/")) + .andExpect(authenticated().withUsername("test")) + .andReturn() + .getRequest() + .getSession(); + + return session; + } + + protected MockHttpSession loginAsTestUserWithTest2Cert(MockMvc mvc) throws Exception { + + MockHttpSession session = + (MockHttpSession) mvc.perform(get("/").headers(test2SSLHeadersVerificationSuccess())) + .andExpect(status().isFound()) + .andExpect(redirectedUrl("http://localhost/login")) + .andExpect(MockMvcResultMatchers.request() + .sessionAttribute(X509_CREDENTIAL_SESSION_KEY, notNullValue())) + .andReturn() + .getRequest() + .getSession(); + + session = (MockHttpSession) mvc + .perform(post("/login").session(session) + .param("username", TEST_USERNAME) + .param("password", TEST_PASSWORD) + .param("submit", "Login")) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("http://localhost/")) + .andExpect(authenticated().withUsername("test")) + .andReturn() + .getRequest() + .getSession(); + + return session; + } + protected void linkTest1CertificateToAccount(IamAccount account) { IamX509Certificate test1Cert = new IamX509Certificate(); test1Cert.setPrimary(false); @@ -214,6 +278,14 @@ protected HttpHeaders test0SSLHeadersVerificationSuccess() { return test0SSLHeaders(true, null); } + protected HttpHeaders test1SSLHeadersVerificationSuccess() { + return test1SSLHeaders(true, null); + } + + protected HttpHeaders test2SSLHeadersVerificationSuccess() { + return test2SSLHeaders(true, null); + } + protected HttpHeaders test0SSLHeadersVerificationFailed(String verificationError) { return test0SSLHeaders(false, verificationError); } @@ -254,6 +326,78 @@ private HttpHeaders test0SSLHeaders(boolean verified, String verificationError) return headers; } + private HttpHeaders test2SSLHeaders(boolean verified, String verificationError) { + HttpHeaders headers = new HttpHeaders(); + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.CLIENT_CERT.getHeader(), + TEST_0_CERT_STRING_NGINX); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.SUBJECT.getHeader(), + TEST_1_SUBJECT); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.ISSUER.getHeader(), + TEST_NEW_ISSUER); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.SERIAL.getHeader(), + TEST_0_SERIAL); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.V_START.getHeader(), + TEST_0_V_START); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.V_END.getHeader(), + TEST_0_V_END); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.PROTOCOL.getHeader(), "TLS"); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.SERVER_NAME.getHeader(), + "serverName"); + + if (verified) { + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.VERIFY.getHeader(), + "SUCCESS"); + } else { + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.VERIFY.getHeader(), + "FAILED:" + verificationError); + } + + return headers; + } + + private HttpHeaders test1SSLHeaders(boolean verified, String verificationError) { + HttpHeaders headers = new HttpHeaders(); + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.CLIENT_CERT.getHeader(), + TEST_0_CERT_STRING_NGINX); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.SUBJECT.getHeader(), + TEST_0_SUBJECT); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.ISSUER.getHeader(), + TEST_NEW_ISSUER); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.SERIAL.getHeader(), + TEST_0_SERIAL); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.V_START.getHeader(), + TEST_0_V_START); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.V_END.getHeader(), + TEST_0_V_END); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.PROTOCOL.getHeader(), "TLS"); + + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.SERVER_NAME.getHeader(), + "serverName"); + + if (verified) { + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.VERIFY.getHeader(), + "SUCCESS"); + } else { + headers.add(DefaultX509AuthenticationCredentialExtractor.Headers.VERIFY.getHeader(), + "FAILED:" + verificationError); + } + + return headers; + } + protected void mockHttpRequestWithTest0SSLHeaders(HttpServletRequest request) { Mockito .when(request diff --git a/iam-persistence/src/main/java/it/infn/mw/iam/persistence/model/IamX509Certificate.java b/iam-persistence/src/main/java/it/infn/mw/iam/persistence/model/IamX509Certificate.java index f394cf0ed..8bf487165 100644 --- a/iam-persistence/src/main/java/it/infn/mw/iam/persistence/model/IamX509Certificate.java +++ b/iam-persistence/src/main/java/it/infn/mw/iam/persistence/model/IamX509Certificate.java @@ -19,6 +19,7 @@ import java.io.Serializable; import java.util.Date; +import java.util.Objects; import javax.persistence.CascadeType; import javax.persistence.Column; @@ -51,7 +52,7 @@ public class IamX509Certificate implements IamAccountRef, Serializable { @Column(nullable = false, length = 36) private String label; - @Column(name = "subject_dn", nullable = false, length = 128, unique = true) + @Column(name = "subject_dn", nullable = false, length = 128) private String subjectDn; @Column(name = "issuer_dn", nullable = false, length = 128) @@ -87,12 +88,10 @@ public IamX509Certificate() { @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((subjectDn == null) ? 0 : subjectDn.hashCode()); - return result; + return Objects.hash(issuerDn, subjectDn); } + @Override public boolean equals(Object obj) { if (this == obj) @@ -102,14 +101,10 @@ public boolean equals(Object obj) { if (getClass() != obj.getClass()) return false; IamX509Certificate other = (IamX509Certificate) obj; - if (subjectDn == null) { - if (other.subjectDn != null) - return false; - } else if (!subjectDn.equals(other.subjectDn)) - return false; - return true; + return Objects.equals(issuerDn, other.issuerDn) && Objects.equals(subjectDn, other.subjectDn); } + @Override public IamAccount getAccount() { diff --git a/iam-persistence/src/main/resources/db/migration/mysql/V94__alter_x509_table.sql b/iam-persistence/src/main/resources/db/migration/mysql/V94__alter_x509_table.sql new file mode 100644 index 000000000..b12f395d7 --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/mysql/V94__alter_x509_table.sql @@ -0,0 +1,2 @@ +ALTER TABLE iam_x509_cert MODIFY subject_dn varchar(256) CHARACTER SET latin1 COLLATE latin1_general_cs; +ALTER TABLE iam_x509_cert MODIFY issuer_dn varchar(256) CHARACTER SET latin1 COLLATE latin1_general_cs; \ No newline at end of file From c0aacc65b8aabb44611c0c9e3698581df23e48c6 Mon Sep 17 00:00:00 2001 From: Roberta Miccoli <85555840+rmiccoli@users.noreply.github.com> Date: Fri, 21 Jul 2023 15:26:03 +0200 Subject: [PATCH 09/22] Delete unsupported response types (#610) --- .../db/migration/h2/V95__remove_client_response_type.sql | 2 ++ .../db/migration/mysql/V95__remove_client_response_type.sql | 2 ++ 2 files changed, 4 insertions(+) create mode 100644 iam-persistence/src/main/resources/db/migration/h2/V95__remove_client_response_type.sql create mode 100644 iam-persistence/src/main/resources/db/migration/mysql/V95__remove_client_response_type.sql diff --git a/iam-persistence/src/main/resources/db/migration/h2/V95__remove_client_response_type.sql b/iam-persistence/src/main/resources/db/migration/h2/V95__remove_client_response_type.sql new file mode 100644 index 000000000..315b578f3 --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/h2/V95__remove_client_response_type.sql @@ -0,0 +1,2 @@ +-- Delete unsupported response types +delete from client_response_type where response_type in ('code token id_token', 'code token', 'code id_token', 'token id_token', 'id_token'); \ No newline at end of file diff --git a/iam-persistence/src/main/resources/db/migration/mysql/V95__remove_client_response_type.sql b/iam-persistence/src/main/resources/db/migration/mysql/V95__remove_client_response_type.sql new file mode 100644 index 000000000..315b578f3 --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/mysql/V95__remove_client_response_type.sql @@ -0,0 +1,2 @@ +-- Delete unsupported response types +delete from client_response_type where response_type in ('code token id_token', 'code token', 'code id_token', 'token id_token', 'id_token'); \ No newline at end of file From ed197ea605dd874397f889ca5d3150f8e9268d3f Mon Sep 17 00:00:00 2001 From: Roberta Miccoli <85555840+rmiccoli@users.noreply.github.com> Date: Fri, 21 Jul 2023 15:27:57 +0200 Subject: [PATCH 10/22] Update account AUP via API (#608) Add the possibility to change user's AUP signature time through IAM AUP API. Details: - when AUP signature is present, update the signature time - when AUP signature is not present, create it - when account is not found, return 404 not 500 --- .../iam/api/aup/AupSignatureController.java | 50 +++++++++++++------ .../api/aup/AupSignatureIntegrationTests.java | 43 +++++++++++++++- 2 files changed, 77 insertions(+), 16 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/aup/AupSignatureController.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/aup/AupSignatureController.java index 3303fc984..f7db70d31 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/aup/AupSignatureController.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/aup/AupSignatureController.java @@ -18,13 +18,17 @@ import java.util.Date; import java.util.function.Supplier; +import javax.security.auth.login.AccountNotFoundException; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; import org.springframework.http.HttpStatus; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.transaction.annotation.Transactional; +import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.ResponseStatus; @@ -62,18 +66,18 @@ public AupSignatureController(AupSignatureConverter conv, AccountUtils utils, this.eventPublisher = publisher; } - private Supplier accountNotFoundException(String message) { - return () -> new IllegalStateException(message); + private Supplier accountNotFoundException(String message) { + return () -> new AccountNotFoundException(message); } - - private Supplier signatureNotFound(IamAccount account){ + + private Supplier signatureNotFound(IamAccount account) { return () -> new AupSignatureNotFoundError(account); } @RequestMapping(value = "/iam/aup/signature", method = RequestMethod.POST) - @PreAuthorize("#oauth2.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_USER')") + @PreAuthorize("#iam.hasDashboardRole('ROLE_USER')") @ResponseStatus(code = HttpStatus.CREATED) - public void signAup() { + public void signAup() throws AccountNotFoundException { IamAccount account = accountUtils.getAuthenticatedUserAccount() .orElseThrow(accountNotFoundException("Account not found for authenticated user")); @@ -84,28 +88,38 @@ public void signAup() { } @RequestMapping(value = "/iam/aup/signature", method = RequestMethod.GET) - @PreAuthorize("#oauth2.hasScope('iam:admin.read') or #iam.hasDashboardRole('ROLE_USER')") - public AupSignatureDTO getSignature() { + @PreAuthorize("#iam.hasDashboardRole('ROLE_USER')") + public AupSignatureDTO getSignature() throws AccountNotFoundException { IamAccount account = accountUtils.getAuthenticatedUserAccount() .orElseThrow(accountNotFoundException("Account not found for authenticated user")); - IamAupSignature sig = signatureRepo.findSignatureForAccount(account) - .orElseThrow(signatureNotFound(account)); + IamAupSignature sig = + signatureRepo.findSignatureForAccount(account).orElseThrow(signatureNotFound(account)); return signatureConverter.dtoFromEntity(sig); } @RequestMapping(value = "/iam/aup/signature/{accountId}", method = RequestMethod.GET) @PreAuthorize("#oauth2.hasScope('iam:admin.read') or #iam.hasAnyDashboardRole('ROLE_ADMIN', 'ROLE_GM') or #iam.isUser(#accountId)") - public AupSignatureDTO getSignatureForAccount(@PathVariable String accountId) { + public AupSignatureDTO getSignatureForAccount(@PathVariable String accountId) throws AccountNotFoundException { IamAccount account = accountUtils.getByAccountId(accountId) .orElseThrow(accountNotFoundException("Account not found for id: " + accountId)); - - IamAupSignature sig = signatureRepo.findSignatureForAccount(account) - .orElseThrow(signatureNotFound(account)); - return signatureConverter.dtoFromEntity(sig); + IamAupSignature sig = + signatureRepo.findSignatureForAccount(account).orElseThrow(signatureNotFound(account)); + + return signatureConverter.dtoFromEntity(sig); + } + + @RequestMapping(value = "/iam/aup/signature/{accountId}", method = RequestMethod.PATCH) + @PreAuthorize("#oauth2.hasScope('iam:admin.write')") + public void setSignatureForAccount(@PathVariable String accountId, + @RequestBody @Validated AupSignatureDTO dto) throws AccountNotFoundException { + IamAccount account = accountUtils.getByAccountId(accountId) + .orElseThrow(accountNotFoundException("Account not found for id: " + accountId)); + + signatureRepo.createSignatureForAccount(account, dto.getSignatureTime()); } @ResponseStatus(value = HttpStatus.NOT_FOUND) @@ -113,4 +127,10 @@ public AupSignatureDTO getSignatureForAccount(@PathVariable String accountId) { public ErrorDTO notFoundError(Exception ex) { return ErrorDTO.fromString(ex.getMessage()); } + + @ResponseStatus(value = HttpStatus.NOT_FOUND) + @ExceptionHandler(AccountNotFoundException.class) + public ErrorDTO accountNotFoundError(Exception ex) { + return ErrorDTO.fromString(ex.getMessage()); + } } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/aup/AupSignatureIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/aup/AupSignatureIntegrationTests.java index 4ac0802a5..d094be313 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/aup/AupSignatureIntegrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/aup/AupSignatureIntegrationTests.java @@ -15,12 +15,13 @@ */ package it.infn.mw.iam.test.api.aup; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.springframework.http.MediaType.APPLICATION_JSON; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch; 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; @@ -47,6 +48,7 @@ import it.infn.mw.iam.test.util.DateEqualModulo1Second; import it.infn.mw.iam.test.util.MockTimeProvider; import it.infn.mw.iam.test.util.WithAnonymousUser; +import it.infn.mw.iam.test.util.WithMockOAuthUser; import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; import it.infn.mw.iam.test.util.oauth.MockOAuth2Filter; @@ -235,4 +237,43 @@ public void adminUserCanSeeOtherUserAup() throws Exception { get("/iam/aup/signature/" + TEST_USER_UUID).with(user("admin").roles("USER", "ADMIN"))) .andExpect(status().isOk()); } + + @Test + @WithMockOAuthUser(user = "admin", authorities = "ROLE_ADMIN", scopes = {"iam:admin.read", "iam:admin.write"}) + public void updateAupWithRightScope() throws Exception { + IamAup aup = buildDefaultAup(); + aupRepo.save(aup); + + mvc.perform(get("/iam/aup/signature/{accountId}", TEST_USER_UUID)) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error", equalTo("AUP signature not found for user 'test'"))); + + Date newEndTime = new Date(); + AupSignatureDTO dto = new AupSignatureDTO(); + dto.setSignatureTime(newEndTime); + + mvc + .perform(patch("/iam/aup/signature/{accountId}", TEST_USER_UUID).content(mapper.writeValueAsString(dto)) + .contentType(APPLICATION_JSON)) + .andExpect(OK); + + String sigString = mvc.perform(get("/iam/aup/signature/{accountId}", TEST_USER_UUID)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.aup").exists()) + .andExpect(jsonPath("$.account.uuid").exists()) + .andExpect(jsonPath("$.account.username", equalTo("test"))) + .andExpect(jsonPath("$.account.name", equalTo("Test User"))) + .andExpect(jsonPath("$.signatureTime").exists()) + .andReturn() + .getResponse() + .getContentAsString(); + + AupSignatureDTO sig = mapper.readValue(sigString, AupSignatureDTO.class); + assertThat(sig.getSignatureTime(), new DateEqualModulo1Second(newEndTime)); + + mvc.perform(get("/iam/aup/signature/{accountId}", "1234")) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.error", equalTo("Account not found for id: 1234"))); + + } } From 7f467c23a7f2ce902ac80f9dc20bc8212b1aa948 Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Fri, 21 Jul 2023 16:18:52 +0200 Subject: [PATCH 11/22] Add empty migration to h2 --- .../src/main/resources/db/migration/h2/V94__alter_x509_table.sql | 1 + 1 file changed, 1 insertion(+) create mode 100644 iam-persistence/src/main/resources/db/migration/h2/V94__alter_x509_table.sql diff --git a/iam-persistence/src/main/resources/db/migration/h2/V94__alter_x509_table.sql b/iam-persistence/src/main/resources/db/migration/h2/V94__alter_x509_table.sql new file mode 100644 index 000000000..3cd0bd8df --- /dev/null +++ b/iam-persistence/src/main/resources/db/migration/h2/V94__alter_x509_table.sql @@ -0,0 +1 @@ +-- Nothing to do because it's already case sensitive \ No newline at end of file From 425e43c0356ababd291811aee08d9443762dc511 Mon Sep 17 00:00:00 2001 From: Federica Agostini Date: Mon, 24 Jul 2023 21:04:25 +0200 Subject: [PATCH 12/22] Enable Redis caching of scope matchers and well-known endpoint (#633) * Replace Google cache for scope matchers with Spring one * Make Redis cache configurable with property redis-cache.enabled * Add well-known to Redis cache --- .../java/it/infn/mw/iam/IamLoginService.java | 5 +- .../client/service/DefaultClientService.java | 19 +-- .../it/infn/mw/iam/config/CacheConfig.java | 36 ++++- .../java/it/infn/mw/iam/config/IamConfig.java | 2 +- .../mw/iam/config/MitreServicesConfig.java | 2 +- .../mw/iam/config/RedisCacheProperties.java | 35 +++++ .../matchers/DefaultScopeMatcherRegistry.java | 28 +--- .../scope/matchers/RegexpScopeMatcher.java | 1 + .../oauth/scope/matchers/ScopeMatcher.java | 4 +- .../ScopeMatcherOAuthRequestValidator.java | 25 +--- .../matchers/StringEqualsScopeMatcher.java | 1 + .../matchers/StructuredPathScopeMatcher.java | 2 + .../main/resources/application-redis-test.yml | 5 +- .../src/main/resources/application.yml | 3 + .../matchers/ScopeMatcherCacheTests.java | 17 +++ .../ScopeMatcherExternalCacheTests.java | 141 ++++++++++++++++++ 16 files changed, 261 insertions(+), 65 deletions(-) create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/config/RedisCacheProperties.java create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherExternalCacheTests.java diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/IamLoginService.java b/iam-login-service/src/main/java/it/infn/mw/iam/IamLoginService.java index 42d0af4bd..df25c2f76 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/IamLoginService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/IamLoginService.java @@ -17,9 +17,9 @@ import org.mitre.discovery.web.DiscoveryEndpoint; import org.mitre.oauth2.web.CorsFilter; +import org.mitre.oauth2.web.OAuthConfirmationController; import org.mitre.openid.connect.web.DynamicClientRegistrationEndpoint; import org.mitre.openid.connect.web.JWKSetPublishingEndpoint; -import org.mitre.oauth2.web.OAuthConfirmationController; import org.mitre.openid.connect.web.RootController; import org.mitre.openid.connect.web.UserInfoEndpoint; import org.springframework.boot.SpringApplication; @@ -27,6 +27,7 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.autoconfigure.h2.H2ConsoleAutoConfiguration; import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration; +import org.springframework.cache.annotation.EnableCaching; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.FilterType; @@ -77,7 +78,7 @@ @ComponentScan.Filter(type=FilterType.ASSIGNABLE_TYPE, value=OAuthConfirmationController.class) }) - +@EnableCaching @EnableAutoConfiguration( exclude = { SecurityAutoConfiguration.class, diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientService.java index f415f20eb..51e75a488 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientService.java @@ -24,6 +24,7 @@ import org.mitre.oauth2.model.OAuth2AccessTokenEntity; import org.mitre.oauth2.service.OAuth2TokenEntityService; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.cache.annotation.CacheEvict; import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -32,7 +33,7 @@ import org.springframework.transaction.annotation.Transactional; import it.infn.mw.iam.audit.events.client.ClientCreatedEvent; -import it.infn.mw.iam.core.oauth.scope.matchers.ScopeMatcherOAuthRequestValidator; +import it.infn.mw.iam.core.oauth.scope.matchers.DefaultScopeMatcherRegistry; import it.infn.mw.iam.persistence.model.IamAccount; import it.infn.mw.iam.persistence.model.IamAccountClient; import it.infn.mw.iam.persistence.repository.client.ClientSpecs; @@ -52,8 +53,6 @@ public class DefaultClientService implements ClientService { private ApplicationEventPublisher eventPublisher; - private OAuth2RequestValidator requestValidator; - private OAuth2TokenEntityService tokenService; @Autowired @@ -64,7 +63,6 @@ public DefaultClientService(Clock clock, IamClientRepository clientRepo, this.clientRepo = clientRepo; this.accountClientRepo = accountClientRepo; this.eventPublisher = eventPublisher; - this.requestValidator = requestValidator; this.tokenService = tokenService; } @@ -103,9 +101,9 @@ public ClientDetailsEntity unlinkClientFromAccount(ClientDetailsEntity client, I } @Override + @CacheEvict(cacheNames = DefaultScopeMatcherRegistry.SCOPE_CACHE_KEY, key = "{#client?.id}") public ClientDetailsEntity updateClient(ClientDetailsEntity client) { - ((ScopeMatcherOAuthRequestValidator) requestValidator).invalidateScope(client); return clientRepo.save(client); } @@ -147,14 +145,11 @@ private void deleteTokensByClient(ClientDetailsEntity client) { // delete all valid access tokens (exclude registration and resource tokens) tokenService.getAccessTokensForClient(client) .stream() - .filter(at -> isValidAccessToken(at)) - .forEach(at -> { - tokenService.revokeAccessToken(at); - }); + .filter(this::isValidAccessToken) + .forEach(at -> tokenService.revokeAccessToken(at)); // delete all valid refresh tokens - tokenService.getRefreshTokensForClient(client).forEach(rt -> { - tokenService.revokeRefreshToken(rt); - }); + tokenService.getRefreshTokensForClient(client) + .forEach(rt -> tokenService.revokeRefreshToken(rt)); } @Override diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheConfig.java index e68bf701a..ff2e5d115 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheConfig.java @@ -15,23 +15,47 @@ */ package it.infn.mw.iam.config; - - +import org.springframework.boot.autoconfigure.cache.RedisCacheManagerBuilderCustomizer; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cache.CacheManager; -import org.springframework.cache.annotation.EnableCaching; import org.springframework.cache.concurrent.ConcurrentMapCacheManager; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.data.redis.cache.RedisCacheConfiguration; +import it.infn.mw.iam.core.oauth.scope.matchers.DefaultScopeMatcherRegistry; import it.infn.mw.iam.core.web.wellknown.IamWellKnownInfoProvider; @Configuration -@EnableCaching public class CacheConfig { + + @Bean + @ConditionalOnProperty(name = "redis-cache.enabled", havingValue = "false") + public CacheManager localCacheManager() { + return new ConcurrentMapCacheManager(IamWellKnownInfoProvider.CACHE_KEY, + DefaultScopeMatcherRegistry.SCOPE_CACHE_KEY); + } + + @Bean - public CacheManager cacheManager() { - return new ConcurrentMapCacheManager(IamWellKnownInfoProvider.CACHE_KEY); + @ConditionalOnProperty(name = "redis-cache.enabled", havingValue = "true") + public RedisCacheManagerBuilderCustomizer redisCacheManagerBuilderCustomizer() { + return builder -> builder + .withCacheConfiguration(IamWellKnownInfoProvider.CACHE_KEY, + RedisCacheConfiguration.defaultCacheConfig()) + .withCacheConfiguration(DefaultScopeMatcherRegistry.SCOPE_CACHE_KEY, + RedisCacheConfiguration.defaultCacheConfig()); + + } + + + @Bean + @ConditionalOnProperty(name = "redis-cache.enabled", havingValue = "true") + public RedisCacheConfiguration redisCacheConfiguration() { + + return RedisCacheConfiguration.defaultCacheConfig().disableCachingNullValues(); + } } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java index bf63a4aad..e45d7100d 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java @@ -254,7 +254,7 @@ FilterRegistrationBean aupSignatureCheckFilter(AUPSignatureChe @Bean ScopeMatcherRegistry customScopeMatchersRegistry(ScopeMatchersProperties properties) { ScopeMatchersPropertiesParser parser = new ScopeMatchersPropertiesParser(); - return new DefaultScopeMatcherRegistry(parser.parseScopeMatchersProperties(properties), 20); + return new DefaultScopeMatcherRegistry(parser.parseScopeMatchersProperties(properties)); } @Bean diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/MitreServicesConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/MitreServicesConfig.java index ea4947bf3..32fa1eaf6 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/MitreServicesConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/MitreServicesConfig.java @@ -159,7 +159,7 @@ RedirectResolver blacklistAwareRedirectResolver() { @Bean OAuth2RequestValidator requestValidator(ScopeMatcherRegistry registry) { - return new ScopeMatcherOAuthRequestValidator(registry, 50); + return new ScopeMatcherOAuthRequestValidator(registry); } @Bean diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/RedisCacheProperties.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/RedisCacheProperties.java new file mode 100644 index 000000000..1751a2579 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/RedisCacheProperties.java @@ -0,0 +1,35 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.config; + +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.context.annotation.Configuration; + +@ConfigurationProperties("redis-cache") +@Configuration +public class RedisCacheProperties { + + private boolean enabled = false; + + public boolean isEnabled() { + return enabled; + } + + public void setEnabled(boolean enable) { + this.enabled = enable; + } + +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/DefaultScopeMatcherRegistry.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/DefaultScopeMatcherRegistry.java index e3ea10a5d..58ac2be88 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/DefaultScopeMatcherRegistry.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/DefaultScopeMatcherRegistry.java @@ -15,46 +15,32 @@ */ package it.infn.mw.iam.core.oauth.scope.matchers; -import static com.google.common.base.Preconditions.checkArgument; -import static java.util.Objects.nonNull; - import java.util.Set; +import org.springframework.cache.annotation.Cacheable; import org.springframework.security.oauth2.provider.ClientDetails; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; import com.google.common.collect.Sets; @SuppressWarnings("deprecation") public class DefaultScopeMatcherRegistry implements ScopeMatcherRegistry { - public static final int DEFAULT_CACHE_SIZE = 10; - + public static final String SCOPE_CACHE_KEY = "scope-matcher"; + private final Set customMatchers; - private final LoadingCache plainMatchersCache; - public DefaultScopeMatcherRegistry(Set customMatchers) { - this(customMatchers, DEFAULT_CACHE_SIZE); - } - - public DefaultScopeMatcherRegistry(Set customMatchers, int plainMatchersCacheSize) { - checkArgument(nonNull(customMatchers), "customMatchers must be non-null"); - int cacheSize = - (plainMatchersCacheSize < DEFAULT_CACHE_SIZE ? DEFAULT_CACHE_SIZE : plainMatchersCacheSize); - plainMatchersCache = - CacheBuilder.newBuilder().maximumSize(cacheSize).build(CacheLoader.from(StringEqualsScopeMatcher::stringEqualsMatcher)); this.customMatchers = customMatchers; } @Override + @Cacheable(value = SCOPE_CACHE_KEY, key = "{#client?.id}") public Set findMatchersForClient(ClientDetails client) { Set result = Sets.newHashSet(); for (String s : client.getScope()) { result.add(findMatcherForScope(s)); + } return result; @@ -62,10 +48,10 @@ public Set findMatchersForClient(ClientDetails client) { @Override public ScopeMatcher findMatcherForScope(String scope) { + return customMatchers.stream() .filter(m -> m.matches(scope)) .findFirst() - .orElse(plainMatchersCache.getUnchecked(scope)); + .orElse(StringEqualsScopeMatcher.stringEqualsMatcher(scope)); } - } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/RegexpScopeMatcher.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/RegexpScopeMatcher.java index b99502d95..2151cc9d9 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/RegexpScopeMatcher.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/RegexpScopeMatcher.java @@ -24,6 +24,7 @@ public class RegexpScopeMatcher implements ScopeMatcher { + private static final long serialVersionUID = -1419325543749683292L; final String regexp; final Pattern pattern; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/ScopeMatcher.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/ScopeMatcher.java index 0eb59615c..79ea80feb 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/ScopeMatcher.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/ScopeMatcher.java @@ -15,6 +15,8 @@ */ package it.infn.mw.iam.core.oauth.scope.matchers; -public interface ScopeMatcher { +import java.io.Serializable; + +public interface ScopeMatcher extends Serializable { boolean matches(String scope); } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/ScopeMatcherOAuthRequestValidator.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/ScopeMatcherOAuthRequestValidator.java index 623fa4ae2..65411678d 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/ScopeMatcherOAuthRequestValidator.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/ScopeMatcherOAuthRequestValidator.java @@ -23,40 +23,27 @@ import org.springframework.security.oauth2.provider.OAuth2RequestValidator; import org.springframework.security.oauth2.provider.TokenRequest; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; - @SuppressWarnings("deprecation") public class ScopeMatcherOAuthRequestValidator implements OAuth2RequestValidator { - public static final int DEFAULT_CACHE_SIZE = 10; public static final String ERROR_MSG_FMT = "Scope '%s' not allowed for client '%s'"; private final ScopeMatcherRegistry registry; - private final LoadingCache> scopeMatchersCache; - - public ScopeMatcherOAuthRequestValidator(ScopeMatcherRegistry matcherRegistry, int cacheSize) { - this.registry = matcherRegistry; - int cs = cacheSize < DEFAULT_CACHE_SIZE ? DEFAULT_CACHE_SIZE : cacheSize; - scopeMatchersCache = CacheBuilder.newBuilder() - .maximumSize(cs) - .build(CacheLoader.from(registry::findMatchersForClient)); - } public ScopeMatcherOAuthRequestValidator(ScopeMatcherRegistry matcherRegistry) { - this(matcherRegistry, DEFAULT_CACHE_SIZE); + this.registry = matcherRegistry; } private void validateScope(Set requestedScopes, ClientDetails client) { - Set scopeMatchers = scopeMatchersCache.getUnchecked(client); + Set scopeMatchers = registry.findMatchersForClient(client); for (String s : requestedScopes) { if (scopeMatchers.stream().noneMatch(m -> m.matches(s))) { throw new InvalidScopeException(String.format(ERROR_MSG_FMT, s, client.getClientId())); } } + } @Override @@ -65,11 +52,9 @@ public void validateScope(AuthorizationRequest authorizationRequest, ClientDetai } @Override - public void validateScope(TokenRequest tokenRequest, ClientDetails client){ + public void validateScope(TokenRequest tokenRequest, ClientDetails client) { validateScope(tokenRequest.getScope(), client); } - public void invalidateScope(ClientDetails client) { - scopeMatchersCache.invalidate(client); - } + } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/StringEqualsScopeMatcher.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/StringEqualsScopeMatcher.java index 03e84d0ff..d4fb64693 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/StringEqualsScopeMatcher.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/StringEqualsScopeMatcher.java @@ -19,6 +19,7 @@ public class StringEqualsScopeMatcher implements ScopeMatcher { + private static final long serialVersionUID = -1283075702068272727L; final String expectedValue; private StringEqualsScopeMatcher(String expectedValue) { diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/StructuredPathScopeMatcher.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/StructuredPathScopeMatcher.java index eabf7fcc7..e195e5e4a 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/StructuredPathScopeMatcher.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/StructuredPathScopeMatcher.java @@ -29,6 +29,8 @@ public class StructuredPathScopeMatcher implements ScopeMatcher { + private static final long serialVersionUID = 8238476071265296982L; + public static final Logger LOG = LoggerFactory.getLogger(StructuredPathScopeMatcher.class); private static final Pattern POINT_DIR_MATCHER = Pattern.compile("\\.\\./?"); diff --git a/iam-login-service/src/main/resources/application-redis-test.yml b/iam-login-service/src/main/resources/application-redis-test.yml index e1979ed7d..9a5b06f54 100644 --- a/iam-login-service/src/main/resources/application-redis-test.yml +++ b/iam-login-service/src/main/resources/application-redis-test.yml @@ -23,4 +23,7 @@ spring: store-type: redis redis: flush-mode: immediate - namespace: iam:session \ No newline at end of file + namespace: iam:session + +redis-cache: + enabled: true \ No newline at end of file diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml index 6a4730eb9..0e0823cbf 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -182,6 +182,9 @@ iam: account-linking: enable: ${IAM_ACCOUNT_LINKING_ENABLE:true} +redis-cache: + enabled: ${IAM_REDIS_CACHE_ENABLED:false} + x509: trustAnchorsDir: ${IAM_X509_TRUST_ANCHORS_DIR:/etc/grid-security/certificates} trustAnchorsRefreshMsec: ${IAM_X509_TRUST_ANCHORS_REFRESH:14400} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherCacheTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherCacheTests.java index dbb92e655..2ba38ac82 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherCacheTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherCacheTests.java @@ -15,9 +15,11 @@ */ package it.infn.mw.iam.test.oauth.scope.matchers; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.in; import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertFalse; import java.text.ParseException; @@ -25,6 +27,7 @@ import org.junit.runner.RunWith; import org.mitre.oauth2.model.ClientDetailsEntity; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.cache.CacheManager; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; @@ -33,6 +36,8 @@ import com.nimbusds.jwt.JWTParser; import it.infn.mw.iam.api.client.service.ClientService; +import it.infn.mw.iam.config.CacheConfig; +import it.infn.mw.iam.config.RedisCacheProperties; import it.infn.mw.iam.test.oauth.EndpointsTestUtils; import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; @@ -47,6 +52,12 @@ public class ScopeMatcherCacheTests extends EndpointsTestUtils { @Autowired private ClientService clientService; + @Autowired + private CacheConfig cacheConfig; + + @Autowired + private RedisCacheProperties redisCacheProperties; + private String getAccessTokenForClient(String scopes) throws Exception { return new AccessTokenGetter().grantType("client_credentials") @@ -56,6 +67,12 @@ private String getAccessTokenForClient(String scopes) throws Exception { .getAccessTokenValue(); } + @Test + public void ensureRedisCashIsDisabled() { + assertFalse(redisCacheProperties.isEnabled()); + assertThat(cacheConfig.localCacheManager(), instanceOf(CacheManager.class)); + } + @Test public void updatingClientScopesInvalidatesCache() throws ParseException, Exception { diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherExternalCacheTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherExternalCacheTests.java new file mode 100644 index 000000000..a3c790369 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherExternalCacheTests.java @@ -0,0 +1,141 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.test.oauth.scope.matchers; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.in; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.text.ParseException; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mitre.oauth2.model.ClientDetailsEntity; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.cache.RedisCacheManagerBuilderCustomizer; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.boot.web.server.LocalServerPort; +import org.springframework.data.redis.cache.RedisCacheConfiguration; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Sets; +import com.nimbusds.jwt.JWT; +import com.nimbusds.jwt.JWTParser; + +import io.restassured.RestAssured; +import it.infn.mw.iam.api.client.service.ClientService; +import it.infn.mw.iam.config.CacheConfig; +import it.infn.mw.iam.config.RedisCacheProperties; +import it.infn.mw.iam.test.TestUtils; +import it.infn.mw.iam.test.oauth.EndpointsTestUtils; +import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; +import it.infn.mw.iam.test.util.redis.RedisContainer; + + + +@Testcontainers +@IamMockMvcIntegrationTest +@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, + properties = {"iam.access_token.include_scope=true", "redis-cache.enabled=true"}) +public class ScopeMatcherExternalCacheTests extends EndpointsTestUtils { + + private static final String CLIENT_ID = "cache-client"; + private static final String CLIENT_SECRET = "secret"; + + static { + System.setProperty("spring.devtools.restart.enabled", "false"); + } + + @Autowired + private ClientService clientService; + + @Autowired + private CacheConfig cacheConfig; + + @Autowired + private RedisCacheProperties redisCacheProperties; + + @LocalServerPort + private Integer iamPort; + + @Autowired + ObjectMapper mapper; + + @Container + private static final RedisContainer REDIS = new RedisContainer(); + + @DynamicPropertySource + static void redisProperties(DynamicPropertyRegistry registry) { + registry.add("spring.redis.port", REDIS::getFirstMappedPort); + + } + + @BeforeEach + public void setup() { + TestUtils.initRestAssured(); + RestAssured.port = iamPort; + assertTrue(redisCacheProperties.isEnabled()); + assertThat(cacheConfig.redisCacheConfiguration(), instanceOf(RedisCacheConfiguration.class)); + assertThat(cacheConfig.redisCacheManagerBuilderCustomizer(), + instanceOf(RedisCacheManagerBuilderCustomizer.class)); + + } + + private String getAccessTokenForClient(String scopes) throws Exception { + + return new AccessTokenGetter().grantType("client_credentials") + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .scope(scopes) + .getAccessTokenValue(); + + } + + @Test + public void ensureRedisRunning() { + assertTrue(REDIS.isRunning()); + } + + @Test + public void updatingClientScopesInvalidatesExternalCache() throws ParseException, Exception { + + ClientDetailsEntity client = new ClientDetailsEntity(); + client.setClientId(CLIENT_ID); + client.setClientSecret(CLIENT_SECRET); + client.setScope(Sets.newHashSet("openid", "profile", "email")); + client = clientService.saveNewClient(client); + + try { + JWT token = JWTParser.parse(getAccessTokenForClient("openid profile email")); + assertThat("scim:read", + not(in(token.getJWTClaimsSet().getClaim("scope").toString().split(" ")))); + client.setScope(Sets.newHashSet("openid", "profile", "email", "scim:read")); + clientService.updateClient(client); + token = JWTParser.parse(getAccessTokenForClient("openid profile email scim:read")); + assertThat("scim:read", in(token.getJWTClaimsSet().getClaim("scope").toString().split(" "))); + } finally { + clientService.deleteClient(client); + } + } + +} From a3e72b9eab65bc066db0f787db67a9d48deaea2c Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Tue, 25 Jul 2023 10:12:30 +0100 Subject: [PATCH 13/22] Fix missing deletion of orphans within #96 migration --- .../main/resources/db/migration/h2/V96__add_foreign_keys.sql | 2 ++ .../main/resources/db/migration/mysql/V96__add_foreign_keys.sql | 2 ++ 2 files changed, 4 insertions(+) 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 bb5853474..81aa1006e 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 @@ -53,7 +53,9 @@ WHERE id NOT IN (SELECT auth_holder_id FROM access_token) AND id NOT IN (SELECT auth_holder_id FROM refresh_token) AND id NOT IN (SELECT auth_holder_id FROM authorization_code); +DELETE FROM authentication_holder WHERE user_auth_id NOT IN (SELECT id FROM saved_user_auth); ALTER TABLE authentication_holder ADD FOREIGN KEY (user_auth_id) REFERENCES saved_user_auth (id) ON DELETE CASCADE; +DELETE FROM authentication_holder WHERE client_id NOT IN (SELECT client_id FROM client_details); ALTER TABLE authentication_holder ADD FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE CASCADE; -- ACCESS TOKEN TABLE and related 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 ea0e804b7..9cc387506 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 @@ -53,7 +53,9 @@ WHERE id NOT IN (SELECT auth_holder_id FROM access_token) AND id NOT IN (SELECT auth_holder_id FROM refresh_token) AND id NOT IN (SELECT auth_holder_id FROM authorization_code); +DELETE FROM authentication_holder WHERE user_auth_id NOT IN (SELECT id FROM saved_user_auth); ALTER TABLE authentication_holder ADD FOREIGN KEY (user_auth_id) REFERENCES saved_user_auth (id) ON DELETE CASCADE; +DELETE FROM authentication_holder WHERE client_id NOT IN (SELECT client_id FROM client_details); ALTER TABLE authentication_holder ADD FOREIGN KEY (client_id) REFERENCES client_details (client_id) ON UPDATE CASCADE ON DELETE CASCADE; -- ACCESS TOKEN TABLE and related From 94631ce695ec0865de6649239e45a86e2b9fa5d8 Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Tue, 25 Jul 2023 18:36:31 +0100 Subject: [PATCH 14/22] Use latest mitre version v1.3.6.*20230726 and fix token hash column --- .../it/infn/mw/iam/core/oauth/profile/IamOIDCTokenService.java | 1 + .../it/infn/mw/iam/core/oauth/profile/IamTokenEnhancer.java | 1 + .../src/main/resources/db/migration/h2/V93__add_at_hash.sql | 2 +- .../src/main/resources/db/migration/mysql/V93__add_at_hash.sql | 3 ++- pom.xml | 2 +- 5 files changed, 6 insertions(+), 3 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/IamOIDCTokenService.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/IamOIDCTokenService.java index c6216f7cd..38d8dacd0 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/IamOIDCTokenService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/IamOIDCTokenService.java @@ -284,6 +284,7 @@ private OAuth2AccessTokenEntity buildRegistrationAccessToken(ClientDetailsEntity jwtService.signJwt(signed); token.setJwt(signed); + token.hashMe(); return token; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/IamTokenEnhancer.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/IamTokenEnhancer.java index 42a2674da..800227c88 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/IamTokenEnhancer.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/IamTokenEnhancer.java @@ -93,6 +93,7 @@ public OAuth2AccessToken enhance(OAuth2AccessToken accessToken, .buildAccessToken(accessTokenEntity, authentication, userInfo, tokenIssueInstant); accessTokenEntity.setJwt(signClaims(atClaims)); + accessTokenEntity.hashMe(); /** * Authorization request scope MUST include "openid" in OIDC, but access token request may or diff --git a/iam-persistence/src/main/resources/db/migration/h2/V93__add_at_hash.sql b/iam-persistence/src/main/resources/db/migration/h2/V93__add_at_hash.sql index b2bf521fd..074783e0b 100644 --- a/iam-persistence/src/main/resources/db/migration/h2/V93__add_at_hash.sql +++ b/iam-persistence/src/main/resources/db/migration/h2/V93__add_at_hash.sql @@ -1,3 +1,3 @@ ALTER TABLE access_token DROP CONSTRAINT at_unique_token_value; -ALTER TABLE access_token ADD COLUMN token_value_hash CHAR(64) AS HASH('SHA256', token_value); +ALTER TABLE access_token ADD COLUMN token_value_hash CHAR(64); ALTER TABLE access_token ADD CONSTRAINT at_tvh_idx UNIQUE (token_value_hash); \ No newline at end of file diff --git a/iam-persistence/src/main/resources/db/migration/mysql/V93__add_at_hash.sql b/iam-persistence/src/main/resources/db/migration/mysql/V93__add_at_hash.sql index 8c839a374..3f1dff2b4 100644 --- a/iam-persistence/src/main/resources/db/migration/mysql/V93__add_at_hash.sql +++ b/iam-persistence/src/main/resources/db/migration/mysql/V93__add_at_hash.sql @@ -1,3 +1,4 @@ ALTER TABLE access_token DROP INDEX token_value; -ALTER TABLE access_token ADD COLUMN token_value_hash CHAR(64) AS (SHA2(token_value, 256)); +ALTER TABLE access_token ADD COLUMN token_value_hash CHAR(64) AS (SHA2(token_value, 256)) STORED; +ALTER TABLE access_token MODIFY COLUMN token_value_hash CHAR(64) NOT NULL; ALTER TABLE access_token ADD CONSTRAINT at_tvh_idx UNIQUE (token_value_hash); \ No newline at end of file diff --git a/pom.xml b/pom.xml index ea2cfa729..7ea2dc01a 100644 --- a/pom.xml +++ b/pom.xml @@ -50,7 +50,7 @@ 1.16.2 - 1.3.6.cnaf-20230717 + 1.3.6.cnaf-20230726 2.5.2.RELEASE 3.3.2 From 3749a7298d9b81c9364264a6a258f329fa1efd93 Mon Sep 17 00:00:00 2001 From: Federica Agostini Date: Thu, 14 Sep 2023 12:46:26 +0200 Subject: [PATCH 15/22] Fix management of tokens lifetime following RFC9068 (#620) The Access Token and Refresh Token lifetimes are configurable by admins via web interface. #545 The exp claim will always appear into access tokens (following RFC9068). This fix is included since mitreId version 1.3.6.cnaf-20230914. #648 --- .../api/client/service/ClientConverter.java | 41 ++--- .../service/DefaultClientDefaultsService.java | 15 -- .../ClientRegistrationProperties.java | 10 +- .../core/web/util/IamViewInfoInterceptor.java | 9 +- .../src/main/resources/application.yml | 4 + .../main/webapp/WEB-INF/tags/iamHeader.tag | 8 + .../tokensettings.component.html | 25 +--- .../tokensettings/tokensettings.component.js | 16 +- .../ClientManagementAPIIntegrationTests.java | 82 ++++++++++ ...ClientRegistrationAPIIntegrationTests.java | 140 ++++++++++++++++++ .../ClientRegistrationTestSupport.java | 53 ++++--- pom.xml | 2 +- 12 files changed, 321 insertions(+), 84 deletions(-) create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java index 9063a5d6a..54b4692ab 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/ClientConverter.java @@ -38,6 +38,7 @@ import it.infn.mw.iam.api.common.client.RegisteredClientDTO; import it.infn.mw.iam.api.common.client.TokenEndpointAuthenticationMethod; import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.config.client_registration.ClientRegistrationProperties; @Component public class ClientConverter { @@ -46,11 +47,14 @@ public class ClientConverter { private final String clientRegistrationBaseUrl; + private final ClientRegistrationProperties clientProperties; + @Autowired - public ClientConverter(IamProperties properties) { + public ClientConverter(IamProperties properties, ClientRegistrationProperties clientProperties) { this.iamProperties = properties; clientRegistrationBaseUrl = String.format("%s%s", iamProperties.getBaseUrl(), ClientRegistrationApiController.ENDPOINT); + this.clientProperties = clientProperties; } private Set cloneSet(Set stringSet) { @@ -67,18 +71,17 @@ public ClientDetailsEntity entityFromClientManagementRequest(RegisteredClientDTO ClientDetailsEntity client = entityFromRegistrationRequest(dto); if (dto.getAccessTokenValiditySeconds() != null) { - if (dto.getAccessTokenValiditySeconds() <= 0) { - client.setAccessTokenValiditySeconds(null); - } else { - client.setAccessTokenValiditySeconds(dto.getAccessTokenValiditySeconds()); - } + client.setAccessTokenValiditySeconds(dto.getAccessTokenValiditySeconds()); + } else { + client.setAccessTokenValiditySeconds( + clientProperties.getClientDefaults().getDefaultAccessTokenValiditySeconds()); } + if (dto.getRefreshTokenValiditySeconds() != null) { - if (dto.getRefreshTokenValiditySeconds() <= 0) { - client.setRefreshTokenValiditySeconds(null); - } else { - client.setRefreshTokenValiditySeconds(dto.getRefreshTokenValiditySeconds()); - } + client.setRefreshTokenValiditySeconds(dto.getRefreshTokenValiditySeconds()); + } else { + client.setRefreshTokenValiditySeconds( + clientProperties.getClientDefaults().getDefaultRefreshTokenValiditySeconds()); } if (dto.getIdTokenValiditySeconds() != null) { @@ -193,19 +196,16 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto client.setLogoUri(dto.getLogoUri()); client.setPolicyUri(dto.getPolicyUri()); - + client.setRedirectUris(cloneSet(dto.getRedirectUris())); client.setScope(cloneSet(dto.getScope())); - - client.setGrantTypes(new HashSet<>()); + + client.setGrantTypes(new HashSet<>()); if (!isNull(dto.getGrantTypes())) { client.setGrantTypes( - dto.getGrantTypes() - .stream() - .map(AuthorizationGrantType::getGrantType) - .collect(toSet())); + dto.getGrantTypes().stream().map(AuthorizationGrantType::getGrantType).collect(toSet())); } if (dto.getScope().contains("offline_access")) { @@ -231,6 +231,11 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto client.setCodeChallengeMethod(pkceAlgo); } + client.setAccessTokenValiditySeconds( + clientProperties.getClientDefaults().getDefaultAccessTokenValiditySeconds()); + client.setRefreshTokenValiditySeconds( + clientProperties.getClientDefaults().getDefaultRefreshTokenValiditySeconds()); + return client; } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java index 617333d71..aa320aeb2 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java @@ -58,33 +58,18 @@ public ClientDetailsEntity setupClientDefaults(ClientDetailsEntity client) { client.setClientId(UUID.randomUUID().toString()); } - client.setAccessTokenValiditySeconds( - properties.getClientDefaults().getDefaultAccessTokenValiditySeconds()); - client .setIdTokenValiditySeconds(properties.getClientDefaults().getDefaultIdTokenValiditySeconds()); client.setDeviceCodeValiditySeconds( properties.getClientDefaults().getDefaultDeviceCodeValiditySeconds()); - final int rtSecs = properties.getClientDefaults().getDefaultRefreshTokenValiditySeconds(); - - if (rtSecs < 0) { - client.setRefreshTokenValiditySeconds(null); - } else { - client.setRefreshTokenValiditySeconds(rtSecs); - } - client.setAllowIntrospection(true); if (isNull(client.getContacts())) { client.setContacts(new HashSet<>()); } - if (isNull(client.getClientId())) { - client.setClientId(UUID.randomUUID().toString()); - } - if (AUTH_METHODS_REQUIRING_SECRET.contains(client.getTokenEndpointAuthMethod())) { client.setClientSecret(generateClientSecret()); } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/client_registration/ClientRegistrationProperties.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/client_registration/ClientRegistrationProperties.java index d12d4d63c..0d5fb2aff 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/client_registration/ClientRegistrationProperties.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/client_registration/ClientRegistrationProperties.java @@ -19,6 +19,8 @@ import java.util.concurrent.TimeUnit; +import javax.validation.constraints.NotNull; + import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.annotation.Configuration; @@ -28,10 +30,14 @@ public class ClientRegistrationProperties { public static class ClientDefaultsProperties { - private int defaultAccessTokenValiditySeconds = (int) TimeUnit.HOURS.toSeconds(1); + @NotNull(message = "Provide a default access token lifetime") + private int defaultAccessTokenValiditySeconds; + + @NotNull(message = "Provide a default refresh token lifetime") + private int defaultRefreshTokenValiditySeconds; + private int defaultIdTokenValiditySeconds = (int) TimeUnit.MINUTES.toSeconds(10); private int defaultDeviceCodeValiditySeconds = (int) TimeUnit.MINUTES.toSeconds(10); - private int defaultRefreshTokenValiditySeconds = (int) TimeUnit.DAYS.toSeconds(30); private int defaultRegistrationAccessTokenValiditySeconds = -1; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/util/IamViewInfoInterceptor.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/util/IamViewInfoInterceptor.java index bec096df0..c481a8078 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/util/IamViewInfoInterceptor.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/util/IamViewInfoInterceptor.java @@ -24,6 +24,7 @@ import org.springframework.web.servlet.HandlerInterceptor; import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.config.client_registration.ClientRegistrationProperties; import it.infn.mw.iam.config.saml.IamSamlProperties; import it.infn.mw.iam.core.web.loginpage.LoginPageConfiguration; import it.infn.mw.iam.rcauth.RCAuthProperties; @@ -39,8 +40,8 @@ public class IamViewInfoInterceptor implements HandlerInterceptor { public static final String GIT_COMMIT_ID_KEY = "gitCommitId"; public static final String SIMULATE_NETWORK_LATENCY_KEY = "simulateNetworkLatency"; public static final String RCAUTH_ENABLED_KEY = "iamRcauthEnabled"; - public static final String RESOURCES_PATH_KEY = "resourcesPrefix"; + public static final String CLIENT_DEFAULTS_PROPERTIES_KEY = "clientDefaultsProperties"; @Value("${iam.version}") String iamVersion; @@ -62,7 +63,10 @@ public class IamViewInfoInterceptor implements HandlerInterceptor { @Autowired IamProperties iamProperties; - + + @Autowired + ClientRegistrationProperties clientRegistrationProperties; + @Override public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { @@ -78,6 +82,7 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons request.setAttribute(RCAUTH_ENABLED_KEY, rcAuthProperties.isEnabled()); + request.setAttribute(CLIENT_DEFAULTS_PROPERTIES_KEY, clientRegistrationProperties.getClientDefaults()); if (iamProperties.getVersionedStaticResources().isEnableVersioning()) { request.setAttribute(RESOURCES_PATH_KEY, String.format("/resources/%s", gitCommitId)); diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml index 0e0823cbf..bf051419b 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -213,6 +213,10 @@ task: client-registration: allow-for: ${IAM_CLIENT_REGISTRATION_ALLOW_FOR:ANYONE} enable: ${IAM_CLIENT_REGISTRATION_ENABLE:true} + client-defaults: + default-access-token-validity-seconds: ${DEFAULT_ACCESS_TOKEN_VALIDITY_SECONDS:3600} + default-refresh-token-validity-seconds: ${DEFAULT_REFRESH_TOKEN_VALIDITY_SECONDS:108000} + management: health: diff --git a/iam-login-service/src/main/webapp/WEB-INF/tags/iamHeader.tag b/iam-login-service/src/main/webapp/WEB-INF/tags/iamHeader.tag index 58109b39c..3abad14d6 100644 --- a/iam-login-service/src/main/webapp/WEB-INF/tags/iamHeader.tag +++ b/iam-login-service/src/main/webapp/WEB-INF/tags/iamHeader.tag @@ -88,6 +88,14 @@ function getOrganisationName() { return '${iamOrganisationName}'; } +function getAccessTokenValiditySeconds() { + return ${clientDefaultsProperties.defaultAccessTokenValiditySeconds}; +} + +function getRefreshTokenValiditySeconds() { + return ${clientDefaultsProperties.defaultRefreshTokenValiditySeconds}; +} + function getOidcEnabled() { return ${loginPageConfiguration.oidcEnabled}; } diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html index 1fb9c33b4..5b0fecc4c 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html @@ -20,16 +20,7 @@
-
- -
- + ng-model="$ctrl.client.access_token_validity_seconds">
@@ -66,15 +57,11 @@ - -
- -
+ ng-required ng-min="30"> +

+ This will setup after how many seconds the refresh token + expires. Type 0 to set an infinite lifetime. +

diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.js b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.js index 9e2b67da4..62a005fa3 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.js +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.js @@ -26,25 +26,27 @@ self.toggleOfflineAccess = toggleOfflineAccess; self.canIssueRefreshTokens = false; self.hasDeviceCodeGrantType = false; - + self.accessTokenValiditySeconds = getAccessTokenValiditySeconds(); + self.refreshTokenValiditySeconds = getRefreshTokenValiditySeconds(); self.$onInit = function () { console.debug('TokenSettingsController.self', self); - if (!self.client.access_token_validity_seconds) { - self.client.access_token_validity_seconds = 0; + if (self.client.access_token_validity_seconds == null) { + self.client.access_token_validity_seconds = self.accessTokenValiditySeconds; } - if (!self.client.refresh_token_validity_seconds) { - self.client.refresh_token_validity_seconds = 0; + + if (self.client.refresh_token_validity_seconds == null) { + self.client.refresh_token_validity_seconds = self.refreshTokenValiditySeconds; } $scope.$watch('$ctrl.client.access_token_validity_seconds', function handleChange(newVal, oldVal) { - if (!self.client.access_token_validity_seconds) { + if (newVal <= 0) { self.client.access_token_validity_seconds = 0; } }); $scope.$watch('$ctrl.client.refresh_token_validity_seconds', function handleChange(newVal, oldVal) { - if (!self.client.refresh_token_validity_seconds) { + if (newVal <= 0) { self.client.refresh_token_validity_seconds = 0; } }); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientManagementAPIIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientManagementAPIIntegrationTests.java index c3d8002ec..6e7693552 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientManagementAPIIntegrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientManagementAPIIntegrationTests.java @@ -20,6 +20,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasSize; +import static org.junit.Assert.assertTrue; import static org.springframework.http.MediaType.APPLICATION_JSON; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -144,4 +145,85 @@ public void ratRotationWorks() throws Exception { client = mapper.readValue(responseJson, RegisteredClientDTO.class); assertThat(client.getRegistrationAccessToken(), notNullValue()); } + + @Test + public void setTokenLifetimesWorks() throws Exception { + + String clientJson = ClientJsonStringBuilder.builder() + .scopes("openid") + .accessTokenValiditySeconds(null) + .refreshTokenValiditySeconds(null) + .build(); + + String responseJson = mvc + .perform(post(ClientManagementAPIController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(CREATED) + .andReturn() + .getResponse() + .getContentAsString(); + + RegisteredClientDTO client = mapper.readValue(responseJson, RegisteredClientDTO.class); + assertTrue(client.getAccessTokenValiditySeconds().equals(3600)); + assertTrue(client.getRefreshTokenValiditySeconds().equals(108000)); + + clientJson = ClientJsonStringBuilder.builder() + .scopes("openid") + .accessTokenValiditySeconds(0) + .refreshTokenValiditySeconds(0) + .build(); + + responseJson = mvc + .perform(post(ClientManagementAPIController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(CREATED) + .andReturn() + .getResponse() + .getContentAsString(); + + client = mapper.readValue(responseJson, RegisteredClientDTO.class); + assertTrue(client.getAccessTokenValiditySeconds().equals(0)); + assertTrue(client.getRefreshTokenValiditySeconds().equals(0)); + + clientJson = ClientJsonStringBuilder.builder() + .scopes("openid") + .accessTokenValiditySeconds(10) + .refreshTokenValiditySeconds(10) + .build(); + + responseJson = mvc + .perform(post(ClientManagementAPIController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(CREATED) + .andReturn() + .getResponse() + .getContentAsString(); + + client = mapper.readValue(responseJson, RegisteredClientDTO.class); + assertTrue(client.getAccessTokenValiditySeconds().equals(10)); + assertTrue(client.getRefreshTokenValiditySeconds().equals(10)); + + } + + @Test + public void negativeTokenLifetimesNotAllowed() throws Exception { + + String clientJson = + ClientJsonStringBuilder.builder().scopes("openid").accessTokenValiditySeconds(-1).build(); + + mvc + .perform(post(ClientManagementAPIController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(BAD_REQUEST) + .andExpect(jsonPath("$.error", containsString("must be greater than or equal to 0"))); + + clientJson = + ClientJsonStringBuilder.builder().scopes("openid").refreshTokenValiditySeconds(-1).build(); + + mvc + .perform(post(ClientManagementAPIController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(BAD_REQUEST) + .andExpect(jsonPath("$.error", containsString("must be greater than or equal to 0"))); + } } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java new file mode 100644 index 000000000..e1dddb7c2 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/api/client/ClientRegistrationAPIIntegrationTests.java @@ -0,0 +1,140 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.test.api.client; + +import static org.hamcrest.Matchers.containsString; +import static org.springframework.http.MediaType.APPLICATION_JSON; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.security.test.context.support.WithAnonymousUser; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.web.servlet.MockMvc; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import it.infn.mw.iam.IamLoginService; +import it.infn.mw.iam.api.client.registration.ClientRegistrationApiController; +import it.infn.mw.iam.api.common.client.RegisteredClientDTO; +import it.infn.mw.iam.test.api.TestSupport; +import it.infn.mw.iam.test.oauth.client_registration.ClientRegistrationTestSupport.ClientJsonStringBuilder; +import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; + +@IamMockMvcIntegrationTest +@WithMockUser(username = "test", roles = "USER") +@SpringBootTest(classes = {IamLoginService.class}) +public class ClientRegistrationAPIIntegrationTests extends TestSupport { + + @Autowired + private MockMvc mvc; + + @Autowired + private ObjectMapper mapper; + + @Test + @WithAnonymousUser + public void dynamicRegistrationWorksForAnonymousUser() throws Exception { + + String clientJson = ClientJsonStringBuilder.builder().scopes("openid").build(); + + mvc + .perform(post(ClientRegistrationApiController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(CREATED) + .andExpect(jsonPath("$.client_id").exists()) + .andExpect(jsonPath("$.client_secret").exists()) + .andExpect(jsonPath("$.client_name").exists()) + .andExpect(jsonPath("$.grant_types").exists()) + .andExpect(jsonPath("$.scope").exists()) + .andExpect(jsonPath("$.dynamically_registered").value(true)) + .andExpect(jsonPath("$.registration_access_token").exists()); + + } + + @Test + public void clientDetailsVisibleWithAuthentication() throws Exception { + + String clientJson = ClientJsonStringBuilder.builder().scopes("openid").build(); + + String responseJson = mvc + .perform(post(ClientRegistrationApiController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(CREATED) + .andReturn() + .getResponse() + .getContentAsString(); + + RegisteredClientDTO client = mapper.readValue(responseJson, RegisteredClientDTO.class); + + final String url = + String.format("%s/%s", ClientRegistrationApiController.ENDPOINT, client.getClientId()); + + mvc.perform(get(url)) + .andExpect(OK) + .andExpect(jsonPath("$.client_id").value(client.getClientId())) + .andExpect(jsonPath("$.client_name").value(client.getClientName())); + + } + + @Test + public void clientRemovalWorksWithAuthentication() throws Exception { + + String clientJson = ClientJsonStringBuilder.builder().scopes("openid").build(); + + String responseJson = mvc + .perform(post(ClientRegistrationApiController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(CREATED) + .andReturn() + .getResponse() + .getContentAsString(); + + RegisteredClientDTO client = mapper.readValue(responseJson, RegisteredClientDTO.class); + + final String url = + String.format("%s/%s", ClientRegistrationApiController.ENDPOINT, client.getClientId()); + + mvc.perform(delete(url)).andExpect(NO_CONTENT); + + mvc.perform(get(url)) + .andExpect(NOT_FOUND) + .andExpect(jsonPath("$.error", containsString("Client not found"))); + } + + @Test + public void tokenLifetimesAreNotEditable() throws Exception { + + String clientJson = ClientJsonStringBuilder.builder() + .scopes("openid") + .accessTokenValiditySeconds(10) + .refreshTokenValiditySeconds(10) + .build(); + + mvc + .perform(post(ClientRegistrationApiController.ENDPOINT).contentType(APPLICATION_JSON) + .content(clientJson)) + .andExpect(CREATED) + .andExpect(jsonPath("$.access_token_validity_seconds").doesNotExist()) + .andExpect(jsonPath("$.refresh_token_validity_seconds").doesNotExist()); + + } + +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTestSupport.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTestSupport.java index e1277cf85..78d3c3fcd 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTestSupport.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/client_registration/ClientRegistrationTestSupport.java @@ -42,47 +42,59 @@ public class ClientRegistrationTestSupport { public static final String LEGACY_REGISTER_ENDPOINT = "/register"; public static class ClientJsonStringBuilder { - + static final Joiner JOINER = Joiner.on(RegisteredClientFields.SCOPE_SEPARATOR); - + String name = "test_client"; Set redirectUris = Sets.newHashSet("http://localhost:9090"); Set grantTypes = Sets.newHashSet("client_credentials"); Set scopes = Sets.newHashSet(); Set responseTypes = Sets.newHashSet(); - - private ClientJsonStringBuilder() { - } - + Integer accessTokenValiditySeconds; + Integer refreshTokenValiditySeconds; + + private ClientJsonStringBuilder() {} + public static ClientJsonStringBuilder builder() { return new ClientJsonStringBuilder(); } - + public ClientJsonStringBuilder name(String name) { this.name = name; return this; } - - public ClientJsonStringBuilder redirectUris(String...uris) { + + public ClientJsonStringBuilder redirectUris(String... uris) { this.redirectUris = Sets.newHashSet(uris); return this; } - - public ClientJsonStringBuilder grantTypes(String...grantTypes) { + + public ClientJsonStringBuilder grantTypes(String... grantTypes) { this.grantTypes = Sets.newHashSet(grantTypes); return this; } - - public ClientJsonStringBuilder scopes(String...scopes) { + + public ClientJsonStringBuilder scopes(String... scopes) { this.scopes = Sets.newHashSet(scopes); return this; } - - public ClientJsonStringBuilder responseTypes(String...responseTypes) { + + public ClientJsonStringBuilder responseTypes(String... responseTypes) { this.responseTypes = Sets.newHashSet(responseTypes); return this; } - + + public ClientJsonStringBuilder accessTokenValiditySeconds(Integer accessTokenValiditySeconds) { + this.accessTokenValiditySeconds = accessTokenValiditySeconds; + return this; + } + + public ClientJsonStringBuilder refreshTokenValiditySeconds( + Integer refreshTokenValiditySeconds) { + this.refreshTokenValiditySeconds = refreshTokenValiditySeconds; + return this; + } + public String build() { JsonObject json = new JsonObject(); json.addProperty(CLIENT_NAME, name); @@ -93,12 +105,13 @@ public String build() { json.add(CLAIMS_REDIRECT_URIS, getAsArray(newHashSet(), true)); json.add(REQUEST_URIS, getAsArray(newHashSet(), true)); json.add(CONTACTS, getAsArray(newHashSet("test@iam.test"))); - + json.addProperty("access_token_validity_seconds", accessTokenValiditySeconds); + json.addProperty("refresh_token_validity_seconds", refreshTokenValiditySeconds); return json.toString(); } - - - + + + } protected String setToString(Set scopes) { diff --git a/pom.xml b/pom.xml index 7ea2dc01a..18e83817a 100644 --- a/pom.xml +++ b/pom.xml @@ -50,7 +50,7 @@ 1.16.2 - 1.3.6.cnaf-20230726 + 1.3.6.cnaf-20230914 2.5.2.RELEASE 3.3.2 From 03636ccf18750eb8574a5578a0d9a98ad8685adb Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Thu, 14 Sep 2023 14:04:32 +0200 Subject: [PATCH 16/22] Enable sonar analysis also on develop branch and update SONAR_TOKEN source value --- .github/workflows/sonar.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/sonar.yaml b/.github/workflows/sonar.yaml index d75ebf1d0..a8136225f 100644 --- a/.github/workflows/sonar.yaml +++ b/.github/workflows/sonar.yaml @@ -1,6 +1,9 @@ name: Sonar analysis -on: +on: + push: + branches: + - develop pull_request: types: [opened, edited, reopened, synchronize] @@ -28,7 +31,7 @@ jobs: - name: Sonar analysis env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN_VIANELLO }} run: mvn -B -U install sonar:sonar -Dsonar.projectKey=indigo-iam_iam -Dsonar.organization=indigo-iam From 952692a5ec03b283da27b7223789a90e96c208ce Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Thu, 14 Sep 2023 14:09:46 +0200 Subject: [PATCH 17/22] Update build status on README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 75eff5b1c..4e4d31ee3 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # INDIGO Identity and Access Management (IAM) service [![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.3496834.svg)](https://doi.org/10.5281/zenodo.3496834) -[![travis-build-tatus](https://travis-ci.org/indigo-iam/iam.svg?branch=develop)](https://travis-ci.org/indigo-iam/iam) +[![build & packaging](https://github.com/indigo-iam/iam/actions/workflows/maven.yml/badge.svg?branch=master&event=push)](https://github.com/indigo-iam/iam/actions/workflows/maven.yml) [![sonarqube-qg](https://sonarcloud.io/api/project_badges/measure?project=indigo-iam_iam&metric=alert_status)](https://sonarcloud.io/dashboard?id=indigo-iam_iam) [![sonarqube-coverage](https://sonarcloud.io/api/project_badges/measure?project=indigo-iam_iam&metric=coverage)](https://sonarcloud.io/dashboard?id=indigo-iam_iam) [![sonarqube-maintainability](https://sonarcloud.io/api/project_badges/measure?project=indigo-iam_iam&metric=sqale_rating)](https://sonarcloud.io/dashboard?id=indigo-iam_iam) From 283d791a7bcf58233fafcd4a3915fab3394cf0e1 Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Thu, 14 Sep 2023 15:41:06 +0200 Subject: [PATCH 18/22] Add new JWT profile that rename 'groups' claim with 'roles' (#637) This PR adds a 'kc' profile which aim is allowing the integration with a Keycloak environment by having "roles" claim instead of "groups". --- .../java/it/infn/mw/iam/config/IamConfig.java | 38 +++- .../it/infn/mw/iam/config/IamProperties.java | 3 +- .../profile/ScopeAwareProfileResolver.java | 3 +- .../profile/keycloak/KeycloakGroupHelper.java | 32 ++++ .../keycloak/KeycloakIdTokenCustomizer.java | 66 +++++++ .../keycloak/KeycloakIntrospectionHelper.java | 61 ++++++ .../profile/keycloak/KeycloakJWTProfile.java | 41 ++++ .../KeycloakProfileAccessTokenBuilder.java | 73 +++++++ .../keycloak/KeycloakUserInfoAdapter.java | 65 +++++++ .../keycloak/KeycloakUserinfoHelper.java | 61 ++++++ .../profile/wlcg/WLCGUserinfoHelper.java | 6 +- .../IamScopeClaimTranslationService.java | 3 +- .../mw/iam/core/userinfo/UserInfoClaim.java | 3 +- .../IntrospectionEndpointTests.java | 4 +- .../KeycloakAccessTokenBuilderTests.java | 122 ++++++++++++ .../KeycloakProfileIntegrationTests.java | 178 ++++++++++++++++++ .../profile/KeycloakProfileUserInfoTests.java | 129 +++++++++++++ .../oauth/profile/ProfileSelectorTests.java | 23 +++ 18 files changed, 901 insertions(+), 10 deletions(-) create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakGroupHelper.java create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakIdTokenCustomizer.java create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakIntrospectionHelper.java create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakJWTProfile.java create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakProfileAccessTokenBuilder.java create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakUserInfoAdapter.java create mode 100644 iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakUserinfoHelper.java create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakAccessTokenBuilderTests.java create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakProfileIntegrationTests.java create mode 100644 iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakProfileUserInfoTests.java diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java index e45d7100d..d069e9234 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java @@ -17,6 +17,7 @@ import static it.infn.mw.iam.core.oauth.profile.ScopeAwareProfileResolver.AARC_PROFILE_ID; import static it.infn.mw.iam.core.oauth.profile.ScopeAwareProfileResolver.IAM_PROFILE_ID; +import static it.infn.mw.iam.core.oauth.profile.ScopeAwareProfileResolver.KC_PROFILE_ID; import static it.infn.mw.iam.core.oauth.profile.ScopeAwareProfileResolver.WLCG_PROFILE_ID; import java.time.Clock; @@ -69,6 +70,12 @@ import it.infn.mw.iam.core.oauth.profile.iam.IamJWTProfileIdTokenCustomizer; import it.infn.mw.iam.core.oauth.profile.iam.IamJWTProfileTokenIntrospectionHelper; import it.infn.mw.iam.core.oauth.profile.iam.IamJWTProfileUserinfoHelper; +import it.infn.mw.iam.core.oauth.profile.keycloak.KeycloakGroupHelper; +import it.infn.mw.iam.core.oauth.profile.keycloak.KeycloakIdTokenCustomizer; +import it.infn.mw.iam.core.oauth.profile.keycloak.KeycloakIntrospectionHelper; +import it.infn.mw.iam.core.oauth.profile.keycloak.KeycloakJWTProfile; +import it.infn.mw.iam.core.oauth.profile.keycloak.KeycloakProfileAccessTokenBuilder; +import it.infn.mw.iam.core.oauth.profile.keycloak.KeycloakUserinfoHelper; import it.infn.mw.iam.core.oauth.profile.wlcg.WLCGGroupHelper; import it.infn.mw.iam.core.oauth.profile.wlcg.WLCGJWTProfile; import it.infn.mw.iam.core.oauth.scope.matchers.DefaultScopeMatcherRegistry; @@ -152,6 +159,27 @@ JWTProfile aarcJwtProfile(IamProperties props, IamAccountRepository accountRepo, return new AarcJWTProfile(atBuilder, idHelper, uiHelper, intrHelper); } + @Bean(name = "kcJwtProfile") + JWTProfile kcJwtProfile(IamProperties props, IamAccountRepository accountRepo, + ScopeClaimTranslationService converter, UserInfoService userInfoService, ScopeMatcherRegistry registry, ClaimValueHelper claimHelper) { + + KeycloakGroupHelper groupHelper = new KeycloakGroupHelper(); + + KeycloakProfileAccessTokenBuilder atBuilder = + new KeycloakProfileAccessTokenBuilder(props, groupHelper); + + KeycloakUserinfoHelper uiHelper = + new KeycloakUserinfoHelper(props, userInfoService); + + KeycloakIdTokenCustomizer idHelper = + new KeycloakIdTokenCustomizer(accountRepo, converter, claimHelper, groupHelper, props); + + BaseIntrospectionHelper intrHelper = new KeycloakIntrospectionHelper(props, + new DefaultIntrospectionResultAssembler(), registry, groupHelper); + + return new KeycloakJWTProfile(atBuilder, idHelper, uiHelper, intrHelper); + } + @Bean(name = "iamJwtProfile") JWTProfile iamJwtProfile(IamProperties props, IamAccountRepository accountRepo, ScopeClaimTranslationService converter, ClaimValueHelper claimHelper, @@ -188,7 +216,9 @@ attributeMapHelper, new DefaultIntrospectionResultAssembler(), registry, @Bean JWTProfileResolver jwtProfileResolver(@Qualifier("iamJwtProfile") JWTProfile iamProfile, @Qualifier("wlcgJwtProfile") JWTProfile wlcgProfile, - @Qualifier("aarcJwtProfile") JWTProfile aarcProfile, IamProperties properties, + @Qualifier("aarcJwtProfile") JWTProfile aarcProfile, + @Qualifier("kcJwtProfile") JWTProfile kcProfile, + IamProperties properties, ClientDetailsService clientDetailsService) { JWTProfile defaultProfile = iamProfile; @@ -203,10 +233,16 @@ JWTProfileResolver jwtProfileResolver(@Qualifier("iamJwtProfile") JWTProfile iam defaultProfile = aarcProfile; } + if (it.infn.mw.iam.config.IamProperties.JWTProfile.Profile.KC + .equals(properties.getJwtProfile().getDefaultProfile())) { + defaultProfile = kcProfile; + } + Map profileMap = Maps.newHashMap(); profileMap.put(IAM_PROFILE_ID, iamProfile); profileMap.put(WLCG_PROFILE_ID, wlcgProfile); profileMap.put(AARC_PROFILE_ID, aarcProfile); + profileMap.put(KC_PROFILE_ID, kcProfile); LOG.info("Default JWT profile: {}", defaultProfile.name()); return new ScopeAwareProfileResolver(defaultProfile, profileMap, clientDetailsService); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java index 02c4f8951..a6793d664 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamProperties.java @@ -354,7 +354,8 @@ public static class JWTProfile { public enum Profile { IAM, WLCG, - AARC + AARC, + KC } Profile defaultProfile = Profile.IAM; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/ScopeAwareProfileResolver.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/ScopeAwareProfileResolver.java index ac03dc430..9d2bdd86d 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/ScopeAwareProfileResolver.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/ScopeAwareProfileResolver.java @@ -34,9 +34,10 @@ public class ScopeAwareProfileResolver implements JWTProfileResolver { public static final String AARC_PROFILE_ID = "aarc"; public static final String IAM_PROFILE_ID = "iam"; public static final String WLCG_PROFILE_ID = "wlcg"; + public static final String KC_PROFILE_ID = "kc"; private static final Set SUPPORTED_PROFILES = - newHashSet(AARC_PROFILE_ID, IAM_PROFILE_ID, WLCG_PROFILE_ID); + newHashSet(AARC_PROFILE_ID, IAM_PROFILE_ID, WLCG_PROFILE_ID, KC_PROFILE_ID); private final Map profileMap; private final JWTProfile defaultProfile; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakGroupHelper.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakGroupHelper.java new file mode 100644 index 000000000..66572635e --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakGroupHelper.java @@ -0,0 +1,32 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.core.oauth.profile.keycloak; + +import java.util.Set; +import java.util.stream.Collectors; + +import it.infn.mw.iam.persistence.model.IamUserInfo; + +public class KeycloakGroupHelper { + + public static final String KEYCLOAK_ROLES_CLAIM = "roles"; + + public Set resolveGroupNames(IamUserInfo userInfo) { + + return userInfo.getGroups().stream().map(g -> g.getName()).collect(Collectors.toSet()); + } + +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakIdTokenCustomizer.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakIdTokenCustomizer.java new file mode 100644 index 000000000..6c1be7320 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakIdTokenCustomizer.java @@ -0,0 +1,66 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.core.oauth.profile.keycloak; + +import java.util.Set; + +import org.mitre.oauth2.model.ClientDetailsEntity; +import org.mitre.oauth2.model.OAuth2AccessTokenEntity; +import org.mitre.openid.connect.service.ScopeClaimTranslationService; +import org.springframework.security.oauth2.provider.OAuth2Request; + +import com.nimbusds.jwt.JWTClaimsSet.Builder; + +import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.core.oauth.profile.iam.ClaimValueHelper; +import it.infn.mw.iam.core.oauth.profile.iam.IamJWTProfileIdTokenCustomizer; +import it.infn.mw.iam.persistence.model.IamAccount; +import it.infn.mw.iam.persistence.model.IamUserInfo; +import it.infn.mw.iam.persistence.repository.IamAccountRepository; + +@SuppressWarnings("deprecation") +public class KeycloakIdTokenCustomizer extends IamJWTProfileIdTokenCustomizer { + + private final KeycloakGroupHelper groupHelper; + + public KeycloakIdTokenCustomizer(IamAccountRepository accountRepo, + ScopeClaimTranslationService scopeClaimConverter, ClaimValueHelper claimValueHelper, + KeycloakGroupHelper groupHelper, IamProperties properties) { + super(accountRepo, scopeClaimConverter, claimValueHelper, properties); + this.groupHelper = groupHelper; + } + + @Override + public void customizeIdTokenClaims(Builder idClaims, ClientDetailsEntity client, + OAuth2Request request, String sub, OAuth2AccessTokenEntity accessToken, IamAccount account) { + + super.customizeIdTokenClaims(idClaims, client, request, sub, accessToken, account); + + IamUserInfo info = account.getUserInfo(); + Set groupNames = groupHelper.resolveGroupNames(info); + + if (!groupNames.isEmpty()) { + idClaims.claim(KeycloakGroupHelper.KEYCLOAK_ROLES_CLAIM, groupNames); + } + + // Drop group claims as set by IAM JWT profile + idClaims.claim("groups", null); + + includeLabelsInIdToken(idClaims, account); + + } + +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakIntrospectionHelper.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakIntrospectionHelper.java new file mode 100644 index 000000000..56448a033 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakIntrospectionHelper.java @@ -0,0 +1,61 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.core.oauth.profile.keycloak; + +import java.util.Map; +import java.util.Set; + +import org.mitre.oauth2.model.OAuth2AccessTokenEntity; +import org.mitre.oauth2.service.IntrospectionResultAssembler; +import org.mitre.openid.connect.model.UserInfo; + +import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.core.oauth.profile.common.BaseIntrospectionHelper; +import it.infn.mw.iam.core.oauth.scope.matchers.ScopeMatcherRegistry; +import it.infn.mw.iam.persistence.repository.UserInfoAdapter; + + +public class KeycloakIntrospectionHelper extends BaseIntrospectionHelper { + + private final KeycloakGroupHelper groupHelper; + + public KeycloakIntrospectionHelper(IamProperties props, IntrospectionResultAssembler assembler, + ScopeMatcherRegistry registry, KeycloakGroupHelper helper) { + super(props, assembler, registry); + this.groupHelper = helper; + } + + @Override + public Map assembleIntrospectionResult(OAuth2AccessTokenEntity accessToken, + UserInfo userInfo, Set authScopes) { + + Map result = getAssembler().assembleFrom(accessToken, userInfo, authScopes); + + addIssuerClaim(result); + addAudience(result, accessToken); + addScopeClaim(result, filterScopes(accessToken, authScopes)); + + Set groups = + groupHelper.resolveGroupNames(((UserInfoAdapter) userInfo).getUserinfo()); + + if (!groups.isEmpty()) { + result.put(KeycloakGroupHelper.KEYCLOAK_ROLES_CLAIM, groups); + } + + return result; + } + +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakJWTProfile.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakJWTProfile.java new file mode 100644 index 000000000..998828e11 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakJWTProfile.java @@ -0,0 +1,41 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.core.oauth.profile.keycloak; + +import it.infn.mw.iam.core.oauth.profile.IDTokenCustomizer; +import it.infn.mw.iam.core.oauth.profile.IntrospectionResultHelper; +import it.infn.mw.iam.core.oauth.profile.JWTAccessTokenBuilder; +import it.infn.mw.iam.core.oauth.profile.UserInfoHelper; +import it.infn.mw.iam.core.oauth.profile.iam.IamJWTProfile; + +public class KeycloakJWTProfile extends IamJWTProfile { + + public static final String PROFILE_VERSION = "1.0"; + public static final String PROFILE_NAME = "Keycloak JWT profile " + PROFILE_VERSION; + + public KeycloakJWTProfile(JWTAccessTokenBuilder accessTokenBuilder, + IDTokenCustomizer idTokenBuilder, UserInfoHelper userInfoHelper, + IntrospectionResultHelper introspectionHelper) { + + super(accessTokenBuilder, idTokenBuilder, userInfoHelper, introspectionHelper); + } + + @Override + public String name() { + return PROFILE_NAME; + } + +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakProfileAccessTokenBuilder.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakProfileAccessTokenBuilder.java new file mode 100644 index 000000000..672538d61 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakProfileAccessTokenBuilder.java @@ -0,0 +1,73 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.core.oauth.profile.keycloak; + +import static java.util.Objects.isNull; +import static java.util.stream.Collectors.joining; + +import java.time.Instant; +import java.util.Date; +import java.util.Set; + +import org.mitre.oauth2.model.OAuth2AccessTokenEntity; +import org.mitre.openid.connect.model.UserInfo; +import org.springframework.security.oauth2.provider.OAuth2Authentication; + +import com.nimbusds.jwt.JWTClaimsSet; +import com.nimbusds.jwt.JWTClaimsSet.Builder; + +import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.core.oauth.profile.common.BaseAccessTokenBuilder; +import it.infn.mw.iam.persistence.repository.UserInfoAdapter; + +@SuppressWarnings("deprecation") +public class KeycloakProfileAccessTokenBuilder extends BaseAccessTokenBuilder { + + public static final String PROFILE_VERSION = "1.0"; + + final KeycloakGroupHelper groupHelper; + + public KeycloakProfileAccessTokenBuilder(IamProperties properties, KeycloakGroupHelper groupHelper) { + super(properties); + this.groupHelper = groupHelper; + } + + + @Override + public JWTClaimsSet buildAccessToken(OAuth2AccessTokenEntity token, + OAuth2Authentication authentication, UserInfo userInfo, Instant issueTime) { + + Builder builder = baseJWTSetup(token, authentication, userInfo, issueTime); + + builder.notBeforeTime(Date.from(issueTime)); + + if (!token.getScope().isEmpty()) { + builder.claim(SCOPE_CLAIM_NAME, token.getScope().stream().collect(joining(SPACE))); + } + + if (!isNull(userInfo)) { + Set groupNames = + groupHelper.resolveGroupNames(((UserInfoAdapter) userInfo).getUserinfo()); + + if (!groupNames.isEmpty()) { + builder.claim(KeycloakGroupHelper.KEYCLOAK_ROLES_CLAIM, groupNames); + } + } + + return builder.build(); + } + +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakUserInfoAdapter.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakUserInfoAdapter.java new file mode 100644 index 000000000..c6e7c1119 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakUserInfoAdapter.java @@ -0,0 +1,65 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.core.oauth.profile.keycloak; + + + +import static java.util.Objects.isNull; + +import org.mitre.openid.connect.model.UserInfo; + +import com.google.gson.JsonArray; +import com.google.gson.JsonObject; +import com.google.gson.JsonPrimitive; + +import it.infn.mw.iam.core.userinfo.DelegateUserInfoAdapter; + +public class KeycloakUserInfoAdapter extends DelegateUserInfoAdapter { + + private static final long serialVersionUID = 1L; + + private final String[] resolvedGroups; + + private KeycloakUserInfoAdapter(UserInfo delegate, String[] resolvedGroups) { + super(delegate); + this.resolvedGroups = resolvedGroups; + } + + @Override + public JsonObject toJson() { + JsonObject json = super.toJson(); + + json.remove("groups"); + + if (!isNull(resolvedGroups)) { + JsonArray groups = new JsonArray(); + for (String g : resolvedGroups) { + groups.add(new JsonPrimitive(g)); + } + json.add(KeycloakGroupHelper.KEYCLOAK_ROLES_CLAIM, groups); + } + + return json; + } + + public static KeycloakUserInfoAdapter forUserInfo(UserInfo delegate, String[] resolvedGroups) { + return new KeycloakUserInfoAdapter(delegate, resolvedGroups); + } + + public static KeycloakUserInfoAdapter forUserInfo(UserInfo delegate) { + return new KeycloakUserInfoAdapter(delegate, null); + } +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakUserinfoHelper.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakUserinfoHelper.java new file mode 100644 index 000000000..8c0d9c00e --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/keycloak/KeycloakUserinfoHelper.java @@ -0,0 +1,61 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.core.oauth.profile.keycloak; + +import static it.infn.mw.iam.core.oauth.profile.keycloak.KeycloakUserInfoAdapter.forUserInfo; +import static java.util.Objects.isNull; + +import java.util.Optional; + +import org.mitre.openid.connect.model.UserInfo; +import org.mitre.openid.connect.service.UserInfoService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.security.oauth2.provider.OAuth2Authentication; + +import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.core.oauth.profile.wlcg.WLCGUserinfoHelper; + +@SuppressWarnings("deprecation") +public class KeycloakUserinfoHelper extends WLCGUserinfoHelper { + + public static final Logger LOG = LoggerFactory.getLogger(KeycloakUserinfoHelper.class); + + public KeycloakUserinfoHelper(IamProperties props, UserInfoService userInfoService) { + super(props, userInfoService); + } + + @Override + public UserInfo resolveUserInfo(OAuth2Authentication authentication) { + + UserInfo ui = lookupUserinfo(authentication); + + if (isNull(ui)) { + return null; + } + + Optional resolvedGroups = + resolveGroupsFromToken(authentication, KeycloakGroupHelper.KEYCLOAK_ROLES_CLAIM); + + if (resolvedGroups.isPresent()) { + return forUserInfo(ui, resolvedGroups.get()); + } else { + return forUserInfo(ui); + } + + } + +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/wlcg/WLCGUserinfoHelper.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/wlcg/WLCGUserinfoHelper.java index 339de2b7d..62baa1e6c 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/wlcg/WLCGUserinfoHelper.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/profile/wlcg/WLCGUserinfoHelper.java @@ -44,7 +44,7 @@ public WLCGUserinfoHelper(IamProperties props, UserInfoService userInfoService) } - private Optional resolveGroupsFromToken(OAuth2Authentication authentication) { + protected Optional resolveGroupsFromToken(OAuth2Authentication authentication, String groupClaim) { OAuth2AuthenticationDetails details = (OAuth2AuthenticationDetails) authentication.getDetails(); if (isNull(details) || isNull(details.getTokenValue())) { @@ -53,7 +53,7 @@ private Optional resolveGroupsFromToken(OAuth2Authentication authentic try { JWT accessToken = JWTParser.parse(details.getTokenValue()); - String[] resolvedGroups = accessToken.getJWTClaimsSet().getStringArrayClaim("wlcg.groups"); + String[] resolvedGroups = accessToken.getJWTClaimsSet().getStringArrayClaim(groupClaim); return Optional.ofNullable(resolvedGroups); @@ -72,7 +72,7 @@ public UserInfo resolveUserInfo(OAuth2Authentication authentication) { return null; } - Optional resolvedGroups = resolveGroupsFromToken(authentication); + Optional resolvedGroups = resolveGroupsFromToken(authentication, WLCGGroupHelper.WLCG_GROUPS_SCOPE); if (resolvedGroups.isPresent()) { return forUserInfo(ui, resolvedGroups.get()); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/userinfo/IamScopeClaimTranslationService.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/userinfo/IamScopeClaimTranslationService.java index 61169241d..314dd15a4 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/userinfo/IamScopeClaimTranslationService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/userinfo/IamScopeClaimTranslationService.java @@ -45,6 +45,7 @@ import static it.infn.mw.iam.core.userinfo.UserInfoClaim.WEBSITE; import static it.infn.mw.iam.core.userinfo.UserInfoClaim.WLCG_GROUPS; import static it.infn.mw.iam.core.userinfo.UserInfoClaim.ZONEINFO; +import static it.infn.mw.iam.core.userinfo.UserInfoClaim.ROLES; import java.util.EnumSet; import java.util.HashSet; @@ -79,7 +80,7 @@ public class IamScopeClaimTranslationService implements ScopeClaimTranslationSer protected static final Set PROFILE_CLAIMS = EnumSet.of(NAME, PREFERRED_USERNAME, GIVEN_NAME, FAMILY_NAME, MIDDLE_NAME, NICKNAME, PROFILE, PICTURE, WEBSITE, GENDER, ZONEINFO, - LOCALE, UPDATED_AT, BIRTHDATE, ORGANISATION_NAME, GROUPS, EXTERNAL_AUTHN); + LOCALE, UPDATED_AT, BIRTHDATE, ORGANISATION_NAME, GROUPS, EXTERNAL_AUTHN, ROLES); protected static final Set EMAIL_CLAIMS = EnumSet.of(EMAIL, EMAIL_VERIFIED); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/userinfo/UserInfoClaim.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/userinfo/UserInfoClaim.java index 720b9a015..cbac68093 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/userinfo/UserInfoClaim.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/userinfo/UserInfoClaim.java @@ -45,7 +45,8 @@ public enum UserInfoClaim { EDUPERSON_ENTITLEMENT("eduperson_entitlement"), ENTITLEMENTS("entitlements"), EDUPERSON_ASSURANCE("eduperson_assurance"), - SSH_KEYS("ssh_keys"); + SSH_KEYS("ssh_keys"), + ROLES("roles"); private UserInfoClaim(String claimName) { this.claimName = claimName; diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/introspection/IntrospectionEndpointTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/introspection/IntrospectionEndpointTests.java index 7c1c477d4..bca7e412c 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/introspection/IntrospectionEndpointTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/introspection/IntrospectionEndpointTests.java @@ -44,7 +44,7 @@ public class IntrospectionEndpointTests extends EndpointsTestUtils { @Value("${iam.organisation.name}") String organisationName; - + @Value("${iam.issuer}") String issuer; @@ -53,7 +53,7 @@ public class IntrospectionEndpointTests extends EndpointsTestUtils { private static final String CLIENT_SECRET = "secret"; @Test - public void testIntrospectionEndpointRetursBasicUserInformation() throws Exception { + public void testIntrospectionEndpointReturnsBasicUserInformation() throws Exception { String accessToken = getPasswordAccessToken(); // @formatter:off diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakAccessTokenBuilderTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakAccessTokenBuilderTests.java new file mode 100644 index 000000000..278bb85ef --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakAccessTokenBuilderTests.java @@ -0,0 +1,122 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.test.oauth.profile; + +import static it.infn.mw.iam.core.oauth.granters.TokenExchangeTokenGranter.TOKEN_EXCHANGE_GRANT_TYPE; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.Mockito.when; + +import java.time.Clock; +import java.time.Instant; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mitre.oauth2.model.ClientDetailsEntity; +import org.mitre.oauth2.model.OAuth2AccessTokenEntity; +import org.mitre.openid.connect.model.UserInfo; +import org.mitre.openid.connect.service.ScopeClaimTranslationService; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.security.oauth2.common.exceptions.InvalidRequestException; +import org.springframework.security.oauth2.provider.OAuth2Authentication; + +import com.google.common.collect.Maps; + +import it.infn.mw.iam.config.IamProperties; +import it.infn.mw.iam.core.oauth.profile.iam.ClaimValueHelper; +import it.infn.mw.iam.core.oauth.profile.keycloak.KeycloakGroupHelper; +import it.infn.mw.iam.core.oauth.profile.keycloak.KeycloakProfileAccessTokenBuilder; +import it.infn.mw.iam.test.util.oauth.MockOAuth2Request; + +@SuppressWarnings("deprecation") +@RunWith(MockitoJUnitRunner.class) +public class KeycloakAccessTokenBuilderTests { + + IamProperties properties = new IamProperties(); + + @Mock + ScopeClaimTranslationService scService; + + @Mock + ClaimValueHelper claimValueHelper; + + @Mock + OAuth2AccessTokenEntity tokenEntity; + + @Mock + ClientDetailsEntity client; + + @Mock + OAuth2Authentication authentication; + + @Spy + MockOAuth2Request oauth2Request = + new MockOAuth2Request("clientId", new String[] {"openid", "profile"}); + + @Mock + UserInfo userInfo; + + final Instant now = Clock.systemDefaultZone().instant(); + + final KeycloakGroupHelper groupHelper = new KeycloakGroupHelper(); + + KeycloakProfileAccessTokenBuilder tokenBuilder; + + @Before + public void setup() { + + tokenBuilder = + new KeycloakProfileAccessTokenBuilder(properties, groupHelper); + when(tokenEntity.getExpiration()).thenReturn(null); + when(tokenEntity.getClient()).thenReturn(client); + when(client.getClientId()).thenReturn("client"); + // when(authentication.getName()).thenReturn("auth-name"); + when(authentication.getOAuth2Request()).thenReturn(oauth2Request); + // when(authentication.isClientOnly()).thenReturn(false); + when(userInfo.getSub()).thenReturn("userinfo-sub"); + when(oauth2Request.getGrantType()).thenReturn(TOKEN_EXCHANGE_GRANT_TYPE); + } + + + @Test(expected = InvalidRequestException.class) + public void testMissingSubjectTokenTokenExchangeErrors() { + try { + tokenBuilder.buildAccessToken(tokenEntity, authentication, userInfo, now); + } catch (InvalidRequestException e) { + assertThat(e.getMessage(), containsString("subject_token not found")); + throw e; + } + } + + @Test(expected = InvalidRequestException.class) + public void testSubjectTokenNotParsable() { + Map paramsMap = Maps.newHashMap(); + paramsMap.put("subject_token", "3427thjdfhgejt73ja"); + + oauth2Request.setRequestParameters(paramsMap); + try { + tokenBuilder.buildAccessToken(tokenEntity, authentication, userInfo, now); + } catch (InvalidRequestException e) { + assertThat(e.getMessage(), containsString("Error parsing subject token")); + throw e; + } + } + +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakProfileIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakProfileIntegrationTests.java new file mode 100644 index 000000000..58f552fd8 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakProfileIntegrationTests.java @@ -0,0 +1,178 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.test.oauth.profile; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.nullValue; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic; +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; + +import java.util.List; + +import org.assertj.core.util.Lists; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringRunner; + +import com.nimbusds.jwt.JWT; +import com.nimbusds.jwt.JWTParser; + +import it.infn.mw.iam.test.oauth.EndpointsTestUtils; +import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; + +@RunWith(SpringRunner.class) +@IamMockMvcIntegrationTest +@TestPropertySource(properties = {"iam.jwt-profile.default-profile=kc",}) +public class KeycloakProfileIntegrationTests extends EndpointsTestUtils { + + private static final String CLIENT_ID = "password-grant"; + private static final String CLIENT_SECRET = "secret"; + private static final String USERNAME = "test"; + private static final String PASSWORD = "password"; + protected static final String KC_GROUP_CLAIM = "roles"; + + private String getAccessTokenForUser(String scopes) throws Exception { + + return new AccessTokenGetter().grantType("password") + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .username(USERNAME) + .password(PASSWORD) + .scope(scopes) + .getAccessTokenValue(); + } + + private String getAccessTokenWithAudience(String scopes, String audience) throws Exception { + + return new AccessTokenGetter().grantType("password") + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .username(USERNAME) + .password(PASSWORD) + .scope(scopes) + .audience(audience) + .getAccessTokenValue(); + } + + @Test + public void testKeycloakProfileAccessToken() throws Exception { + JWT token = JWTParser.parse(getAccessTokenForUser("openid profile")); + + assertThat(token.getJWTClaimsSet().getClaim("scope"), is("openid profile")); + assertThat(token.getJWTClaimsSet().getClaim("nbf"), notNullValue()); + assertThat(token.getJWTClaimsSet().getClaim("groups"), nullValue()); + assertThat(token.getJWTClaimsSet().getClaim("roles"), notNullValue()); + List roles = + Lists.newArrayList(token.getJWTClaimsSet().getStringArrayClaim(KC_GROUP_CLAIM)); + assertThat(roles, hasSize(2)); + assertThat(roles, hasItem("Analysis")); + assertThat(roles, hasItem("Production")); + } + + @Test + public void testKeycloakProfileAccessTokenForUserNotInGroups() throws Exception { + String accessTokenString = (String) new AccessTokenGetter().grantType("password") + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .username("admin") + .password("password") + .scope("openid profile") + .getAccessTokenValue(); + + assertThat(!accessTokenString.contains("roles"), is(true)); + + } + + @Test + public void testKeycloakProfileAccessTokenWithClientCredentials() throws Exception { + String accessTokenString = (String) new AccessTokenGetter().grantType("client_credentials") + .clientId("client-cred") + .clientSecret("secret") + .scope("openid profile") + .getAccessTokenValue(); + + assertThat(!accessTokenString.contains("roles"), is(true)); + + } + + @Test + public void testKeycloackProfileIntrospect() throws Exception { + + JWT token = JWTParser.parse(getAccessTokenForUser("openid profile")); + + // @formatter:off + mvc.perform(post("/introspect") + .with(httpBasic(CLIENT_ID, CLIENT_SECRET)) + .param("token", token.getParsedString())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.active", equalTo(true))) + .andExpect(jsonPath("$." + KC_GROUP_CLAIM, containsInAnyOrder("Analysis", "Production"))) + .andExpect(jsonPath("$." + KC_GROUP_CLAIM, hasSize(equalTo(2)))) + .andExpect(jsonPath("$.iss", equalTo("http://localhost:8080/"))) + .andExpect(jsonPath("$.scope", containsString("openid"))) + .andExpect(jsonPath("$.scope", containsString("profile"))); + // @formatter:on + + } + + @Test + public void testKeycloackProfileIntrospectWithAudience() throws Exception { + + JWT token = JWTParser.parse(getAccessTokenWithAudience("openid profile", "myAudience")); + + // @formatter:off + mvc.perform(post("/introspect") + .with(httpBasic(CLIENT_ID, CLIENT_SECRET)) + .param("token", token.getParsedString())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.active", equalTo(true))) + .andExpect(jsonPath("$.aud", equalTo("myAudience"))); + // @formatter:on + + } + + @Test + public void testKeycloackProfileForUserNotInGroups() throws Exception { + + String accessTokenString = (String) new AccessTokenGetter().grantType("password") + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .username("admin") + .password("password") + .scope("openid profile") + .getAccessTokenValue(); + + // @formatter:off + mvc.perform(post("/introspect") + .with(httpBasic(CLIENT_ID, CLIENT_SECRET)) + .param("token", accessTokenString)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.active", equalTo(true))) + .andExpect(jsonPath("$.roles").doesNotExist()); + // @formatter:on + + } +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakProfileUserInfoTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakProfileUserInfoTests.java new file mode 100644 index 000000000..047cd3533 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/KeycloakProfileUserInfoTests.java @@ -0,0 +1,129 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.test.oauth.profile; + +import static org.hamcrest.CoreMatchers.hasItems; +import static org.hamcrest.Matchers.nullValue; + +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpStatus; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringRunner; + +import io.restassured.RestAssured; +import it.infn.mw.iam.test.TestUtils; +import it.infn.mw.iam.test.util.annotation.IamRandomPortIntegrationTest; + +@RunWith(SpringRunner.class) +@IamRandomPortIntegrationTest +@TestPropertySource(properties = { +// @formatter:off + "iam.jwt-profile.default-profile=kc", + // @formatter:on +}) +public class KeycloakProfileUserInfoTests { + + @Value("${local.server.port}") + private Integer iamPort; + + private static final String USERNAME = "test"; + private static final String PASSWORD = "password"; + + private String userinfoUrl; + private static final String USERINFO_URL_TEMPLATE = "http://localhost:%d/userinfo"; + + @BeforeClass + public static void init() { + TestUtils.initRestAssured(); + } + + @Before + public void setup() { + RestAssured.enableLoggingOfRequestAndResponseIfValidationFails(); + RestAssured.port = iamPort; + userinfoUrl = String.format(USERINFO_URL_TEMPLATE, iamPort); + } + + @Test + public void testUserinfoResponseWithGroups() { + String accessToken = TestUtils.passwordTokenGetter() + .port(iamPort) + .username(USERNAME) + .password(PASSWORD) + .scope("openid profile") + .getAccessToken(); + + RestAssured.given() + .header("Authorization", String.format("Bearer %s", accessToken)) + .when() + .get(userinfoUrl) + .then() + .statusCode(HttpStatus.OK.value()) + .body("\"roles\"", hasItems("Analysis", "Production")); + } + + @Test + public void testUserinfoResponseWithoutGroups() { + String accessToken = TestUtils.passwordTokenGetter() + .port(iamPort) + .username(USERNAME) + .password(PASSWORD) + .scope("openid") + .getAccessToken(); + + RestAssured.given() + .header("Authorization", String.format("Bearer %s", accessToken)) + .when() + .get(userinfoUrl) + .then() + .statusCode(HttpStatus.OK.value()) + .body("\"roles\"", nullValue()); + } + + @Test + public void testUserinfoResponseWithoutGroupsTwo() { + String accessToken = TestUtils.passwordTokenGetter() + .port(iamPort) + .username("admin") + .password(PASSWORD) + .scope("openid profile") + .getAccessToken(); + + RestAssured.given() + .header("Authorization", String.format("Bearer %s", accessToken)) + .when() + .get(userinfoUrl) + .then() + .statusCode(HttpStatus.OK.value()) + .body("\"roles\"", nullValue()); + } + + @Test + public void testUserinfoResponseWithoutUser() { + String accessToken = TestUtils.clientCredentialsTokenGetter().port(iamPort).getAccessToken(); + + RestAssured.given() + .header("Authorization", String.format("Bearer %s", accessToken)) + .when() + .get(userinfoUrl) + .then() + .statusCode(HttpStatus.FORBIDDEN.value()); + } +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/ProfileSelectorTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/ProfileSelectorTests.java index d3470cc5c..5703fea6b 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/ProfileSelectorTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/ProfileSelectorTests.java @@ -58,6 +58,9 @@ public class ProfileSelectorTests { @Mock JWTProfile wlcgProfile; + @Mock + JWTProfile kcProfile; + ScopeAwareProfileResolver profileResolver; @Before @@ -67,6 +70,7 @@ public void setup() { profileMap.put(ScopeAwareProfileResolver.AARC_PROFILE_ID, aarcProfile); profileMap.put(ScopeAwareProfileResolver.IAM_PROFILE_ID, iamProfile); profileMap.put(ScopeAwareProfileResolver.WLCG_PROFILE_ID, wlcgProfile); + profileMap.put(ScopeAwareProfileResolver.KC_PROFILE_ID, kcProfile); profileResolver = new ScopeAwareProfileResolver(iamProfile, profileMap, clientsService); } @@ -133,5 +137,24 @@ public void multipleProfilesLeadToDefaultProfile() throws Exception { profile = profileResolver.resolveProfile(CLIENT_ID); assertThat(profile, is(iamProfile)); + + when(client.getScope()).thenReturn( + newLinkedHashSet(() -> Arrays.stream(new String[] {"openid", "kc"}).iterator())); + + profile = profileResolver.resolveProfile(CLIENT_ID); + assertThat(profile, is(kcProfile)); + + when(client.getScope()).thenReturn( + newLinkedHashSet(() -> Arrays.stream(new String[] {"openid", "kc", "iam"}).iterator())); + + profile = profileResolver.resolveProfile(CLIENT_ID); + assertThat(profile, is(iamProfile)); + + when(client.getScope()).thenReturn( + newLinkedHashSet(() -> Arrays.stream(new String[] {"openid", "kc", "wlcg"}).iterator())); + + profile = profileResolver.resolveProfile(CLIENT_ID); + assertThat(profile, is(iamProfile)); + } } From 8e382421b0aa3bc0c5597ea9b062e551080472b9 Mon Sep 17 00:00:00 2001 From: Roberta Miccoli <85555840+rmiccoli@users.noreply.github.com> Date: Thu, 14 Sep 2023 15:42:21 +0200 Subject: [PATCH 19/22] Consider scope matcher based on string equality for custom scopes (#642) --- .../java/it/infn/mw/iam/config/IamConfig.java | 5 +- .../matchers/DefaultScopeMatcherRegistry.java | 12 +- .../profile/WLCGProfileIntegrationTests.java | 11 ++ .../test/oauth/scope/ScopeRegistryTests.java | 58 +++++--- .../oauth/scope/pdp/ScopePolicyPdpTests.java | 125 +++++++++++++----- 5 files changed, 156 insertions(+), 55 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java index d069e9234..f3d3e72ae 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/IamConfig.java @@ -25,6 +25,7 @@ import java.util.Map; import org.h2.server.web.WebServlet; +import org.mitre.oauth2.repository.SystemScopeRepository; import org.mitre.oauth2.service.IntrospectionResultAssembler; import org.mitre.oauth2.service.impl.DefaultIntrospectionResultAssembler; import org.mitre.oauth2.service.impl.DefaultOAuth2AuthorizationCodeService; @@ -288,9 +289,9 @@ FilterRegistrationBean aupSignatureCheckFilter(AUPSignatureChe @Bean - ScopeMatcherRegistry customScopeMatchersRegistry(ScopeMatchersProperties properties) { + ScopeMatcherRegistry customScopeMatchersRegistry(ScopeMatchersProperties properties, SystemScopeRepository scopeRepo) { ScopeMatchersPropertiesParser parser = new ScopeMatchersPropertiesParser(); - return new DefaultScopeMatcherRegistry(parser.parseScopeMatchersProperties(properties)); + return new DefaultScopeMatcherRegistry(parser.parseScopeMatchersProperties(properties), scopeRepo); } @Bean diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/DefaultScopeMatcherRegistry.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/DefaultScopeMatcherRegistry.java index 58ac2be88..21bcd02b4 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/DefaultScopeMatcherRegistry.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/matchers/DefaultScopeMatcherRegistry.java @@ -17,6 +17,8 @@ import java.util.Set; +import org.mitre.oauth2.model.SystemScope; +import org.mitre.oauth2.repository.SystemScopeRepository; import org.springframework.cache.annotation.Cacheable; import org.springframework.security.oauth2.provider.ClientDetails; @@ -26,11 +28,14 @@ public class DefaultScopeMatcherRegistry implements ScopeMatcherRegistry { public static final String SCOPE_CACHE_KEY = "scope-matcher"; - + private final Set customMatchers; - public DefaultScopeMatcherRegistry(Set customMatchers) { + private final SystemScopeRepository scopeRepo; + + public DefaultScopeMatcherRegistry(Set customMatchers, SystemScopeRepository scopeRepo) { this.customMatchers = customMatchers; + this.scopeRepo = scopeRepo; } @Override @@ -49,7 +54,10 @@ public Set findMatchersForClient(ClientDetails client) { @Override public ScopeMatcher findMatcherForScope(String scope) { + Set systemScopes = scopeRepo.getAll(); + return customMatchers.stream() + .filter(s -> systemScopes.toString().contains(scope)) .filter(m -> m.matches(scope)) .findFirst() .orElse(StringEqualsScopeMatcher.stringEqualsMatcher(scope)); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/WLCGProfileIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/WLCGProfileIntegrationTests.java index d01bd692b..8339de7f8 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/WLCGProfileIntegrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/profile/WLCGProfileIntegrationTests.java @@ -42,6 +42,8 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mitre.oauth2.model.SystemScope; +import org.mitre.oauth2.service.SystemScopeService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; @@ -130,9 +132,18 @@ public class WLCGProfileIntegrationTests extends EndpointsTestUtils { @Autowired private MockOAuth2Filter oauth2Filter; + @Autowired + private SystemScopeService scopeService; + @Before public void setup() { oauth2Filter.cleanupSecurityContext(); + SystemScope wlcgGroupsScope = new SystemScope("wlcg.groups"); + SystemScope storageReadScope = new SystemScope("storage.read:/"); + SystemScope storageWriteScope = new SystemScope("storage.write:/"); + scopeService.save(wlcgGroupsScope); + scopeService.save(storageReadScope); + scopeService.save(storageWriteScope); } @After diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/ScopeRegistryTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/ScopeRegistryTests.java index 8e7f6fefb..fd9c334f8 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/ScopeRegistryTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/ScopeRegistryTests.java @@ -18,6 +18,7 @@ import static com.google.common.collect.Sets.newHashSet; import static it.infn.mw.iam.core.oauth.scope.matchers.RegexpScopeMatcher.regexpMatcher; import static it.infn.mw.iam.core.oauth.scope.matchers.StringEqualsScopeMatcher.stringEqualsMatcher; +import static it.infn.mw.iam.core.oauth.scope.matchers.StructuredPathScopeMatcher.structuredPathMatcher; import static java.util.Collections.emptySet; import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.CoreMatchers.not; @@ -28,8 +29,11 @@ import java.util.Set; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mitre.oauth2.model.SystemScope; +import org.mitre.oauth2.repository.SystemScopeRepository; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import org.springframework.security.oauth2.provider.ClientDetails; @@ -43,52 +47,66 @@ @RunWith(MockitoJUnitRunner.class) public class ScopeRegistryTests { - + @Mock ClientDetails client; - + + @Mock + SystemScopeRepository scopeRepo; + + @Before + public void setup() { + SystemScope testScope = new SystemScope("test:/whatever"); + when(scopeRepo.getAll()).thenReturn(Sets.newHashSet(testScope)); + } + @Test public void testEmptyScopes() { - - DefaultScopeMatcherRegistry matcherRegistry = new DefaultScopeMatcherRegistry(emptySet()); - - when(client.getScope()).thenReturn(Sets.newHashSet("openid","profile")); + + DefaultScopeMatcherRegistry matcherRegistry = + new DefaultScopeMatcherRegistry(emptySet(), scopeRepo); + + when(client.getScope()).thenReturn(Sets.newHashSet("openid", "profile")); Set matchers = matcherRegistry.findMatchersForClient(client); - + assertThat(matchers, not(nullValue())); assertThat(matchers, hasSize(2)); assertThat(matchers, hasItem(stringEqualsMatcher("openid"))); assertThat(matchers, hasItem(stringEqualsMatcher("profile"))); } - + @Test public void testNonMatchingScope() { - - DefaultScopeMatcherRegistry matcherRegistry = new DefaultScopeMatcherRegistry(newHashSet(regexpMatcher("^test:/.*$"))); - - when(client.getScope()).thenReturn(Sets.newHashSet("openid","profile")); + + DefaultScopeMatcherRegistry matcherRegistry = + new DefaultScopeMatcherRegistry(newHashSet(regexpMatcher("^test:/.*$")), scopeRepo); + + when(client.getScope()).thenReturn(Sets.newHashSet("openid", "profile")); Set matchers = matcherRegistry.findMatchersForClient(client); - + assertThat(matchers, not(nullValue())); assertThat(matchers, hasSize(2)); assertThat(matchers, hasItem(stringEqualsMatcher("openid"))); assertThat(matchers, hasItem(stringEqualsMatcher("profile"))); } - + @Test public void testMatchingScope() { - - DefaultScopeMatcherRegistry matcherRegistry = new DefaultScopeMatcherRegistry(newHashSet(regexpMatcher("^test:/.*$"))); - - when(client.getScope()).thenReturn(Sets.newHashSet("openid","profile", "test", "test:/whatever")); + + DefaultScopeMatcherRegistry matcherRegistry = + new DefaultScopeMatcherRegistry(newHashSet(regexpMatcher("^test:/.*$"), structuredPathMatcher("storage.create", "/")), scopeRepo); + + when(client.getScope()) + .thenReturn(Sets.newHashSet("openid", "profile", "test", "test:/whatever", "storage.create:/whatever")); Set matchers = matcherRegistry.findMatchersForClient(client); - + assertThat(matchers, not(nullValue())); - assertThat(matchers, hasSize(4)); + assertThat(matchers, hasSize(5)); assertThat(matchers, hasItem(stringEqualsMatcher("openid"))); assertThat(matchers, hasItem(stringEqualsMatcher("profile"))); assertThat(matchers, hasItem(stringEqualsMatcher("test"))); assertThat(matchers, hasItem(regexpMatcher("^test:/.*$"))); + assertThat(matchers, hasItem(stringEqualsMatcher("storage.create:/whatever"))); } } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/pdp/ScopePolicyPdpTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/pdp/ScopePolicyPdpTests.java index 8e1acdb7d..cc70bb036 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/pdp/ScopePolicyPdpTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/pdp/ScopePolicyPdpTests.java @@ -17,6 +17,8 @@ import static it.infn.mw.iam.persistence.model.IamScopePolicy.MatchingPolicy.PATH; +import static org.hamcrest.CoreMatchers.allOf; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItems; @@ -35,6 +37,7 @@ import org.mitre.oauth2.model.SystemScope; import org.mitre.oauth2.service.SystemScopeService; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.MockMvc; @@ -52,6 +55,7 @@ @RunWith(SpringRunner.class) +@ActiveProfiles({"h2-test", "h2", "saml", "registration", "wlcg-scopes"}) @IamMockMvcIntegrationTest public class ScopePolicyPdpTests extends ScopePolicyTestUtils { @@ -151,7 +155,7 @@ public void testGroupPolicyIsEnforced() { assertThat(filteredScopes, hasItems("openid", "profile")); } - + @Test public void testChainedOverrideAtGroupIsEnforced() { IamAccount testAccount = findTestAccount(); @@ -163,18 +167,18 @@ public void testChainedOverrideAtGroupIsEnforced() { IamScopePolicy gp = initPermitScopePolicy(); gp.linkGroup(firstGroup); gp.setScopes(Sets.newHashSet(OPENID, PROFILE)); - - + + policyScopeRepo.save(gp); - - Set filteredScopes = pdp - .filterScopes(Sets.newHashSet("openid", "profile"), testAccount); - + + Set filteredScopes = + pdp.filterScopes(Sets.newHashSet("openid", "profile"), testAccount); + assertThat(filteredScopes, hasSize(2)); assertThat(filteredScopes, hasItems("openid", "profile")); } - - + + @Test public void testChainedOverrideIsEnforced() { IamAccount testAccount = findTestAccount(); @@ -257,46 +261,72 @@ public void testConflictingGroupPolicyDenyOverrides2() { assertThat(filteredScopes, hasSize(2)); assertThat(filteredScopes, hasItems("openid", "profile")); } - - + + @Test public void testPathFiltering() { - + IamAccount testAccount = findTestAccount(); IamScopePolicy up = initDenyScopePolicy(); - + up.getScopes().add("read:/"); up.getScopes().add("write:/"); up.setMatchingPolicy(PATH); - + policyScopeRepo.save(up); - Set filteredScopes = - pdp.filterScopes(Sets.newHashSet("openid", "profile", "read:/", "write", "read:/sub/path"), testAccount); + Set filteredScopes = pdp.filterScopes( + Sets.newHashSet("openid", "profile", "read:/", "write", "read:/sub/path"), testAccount); assertThat(filteredScopes, hasSize(3)); assertThat(filteredScopes, hasItems("openid", "profile", "write")); } - + @Test public void testPathPermit() { - + IamAccount testAccount = findTestAccount(); IamScopePolicy up = initPermitScopePolicy(); - + up.getScopes().add("read:/"); up.getScopes().add("write:/"); up.setMatchingPolicy(PATH); - + policyScopeRepo.save(up); - Set filteredScopes = - pdp.filterScopes(Sets.newHashSet("openid", "profile", "read:/", "write", "read:/sub/path"), testAccount); + Set filteredScopes = pdp.filterScopes( + Sets.newHashSet("openid", "profile", "read:/", "write", "read:/sub/path"), testAccount); assertThat(filteredScopes, hasSize(5)); assertThat(filteredScopes, hasItems("openid", "profile", "write", "read:/", "read:/sub/path")); } + @Test + public void testPathForCustomScope() { + + IamAccount testAccount = findTestAccount(); + IamScopePolicy up = initDenyScopePolicy(); + + up.getScopes().add("storage.write:/"); + up.setMatchingPolicy(PATH); + + policyScopeRepo.save(up); + + up = initPermitScopePolicy(); + up.getScopes().add("storage.write:/path"); + up.linkAccount(testAccount); + up.setMatchingPolicy(PATH); + + policyScopeRepo.save(up); + + Set filteredScopes = pdp.filterScopes(Sets.newHashSet("openid", "profile", + "storage.write:/", "storage.write:/path", "storage.write:/path/sub"), testAccount); + + assertThat(filteredScopes, hasSize(4)); + assertThat(filteredScopes, + hasItems("openid", "profile", "storage.write:/path", "storage.write:/path/sub")); + } + @Test public void testMisspelledScopeInScopePolicy() throws Exception { @@ -309,16 +339,49 @@ public void testMisspelledScopeInScopePolicy() throws Exception { policyScopeRepo.save(up); mvc - .perform( - post("/token").with(httpBasic("password-grant", "secret")) - .param("grant_type", "password") - .param("username", "test") - .param("password", "password") - .param("scope", "openid storage.read:/")) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.error", equalTo("invalid_scope"))) - .andExpect(jsonPath("$.error_description", equalTo("Misspelled storage.read/ scope in the scope policy"))); + .perform(post("/token").with(httpBasic("password-grant", "secret")) + .param("grant_type", "password") + .param("username", "test") + .param("password", "password") + .param("scope", "openid storage.read:/")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error", equalTo("invalid_scope"))) + .andExpect(jsonPath("$.error_description", + equalTo("Misspelled storage.read/ scope in the scope policy"))); + + } + + @Test + public void testFakeWLCGScopeAsCustomScopeNotIncluded() throws Exception { + + mvc + .perform(post("/token").with(httpBasic("password-grant", "secret")) + .param("grant_type", "password") + .param("username", "test") + .param("password", "password") + .param("scope", "openid storage.create:/")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.access_token").exists()) + .andExpect( + jsonPath("$.scope", allOf(containsString("openid"), containsString("storage.create:/")))); + + IamScopePolicy up = initDenyScopePolicy(); + up.getScopes().add("storage.create:/"); + up.setMatchingPolicy(PATH); + up.linkAccount(findTestAccount()); + up = policyScopeRepo.save(up); + mvc + .perform(post("/token").with(httpBasic("password-grant", "secret")) + .param("grant_type", "password") + .param("username", "test") + .param("password", "password") + .param("scope", "openid storage.create:/")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.access_token").exists()) + .andExpect(jsonPath("$.scope", allOf(containsString("openid")))); + + policyScopeRepo.delete(up); } } From 6774b6755ccf5a217f910469046d8f2aab149232 Mon Sep 17 00:00:00 2001 From: Saiteja Reddy Vennapusa <127398882+Sae126V@users.noreply.github.com> Date: Thu, 14 Sep 2023 14:47:03 +0100 Subject: [PATCH 20/22] Add support for displaying specific language name in federation Metadata. (#640) --- .../saml/DefaultMetadataLookupService.java | 37 +++++++- .../iam/authn/saml/model/IdpDescription.java | 18 +++- .../saml/MetadataLookupServiceTests.java | 84 +++++++++++-------- 3 files changed, 101 insertions(+), 38 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/saml/DefaultMetadataLookupService.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/saml/DefaultMetadataLookupService.java index 8d0272bd0..3ab833ea6 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/authn/saml/DefaultMetadataLookupService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/saml/DefaultMetadataLookupService.java @@ -26,11 +26,13 @@ import java.util.Set; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.opensaml.common.xml.SAMLConstants; import org.opensaml.saml2.metadata.EntityDescriptor; import org.opensaml.saml2.metadata.IDPSSODescriptor; +import org.opensaml.saml2.metadata.LocalizedString; import org.opensaml.saml2.metadata.provider.MetadataProvider; import org.opensaml.saml2.metadata.provider.MetadataProviderException; import org.opensaml.saml2.metadata.provider.ObservableMetadataProvider; @@ -49,7 +51,8 @@ @Component @Profile("saml") -public class DefaultMetadataLookupService implements MetadataLookupService, ObservableMetadataProvider.Observer { +public class DefaultMetadataLookupService + implements MetadataLookupService, ObservableMetadataProvider.Observer { private static final int MAX_RESULTS = 20; private static final Logger LOG = LoggerFactory.getLogger(DefaultMetadataLookupService.class); @@ -70,7 +73,7 @@ private void initializeMetadataSet() throws MetadataProviderException { final Instant startTime = Instant.now(); LOG.debug("Initializing IdP descriptor list from metadata"); - + Set newDescriptions = new HashSet<>(); for (String idpName : metadataManager.getIDPEntityNames()) { @@ -107,6 +110,8 @@ private IdpDescription descriptionFromMetadata(EntityDescriptor descriptor) { if (!uiInfo.getDisplayNames().isEmpty()) { result.setOrganizationName(uiInfo.getDisplayNames().get(0).getName().getLocalString()); + result + .setDisplayNames(uiInfo.getDisplayNames().stream().map(dn -> dn.getName()).toList()); } } } @@ -140,6 +145,17 @@ private Optional> lookupByEntityId(String text) { public List lookupIdp(String text) { List result = new ArrayList<>(); + String textToFind = text.toLowerCase(); + + Predicate filterForDescriptions = description -> { + if (description.getDisplayNames() != null) { + return description.getDisplayNames() + .stream() + .anyMatch(name -> name.getLocalString().toLowerCase().contains(textToFind)); + } else { + return description.getEntityId().toLowerCase().contains(textToFind); + } + }; lookupByEntityId(text).ifPresent(result::addAll); @@ -149,9 +165,24 @@ public List lookupIdp(String text) { try { lock.readLock().lock(); + return descriptions.stream() - .filter(p -> p.getOrganizationName().toLowerCase().contains(text.toLowerCase())) + .filter(filterForDescriptions) .limit(MAX_RESULTS) + .map(description -> { + List displayNames = description.getDisplayNames(); + if (displayNames != null) { + + for (LocalizedString displayName : displayNames) { + String localString = displayName.getLocalString(); + if (localString.toLowerCase().contains(textToFind)) { + description.setOrganizationName(localString); + break; + } + } + } + return description; + }) .collect(Collectors.toList()); } finally { lock.readLock().unlock(); diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/saml/model/IdpDescription.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/saml/model/IdpDescription.java index 039b3572f..7b92df54d 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/authn/saml/model/IdpDescription.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/saml/model/IdpDescription.java @@ -15,15 +15,20 @@ */ package it.infn.mw.iam.authn.saml.model; +import java.util.List; + +import org.opensaml.saml2.metadata.LocalizedString; + import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; @JsonInclude(Include.NON_EMPTY) -public class IdpDescription { +public class IdpDescription { private String entityId; private String organizationName; private String imageUrl; + private List displayNames; public String getEntityId() { return entityId; @@ -49,9 +54,18 @@ public void setImageUrl(String imageUrl) { this.imageUrl = imageUrl; } + public List getDisplayNames() { + return displayNames; + } + + public void setDisplayNames(List displayNames) { + this.displayNames = displayNames; + } + @Override public String toString() { return "IdpDescription [entityId=" + entityId + ", organizationName=" + organizationName - + ", imageUrl=" + imageUrl + "]"; + + ", imageUrl=" + imageUrl + ", displayNames=" + displayNames + "]"; } + } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/saml/MetadataLookupServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/saml/MetadataLookupServiceTests.java index 7c21a92e7..5fdde229f 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/saml/MetadataLookupServiceTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/ext_authn/saml/MetadataLookupServiceTests.java @@ -55,10 +55,11 @@ public class MetadataLookupServiceTests { public static final String IDP2_ENTITY_ID = "urn:test:idp2"; public static final String IDP3_ENTITY_ID = "urn:test:idp3"; public static final String IDP4_ENTITY_ID = "urn:test:idp4"; - + public static final String IDP1_ORGANIZATION_NAME = "IDP1 organization"; public static final String IDP2_ORGANIZATION_NAME = "IDP2 organization"; - + public static final String IDP4_ORGANIZATION_NAME = "IDP4 organization"; + @Mock MetadataManager manager; @@ -75,23 +76,26 @@ public class MetadataLookupServiceTests { UIInfo idp1UIInfo, idp2UIInfo, idp4UIInfo; @Mock - DisplayName idp1DisplayName, idp2DisplayName, idp4DisplayName; + DisplayName idp1DisplayName, idp1ItDisplayName, idp2DisplayName, idp4DisplayName; @Mock - LocalizedString idp1LocalizedString, idp2LocalizedString, idp4LocalizedString; + LocalizedString idp1LocalizedString, idp1ItLocalizedString, idp2LocalizedString, + idp4LocalizedString; @Before public void setup() throws MetadataProviderException { when(idp1LocalizedString.getLocalString()).thenReturn(IDP1_ORGANIZATION_NAME); + when(idp1ItLocalizedString.getLocalString()).thenReturn("IDP1 organizzazione"); when(idp1DisplayName.getName()).thenReturn(idp1LocalizedString); - when(idp1UIInfo.getDisplayNames()).thenReturn(asList(idp1DisplayName)); + when(idp1ItDisplayName.getName()).thenReturn(idp1ItLocalizedString); + when(idp1UIInfo.getDisplayNames()).thenReturn(asList(idp1DisplayName, idp1ItDisplayName)); when(idp2LocalizedString.getLocalString()).thenReturn(IDP2_ORGANIZATION_NAME); when(idp2DisplayName.getName()).thenReturn(idp2LocalizedString); when(idp2UIInfo.getDisplayNames()).thenReturn(asList(idp2DisplayName)); - when(idp4LocalizedString.getLocalString()).thenReturn(""); + when(idp4LocalizedString.getLocalString()).thenReturn(IDP4_ORGANIZATION_NAME); when(idp4DisplayName.getName()).thenReturn(idp4LocalizedString); when(idp4UIInfo.getDisplayNames()).thenReturn(asList(idp4DisplayName)); @@ -102,7 +106,7 @@ public void setup() throws MetadataProviderException { .thenReturn(asList(idp2UIInfo)); when(idp4SsoExtensions.getUnknownXMLObjects(UIInfo.DEFAULT_ELEMENT_NAME)) - .thenReturn(asList(idp4UIInfo)); + .thenReturn(asList(idp4UIInfo)); when(idp1SsoDesc.getExtensions()).thenReturn(idp1SsoExtensions); @@ -115,7 +119,7 @@ public void setup() throws MetadataProviderException { when(idp2Desc.getEntityID()).thenReturn(IDP2_ENTITY_ID); when(idp2Desc.getIDPSSODescriptor(SAMLConstants.SAML20P_NS)).thenReturn(idp2SsoDesc); - + when(idp3Desc.getEntityID()).thenReturn(IDP3_ENTITY_ID); when(idp4Desc.getEntityID()).thenReturn(IDP4_ENTITY_ID); @@ -126,8 +130,8 @@ public void setup() throws MetadataProviderException { when(manager.getEntityDescriptor(IDP3_ENTITY_ID)).thenReturn(idp3Desc); when(manager.getEntityDescriptor(IDP4_ENTITY_ID)).thenReturn(idp4Desc); - when(manager.getIDPEntityNames()).thenReturn(Sets.newHashSet(IDP1_ENTITY_ID, IDP2_ENTITY_ID, - IDP3_ENTITY_ID, IDP4_ENTITY_ID)); + when(manager.getIDPEntityNames()) + .thenReturn(Sets.newHashSet(IDP1_ENTITY_ID, IDP2_ENTITY_ID, IDP3_ENTITY_ID, IDP4_ENTITY_ID)); } @@ -138,72 +142,86 @@ public void testServiceInitialization() throws MetadataProviderException { assertNotNull(service.listIdps()); List idps = service.listIdps(); - + assertThat(idps, hasSize(4)); - + assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP1_ENTITY_ID)), hasProperty("organizationName", is(IDP1_ORGANIZATION_NAME))))); - + assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP2_ENTITY_ID)), hasProperty("organizationName", is(IDP2_ORGANIZATION_NAME))))); - + assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP3_ENTITY_ID)), hasProperty("organizationName", is(IDP3_ENTITY_ID))))); assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP4_ENTITY_ID)), - hasProperty("organizationName", is(IDP4_ENTITY_ID))))); + hasProperty("organizationName", is(IDP4_ORGANIZATION_NAME))))); } - - + + @Test public void testEmptyMetadataInitialization() { when(manager.getIDPEntityNames()).thenReturn(emptySet()); DefaultMetadataLookupService service = new DefaultMetadataLookupService(manager); - + assertThat(service.listIdps(), hasSize(0)); } @Test - public void testLookupByOrganizationNameWorks() { + public void testEmptyTextToFind() { DefaultMetadataLookupService service = new DefaultMetadataLookupService(manager); - List idps = service.lookupIdp(IDP1_ORGANIZATION_NAME); - assertThat(idps, hasSize(1)); - - assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP1_ENTITY_ID)), + List idps = service.lookupIdp("noMatchOnTextToFind"); + assertThat(idps, hasSize(0)); + } + + @Test + public void testLookupByOrganizationNameWorks() { + DefaultMetadataLookupService service = new DefaultMetadataLookupService(manager); + + List idpsIt = service.lookupIdp("organizz"); + assertThat(idpsIt, hasSize(1)); + + assertThat(idpsIt, hasItem(allOf(hasProperty("entityId", is(IDP1_ENTITY_ID)), + hasProperty("organizationName", is("IDP1 organizzazione"))))); + + List idpsEn = service.lookupIdp(IDP1_ORGANIZATION_NAME); + assertThat(idpsEn, hasSize(1)); + + assertThat(idpsEn, hasItem(allOf(hasProperty("entityId", is(IDP1_ENTITY_ID)), hasProperty("organizationName", is(IDP1_ORGANIZATION_NAME))))); } - + @Test public void testPartialLookupWorks() { DefaultMetadataLookupService service = new DefaultMetadataLookupService(manager); - + List idps = service.lookupIdp("idp"); assertThat(idps, hasSize(4)); - + assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP1_ENTITY_ID)), hasProperty("organizationName", is(IDP1_ORGANIZATION_NAME))))); - + assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP2_ENTITY_ID)), hasProperty("organizationName", is(IDP2_ORGANIZATION_NAME))))); - + assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP3_ENTITY_ID)), hasProperty("organizationName", is(IDP3_ENTITY_ID))))); assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP4_ENTITY_ID)), - hasProperty("organizationName", is(IDP4_ENTITY_ID))))); + hasProperty("organizationName", is(IDP4_ORGANIZATION_NAME))))); } - + @Test public void testEntityIdLookupWorks() { - + DefaultMetadataLookupService service = new DefaultMetadataLookupService(manager); List idps = service.lookupIdp(IDP1_ENTITY_ID); assertThat(idps, hasSize(1)); - + assertThat(idps, hasItem(allOf(hasProperty("entityId", is(IDP1_ENTITY_ID)), hasProperty("organizationName", is(IDP1_ORGANIZATION_NAME))))); - + idps = service.lookupIdp("unknown"); assertThat(idps, hasSize(0)); } From b121ebd3592bedc01d39c7df6bd4d7d7bb987f51 Mon Sep 17 00:00:00 2001 From: Roberta Miccoli <85555840+rmiccoli@users.noreply.github.com> Date: Thu, 14 Sep 2023 15:54:41 +0200 Subject: [PATCH 21/22] Fix scope matching which affects device code flow (#649) --- .../oauth/scope/IamSystemScopeService.java | 2 +- .../oauth/devicecode/DeviceCodeTests.java | 12 ++++++- ...tructuredScopeRequestIntegrationTests.java | 35 ++++++++++++++----- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/IamSystemScopeService.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/IamSystemScopeService.java index 7f83355cf..4f655b70d 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/IamSystemScopeService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/oauth/scope/IamSystemScopeService.java @@ -37,7 +37,7 @@ public IamSystemScopeService(ScopeMatcherRegistry matcherRegistry) { public boolean scopesMatch(Set allowedScopes, Set requestedScopes) { Set allowedScopeMatchers = - requestedScopes.stream().map(scopeMatcherRegistry::findMatcherForScope).collect(toSet()); + allowedScopes.stream().map(scopeMatcherRegistry::findMatcherForScope).collect(toSet()); for (String rs : requestedScopes) { if (allowedScopeMatchers.stream().noneMatch(m -> m.matches(rs))) { diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/devicecode/DeviceCodeTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/devicecode/DeviceCodeTests.java index 34ed6b915..9e004c1f1 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/devicecode/DeviceCodeTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/devicecode/DeviceCodeTests.java @@ -34,6 +34,8 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.view; import java.io.UnsupportedEncodingException; +import java.util.Optional; +import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; @@ -47,6 +49,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Sets; import com.nimbusds.jwt.JWT; import com.nimbusds.jwt.JWTClaimsSet; import com.nimbusds.jwt.JWTParser; @@ -433,7 +436,7 @@ public void deviceCodeWorksForDynamicallyRegisteredClient() RegisteredClientDTO registrationResponse = objectMapper.readValue(clientJson, RegisteredClientDTO.class); - + ClientDetailsEntity newClient = clientRepo.findByClientId(registrationResponse.getClientId()).orElseThrow(); @@ -559,6 +562,13 @@ public void deviceCodeWorksForDynamicallyRegisteredClient() @Test public void publicClientDeviceCodeWorks() throws Exception { + Optional client = clientRepo.findByClientId(PUBLIC_DEVICE_CODE_CLIENT_ID); + Set scopes = Sets.newHashSet(); + scopes.add("openid"); + scopes.add("profile"); + if (client.isPresent()) { + client.get().setScope(scopes); + } String deviceResponse = mvc .perform(post(DEVICE_CODE_ENDPOINT).contentType(APPLICATION_FORM_URLENCODED) .param("client_id", PUBLIC_DEVICE_CODE_CLIENT_ID) diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/StructuredScopeRequestIntegrationTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/StructuredScopeRequestIntegrationTests.java index 37948896e..0a1fcb993 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/StructuredScopeRequestIntegrationTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/StructuredScopeRequestIntegrationTests.java @@ -29,9 +29,13 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.view; +import java.util.Optional; +import java.util.Set; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mitre.oauth2.model.ClientDetailsEntity; import org.mitre.oauth2.model.SystemScope; import org.mitre.oauth2.service.SystemScopeService; import org.springframework.beans.factory.annotation.Autowired; @@ -44,8 +48,10 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Sets; import it.infn.mw.iam.IamLoginService; +import it.infn.mw.iam.persistence.repository.client.IamClientRepository; import it.infn.mw.iam.test.oauth.EndpointsTestUtils; import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; @@ -67,6 +73,9 @@ public class StructuredScopeRequestIntegrationTests extends EndpointsTestUtils @Autowired private ObjectMapper mapper; + @Autowired + private IamClientRepository clientRepo; + @Before public void setup() throws Exception { SystemScope storageReadScope = new SystemScope("storage.read:/"); @@ -124,6 +133,17 @@ public void testIntrospectionResponse() throws Exception { @Test public void testDeviceCodeStructuredScopeRequest() throws Exception { + + Optional client = clientRepo.findByClientId(DEVICE_CODE_CLIENT_ID); + Set scopes = Sets.newHashSet(); + scopes.add("storage.read:/"); + scopes.add("openid"); + scopes.add("profile"); + scopes.add("offline_access"); + if (client.isPresent()) { + client.get().setScope(scopes); + } + String response = mvc .perform(post(DEVICE_CODE_ENDPOINT).contentType(APPLICATION_FORM_URLENCODED) .with(httpBasic(DEVICE_CODE_CLIENT_ID, DEVICE_CODE_CLIENT_SECRET)) @@ -243,11 +263,11 @@ public void testDeviceCodeStructuredScopeRequest() throws Exception { public void testRefreshTokenStructuredScopeRequest() throws Exception { DefaultOAuth2AccessToken tokenResponse = new AccessTokenGetter().grantType(PASSWORD_GRANT_TYPE) - .clientId(PASSWORD_CLIENT_ID) - .clientSecret(PASSWORD_CLIENT_SECRET) - .username(TEST_USERNAME) - .password(TEST_PASSWORD) - .scope("openid storage.read:/ offline_access") + .clientId(PASSWORD_CLIENT_ID) + .clientSecret(PASSWORD_CLIENT_SECRET) + .username(TEST_USERNAME) + .password(TEST_PASSWORD) + .scope("openid storage.read:/ offline_access") .getTokenResponseObject(); assertThat(tokenResponse.getScope(), hasItem("storage.read:/")); @@ -260,9 +280,8 @@ public void testRefreshTokenStructuredScopeRequest() throws Exception { .andExpect(status().isOk()) .andExpect(jsonPath("$.access_token").exists()) .andExpect(jsonPath("$.refresh_token").exists()) - .andExpect(jsonPath("$.scope", - allOf(containsString("storage.read:/test "), containsString("offline_access"), - containsString("openid")))); + .andExpect(jsonPath("$.scope", allOf(containsString("storage.read:/test "), + containsString("offline_access"), containsString("openid")))); } } From 986d684a5eacc6f0d4730ae5924e9c8acf998d2b Mon Sep 17 00:00:00 2001 From: Roberta Miccoli <85555840+rmiccoli@users.noreply.github.com> Date: Thu, 14 Sep 2023 15:56:37 +0200 Subject: [PATCH 22/22] Add missing "Reuse refresh token" box within client management page (#650) --- .../api/client/service/DefaultClientDefaultsService.java | 1 - .../client/tokensettings/tokensettings.component.html | 8 ++++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java index aa320aeb2..4a97f1b21 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientDefaultsService.java @@ -75,7 +75,6 @@ public ClientDetailsEntity setupClientDefaults(ClientDetailsEntity client) { } client.setAuthorities(Sets.newHashSet(Authorities.ROLE_CLIENT)); - return client; } diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html index 5b0fecc4c..3820c1cd0 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/tokensettings/tokensettings.component.html @@ -64,6 +64,14 @@

+
+
+ +
+