From b0d672a29239dfe59f5eae2456e810af16db80ad Mon Sep 17 00:00:00 2001 From: Emmanuel Durin Date: Thu, 18 Jan 2024 10:34:30 +0100 Subject: [PATCH 1/2] Changed uid format --- .../gateway/security/oauth2/OpenIdConnectUserMapper.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java b/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java index 559fd445..71c17d90 100644 --- a/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java +++ b/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java @@ -152,6 +152,8 @@ public class OpenIdConnectUserMapper extends OAuth2UserMapper { try { applyStandardClaims(oidcUser, user); applyNonStandardClaims(oidcUser.getClaims(), user); + user.setUsername((token.getAuthorizedClientRegistrationId() + "_" + user.getUsername()) + .replaceAll("[^a-zA-Z0-9-_]", "_")); } catch (Exception e) { log.error("Error mapping non-standard OIDC claims for authenticated user", e); throw new IllegalStateException(e); @@ -193,7 +195,7 @@ void applyStandardClaims(StandardClaimAccessor standardClaims, GeorchestraUser t String formattedAddress = address == null ? null : address.getFormatted(); apply(target::setId, subjectId); - apply(target::setUsername, preferredUsername, email); + apply(target::setUsername, preferredUsername, subjectId); apply(target::setFirstName, givenName); apply(target::setLastName, familyName); apply(target::setEmail, email); From 28074b1121d0361403fb4f5b57ddd03c3f2ea517 Mon Sep 17 00:00:00 2001 From: Emmanuel Durin Date: Thu, 18 Jan 2024 13:03:21 +0100 Subject: [PATCH 2/2] Split oAuth2ProviderId field to oAuth2Provider and oAuth2Uid --- .../accounts/admin/AbstractAccountsManager.java | 6 +++--- .../admin/CreateAccountUserCustomizer.java | 3 ++- .../accounts/admin/ldap/LdapAccountsManager.java | 9 +++++---- .../RabbitmqAccountCreatedEventSender.java | 15 ++++----------- .../gateway/security/oauth2/OAuth2UserMapper.java | 5 ++--- .../security/oauth2/OpenIdConnectUserMapper.java | 2 +- 6 files changed, 17 insertions(+), 23 deletions(-) diff --git a/gateway/src/main/java/org/georchestra/gateway/accounts/admin/AbstractAccountsManager.java b/gateway/src/main/java/org/georchestra/gateway/accounts/admin/AbstractAccountsManager.java index 5e384f38..e341325b 100644 --- a/gateway/src/main/java/org/georchestra/gateway/accounts/admin/AbstractAccountsManager.java +++ b/gateway/src/main/java/org/georchestra/gateway/accounts/admin/AbstractAccountsManager.java @@ -50,8 +50,8 @@ protected Optional find(GeorchestraUser mappedUser) { } protected Optional findInternal(GeorchestraUser mappedUser) { - if (null != mappedUser.getOAuth2ProviderId()) { - return findByOAuth2ProviderId(mappedUser.getOAuth2ProviderId()); + if ((null != mappedUser.getOAuth2Provider()) && (null != mappedUser.getOAuth2Uid())) { + return findByOAuth2Uid(mappedUser.getOAuth2Provider(), mappedUser.getOAuth2Uid()); } return findByUsername(mappedUser.getUsername()); } @@ -73,7 +73,7 @@ GeorchestraUser createIfMissing(GeorchestraUser mapped) { } } - protected abstract Optional findByOAuth2ProviderId(String oauth2ProviderId); + protected abstract Optional findByOAuth2Uid(String oauth2Provider, String oauth2Uid); protected abstract Optional findByUsername(String username); diff --git a/gateway/src/main/java/org/georchestra/gateway/accounts/admin/CreateAccountUserCustomizer.java b/gateway/src/main/java/org/georchestra/gateway/accounts/admin/CreateAccountUserCustomizer.java index 04d8bd5a..4cd393c9 100644 --- a/gateway/src/main/java/org/georchestra/gateway/accounts/admin/CreateAccountUserCustomizer.java +++ b/gateway/src/main/java/org/georchestra/gateway/accounts/admin/CreateAccountUserCustomizer.java @@ -61,7 +61,8 @@ public class CreateAccountUserCustomizer implements GeorchestraUserCustomizerExt final boolean isOauth2 = auth instanceof OAuth2AuthenticationToken; final boolean isPreAuth = auth instanceof PreAuthenticatedAuthenticationToken; if (isOauth2) { - Objects.requireNonNull(mappedUser.getOAuth2ProviderId(), "GeorchestraUser.oAuth2ProviderId is null"); + Objects.requireNonNull(mappedUser.getOAuth2Provider(), "GeorchestraUser.oAuth2Provider is null"); + Objects.requireNonNull(mappedUser.getOAuth2Uid(), "GeorchestraUser.oAuth2Uid is null"); } if (isPreAuth) { Objects.requireNonNull(mappedUser.getUsername(), "GeorchestraUser.username is null"); diff --git a/gateway/src/main/java/org/georchestra/gateway/accounts/admin/ldap/LdapAccountsManager.java b/gateway/src/main/java/org/georchestra/gateway/accounts/admin/ldap/LdapAccountsManager.java index ae1b3894..35f8393f 100644 --- a/gateway/src/main/java/org/georchestra/gateway/accounts/admin/ldap/LdapAccountsManager.java +++ b/gateway/src/main/java/org/georchestra/gateway/accounts/admin/ldap/LdapAccountsManager.java @@ -73,8 +73,8 @@ public LdapAccountsManager(Consumer eventPublisher, AccountDao a } @Override - protected Optional findByOAuth2ProviderId(@NonNull String oauth2ProviderId) { - return usersApi.findByOAuth2ProviderId(oauth2ProviderId).map(this::ensureRolesPrefixed); + protected Optional findByOAuth2Uid(@NonNull String oAuth2Provider, @NonNull String oAuth2Uid) { + return usersApi.findByOAuth2Uid(oAuth2Provider, oAuth2Uid).map(this::ensureRolesPrefixed); } @Override @@ -145,10 +145,11 @@ private Account mapToAccountBrief(@NonNull GeorchestraUser preAuth) { String phone = ""; String title = ""; String description = ""; - final @javax.annotation.Nullable String oAuth2ProviderId = preAuth.getOAuth2ProviderId(); + final @javax.annotation.Nullable String oAuth2Provider = preAuth.getOAuth2Provider(); + final @javax.annotation.Nullable String oAuth2Uid = preAuth.getOAuth2Uid(); Account newAccount = AccountFactory.createBrief(username, password, firstName, lastName, email, phone, title, - description, oAuth2ProviderId); + description, oAuth2Provider, oAuth2Uid); newAccount.setPending(false); if (StringUtils.isEmpty(org) && !StringUtils.isBlank(defaultOrganization)) { newAccount.setOrg(defaultOrganization); diff --git a/gateway/src/main/java/org/georchestra/gateway/accounts/events/rabbitmq/RabbitmqAccountCreatedEventSender.java b/gateway/src/main/java/org/georchestra/gateway/accounts/events/rabbitmq/RabbitmqAccountCreatedEventSender.java index 6f853ce6..4ac349ca 100644 --- a/gateway/src/main/java/org/georchestra/gateway/accounts/events/rabbitmq/RabbitmqAccountCreatedEventSender.java +++ b/gateway/src/main/java/org/georchestra/gateway/accounts/events/rabbitmq/RabbitmqAccountCreatedEventSender.java @@ -44,21 +44,14 @@ public RabbitmqAccountCreatedEventSender(AmqpTemplate eventTemplate) { @EventListener(AccountCreated.class) public void on(AccountCreated event) { GeorchestraUser user = event.getUser(); - final String oAuth2ProviderId = user.getOAuth2ProviderId(); - if (null != oAuth2ProviderId) { + final String oAuth2Provider = user.getOAuth2Provider(); + if (null != oAuth2Provider) { String fullName = user.getFirstName() + " " + user.getLastName(); String localUid = user.getUsername(); String email = user.getEmail(); String organization = user.getOrganization(); - String[] providerFields = oAuth2ProviderId.split(";"); - String providerName = ""; - String providerUid = ""; - if(providerFields.length == 2) - { - providerName = providerFields[0]; - providerUid = providerFields[1]; - } - sendNewOAuthAccountMessage(fullName, localUid, email, organization, providerName, providerUid); + String oAuth2Uid = user.getOAuth2Uid(); + sendNewOAuthAccountMessage(fullName, localUid, email, organization, oAuth2Provider, oAuth2Uid); } } diff --git a/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OAuth2UserMapper.java b/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OAuth2UserMapper.java index f5ed3f00..eb5dafc0 100644 --- a/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OAuth2UserMapper.java +++ b/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OAuth2UserMapper.java @@ -78,9 +78,8 @@ protected Optional map(OAuth2AuthenticationToken token) { OAuth2User oAuth2User = token.getPrincipal(); GeorchestraUser user = new GeorchestraUser(); - final String oAuth2ProviderId = String.format("%s;%s", token.getAuthorizedClientRegistrationId(), - token.getName()); - user.setOAuth2ProviderId(oAuth2ProviderId); + user.setOAuth2Provider(token.getAuthorizedClientRegistrationId()); + user.setOAuth2Uid(token.getName()); Map attributes = oAuth2User.getAttributes(); diff --git a/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java b/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java index 71c17d90..b5f906d1 100644 --- a/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java +++ b/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java @@ -153,7 +153,7 @@ public class OpenIdConnectUserMapper extends OAuth2UserMapper { applyStandardClaims(oidcUser, user); applyNonStandardClaims(oidcUser.getClaims(), user); user.setUsername((token.getAuthorizedClientRegistrationId() + "_" + user.getUsername()) - .replaceAll("[^a-zA-Z0-9-_]", "_")); + .replaceAll("[^a-zA-Z0-9-_]", "_").toLowerCase()); } catch (Exception e) { log.error("Error mapping non-standard OIDC claims for authenticated user", e); throw new IllegalStateException(e);