Skip to content

Commit

Permalink
trigger REMOVE_TOTP event on removal of an OTP credential
Browse files Browse the repository at this point in the history
Closes keycloak#15403

Signed-off-by: Theresa Henze <[email protected]>
  • Loading branch information
resah authored and sventorben committed Mar 6, 2024
1 parent d4a4027 commit 74e0494
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public interface Details {
String CREDENTIAL_TYPE = "credential_type";
String SELECTED_CREDENTIAL_ID = "selected_credential_id";
String AUTHENTICATION_ERROR_DETAIL = "authentication_error_detail";
String CREDENTIAL_USER_LABEL = "credential_user_label";

String NOT_BEFORE = "not_before";
String NUM_FAILURES = "num_failures";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,16 @@
import org.keycloak.credential.CredentialProviderFactory;
import org.keycloak.credential.CredentialTypeMetadata;
import org.keycloak.credential.CredentialTypeMetadataContext;
import org.keycloak.events.Details;
import org.keycloak.events.EventBuilder;
import org.keycloak.events.EventType;
import org.keycloak.models.AccountRoles;
import org.keycloak.models.AuthenticationExecutionModel;
import org.keycloak.models.AuthenticationFlowModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.credential.OTPCredentialModel;
import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.account.CredentialMetadataRepresentation;
import org.keycloak.representations.idm.CredentialRepresentation;
Expand Down Expand Up @@ -61,11 +65,13 @@ public class AccountCredentialResource {
private final UserModel user;
private final RealmModel realm;
private Auth auth;
private final EventBuilder event;

public AccountCredentialResource(KeycloakSession session, UserModel user, Auth auth) {
public AccountCredentialResource(KeycloakSession session, UserModel user, Auth auth, EventBuilder event) {
this.session = session;
this.user = user;
this.auth = auth;
this.event = event;
realm = session.getContext().getRealm();
}

Expand Down Expand Up @@ -297,11 +303,17 @@ public void removeCredential(final @PathParam("credentialId") String credentialI
user.credentialManager().disableCredentialType(credentialType);
return;
}

throw new NotFoundException("Credential not found");
}
checkIfCanBeRemoved(credential.getType());
user.credentialManager().removeStoredCredentialById(credentialId);

if (OTPCredentialModel.TYPE.equals(credential.getType())) {
event.event(EventType.REMOVE_TOTP)
.detail(Details.SELECTED_CREDENTIAL_ID, credentialId)
.detail(Details.CREDENTIAL_USER_LABEL, credential.getUserLabel());
event.success();
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ public SessionResource sessions() {
@Path("/credentials")
public AccountCredentialResource credentials() {
checkAccountApiEnabled();
return new AccountCredentialResource(session, user, auth);
return new AccountCredentialResource(session, user, auth, event);
}

@Path("/resources")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.keycloak.models.credential.PasswordCredentialModel;
import org.keycloak.models.credential.WebAuthnCredentialModel;
import org.keycloak.models.utils.DefaultAuthenticationFlows;
import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.account.ClientRepresentation;
import org.keycloak.representations.account.ConsentRepresentation;
import org.keycloak.representations.account.ConsentScopeRepresentation;
Expand Down Expand Up @@ -789,6 +790,79 @@ public void testCRUDCredentialOfDifferentUser() throws IOException {
Assert.assertTrue(ObjectUtil.isEqualOrBothNull(otpCredential.getUserLabel(), otpCredentialLoaded.getUserLabel()));
}

@Test
public void testRemoveCredentialWithNonOtpCredentialTriggeringNoEvent() throws IOException {

List<AccountCredentialResource.CredentialContainer> credentials = getCredentials();

UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "test-user@localhost");
assertEquals(1, user.credentials().size());

// Add non-OTP credential to the user through admin REST API
CredentialRepresentation nonOtpCredential = ModelToRepresentation.toRepresentation(
WebAuthnCredentialModel.create(WebAuthnCredentialModel.TYPE_TWOFACTOR, "foo", "foo", "foo", "foo", "foo", 2L, "foo"));
org.keycloak.representations.idm.UserRepresentation userRep = UserBuilder.edit(user.toRepresentation())
.secret(nonOtpCredential)
.build();
user.update(userRep);

credentials = getCredentials();
Assert.assertEquals(2, credentials.size());
Assert.assertTrue(credentials.get(1).isRemoveable());

// Remove credential
CredentialRepresentation credential = user.credentials().stream()
.filter(credentialRep -> WebAuthnCredentialModel.TYPE_TWOFACTOR.equals(credentialRep.getType()))
.findFirst()
.get();
Assert.assertNotNull(credential);
user.removeCredential(credential.getId());

events.poll();
events.assertEmpty();
}

@Test
public void testRemoveCredentialWithOtpCredentialTriggeringEvent() throws IOException {

List<AccountCredentialResource.CredentialContainer> credentials = getCredentials();

UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "test-user@localhost");
assertEquals(1, user.credentials().size());

// Add OTP credential to the user through admin REST API
org.keycloak.representations.idm.UserRepresentation userRep = UserBuilder.edit(user.toRepresentation())
.totpSecret("totpSecret")
.build();
userRep.getCredentials().get(0).setUserLabel("totpCredentialUserLabel");
user.update(userRep);

credentials = getCredentials();
Assert.assertEquals(2, credentials.size());
Assert.assertTrue(credentials.get(1).isRemoveable());

// Remove credential
CredentialRepresentation otpCredential = user.credentials().stream()
.filter(credentialRep -> OTPCredentialModel.TYPE.equals(credentialRep.getType()))
.findFirst()
.get();
SimpleHttp.Response response = SimpleHttp
.doDelete(getAccountUrl("credentials/" + otpCredential.getId()), httpClient)
.acceptJson()
.auth(tokenUtil.getToken())
.asResponse();
assertEquals(204, response.getStatus());

events.poll();
events.expect(EventType.REMOVE_TOTP)
.client("account")
.user(user.toRepresentation().getId())
.detail(Details.SELECTED_CREDENTIAL_ID, otpCredential.getId())
.detail(Details.CREDENTIAL_USER_LABEL, "totpCredentialUserLabel")
.assertEvent();
events.assertEmpty();
}

// Send REST request to get all credential containers and credentials of current user
private List<AccountCredentialResource.CredentialContainer> getCredentials() throws IOException {
return SimpleHttp.doGet(getAccountUrl("credentials"), httpClient)
Expand Down Expand Up @@ -1688,7 +1762,7 @@ public void testCustomAccountResourceTheme() throws Exception {
testRealm().update(realmRep);
}
}

@EnableFeature(Profile.Feature.UPDATE_EMAIL)
public void testEmailWhenUpdateEmailEnabled() throws Exception {
reconnectAdminClient();
Expand Down

0 comments on commit 74e0494

Please sign in to comment.