Skip to content

Commit

Permalink
Response type should be part of the URL and created dynamically both …
Browse files Browse the repository at this point in the history 
…for redirect and login page

If id_token is in the response, then use that.
[#122654431] https://www.pivotaltracker.com/story/show/122654431
  • Loading branch information
@fhanik
fhanik committed Nov 4, 2016
1 parent 5c451a0 commit 492df57
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 23 deletions.
2 changes: 1 addition & 1 deletion server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java
View file
Expand Up @@ -445,7 +445,7 @@ private String getRedirectUrlForXOAuthIDP(HttpServletRequest request, String ali
String queryAppendDelimiter = authUrlBase.contains("?") ? "&" : "?"; String queryAppendDelimiter = authUrlBase.contains("?") ? "&" : "?";
List<String> query = new ArrayList<>(); List<String> query = new ArrayList<>();
query.add("client_id=" + definition.getRelyingPartyId()); query.add("client_id=" + definition.getRelyingPartyId());
query.add("response_type=code"); query.add("response_type="+URLEncoder.encode(definition.getResponseType(), "UTF-8"));
String requestURL = request.getRequestURL().toString(); String requestURL = request.getRequestURL().toString();
String rootContext = StringUtils.hasText(request.getServletPath()) ? requestURL.substring(0, requestURL.indexOf(request.getServletPath())) : requestURL; String rootContext = StringUtils.hasText(request.getServletPath()) ? requestURL.substring(0, requestURL.indexOf(request.getServletPath())) : requestURL;
query.add("redirect_uri=" + URLEncoder.encode(rootContext + "/login/callback/" + alias, "UTF-8")); query.add("redirect_uri=" + URLEncoder.encode(rootContext + "/login/callback/" + alias, "UTF-8"));
Expand Down
7 changes: 5 additions & 2 deletions ...c/main/java/org/cloudfoundry/identity/uaa/provider/oauth/XOAuthAuthenticationManager.java
View file
Expand Up @@ -287,12 +287,12 @@ private String getResponseType(AbstractXOAuthIdentityProviderDefinition config)
} }
} }


private Map<String,Object> getClaimsFromToken(XOAuthCodeToken codeToken, AbstractXOAuthIdentityProviderDefinition config) { protected Map<String,Object> getClaimsFromToken(XOAuthCodeToken codeToken, AbstractXOAuthIdentityProviderDefinition config) {
String idToken = getTokenFromCode(codeToken, config); String idToken = getTokenFromCode(codeToken, config);
return getClaimsFromToken(idToken, config); return getClaimsFromToken(idToken, config);
} }


private Map<String,Object> getClaimsFromToken(String idToken, AbstractXOAuthIdentityProviderDefinition config) { protected Map<String,Object> getClaimsFromToken(String idToken, AbstractXOAuthIdentityProviderDefinition config) {
if(idToken == null) { if(idToken == null) {
return null; return null;
} }
Expand Down Expand Up @@ -324,6 +324,9 @@ private String getTokenKeyFromOAuth(AbstractXOAuthIdentityProviderDefinition con
} }


private String getTokenFromCode(XOAuthCodeToken codeToken, AbstractXOAuthIdentityProviderDefinition config) { private String getTokenFromCode(XOAuthCodeToken codeToken, AbstractXOAuthIdentityProviderDefinition config) {
if (StringUtils.hasText(codeToken.getIdToken()) && "id_token".equals(getResponseType(config))) {
return codeToken.getIdToken();
}
MultiValueMap<String, String> body = new LinkedMultiValueMap<>(); MultiValueMap<String, String> body = new LinkedMultiValueMap<>();
body.add("grant_type", "authorization_code"); body.add("grant_type", "authorization_code");
body.add("response_type", getResponseType(config)); body.add("response_type", getResponseType(config));
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/resources/templates/web/login.html
View file
Expand Up @@ -34,7 +34,7 @@ <h1 th:text="${T(org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder).uaa ? 'W
</div> </div>
<div th:each="oauthIdp : ${oauthDefinitions}" th:if="${oauthIdp.value.showLinkText}"> <div th:each="oauthIdp : ${oauthDefinitions}" th:if="${oauthIdp.value.showLinkText}">
<div th:if="${oauthIdp.value.scopes == null}"> <div th:if="${oauthIdp.value.scopes == null}">
<a href="" th:href="@{${oauthIdp.value.authUrl}(client_id=${oauthIdp.value.relyingPartyId},response_type=code,redirect_uri=${#httpServletRequest.requestURL + '/callback/' + oauthIdp.key})}" th:text="${oauthIdp.value.linkText}" class="saml-login-link">Use your corporate credentials</a> <a href="" th:href="@{${oauthIdp.value.authUrl}(client_id=${oauthIdp.value.relyingPartyId},response_type=${oauthIdp.value.responseType},redirect_uri=${#httpServletRequest.requestURL + '/callback/' + oauthIdp.key})}" th:text="${oauthIdp.value.linkText}" class="saml-login-link">Use your corporate credentials</a>
</div> </div>
<div th:if="${oauthIdp.value.scopes != null}"> <div th:if="${oauthIdp.value.scopes != null}">
<a href="" th:href="@{${oauthIdp.value.authUrl}(client_id=${oauthIdp.value.relyingPartyId},response_type=code,redirect_uri=${#httpServletRequest.requestURL + '/callback/' + oauthIdp.key},scope=${#strings.listJoin(oauthIdp.value.scopes,' ')})}" th:text="${oauthIdp.value.linkText}" class="saml-login-link">Use your corporate credentials</a> <a href="" th:href="@{${oauthIdp.value.authUrl}(client_id=${oauthIdp.value.relyingPartyId},response_type=code,redirect_uri=${#httpServletRequest.requestURL + '/callback/' + oauthIdp.key},scope=${#strings.listJoin(oauthIdp.value.scopes,' ')})}" th:text="${oauthIdp.value.linkText}" class="saml-login-link">Use your corporate credentials</a>
Expand Down
58 changes: 48 additions & 10 deletions ...st/java/org/cloudfoundry/identity/uaa/provider/oauth/XOAuthAuthenticationManagerTest.java
View file
Expand Up @@ -14,8 +14,8 @@
package org.cloudfoundry.identity.uaa.provider.oauth; package org.cloudfoundry.identity.uaa.provider.oauth;


import org.apache.commons.codec.binary.Base64; import org.apache.commons.codec.binary.Base64;
import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication;
import org.cloudfoundry.identity.uaa.authentication.AccountNotPreCreatedException; import org.cloudfoundry.identity.uaa.authentication.AccountNotPreCreatedException;
import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication;
import org.cloudfoundry.identity.uaa.authentication.manager.ExternalGroupAuthorizationEvent; import org.cloudfoundry.identity.uaa.authentication.manager.ExternalGroupAuthorizationEvent;
import org.cloudfoundry.identity.uaa.authentication.manager.InvitedUserAuthenticatedEvent; import org.cloudfoundry.identity.uaa.authentication.manager.InvitedUserAuthenticatedEvent;
import org.cloudfoundry.identity.uaa.authentication.manager.NewUserAuthenticatedEvent; import org.cloudfoundry.identity.uaa.authentication.manager.NewUserAuthenticatedEvent;
Expand Down Expand Up @@ -80,10 +80,15 @@
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.mockito.Matchers.anyObject;
import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq; import static org.mockito.Matchers.eq;
import static org.mockito.Matchers.same;
import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -144,7 +149,7 @@ public void setUp() throws Exception {
provisioning = mock(JdbcIdentityProviderProvisioning.class); provisioning = mock(JdbcIdentityProviderProvisioning.class);
userDatabase = new InMemoryUaaUserDatabase(Collections.emptySet()); userDatabase = new InMemoryUaaUserDatabase(Collections.emptySet());
publisher = mock(ApplicationEventPublisher.class); publisher = mock(ApplicationEventPublisher.class);
xoAuthAuthenticationManager = new XOAuthAuthenticationManager(provisioning); xoAuthAuthenticationManager = spy(new XOAuthAuthenticationManager(provisioning));
xoAuthAuthenticationManager.setUserDatabase(userDatabase); xoAuthAuthenticationManager.setUserDatabase(userDatabase);
xoAuthAuthenticationManager.setApplicationEventPublisher(publisher); xoAuthAuthenticationManager.setApplicationEventPublisher(publisher);
xCodeToken = new XOAuthCodeToken(CODE, ORIGIN, "http://localhost/callback/the_origin"); xCodeToken = new XOAuthCodeToken(CODE, ORIGIN, "http://localhost/callback/the_origin");
Expand Down Expand Up @@ -188,9 +193,38 @@ public void setUp() throws Exception {
"-----END PUBLIC KEY-----"); "-----END PUBLIC KEY-----");


mockUaaServer = MockRestServiceServer.createServer(xoAuthAuthenticationManager.getRestTemplate(config)); mockUaaServer = MockRestServiceServer.createServer(xoAuthAuthenticationManager.getRestTemplate(config));
reset(xoAuthAuthenticationManager);


} }


@Test
public void idToken_In_Redirect_Should_Use_it() throws Exception {
mockToken();
addTheUserOnAuth();
String tokenResponse = getIdTokenResponse();
String idToken = (String) JsonUtils.readValue(tokenResponse, Map.class).get("id_token");
xCodeToken.setIdToken(idToken);
xoAuthAuthenticationManager.authenticate(xCodeToken);

verify(xoAuthAuthenticationManager, times(1)).getClaimsFromToken(same(xCodeToken), anyObject());
verify(xoAuthAuthenticationManager, times(1)).getClaimsFromToken(eq(idToken), anyObject());
verify(xoAuthAuthenticationManager, never()).getRestTemplate(anyObject());

ArgumentCaptor<ApplicationEvent> userArgumentCaptor = ArgumentCaptor.forClass(ApplicationEvent.class);
verify(publisher,times(3)).publishEvent(userArgumentCaptor.capture());
assertEquals(3, userArgumentCaptor.getAllValues().size());
NewUserAuthenticatedEvent event = (NewUserAuthenticatedEvent)userArgumentCaptor.getAllValues().get(0);

UaaUser uaaUser = event.getUser();
assertEquals("Marissa",uaaUser.getGivenName());
assertEquals("Bloggs", uaaUser.getFamilyName());
assertEquals("marissa@bloggs.com", uaaUser.getEmail());
assertEquals("the_origin", uaaUser.getOrigin());
assertEquals("1234567890", uaaUser.getPhoneNumber());
assertEquals("marissa",uaaUser.getUsername());
assertEquals(OriginKeys.UAA, uaaUser.getZoneId());
}

@Test @Test
public void exchangeExternalCodeForIdToken_andCreateShadowUser() throws Exception { public void exchangeExternalCodeForIdToken_andCreateShadowUser() throws Exception {
mockToken(); mockToken();
Expand Down Expand Up @@ -546,14 +580,7 @@ public void authenticationContainsAMRClaim_fromExternalOIDCProvider() throws Exc
} }


private void mockToken() throws MalformedURLException { private void mockToken() throws MalformedURLException {
String idTokenJwt = UaaTokenUtils.constructToken(header, claims, signer); String response = getIdTokenResponse();
identityProvider = getProvider();

when(provisioning.retrieveByOrigin(eq(ORIGIN), anyString())).thenReturn(identityProvider);

CompositeAccessToken compositeAccessToken = new CompositeAccessToken("accessToken");
compositeAccessToken.setIdTokenValue(idTokenJwt);
String response = JsonUtils.writeValueAsString(compositeAccessToken);
mockUaaServer.expect(requestTo("http://oidc10.identity.cf-app.com/oauth/token")) mockUaaServer.expect(requestTo("http://oidc10.identity.cf-app.com/oauth/token"))
.andExpect(header("Authorization", "Basic " + new String(Base64.encodeBase64("identity:identitysecret".getBytes())))) .andExpect(header("Authorization", "Basic " + new String(Base64.encodeBase64("identity:identitysecret".getBytes()))))
.andExpect(header("Accept", "application/json")) .andExpect(header("Accept", "application/json"))
Expand All @@ -564,6 +591,17 @@ private void mockToken() throws MalformedURLException {
.andRespond(withStatus(OK).contentType(APPLICATION_JSON).body(response)); .andRespond(withStatus(OK).contentType(APPLICATION_JSON).body(response));
} }


private String getIdTokenResponse() throws MalformedURLException {
String idTokenJwt = UaaTokenUtils.constructToken(header, claims, signer);
identityProvider = getProvider();

when(provisioning.retrieveByOrigin(eq(ORIGIN), anyString())).thenReturn(identityProvider);

CompositeAccessToken compositeAccessToken = new CompositeAccessToken("accessToken");
compositeAccessToken.setIdTokenValue(idTokenJwt);
return JsonUtils.writeValueAsString(compositeAccessToken);
}

private IdentityProvider<AbstractXOAuthIdentityProviderDefinition> getProvider() throws MalformedURLException { private IdentityProvider<AbstractXOAuthIdentityProviderDefinition> getProvider() throws MalformedURLException {
IdentityProvider<AbstractXOAuthIdentityProviderDefinition> identityProvider = new IdentityProvider<>(); IdentityProvider<AbstractXOAuthIdentityProviderDefinition> identityProvider = new IdentityProvider<>();
identityProvider.setName("my oidc provider"); identityProvider.setName("my oidc provider");
Expand Down
48 changes: 39 additions & 9 deletions uaa/src/test/java/org/cloudfoundry/identity/uaa/login/LoginMockMvcTests.java
View file
Expand Up @@ -71,6 +71,7 @@
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
import org.springframework.util.ReflectionUtils; import org.springframework.util.ReflectionUtils;
import org.springframework.util.StringUtils;
import org.springframework.web.context.support.XmlWebApplicationContext; import org.springframework.web.context.support.XmlWebApplicationContext;


import javax.servlet.http.Cookie; import javax.servlet.http.Cookie;
Expand Down Expand Up @@ -1137,7 +1138,7 @@ public void samlRedirect_onlyOneProvider_noClientContext() throws Exception {
} }


@Test @Test
public void xOAuthRedirect_onlyOneProvider_noClientContext() throws Exception { public void xOAuthRedirect_onlyOneProvider_noClientContext_and_ResponseType_Set() throws Exception {
final String zoneAdminClientId = "admin"; final String zoneAdminClientId = "admin";
BaseClientDetails zoneAdminClient = new BaseClientDetails(zoneAdminClientId, null, "openid", "client_credentials,authorization_code", "clients.admin,scim.read,scim.write","http://test.redirect.com"); BaseClientDetails zoneAdminClient = new BaseClientDetails(zoneAdminClientId, null, "openid", "client_credentials,authorization_code", "clients.admin,scim.read,scim.write","http://test.redirect.com");
zoneAdminClient.setClientSecret("admin-secret"); zoneAdminClient.setClientSecret("admin-secret");
Expand All @@ -1155,6 +1156,7 @@ public void xOAuthRedirect_onlyOneProvider_noClientContext() throws Exception {
definition.setRelyingPartySecret("secret"); definition.setRelyingPartySecret("secret");
definition.setShowLinkText(false); definition.setShowLinkText(false);
definition.setScopes(asList("openid", "roles")); definition.setScopes(asList("openid", "roles"));
definition.setResponseType("code id_token");
String oauthAlias = "login-oauth-" + generator.generate(); String oauthAlias = "login-oauth-" + generator.generate();


IdentityProvider<OIDCIdentityProviderDefinition> oauthIdentityProvider = MultitenancyFixture.identityProvider(oauthAlias, "uaa"); IdentityProvider<OIDCIdentityProviderDefinition> oauthIdentityProvider = MultitenancyFixture.identityProvider(oauthAlias, "uaa");
Expand All @@ -1173,7 +1175,7 @@ public void xOAuthRedirect_onlyOneProvider_noClientContext() throws Exception {
.servletPath("/login") .servletPath("/login")
.with(new SetServerNameRequestPostProcessor(identityZone.getSubdomain() + ".localhost"))) .with(new SetServerNameRequestPostProcessor(identityZone.getSubdomain() + ".localhost")))
.andExpect(status().isFound()) .andExpect(status().isFound())
.andExpect(redirectedUrl("http://auth.url?client_id=uaa&response_type=code&redirect_uri=http%3A%2F%2F" + identityZone.getSubdomain() + ".localhost%2Flogin%2Fcallback%2F" + oauthAlias + "&scope=openid+roles")); .andExpect(redirectedUrl("http://auth.url?client_id=uaa&response_type=code+id_token&redirect_uri=http%3A%2F%2F" + identityZone.getSubdomain() + ".localhost%2Flogin%2Fcallback%2F" + oauthAlias + "&scope=openid+roles"));
IdentityZoneHolder.clear(); IdentityZoneHolder.clear();
} }


Expand Down Expand Up @@ -1967,6 +1969,37 @@ public void idpDiscoveryRedirectsToOIDCProvider() throws Exception {
IdentityZone zone = MultitenancyFixture.identityZone("oidc-idp-discovery", "oidc-idp-discovery"); IdentityZone zone = MultitenancyFixture.identityZone("oidc-idp-discovery", "oidc-idp-discovery");
createOtherIdentityZone(zone.getSubdomain(), getMockMvc(), getWebApplicationContext()); createOtherIdentityZone(zone.getSubdomain(), getMockMvc(), getWebApplicationContext());


String originKey = createOIDCProvider(zone, "id_token code");

getMockMvc().perform(post("/login/idp_discovery")
.header("Accept", TEXT_HTML)
.servletPath("/login/idp_discovery")
.param("email", "marissa@test.org")
.with(new SetServerNameRequestPostProcessor(zone.getSubdomain() + ".localhost")))
.andExpect(redirectedUrl("http://myauthurl.com?client_id=id&response_type=id_token+code&redirect_uri=http%3A%2F%2Foidc-idp-discovery.localhost%2Flogin%2Fcallback%2F" +originKey));
}

@Test
public void multiple_oidc_providers_use_response_type_in_url() throws Exception {
IdentityZone zone = MultitenancyFixture.identityZone("oidc-idp-discovery-multi", "oidc-idp-discovery-multi");
createOtherIdentityZone(zone.getSubdomain(), getMockMvc(), getWebApplicationContext());

String originKey = createOIDCProvider(zone);
String originKey2 = createOIDCProvider(zone,"code id_token");

getMockMvc().perform(get("/login")
.header("Accept", TEXT_HTML)
.with(new SetServerNameRequestPostProcessor(zone.getSubdomain() + ".localhost")))
.andExpect(status().isOk())
.andExpect(content().string(containsString("http://myauthurl.com?client_id=id&amp;response_type=code&amp;redirect_uri=http%3A%2F%2Foidc-idp-discovery-multi.localhost%2Flogin%2Fcallback%2F" +originKey)))
.andExpect(content().string(containsString("http://myauthurl.com?client_id=id&amp;response_type=code+id_token&amp;redirect_uri=http%3A%2F%2Foidc-idp-discovery-multi.localhost%2Flogin%2Fcallback%2F" +originKey2)));

}

public String createOIDCProvider(IdentityZone zone) throws Exception {
return createOIDCProvider(zone, null);
}
public String createOIDCProvider(IdentityZone zone, String responseType) throws Exception {
String originKey = generator.generate(); String originKey = generator.generate();
AbstractXOAuthIdentityProviderDefinition definition = new OIDCIdentityProviderDefinition(); AbstractXOAuthIdentityProviderDefinition definition = new OIDCIdentityProviderDefinition();
definition.setEmailDomain(asList("test.org")); definition.setEmailDomain(asList("test.org"));
Expand All @@ -1976,18 +2009,15 @@ public void idpDiscoveryRedirectsToOIDCProvider() throws Exception {
definition.setRelyingPartyId("id"); definition.setRelyingPartyId("id");
definition.setRelyingPartySecret("secret"); definition.setRelyingPartySecret("secret");
definition.setLinkText("my oidc provider"); definition.setLinkText("my oidc provider");
if (StringUtils.hasText(responseType)) {
definition.setResponseType(responseType);
}


IdentityProvider identityProvider = MultitenancyFixture.identityProvider(originKey, zone.getId()); IdentityProvider identityProvider = MultitenancyFixture.identityProvider(originKey, zone.getId());
identityProvider.setType(OriginKeys.OIDC10); identityProvider.setType(OriginKeys.OIDC10);
identityProvider.setConfig(definition); identityProvider.setConfig(definition);
MockMvcUtils.createIdpUsingWebRequest(getMockMvc(), zone.getId(), adminToken, identityProvider, status().isCreated()); MockMvcUtils.createIdpUsingWebRequest(getMockMvc(), zone.getId(), adminToken, identityProvider, status().isCreated());

return originKey;
getMockMvc().perform(post("/login/idp_discovery")
.header("Accept", TEXT_HTML)
.servletPath("/login/idp_discovery")
.param("email", "marissa@test.org")
.with(new SetServerNameRequestPostProcessor(zone.getSubdomain() + ".localhost")))
.andExpect(redirectedUrl("http://myauthurl.com?client_id=id&response_type=code&redirect_uri=http%3A%2F%2Foidc-idp-discovery.localhost%2Flogin%2Fcallback%2F" +originKey));
} }


@Test @Test
Expand Down
1 change: 1 addition & 0 deletions ...test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsDocs.java
View file
Expand Up @@ -429,6 +429,7 @@ public void createOAuthIdentityProvider() throws Exception {
"`openid`, `roles`, or `profile` to request ID token, scopes populated in the ID token external groups attribute mappings, or the user profile information, respectively."), "`openid`, `roles`, or `profile` to request ID token, scopes populated in the ID token external groups attribute mappings, or the user profile information, respectively."),
fieldWithPath("config.checkTokenUrl").optional(null).type(OBJECT).description("Reserved for future OAuth use."), fieldWithPath("config.checkTokenUrl").optional(null).type(OBJECT).description("Reserved for future OAuth use."),
fieldWithPath("config.userInfoUrl").optional(null).type(OBJECT).description("Reserved for future OIDC use."), fieldWithPath("config.userInfoUrl").optional(null).type(OBJECT).description("Reserved for future OIDC use."),
fieldWithPath("config.responseType").optional("code").type(STRING).description("Response type for the authorize request, defaults to `code`, but can be `code id_token` if the OIDC server can return an id_token as a query parameter in the redirect."),
ADD_SHADOW_USER_ON_LOGIN, ADD_SHADOW_USER_ON_LOGIN,
EXTERNAL_GROUPS, EXTERNAL_GROUPS,
ATTRIBUTE_MAPPING, ATTRIBUTE_MAPPING,
Expand Down

0 comments on commit 492df57

Please sign in to comment.