Merge branch '3.4.3.Final'

Modifications to documentation and code to update the module to run with
keycloak 3.4.3.Final

README.md

@@ -1,6 +1,6 @@
 # WS-Federation for keycloak
 
-* Currently working on 3.4.1.Final (check tags for compatibility with previous keycloak versions)
+* Currently working on 3.4.3.Final (check tags for compatibility with previous keycloak versions)
 
 ## Install
 
@@ -14,7 +14,7 @@ install -d -v -m755 /opt/keycloak/modules/system/layers/wsfed -o keycloak -g key
 install -d -v -m755 /opt/keycloak/modules/system/layers/wsfed/com/quest/keycloak-wsfed/main/ -o keycloak -g keycloak
 
 #Install jar
-install -v -m0755 -o keycloak -g keycloak -D target/keycloak-wsfed-3.4.1.Final.jar /opt/keycloak/modules/system/layers/wsfed/com/quest/keycloak-wsfed/main/
+install -v -m0755 -o keycloak -g keycloak -D target/keycloak-wsfed-3.4.3.Final.jar /opt/keycloak/modules/system/layers/wsfed/com/quest/keycloak-wsfed/main/
 
 #Install module file
 install -v -m0755 -o keycloak -g keycloak -D module.xml /opt/keycloak/modules/system/layers/wsfed/com/quest/keycloak-wsfed/main/

module.xml

@@ -3,7 +3,7 @@
 <module xmlns="urn:jboss:module:1.1" name="com.quest.keycloak-wsfed">
 
  <resources>
- <resource-root path="keycloak-wsfed-3.4.1.Final.jar"/>
+ <resource-root path="keycloak-wsfed-3.4.3.Final.jar"/>
  </resources>
 
  <dependencies>

pom.xml

@@ -17,7 +17,7 @@
  <parent>
  <artifactId>keycloak-parent</artifactId>
  <groupId>org.keycloak</groupId>
- <version>3.4.1.Final</version>
+ <version>3.4.3.Final</version>
  </parent>
 
  <groupId>com.quest</groupId>

src/main/java/com/quest/keycloak/broker/wsfed/WSFedEndpoint.java

