You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2016/06/09 07:44:40 UTC
svn commit: r1747505 -
/jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java
Author: angela
Date: Thu Jun 9 07:44:40 2016
New Revision: 1747505
URL: http://svn.apache.org/viewvc?rev=1747505&view=rev
Log:
OAK-4443 : ExternalLoginModule: refactor to handle all reasons for 'ignore' together
Modified:
jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java
Modified: jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java?rev=1747505&r1=1747504&r2=1747505&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java (original)
+++ jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java Thu Jun 9 07:44:40 2016
@@ -24,6 +24,7 @@ import java.util.Set;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import javax.jcr.Credentials;
+import javax.jcr.RepositoryException;
import javax.security.auth.Subject;
import javax.security.auth.callback.CallbackHandler;
import javax.security.auth.login.LoginException;
@@ -199,31 +200,20 @@ public class ExternalLoginModule extends
// remember identification for log-output
Object logId = (userId != null) ? userId : credentials;
try {
- SyncedIdentity sId = null;
- UserManager userMgr = getUserManager();
- if (userId != null && userMgr != null) {
- sId = syncHandler.findIdentity(userMgr, userId);
- // if there exists an authorizable with the given userid but is
- // not an external one or if it belongs to another IDP, we just ignore it.
- if (sId != null) {
- ExternalIdentityRef externalIdRef = sId.getExternalIdRef();
- if (externalIdRef == null) {
- log.debug("ignoring local user: {}", sId.getId());
- return false;
- } else if (!idp.getName().equals(externalIdRef.getProviderName())) {
- if (log.isDebugEnabled()) {
- log.debug("ignoring foreign identity: {} (idp={})", externalIdRef.getString(), idp.getName());
- }
- return false;
- }
- }
+ // check if there exists a user with the given ID that has been synchronized
+ // before into the repository.
+ SyncedIdentity sId = getSyncedIdentity(userId);
+
+ // if there exists an authorizable with the given userid (syncedIdentity != null),
+ // ignore it if any of the following conditions is met:
+ // - identity is local (i.e. not an external identity)
+ // - identity belongs to another IDP
+ // - identity is valid but we have a preAuthLogin and the user doesn't need an updating sync (OAK-3508)
+ if (ignore(sId, preAuthLogin)) {
+ return false;
}
if (preAuthLogin != null) {
- if (sId != null && !syncHandler.requiresSync(sId)) {
- log.debug("pre-authenticated external user {} does not require syncing.", sId);
- return false;
- }
externalUser = idp.getUser(preAuthLogin.getUserId());
} else {
externalUser = idp.authenticate(credentials);
@@ -244,9 +234,7 @@ public class ExternalLoginModule extends
return true;
} else {
- if (log.isDebugEnabled()) {
- log.debug("IDP {} returned null for {}", idp.getName(), logId);
- }
+ debug("IDP {} returned null for {}", idp.getName(), logId.toString());
if (sId != null) {
// invalidate the user if it exists as synced variant
@@ -313,6 +301,35 @@ public class ExternalLoginModule extends
}
}
+ @CheckForNull
+ private SyncedIdentity getSyncedIdentity(@CheckForNull String userId) throws RepositoryException {
+ UserManager userMgr = getUserManager();
+ if (userId != null && userMgr != null) {
+ return syncHandler.findIdentity(userMgr, userId);
+ } else {
+ return null;
+ }
+ }
+
+ private boolean ignore(@CheckForNull SyncedIdentity syncedIdentity, @CheckForNull PreAuthenticatedLogin preAuthLogin) {
+ if (syncedIdentity != null) {
+ ExternalIdentityRef externalIdRef = syncedIdentity.getExternalIdRef();
+ if (externalIdRef == null) {
+ debug("ignoring local user: {}", syncedIdentity.getId());
+ return true;
+ } else if (!idp.getName().equals(externalIdRef.getProviderName())) {
+ debug("ignoring foreign identity: {} (idp={})", externalIdRef.getString(), idp.getName());
+ return true;
+ }
+
+ if (preAuthLogin != null && !syncHandler.requiresSync(syncedIdentity)) {
+ debug("pre-authenticated external user {} does not require syncing.", syncedIdentity.toString());
+ return true;
+ }
+ }
+ return false;
+ }
+
/**
* Initiates synchronization of the external user.
* @param user the external user
@@ -338,9 +355,7 @@ public class ExternalLoginModule extends
timer.mark("sync");
root.commit();
timer.mark("commit");
- if (log.isDebugEnabled()) {
- log.debug("syncUser({}) {}", user.getId(), timer.getString());
- }
+ debug("syncUser({}) {}", user.getId(), timer.getString());
return;
} catch (CommitFailedException e) {
log.warn("User synchronization failed during commit: {}. (attempt {}/{})", e.toString(), numAttempt, MAX_SYNC_ATTEMPTS);
@@ -375,9 +390,7 @@ public class ExternalLoginModule extends
timer.mark("sync");
root.commit();
timer.mark("commit");
- if (log.isDebugEnabled()) {
- log.debug("validateUser({}) {}", id, timer.getString());
- }
+ debug("validateUser({}) {}", id, timer.getString());
} catch (CommitFailedException e) {
throw new SyncException("User synchronization failed during commit.", e);
} finally {
@@ -408,6 +421,12 @@ public class ExternalLoginModule extends
return new AuthInfoImpl(userId, attributes, principals);
}
+ private static void debug(@Nonnull String msg, String... args) {
+ if (log.isDebugEnabled()) {
+ log.debug(msg, args);
+ }
+ }
+
//------------------------------------------------< AbstractLoginModule >---
@Override