From ce3aeab653ab0a7e28e3ef105a66c7b79b4f09a8 Mon Sep 17 00:00:00 2001 From: Alexander Klimetschek Date: Wed, 21 Sep 2016 16:02:08 -0700 Subject: [PATCH] OAK-4825 - Support disabling of users instead of removal in DefaultSyncHandler --- .../authentication/external/SyncResult.java | 10 ++++ .../external/basic/DefaultSyncConfig.java | 18 ++++++ .../external/basic/DefaultSyncContext.java | 34 ++++++++++-- .../external/impl/DefaultSyncConfigImpl.java | 17 ++++++ .../external/impl/jmx/Delegatee.java | 8 +++ .../authentication/external/package-info.java | 2 +- .../external/AbstractExternalAuthTest.java | 4 ++ .../external/TestIdentityProvider.java | 6 +- .../external/basic/DefaultSyncContextTest.java | 65 ++++++++++++++++++++++ 9 files changed, 155 insertions(+), 8 deletions(-) diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/SyncResult.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/SyncResult.java index 45760e6..954ef50 100644 --- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/SyncResult.java +++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/SyncResult.java @@ -64,6 +64,16 @@ DELETE, /** + * authorizable enabled + */ + ENABLE, + + /** + * authorizable disabled + */ + DISABLE, + + /** * nothing changed. no such authorizable found. */ NO_SUCH_AUTHORIZABLE, diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfig.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfig.java index 38736b8..d17174c 100644 --- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfig.java +++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncConfig.java @@ -229,6 +229,8 @@ public Authorizable setPathPrefix(@Nonnull String pathPrefix) { private boolean dynamicMembership; + private boolean disableMissing; + /** * Returns the duration in milliseconds until the group membership of a user is expired. If the * membership information is expired it is re-synced according to the maximum nesting depth. @@ -310,6 +312,22 @@ public User setDynamicMembership(boolean dynamicMembership) { return this; } + /** + * Controls the behavior for users that no longer exist on the external provider. The default is to delete the repository users + * if they no longer exist on the external provider. If set to true, they will be disabled instead, and re-enabled once they appear + * again. + */ + public boolean getDisableMissing() { + return disableMissing; + } + + /** + * @see #getDisableMissing() + */ + public User setDisableMissing(boolean disableMissing) { + this.disableMissing = disableMissing; + return this; + } } /** diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java index a46ff35..e4badec 100644 --- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java +++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java @@ -325,11 +325,27 @@ private DefaultSyncResultImpl handleMissingIdentity(@Nonnull String id, if (authorizable.isGroup() && ((Group) authorizable).getDeclaredMembers().hasNext()) { log.info("won't remove local group with members: {}", id); status = SyncResult.Status.NOP; + } else if (!keepMissing) { - authorizable.remove(); - log.debug("removing authorizable '{}' that no longer exists on IDP {}", id, idp.getName()); - timer.mark("remove"); status = SyncResult.Status.DELETE; + + if (authorizable instanceof User) { + User user = (User) authorizable; + if (config.user().getDisableMissing()) { + user.disable("No longer exists on external identity provider '" + idp.getName() + "'"); + log.debug("disabling user '{}' that no longer exists on IDP {}", id, idp.getName()); + status = SyncResult.Status.DISABLE; + + } else { + user.remove(); + log.debug("removing user '{}' that no longer exists on IDP {}", id, idp.getName()); + } + } else { + authorizable.remove(); + log.debug("removing authorizable '{}' that no longer exists on IDP {}", id, idp.getName()); + } + timer.mark("remove"); + } else { status = SyncResult.Status.MISSING; log.info("external identity missing for {}, but purge == false.", id); @@ -432,9 +448,19 @@ protected DefaultSyncResultImpl syncUser(@Nonnull ExternalUser external, @Nonnul // synchronize external memberships syncMembership(external, user, config.user().getMembershipNestingDepth()); } + + status = SyncResult.Status.UPDATE; + + if (this.config.user().getDisableMissing()) { + // ensure users get re-enabled + if (user.isDisabled()) { + status = SyncResult.Status.ENABLE; + user.disable(null); + } + } + // finally "touch" the sync property user.setProperty(REP_LAST_SYNCED, nowValue); - status = SyncResult.Status.UPDATE; } return new DefaultSyncResultImpl(createSyncedIdentity(user), status); } diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncConfigImpl.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncConfigImpl.java index c1e840f..fdb30f3 100644 --- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncConfigImpl.java +++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncConfigImpl.java @@ -178,6 +178,22 @@ public static final String PARAM_USER_DYNAMIC_MEMBERSHIP = "user.dynamicMembership"; /** + * @see User#getDisableMissing() + */ + public static final boolean PARAM_DISABLE_MISSING_USERS_DEFAULT = false; + + /** + * @see User#getDisableMissing() + */ + @Property( + label = "Disable missing users", + description = "If true, users that no longer exist on the external provider will be locally disabled, " + + "and re-enabled if they become valid again. If false (default) they will be removed.", + boolValue = false + ) + public static final String PARAM_DISABLE_MISSING_USERS = "user.disableMissing"; + + /** * @see DefaultSyncConfig.Group#getExpirationTime() */ public static final String PARAM_GROUP_EXPIRATION_TIME_DEFAULT = "1d"; @@ -268,6 +284,7 @@ public static DefaultSyncConfig of(ConfigurationParameters params) { .setName(params.getConfigValue(PARAM_NAME, PARAM_NAME_DEFAULT)); cfg.user() + .setDisableMissing(params.getConfigValue(PARAM_DISABLE_MISSING_USERS, PARAM_DISABLE_MISSING_USERS_DEFAULT)) .setMembershipExpirationTime(getMilliSeconds(params, PARAM_USER_MEMBERSHIP_EXPIRATION_TIME, PARAM_USER_MEMBERSHIP_EXPIRATION_TIME_DEFAULT, ONE_HOUR)) .setMembershipNestingDepth(params.getConfigValue(PARAM_USER_MEMBERSHIP_NESTING_DEPTH, PARAM_USER_MEMBERSHIP_NESTING_DEPTH_DEFAULT)) .setDynamicMembership(params.getConfigValue(PARAM_USER_DYNAMIC_MEMBERSHIP, PARAM_USER_DYNAMIC_MEMBERSHIP_DEFAULT)) diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/jmx/Delegatee.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/jmx/Delegatee.java index 076a2f2..a345c44 100644 --- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/jmx/Delegatee.java +++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/jmx/Delegatee.java @@ -391,6 +391,8 @@ private static void append(@Nonnull List list, @Nonnull List case ADD: case DELETE: case UPDATE: + case ENABLE: + case DISABLE: append(list, result.getIdentity(), e); break; default: @@ -415,6 +417,12 @@ private static String getOperationFromStatus(SyncResult.Status syncStatus) { case DELETE: op = "del"; break; + case ENABLE: + op = "ena"; + break; + case DISABLE: + op = "dis"; + break; case NO_SUCH_AUTHORIZABLE: op = "nsa"; break; diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/package-info.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/package-info.java index bd71b1a..479c021 100644 --- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/package-info.java +++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/package-info.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@Version("2.0.0") +@Version("2.1.0") @Export package org.apache.jackrabbit.oak.spi.security.authentication.external; diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/AbstractExternalAuthTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/AbstractExternalAuthTest.java index addf9fa..a6a1cec 100644 --- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/AbstractExternalAuthTest.java +++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/AbstractExternalAuthTest.java @@ -144,6 +144,10 @@ protected void destroyIDP() { // nothing to do } + protected void addIDPUser(String id) { + ((TestIdentityProvider) idp).addUser(new TestIdentityProvider.TestUser(id, idp.getName())); + } + protected DefaultSyncConfig createSyncConfig() { DefaultSyncConfig syncConfig = new DefaultSyncConfig(); Map mapping = new HashMap(); diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java index 9df96a0..6ccb2b3 100644 --- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java +++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java @@ -81,12 +81,12 @@ public TestIdentityProvider(@Nonnull String idpName) { .withGroups("_gr_u_", "g%r%")); } - private void addUser(TestIdentity user) { - externalUsers.put(user.getId().toLowerCase(), (TestUser) user); + public void addUser(TestIdentity user) { + externalUsers.put(user.getId().toLowerCase(), (ExternalUser) user); } private void addGroup(TestIdentity group) { - externalGroups.put(group.getId().toLowerCase(), (TestGroup) group); + externalGroups.put(group.getId().toLowerCase(), (ExternalGroup) group); } @Nonnull diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java index a09a98a..91a53ca 100644 --- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java +++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java @@ -386,6 +386,71 @@ public void testSyncRemovedUserById() throws Exception { } @Test + public void testSyncDisabledUserById() throws Exception { + // configure to disable missing users + syncConfig.user().setDisableMissing(true); + + // mark a regular repo user as external user from the test IDP + User u = userManager.createUser("test" + UUID.randomUUID(), null); + String userId = u.getID(); + setExternalID(u, idp.getName()); + + // test sync with 'keepmissing' = true + syncCtx.setKeepMissing(true); + SyncResult result = syncCtx.sync(userId); + assertEquals(SyncResult.Status.MISSING, result.getStatus()); + assertNotNull(userManager.getAuthorizable(userId)); + + // test sync with 'keepmissing' = false + syncCtx.setKeepMissing(false); + result = syncCtx.sync(userId); + assertEquals(SyncResult.Status.DISABLE, result.getStatus()); + + Authorizable authorizable = userManager.getAuthorizable(userId); + assertNotNull(authorizable); + assertTrue(((User)authorizable).isDisabled()); + + // add external user back + addIDPUser(userId); + + result = syncCtx.sync(userId); + assertEquals(SyncResult.Status.ENABLE, result.getStatus()); + assertNotNull(userManager.getAuthorizable(userId)); + assertFalse(((User)authorizable).isDisabled()); + } + + @Test + public void testSyncDoesNotEnableUsers() throws Exception { + // configure to remove missing users, check that sync does not mess with disabled status + syncConfig.user().setDisableMissing(false); + // test sync with 'keepmissing' = false + syncCtx.setKeepMissing(false); + + ExternalUser user = idp.listUsers().next(); + assertNotNull(user); + + SyncResult result = syncCtx.sync(user); + assertEquals(SyncResult.Status.ADD, result.getStatus()); + + Authorizable authorizable = userManager.getAuthorizable(user.getId()); + assertTrue(authorizable instanceof User); + User u = (User) authorizable; + + // disable user + u.disable("disabled locally"); + root.commit(); + + // sync + syncCtx.setForceUserSync(true); + result = syncCtx.sync(user.getId()); + assertEquals(SyncResult.Status.UPDATE, result.getStatus()); + authorizable = userManager.getAuthorizable(user.getId()); + assertNotNull(authorizable); + assertTrue(authorizable instanceof User); + assertTrue(((User)authorizable).isDisabled()); + } + + @Test public void testSyncGroupById() throws Exception { ExternalIdentity externalId = idp.listGroups().next();