@@ -245,8 +245,8 @@ protected Response handleLoginResponse(String wsfedResponse, RequestedToken toke
  Map<String, String> map = getContextParameters(decodedContext);
  String redirectUri = URLDecoder.decode(map.get("redirectUri"), StandardCharsets.UTF_8.name());
  if (decodedContext.contains("&code=")) {
- //TODO not sure that we indeed have a AuthenticationSessionModel here. It could potentially be a AuthenticatedClientSessionModel
- ClientSessionCode.ParseResult<AuthenticationSessionModel> clientCode = ClientSessionCode.parseResult(map.get("code"), this.session, this.session.getContext().getRealm(), this.session.getContext().getClient(), event, AuthenticationSessionModel.class);
+ //TODO not sure that we indeed have a AuthenticationSessionModel here. It could potentially be a AuthenticatedClientSessionModel. ALSO tabID set to null, but likely broken
+ ClientSessionCode.ParseResult<AuthenticationSessionModel> clientCode = ClientSessionCode.parseResult(map.get("code"), null, this.session, this.session.getContext().getRealm(), this.session.getContext().getClient(), event, AuthenticationSessionModel.class);
  if (clientCode != null && clientCode.getCode().isValid(CommonClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
  String ACTIVE_CODE = "active_code"; // duplicating because ClientSessionCode.ACTIVE_CODE is private
  // restore ACTIVE_CODE note because it must have been removed by parse() if code==activeCode

src/main/java/com/quest/keycloak/broker/wsfed/WSFedIdentityProvider.java

@@ -63,7 +63,7 @@ public Response performLogin(AuthenticationRequest request) {
  String destinationUrl = getConfig().getSingleSignOnServiceUrl();
  String reply = request.getRedirectUri();
  String wsFedRealm = getConfig().getWsFedRealm();
- String context = request.getState().getEncodedState();
+ String context = request.getState().getEncoded();
  // not sure how valuable this null-check is in real life, but it breaks in the tests without it.
  if( request.getHttpRequest() != null && request.getHttpRequest().getUri() != null ) {
  MultivaluedMap<String, String> params = request.getHttpRequest().getUri().getQueryParameters();

src/main/java/com/quest/keycloak/protocol/wsfed/WSFedService.java

@@ -307,12 +307,8 @@ protected Response handleLoginRequest(WSFedProtocolParameters params, ClientMode
 
  //WS-FED doesn't carry connection state at this point, but a freshness of 0 indicates a demand to re-prompt
  //for authentication (indicating the request is not new), maybe. TODO check logic
- AuthorizationEndpointChecks checks = getOrCreateAuthenticationSession(client, params.getWsfed_freshness());
- if (checks.response != null) {
- return checks.response;
- }
- AuthenticationSessionModel authSession = checks.authSession;
-
+ //However, requestState isn't actually used any more :-/
+ AuthenticationSessionModel authSession = createAuthenticationSession(client, params.getWsfed_freshness());
 
  authSession.setProtocol(WSFedLoginProtocol.LOGIN_PROTOCOL);
  authSession.setRedirectUri(redirect);
@@ -406,21 +402,4 @@ protected Response handleLogoutResponse(WSFedProtocolParameters params, ClientMo
  protected AuthenticationManager.AuthResult authenticateIdentityCookie() {
  return authManager.authenticateIdentityCookie(session, realm, false);
  }
-
- /**
- * This method checks if the request is a new one. WS doesn't really carry this sort of state, so we return true
- * except if the freshness was 0.
- * TODO check if we want/need to keep some server-side data to better support this operation
- * @param authSession The authorisation model
- * @param clientFromRequest the client model
- * @param requestState the content of the wfresh parameter
- * @return true if we consider this to be a new request
- */
- @Override
- protected boolean isNewRequest(AuthenticationSessionModel authSession, ClientModel clientFromRequest, String requestState) {
- if ("0".equals(requestState)) {
- return false;
- }
- return true;
- }
 }

src/test/java/com/quest/keycloak/broker/wsfed/WSFedIdentityProviderTest.java

@@ -91,16 +91,15 @@ public void testPerformLogin() throws Exception {
  AuthenticationRequest request = mock(AuthenticationRequest.class);
  doReturn("https://redirectUri").when(request).getRedirectUri();
  when(request.getState()).thenReturn(mock(IdentityBrokerState.class));
- when(request.getState().getEncodedState()).thenReturn("context");
-
+ when(request.getState().getEncoded()).thenReturn("context");
  Response response = identityProvider.performLogin(request);
  Document doc = responseToDocument(response);
 
  assertFormAction(doc, HttpMethod.GET, config.getSingleSignOnServiceUrl());
  assertInputNode(doc, WSFedConstants.WSFED_ACTION, WSFedConstants.WSFED_SIGNIN_ACTION);
  assertInputNode(doc, WSFedConstants.WSFED_REALM, config.getWsFedRealm());
  assertInputNode(doc, WSFedConstants.WSFED_REPLY, request.getRedirectUri());
- assertInputNode(doc, WSFedConstants.WSFED_CONTEXT, request.getState().getEncodedState());
+ assertInputNode(doc, WSFedConstants.WSFED_CONTEXT, request.getState().getEncoded());
  assertInputNodeMissing(doc, WSFedConstants.WSFED_RESULT);
  }
 

src/test/java/com/quest/keycloak/protocol/wsfed/WSFedServiceTest.java

@@ -558,7 +558,9 @@ public void testHandleLoginRequest() throws Exception {
  Response response = service.handleLoginRequest(params, mockHelper.getClient(), false);
  assertNotNull(response);
 
- assertErrorPage(mockHelper.getLoginFormsProvider(), Messages.EXPIRED_CODE);
+ verify(mockHelper.getLoginFormsProvider(), times(2)).setAuthenticationSession(any());
+ verify(mockHelper.getLoginFormsProvider(), times(1)).setError(eq(Messages.EXPIRED_CODE));
+ verify(mockHelper.getLoginFormsProvider(), times(1)).createErrorPage(Response.Status.BAD_REQUEST);
 
  verify(mockHelper.getAuthSessionModel(), times(1)).setProtocol(eq(WSFedLoginProtocol.LOGIN_PROTOCOL));
  verify(mockHelper.getAuthSessionModel(), times(1)).setRedirectUri(eq(params.getWsfed_reply()));