diff --git a/core/src/main/java/org/keycloak/jose/jws/crypto/HMACProvider.java b/core/src/main/java/org/keycloak/jose/jws/crypto/HMACProvider.java index fc63fd62a1..5157577c58 100755 --- a/core/src/main/java/org/keycloak/jose/jws/crypto/HMACProvider.java +++ b/core/src/main/java/org/keycloak/jose/jws/crypto/HMACProvider.java @@ -63,7 +63,7 @@ public class HMACProvider implements SignatureProvider { public static boolean verify(JWSInput input, SecretKey key) { try { - byte[] signature = sign(input.getContent(), input.getHeader().getAlgorithm(), key); + byte[] signature = sign(input.getEncodedSignatureInput().getBytes("UTF-8"), input.getHeader().getAlgorithm(), key); String x = Base64Url.encode(signature); return x.equals(input.getEncodedSignature()); } catch (Exception e) { diff --git a/core/src/test/java/org/keycloak/RSAVerifierTest.java b/core/src/test/java/org/keycloak/RSAVerifierTest.java index eaa689cddd..e1eb84693b 100755 --- a/core/src/test/java/org/keycloak/RSAVerifierTest.java +++ b/core/src/test/java/org/keycloak/RSAVerifierTest.java @@ -96,6 +96,7 @@ public class RSAVerifierTest { String encoded = new JWSBuilder() .jsonContent(token) .rsa256(idpPair.getPrivate()); + System.out.print("encoded size: " + encoded.length()); AccessToken token = verifySkeletonKeyToken(encoded); Assert.assertTrue(token.getResourceAccess("service").getRoles().contains("admin")); Assert.assertEquals("CN=Client", token.getSubject()); diff --git a/core/src/test/java/org/keycloak/jose/HmacTest.java b/core/src/test/java/org/keycloak/jose/HmacTest.java new file mode 100755 index 0000000000..8436e1440f --- /dev/null +++ b/core/src/test/java/org/keycloak/jose/HmacTest.java @@ -0,0 +1,27 @@ +package org.keycloak.jose; + +import org.junit.Assert; +import org.junit.Test; +import org.keycloak.jose.jws.JWSBuilder; +import org.keycloak.jose.jws.JWSInput; +import org.keycloak.jose.jws.crypto.HMACProvider; + +import javax.crypto.SecretKey; +import javax.crypto.spec.SecretKeySpec; +import java.util.UUID; + +/** + * @author Bill Burke + * @version $Revision: 1 $ + */ +public class HmacTest { + + @Test + public void testHmacSignatures() throws Exception { + SecretKey secret = new SecretKeySpec(UUID.randomUUID().toString().getBytes(), "HmacSHA256"); + String encoded = new JWSBuilder().content("hello world".getBytes()) + .hmac256(secret); + JWSInput input = new JWSInput(encoded); + Assert.assertTrue(HMACProvider.verify(input, secret)); + } +} diff --git a/events/api/src/main/java/org/keycloak/events/Details.java b/events/api/src/main/java/org/keycloak/events/Details.java index 23cc2f7f4b..679b67010a 100755 --- a/events/api/src/main/java/org/keycloak/events/Details.java +++ b/events/api/src/main/java/org/keycloak/events/Details.java @@ -27,6 +27,7 @@ public interface Details { String REVOKED_CLIENT = "revoked_client"; String CLIENT_SESSION_STATE = "client_session_state"; String CLIENT_SESSION_HOST = "client_session_host"; + String RESTART_AFTER_TIMEOUT = "restart_after_timeout"; String CONSENT = "consent"; String CONSENT_VALUE_NO_CONSENT_REQUIRED = "no_consent_required"; // No consent is required by client diff --git a/events/api/src/main/java/org/keycloak/events/Errors.java b/events/api/src/main/java/org/keycloak/events/Errors.java index 41b501d2ce..dce5e7583a 100755 --- a/events/api/src/main/java/org/keycloak/events/Errors.java +++ b/events/api/src/main/java/org/keycloak/events/Errors.java @@ -13,6 +13,8 @@ public interface Errors { String CLIENT_DISABLED = "client_disabled"; String INVALID_CLIENT_CREDENTIALS = "invalid_client_credentials"; String INVALID_CLIENT = "invalid_client"; + String CONSENT_DENIED = "consent_denied"; + String RESOLVE_REQUIRED_ACTIONS = "resolve_required_actions"; String USER_NOT_FOUND = "user_not_found"; String USER_DISABLED = "user_disabled"; diff --git a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java index e6a2175602..13233b3b85 100755 --- a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java +++ b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java @@ -22,6 +22,7 @@ import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import org.keycloak.protocol.LoginProtocol; import org.keycloak.protocol.ProtocolMapper; +import org.keycloak.protocol.RestartLoginCookie; import org.keycloak.protocol.saml.mappers.SAMLAttributeStatementMapper; import org.keycloak.protocol.saml.mappers.SAMLLoginResponseMapper; import org.keycloak.protocol.saml.mappers.SAMLRoleListMapper; @@ -141,6 +142,7 @@ public class SamlProtocol implements LoginProtocol { @Override public Response cancelLogin(ClientSessionModel clientSession) { + RestartLoginCookie.expireRestartCookie(realm, session.getContext().getConnection(), uriInfo); if ("true".equals(clientSession.getClient().getAttribute(SAML_IDP_INITIATED_LOGIN))) { UriBuilder builder = RealmsResource.protocolUrl(uriInfo).path(SamlService.class, "idpInitiatedSSO"); Map params = new HashMap<>(); @@ -443,6 +445,7 @@ public class SamlProtocol implements LoginProtocol { @Override public Response consentDenied(ClientSessionModel clientSession) { + RestartLoginCookie.expireRestartCookie(realm, session.getContext().getConnection(), uriInfo); if ("true".equals(clientSession.getClient().getAttribute(SAML_IDP_INITIATED_LOGIN))) { session.sessions().removeClientSession(realm, clientSession); return ErrorPage.error(session, Messages.CONSENT_DENIED); diff --git a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlService.java b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlService.java index eae7adeeaf..fe4b13e2a2 100755 --- a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlService.java +++ b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlService.java @@ -26,6 +26,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserSessionModel; import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.protocol.RestartLoginCookie; import org.keycloak.protocol.oidc.utils.RedirectUtils; import org.keycloak.saml.common.constants.GeneralConstants; import org.keycloak.saml.common.constants.JBossSAMLURIConstants; @@ -513,6 +514,7 @@ public class SamlService { .setRequest(request); try { + RestartLoginCookie.setRestartCookie(realm, clientConnection, uriInfo, clientSession); return processor.authenticate(); } catch (Exception e) { return processor.handleBrowserException(e); diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index fbe37b482f..f3daf788bb 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -498,6 +498,7 @@ public class AuthenticationProcessor { } public static void resetFlow(ClientSessionModel clientSession) { + clientSession.setTimestamp(Time.currentTime()); clientSession.setAuthenticatedUser(null); clientSession.clearExecutionStatus(); clientSession.clearUserSessionNotes(); @@ -574,7 +575,8 @@ public class AuthenticationProcessor { String attemptedUsername = clientSession.getNote(AbstractFormAuthenticator.ATTEMPTED_USERNAME); if (attemptedUsername != null) username = attemptedUsername; if (userSession == null) { // if no authenticator attached a usersession - userSession = session.sessions().createUserSession(realm, clientSession.getAuthenticatedUser(), username, connection.getRemoteAddr(), "form", false, null, null); + boolean remember = "true".equals(clientSession.getNote(Details.REMEMBER_ME)); + userSession = session.sessions().createUserSession(realm, clientSession.getAuthenticatedUser(), username, connection.getRemoteAddr(), "form", remember, null, null); userSession.setState(UserSessionModel.State.LOGGING_IN); userSessionCreated = true; } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java index ec7163f00e..d129eafb3d 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/UsernamePasswordForm.java @@ -10,6 +10,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.protocol.LoginProtocol; +import org.keycloak.protocol.RestartLoginCookie; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.services.managers.AuthenticationManager; diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java index 1b30df1d06..842b6e4a8f 100755 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java @@ -20,10 +20,6 @@ public class UpdateProfile implements RequiredActionProvider, RequiredActionFact protected static Logger logger = Logger.getLogger(UpdateProfile.class); @Override public void evaluateTriggers(RequiredActionContext context) { - if (context.getRealm().isVerifyEmail() && !context.getUser().isEmailVerified()) { - context.getUser().addRequiredAction(UserModel.RequiredAction.VERIFY_EMAIL); - logger.debug("User is required to verify email"); - } } @Override diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java b/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java index ca0a1539e6..2d1726a752 100755 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java @@ -28,29 +28,11 @@ public class VerifyEmail implements RequiredActionProvider, RequiredActionFactor protected static Logger logger = Logger.getLogger(VerifyEmail.class); @Override public void evaluateTriggers(RequiredActionContext context) { - int daysToExpirePassword = context.getRealm().getPasswordPolicy().getDaysToExpirePassword(); - if(daysToExpirePassword != -1) { - for (UserCredentialValueModel entity : context.getUser().getCredentialsDirectly()) { - if (entity.getType().equals(UserCredentialModel.PASSWORD)) { - - if(entity.getCreatedDate() == null) { - context.getUser().addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD); - logger.debug("User is required to update password"); - } else { - long timeElapsed = Time.toMillis(Time.currentTime()) - entity.getCreatedDate(); - long timeToExpire = TimeUnit.DAYS.toMillis(daysToExpirePassword); - - if(timeElapsed > timeToExpire) { - context.getUser().addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD); - logger.debug("User is required to update password"); - } - } - break; - } - } + if (context.getRealm().isVerifyEmail() && !context.getUser().isEmailVerified()) { + context.getUser().addRequiredAction(UserModel.RequiredAction.VERIFY_EMAIL); + logger.debug("User is required to verify email"); } } - @Override public Response invokeRequiredAction(RequiredActionContext context) { if (Validation.isBlank(context.getUser().getEmail())) { diff --git a/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java b/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java new file mode 100755 index 0000000000..d6336d3093 --- /dev/null +++ b/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java @@ -0,0 +1,175 @@ +package org.keycloak.protocol; + +import org.codehaus.jackson.annotate.JsonProperty; +import org.jboss.logging.Logger; +import org.keycloak.ClientConnection; +import org.keycloak.jose.jws.JWSBuilder; +import org.keycloak.jose.jws.JWSInput; +import org.keycloak.jose.jws.crypto.HMACProvider; +import org.keycloak.jose.jws.crypto.RSAProvider; +import org.keycloak.models.ClientModel; +import org.keycloak.models.ClientSessionModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.services.managers.AuthenticationManager; +import org.keycloak.services.util.CookieHelper; + +import javax.crypto.SecretKey; +import javax.ws.rs.core.Cookie; +import javax.ws.rs.core.UriInfo; +import java.util.HashMap; +import java.util.Map; + +/** + * This is an an encoded token that is stored as a cookie so that if there is a client timeout, then the client session + * can be restarted. + * + * @author Bill Burke + * @version $Revision: 1 $ + */ +public class RestartLoginCookie { + private static final Logger logger = Logger.getLogger(RestartLoginCookie.class); + public static final String KC_RESTART = "KC_RESTART"; + @JsonProperty("cs") + protected String clientSession; + + @JsonProperty("cid") + protected String clientId; + + @JsonProperty("pty") + protected String authMethod; + + @JsonProperty("ruri") + protected String redirectUri; + + @JsonProperty("act") + protected String action; + + @JsonProperty("notes") + protected Map notes = new HashMap<>(); + + public String getClientSession() { + return clientSession; + } + + public void setClientSession(String clientSession) { + this.clientSession = clientSession; + } + + public Map getNotes() { + return notes; + } + + public void setNotes(Map notes) { + this.notes = notes; + } + + public String getClientId() { + return clientId; + } + + public void setClientId(String clientId) { + this.clientId = clientId; + } + + public String getAuthMethod() { + return authMethod; + } + + public void setAuthMethod(String authMethod) { + this.authMethod = authMethod; + } + + public String getRedirectUri() { + return redirectUri; + } + + public void setRedirectUri(String redirectUri) { + this.redirectUri = redirectUri; + } + + public String getAction() { + return action; + } + + public void setAction(String action) { + this.action = action; + } + + public String encode(RealmModel realm) { + JWSBuilder builder = new JWSBuilder(); + return builder.jsonContent(this) + .hmac256((SecretKey)realm.getCodeSecretKey()); + //.rsa256(realm.getPrivateKey()); + + } + + public RestartLoginCookie() { + } + public RestartLoginCookie(ClientSessionModel clientSession) { + this.action = clientSession.getAction(); + this.clientId = clientSession.getClient().getClientId(); + this.authMethod = clientSession.getAuthMethod(); + this.redirectUri = clientSession.getRedirectUri(); + this.clientSession = clientSession.getId(); + for (Map.Entry entry : clientSession.getNotes().entrySet()) { + notes.put(entry.getKey(), entry.getValue()); + } + } + + public static void setRestartCookie(RealmModel realm, ClientConnection connection, UriInfo uriInfo, ClientSessionModel clientSession) { + RestartLoginCookie restart = new RestartLoginCookie(clientSession); + String encoded = restart.encode(realm); + int keySize = realm.getCodeSecret().length(); + int size = encoded.length(); + String path = AuthenticationManager.getRealmCookiePath(realm, uriInfo); + boolean secureOnly = realm.getSslRequired().isRequired(connection); + CookieHelper.addCookie(KC_RESTART, encoded, path, null, null, -1, secureOnly, true); + } + + public static void expireRestartCookie(RealmModel realm, ClientConnection connection, UriInfo uriInfo) { + String path = AuthenticationManager.getRealmCookiePath(realm, uriInfo); + boolean secureOnly = realm.getSslRequired().isRequired(connection); + CookieHelper.addCookie(KC_RESTART, "", path, null, null, 0, secureOnly, true); + } + + public static ClientSessionModel restartSession(KeycloakSession session, RealmModel realm, String code) throws Exception { + Cookie cook = session.getContext().getRequestHeaders().getCookies().get(KC_RESTART); + if (cook == null) { + logger.debug("KC_RESTART cookie doesn't exist"); + return null; + } + String encodedCookie = cook.getValue(); + JWSInput input = new JWSInput(encodedCookie); + /* + if (!RSAProvider.verify(input, realm.getPublicKey())) { + logger.debug("Failed to verify encoded RestartLoginCookie"); + return null; + } + */ + if (!HMACProvider.verify(input, (SecretKey)realm.getCodeSecretKey())) { + logger.debug("Failed to verify encoded RestartLoginCookie"); + return null; + } + RestartLoginCookie cookie = input.readJsonContent(RestartLoginCookie.class); + String[] parts = code.split("\\."); + String clientSessionId = parts[1]; + if (!clientSessionId.equals(cookie.getClientSession())) { + logger.debug("RestartLoginCookie clientSession does not match code's clientSession"); + return null; + } + + ClientModel client = realm.getClientByClientId(cookie.getClientId()); + if (client == null) return null; + + ClientSessionModel clientSession = session.sessions().createClientSession(realm, client); + clientSession.setAuthMethod(cookie.getAuthMethod()); + clientSession.setRedirectUri(cookie.getRedirectUri()); + clientSession.setAction(cookie.getAction()); + for (Map.Entry entry : cookie.getNotes().entrySet()) { + clientSession.setNote(entry.getKey(), entry.getValue()); + } + + return clientSession; + } +} diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java index bfdffadda3..332a547c3b 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java @@ -32,6 +32,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserSessionModel; import org.keycloak.protocol.LoginProtocol; +import org.keycloak.protocol.RestartLoginCookie; import org.keycloak.services.managers.ClientSessionCode; import org.keycloak.services.managers.ResourceAdminManager; @@ -124,6 +125,7 @@ public class OIDCLoginProtocol implements LoginProtocol { redirectUri.queryParam(OAuth2Constants.STATE, state); } session.sessions().removeClientSession(realm, clientSession); + RestartLoginCookie.expireRestartCookie(realm, session.getContext().getConnection(), uriInfo); return Response.status(302).location(redirectUri.build()).build(); } @@ -149,6 +151,7 @@ public class OIDCLoginProtocol implements LoginProtocol { if (state != null) redirectUri.queryParam(OAuth2Constants.STATE, state); session.sessions().removeClientSession(realm, clientSession); + RestartLoginCookie.expireRestartCookie(realm, session.getContext().getConnection(), uriInfo); Response.ResponseBuilder location = Response.status(302).location(redirectUri.build()); return location.build(); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java index bb4e0d38d4..64a26d4a69 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java @@ -19,6 +19,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.protocol.RestartLoginCookie; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.utils.RedirectUtils; import org.keycloak.services.ErrorPageException; @@ -261,6 +262,7 @@ public class AuthorizationEndpoint { } clientSession.setNote(Details.AUTH_TYPE, CODE_AUTH_TYPE); + AuthenticationFlowModel flow = realm.getFlowByAlias(DefaultAuthenticationFlows.BROWSER_FLOW); String flowId = flow.getId(); AuthenticationProcessor processor = new AuthenticationProcessor(); @@ -295,6 +297,7 @@ public class AuthorizationEndpoint { if (challenge == null) { return processor.finishAuthentication(); } else { + RestartLoginCookie.setRestartCookie(realm, clientConnection, uriInfo, clientSession); return challenge; } } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java index 4281c7dc8e..b6dfcd793b 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java @@ -321,6 +321,7 @@ public class TokenEndpoint { event.detail(Details.AUTH_METHOD, "oauth_credentials").detail(Details.RESPONSE_TYPE, "token"); if (client.isConsentRequired()) { + event.error(Errors.CONSENT_DENIED); throw new ErrorResponseException("invalid_client", "Client requires user consent", Response.Status.BAD_REQUEST); } String scope = formParams.getFirst(OAuth2Constants.SCOPE); @@ -347,6 +348,7 @@ public class TokenEndpoint { processor.evaluateRequiredActionTriggers(); UserModel user = clientSession.getAuthenticatedUser(); if (user.getRequiredActions() != null && user.getRequiredActions().size() > 0) { + event.error(Errors.RESOLVE_REQUIRED_ACTIONS); throw new ErrorResponseException("invalid_grant", "Account is not fully set up", Response.Status.BAD_REQUEST); } diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index ea95c14310..3ea7d004c3 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -31,6 +31,7 @@ import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.models.UserSessionModel; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.protocol.LoginProtocol; +import org.keycloak.protocol.RestartLoginCookie; import org.keycloak.protocol.oidc.TokenManager; import org.keycloak.provider.ProviderFactory; import org.keycloak.representations.AccessToken; @@ -407,6 +408,7 @@ public class AuthenticationManager { protocol.setRealm(realm) .setHttpHeaders(request.getHttpHeaders()) .setUriInfo(uriInfo); + RestartLoginCookie.expireRestartCookie(realm, clientConnection, uriInfo); return protocol.authenticated(userSession, new ClientSessionCode(realm, clientSession)); } diff --git a/services/src/main/java/org/keycloak/services/managers/ClientSessionCode.java b/services/src/main/java/org/keycloak/services/managers/ClientSessionCode.java index 459aab321c..6ebe4f32e2 100755 --- a/services/src/main/java/org/keycloak/services/managers/ClientSessionCode.java +++ b/services/src/main/java/org/keycloak/services/managers/ClientSessionCode.java @@ -54,6 +54,50 @@ public class ClientSessionCode { } } + public static class ParseResult { + ClientSessionCode code; + boolean clientSessionNotFound; + boolean illegalHash; + + public ClientSessionCode getCode() { + return code; + } + + public boolean isClientSessionNotFound() { + return clientSessionNotFound; + } + + public boolean isIllegalHash() { + return illegalHash; + } + } + + public static ParseResult parseResult(String code, KeycloakSession session, RealmModel realm) { + try { + ParseResult result = new ParseResult(); + String[] parts = code.split("\\."); + String id = parts[1]; + + ClientSessionModel clientSession = session.sessions().getClientSession(realm, id); + if (clientSession == null) { + result.clientSessionNotFound = true; + return result; + } + + String hash = createHash(realm, clientSession); + if (!hash.equals(parts[0])) { + result.illegalHash = true; + return result; + } + + result.code = new ClientSessionCode(realm, clientSession); + return result; + } catch (RuntimeException e) { + return null; + } + } + + public static ClientSessionCode parse(String code, KeycloakSession session, RealmModel realm) { try { diff --git a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java index 95f9be6590..21f481b897 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -37,12 +37,12 @@ import org.keycloak.login.LoginFormsProvider; import org.keycloak.models.AuthenticationFlowModel; import org.keycloak.models.ClientModel; import org.keycloak.models.ClientSessionModel; -import org.keycloak.models.RoleModel; -import org.keycloak.models.UserConsentModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelException; import org.keycloak.models.ProtocolMapperModel; import org.keycloak.models.RealmModel; +import org.keycloak.models.RoleModel; +import org.keycloak.models.UserConsentModel; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserModel.RequiredAction; @@ -51,13 +51,14 @@ import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.models.utils.FormMessage; import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.protocol.LoginProtocol; +import org.keycloak.protocol.RestartLoginCookie; import org.keycloak.protocol.oidc.TokenManager; import org.keycloak.representations.idm.CredentialRepresentation; +import org.keycloak.services.ErrorPage; +import org.keycloak.services.Urls; import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.ClientSessionCode; import org.keycloak.services.messages.Messages; -import org.keycloak.services.ErrorPage; -import org.keycloak.services.Urls; import org.keycloak.services.util.CookieHelper; import org.keycloak.services.validation.Validation; import org.keycloak.util.Time; @@ -150,8 +151,8 @@ public class LoginActionsService { ClientSessionCode clientCode; Response response; - boolean check(String code, String requiredAction) { - if (!check(code)) { + boolean verifyCode(String flow, String code, String requiredAction) { + if (!verifyCode(flow, code)) { return false; } else if (!clientCode.isValidAction(requiredAction)) { event.client(clientCode.getClientSession().getClient()); @@ -160,7 +161,12 @@ public class LoginActionsService { return false; } else if (!clientCode.isActionActive(requiredAction)) { event.client(clientCode.getClientSession().getClient()); - event.error(Errors.EXPIRED_CODE); + event.clone().error(Errors.EXPIRED_CODE); + if (clientCode.getClientSession().getAction().equals(ClientSessionModel.Action.AUTHENTICATE.name())) { + AuthenticationProcessor.resetFlow(clientCode.getClientSession()); + response = processAuthentication(null, clientCode.getClientSession()); + return false; + } response = ErrorPage.error(session, Messages.EXPIRED_CODE); return false; } else { @@ -168,8 +174,8 @@ public class LoginActionsService { } } - boolean check(String code, String requiredAction, String alternativeRequiredAction) { - if (!check(code)) { + boolean verifyCode(String flow, String code, String requiredAction, String alternativeRequiredAction) { + if (!verifyCode(flow, code)) { return false; } else if (!(clientCode.isValidAction(requiredAction) || clientCode.isValidAction(alternativeRequiredAction))) { event.client(clientCode.getClientSession().getClient()); @@ -178,7 +184,7 @@ public class LoginActionsService { return false; } else if (!(clientCode.isActionActive(requiredAction) || clientCode.isActionActive(alternativeRequiredAction))) { event.client(clientCode.getClientSession().getClient()); - event.error(Errors.EXPIRED_CODE); + event.clone().error(Errors.EXPIRED_CODE); if (clientCode.getClientSession().getAction().equals(ClientSessionModel.Action.AUTHENTICATE.name())) { AuthenticationProcessor.resetFlow(clientCode.getClientSession()); response = processAuthentication(null, clientCode.getClientSession()); @@ -194,7 +200,7 @@ public class LoginActionsService { } } - public boolean check(String code) { + public boolean verifyCode(String flow, String code) { if (!checkSsl()) { event.error(Errors.SSL_REQUIRED); response = ErrorPage.error(session, Messages.HTTPS_REQUIRED); @@ -205,8 +211,21 @@ public class LoginActionsService { response = ErrorPage.error(session, Messages.REALM_NOT_ENABLED); return false; } - clientCode = ClientSessionCode.parse(code, session, realm); + ClientSessionCode.ParseResult result = ClientSessionCode.parseResult(code, session, realm); + clientCode = result.getCode(); if (clientCode == null) { + if (result.isClientSessionNotFound()) { // timeout + try { + ClientSessionModel clientSession = RestartLoginCookie.restartSession(session, realm, code); + if (clientSession != null) { + event.clone().detail(Details.RESTART_AFTER_TIMEOUT, "true").error(Errors.EXPIRED_CODE); + response = processFlow(null, clientSession, flow); + return false; + } + } catch (Exception e) { + logger.error("failed to parse RestartLoginCookie", e); + } + } event.error(Errors.INVALID_CODE); response = ErrorPage.error(session, Messages.INVALID_CODE); return false; @@ -239,7 +258,6 @@ public class LoginActionsService { /** * protocol independent login page entry point * - * * @param code * @return */ @@ -249,7 +267,7 @@ public class LoginActionsService { @QueryParam("execution") String execution) { event.event(EventType.LOGIN); Checks checks = new Checks(); - if (!checks.check(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionModel.Action.RECOVER_PASSWORD.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionModel.Action.RECOVER_PASSWORD.name())) { return checks.response; } event.detail(Details.CODE_ID, code); @@ -305,7 +323,7 @@ public class LoginActionsService { @QueryParam("execution") String execution) { event.event(EventType.LOGIN); Checks checks = new Checks(); - if (!checks.check(code, ClientSessionModel.Action.AUTHENTICATE.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, code, ClientSessionModel.Action.AUTHENTICATE.name())) { return checks.response; } final ClientSessionCode clientCode = checks.clientCode; @@ -320,7 +338,6 @@ public class LoginActionsService { } - /** * protocol independent registration page entry point * @@ -338,7 +355,7 @@ public class LoginActionsService { } Checks checks = new Checks(); - if (!checks.check(code, ClientSessionModel.Action.AUTHENTICATE.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.REGISTRATION_FLOW, code, ClientSessionModel.Action.AUTHENTICATE.name())) { return checks.response; } event.detail(Details.CODE_ID, code); @@ -364,7 +381,7 @@ public class LoginActionsService { @QueryParam("execution") String execution) { event.event(EventType.REGISTER); Checks checks = new Checks(); - if (!checks.check(code, ClientSessionModel.Action.AUTHENTICATE.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.REGISTRATION_FLOW, code, ClientSessionModel.Action.AUTHENTICATE.name())) { return checks.response; } if (!realm.isRegistrationAllowed()) { @@ -465,7 +482,7 @@ public class LoginActionsService { final MultivaluedMap formData) { event.event(EventType.UPDATE_PROFILE); Checks checks = new Checks(); - if (!checks.check(code, ClientSessionModel.Action.UPDATE_PROFILE.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, code, ClientSessionModel.Action.UPDATE_PROFILE.name())) { return checks.response; } ClientSessionCode accessCode = checks.clientCode; @@ -509,7 +526,7 @@ public class LoginActionsService { } AttributeFormDataProcessor.process(formData, realm, user); - + user.removeRequiredAction(RequiredAction.UPDATE_PROFILE); event.clone().event(EventType.UPDATE_PROFILE).success(); @@ -527,7 +544,7 @@ public class LoginActionsService { final MultivaluedMap formData) { event.event(EventType.UPDATE_TOTP); Checks checks = new Checks(); - if (!checks.check(code, ClientSessionModel.Action.CONFIGURE_TOTP.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, code, ClientSessionModel.Action.CONFIGURE_TOTP.name())) { return checks.response; } ClientSessionCode accessCode = checks.clientCode; @@ -572,7 +589,7 @@ public class LoginActionsService { final MultivaluedMap formData) { event.event(EventType.UPDATE_PASSWORD); Checks checks = new Checks(); - if (!checks.check(code, ClientSessionModel.Action.UPDATE_PASSWORD.name(), ClientSessionModel.Action.RECOVER_PASSWORD.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, code, ClientSessionModel.Action.UPDATE_PASSWORD.name(), ClientSessionModel.Action.RECOVER_PASSWORD.name())) { return checks.response; } ClientSessionCode accessCode = checks.clientCode; @@ -635,7 +652,7 @@ public class LoginActionsService { event.event(EventType.VERIFY_EMAIL); if (key != null) { Checks checks = new Checks(); - if (!checks.check(key, ClientSessionModel.Action.VERIFY_EMAIL.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, key, ClientSessionModel.Action.VERIFY_EMAIL.name())) { return checks.response; } ClientSessionCode accessCode = checks.clientCode; @@ -662,7 +679,7 @@ public class LoginActionsService { return AuthenticationManager.nextActionAfterAuthentication(session, userSession, clientSession, clientConnection, request, uriInfo, event); } else { Checks checks = new Checks(); - if (!checks.check(code, ClientSessionModel.Action.VERIFY_EMAIL.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, code, ClientSessionModel.Action.VERIFY_EMAIL.name())) { return checks.response; } ClientSessionCode accessCode = checks.clientCode; @@ -685,7 +702,7 @@ public class LoginActionsService { event.event(EventType.RESET_PASSWORD); if (key != null) { Checks checks = new Checks(); - if (!checks.check(key, ClientSessionModel.Action.RECOVER_PASSWORD.name())) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, key, ClientSessionModel.Action.RECOVER_PASSWORD.name())) { return checks.response; } ClientSessionCode accessCode = checks.clientCode; @@ -706,7 +723,7 @@ public class LoginActionsService { final MultivaluedMap formData) { event.event(EventType.SEND_RESET_PASSWORD); Checks checks = new Checks(); - if (!checks.check(code)) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, code)) { return checks.response; } final ClientSessionCode accessCode = checks.clientCode; @@ -715,7 +732,7 @@ public class LoginActionsService { String username = formData.getFirst("username"); - if(username == null || username.isEmpty()) { + if (username == null || username.isEmpty()) { event.error(Errors.USERNAME_MISSING); return session.getProvider(LoginFormsProvider.class) .setError(Messages.MISSING_USERNAME) @@ -736,12 +753,11 @@ public class LoginActionsService { if (user == null) { event.error(Errors.USER_NOT_FOUND); - } else if(!user.isEnabled()) { + } else if (!user.isEnabled()) { event.user(user).error(Errors.USER_DISABLED); - } - else if(user.getEmail() == null || user.getEmail().trim().length() == 0) { + } else if (user.getEmail() == null || user.getEmail().trim().length() == 0) { event.user(user).error(Errors.INVALID_EMAIL); - } else{ + } else { event.user(user); UserSessionModel userSession = session.sessions().createUserSession(realm, user, username, clientConnection.getRemoteAddr(), "form", false, null, null); @@ -825,7 +841,7 @@ public class LoginActionsService { throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE)); } Checks checks = new Checks(); - if (!checks.check(code, action)) { + if (!checks.verifyCode(DefaultAuthenticationFlows.BROWSER_FLOW, code, action)) { return checks.response; } final ClientSessionCode clientCode = checks.clientCode; @@ -895,11 +911,10 @@ public class LoginActionsService { code.setAction(action); return code.getCode(); } - }; + }; return provider.jaxrsService(context); - } } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java index a5eaa648cc..19cbbf06a7 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java @@ -241,6 +241,47 @@ public class BruteForceTest { events.clear(); } + } @Test + public void testGrantMissingOtp() throws Exception { + { + String totpSecret = totp.generate("totpSecret"); + OAuthClient.AccessTokenResponse response = getTestToken("password", totpSecret); + Assert.assertNotNull(response.getAccessToken()); + Assert.assertNull(response.getError()); + events.clear(); + } + { + OAuthClient.AccessTokenResponse response = getTestToken("password", null); + Assert.assertNull(response.getAccessToken()); + Assert.assertEquals(response.getError(), "invalid_grant"); + Assert.assertEquals(response.getErrorDescription(), "Invalid user credentials"); + events.clear(); + } + { + OAuthClient.AccessTokenResponse response = getTestToken("password", null); + Assert.assertNull(response.getAccessToken()); + Assert.assertEquals(response.getError(), "invalid_grant"); + Assert.assertEquals(response.getErrorDescription(), "Invalid user credentials"); + events.clear(); + } + { + String totpSecret = totp.generate("totpSecret"); + OAuthClient.AccessTokenResponse response = getTestToken("password", totpSecret); + Assert.assertNull(response.getAccessToken()); + Assert.assertNotNull(response.getError()); + Assert.assertEquals(response.getError(), "invalid_grant"); + Assert.assertEquals(response.getErrorDescription(), "Account temporarily disabled"); + events.clear(); + } + clearUserFailures(); + { + String totpSecret = totp.generate("totpSecret"); + OAuthClient.AccessTokenResponse response = getTestToken("password", totpSecret); + Assert.assertNotNull(response.getAccessToken()); + Assert.assertNull(response.getError()); + events.clear(); + } + } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java index 745a5a2e1d..c4f6a8488d 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/LoginTest.java @@ -470,16 +470,23 @@ public class LoginTest { try { loginPage.open(); Time.setOffset(5000); + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + manager.getSession().sessions().removeExpiredUserSessions(appRealm); + } + }); + loginPage.login("login@test.com", "password"); //loginPage.assertCurrent(); - errorPage.assertCurrent(); + loginPage.assertCurrent(); //Assert.assertEquals("Login timeout. Please login again.", loginPage.getError()); events.expectLogin().user((String) null).session((String) null).error("expired_code").clearDetails() - .detail(Details.CODE_ID, AssertEvents.isCodeId()) - .removeDetail(Details.CONSENT) + .detail(Details.RESTART_AFTER_TIMEOUT, "true") + .client((String) null) .assertEvent(); } finally { diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java index e5b44a9e0b..bc4c991bcf 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java @@ -144,6 +144,8 @@ public class ResourceOwnerPasswordCredentialsGrantTest { .error(Errors.INVALID_TOKEN).assertEvent(); } + + @Test public void grantAccessTokenInvalidClientCredentials() throws Exception { oauth.clientId("resource-owner"); @@ -163,6 +165,48 @@ public class ResourceOwnerPasswordCredentialsGrantTest { .assertEvent(); } + @Test + public void grantAccessTokenVerifyEmail() throws Exception { + + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + appRealm.setVerifyEmail(true); + } + }); + + + oauth.clientId("resource-owner"); + + OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("secret", "test-user@localhost", "password"); + + assertEquals(400, response.getStatusCode()); + + assertEquals("invalid_grant", response.getError()); + assertEquals("Account is not fully set up", response.getErrorDescription()); + + events.expectLogin() + .client("resource-owner") + .session((String) null) + .clearDetails() + .error(Errors.RESOLVE_REQUIRED_ACTIONS) + .user((String) null) + .assertEvent(); + + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + appRealm.setVerifyEmail(false); + UserModel user = manager.getSession().users().getUserByEmail("test-user@localhost", appRealm); + user.removeRequiredAction(UserModel.RequiredAction.VERIFY_EMAIL); + } + }); + + } + + + + @Test public void grantAccessTokenInvalidUserCredentials() throws Exception { oauth.clientId("resource-owner");