anchela commented on code in PR #2581:
URL: https://github.com/apache/jackrabbit-oak/pull/2581#discussion_r2431458967
##########
oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/token/TokenConstants.java:
##########
@@ -34,11 +35,15 @@ public interface TokenConstants {
String TOKENS_NT_NAME = NodeTypeConstants.NT_REP_UNSTRUCTURED;
String TOKEN_NT_NAME = "rep:Token";
+
+ String EXTERNAL_ID_ATTRIBUTE = ":externalId";
Review Comment:
this constant is now duplicated here and in the externalidentity constants.
i wonder if it would make sense to place it in a single common place, either in
security-spi or in oak-jackrabbit-api.
we could also add the 'attribute-login-name' there and then reuse it in
AbstractLoginModule and here in TokenConstants.Reserved-Attributes.
wdyt?
##########
oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/token/TokenConstants.java:
##########
@@ -34,11 +35,15 @@ public interface TokenConstants {
String TOKENS_NT_NAME = NodeTypeConstants.NT_REP_UNSTRUCTURED;
String TOKEN_NT_NAME = "rep:Token";
+
+ String EXTERNAL_ID_ATTRIBUTE = ":externalId";
Set<String> RESERVED_ATTRIBUTES = Set.of(
TOKEN_ATTRIBUTE,
TOKEN_ATTRIBUTE_EXPIRY,
- TOKEN_ATTRIBUTE_KEY);
+ TOKEN_ATTRIBUTE_KEY,
+ AbstractLoginModule.SHARED_KEY_LOGIN_NAME,
Review Comment:
see above for a suggestion that would make both new constants generic and
not have abstractloginmodule, external-identity-constants and tokenconstants
depend on each other or duplicate constants.
##########
oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/ExternalLoginModule.java:
##########
@@ -228,7 +230,22 @@ public boolean login() throws LoginException {
// before into the repository.
UserManager userManager = getUserManager();
SyncedIdentity sId = getSyncedIdentity(userId, userManager);
-
+ if (sId ==null && userManager != null && creds != null) {
+ // Check if the external user was registered with a different
userId, and the same externalId
Review Comment:
imho it would make sense to add a tmp toggle for this change and clean it up
if we see that it doesn't cause any trouble.
if i remember correctly we had to do this with system-properties in the
past. let's check with the rest of the oak-folks if there a better alternative
available now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
