From 5c8fc3d8e9ab5370240fa9820c0c75b1130493e6 Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 27 Jul 2024 10:10:03 +0200 Subject: [PATCH 1/7] feat: Return a sorted IdP list Sort the IdP list returned to thymeleaf Sort always by login link text Do not sort the JSON list but only HTML page --- .../cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java index 732c82a6926..199da352352 100755 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.Date; import java.util.HashMap; import java.util.LinkedHashMap; @@ -450,9 +451,10 @@ private void updateLoginPageModel( boolean fieldUsernameShow, boolean linkCreateAccountShow ) { + Comparator sortingByLinkText = Comparator.comparing(SamlIdentityProviderDefinition::getLinkText); model.addAttribute(LINK_CREATE_ACCOUNT_SHOW, linkCreateAccountShow); model.addAttribute(FIELD_USERNAME_SHOW, fieldUsernameShow); - model.addAttribute(IDP_DEFINITIONS, samlIdentityProviders.values()); + model.addAttribute(IDP_DEFINITIONS, samlIdentityProviders.values().stream().sorted(sortingByLinkText).toList()); Map oauthLinks = new HashMap<>(); ofNullable(oauthIdentityProviders).orElse(emptyMap()).entrySet().stream() .filter(e -> e.getValue().isShowLinkText()) @@ -465,7 +467,7 @@ private void updateLoginPageModel( e.getValue().getLinkText() ) ); - model.addAttribute(OAUTH_LINKS, oauthLinks); + model.addAttribute(OAUTH_LINKS, oauthLinks.entrySet().stream().sorted(Map.Entry.comparingByValue()).toList()); model.addAttribute("clientName", clientName); } From a5a0e9a16506c3e717fd8eeac90a45a326165565 Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 27 Jul 2024 11:30:43 +0200 Subject: [PATCH 2/7] test fix --- .../identity/uaa/login/LoginInfoEndpoint.java | 2 +- .../identity/uaa/login/LoginInfoEndpointTests.java | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java index 199da352352..2e219bb3333 100755 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java @@ -457,7 +457,7 @@ private void updateLoginPageModel( model.addAttribute(IDP_DEFINITIONS, samlIdentityProviders.values().stream().sorted(sortingByLinkText).toList()); Map oauthLinks = new HashMap<>(); ofNullable(oauthIdentityProviders).orElse(emptyMap()).entrySet().stream() - .filter(e -> e.getValue().isShowLinkText()) + .filter(e -> e.getValue().isShowLinkText() && e.getValue() != null && e.getKey() != null) .forEach(e -> oauthLinks.put( externalOAuthProviderConfigurator.getIdpAuthenticationUrl( diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java index 6022834edf3..dd2b623d4fe 100755 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java @@ -827,9 +827,9 @@ void allowedIdpsforClientOIDCProvider() throws Exception { when(clientDetailsService.loadClientByClientId("client-id", "uaa")).thenReturn(clientDetails); List clientAllowedIdps = new LinkedList<>(); - clientAllowedIdps.add(createOIDCIdentityProvider("my-OIDC-idp1")); - clientAllowedIdps.add(createOIDCIdentityProvider("my-OIDC-idp2")); clientAllowedIdps.add(createOIDCIdentityProvider("my-OIDC-idp3")); + clientAllowedIdps.add(createOIDCIdentityProvider("my-OIDC-idp2")); + clientAllowedIdps.add(createOIDCIdentityProvider("my-OIDC-idp1")); when(mockIdentityProviderProvisioning.retrieveAll(eq(true), anyString())).thenReturn(clientAllowedIdps); @@ -837,8 +837,9 @@ void allowedIdpsforClientOIDCProvider() throws Exception { endpoint.loginForHtml(extendedModelMap, null, request, singletonList(MediaType.TEXT_HTML)); - Map idpDefinitions = (Map) extendedModelMap.asMap().get("oauthLinks"); + Collection> idpDefinitions = (Collection>) extendedModelMap.asMap().get("oauthLinks"); assertEquals(2, idpDefinitions.size()); + assertEquals("my-OIDC-idp1", ((Map.Entry) idpDefinitions.iterator().next()).getValue()); } @Test @@ -1243,7 +1244,7 @@ public void testInvalidLoginHintLoginPageReturnsList() throws Exception { endpoint.loginForHtml(extendedModelMap, null, mockHttpServletRequest, Collections.singletonList(MediaType.TEXT_HTML)); - assertFalse(((Map)extendedModelMap.get("oauthLinks")).isEmpty()); + assertFalse(((Collection>)extendedModelMap.get("oauthLinks")).isEmpty()); } @Test @@ -1701,7 +1702,7 @@ void allowedProvidersLoginHintDoesKeepExternalProviders() throws Exception { assertEquals("{\"origin\":\"uaa\"}", extendedModelMap.get("login_hint")); assertEquals("login", redirect); - Map oauthLinks = (Map) extendedModelMap.get("oauthLinks"); + Collection> oauthLinks = (Collection>) extendedModelMap.get("oauthLinks"); assertEquals(1, oauthLinks.size()); } @@ -1812,6 +1813,7 @@ private static IdentityProvider createOIDCIdentityProvider(String originKey) thr OIDCIdentityProviderDefinition definition = new OIDCIdentityProviderDefinition(); definition.setAuthUrl(new URL("https://" + originKey + ".com")); definition.setRelyingPartySecret("client-secret"); + definition.setLinkText(originKey); oidcIdentityProvider.setConfig(definition); return oidcIdentityProvider; From 9fd6ecf9aae553a2c47a948dbb12d8129f8fb7a3 Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 27 Jul 2024 11:34:28 +0200 Subject: [PATCH 3/7] Sonar smell --- .../org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java index 2e219bb3333..ac047872060 100755 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java @@ -451,7 +451,7 @@ private void updateLoginPageModel( boolean fieldUsernameShow, boolean linkCreateAccountShow ) { - Comparator sortingByLinkText = Comparator.comparing(SamlIdentityProviderDefinition::getLinkText); + Comparator sortingByLinkText = Comparator.comparing(SamlIdentityProviderDefinition::getLinkText); model.addAttribute(LINK_CREATE_ACCOUNT_SHOW, linkCreateAccountShow); model.addAttribute(FIELD_USERNAME_SHOW, fieldUsernameShow); model.addAttribute(IDP_DEFINITIONS, samlIdentityProviders.values().stream().sorted(sortingByLinkText).toList()); From a86187f76e5e955490e3a7003e0d8ba43c5e4c96 Mon Sep 17 00:00:00 2001 From: d036670 Date: Sat, 27 Jul 2024 11:36:43 +0200 Subject: [PATCH 4/7] comment --- .../cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java index dd2b623d4fe..986cb508593 100755 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java @@ -839,6 +839,7 @@ void allowedIdpsforClientOIDCProvider() throws Exception { Collection> idpDefinitions = (Collection>) extendedModelMap.asMap().get("oauthLinks"); assertEquals(2, idpDefinitions.size()); + // Expect this always on top of list because of sorting assertEquals("my-OIDC-idp1", ((Map.Entry) idpDefinitions.iterator().next()).getValue()); } From 3686cdf14c8aef0b6aec6286de1ad323c2c4e691 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 30 Jul 2024 22:04:32 +0200 Subject: [PATCH 5/7] use compareToIgnoreCase and ignore case --- .../cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java index ac047872060..6dde1fcbfb9 100755 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java @@ -451,7 +451,7 @@ private void updateLoginPageModel( boolean fieldUsernameShow, boolean linkCreateAccountShow ) { - Comparator sortingByLinkText = Comparator.comparing(SamlIdentityProviderDefinition::getLinkText); + Comparator sortingByLinkText = Comparator.comparing(SamlIdentityProviderDefinition::getLinkText, String.CASE_INSENSITIVE_ORDER); model.addAttribute(LINK_CREATE_ACCOUNT_SHOW, linkCreateAccountShow); model.addAttribute(FIELD_USERNAME_SHOW, fieldUsernameShow); model.addAttribute(IDP_DEFINITIONS, samlIdentityProviders.values().stream().sorted(sortingByLinkText).toList()); @@ -467,7 +467,7 @@ private void updateLoginPageModel( e.getValue().getLinkText() ) ); - model.addAttribute(OAUTH_LINKS, oauthLinks.entrySet().stream().sorted(Map.Entry.comparingByValue()).toList()); + model.addAttribute(OAUTH_LINKS, oauthLinks.entrySet().stream().sorted(Map.Entry.comparingByValue(String::compareToIgnoreCase)).toList()); model.addAttribute("clientName", clientName); } From 4fef2f49d2cf71a91190f4f6b26144ea8f8b661a Mon Sep 17 00:00:00 2001 From: Markus Strehle <11627201+strehle@users.noreply.github.com> Date: Fri, 16 Aug 2024 08:22:24 +0200 Subject: [PATCH 6/7] LoginInfoEndpoint.java aktualisieren Done --- .../org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java index 6dde1fcbfb9..9f4f697ca99 100755 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java @@ -457,7 +457,7 @@ private void updateLoginPageModel( model.addAttribute(IDP_DEFINITIONS, samlIdentityProviders.values().stream().sorted(sortingByLinkText).toList()); Map oauthLinks = new HashMap<>(); ofNullable(oauthIdentityProviders).orElse(emptyMap()).entrySet().stream() - .filter(e -> e.getValue().isShowLinkText() && e.getValue() != null && e.getKey() != null) + .filter(e.getValue() != null && e -> e.getValue().isShowLinkText() && e.getValue() != null && e.getKey() != null) .forEach(e -> oauthLinks.put( externalOAuthProviderConfigurator.getIdpAuthenticationUrl( From 22de6a6cf5964734b69550dd990522955ec6a829 Mon Sep 17 00:00:00 2001 From: Markus Strehle <11627201+strehle@users.noreply.github.com> Date: Fri, 16 Aug 2024 08:25:00 +0200 Subject: [PATCH 7/7] LoginInfoEndpoint.java aktualisieren --- .../org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java index 9f4f697ca99..b0c617589ad 100755 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java @@ -457,7 +457,7 @@ private void updateLoginPageModel( model.addAttribute(IDP_DEFINITIONS, samlIdentityProviders.values().stream().sorted(sortingByLinkText).toList()); Map oauthLinks = new HashMap<>(); ofNullable(oauthIdentityProviders).orElse(emptyMap()).entrySet().stream() - .filter(e.getValue() != null && e -> e.getValue().isShowLinkText() && e.getValue() != null && e.getKey() != null) + .filter(e -> e.getValue() != null && e.getValue().isShowLinkText() && e.getKey() != null) .forEach(e -> oauthLinks.put( externalOAuthProviderConfigurator.getIdpAuthenticationUrl(