Skip to content

Commit

Permalink
Change email now allows wildcards for redirect urls
Browse files Browse the repository at this point in the history
- refactor EmailChangeEmailService to not use the endpoint

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

Signed-off-by: Jonathan Lo <jlo@us.ibm.com>
  • Loading branch information
mbhave authored and jlo committed Sep 11, 2015
1 parent 48d64c0 commit a2c3c46
Show file tree
Hide file tree
Showing 11 changed files with 347 additions and 134 deletions.
Expand Up @@ -187,6 +187,9 @@ private String getRedirectUris(Map<String, Object> map) {
if (map.get("signup_redirect_url") != null) { if (map.get("signup_redirect_url") != null) {
redirectUris.add((String) map.get("signup_redirect_url")); redirectUris.add((String) map.get("signup_redirect_url"));
} }
if (map.get("change_email_redirect_url") != null) {
redirectUris.add((String) map.get("change_email_redirect_url"));
}
return StringUtils.arrayToCommaDelimitedString(redirectUris.toArray(new String[] {})); return StringUtils.arrayToCommaDelimitedString(redirectUris.toArray(new String[] {}));
} }


Expand Down
Expand Up @@ -95,7 +95,7 @@ public void testSimpleAddClientWithSignupSuccessRedirectUrl() throws Exception {
} }


@Test @Test
public void testAllowedIdentityProvidersAsAdditionalInformation() throws Exception { public void testAdditionalInformation() throws Exception {
List<String> idps = Arrays.asList("idp1", "idp1"); List<String> idps = Arrays.asList("idp1", "idp1");
Map<String, Object> map = new HashMap<>(); Map<String, Object> map = new HashMap<>();
map.put("id", "foo"); map.put("id", "foo");
Expand All @@ -104,10 +104,12 @@ public void testAllowedIdentityProvidersAsAdditionalInformation() throws Excepti
map.put("authorized-grant-types", "authorization_code"); map.put("authorized-grant-types", "authorization_code");
map.put("authorities", "uaa.none"); map.put("authorities", "uaa.none");
map.put("signup_redirect_url", "callback_url"); map.put("signup_redirect_url", "callback_url");
map.put("change_email_redirect_url", "change_email_url");
map.put(ClientConstants.ALLOWED_PROVIDERS, idps); map.put(ClientConstants.ALLOWED_PROVIDERS, idps);
ClientDetails created = doSimpleTest(map); ClientDetails created = doSimpleTest(map);
assertEquals(idps, created.getAdditionalInformation().get(ClientConstants.ALLOWED_PROVIDERS)); assertEquals(idps, created.getAdditionalInformation().get(ClientConstants.ALLOWED_PROVIDERS));
assertTrue(created.getRegisteredRedirectUri().contains("callback_url")); assertTrue(created.getRegisteredRedirectUri().contains("callback_url"));
assertTrue(created.getRegisteredRedirectUri().contains("change_email_url"));
} }


@Test @Test
Expand All @@ -120,7 +122,7 @@ public void testSimpleAddClientWithChangeEmailRedirectUrl() throws Exception {
map.put("authorities", "uaa.none"); map.put("authorities", "uaa.none");
map.put("change_email_redirect_url", "change_email_callback_url"); map.put("change_email_redirect_url", "change_email_callback_url");
ClientDetails created = doSimpleTest(map); ClientDetails created = doSimpleTest(map);
assertSet((String) map.get("redirect-uri"), null, created.getRegisteredRedirectUri(), String.class); assertTrue(created.getRegisteredRedirectUri().contains("change_email_callback_url"));
} }


@Test @Test
Expand Down
Expand Up @@ -5,13 +5,11 @@
import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails; import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails;
import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal; import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
import org.cloudfoundry.identity.uaa.error.UaaException; import org.cloudfoundry.identity.uaa.error.UaaException;
import org.cloudfoundry.identity.uaa.user.UaaAuthority;
import org.cloudfoundry.identity.uaa.user.UaaUser; import org.cloudfoundry.identity.uaa.user.UaaUser;
import org.cloudfoundry.identity.uaa.user.UaaUserDatabase; import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
import org.hibernate.validator.constraints.Email; import org.hibernate.validator.constraints.Email;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
import org.springframework.security.authentication.AnonymousAuthenticationToken; import org.springframework.security.authentication.AnonymousAuthenticationToken;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.Authentication; import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolder;
Expand All @@ -38,10 +36,6 @@ public class ChangeEmailController {


private UaaUserDatabase uaaUserDatabase; private UaaUserDatabase uaaUserDatabase;


public UaaUserDatabase getUaaUserDatabase() {
return uaaUserDatabase;
}

public void setUaaUserDatabase(UaaUserDatabase uaaUserDatabase) { public void setUaaUserDatabase(UaaUserDatabase uaaUserDatabase) {
this.uaaUserDatabase = uaaUserDatabase; this.uaaUserDatabase = uaaUserDatabase;
} }
Expand All @@ -51,15 +45,20 @@ public ChangeEmailController(ChangeEmailService changeEmailService) {
} }


