KEYCLOAK-12869 REST sends credential type when no credential exists and credential disabled

services/src/main/java/org/keycloak/services/resources/account/AccountCredentialResource.java

@@ -158,13 +158,13 @@ public class AccountCredentialResource {
                                                      @QueryParam(USER_CREDENTIALS) Boolean userCredentials) {
         auth.requireOneOf(AccountRoles.MANAGE_ACCOUNT, AccountRoles.VIEW_PROFILE);
 
-        boolean filterUserCredentials = userCredentials != null && !userCredentials;
+        boolean includeUserCredentials = userCredentials == null || userCredentials;
 
         List<CredentialContainer> credentialTypes = new LinkedList<>();
         List<CredentialProvider> credentialProviders = UserCredentialStoreManager.getCredentialProviders(session, realm, CredentialProvider.class);
         Set<String> enabledCredentialTypes = getEnabledCredentialTypes(credentialProviders);
 
-        List<CredentialModel> models = filterUserCredentials ? null : session.userCredentialManager().getStoredCredentials(realm, user);
+        List<CredentialModel> models = includeUserCredentials ? session.userCredentialManager().getStoredCredentials(realm, user) : null;
 
         // Don't return secrets from REST endpoint
         if (models != null) {
@@ -193,18 +193,27 @@ public class AccountCredentialResource {
                     .build(session);
             CredentialTypeMetadata metadata = credentialProvider.getCredentialTypeMetadata(ctx);
 
-            List<CredentialRepresentation> userCredentialModels = filterUserCredentials ? null : models.stream()
+            List<CredentialRepresentation> userCredentialModels = null;
-                    .filter(credentialModel -> credentialProvider.getType().equals(credentialModel.getType()))
+            if (includeUserCredentials) {
-                    .map(ModelToRepresentation::toRepresentation)
+                userCredentialModels = models.stream()
-                    .collect(Collectors.toList());
+                        .filter(credentialModel -> credentialProvider.getType().equals(credentialModel.getType()))
+                        .map(ModelToRepresentation::toRepresentation)
+                        .collect(Collectors.toList());
 
-            if (userCredentialModels != null && userCredentialModels.isEmpty() &&
+                if (userCredentialModels.isEmpty() &&
-                    session.userCredentialManager().isConfiguredFor(realm, user, credentialProviderType)) {
+                        session.userCredentialManager().isConfiguredFor(realm, user, credentialProviderType)) {
-                // In case user is federated in the userStorage, he may have credential configured on the userStorage side. We're
+                    // In case user is federated in the userStorage, he may have credential configured on the userStorage side. We're
-                // creating "dummy" credential representing the credential provided by userStorage
+                    // creating "dummy" credential representing the credential provided by userStorage
-                CredentialRepresentation credential = createUserStorageCredentialRepresentation(credentialProviderType);
+                    CredentialRepresentation credential = createUserStorageCredentialRepresentation(credentialProviderType);
 
-                userCredentialModels = Collections.singletonList(credential);
+                    userCredentialModels = Collections.singletonList(credential);
+                }
+
+                // In case that there are no userCredentials AND there are not required actions for setup new credential,
+                // we won't include credentialType as user won't be able to do anything with it
+                if (userCredentialModels.isEmpty() && metadata.getCreateAction() == null && metadata.getUpdateAction() == null) {
+                    continue;
+                }
             }
 
             CredentialContainer credType = new CredentialContainer(metadata, userCredentialModels);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceTest.java

@@ -75,6 +75,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.keycloak.common.Profile.Feature.ACCOUNT_API;
+import org.keycloak.testsuite.util.UserBuilder;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -320,6 +321,19 @@ public class AccountRestServiceTest extends AbstractRestServiceTest {
     public void testCredentialsGet() throws IOException {
         configureBrowserFlowWithWebAuthnAuthenticator("browser-webauthn");
 
+        // Register requiredActions for WebAuthn and WebAuthn Passwordless
+        RequiredActionProviderSimpleRepresentation requiredAction = new RequiredActionProviderSimpleRepresentation();
+        requiredAction.setId("12345");
+        requiredAction.setName(WebAuthnRegisterFactory.PROVIDER_ID);
+        requiredAction.setProviderId(WebAuthnRegisterFactory.PROVIDER_ID);
+        testRealm().flows().registerRequiredAction(requiredAction);
+
+        requiredAction = new RequiredActionProviderSimpleRepresentation();
+        requiredAction.setId("6789");
+        requiredAction.setName(WebAuthnPasswordlessRegisterFactory.PROVIDER_ID);
+        requiredAction.setProviderId(WebAuthnPasswordlessRegisterFactory.PROVIDER_ID);
+        testRealm().flows().registerRequiredAction(requiredAction);
+
         List<AccountCredentialResource.CredentialContainer> credentials = getCredentials();
 
         Assert.assertEquals(4, credentials.size());
@@ -342,47 +356,25 @@ public class AccountRestServiceTest extends AbstractRestServiceTest {
         AccountCredentialResource.CredentialContainer webauthn = credentials.get(2);
         assertCredentialContainerExpected(webauthn, WebAuthnCredentialModel.TYPE_TWOFACTOR, CredentialTypeMetadata.Category.TWO_FACTOR.toString(),
                 "webauthn-display-name", "webauthn-help-text", "kcAuthenticatorWebAuthnClass",
-                null, null, true, 0);
+                WebAuthnRegisterFactory.PROVIDER_ID, null, true, 0);
 
         AccountCredentialResource.CredentialContainer webauthnPasswordless = credentials.get(3);
         assertCredentialContainerExpected(webauthnPasswordless, WebAuthnCredentialModel.TYPE_PASSWORDLESS, CredentialTypeMetadata.Category.PASSWORDLESS.toString(),
                 "webauthn-passwordless-display-name", "webauthn-passwordless-help-text", "kcAuthenticatorWebAuthnPasswordlessClass",
-                null, null, true, 0);
+                WebAuthnPasswordlessRegisterFactory.PROVIDER_ID, null, true, 0);
 
-        // Register requiredActions for WebAuthn
+        // disable WebAuthn passwordless required action. User doesn't have WebAuthnPasswordless credential, so WebAuthnPasswordless credentialType won't be returned
-        RequiredActionProviderSimpleRepresentation requiredAction = new RequiredActionProviderSimpleRepresentation();
+        setRequiredActionEnabledStatus(WebAuthnPasswordlessRegisterFactory.PROVIDER_ID, false);
-        requiredAction.setId("12345");
-        requiredAction.setName(WebAuthnRegisterFactory.PROVIDER_ID);
-        requiredAction.setProviderId(WebAuthnRegisterFactory.PROVIDER_ID);
-        testRealm().flows().registerRequiredAction(requiredAction);
-
-        requiredAction = new RequiredActionProviderSimpleRepresentation();
-        requiredAction.setId("6789");
-        requiredAction.setName(WebAuthnPasswordlessRegisterFactory.PROVIDER_ID);
-        requiredAction.setProviderId(WebAuthnPasswordlessRegisterFactory.PROVIDER_ID);
-        testRealm().flows().registerRequiredAction(requiredAction);
-
-        // requiredActions should be available
-        credentials = getCredentials();
-        Assert.assertEquals(WebAuthnRegisterFactory.PROVIDER_ID, credentials.get(2).getCreateAction());
-        Assert.assertEquals(WebAuthnPasswordlessRegisterFactory.PROVIDER_ID, credentials.get(3).getCreateAction());
-
-        // disable WebAuthn passwordless required action. It won't be returned then
-        RequiredActionProviderRepresentation requiredActionRep = testRealm().flows().getRequiredAction(WebAuthnPasswordlessRegisterFactory.PROVIDER_ID);
-        requiredActionRep.setEnabled(false);
-        testRealm().flows().updateRequiredAction(WebAuthnRegisterFactory.PROVIDER_ID, requiredActionRep);
 
         credentials = getCredentials();
-        Assert.assertNull(credentials.get(2).getCreateAction());
+        assertExpectedCredentialTypes(credentials, PasswordCredentialModel.TYPE, OTPCredentialModel.TYPE, WebAuthnCredentialModel.TYPE_TWOFACTOR);
 
         // Test that WebAuthn won't be returned when removed from the authentication flow
         removeWebAuthnFlow("browser-webauthn");
 
         credentials = getCredentials();
 
-        Assert.assertEquals(2, credentials.size());
+        assertExpectedCredentialTypes(credentials, PasswordCredentialModel.TYPE, OTPCredentialModel.TYPE);
-        Assert.assertEquals(PasswordCredentialModel.TYPE, credentials.get(0).getType());
-        Assert.assertNotNull(OTPCredentialModel.TYPE, credentials.get(1).getType());
 
         // Test password-only
         credentials = SimpleHttp.doGet(getAccountUrl("credentials?" + AccountCredentialResource.TYPE + "=password"), httpClient)
@@ -452,6 +444,59 @@ public class AccountRestServiceTest extends AbstractRestServiceTest {
         }
     }
 
+    @Test
+    public void testCredentialsGetWithDisabledOtpRequiredAction() throws IOException {
+        // Assert OTP will be returned by default
+        List<AccountCredentialResource.CredentialContainer> credentials = getCredentials();
+        assertExpectedCredentialTypes(credentials, PasswordCredentialModel.TYPE, OTPCredentialModel.TYPE);
+
+        // Disable OTP required action
+        setRequiredActionEnabledStatus(UserModel.RequiredAction.CONFIGURE_TOTP.name(), false);
+
+        // Assert OTP won't be returned
+        credentials = getCredentials();
+        assertExpectedCredentialTypes(credentials, PasswordCredentialModel.TYPE);
+
+        // Add OTP credential to the user through admin REST API
+        UserResource adminUserResource = ApiUtil.findUserByUsernameId(testRealm(), "test-user@localhost");
+        org.keycloak.representations.idm.UserRepresentation userRep = UserBuilder.edit(adminUserResource.toRepresentation())
+                .totpSecret("abcdefabcdef")
+                .build();
+        adminUserResource.update(userRep);
+
+        // Assert OTP will be returned without requiredAction
+        credentials = getCredentials();
+        assertExpectedCredentialTypes(credentials, PasswordCredentialModel.TYPE, OTPCredentialModel.TYPE);
+        AccountCredentialResource.CredentialContainer otpCredential = credentials.get(1);
+        Assert.assertNull(otpCredential.getCreateAction());
+        Assert.assertNull(otpCredential.getUpdateAction());
+
+        // Revert - re-enable requiredAction and remove OTP credential from the user
+        setRequiredActionEnabledStatus(UserModel.RequiredAction.CONFIGURE_TOTP.name(), true);
+
+        String otpCredentialId = adminUserResource.credentials().stream()
+                .filter(credential -> OTPCredentialModel.TYPE.equals(credential.getType()))
+                .findFirst()
+                .get()
+                .getId();
+        adminUserResource.removeCredential(otpCredentialId);
+    }
+
+    private void setRequiredActionEnabledStatus(String requiredActionProviderId, boolean enabled) {
+        RequiredActionProviderRepresentation requiredActionRep = testRealm().flows().getRequiredAction(requiredActionProviderId);
+        requiredActionRep.setEnabled(enabled);
+        testRealm().flows().updateRequiredAction(requiredActionProviderId, requiredActionRep);
+    }
+
+    private void assertExpectedCredentialTypes(List<AccountCredentialResource.CredentialContainer> credentialTypes, String... expectedCredentialTypes) {
+        Assert.assertEquals(credentialTypes.size(), expectedCredentialTypes.length);
+        int i = 0;
+        for (AccountCredentialResource.CredentialContainer credential : credentialTypes) {
+            Assert.assertEquals(credential.getType(), expectedCredentialTypes[i]);
+            i++;
+        }
+    }
+
     @Test
     public void testCredentialsForUserWithoutPassword() throws IOException {
         // This is just to call REST to ensure tokenUtil will authenticate user and create the tokens.