diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidPostLogoutRedirectURIs.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidPostLogoutRedirectURIs.java new file mode 100644 index 0000000000..1dc5b83e9b --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidPostLogoutRedirectURIs.java @@ -0,0 +1,36 @@ +/** + * 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.api.client.registration.validation; + +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import javax.validation.Constraint; +import javax.validation.Payload; + +@Retention(RUNTIME) +@Target(TYPE) +@Constraint(validatedBy = ValidPostLogoutRedirectURIsValidator.class) +public @interface ValidPostLogoutRedirectURIs { + String message() default "Invalid post logout redirect URIs configuration"; + + Class[] groups() default {}; + + Class[] payload() default {}; +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidPostLogoutRedirectURIsValidator.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidPostLogoutRedirectURIsValidator.java new file mode 100644 index 0000000000..5ad5d47e72 --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidPostLogoutRedirectURIsValidator.java @@ -0,0 +1,109 @@ +/** + * 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.api.client.registration.validation; + +import static java.lang.String.format; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Objects; +import java.util.Set; + +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; + +import org.springframework.context.annotation.Scope; +import org.springframework.stereotype.Component; + +import it.infn.mw.iam.api.common.client.RegisteredClientDTO; +import it.infn.mw.iam.api.common.client.TokenEndpointAuthenticationMethod; +import it.infn.mw.iam.core.oauth.consent.BlockedUriService; + +@Component +@Scope("prototype") +public class ValidPostLogoutRedirectURIsValidator + implements ConstraintValidator { + + private static final Set ALLOWED_SCHEMES = Set.of("https", "http"); + private static final Set ALLOWED_HTTP_HOSTS = + Set.of("localhost", "127.0.0.1", "[::1]", "[0:0:0:0:0:0:0:1]"); + private final BlockedUriService denyListService; + + public ValidPostLogoutRedirectURIsValidator(BlockedUriService denyListService) { + this.denyListService = denyListService; + } + + private boolean invalid(ConstraintValidatorContext context, String message) { + context.disableDefaultConstraintViolation(); + context.buildConstraintViolationWithTemplate(message).addConstraintViolation(); + return false; + } + + @Override + public boolean isValid(RegisteredClientDTO value, ConstraintValidatorContext context) { + + if (!Objects.isNull(value.getPostLogoutRedirectUris())) { + for (String uri : value.getPostLogoutRedirectUris()) { + if (!isValid(uri, value, context)) { + return false; + } + } + } + return true; + } + + private boolean isValid(String uri, RegisteredClientDTO value, + ConstraintValidatorContext context) { + + URI parsedUri; + + try { + parsedUri = new URI(uri); + } catch (URISyntaxException e) { + return invalid(context, "Invalid post logout redirect URI"); + } + + String scheme = parsedUri.getScheme(); + if (scheme == null || !ALLOWED_SCHEMES.contains(scheme.toLowerCase())) { + return invalid(context, format("Invalid post logout redirect URI scheme: %s", scheme)); + } + + boolean isHttp = "http".equalsIgnoreCase(scheme); + boolean isLoopback = ALLOWED_HTTP_HOSTS.contains(parsedUri.getHost()); + boolean isPublicClient = + TokenEndpointAuthenticationMethod.none.equals(value.getTokenEndpointAuthMethod()); + + if (isHttp && isPublicClient) { + return invalid(context, + "Plain http post logout redirect URIs are only allowed for confidential clients"); + } + + if (isHttp && !isLoopback) { + return invalid(context, + "Plain http post logout redirect URIs are only allowed for loopback hosts"); + } + + if (parsedUri.getFragment() != null) { + return invalid(context, "Invalid post logout redirect URI: contains a fragment"); + } + + if (denyListService.isBlockedUri(uri)) { + return invalid(context, format("Invalid post logout redirect URI: %s is not allowed", uri)); + } + + return true; + } +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidRedirectURIsValidator.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidRedirectURIsValidator.java index 372aef5fc1..dc35a65209 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidRedirectURIsValidator.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/registration/validation/ValidRedirectURIsValidator.java @@ -55,39 +55,46 @@ private boolean invalid(ConstraintValidatorContext context, String message) { @Override public boolean isValid(RegisteredClientDTO value, ConstraintValidatorContext context) { - if (Objects.isNull(value.getRedirectUris())) { - return true; - } - for (String uri : value.getRedirectUris()) { - URI parsedUri; - try { - parsedUri = new URI(uri); - } catch (URISyntaxException e) { - return invalid(context, "Invalid redirect URI"); + if (!Objects.isNull(value.getRedirectUris())) { + for (String uri : value.getRedirectUris()) { + if (!isValid(uri, context)) { + return false; + } } + } + return true; + } - if (ALLOWED_REDIRECT_URIS.contains(uri)) { - continue; - } + private boolean isValid(String uri, ConstraintValidatorContext context) { - String scheme = parsedUri.getScheme(); - if (scheme == null || !ALLOWED_SCHEMES.contains(scheme.toLowerCase())) { - return invalid(context, format("Invalid redirect URI scheme: %s", scheme)); - } + URI parsedUri; - if ("http".equalsIgnoreCase(scheme) - && !ALLOWED_HOSTS_FOR_HTTP.contains(parsedUri.getHost())) { - return invalid(context, "Plain http redirect URIs are only allowed for loopback"); - } + try { + parsedUri = new URI(uri); + } catch (URISyntaxException e) { + return invalid(context, "Invalid redirect URI"); + } - if (parsedUri.getFragment() != null) { - return invalid(context, "Invalid redirect URI: contains a fragment"); - } + if (ALLOWED_REDIRECT_URIS.contains(uri)) { + return true; + } - if (denyListService.isBlockedUri(uri)) { - return invalid(context, format("Invalid redirect URI: %s is not allowed", uri)); - } + String scheme = parsedUri.getScheme(); + if (scheme == null || !ALLOWED_SCHEMES.contains(scheme.toLowerCase())) { + return invalid(context, format("Invalid redirect URI scheme: %s", scheme)); + } + + if ("http".equalsIgnoreCase(scheme) && !ALLOWED_HOSTS_FOR_HTTP.contains(parsedUri.getHost())) { + return invalid(context, "Plain http redirect URIs are only allowed for loopback"); + } + + if (parsedUri.getFragment() != null) { + return invalid(context, "Invalid redirect URI: contains a fragment"); + } + + if (denyListService.isBlockedUri(uri)) { + return invalid(context, format("Invalid redirect URI: %s is not allowed", uri)); } return true; } 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 712d10a6e9..c9fa900c6c 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 @@ -35,9 +35,9 @@ 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; -import it.infn.mw.iam.persistence.model.IamFederatedClientEntity; import it.infn.mw.iam.persistence.model.ClientAuthMethod; import it.infn.mw.iam.persistence.model.ClientDetailsEntity; +import it.infn.mw.iam.persistence.model.IamFederatedClientEntity; @Component public class ClientConverter { @@ -63,7 +63,6 @@ private Set cloneSet(Set stringSet) { return result; } - public ClientDetailsEntity entityFromClientManagementRequest(RegisteredClientDTO dto) throws ParseException { ClientDetailsEntity client = entityFromRegistrationRequest(dto); @@ -90,8 +89,8 @@ public ClientDetailsEntity entityFromClientManagementRequest(RegisteredClientDTO } if (dto.getTokenEndpointAuthMethod() != null) { - client - .setTokenEndpointAuthMethod(ClientAuthMethod.getByValue(dto.getTokenEndpointAuthMethod().name())); + client.setTokenEndpointAuthMethod( + ClientAuthMethod.getByValue(dto.getTokenEndpointAuthMethod().name())); } client.setRequireAuthTime(Boolean.valueOf(dto.isRequireAuthTime())); @@ -100,8 +99,6 @@ public ClientDetailsEntity entityFromClientManagementRequest(RegisteredClientDTO return client; } - - public RegisteredClientDTO registeredClientDtoFromEntity(ClientDetailsEntity entity) { RegisteredClientDTO clientDTO = new RegisteredClientDTO(); @@ -117,6 +114,7 @@ public RegisteredClientDTO registeredClientDtoFromEntity(ClientDetailsEntity ent clientDTO.setJwksUri(entity.getJwksUri()); clientDTO.setRedirectUris(cloneSet(entity.getRedirectUris())); + clientDTO.setPostLogoutRedirectUris(entity.getPostLogoutRedirectUris()); clientDTO.setTokenEndpointAuthMethod(TokenEndpointAuthenticationMethod .valueOf(Optional.ofNullable(entity.getTokenEndpointAuthMethod()) @@ -198,6 +196,8 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto client.setRedirectUris(cloneSet(dto.getRedirectUris())); + client.setPostLogoutRedirectUris(dto.getPostLogoutRedirectUris()); + client.setScope(cloneSet(dto.getScope())); client.setGrantTypes(new HashSet<>()); @@ -209,7 +209,7 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto dto.getGrantTypes().stream().map(AuthorizationGrantType::getGrantType).collect(toSet())); } - if (dto.getScope().contains("offline_access")) { + if (client.getScope().contains("offline_access")) { client.getGrantTypes().add(AuthorizationGrantType.REFRESH_TOKEN.getGrantType()); } @@ -221,8 +221,8 @@ public ClientDetailsEntity entityFromRegistrationRequest(RegisteredClientDTO dto client.setContacts(cloneSet(dto.getContacts())); if (!isNull(dto.getTokenEndpointAuthMethod())) { - client - .setTokenEndpointAuthMethod(ClientAuthMethod.getByValue(dto.getTokenEndpointAuthMethod().name())); + client.setTokenEndpointAuthMethod( + ClientAuthMethod.getByValue(dto.getTokenEndpointAuthMethod().name())); } if (dto.getCodeChallengeMethod() != null) { diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/common/client/RegisteredClientDTO.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/common/client/RegisteredClientDTO.java index a12496de48..9af7469486 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/common/client/RegisteredClientDTO.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/common/client/RegisteredClientDTO.java @@ -44,6 +44,7 @@ import it.infn.mw.iam.api.client.registration.validation.OnDynamicClientRegistration; import it.infn.mw.iam.api.client.registration.validation.OnDynamicClientUpdate; import it.infn.mw.iam.api.client.registration.validation.ValidGrantType; +import it.infn.mw.iam.api.client.registration.validation.ValidPostLogoutRedirectURIs; import it.infn.mw.iam.api.client.registration.validation.ValidRedirectURIs; import it.infn.mw.iam.api.client.registration.validation.ValidTokenEndpointAuthMethod; import it.infn.mw.iam.api.common.ClientViews; @@ -56,6 +57,8 @@ OnDynamicClientRegistration.class, OnDynamicClientUpdate.class}) @ValidRedirectURIs(groups = {OnClientCreation.class, OnClientUpdate.class, OnDynamicClientRegistration.class, OnDynamicClientUpdate.class}) +@ValidPostLogoutRedirectURIs(groups = {OnClientCreation.class, OnClientUpdate.class, + OnDynamicClientRegistration.class, OnDynamicClientUpdate.class}) @ValidTokenEndpointAuthMethod(groups = {OnClientCreation.class, OnClientUpdate.class, OnDynamicClientRegistration.class, OnDynamicClientUpdate.class}) /** @@ -115,6 +118,11 @@ public class RegisteredClientDTO { ClientViews.NoSecretDynamicRegistration.class, ClientViews.DynamicRegistration.class}) private String clientUri; + @Valid + @JsonView({ClientViews.Limited.class, ClientViews.ClientManagement.class, + ClientViews.NoSecretDynamicRegistration.class, ClientViews.DynamicRegistration.class}) + private Set postLogoutRedirectUris; + @Size(max = 2048, groups = {OnDynamicClientRegistration.class, OnDynamicClientUpdate.class, OnClientCreation.class, OnClientUpdate.class}) @@ -324,6 +332,14 @@ public void setClientUri(String clientUri) { this.clientUri = clientUri; } + public Set getPostLogoutRedirectUris() { + return postLogoutRedirectUris; + } + + public void setPostLogoutRedirectUris(Set postLogoutRedirectUris) { + this.postLogoutRedirectUris = postLogoutRedirectUris; + } + public String getTosUri() { return tosUri; } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/oidc/OidcLogoutSuccessHandler.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/oidc/OidcLogoutSuccessHandler.java new file mode 100644 index 0000000000..c3307ab00b --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/oidc/OidcLogoutSuccessHandler.java @@ -0,0 +1,113 @@ +/** + * 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.authn.oidc; + +import java.io.IOException; +import java.text.ParseException; +import java.util.List; +import java.util.Optional; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.security.core.Authentication; +import org.springframework.security.web.authentication.logout.LogoutSuccessHandler; +import org.springframework.stereotype.Component; +import org.springframework.web.util.UriComponentsBuilder; + +import com.nimbusds.jwt.JWTClaimsSet; +import com.nimbusds.jwt.SignedJWT; + +import it.infn.mw.iam.authn.oidc.validator.OidcIdTokenHintValidator; +import it.infn.mw.iam.persistence.model.ClientDetailsEntity; +import it.infn.mw.iam.persistence.repository.client.IamClientRepository; + +@Component +public class OidcLogoutSuccessHandler implements LogoutSuccessHandler { + + private static final Logger LOG = LoggerFactory.getLogger(OidcLogoutSuccessHandler.class); + + private final IamClientRepository clientRepo; + private final OidcIdTokenHintValidator validator; + + public OidcLogoutSuccessHandler(IamClientRepository clientRepo, + OidcIdTokenHintValidator validator) { + this.clientRepo = clientRepo; + this.validator = validator; + } + + @Override + public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, + Authentication authentication) throws IOException { + + String idTokenHint = request.getParameter("id_token_hint"); + String redirectUri = request.getParameter("post_logout_redirect_uri"); + String state = request.getParameter("state"); + + String fallback = "/login?logout"; + + if (idTokenHint == null || redirectUri == null) { + LOG.debug("OIDC logout: missing id_token_hint or post_logout_redirect_uri"); + response.sendRedirect(fallback); + return; + } + + try { + SignedJWT idToken = SignedJWT.parse(idTokenHint); + + if (!validator.isValid(idToken)) { + LOG.debug("OIDC logout: id_token_hint validation failed"); + response.sendRedirect(fallback); + return; + } + + JWTClaimsSet idTokenClaims = idToken.getJWTClaimsSet(); + List audience = idTokenClaims.getAudience(); + + Optional client = audience.stream() + .map(clientRepo::findByClientId) + .filter(Optional::isPresent) + .map(Optional::get) + .findFirst(); + + if (client.isEmpty()) { + LOG.debug("OIDC logout: no matching client found in audiences {}", audience); + response.sendRedirect(fallback); + return; + } + + if (!client.get().getPostLogoutRedirectUris().contains(redirectUri)) { + LOG.debug("OIDC logout redirect denied: invalid redirect URI {}", redirectUri); + response.sendRedirect(fallback); + return; + } + + UriComponentsBuilder uriBuilder = UriComponentsBuilder.fromHttpUrl(redirectUri); + + if (state != null) { + uriBuilder.queryParam("state", state); + } + + response.sendRedirect(uriBuilder.toUriString()); + + } catch (ParseException e) { + LOG.debug("OIDC logout: invalid id_token_hint format", e); + response.sendRedirect(fallback); + } + } +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/authn/oidc/validator/OidcIdTokenHintValidator.java b/iam-login-service/src/main/java/it/infn/mw/iam/authn/oidc/validator/OidcIdTokenHintValidator.java new file mode 100644 index 0000000000..75685d3a1c --- /dev/null +++ b/iam-login-service/src/main/java/it/infn/mw/iam/authn/oidc/validator/OidcIdTokenHintValidator.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.authn.oidc.validator; + +import java.text.ParseException; +import java.util.List; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Component; + +import com.nimbusds.jwt.JWTClaimsSet; +import com.nimbusds.jwt.SignedJWT; + +import it.infn.mw.iam.core.jwk.JWTSigningAndValidationService; + +@Component +public class OidcIdTokenHintValidator { + + private static final Logger LOG = LoggerFactory.getLogger(OidcIdTokenHintValidator.class); + + private final JWTSigningAndValidationService jwtService; + private final String issuer; + + public OidcIdTokenHintValidator(JWTSigningAndValidationService jwtService, + @Value("${iam.issuer}") String issuer) { + this.jwtService = jwtService; + this.issuer = issuer; + } + + public boolean isValid(SignedJWT idToken) { + if (!jwtService.validateSignature(idToken)) { + return false; + } + + try { + JWTClaimsSet claims = idToken.getJWTClaimsSet(); + + String tokenIssuer = claims.getIssuer(); + + if (tokenIssuer == null || tokenIssuer.isBlank()) { + LOG.debug("No ID token issuer found"); + return false; + } + + if (!issuer.equals(tokenIssuer)) { + LOG.debug("ID token issuer is not the same as this server"); + return false; + } + + List audience = claims.getAudience(); + return audience != null && !audience.isEmpty(); + + } catch (ParseException e) { + LOG.debug("Invalid ID token claims"); + return false; + } + } +} diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java index b4e243c525..b75fe709f7 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/security/IamWebSecurityConfig.java @@ -68,6 +68,7 @@ import it.infn.mw.iam.authn.multi_factor_authentication.MultiFactorVerificationFilter; import it.infn.mw.iam.authn.oidc.OIDCAuthenticationFilter; import it.infn.mw.iam.authn.oidc.OIDCAuthenticationProvider; +import it.infn.mw.iam.authn.oidc.OidcLogoutSuccessHandler; import it.infn.mw.iam.authn.x509.IamX509AuthenticationProvider; import it.infn.mw.iam.authn.x509.IamX509AuthenticationUserDetailService; import it.infn.mw.iam.authn.x509.IamX509PreauthenticationProcessingFilter; @@ -139,6 +140,9 @@ public static class UserLoginConfig extends WebSecurityConfigurerAdapter { @Autowired private IamProperties iamProperties; + @Autowired + private OidcLogoutSuccessHandler oidcLogoutSuccessHandler; + @Autowired private IamTotpMfaProperties iamTotpMfaProperties; @@ -176,7 +180,6 @@ protected AuthenticationEntryPoint entryPoint() { return new HintAwareAuthenticationEntryPoint(delegate, hintService, aarcHintService); } - @Override protected void configure(final HttpSecurity http) throws Exception { @@ -211,6 +214,7 @@ protected void configure(final HttpSecurity http) throws Exception { .addFilterAfter(extendedHttpServletRequestFilter(), UsernamePasswordAuthenticationFilter.class) .logout() .logoutUrl("/logout") + .logoutSuccessHandler(oidcLogoutSuccessHandler) .and().anonymous() .and() .csrf() diff --git a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/clientsettings/clientsettings.component.html b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/clientsettings/clientsettings.component.html index 63049573a8..2624452fbb 100644 --- a/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/clientsettings/clientsettings.component.html +++ b/iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/components/clients/client/clientsettings/clientsettings.component.html @@ -66,10 +66,20 @@

{{$ctrl.client.registration_client_uri}}

- + + +
+
aud) { + JWTClaimsSet claims = new JWTClaimsSet.Builder().issuer(ISSUER).audience(aud).build(); + + SignedJWT jwt = new SignedJWT(new JWSHeader(JWSAlgorithm.HS256), claims); + + when(jwtService.validateSignature(jwt)).thenReturn(true); + + assertFalse(validator.isValid(jwt)); + } +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/logout/OidcLogoutSuccessHanlderTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/logout/OidcLogoutSuccessHanlderTests.java new file mode 100644 index 0000000000..eb164e5dc8 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/logout/OidcLogoutSuccessHanlderTests.java @@ -0,0 +1,148 @@ +/** + * 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.logout; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +import java.util.Optional; +import java.util.Set; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import com.nimbusds.jose.JOSEException; +import com.nimbusds.jose.JWSAlgorithm; +import com.nimbusds.jose.JWSHeader; +import com.nimbusds.jose.crypto.MACSigner; +import com.nimbusds.jwt.JWTClaimsSet; +import com.nimbusds.jwt.SignedJWT; + +import it.infn.mw.iam.authn.oidc.OidcLogoutSuccessHandler; +import it.infn.mw.iam.authn.oidc.validator.OidcIdTokenHintValidator; +import it.infn.mw.iam.persistence.model.ClientDetailsEntity; +import it.infn.mw.iam.persistence.repository.client.IamClientRepository; + +@ExtendWith(MockitoExtension.class) +class OidcLogoutSuccessHanlderTests { + + @Mock + private IamClientRepository clientRepo; + + @Mock + private OidcIdTokenHintValidator validator; + + private OidcLogoutSuccessHandler handler; + + private MockHttpServletRequest request; + private MockHttpServletResponse response; + + @BeforeEach + void setup() { + handler = new OidcLogoutSuccessHandler(clientRepo, validator); + request = new MockHttpServletRequest(); + response = new MockHttpServletResponse(); + } + + private void mockClient(String clientId, Set redirectUris) { + ClientDetailsEntity client = new ClientDetailsEntity(); + client.setClientId(clientId); + client.setPostLogoutRedirectUris(redirectUris); + + when(clientRepo.findByClientId(clientId)).thenReturn(Optional.of(client)); + } + + private String validJwtForClient(String clientId) throws JOSEException { + JWTClaimsSet claims = new JWTClaimsSet.Builder().audience(clientId) + .issuer("https://iam.example.org") + .subject("subject") + .build(); + + SignedJWT jwt = new SignedJWT(new JWSHeader(JWSAlgorithm.HS256), claims); + + jwt.sign(new MACSigner("01234567890123456789012345678901")); + + return jwt.serialize(); + } + + @Test + void missingParametersRedirectsToFallback() throws Exception { + handler.onLogoutSuccess(request, response, null); + + assertEquals("/login?logout", response.getRedirectedUrl()); + } + + @Test + void invalidIdTokenRedirectsToFallback() throws Exception { + request.setParameter("id_token_hint", "not-a-jwt"); + request.setParameter("post_logout_redirect_uri", "https://rp.example.org"); + + handler.onLogoutSuccess(request, response, null); + + assertEquals("/login?logout", response.getRedirectedUrl()); + } + + @Test + void tokenValidationFailsRedirectsToFallback() throws Exception { + String jwt = validJwtForClient("client"); + + request.setParameter("id_token_hint", jwt); + request.setParameter("post_logout_redirect_uri", "https://rp.example.org"); + + when(validator.isValid(any(SignedJWT.class))).thenReturn(false); + + handler.onLogoutSuccess(request, response, null); + + assertEquals("/login?logout", response.getRedirectedUrl()); + } + + @Test + void redirectUriNotRegisteredRedirectsToFallback() throws Exception { + String jwt = validJwtForClient("client"); + + request.setParameter("id_token_hint", jwt); + request.setParameter("post_logout_redirect_uri", "https://evil.example.org"); + + when(validator.isValid(any(SignedJWT.class))).thenReturn(true); + mockClient("client", Set.of("https://rp.example.org/logout")); + + handler.onLogoutSuccess(request, response, null); + + assertEquals("/login?logout", response.getRedirectedUrl()); + } + + @Test + void validLogoutRedirectsToPostLogoutUri() throws Exception { + String jwt = validJwtForClient("client"); + + request.setParameter("id_token_hint", jwt); + request.setParameter("post_logout_redirect_uri", "https://rp.example.org/logout"); + request.setParameter("state", "xyz"); + + when(validator.isValid(any(SignedJWT.class))).thenReturn(true); + mockClient("client", Set.of("https://rp.example.org/logout")); + + handler.onLogoutSuccess(request, response, null); + + assertEquals("https://rp.example.org/logout?state=xyz", response.getRedirectedUrl()); + } +} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientRegistrationServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientRegistrationServiceTests.java index b7eb4ae878..cbe8f67c4e 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientRegistrationServiceTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientRegistrationServiceTests.java @@ -18,7 +18,6 @@ import static it.infn.mw.iam.config.client_registration.ClientRegistrationProperties.ClientRegistrationAuthorizationPolicy.ADMINISTRATORS; import static it.infn.mw.iam.config.client_registration.ClientRegistrationProperties.ClientRegistrationAuthorizationPolicy.REGISTERED_USERS; import static java.util.Collections.emptySet; -import static org.assertj.core.api.Assertions.not; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.CoreMatchers.hasItems; @@ -39,6 +38,7 @@ import java.text.ParseException; import java.util.HashSet; +import java.util.List; import java.util.Optional; import java.util.Set; @@ -47,6 +47,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; @@ -98,6 +100,14 @@ @ActiveProfiles({"h2-test", "wlcg-scopes"}) class ClientRegistrationServiceTests extends TokenGetterUtils { + public static final List VALID_REDIRECT_URIS = List.of("http://localhost/redirect", + "http://127.0.0.1:8080/redirect", "http://[::1]:61023/oauth2redirect", + "http://[0:0:0:0:0:0:0:1]:61023/oauth2redirect", "edu.kit.data.oidc-agent:/redirect"); + + public static final List VALID_POST_LOGOUT_REDIRECT_URIS = + List.of("http://localhost/redirect", "http://127.0.0.1:8080/redirect", + "http://[::1]:61023/oauth2redirect", "http://[0:0:0:0:0:0:0:1]:61023/oauth2redirect"); + @Autowired IamClientRepository clientRepo; @@ -185,6 +195,15 @@ private RegisteredClientDTO createClientDTO(String uri) { return request; } + private RegisteredClientDTO createClientDTO(String redirectUri, String postLogoutRedirectUri) { + RegisteredClientDTO request = new RegisteredClientDTO(); + request.setClientName("example"); + request.setGrantTypes(Set.of(AuthorizationGrantType.CODE)); + request.setRedirectUris(Set.of(redirectUri)); + request.setPostLogoutRedirectUris(Set.of(postLogoutRedirectUri)); + return request; + } + @Test void testRegistrationRequestRequiresClientName() { ConstraintViolationException exception = @@ -205,7 +224,6 @@ void testNoRedirectUriWithAuthzCodeValidation() { RegisteredClientDTO request = new RegisteredClientDTO(); request.setClientName("example"); request.setGrantTypes(Set.of(AuthorizationGrantType.CODE)); - service.registerClient(request, userAuth); }); @@ -221,7 +239,6 @@ void testScopeValidation() { request.setClientName("example"); request.setGrantTypes(Set.of(AuthorizationGrantType.CLIENT_CREDENTIALS)); request.setScope(Set.of("")); - service.registerClient(request, userAuth); }); @@ -233,7 +250,6 @@ void testInvalidRedirectUriScheme() { ConstraintViolationException exception = Assertions.assertThrows(ConstraintViolationException.class, () -> { RegisteredClientDTO request = createClientDTO("not-a-uri"); - service.registerClient(request, userAuth); }); @@ -241,7 +257,6 @@ void testInvalidRedirectUriScheme() { exception = Assertions.assertThrows(ConstraintViolationException.class, () -> { RegisteredClientDTO request = createClientDTO("myapp://redirect"); - service.registerClient(request, userAuth); }); @@ -249,7 +264,6 @@ void testInvalidRedirectUriScheme() { exception = Assertions.assertThrows(ConstraintViolationException.class, () -> { RegisteredClientDTO request = createClientDTO("javascript:alert(1)"); - service.registerClient(request, userAuth); }); @@ -261,7 +275,6 @@ void testMalformedRedirectUri() { ConstraintViolationException exception = Assertions.assertThrows(ConstraintViolationException.class, () -> { RegisteredClientDTO request = createClientDTO(" "); - service.registerClient(request, userAuth); }); @@ -273,7 +286,6 @@ void testNonLoopbackHttpRedirectUri() { ConstraintViolationException exception = Assertions.assertThrows(ConstraintViolationException.class, () -> { RegisteredClientDTO request = createClientDTO("http://example/redirect"); - service.registerClient(request, userAuth); }); @@ -287,7 +299,6 @@ void testRedirectUriContainingFragments() { Assertions.assertThrows(ConstraintViolationException.class, () -> { RegisteredClientDTO request = createClientDTO("https://example.com/callback#token=abc123"); - service.registerClient(request, userAuth); }); @@ -296,38 +307,70 @@ void testRedirectUriContainingFragments() { @Test void testValidRedirectUris() { - Assertions.assertDoesNotThrow(() -> { - RegisteredClientDTO request = createClientDTO("http://localhost/redirect"); - - service.registerClient(request, userAuth); + VALID_REDIRECT_URIS.forEach(uri -> { + Assertions.assertDoesNotThrow(() -> { + RegisteredClientDTO request = createClientDTO(uri); + service.registerClient(request, userAuth); + }); }); + } - Assertions.assertDoesNotThrow(() -> { - RegisteredClientDTO request = createClientDTO("http://127.0.0.1:8080/redirect"); + @ParameterizedTest + @CsvSource({"'not-a-uri', 'null'", "'myapp://redirect', 'myapp'", + "'javascript:alert(1)', 'javascript'"}) + void testInvalidPostLogoutRedirectUriScheme(String postLogoutUri, String expectedScheme) { + final String redirectUri = VALID_REDIRECT_URIS.get(0); + final RegisteredClientDTO request = createClientDTO(redirectUri, postLogoutUri); - service.registerClient(request, userAuth); - }); + ConstraintViolationException exception = Assertions.assertThrows( + ConstraintViolationException.class, () -> service.registerClient(request, userAuth)); - Assertions.assertDoesNotThrow(() -> { - RegisteredClientDTO request = createClientDTO("http://[::1]:61023/oauth2redirect"); + assertThat(exception.getMessage(), + containsString("Invalid post logout redirect URI scheme: " + expectedScheme)); + } - service.registerClient(request, userAuth); - }); + @ParameterizedTest + @CsvSource({"' ', 'Invalid post logout redirect URI'", + "'http://example/redirect', 'Plain http post logout redirect URIs are only allowed for loopback'", + "'https://example.com/callback#token=abc123', 'Invalid post logout redirect URI: contains a fragment'"}) + void testInvalidPostLogoutRedirectUri(String postLogoutRedirectUri, String expectedMessage) { - Assertions.assertDoesNotThrow(() -> { - RegisteredClientDTO request = - createClientDTO("http://[0:0:0:0:0:0:0:1]:61023/oauth2redirect"); + final String redirectUri = VALID_REDIRECT_URIS.get(0); + final RegisteredClientDTO request = createClientDTO(redirectUri, postLogoutRedirectUri); - service.registerClient(request, userAuth); - }); + ConstraintViolationException exception = Assertions.assertThrows( + ConstraintViolationException.class, () -> service.registerClient(request, userAuth)); - Assertions.assertDoesNotThrow(() -> { - RegisteredClientDTO request = createClientDTO("edu.kit.data.oidc-agent:/redirect"); + assertThat(exception.getMessage(), containsString(expectedMessage)); + } - service.registerClient(request, userAuth); + @Test + void testValidPostLogoutRedirectUris() { + String validRedirectUri = VALID_REDIRECT_URIS.get(0); + + VALID_POST_LOGOUT_REDIRECT_URIS.forEach(postLogoutUri -> { + Assertions.assertDoesNotThrow(() -> { + RegisteredClientDTO request = createClientDTO(validRedirectUri, postLogoutUri); + service.registerClient(request, userAuth); + }); }); } + @Test + void testRejectsHttpPostLogoutRedirectUriForPublicClient() { + + String validRedirectUri = VALID_REDIRECT_URIS.get(0); + String postLogoutUri = VALID_POST_LOGOUT_REDIRECT_URIS.get(0); + RegisteredClientDTO request = createClientDTO(validRedirectUri, postLogoutUri); + request.setTokenEndpointAuthMethod(TokenEndpointAuthenticationMethod.none); + + ConstraintViolationException exception = Assertions.assertThrows( + ConstraintViolationException.class, () -> service.registerClient(request, userAuth)); + + assertThat(exception.getMessage(), containsString( + "Plain http post logout redirect URIs are only allowed for confidential clients")); + } + @Test void testBlacklistedUriValidation() { lenient().when(blsService.isBlockedUri("https://deny.example/cb")).thenReturn(true); @@ -338,7 +381,6 @@ void testBlacklistedUriValidation() { request.setClientName("example"); request.setGrantTypes(Set.of(AuthorizationGrantType.CODE)); request.setRedirectUris(Set.of("https://deny.example/cb")); - service.registerClient(request, userAuth); });