diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunOidcLogoutSuccessHandler.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunOidcLogoutSuccessHandler.java index 5e07d0d89afefda49cdd385fe6fcf40908666b78..509034558cfb1c087a6356ced704dcf0021aadf4 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunOidcLogoutSuccessHandler.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunOidcLogoutSuccessHandler.java @@ -12,8 +12,8 @@ import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; -import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_POST_LOGOUT_REDIRECT_URI; import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_STATE; +import static cz.muni.ics.openid.connect.web.endpoint.EndSessionEndpoint.PARAM_POST_LOGOUT_REDIRECT_URI; @Slf4j public class PerunOidcLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFilterConstants.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFilterConstants.java index 2baf755d74f6551ec2233284a155d2c75775fbbc..9ab4621c89bc4fe083724709d9ca88471d5f10a2 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFilterConstants.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFilterConstants.java @@ -23,7 +23,6 @@ public interface AuthProcFilterConstants { String PARAM_ACCEPTED = "accepted"; String PARAM_ACR_VALUES = "acr_values"; String PARAM_MAX_AGE = "max_age"; - String PARAM_POST_LOGOUT_REDIRECT_URI = "post_logout_redirect_uri"; String PARAM_STATE = "state"; String CLIENT_ID_PREFIX = "urn:cesnet:proxyidp:client_id:"; String AARC_IDP_HINT = "aarc_idp_hint"; diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/EndSessionEndpoint.java b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/EndSessionEndpoint.java index 399306123dbb00184c2689de07605362d1cff411..b45b7c6c09a46a288fdd65d2ddc4f0f16f81310d 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/EndSessionEndpoint.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/EndSessionEndpoint.java @@ -31,11 +31,12 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.common.exceptions.InvalidClientException; +import org.springframework.security.oauth2.common.exceptions.InvalidRequestException; import org.springframework.security.saml.SAMLLogoutFilter; import org.springframework.stereotype.Controller; import org.springframework.util.StringUtils; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; @@ -45,8 +46,6 @@ import javax.servlet.http.HttpSession; import java.text.ParseException; import java.util.Map; -import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_POST_LOGOUT_REDIRECT_URI; -import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_STATE; import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_TARGET; /** @@ -69,9 +68,23 @@ public class EndSessionEndpoint { public static final String URL = "endsession"; - private static final String CLIENT_KEY = "client"; - private static final String STATE_KEY = "state"; - private static final String REDIRECT_URI_KEY = "redirectUri"; + public static final String PARAM_POST_LOGOUT_REDIRECT_URI = "post_logout_redirect_uri"; + + public static final String PARAM_STATE = "state"; + + public static final String PARAM_CLIENT_ID = "client_id"; + + public static final String PARAM_ID_TOKEN_HINT = "id_token_hint"; + + private static final String SESSION_KEY_CLIENT = "client"; + + private static final String SESSION_KEY_STATE = "state"; + + private static final String SESSION_KEY_REDIRECT_URI = "redirect_uri"; + + private static final String MODEL_CLIENT_KEY = "client"; + + private static final String PREFIX_REDIRECT = "redirect:"; private final SelfAssertionValidator validator; private final PerunOidcConfig perunOidcConfig; @@ -90,73 +103,67 @@ public class EndSessionEndpoint { this.htmlClasses = htmlClasses; } - @RequestMapping(value = "/" + URL, method = RequestMethod.GET) - public String endSession(@RequestParam(value = "id_token_hint", required = false) String idTokenHint, + @GetMapping(value = "/" + URL) + public String endSession(@RequestParam(value = PARAM_ID_TOKEN_HINT, required = false) String idTokenHint, @RequestParam(value = PARAM_POST_LOGOUT_REDIRECT_URI, required = false) String postLogoutRedirectUri, - @RequestParam(value = STATE_KEY, required = false) String state, + @RequestParam(value = PARAM_STATE, required = false) String state, + @RequestParam(value = PARAM_CLIENT_ID, required = false) String clientId, HttpServletRequest request, HttpSession session, - Authentication auth, Map<String, Object> model) + Authentication auth, + Map<String, Object> model) { - JWTClaimsSet idTokenClaims = null; // pulled from the parsed and validated ID token ClientDetailsEntity client = null; // pulled from ID token's audience field if (!Strings.isNullOrEmpty(postLogoutRedirectUri)) { - session.setAttribute(REDIRECT_URI_KEY, postLogoutRedirectUri); + session.setAttribute(SESSION_KEY_REDIRECT_URI, postLogoutRedirectUri); } if (!Strings.isNullOrEmpty(state)) { - session.setAttribute(STATE_KEY, state); + session.setAttribute(SESSION_KEY_STATE, state); } // parse the ID token hint to see if it's valid if (!Strings.isNullOrEmpty(idTokenHint)) { - try { - JWT idToken = JWTParser.parse(idTokenHint); - - if (validator.isValid(idToken)) { - // we issued this ID token, figure out who it's for - idTokenClaims = idToken.getJWTClaimsSet(); - - String clientId = Iterables.getOnlyElement(idTokenClaims.getAudience()); - - client = clientService.loadClientByClientId(clientId); + clientId = resolveClientIdFromTokenAndParameter(idTokenHint, clientId); + } - // save a reference in the session for us to pick up later - //session.setAttribute("endSession_idTokenHint_claims", idTokenClaims); - session.setAttribute(CLIENT_KEY, client); - } - } catch (ParseException e) { - // it's not a valid ID token, ignore it - log.debug("Invalid id token hint", e); - } catch (InvalidClientException e) { + if (StringUtils.hasText(clientId)) { + try { + client = clientService.loadClientByClientId(clientId); + session.setAttribute(SESSION_KEY_CLIENT, client); + } catch (InvalidClientException e) { // couldn't find the client, ignore it - log.debug("Invalid client", e); + throw new InvalidRequestException( + "Client requesting the logout cannot be found. Is someone doing something nasty?" + ); } } // are we logged in or not? if (auth == null || !request.isUserInRole("ROLE_USER")) { - // we're not logged in anyway, process the final redirect bits if needed - return processLogout(null, null, session); + // We're not logged in, anyway. Process the final redirect bits if needed. + return processLogout(null, null, request, session, model); } else { - log.info("Logout confirmating for user {} from client {}", auth.getName(), client != null ? client.getClientName() : "unknown"); + log.info("Display logout confirm prompt for user {} from client {}", + auth.getName(), client != null ? client.getClientName() : "unknown" + ); // we are logged in, need to prompt the user before we log out - model.put("client", client); - model.put("idToken", idTokenClaims); - + model.put(MODEL_CLIENT_KEY, client); ControllerUtils.setPageOptions(model, request, htmlClasses, perunOidcConfig); return "logout"; } } - @RequestMapping(value = "/" + URL, method = RequestMethod.POST) + @PostMapping(value = "/" + URL) public String processLogout(@RequestParam(value = "approve", required = false) String approved, @RequestParam(value = "deny", required = false) String deny, - HttpSession session) + HttpServletRequest request, + HttpSession session, + Map<String, Object> model) { - String redirectUri = (String) session.getAttribute(REDIRECT_URI_KEY); - String state = (String) session.getAttribute(STATE_KEY); - ClientDetailsEntity client = (ClientDetailsEntity) session.getAttribute(CLIENT_KEY); + String redirectUri = (String) session.getAttribute(SESSION_KEY_REDIRECT_URI); + String state = (String) session.getAttribute(SESSION_KEY_STATE); + ClientDetailsEntity client = (ClientDetailsEntity) session.getAttribute(SESSION_KEY_CLIENT); String redirectURL = null; // if we have a client AND the client has post-logout redirect URIs @@ -164,10 +171,9 @@ public class EndSessionEndpoint { if (isUriValid(redirectUri, client)) { UriComponentsBuilder uri = UriComponentsBuilder.fromHttpUrl(redirectUri); if (StringUtils.hasText(state)) { - uri = uri.queryParam("state", state); + uri = uri.queryParam(PARAM_STATE, state); } UriComponents uriComponents = uri.build(); - log.trace("redirect URL: {}", uriComponents); redirectURL = uriComponents.toString(); } @@ -175,22 +181,52 @@ public class EndSessionEndpoint { String target = getRedirectUrl(redirectUri, state); if (StringUtils.hasText(approved)) { target = getLogoutUrl(target); - log.trace("redirecting to logout SAML and then {}", target); - return "redirect:" + target; + log.trace("Endsession - redirecting to SAML logout, then to {}", target); + return PREFIX_REDIRECT + target; } else { - log.trace("redirecting to {}", target); - return "redirect:" + redirectURL; + log.trace("Endsession - redirecting to {}", target); + return PREFIX_REDIRECT + redirectURL; } } else { if (StringUtils.hasText(approved)) { - log.trace("redirecting to logout SAML only"); - return "redirect:" + getLogoutUrl(null); + log.trace("Endsession - redirecting to SAML logout only"); + return PREFIX_REDIRECT + getLogoutUrl(null); } else { + ControllerUtils.setPageOptions(model, request, htmlClasses, perunOidcConfig); + log.trace("Endsession - user denied the logout and we have no redirect, display logout denied page"); return "logout_denied"; } } } + private String resolveClientIdFromTokenAndParameter(String idTokenHint, String clientId) { + JWTClaimsSet idTokenClaims = null; + try { + JWT idToken = JWTParser.parse(idTokenHint); + if (validator.isValid(idToken)) { + idTokenClaims = idToken.getJWTClaimsSet(); + } + } catch (ParseException e) { + // it's not a valid ID token, ignore it + log.debug("Invalid id token hint", e); + } + if (idTokenClaims != null) { + String clientIdFromToken = Iterables.getOnlyElement(idTokenClaims.getAudience()); + + if (StringUtils.hasText(clientId)) { + if (StringUtils.hasText(clientIdFromToken) && !clientIdFromToken.equals(clientId)) { + throw new InvalidRequestException( + "Client ID and client for which the ID token has been issued do not match. Is someone doing something nasty?" + ); + } + } else { + clientId = clientIdFromToken; + } + } + + return clientId; + } + private boolean isUriValid(String redirectUri, ClientDetailsEntity client) { return StringUtils.hasText(redirectUri) && client != null