Uploaded image for project: 'Keycloak'
  1. Keycloak
  2. KEYCLOAK-6832

Destination Validation should ignore whether default port is explicitly specified

    XMLWordPrintable

Details

    • Keycloak Sprint 10
    • 1
    • Hide

      Same as in KEYCLOAK-4813

      Show
      Same as in KEYCLOAK-4813
    • NEW
    • NEW

    Description

      Call to loginRequest from SAP Hana SAML SP are rejected as having an invalid destination.

      We coded up a quick fix that works for us, applying the same fix to handleSamlResponse, loginRequest, and logoutRequest in SamlService.java (attached).

      Fix from https://issues.jboss.org/browse/KEYCLOAK-4813 was incomplete.

      There are two problems with the code as it currently is in 3.4.3

      1. In https://github.com/keycloak/keycloak/blob/master/services/src/main/java/org/keycloak/protocol/saml/SamlService.java you have this:
      • in handleSamlResponse:
        // validate destination
        if (statusResponse.getDestination() != null && !uriInfo.getAbsolutePath().toString().equals(statusResponse.getDestination())) {
        event.detail(Details.REASON, "invalid_destination");

      This the below is better, since it avoids partially duplicating the logic in isValidDestination:
      // validate destination
      try

      { uri = new URI(statusResponse.getDestination()); }

      catch (URISyntaxException e)

      { e.printStackTrace(); }

      if (! isValidDestination(uri))

      { event.detail(Details.REASON, "invalid_destination"); event.error(Errors.INVALID_SAML_LOGOUT_RESPONSE); return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REQUEST); }
      1. In https://github.com/keycloak/keycloak/blob/master/services/src/main/java/org/keycloak/protocol/saml/SamlService.javayou have this:
      • in isValidDestination:
        Integer portByScheme = knownPorts.get(expected.getScheme());
        if (expected.getPort() < 0 && portByScheme != null) { return Objects.equals(uriInfo.getRequestUriBuilder().port(portByScheme).build(), destination); }

      String protocolByPort = knownProtocols.get(expected.getPort());
      if (expected.getPort() >= 0 && Objects.equals(protocolByPort, expected.getScheme()))

      { return Objects.equals(uriInfo.getRequestUriBuilder().port(-1).build(), destination); }

      The above code adds the request parameters to the end and therefore does not match the destination in the SAML.

      The below code does work:
      Integer portByScheme = knownPorts.get(expected.getScheme());
      if (expected.getPort() < 0 && portByScheme != null)

      { return Objects.equals(uriInfo.getAbsolutePathBuilder().port(portByScheme).build(), destination); }

      String protocolByPort = knownProtocols.get(expected.getPort());
      if (expected.getPort() >= 0 && Objects.equals(protocolByPort, expected.getScheme()))

      { return Objects.equals(uriInfo.getAbsolutePathBuilder().port(-1).build(), destination); }

      Updated SamlService.Java attached.

      Attachments

        Activity

          People

            hmlnarik@redhat.com Hynek Mlnařík
            pkboucher801@gmail.com Peter Boucher (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: