Skip to content

Commit 6ff5930

Browse files
strehlestrehle
authored
fix passcode prompt for json responses (#3323)
See issue #3321
1 parent 0b59fbe commit 6ff5930

File tree

2 files changed

+65
-28
lines changed

2 files changed

+65
-28
lines changed

‎server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@
7171
import java.security.Principal;
7272
import java.sql.Timestamp;
7373
import java.text.SimpleDateFormat;
74-
import java.util.ArrayList;
7574
import java.util.Arrays;
7675
import java.util.Comparator;
7776
import java.util.Date;
@@ -85,7 +84,6 @@
8584
import java.util.Properties;
8685
import java.util.regex.Matcher;
8786
import java.util.regex.Pattern;
88-
import java.util.stream.Stream;
8987

9088
import static java.nio.charset.StandardCharsets.UTF_8;
9189
import static java.util.Base64.getDecoder;
@@ -289,8 +287,8 @@ private String login(Model model, Principal principal, List<String> excludedProm
289287
String loginHintParam = extractLoginHintParam(session, request);
290288
UaaLoginHint uaaLoginHint = UaaLoginHint.parseRequestParameter(loginHintParam);
291289

292-
Map<String, SamlIdentityProviderDefinition> samlIdentityProviders;
293-
Map<String, AbstractExternalOAuthIdentityProviderDefinition> oauthIdentityProviders;
290+
Map<String, SamlIdentityProviderDefinition> samlIdentityProviders = null;
291+
Map<String, AbstractExternalOAuthIdentityProviderDefinition> oauthIdentityProviders = null;
294292
Map<String, AbstractIdentityProviderDefinition> allIdentityProviders = Map.of();
295293
Map.Entry<String, AbstractIdentityProviderDefinition> loginHintProvider = null;
296294

@@ -304,32 +302,34 @@ private String login(Model model, Principal principal, List<String> excludedProm
304302
);
305303
if (idp != null) {
306304
loginHintProvider = Map.entry(idp.getOriginKey(), idp.getConfig());
305+
oauthIdentityProviders = new HashMap<>();
306+
oauthIdentityProviders.put(idp.getOriginKey(), (AbstractExternalOAuthIdentityProviderDefinition) idp.getConfig());
307307
}
308308
} catch (EmptyResultDataAccessException ignored) {
309309
// ignore
310310
}
311311
}
312312
if (loginHintProvider != null) {
313-
oauthIdentityProviders = Map.of();
314-
samlIdentityProviders = Map.of();
313+
oauthIdentityProviders = addDefaultOauthMap(oauthIdentityProviders, allowedIdentityProviderKeys, defaultIdentityProviderName);
314+
samlIdentityProviders = addDefaultSamlMap(samlIdentityProviders, allowedIdentityProviderKeys, defaultIdentityProviderName);
315315
} else {
316316
accountChooserNeeded = false;
317317
samlIdentityProviders = getSamlIdentityProviderDefinitions(allowedIdentityProviderKeys);
318318
oauthIdentityProviders = getOauthIdentityProviderDefinitions(allowedIdentityProviderKeys);
319319
allIdentityProviders = concatenateMaps(samlIdentityProviders, oauthIdentityProviders);
320320
}
321-
} else if (!jsonResponse && (accountChooserNeeded || (accountChooserEnabled && !discoveryEnabled && !discoveryPerformed))) {
321+
} else if (!jsonResponse && (accountChooserNeeded || (accountChooserEnabled && !discoveryEnabled && !discoveryPerformed))) {
322322
// when `/login` is requested to return html response (as opposed to json response)
323323
//Account and origin chooser do not need idp information
324-
oauthIdentityProviders = Map.of();
325-
samlIdentityProviders = Map.of();
324+
oauthIdentityProviders = addDefaultOauthMap(oauthIdentityProviders, allowedIdentityProviderKeys, defaultIdentityProviderName);
325+
samlIdentityProviders = addDefaultSamlMap(samlIdentityProviders, allowedIdentityProviderKeys, defaultIdentityProviderName);
326326
} else {
327327
samlIdentityProviders = getSamlIdentityProviderDefinitions(allowedIdentityProviderKeys);
328328

329329
if (jsonResponse) {
330330
/* the OAuth IdPs and all IdPs are used for determining the redirect; if jsonResponse is true, the
331331
* redirect is ignored anyway */
332-
oauthIdentityProviders = Map.of();
332+
oauthIdentityProviders = addDefaultOauthMap(oauthIdentityProviders, allowedIdentityProviderKeys, defaultIdentityProviderName);
333333
} else {
334334
oauthIdentityProviders = getOauthIdentityProviderDefinitions(allowedIdentityProviderKeys);
335335
}
@@ -652,6 +652,36 @@ private Map<String, SamlIdentityProviderDefinition> getSamlIdentityProviderDefin
652652
return filteredIdps.stream().collect(new MapCollector<>(SamlIdentityProviderDefinition::getIdpEntityAlias, idp -> idp));
653653
}
654654

