Skip to content

Fix #253 - one user from many external #254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Gaetanbrl wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix #253 - one user from many external #254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82817e4
fix ensure org exist and mapped user
Feb 24, 2026
a5262bc
improve test
Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions gateway/src/main/java/org/georchestra/gateway/accounts/admin/ldap/LdapAccountsManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,8 @@ private void ensureRolesExist(GeorchestraUser mapped, Account newAccount) {
if (newAccount.getOrg() != null) {
List<Role> r = roleDao.findAllForOrg(orgsDao.findByCommonName(newAccount.getOrg()));
if (!r.isEmpty())
roleDao.addUsersInRoles(r.stream().map(Role::getName).collect(Collectors.toList()), List.of(newAccount));
roleDao.addUsersInRoles(r.stream().map(Role::getName).collect(Collectors.toList()),
List.of(newAccount));
}
for (String role : mapped.getRoles()) {
role = role.replaceFirst("^ROLE_", "");
Expand Down Expand Up @@ -297,7 +298,18 @@ void ensureRoleExists(String role) throws DataServiceException {
* @return a newly created {@link Account} object with mapped attributes
*/
private Account mapToAccountBrief(@NonNull GeorchestraUser preAuth) {
String username = preAuth.getUsername();
return mapToAccountBrief(preAuth, preAuth.getUsername());
}

/**
* Maps a {@link GeorchestraUser} to an {@link Account} using the provided uid.
*
* @param preAuth the source user
* @param uid LDAP uid to use for the mapped account
* @return a newly created {@link Account} object with mapped attributes
*/
private Account mapToAccountBrief(@NonNull GeorchestraUser preAuth, @Nullable String uid) {
String username = uid;
String email = preAuth.getEmail();
String firstName = preAuth.getFirstName();
String lastName = preAuth.getLastName();
Expand Down Expand Up @@ -345,7 +357,11 @@ protected void unlinkUserOrg(@NonNull GeorchestraUser user) {
*/
@Override
protected void ensureOrgExists(@NonNull GeorchestraUser mappedUser) {
Account newAccount = mapToAccountBrief(mappedUser);
// Reuse the persisted LDAP uid when the user is already known (e.g. lookup by
// email).
String effectiveUid = findInternal(mappedUser).map(GeorchestraUser::getUsername).filter(StringUtils::isNotBlank)
.orElse(mappedUser.getUsername());
Account newAccount = mapToAccountBrief(mappedUser, effectiveUid);
ensureOrgExists(newAccount);
}

Expand Down
108 changes: 108 additions & 0 deletions ...ay/src/test/java/org/georchestra/gateway/accounts/admin/ldap/LdapAccountsManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
package org.georchestra.gateway.accounts.admin.ldap;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import org.georchestra.ds.DataServiceException;
import org.georchestra.ds.orgs.Org;
import org.georchestra.ds.orgs.OrgsDao;
import org.georchestra.ds.roles.RoleDao;
import org.georchestra.ds.users.Account;
import org.georchestra.ds.users.AccountDao;
import org.georchestra.gateway.security.GeorchestraGatewaySecurityConfigProperties;
import org.georchestra.gateway.security.ldap.extended.DemultiplexingUsersApi;
import org.georchestra.gateway.security.ldap.extended.ExtendedGeorchestraUser;
import org.georchestra.gateway.security.oauth2.OpenIdConnectCustomConfig;
import org.georchestra.security.model.GeorchestraUser;
import org.junit.jupiter.api.Test;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.ldap.NameNotFoundException;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class LdapAccountsManagerTest {
Expand Down Expand Up @@ -82,4 +92,102 @@ void verifySingleOrgMembership_failsWhenUserInMultipleOrgs() {

assertThrows(IllegalStateException.class, () -> toTest.verifySingleOrgMembership(account, "ORG_A"));
}

@Test
void ensureOrgExists_usesExistingLdapUidWhenOAuthUidDiffers() {
OrgsDao orgsDao = mock(OrgsDao.class);
AccountDao accountDao = mock(AccountDao.class);
RoleDao roleDao = mock(RoleDao.class);
DemultiplexingUsersApi usersApi = mock(DemultiplexingUsersApi.class);

GeorchestraGatewaySecurityConfigProperties securityConfig = new GeorchestraGatewaySecurityConfigProperties();
securityConfig.setModeratedSignup(false);
securityConfig.setDefaultOrganization("");

OpenIdConnectCustomConfig providersConfig = new OpenIdConnectCustomConfig();
OpenIdConnectCustomConfig proconnectConfig = new OpenIdConnectCustomConfig();
proconnectConfig.setSearchEmail(true);
providersConfig.getProvider().put("proconnect", proconnectConfig);

Org existingOrg = new Org();
existingOrg.setId("ORG_B");
existingOrg.setMembers(new ArrayList<>());
when(orgsDao.findByOrgUniqueId("12345678901234")).thenReturn(existingOrg);
when(orgsDao.findByCommonName("ORG_B")).thenReturn(existingOrg);
when(orgsDao.findAll()).thenReturn(List.of(existingOrg));
when(orgsDao.findByUser(any(Account.class))).thenReturn(existingOrg);

GeorchestraUser existingUser = new GeorchestraUser();
existingUser.setUsername("fake_uid");
existingUser.setRoles(new ArrayList<>());
ExtendedGeorchestraUser existingLdapUser = new ExtendedGeorchestraUser(existingUser);
when(usersApi.findByEmail("user@example.org", false)).thenReturn(Optional.of(existingLdapUser));

LdapAccountsManager toTest = new LdapAccountsManager(mock(ApplicationEventPublisher.class), accountDao, roleDao,
orgsDao, usersApi, securityConfig, providersConfig);

GeorchestraUser mappedUser = new GeorchestraUser();
mappedUser.setUsername("proconnect_uid");
mappedUser.setEmail("user@example.org");
mappedUser.setOrganization("ORG_B");
mappedUser.setOAuth2Provider("proconnect");
mappedUser.setOAuth2Uid("0226c002-a462-4127-887d-ae2b03633bd9");
mappedUser.setOAuth2OrgId("12345678901234");
mappedUser.setRoles(new ArrayList<>());

toTest.ensureOrgExists(mappedUser);

assertTrue(existingOrg.getMembers().contains("fake_uid"));
assertFalse(existingOrg.getMembers().contains("proconnect_uid"));
verify(orgsDao).update(existingOrg);
}

@Test
void ensureOrgExists_usesExistingLdapUidWhenOAuthUidDiffers_withFakeSearchByEmail() {
OrgsDao orgsDao = mock(OrgsDao.class);
AccountDao accountDao = mock(AccountDao.class);
RoleDao roleDao = mock(RoleDao.class);
DemultiplexingUsersApi usersApi = mock(DemultiplexingUsersApi.class);

GeorchestraGatewaySecurityConfigProperties securityConfig = new GeorchestraGatewaySecurityConfigProperties();
securityConfig.setModeratedSignup(false);
securityConfig.setDefaultOrganization("");

OpenIdConnectCustomConfig providersConfig = new OpenIdConnectCustomConfig();
OpenIdConnectCustomConfig fakeConfig = new OpenIdConnectCustomConfig();
fakeConfig.setSearchEmail(true);
providersConfig.getProvider().put("fake", fakeConfig);

Org existingOrg = new Org();
existingOrg.setId("ORG_A");
existingOrg.setMembers(new ArrayList<>());
when(orgsDao.findByOrgUniqueId("12345678901234")).thenReturn(existingOrg);
when(orgsDao.findByCommonName("ORG_A")).thenReturn(existingOrg);
when(orgsDao.findAll()).thenReturn(List.of(existingOrg));
when(orgsDao.findByUser(any(Account.class))).thenReturn(existingOrg);

GeorchestraUser existingUser = new GeorchestraUser();
existingUser.setUsername("proconnect_uid");
existingUser.setRoles(new ArrayList<>());
ExtendedGeorchestraUser existingLdapUser = new ExtendedGeorchestraUser(existingUser);
when(usersApi.findByEmail("user@example.org", false)).thenReturn(Optional.of(existingLdapUser));

LdapAccountsManager toTest = new LdapAccountsManager(mock(ApplicationEventPublisher.class), accountDao, roleDao,
orgsDao, usersApi, securityConfig, providersConfig);

GeorchestraUser mappedUser = new GeorchestraUser();
mappedUser.setUsername("fake_uid");
mappedUser.setEmail("user@example.org");
mappedUser.setOrganization("ORG_A");
mappedUser.setOAuth2Provider("fake");
mappedUser.setOAuth2Uid("fake-external-id");
mappedUser.setOAuth2OrgId("12345678901234");
mappedUser.setRoles(new ArrayList<>());

toTest.ensureOrgExists(mappedUser);

assertTrue(existingOrg.getMembers().contains("proconnect_uid"));
assertFalse(existingOrg.getMembers().contains("fake_uid"));
verify(orgsDao).update(existingOrg);
}
}
Loading