KEYCLOAK-15170 Reset password link is not invalidated if email address is changed

This commit is contained in:
Václav Muzikář 2021-04-30 16:25:28 +02:00 committed by Marek Posolda
parent 4f08912071
commit 57fca2a34f
15 changed files with 107 additions and 37 deletions

View file

@ -17,18 +17,21 @@
package org.keycloak.authentication.actiontoken;
import org.keycloak.Config.Scope;
import org.keycloak.TokenVerifier;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.representations.JsonWebToken;
import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.messages.Messages;
import org.keycloak.sessions.AuthenticationSessionModel;
/**
*
* @author hmlnarik
*/
public abstract class AbstractActionTokenHander<T extends JsonWebToken> implements ActionTokenHandler<T>, ActionTokenHandlerFactory<T> {
public abstract class AbstractActionTokenHandler<T extends JsonWebToken> implements ActionTokenHandler<T>, ActionTokenHandlerFactory<T> {
private final String id;
private final Class<T> tokenClass;
@ -36,7 +39,7 @@ public abstract class AbstractActionTokenHander<T extends JsonWebToken> implemen
private final EventType defaultEventType;
private final String defaultEventError;
public AbstractActionTokenHander(String id, Class<T> tokenClass, String defaultErrorMessage, EventType defaultEventType, String defaultEventError) {
public AbstractActionTokenHandler(String id, Class<T> tokenClass, String defaultErrorMessage, EventType defaultEventType, String defaultEventError) {
this.id = id;
this.tokenClass = tokenClass;
this.defaultErrorMessage = defaultErrorMessage;
@ -102,4 +105,11 @@ public abstract class AbstractActionTokenHander<T extends JsonWebToken> implemen
public boolean canUseTokenRepeatedly(T token, ActionTokenContext<T> tokenContext) {
return true;
}
protected TokenVerifier.Predicate<DefaultActionToken> verifyEmail(ActionTokenContext<? extends DefaultActionToken> context) {
return TokenUtils.checkThat(
t -> t.getEmail() == null || t.getEmail().equals(context.getAuthenticationSession().getAuthenticatedUser().getEmail()),
Errors.INVALID_EMAIL, Messages.INVALID_EMAIL
);
}
}

View file

@ -17,16 +17,20 @@
package org.keycloak.authentication.actiontoken;
import org.keycloak.TokenVerifier.Predicate;
import org.keycloak.common.VerificationException;
import org.keycloak.common.util.Time;
import org.keycloak.models.*;
import org.keycloak.services.Urls;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.util.*;
import org.keycloak.TokenVerifier.Predicate;
import org.keycloak.common.VerificationException;
import org.keycloak.common.util.Time;
import org.keycloak.models.ActionTokenValueModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.services.Urls;
import javax.ws.rs.core.UriInfo;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
/**
* Part of action token that is intended to be used e.g. in link sent in password-reset email.
@ -37,6 +41,10 @@ import javax.ws.rs.core.UriInfo;
public class DefaultActionToken extends DefaultActionTokenKey implements ActionTokenValueModel {
public static final String JSON_FIELD_AUTHENTICATION_SESSION_ID = "asid";
public static final String JSON_FIELD_EMAIL = "eml";
@JsonProperty(value = JSON_FIELD_EMAIL)
private String email;
public static final Predicate<DefaultActionTokenKey> ACTION_TOKEN_BASIC_CHECKS = t -> {
if (t.getActionVerificationNonce() == null) {
@ -122,6 +130,14 @@ public class DefaultActionToken extends DefaultActionTokenKey implements ActionT
return res instanceof String ? (String) res : null;
}
public void setEmail(String email) {
this.email = email;
}
public String getEmail() {
return email;
}
/**
* Updates the following fields and serializes this token into a signed JWT. The list of updated fields follows:
* <ul>

View file

@ -38,6 +38,11 @@ public class ExecuteActionsActionToken extends DefaultActionToken {
this.issuedFor = clientId;
}
public ExecuteActionsActionToken(String userId, String email, int absoluteExpirationInSecs, List<String> requiredActions, String redirectUri, String clientId) {
this(userId, absoluteExpirationInSecs, requiredActions, redirectUri, clientId);
setEmail(email);
}
private ExecuteActionsActionToken() {
}

View file

@ -40,7 +40,7 @@ import javax.ws.rs.core.UriInfo;
*
* @author hmlnarik
*/
public class ExecuteActionsActionTokenHandler extends AbstractActionTokenHander<ExecuteActionsActionToken> {
public class ExecuteActionsActionTokenHandler extends AbstractActionTokenHandler<ExecuteActionsActionToken> {
public ExecuteActionsActionTokenHandler() {
super(
@ -62,7 +62,9 @@ public class ExecuteActionsActionTokenHandler extends AbstractActionTokenHander<
tokenContext.getAuthenticationSession().getClient()) != null,
Errors.INVALID_REDIRECT_URI,
Messages.INVALID_REDIRECT_URI
)
),
verifyEmail(tokenContext)
);
}

View file

@ -42,12 +42,13 @@ public class IdpVerifyAccountLinkActionToken extends DefaultActionToken {
private String originalAuthenticationSessionId;
public IdpVerifyAccountLinkActionToken(String userId, int absoluteExpirationInSecs, String compoundAuthenticationSessionId,
public IdpVerifyAccountLinkActionToken(String userId, String email, int absoluteExpirationInSecs, String compoundAuthenticationSessionId,
String identityProviderUsername, String identityProviderAlias, String clientId) {
super(userId, TOKEN_TYPE, absoluteExpirationInSecs, null, compoundAuthenticationSessionId);
this.identityProviderUsername = identityProviderUsername;
this.identityProviderAlias = identityProviderAlias;
this.issuedFor = clientId;
setEmail(email);
}
private IdpVerifyAccountLinkActionToken() {

View file

@ -16,7 +16,7 @@
*/
package org.keycloak.authentication.actiontoken.idpverifyemail;
import org.keycloak.authentication.actiontoken.AbstractActionTokenHander;
import org.keycloak.authentication.actiontoken.AbstractActionTokenHandler;
import org.keycloak.TokenVerifier.Predicate;
import org.keycloak.authentication.AuthenticationProcessor;
import org.keycloak.authentication.actiontoken.*;
@ -43,7 +43,7 @@ import javax.ws.rs.core.UriInfo;
* Action token handler for verification of e-mail address.
* @author hmlnarik
*/
public class IdpVerifyAccountLinkActionTokenHandler extends AbstractActionTokenHander<IdpVerifyAccountLinkActionToken> {
public class IdpVerifyAccountLinkActionTokenHandler extends AbstractActionTokenHandler<IdpVerifyAccountLinkActionToken> {
public IdpVerifyAccountLinkActionTokenHandler() {
super(
@ -58,6 +58,7 @@ public class IdpVerifyAccountLinkActionTokenHandler extends AbstractActionTokenH
@Override
public Predicate<? super IdpVerifyAccountLinkActionToken>[] getVerifiers(ActionTokenContext<IdpVerifyAccountLinkActionToken> tokenContext) {
return TokenUtils.predicates(
verifyEmail(tokenContext)
);
}

View file

@ -27,9 +27,10 @@ public class ResetCredentialsActionToken extends DefaultActionToken {
public static final String TOKEN_TYPE = "reset-credentials";
public ResetCredentialsActionToken(String userId, int absoluteExpirationInSecs, String compoundAuthenticationSessionId, String clientId) {
public ResetCredentialsActionToken(String userId, String email, int absoluteExpirationInSecs, String compoundAuthenticationSessionId, String clientId) {
super(userId, TOKEN_TYPE, absoluteExpirationInSecs, null, compoundAuthenticationSessionId);
this.issuedFor = clientId;
setEmail(email);
}
private ResetCredentialsActionToken() {

View file

@ -24,19 +24,19 @@ import org.keycloak.authentication.authenticators.broker.util.SerializedBrokered
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
import org.keycloak.models.UserModel;
import org.keycloak.services.ErrorPage;
import org.keycloak.services.messages.Messages;
import org.keycloak.services.resources.LoginActionsService;
import org.keycloak.services.resources.LoginActionsServiceChecks.IsActionRequired;
import org.keycloak.sessions.CommonClientSessionModel.Action;
import javax.ws.rs.core.Response;
import static org.keycloak.services.resources.LoginActionsService.RESET_CREDENTIALS_PATH;
/**
*
* @author hmlnarik
*/
public class ResetCredentialsActionTokenHandler extends AbstractActionTokenHander<ResetCredentialsActionToken> {
public class ResetCredentialsActionTokenHandler extends AbstractActionTokenHandler<ResetCredentialsActionToken> {
public ResetCredentialsActionTokenHandler() {
super(
@ -51,11 +51,13 @@ public class ResetCredentialsActionTokenHandler extends AbstractActionTokenHande
@Override
public Predicate<? super ResetCredentialsActionToken>[] getVerifiers(ActionTokenContext<ResetCredentialsActionToken> tokenContext) {
return new Predicate[] {
return TokenUtils.predicates(
TokenUtils.checkThat(tokenContext.getRealm()::isResetPasswordAllowed, Errors.NOT_ALLOWED, Messages.RESET_CREDENTIAL_NOT_ALLOWED),
verifyEmail(tokenContext),
new IsActionRequired(tokenContext, Action.AUTHENTICATE)
};
);
}
@Override

View file

@ -28,32 +28,20 @@ public class VerifyEmailActionToken extends DefaultActionToken {
public static final String TOKEN_TYPE = "verify-email";
private static final String JSON_FIELD_EMAIL = "eml";
private static final String JSON_FIELD_ORIGINAL_AUTHENTICATION_SESSION_ID = "oasid";
@JsonProperty(value = JSON_FIELD_EMAIL)
private String email;
@JsonProperty(value = JSON_FIELD_ORIGINAL_AUTHENTICATION_SESSION_ID)
private String originalAuthenticationSessionId;
public VerifyEmailActionToken(String userId, int absoluteExpirationInSecs, String compoundAuthenticationSessionId, String email, String clientId) {
super(userId, TOKEN_TYPE, absoluteExpirationInSecs, null, compoundAuthenticationSessionId);
this.email = email;
setEmail(email);
this.issuedFor = clientId;
}
private VerifyEmailActionToken() {
}
public String getEmail() {
return email;
}
public void setEmail(String email) {
this.email = email;
}
public String getCompoundOriginalAuthenticationSessionId() {
return originalAuthenticationSessionId;
}

View file

@ -16,7 +16,7 @@
*/
package org.keycloak.authentication.actiontoken.verifyemail;
import org.keycloak.authentication.actiontoken.AbstractActionTokenHander;
import org.keycloak.authentication.actiontoken.AbstractActionTokenHandler;
import org.keycloak.TokenVerifier.Predicate;
import org.keycloak.authentication.actiontoken.*;
import org.keycloak.events.*;
@ -41,7 +41,7 @@ import javax.ws.rs.core.UriInfo;
* Action token handler for verification of e-mail address.
* @author hmlnarik
*/
public class VerifyEmailActionTokenHandler extends AbstractActionTokenHander<VerifyEmailActionToken> {
public class VerifyEmailActionTokenHandler extends AbstractActionTokenHandler<VerifyEmailActionToken> {
public VerifyEmailActionTokenHandler() {
super(

View file

@ -129,7 +129,7 @@ public class IdpEmailVerificationAuthenticator extends AbstractIdpAuthenticator
String authSessionEncodedId = AuthenticationSessionCompoundId.fromAuthSession(authSession).getEncodedId();
IdpVerifyAccountLinkActionToken token = new IdpVerifyAccountLinkActionToken(
existingUser.getId(), absoluteExpirationInSecs, authSessionEncodedId,
existingUser.getId(), existingUser.getEmail(), absoluteExpirationInSecs, authSessionEncodedId,
brokerContext.getUsername(), brokerContext.getIdpConfig().getAlias(), authSession.getClient().getClientId()
);
UriBuilder builder = Urls.actionTokenBuilder(uriInfo.getBaseUri(), token.serialize(session, realm, uriInfo),

View file

@ -91,7 +91,7 @@ public class ResetCredentialEmail implements Authenticator, AuthenticatorFactory
// We send the secret in the email in a link as a query param.
String authSessionEncodedId = AuthenticationSessionCompoundId.fromAuthSession(authenticationSession).getEncodedId();
ResetCredentialsActionToken token = new ResetCredentialsActionToken(user.getId(), absoluteExpirationInSecs, authSessionEncodedId, authenticationSession.getClient().getClientId());
ResetCredentialsActionToken token = new ResetCredentialsActionToken(user.getId(), user.getEmail(), absoluteExpirationInSecs, authSessionEncodedId, authenticationSession.getClient().getClientId());
String link = UriBuilder
.fromUri(context.getActionTokenUrl(token.serialize(context.getSession(), context.getRealm(), context.getUriInfo())))
.build()

View file

@ -781,7 +781,7 @@ public class UserResource {
lifespan = realm.getActionTokenGeneratedByAdminLifespan();
}
int expiration = Time.currentTime() + lifespan;
ExecuteActionsActionToken token = new ExecuteActionsActionToken(user.getId(), expiration, actions, redirectUri, clientId);
ExecuteActionsActionToken token = new ExecuteActionsActionToken(user.getId(), user.getEmail(), expiration, actions, redirectUri, clientId);
try {
UriBuilder builder = LoginActionsService.actionTokenProcessor(session.getContext().getUri());

View file

@ -17,6 +17,7 @@
package org.keycloak.testsuite.actions;
import org.jboss.arquillian.drone.api.annotation.Drone;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.authentication.actiontoken.verifyemail.VerifyEmailActionToken;
import org.jboss.arquillian.graphene.page.Page;
import org.junit.Assert;
@ -999,4 +1000,27 @@ public class RequiredActionEmailVerificationTest extends AbstractTestRealmKeyclo
setTimeOffset(0);
}
}
// KEYCLOAK-15170
@Test
public void changeEmailAddressAfterSendingEmail() throws Exception {
loginPage.open();
loginPage.login("test-user@localhost", "password");
verifyEmailPage.assertCurrent();
assertEquals(1, greenMail.getReceivedMessages().length);
MimeMessage message = greenMail.getReceivedMessages()[0];
String verificationUrl = getPasswordResetEmailLink(message);
UserResource user = testRealm().users().get(testUserId);
UserRepresentation userRep = user.toRepresentation();
userRep.setEmail("vmuzikar@redhat.com");
user.update(userRep);
driver.navigate().to(verificationUrl.trim());
errorPage.assertCurrent();
assertEquals("The link you clicked is an old stale link and is no longer valid. Maybe you have already verified your email.", errorPage.getError());
}
}

View file

@ -1156,6 +1156,26 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest {
}
}
// KEYCLOAK-15170
@Test
public void changeEmailAddressAfterSendingEmail() throws IOException {
initiateResetPasswordFromResetPasswordPage(defaultUser.getUsername());
assertEquals(1, greenMail.getReceivedMessages().length);
MimeMessage message = greenMail.getReceivedMessages()[0];
String changePasswordUrl = MailUtils.getPasswordResetEmailLink(message);
UserResource user = testRealm().users().get(defaultUser.getId());
UserRepresentation userRep = user.toRepresentation();
userRep.setEmail("vmuzikar@redhat.com");
user.update(userRep);
driver.navigate().to(changePasswordUrl.trim());
errorPage.assertCurrent();
assertEquals("Invalid email address.", errorPage.getError());
}
private void changePasswordOnUpdatePage(WebDriver driver) {
assertThat(driver.getPageSource(), Matchers.containsString("You need to change your password."));