Amoratinos commented on code in PR #2581:
URL: https://github.com/apache/jackrabbit-oak/pull/2581#discussion_r2428541991
##########
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:
Looks a bit odd to me that we're saving an attribute from other class.
Wouldn't be better to specify here the attribute name?
If we do not want to remove the constant from `AbstractLoginModule` it can
be initialized with the TokenConstant value:
```
public static final String SHARED_KEY_LOGIN_NAME =
TokenConstants.SHARED_KEY_LOGIN_NAME;
```
##########
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) {
Review Comment:
Trivial typo
```suggestion
if (sId == null && userManager != null && creds != null) {
```
##########
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
+ Object externalAttribute =
credentialsSupport.getAttributes(creds).get(ExternalIdentityConstants.EXTERNAL_ID_ATTRIBUTE);
+ if (externalAttribute != null ) {
+ @NotNull Iterator<Authorizable> authIterator =
userManager.findAuthorizables(ExternalIdentityConstants.REP_EXTERNAL_ID,
externalAttribute + ";" + idp.getName(), UserManager.SEARCH_TYPE_USER);
+ if (authIterator.hasNext()) {
+ //modify credentials to reflect the login name stored
in oak
+ Authorizable authorizable = authIterator.next();
+ sId = getSyncedIdentity(authorizable.getID(),
userManager);
+ Map<String, ?> attributes =
credentialsSupport.getAttributes(creds);
+ HashMap<String, Object> newAttributes = new
HashMap<>(attributes);
+
newAttributes.put(AbstractLoginModule.SHARED_KEY_LOGIN_NAME,
authorizable.getID());
Review Comment:
This class already inherits from `AbstractLoginModule` so it's not needed to
add it while using the constant.
```suggestion
newAttributes.put(SHARED_KEY_LOGIN_NAME,
authorizable.getID());
```
--
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]
