Skip to content

Commit

Permalink
Refactor EmailAccountCreationService to use new UaaUrlUtils for URLs.
Browse files Browse the repository at this point in the history
- Remove unused default redirect location.

[#82406702] https://www.pivotaltracker.com/story/show/82406702

Signed-off-by: Chris Dutra <[email protected]>
  • Loading branch information
rdgallagher committed Jan 28, 2015
1 parent 5a78284 commit 32fbc2d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.http.client.utils.URIBuilder;
import org.cloudfoundry.identity.uaa.authentication.Origin;
import org.cloudfoundry.identity.uaa.codestore.ExpiringCode;
import org.cloudfoundry.identity.uaa.codestore.ExpiringCodeStore;
import org.cloudfoundry.identity.uaa.error.UaaException;
import org.cloudfoundry.identity.uaa.scim.ScimUser;
import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
import org.cloudfoundry.identity.uaa.scim.exception.ScimResourceAlreadyExistsException;
import org.cloudfoundry.identity.uaa.util.UaaUrlUtils;
import org.cloudfoundry.identity.uaa.zone.IdentityZone;
import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
import org.codehaus.jackson.map.ObjectMapper;
Expand All @@ -18,13 +18,11 @@
import org.springframework.security.oauth2.provider.ClientDetails;
import org.springframework.security.oauth2.provider.ClientDetailsService;
import org.springframework.security.oauth2.provider.NoSuchClientException;
import org.springframework.util.StringUtils;
import org.springframework.web.client.HttpClientErrorException;
import org.thymeleaf.context.Context;
import org.thymeleaf.spring4.SpringTemplateEngine;

import java.io.IOException;
import java.net.URISyntaxException;
import java.sql.Timestamp;
import java.util.Arrays;
import java.util.HashMap;
Expand All @@ -39,13 +37,12 @@ public class EmailAccountCreationService implements AccountCreationService {

private final SpringTemplateEngine templateEngine;
private final MessageService messageService;
private final String uaaBaseUrl;
private final String brand;
private final ObjectMapper objectMapper;
private final String baseUrl;
private final ExpiringCodeStore codeStore;
private final ScimUserProvisioning scimUserProvisioning;
private final ClientDetailsService clientDetailsService;
private final String brand;
private final UaaUrlUtils uaaUrlUtils;
private final ObjectMapper objectMapper;

public EmailAccountCreationService(
ObjectMapper objectMapper,
Expand All @@ -54,19 +51,17 @@ public EmailAccountCreationService(
ExpiringCodeStore codeStore,
ScimUserProvisioning scimUserProvisioning,
ClientDetailsService clientDetailsService,
String uaaBaseUrl,
String brand,
String baseUrl) {
UaaUrlUtils uaaUrlUtils,
String brand) {

this.objectMapper = objectMapper;
this.templateEngine = templateEngine;
this.messageService = messageService;
this.codeStore= codeStore;
this.scimUserProvisioning = scimUserProvisioning;
this.clientDetailsService = clientDetailsService;
this.uaaBaseUrl = uaaBaseUrl;
this.uaaUrlUtils = uaaUrlUtils;
this.brand = brand;
this.baseUrl = baseUrl;
}

@Override
Expand Down Expand Up @@ -126,35 +121,20 @@ public AccountCreationResponse completeActivation(String code) throws IOExceptio
user = scimUserProvisioning.verifyUser(user.getId(), user.getVersion());

String clientId = data.get("client_id");
String redirectLocation;
String redirectLocation = null;
if (clientId != null) {
try {
ClientDetails clientDetails = clientDetailsService.loadClientByClientId(clientId);
redirectLocation = (String) clientDetails.getAdditionalInformation().get(SIGNUP_REDIRECT_URL);
}
catch (NoSuchClientException e) {
redirectLocation = getDefaultRedirect();
logger.warn("Client deleted before user completed registration: " + clientId);
}
} else {
redirectLocation = getDefaultRedirect();
}

return new AccountCreationResponse(user.getId(), user.getUserName(), user.getUserName(), redirectLocation);
}

private String getDefaultRedirect() throws IOException {
String redirectLocation;
try {
URIBuilder builder = new URIBuilder(baseUrl);
String subdomain = IdentityZoneHolder.get().getSubdomain();
builder.setHost((StringUtils.isEmpty(subdomain) ? "" : subdomain + ".") + builder.getHost());
redirectLocation = builder.toString() + "/home";
} catch (URISyntaxException e) {
throw new IOException(e);
}
return redirectLocation;
}

@Override
public void resendVerificationCode(String email, String clientId) {
List<ScimUser> resources = scimUserProvisioning.query("userName eq \"" + email + "\" and origin eq \"" + Origin.UAA + "\"");
Expand Down Expand Up @@ -192,17 +172,7 @@ private String getSubjectText() {
}

private String getEmailHtml(String code, String email) {
String accountsUrl = null;
try {
URIBuilder builder = new URIBuilder(baseUrl + "/verify_user");
String subdomain = IdentityZoneHolder.get().getSubdomain();
if (!StringUtils.isEmpty(subdomain)) {
builder.setHost(subdomain + "." +builder.getHost());
}
accountsUrl = builder.build().toString();
} catch (URISyntaxException e) {
logger.error("Exception raised when building URI " + e);
}
String accountsUrl = uaaUrlUtils.getUaaUrl("/verify_user");

final Context ctx = new Context();
if (IdentityZoneHolder.get().equals(IdentityZone.getUaa())) {
Expand Down
3 changes: 1 addition & 2 deletions login/src/main/resources/login-ui.xml
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,8 @@
<constructor-arg ref="codeStore"/>
<constructor-arg ref="scimUserProvisioning"/>
<constructor-arg ref="jdbcClientDetailsService"/>
<constructor-arg ref="uaaUrl"/>
<constructor-arg ref="uaaUrlUtils" />
<constructor-arg value="${login.brand:oss}"/>
<constructor-arg value="${login.url:http://localhost:8080/uaa}"/>
</bean>

<bean id="invitationsService" class="org.cloudfoundry.identity.uaa.login.EmailInvitationsService">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.cloudfoundry.identity.uaa.scim.ScimUser;
import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
import org.cloudfoundry.identity.uaa.scim.exception.ScimResourceAlreadyExistsException;
import org.cloudfoundry.identity.uaa.util.UaaUrlUtils;
import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
import org.cloudfoundry.identity.uaa.zone.MultitenancyFixture;
import org.codehaus.jackson.map.ObjectMapper;
Expand All @@ -37,15 +38,13 @@
import org.mockito.ArgumentCaptor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.oauth2.provider.ClientDetails;
import org.springframework.security.oauth2.provider.ClientDetailsService;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletRequestAttributes;
import org.thymeleaf.spring4.SpringTemplateEngine;
Expand All @@ -57,7 +56,6 @@ public class EmailAccountCreationServiceTests {

private EmailAccountCreationService emailAccountCreationService;
private MessageService messageService;
private RestTemplate uaaTemplate;
private ExpiringCodeStore codeStore;
private ScimUserProvisioning scimUserProvisioning;
private ClientDetailsService clientDetailsService;
Expand All @@ -72,7 +70,6 @@ public class EmailAccountCreationServiceTests {
@Before
public void setUp() throws Exception {
SecurityContextHolder.clearContext();
uaaTemplate = new RestTemplate();
messageService = mock(MessageService.class);
codeStore = mock(ExpiringCodeStore.class);
scimUserProvisioning = mock(ScimUserProvisioning.class);
Expand All @@ -85,9 +82,8 @@ public void setUp() throws Exception {
codeStore,
scimUserProvisioning,
clientDetailsService,
"http://uaa.example.com",
"pivotal",
"http://login.example.com"
new UaaUrlUtils("http://uaa.example.com"),
"pivotal"
);
}

Expand Down Expand Up @@ -116,7 +112,7 @@ public void testBeginActivation() throws Exception {
);
String emailBody = emailBodyArgument.getValue();
assertThat(emailBody, containsString("a Pivotal ID"));
assertThat(emailBody, containsString("<a href=\"http://login.example.com/verify_user?code=the_secret_code&amp;email=user%40example.com\">Activate your account</a>"));
assertThat(emailBody, containsString("<a href=\"http://uaa.example.com/verify_user?code=the_secret_code&amp;email=user%40example.com\">Activate your account</a>"));
assertThat(emailBody, not(containsString("Cloud Foundry")));
}

Expand All @@ -141,7 +137,7 @@ public void testBeginActivationInOtherZone() throws Exception {
);
String emailBody = emailBodyArgument.getValue();
assertThat(emailBody, containsString("a Pivotal ID"));
assertThat(emailBody, containsString("<a href=\"http://test.login.example.com/verify_user?code=the_secret_code&amp;email=user%40example.com\">Activate your account</a>"));
assertThat(emailBody, containsString("<a href=\"http://test.uaa.example.com/verify_user?code=the_secret_code&amp;email=user%40example.com\">Activate your account</a>"));
assertThat(emailBody, not(containsString("Cloud Foundry")));
}

Expand All @@ -154,9 +150,8 @@ public void testBeginActivationWithOssBrand() throws Exception {
codeStore,
scimUserProvisioning,
clientDetailsService,
"http://uaa.example.com",
"oss",
"http://login.example.com");
new UaaUrlUtils("http://uaa.example.com"),
"oss");

setUpForSuccess();

Expand All @@ -177,15 +172,15 @@ public void testBeginActivationWithOssBrand() throws Exception {
);
String emailBody = emailBodyArgument.getValue();
assertThat(emailBody, containsString("an account"));
assertThat(emailBody, containsString("<a href=\"http://login.example.com/verify_user?code=the_secret_code&amp;email=user%40example.com\">Activate your account</a>"));
assertThat(emailBody, containsString("<a href=\"http://uaa.example.com/verify_user?code=the_secret_code&amp;email=user%40example.com\">Activate your account</a>"));
assertThat(emailBody, not(containsString("Pivotal")));
}

@Test(expected = UaaException.class)
public void testBeginActivationWithExistingUser() throws Exception {
setUpForSuccess();
user.setVerified(true);
when(scimUserProvisioning.query(anyString())).thenReturn(Arrays.asList(new ScimUser[] {user}));
when(scimUserProvisioning.query(anyString())).thenReturn(Arrays.asList(new ScimUser[]{user}));
when(scimUserProvisioning.createUser(any(ScimUser.class), anyString())).thenThrow(new ScimResourceAlreadyExistsException("duplicate"));
emailAccountCreationService.beginActivation("[email protected]", "password", "login");
}
Expand All @@ -195,7 +190,7 @@ public void testBeginActivationWithUnverifiedExistingUser() throws Exception {
setUpForSuccess();
user.setId("existing-user-id");
user.setVerified(false);
when(scimUserProvisioning.query(anyString())).thenReturn(Arrays.asList(new ScimUser[] {user}));
when(scimUserProvisioning.query(anyString())).thenReturn(Arrays.asList(new ScimUser[]{user}));
when(scimUserProvisioning.createUser(any(ScimUser.class), anyString())).thenThrow(new ScimResourceAlreadyExistsException("duplicate"));
when(codeStore.generateCode(anyString(), any(Timestamp.class))).thenReturn(code);
when(codeStore.retrieveCode(anyString())).thenReturn(code);
Expand All @@ -208,16 +203,38 @@ public void testBeginActivationWithUnverifiedExistingUser() throws Exception {
emailAccountCreationService.beginActivation("[email protected]", "password", "login");

verify(messageService).sendMessage(
eq("existing-user-id"),
eq("[email protected]"),
eq(MessageType.CREATE_ACCOUNT_CONFIRMATION),
anyString(),
anyString()
eq("existing-user-id"),
eq("[email protected]"),
eq(MessageType.CREATE_ACCOUNT_CONFIRMATION),
anyString(),
anyString()
);
}

@Test
public void testCompleteActivation() throws Exception {
setUpForSuccess();
when(scimUserProvisioning.createUser(any(ScimUser.class), anyString())).thenReturn(user);
when(codeStore.generateCode(anyString(), any(Timestamp.class))).thenReturn(code);
when(codeStore.retrieveCode("the_secret_code")).thenReturn(code);
when(scimUserProvisioning.verifyUser(anyString(), anyInt())).thenReturn(user);
when(scimUserProvisioning.retrieve(anyString())).thenReturn(user);

ClientDetails client = mock(ClientDetails.class);
when(details.getClientId()).thenReturn("login");
when(details.getAdditionalInformation()).thenReturn(new HashMap<String, Object>());
when(clientDetailsService.loadClientByClientId(anyString())).thenReturn(client);

AccountCreationService.AccountCreationResponse accountCreation = emailAccountCreationService.completeActivation("the_secret_code");

assertEquals("[email protected]", accountCreation.getUsername());
assertEquals("newly-created-user-id", accountCreation.getUserId());
assertEquals(null, accountCreation.getRedirectLocation());
assertNotNull(accountCreation.getUserId());
}

@Test
public void testCompleteActivationWithClientRedirect() throws Exception {
setUpForSuccess();
when(scimUserProvisioning.createUser(any(ScimUser.class), anyString())).thenReturn(user);
when(codeStore.generateCode(anyString(), any(Timestamp.class))).thenReturn(code);
Expand All @@ -226,7 +243,6 @@ public void testCompleteActivation() throws Exception {
when(scimUserProvisioning.retrieve(anyString())).thenReturn(user);
when(clientDetailsService.loadClientByClientId(anyString())).thenReturn(details);


AccountCreationService.AccountCreationResponse accountCreation = emailAccountCreationService.completeActivation("the_secret_code");

assertEquals("[email protected]", accountCreation.getUsername());
Expand Down Expand Up @@ -268,7 +284,7 @@ public void testResendVerificationCode() throws Exception {
);
String emailBody = emailBodyArgument.getValue();
assertThat(emailBody, containsString("a Pivotal ID"));
assertThat(emailBody, containsString("<a href=\"http://login.example.com/verify_user?code=the_secret_code&amp;email=user%40example.com\">Activate your account</a>"));
assertThat(emailBody, containsString("<a href=\"http://uaa.example.com/verify_user?code=the_secret_code&amp;email=user%40example.com\">Activate your account</a>"));
assertThat(emailBody, not(containsString("Cloud Foundry")));
}

Expand All @@ -287,17 +303,12 @@ private void setUpForSuccess() throws Exception {
Timestamp ts = new Timestamp(System.currentTimeMillis() + (60 * 60 * 1000)); // 1 hour
Map<String,Object> data = new HashMap<>();
data.put("user_id","newly-created-user-id");
data.put("client_id","login");
data.put("client_id", "login");
code = new ExpiringCode("the_secret_code", ts, new ObjectMapper().writeValueAsString(data));

Map<String, Object> additionalInfo = new HashMap<>();
additionalInfo.put(EmailAccountCreationService.SIGNUP_REDIRECT_URL, "http://example.com/redirect");
when(details.getClientId()).thenReturn("login");
when(details.getAdditionalInformation()).thenReturn(additionalInfo);

MockHttpServletRequest request = new MockHttpServletRequest();
request.setProtocol("http");
request.setContextPath("/login");
RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request));
}
}

0 comments on commit 32fbc2d

Please sign in to comment.