You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-issues@jackrabbit.apache.org by "angela (JIRA)" <ji...@apache.org> on 2016/02/11 16:50:18 UTC
[jira] [Commented] (OAK-3302) ExternalLoginModule:193 can never be
reached
[ https://issues.apache.org/jira/browse/OAK-3302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15142916#comment-15142916 ]
angela commented on OAK-3302:
-----------------------------
I had a quick look at the code base and don't think we have a bug here. {{SyncHandler.findIdentity}} explicitly allows for {{null}} return values and so does {{SyncedIdentity.getExternalIdRef}}. The defaults you are referring to are just one possible implementation and the {{ExternalLoginModule}} must not rely on a given implementation. Instead it should work properly with any possible {{SyncHandler}} and thus should respect the API contract and behave sufficiently defensive.
> ExternalLoginModule:193 can never be reached
> --------------------------------------------
>
> Key: OAK-3302
> URL: https://issues.apache.org/jira/browse/OAK-3302
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: auth-external
> Affects Versions: 1.2.2
> Environment: AEM 6.1
> Reporter: Thorsten Biegner
> Priority: Minor
>
> Starting at line 193 in Version 1.2.2 which shipped with AEM 6.1 this code can never be reached.
> https://github.com/apache/jackrabbit-oak/blob/jackrabbit-oak-1.2.2/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java#L189
> 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) {
> Line 193 ExternalIdentityRef externalIdRef = sId.getExternalIdRef();
> if (externalIdRef == null) {
> Because when no ExternalReference is present sId will be null.
> See https://github.com/apache/jackrabbit-oak/blob/jackrabbit-oak-1.2.2/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncHandler.java#L187
> Instead of being null it should return a SyncedIdentity with the ExternalIdRef set to null.
> As far as I can see the same bug still exists in the current trunk see
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java#L193
> and
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContext.java#L120
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)