@RequestMapping(value = "/change_email", method = RequestMethod.GET) @RequestMapping(value = "/change_email", method = RequestMethod.GET)
public String changeEmailPage(Model model, @RequestParam(value = "client_id", required = false) String clientId) { public String changeEmailPage(Model model, @RequestParam(value = "client_id", required = false) String clientId,
@RequestParam(value = "redirect_uri", required = false) String redirectUri) {
SecurityContext securityContext = SecurityContextHolder.getContext(); SecurityContext securityContext = SecurityContextHolder.getContext();
model.addAttribute("email", ((UaaPrincipal)securityContext.getAuthentication().getPrincipal()).getEmail()); model.addAttribute("email", ((UaaPrincipal)securityContext.getAuthentication().getPrincipal()).getEmail());
model.addAttribute("client_id", clientId); model.addAttribute("client_id", clientId);
model.addAttribute("redirect_uri", redirectUri);
return "change_email"; return "change_email";
} }


@RequestMapping(value = "/change_email.do", method = RequestMethod.POST) @RequestMapping(value = "/change_email.do", method = RequestMethod.POST)
public String changeEmail(Model model, @Valid @ModelAttribute("newEmail") ValidEmail newEmail, BindingResult result, @RequestParam("client_id") String clientId, RedirectAttributes redirectAttributes, HttpServletResponse response) { public String changeEmail(Model model, @Valid @ModelAttribute("newEmail") ValidEmail newEmail, BindingResult result,
@RequestParam(required = false, value = "client_id") String clientId,
@RequestParam(required = false, value = "redirect_uri") String redirectUri,
RedirectAttributes redirectAttributes, HttpServletResponse response) {
SecurityContext securityContext = SecurityContextHolder.getContext(); SecurityContext securityContext = SecurityContextHolder.getContext();


if(result.hasErrors()) { if(result.hasErrors()) {
Expand All @@ -78,7 +77,7 @@ public String changeEmail(Model model, @Valid @ModelAttribute("newEmail") ValidE
String userEmail = ((UaaPrincipal)securityContext.getAuthentication().getPrincipal()).getName(); String userEmail = ((UaaPrincipal)securityContext.getAuthentication().getPrincipal()).getName();


try { try {
changeEmailService.beginEmailChange(userId, userEmail, newEmail.getNewEmail(), clientId); changeEmailService.beginEmailChange(userId, userEmail, newEmail.getNewEmail(), clientId, redirectUri);
} catch (UaaException e) { } catch (UaaException e) {
if (e.getHttpStatus() == 409) { if (e.getHttpStatus() == 409) {
model.addAttribute("error_message_code", "username_exists"); model.addAttribute("error_message_code", "username_exists");
Expand All @@ -92,7 +91,8 @@ public String changeEmail(Model model, @Valid @ModelAttribute("newEmail") ValidE
} }


@RequestMapping(value = "/verify_email", method = RequestMethod.GET) @RequestMapping(value = "/verify_email", method = RequestMethod.GET)
public String verifyEmail(Model model, @RequestParam("code") String code, RedirectAttributes redirectAttributes, HttpServletResponse httpServletResponse, HttpServletRequest request) { public String verifyEmail(Model model, @RequestParam("code") String code, RedirectAttributes redirectAttributes,
HttpServletResponse httpServletResponse, HttpServletRequest request) {
Map<String,String> response; Map<String,String> response;


try { try {
Expand Down
Expand Up @@ -4,8 +4,8 @@


public interface ChangeEmailService { public interface ChangeEmailService {


public void beginEmailChange(String userId, String userEmail, String newEmail, String clientId); void beginEmailChange(String userId, String userEmail, String newEmail, String clientId, String redirectUri);


public Map<String, String> completeVerification(String code); Map<String, String> completeVerification(String code);


} }
Expand Up @@ -12,18 +12,29 @@
*******************************************************************************/ *******************************************************************************/
package org.cloudfoundry.identity.uaa.login; package org.cloudfoundry.identity.uaa.login;


import java.io.IOException; import java.sql.Timestamp;
import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;


import com.fasterxml.jackson.core.type.TypeReference;
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.error.UaaException;
import org.cloudfoundry.identity.uaa.scim.endpoints.ChangeEmailEndpoints; import org.cloudfoundry.identity.uaa.scim.ScimUser;
import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
import org.cloudfoundry.identity.uaa.util.UaaUrlUtils; import org.cloudfoundry.identity.uaa.util.UaaUrlUtils;
import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZone;
import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
import org.springframework.http.HttpStatus; import org.springframework.security.oauth2.provider.ClientDetails;
import org.springframework.http.ResponseEntity; import org.springframework.security.oauth2.provider.ClientDetailsService;
import org.springframework.util.StringUtils; import org.springframework.security.oauth2.provider.NoSuchClientException;
import org.thymeleaf.TemplateEngine; import org.thymeleaf.TemplateEngine;
import org.thymeleaf.context.Context; import org.thymeleaf.context.Context;


Expand All @@ -32,58 +43,96 @@ public class EmailChangeEmailService implements ChangeEmailService {
private final TemplateEngine templateEngine; private final TemplateEngine templateEngine;
private final MessageService messageService; private final MessageService messageService;
private final String brand; private final String brand;
private final ChangeEmailEndpoints endpoints; private final ScimUserProvisioning scimUserProvisioning;
private final ExpiringCodeStore codeStore;
private final ClientDetailsService clientDetailsService;
private static final int EMAIL_CHANGE_LIFETIME = 30 * 60 * 1000;
public static final String CHANGE_EMAIL_REDIRECT_URL = "change_email_redirect_url";
private final UaaUrlUtils uaaUrlUtils; private final UaaUrlUtils uaaUrlUtils;


public EmailChangeEmailService(TemplateEngine templateEngine, MessageService messageService, ChangeEmailEndpoints endpoints, UaaUrlUtils uaaUrlUtils, String brand) { public EmailChangeEmailService(TemplateEngine templateEngine, MessageService messageService, ScimUserProvisioning scimUserProvisioning, UaaUrlUtils uaaUrlUtils, String brand, ExpiringCodeStore codeStore, ClientDetailsService clientDetailsService) {
this.templateEngine = templateEngine; this.templateEngine = templateEngine;
this.messageService = messageService; this.messageService = messageService;
this.endpoints = endpoints; this.scimUserProvisioning = scimUserProvisioning;
this.uaaUrlUtils = uaaUrlUtils; this.uaaUrlUtils = uaaUrlUtils;
this.brand = brand; this.brand = brand;
this.codeStore = codeStore;
this.clientDetailsService = clientDetailsService;
} }


@Override @Override
public void beginEmailChange(String userId, String email, String newEmail, String clientId) { public void beginEmailChange(String userId, String email, String newEmail, String clientId, String redirectUri) {
ChangeEmailEndpoints.EmailChange change = new ChangeEmailEndpoints.EmailChange(); ScimUser user = scimUserProvisioning.retrieve(userId);
change.setClientId(clientId); List<ScimUser> results = scimUserProvisioning.query("userName eq \"" + newEmail + "\" and origin eq \"" + Origin.UAA + "\"");
change.setEmail(newEmail);
change.setUserId(userId); if (user.getUserName().equals(user.getPrimaryEmail())) {
ResponseEntity<String> result = endpoints.generateEmailVerificationCode(change); if (!results.isEmpty()) {
String htmlContent = null; throw new UaaException("Conflict", 409);
if (result.getStatusCode()== HttpStatus.CONFLICT) { }
throw new UaaException("Conflict", 409);
} else if (result.getStatusCode()==HttpStatus.CREATED) {
htmlContent = getEmailChangeEmailHtml(email, newEmail, result.getBody());
} }


String code = generateExpiringCode(userId, newEmail, clientId, redirectUri);
String htmlContent = getEmailChangeEmailHtml(email, newEmail, code);

if(htmlContent != null) { if(htmlContent != null) {
String subject = getSubjectText(); String subject = getSubjectText();
messageService.sendMessage(newEmail, MessageType.CHANGE_EMAIL, subject, htmlContent); messageService.sendMessage(newEmail, MessageType.CHANGE_EMAIL, subject, htmlContent);
} }
} }


private String generateExpiringCode(String userId, String newEmail, String clientId, String redirectUri) {
Map<String, String> codeData = new HashMap<>();
codeData.put("user_id", userId);
codeData.put("client_id", clientId);
codeData.put("redirect_uri", redirectUri);
codeData.put("email", newEmail);

return codeStore.generateCode(JsonUtils.writeValueAsString(codeData), new Timestamp(System.currentTimeMillis() + EMAIL_CHANGE_LIFETIME)).getCode();
}

@Override @Override
public Map<String, String> completeVerification(String code) { public Map<String, String> completeVerification(String code) {
ResponseEntity<ChangeEmailEndpoints.EmailChangeResponse> responseEntity; ExpiringCode expiringCode = codeStore.retrieveCode(code);
ChangeEmailEndpoints.EmailChangeResponse response = null; if (expiringCode == null) {
try { throw new UaaException("Error", 400);
responseEntity = endpoints.changeEmail(code); }
if (responseEntity.getStatusCode()==HttpStatus.OK) {
response = responseEntity.getBody(); Map<String, String> codeData = JsonUtils.readValue(expiringCode.getData(), new TypeReference<Map<String, String>>() {});
} else { String userId = codeData.get("user_id");
throw new UaaException("Error",responseEntity.getStatusCode().value()); String email = codeData.get("email");
ScimUser user = scimUserProvisioning.retrieve(userId);

if (user.getUserName().equals(user.getPrimaryEmail())) {
user.setUserName(email);
}
user.setPrimaryEmail(email);
scimUserProvisioning.update(userId, user);

String clientId = codeData.get("client_id");
String redirectLocation = null;

if (clientId != null) {
String redirectUri = codeData.get("redirect_uri") == null ? "" : codeData.get("redirect_uri");

try {
ClientDetails clientDetails = clientDetailsService.loadClientByClientId(clientId);
Set<String> redirectUris = clientDetails.getRegisteredRedirectUri() == null ? Collections.emptySet() :
clientDetails.getRegisteredRedirectUri();
Set<Pattern> wildcards = UaaStringUtils.constructWildcards(redirectUris);
if (UaaStringUtils.matches(wildcards, redirectUri)) {
redirectLocation = redirectUri;
} else {
redirectLocation = (String) clientDetails.getAdditionalInformation().get(CHANGE_EMAIL_REDIRECT_URL);
}
} catch (NoSuchClientException e) {
} }
} catch (IOException e) {
throw new UaaException(e.getMessage(), e);
} }

Map<String,String> result = new HashMap<>(); Map<String,String> result = new HashMap<>();
result.put("userId", response.getUserId()); result.put("userId", user.getId());
result.put("username", response.getUsername()); result.put("username", user.getUserName());
result.put("email",response.getEmail()); result.put("email", user.getPrimaryEmail());
if (StringUtils.hasText(response.getRedirectUrl())) { result.put("redirect_url", redirectLocation);
result.put("redirect_url", response.getRedirectUrl());
}
return result; return result;
} }


Expand Down
4 changes: 3 additions & 1 deletion login/src/main/resources/login-ui.xml
Expand Up @@ -519,8 +519,10 @@
<bean id="changeEmailService" class="org.cloudfoundry.identity.uaa.login.EmailChangeEmailService"> <bean id="changeEmailService" class="org.cloudfoundry.identity.uaa.login.EmailChangeEmailService">
<constructor-arg ref="mailTemplateEngine"/> <constructor-arg ref="mailTemplateEngine"/>
<constructor-arg ref="messageService"/> <constructor-arg ref="messageService"/>
<constructor-arg ref="changeEmailEndpoints"/>
<constructor-arg ref="uaaUrlUtils"/> <constructor-arg ref="uaaUrlUtils"/>
<constructor-arg ref="jdbcClientDetailsService"/>
<constructor-arg ref="scimUserProvisioning"/>
<constructor-arg ref="codeStore"/>
<constructor-arg value="${login.brand:oss}"/> <constructor-arg value="${login.brand:oss}"/>
</bean> </bean>


Expand Down
1 change: 1 addition & 0 deletions login/src/main/resources/templates/web/change_email.html
Expand Up @@ -18,6 +18,7 @@ <h1>Change Email</h1>
<p th:text="#{'email_change.' + ${error_message_code}}">Error Message</p> <p th:text="#{'email_change.' + ${error_message_code}}">Error Message</p>
</div> </div>
<input type="hidden" name="client_id" th:value="${client_id}"/> <input type="hidden" name="client_id" th:value="${client_id}"/>
<input type="hidden" name="redirect_uri" th:value="${redirect_uri}"/>
<input name="newEmail" type="email" placeholder="New Email Address" autocomplete="off" class="form-control"/> <input name="newEmail" type="email" placeholder="New Email Address" autocomplete="off" class="form-control"/>
<input type="submit" value="Send Verification Link" class="island-button"/> <input type="submit" value="Send Verification Link" class="island-button"/>
</form> </form>
Expand Down

0 comments on commit a2c3c46

Please sign in to comment.