refactor: Refactor endsession endpoint

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 {

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";

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