655+
private Map<String, SamlIdentityProviderDefinition> addDefaultSamlMap(Map<String, SamlIdentityProviderDefinition> list, List<String> allowedIdps, String defaultIdp) {
656+
Map<String, SamlIdentityProviderDefinition> defaultList = list == null ? new HashMap<>() : list;
657+
IdentityProvider samlIdP = getIdentityProviderByOrigin(allowedIdps, defaultIdp);
658+
if (samlIdP != null && samlIdP.getConfig() instanceof SamlIdentityProviderDefinition samlDefinition) {
659+
defaultList.putIfAbsent(samlDefinition.getIdpEntityAlias(), samlDefinition);
660+
}
661+
return defaultList;
662+
}
663+
664+
private Map<String, AbstractExternalOAuthIdentityProviderDefinition> addDefaultOauthMap(Map<String, AbstractExternalOAuthIdentityProviderDefinition> list, List<String> allowedIdps, String defaultIdp) {
665+
Map<String, AbstractExternalOAuthIdentityProviderDefinition> defaultList = list == null ? new HashMap<>() : list;
666+
IdentityProvider oauthIdP = getIdentityProviderByOrigin(allowedIdps, defaultIdp);
667+
if (oauthIdP != null && oauthIdP.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition oDefinition) {
668+
defaultList.putIfAbsent(oauthIdP.getOriginKey(), oDefinition);
669+
}
670+
return defaultList;
671+
}
672+
673+
private IdentityProvider getIdentityProviderByOrigin(List<String> allowedIdps, String originKey) {
674+
IdentityProvider identityProvider = null;
675+
try {
676+
if (originKey != null && (allowedIdps == null || allowedIdps.contains(originKey))) {
677+
identityProvider = providerProvisioning.retrieveByOrigin(originKey, IdentityZoneHolder.get().getId());
678+
}
679+
} catch (EmptyResultDataAccessException ignored) {
680+
// ignore
681+
}
682+
return identityProvider;
683+
}
684+
655685
protected Map<String, AbstractExternalOAuthIdentityProviderDefinition> getOauthIdentityProviderDefinitions(List<String> allowedIdps) {
656686
List<IdentityProvider> identityProviders = externalOAuthProviderConfigurator.retrieveActiveByTypes(
657687
IdentityZoneHolder.get().getId(),
@@ -704,22 +734,28 @@ private void populatePrompts(
704734
Map<String, AbstractExternalOAuthIdentityProviderDefinition> oauthIdentityProviders,
705735
boolean returnLoginPrompts
706736
) {
737+
boolean noIdpsPresent = true;
707738
for (SamlIdentityProviderDefinition idp : samlIdentityProviders.values()) {
708739
if (idp.isShowSamlLink()) {
709740
model.addAttribute(SHOW_LOGIN_LINKS, true);
741+
noIdpsPresent = false;
710742
break;
711743
}
712744
}
713745
for (AbstractExternalOAuthIdentityProviderDefinition oauthIdp : oauthIdentityProviders.values()) {
714746
if (oauthIdp.isShowLinkText()) {
715747
model.addAttribute(SHOW_LOGIN_LINKS, true);
748+
noIdpsPresent = false;
716749
break;
717750
}
718751
}
719752

720753
//make the list writeable
721754
final List<String> excludedPrompts = new LinkedList<>(exclude);
722755

756+
if (noIdpsPresent) {
757+
excludedPrompts.add(PASSCODE);
758+
}
723759
if (!returnLoginPrompts) {
724760
excludedPrompts.add(USERNAME_PARAMETER);
725761
excludedPrompts.add("password");

‎server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -580,10 +580,10 @@ void promptLogic() throws Exception {
580580
assertThat(extendedModelMap).as("prompts attribute should be present").containsKey("prompts");
581581
assertThat(extendedModelMap.get("prompts")).as("prompts should be a Map for JSON content").isInstanceOf(Map.class);
582582
mapPrompts = (Map<String, Object>) extendedModelMap.get("prompts");
583-
assertThat(mapPrompts).as("there should be three prompts for html").hasSize(3)
583+
assertThat(mapPrompts).as("there should be three prompts for html").hasSize(2)
584584
.containsKey("username")
585585
.containsKey("password")
586-
.containsKey("passcode");
586+
.doesNotContainKey("passcode");
587587

588588
//add a SAML IDP
589589
extendedModelMap.clear();
@@ -832,7 +832,7 @@ void oauth_provider_links_shown() throws Exception {
832832
}
833833

834834
@Test
835-
void passcode_prompt_present_whenThereIsAtleastOneActiveOauthProvider() throws Exception {
835+
void no_passcode_prompt_present_whenThereIsAtleastOneActiveOauthProvider() throws Exception {
836836
LoginInfoEndpoint endpoint = getEndpoint(IdentityZoneHolder.get());
837837

838838
RawExternalOAuthIdentityProviderDefinition definition = new RawExternalOAuthIdentityProviderDefinition()
@@ -847,11 +847,11 @@ void passcode_prompt_present_whenThereIsAtleastOneActiveOauthProvider() throws E
847847
endpoint.infoForLoginJson(extendedModelMap, null, new MockHttpServletRequest("GET", "http://someurl"));
848848

849849
Map<String, Object> mapPrompts = (Map<String, Object>) extendedModelMap.get("prompts");
850-
assertThat(mapPrompts).containsKey("passcode");
850+
assertThat(mapPrompts).doesNotContainKey("passcode");
851851
}
852852

853853
@Test
854-
void passcode_prompt_present_whenThereIsAtleastOneActiveOauthProvider_stillWorksWithAccountChooser() throws Exception {
854+
void no_passcode_prompt_present_whenThereIsAtleastOneActiveOauthProvider_stillWorksWithAccountChooser() throws Exception {
855855
IdentityZoneHolder.get().getConfig().setAccountChooserEnabled(true);
856856
LoginInfoEndpoint endpoint = getEndpoint(IdentityZoneHolder.get());
857857

@@ -867,11 +867,11 @@ void passcode_prompt_present_whenThereIsAtleastOneActiveOauthProvider_stillWorks
867867
endpoint.infoForLoginJson(extendedModelMap, null, new MockHttpServletRequest("GET", "http://someurl"));
868868

869869
Map<String, Object> mapPrompts = (Map<String, Object>) extendedModelMap.get("prompts");
870-
assertThat(mapPrompts).containsKey("passcode");
870+
assertThat(mapPrompts).doesNotContainKey("passcode");
871871
}
872872

873873
@Test
874-
void passcode_prompt_present_whenThereIsAtleastOneActiveOauthProvider_stillWorksWithDiscovery() throws Exception {
874+
void no_passcode_prompt_present_whenThereIsAtleastOneActiveOauthProvider_stillWorksWithDiscovery() throws Exception {
875875
IdentityZoneHolder.get().getConfig().setIdpDiscoveryEnabled(true);
876876
LoginInfoEndpoint endpoint = getEndpoint(IdentityZoneHolder.get());
877877

@@ -886,7 +886,7 @@ void passcode_prompt_present_whenThereIsAtleastOneActiveOauthProvider_stillWorks
886886
endpoint.infoForLoginJson(extendedModelMap, null, new MockHttpServletRequest("GET", "http://someurl"));
887887

888888
Map<String, Object> mapPrompts = (Map<String, Object>) extendedModelMap.get("prompts");
889-
assertThat(mapPrompts).containsKey("passcode");
889+
assertThat(mapPrompts).doesNotContainKey("passcode");
890890
}
891891

892892
@Test
@@ -1267,13 +1267,13 @@ void getPromptsFromOIDCProvider() {
12671267
assertThat(extendedModelMap).containsKey("prompts");
12681268
assertThat(extendedModelMap.get("prompts")).isInstanceOf(Map.class);
12691269
Map<String, String[]> returnedPrompts = (Map<String, String[]>) extendedModelMap.get("prompts");
1270-
assertThat(returnedPrompts).hasSize(3)
1270+
assertThat(returnedPrompts).hasSize(2)
12711271
.containsKey("username")
12721272
.containsKey("password")
1273-
.containsKey("passcode");
1273+
.doesNotContainKey("passcode");
12741274
assertThat(returnedPrompts.get("username")[1]).isEqualTo("MyEmail");
12751275
assertThat(returnedPrompts.get("password")[1]).isEqualTo("MyPassword");
1276-
assertThat(returnedPrompts.get("passcode")[1]).isEqualTo(passcodePromptText);
1276+
assertThat(returnedPrompts.get("passcode")).isNull();
12771277
}
12781278

12791279
@Test
@@ -1292,7 +1292,7 @@ void getPromptsFromNonOIDCProvider() {
12921292
assertThat(extendedModelMap).containsKey("prompts");
12931293
assertThat(extendedModelMap.get("prompts")).isInstanceOf(Map.class);
12941294
Map<String, String[]> returnedPrompts = (Map<String, String[]>) extendedModelMap.get("prompts");
1295-
assertUsernamePasswordAndPasscodePromptsAreReturned(returnedPrompts);
1295+
assertUsernamePasswordButNoPasscodePromptsAreReturned(returnedPrompts);
12961296
}
12971297

12981298
@Test
@@ -1308,7 +1308,7 @@ void getPromptsFromNonExistentProvider() {
13081308
assertThat(extendedModelMap).containsKey("prompts");
13091309
assertThat(extendedModelMap.get("prompts")).isInstanceOf(Map.class);
13101310
Map<String, String[]> returnedPrompts = (Map<String, String[]>) extendedModelMap.get("prompts");
1311-
assertUsernamePasswordAndPasscodePromptsAreReturned(returnedPrompts);
1311+
assertUsernamePasswordButNoPasscodePromptsAreReturned(returnedPrompts);
13121312
}
13131313

13141314
@Test
@@ -1330,7 +1330,7 @@ void getPromptsFromOIDCProviderWithoutPrompts() {
13301330
assertThat(extendedModelMap).containsKey("prompts");
13311331
assertThat(extendedModelMap.get("prompts")).isInstanceOf(Map.class);
13321332
Map<String, String[]> returnedPrompts = (Map<String, String[]>) extendedModelMap.get("prompts");
1333-
assertUsernamePasswordAndPasscodePromptsAreReturned(returnedPrompts);
1333+
assertUsernamePasswordButNoPasscodePromptsAreReturned(returnedPrompts);
13341334
}
13351335

13361336
@Test
@@ -1733,6 +1733,7 @@ private static void mockOidcProvider(IdentityProviderProvisioning mockIdentityPr
17331733
when(mockProvider.getConfig()).thenReturn(mockOidcConfig);
17341734
when(mockOidcConfig.isShowLinkText()).thenReturn(true);
17351735
when(mockIdentityProviderProvisioning.retrieveActiveByTypes(anyString(), any())).thenReturn(singletonList(mockProvider));
1736+
when(mockIdentityProviderProvisioning.retrieveByOrigin(eq("my-OIDC-idp1"), any())).thenReturn(mockProvider);
17361737
}
17371738

17381739
private static void mockLoginHintProvider(ExternalOAuthProviderConfigurator mockIdentityProviderProvisioning)
@@ -1750,15 +1751,15 @@ private static void mockLoginHintProvider(ExternalOAuthProviderConfigurator mock
17501751
when(mockIdentityProviderProvisioning.retrieveByOrigin(eq("my-OIDC-idp1"), any())).thenReturn(mockProvider);
17511752
}
17521753

1753-
private static void assertUsernamePasswordAndPasscodePromptsAreReturned(
1754+
private static void assertUsernamePasswordButNoPasscodePromptsAreReturned(
17541755
final Map<String, String[]> returnedPrompts
17551756
) {
1756-
assertThat(returnedPrompts).hasSize(3)
1757+
assertThat(returnedPrompts).hasSize(2)
17571758
.containsKey("username")
17581759
.containsKey("password")
1759-
.containsKey("passcode");
1760+
.doesNotContainKey("passcode");
17601761
assertThat(returnedPrompts.get("username")[1]).isEqualTo("Email");
17611762
assertThat(returnedPrompts.get("password")[1]).isEqualTo("Password");
1762-
assertThat(returnedPrompts.get("passcode")[1]).startsWith("Temporary Authentication Code ( Get one at");
1763+
assertThat(returnedPrompts.get("passcode")).isNull();
17631764
}
17641765
}

0 commit comments

Comments
 (0)