diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/codestore/ExpiringCodeStore.java b/server/src/main/java/org/cloudfoundry/identity/uaa/codestore/ExpiringCodeStore.java index ed19be246e0..22ac65cfe8c 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/codestore/ExpiringCodeStore.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/codestore/ExpiringCodeStore.java @@ -30,6 +30,19 @@ public interface ExpiringCodeStore { */ ExpiringCode generateCode(String data, Timestamp expiresAt, String intent, String zoneId); + /** + * Retrieve a code BUT DO NOT DELETE IT. + * + * WARNING - if you intend to expire the code as soon as you read it, + * use {@link #retrieveCode(String, String)} instead. + * + * @param code the one-time code to look for + * @param zoneId + * @return code or null if the code is not found + * @throws java.lang.NullPointerException if the code is null + */ + ExpiringCode peekCode(String code, String zoneId); + /** * Retrieve a code and delete it if it exists. * diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/codestore/JdbcExpiringCodeStore.java b/server/src/main/java/org/cloudfoundry/identity/uaa/codestore/JdbcExpiringCodeStore.java index 27a5b9e8788..79fa9a4e05f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/codestore/JdbcExpiringCodeStore.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/codestore/JdbcExpiringCodeStore.java @@ -111,6 +111,25 @@ public ExpiringCode generateCode(String data, Timestamp expiresAt, String intent return null; } + @Override + public ExpiringCode peekCode(String code, String zoneId) { + cleanExpiredEntries(); + + if (code == null) { + throw new NullPointerException(); + } + + try { + ExpiringCode expiringCode = jdbcTemplate.queryForObject(selectAllFields, rowMapper, code, zoneId); + if (expiringCode.getExpiresAt().getTime() < timeService.getCurrentTimeMillis()) { + expiringCode = null; + } + return expiringCode; + } catch (EmptyResultDataAccessException x) { + return null; + } + } + @Override public ExpiringCode retrieveCode(String code, String zoneId) { cleanExpiredEntries(); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/invitations/InvitationsController.java b/server/src/main/java/org/cloudfoundry/identity/uaa/invitations/InvitationsController.java index 94fa0587d79..8df3b94b410 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/invitations/InvitationsController.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/invitations/InvitationsController.java @@ -116,7 +116,7 @@ public void return404(HttpServletResponse response) { @RequestMapping(value = "/accept", method = GET, params = {"code"}) public String acceptInvitePage(@RequestParam String code, Model model, HttpServletRequest request, HttpServletResponse response) { - ExpiringCode expiringCode = expiringCodeStore.retrieveCode(code, IdentityZoneHolder.get().getId()); + ExpiringCode expiringCode = expiringCodeStore.peekCode(code, IdentityZoneHolder.get().getId()); if ((null == expiringCode) || (null != expiringCode.getIntent() && !INVITATION.name().equals(expiringCode.getIntent()))) { return handleUnprocessableEntity(model, response, "error_message_code", "code_expired", "invitations/accept_invite"); } @@ -128,7 +128,6 @@ public String acceptInvitePage(@RequestParam String code, Model model, HttpServl String origin = codeData.get(ORIGIN); try { IdentityProvider provider = identityProviderProvisioning.retrieveByOrigin(origin, IdentityZoneHolder.get().getId()); - final String newCode = expiringCodeStore.generateCode(expiringCode.getData(), new Timestamp(System.currentTimeMillis() + (10 * 60 * 1000)), expiringCode.getIntent(), IdentityZoneHolder.get().getId()).getCode(); UaaUser user = userDatabase.retrieveUserById(codeData.get("user_id")); boolean isUaaUserAndVerified = @@ -136,12 +135,12 @@ public String acceptInvitePage(@RequestParam String code, Model model, HttpServl boolean isExternalUserAndAcceptedInvite = !UAA.equals(provider.getType()) && UaaHttpRequestUtils.isAcceptedInvitationAuthentication(); if (isUaaUserAndVerified || isExternalUserAndAcceptedInvite) { - AcceptedInvitation accepted = invitationsService.acceptInvitation(newCode, ""); + AcceptedInvitation accepted = invitationsService.acceptInvitation(code, ""); String redirect = "redirect:" + accepted.getRedirectUri(); logger.debug(String.format("Redirecting accepted invitation for email:%s, id:%s to URL:%s", codeData.get("email"), codeData.get("user_id"), redirect)); return redirect; } else if (SAML.equals(provider.getType())) { - setRequestAttributes(request, newCode, user); + setRequestAttributes(request, code, user); SamlIdentityProviderDefinition definition = ObjectUtils.castInstance(provider.getConfig(), SamlIdentityProviderDefinition.class); @@ -149,7 +148,7 @@ public String acceptInvitePage(@RequestParam String code, Model model, HttpServl logger.debug(String.format("Redirecting invitation for email:%s, id:%s single SAML IDP URL:%s", codeData.get("email"), codeData.get("user_id"), redirect)); return redirect; } else if (OIDC10.equals(provider.getType()) || OAUTH20.equals(provider.getType())) { - setRequestAttributes(request, newCode, user); + setRequestAttributes(request, code, user); AbstractExternalOAuthIdentityProviderDefinition definition = ObjectUtils.castInstance(provider.getConfig(), AbstractExternalOAuthIdentityProviderDefinition.class); @@ -162,7 +161,7 @@ public String acceptInvitePage(@RequestParam String code, Model model, HttpServl Collections.singletonList(UaaAuthority.UAA_INVITED)); SecurityContextHolder.getContext().setAuthentication(token); model.addAttribute("provider", provider.getType()); - model.addAttribute("code", newCode); + model.addAttribute("code", code); model.addAttribute("email", codeData.get("email")); logger.debug(String.format("Sending user to accept invitation page email:%s, id:%s", codeData.get("email"), codeData.get("user_id"))); } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/codestore/ExpiringCodeStoreTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/codestore/ExpiringCodeStoreTests.java index 1de646002ad..07bd3c588a0 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/codestore/ExpiringCodeStoreTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/codestore/ExpiringCodeStoreTests.java @@ -92,6 +92,19 @@ void generateCodeWithDuplicateCode() { () -> expiringCodeStore.generateCode(data, expiresAt, null, IdentityZone.getUaaZoneId())); } + @Test + void peekCode() { + String data = "{}"; + Timestamp expiresAt = new Timestamp(System.currentTimeMillis() + 60000); + String zoneId = IdentityZone.getUaaZoneId(); + + ExpiringCode generatedCode = expiringCodeStore.generateCode(data, expiresAt, null, zoneId); + + assertEquals(generatedCode, expiringCodeStore.peekCode(generatedCode.getCode(), zoneId)); + assertEquals(generatedCode, expiringCodeStore.peekCode(generatedCode.getCode(), zoneId)); + assertEquals(generatedCode, expiringCodeStore.peekCode(generatedCode.getCode(), zoneId)); + } + @Test void retrieveCode() { String data = "{}"; diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/codestore/InMemoryExpiringCodeStore.java b/server/src/test/java/org/cloudfoundry/identity/uaa/codestore/InMemoryExpiringCodeStore.java index 87ff302befc..b7e24e385f8 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/codestore/InMemoryExpiringCodeStore.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/codestore/InMemoryExpiringCodeStore.java @@ -44,6 +44,21 @@ public ExpiringCode generateCode(String data, Timestamp expiresAt, String intent return expiringCode; } + @Override + public ExpiringCode peekCode(String code, String zoneId) { + if (code == null) { + throw new NullPointerException(); + } + + ExpiringCode expiringCode = store.get(code + zoneId); + + if (expiringCode == null || isExpired(expiringCode)) { + expiringCode = null; + } + + return expiringCode; + } + @Override public ExpiringCode retrieveCode(String code, String zoneId) { if (code == null) { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsControllerTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsControllerTest.java index 5d13ede7424..d592bda6097 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsControllerTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/invitations/InvitationsControllerTest.java @@ -150,8 +150,7 @@ public void testAcceptInvitationsPage() throws Exception { codeData.put("email", "user@example.com"); codeData.put("client_id", "client-id"); codeData.put("redirect_uri", "blah.test.com"); - when(expiringCodeStore.retrieveCode("code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData), null); - when(expiringCodeStore.generateCode(anyString(), any(), eq(INVITATION.name()), eq(IdentityZoneHolder.get().getId()))).thenReturn(createCode(codeData)); + when(expiringCodeStore.peekCode("code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData), null); IdentityProvider provider = new IdentityProvider(); provider.setType(OriginKeys.UAA); when(providerProvisioning.retrieveByOrigin(any(), any())).thenReturn(provider); @@ -193,8 +192,7 @@ public void incorrectCodeIntent() throws Exception { @Test public void acceptInvitePage_for_unverifiedSamlUser() throws Exception { Map codeData = getInvitationsCode("test-saml"); - when(expiringCodeStore.retrieveCode("the_secret_code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData)); - when(expiringCodeStore.generateCode(anyString(), any(), eq(INVITATION.name()), eq(IdentityZoneHolder.get().getId()))).thenReturn(createCode(codeData)); + when(expiringCodeStore.peekCode("the_secret_code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData)); IdentityProvider provider = new IdentityProvider(); SamlIdentityProviderDefinition definition = new SamlIdentityProviderDefinition() .setMetaDataLocation("http://test.saml.com") @@ -220,8 +218,7 @@ public void acceptInvitePage_for_unverifiedSamlUser() throws Exception { @Test public void acceptInvitePage_for_unverifiedOIDCUser() throws Exception { Map codeData = getInvitationsCode("test-oidc"); - when(expiringCodeStore.retrieveCode("the_secret_code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData)); - when(expiringCodeStore.generateCode(anyString(), any(), eq(INVITATION.name()), eq(IdentityZoneHolder.get().getId()))).thenReturn(createCode(codeData)); + when(expiringCodeStore.peekCode("the_secret_code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData)); OIDCIdentityProviderDefinition definition = new OIDCIdentityProviderDefinition(); definition.setAuthUrl(new URL("https://oidc10.auth.url")); @@ -247,8 +244,7 @@ public void acceptInvitePage_for_unverifiedOIDCUser() throws Exception { @Test public void acceptInvitePage_for_unverifiedLdapUser() throws Exception { Map codeData = getInvitationsCode(LDAP); - when(expiringCodeStore.retrieveCode("the_secret_code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData)); - when(expiringCodeStore.generateCode(anyString(), any(), eq(INVITATION.name()), eq(IdentityZoneHolder.get().getId()))).thenReturn(createCode(codeData)); + when(expiringCodeStore.peekCode("the_secret_code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData)); IdentityProvider provider = new IdentityProvider(); provider.setType(LDAP); @@ -263,7 +259,7 @@ public void acceptInvitePage_for_unverifiedLdapUser() throws Exception { .andExpect(content().string(containsString("Email: " + "user@example.com"))) .andExpect(content().string(containsString("Sign in with enterprise credentials:"))) .andExpect(content().string(containsString("username"))) - .andExpect(model().attribute("code", "code")) + .andExpect(model().attribute("code", "the_secret_code")) .andReturn(); } @@ -397,8 +393,7 @@ public void acceptInvitePage_for_verifiedUser() throws Exception { codeData.put("email", "user@example.com"); codeData.put("origin", "some-origin"); - when(expiringCodeStore.retrieveCode("the_secret_code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData), null); - when(expiringCodeStore.generateCode(anyString(), any(), eq(INVITATION.name()), eq(IdentityZoneHolder.get().getId()))).thenReturn(createCode(codeData)); + when(expiringCodeStore.peekCode("the_secret_code", IdentityZoneHolder.get().getId())).thenReturn(createCode(codeData), null); when(invitationsService.acceptInvitation(anyString(), eq(""))).thenReturn(new AcceptedInvitation("blah.test.com", new ScimUser())); IdentityProvider provider = new IdentityProvider(); provider.setType(OriginKeys.UAA); @@ -668,10 +663,8 @@ public void testAcceptInvite_displaysConsentText() throws Exception { Map codeData = getInvitationsCode(OriginKeys.UAA); String codeDataString = JsonUtils.writeValueAsString(codeData); ExpiringCode expiringCode = new ExpiringCode("thecode", new Timestamp(1), codeDataString, INVITATION.name()); - when(expiringCodeStore.retrieveCode("thecode", IdentityZoneHolder.get().getId())) + when(expiringCodeStore.peekCode("thecode", IdentityZoneHolder.get().getId())) .thenReturn(expiringCode, null); - when(expiringCodeStore.generateCode(anyString(), any(), eq(INVITATION.name()), eq(IdentityZoneHolder.get().getId()))) - .thenReturn(expiringCode); mockMvc.perform(get("/invitations/accept") .param("code", "thecode")) @@ -714,6 +707,8 @@ public void testAcceptInvite_displaysErrorMessageIfConsentNotChecked() throws Ex Map codeData = getInvitationsCode(OriginKeys.UAA); String codeDataString = JsonUtils.writeValueAsString(codeData); ExpiringCode expiringCode = new ExpiringCode("thecode", new Timestamp(1), codeDataString, INVITATION.name()); + when(expiringCodeStore.peekCode(anyString(), eq(IdentityZoneHolder.get().getId()))) + .thenReturn(expiringCode); when(expiringCodeStore.retrieveCode(anyString(), eq(IdentityZoneHolder.get().getId()))) .thenReturn(expiringCode); when(expiringCodeStore.generateCode(anyString(), any(), eq(INVITATION.name()), eq(IdentityZoneHolder.get().getId()))) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/login/InvitationsServiceMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/login/InvitationsServiceMockMvcTests.java index 9c929b024b0..0dd9870385d 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/login/InvitationsServiceMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/login/InvitationsServiceMockMvcTests.java @@ -207,7 +207,7 @@ void acceptInvitationForVerifiedUserSendsRedirect() throws Exception { } @Test - void acceptInvitationForUaaUserShouldExpireInvitelink() throws Exception { + void acceptInvitationForUaaUserShouldNotExpireInvitelink() throws Exception { String email = new RandomValueStringGenerator().generate().toLowerCase() + "@test.org"; URL inviteLink = inviteUser(webApplicationContext, mockMvc, email, userInviteToken, null, clientId, OriginKeys.UAA); assertEquals(OriginKeys.UAA, queryUserForField(jdbcTemplate, email, OriginKeys.ORIGIN, String.class)); @@ -218,9 +218,10 @@ void acceptInvitationForUaaUserShouldExpireInvitelink() throws Exception { .accept(MediaType.TEXT_HTML); mockMvc.perform(get) .andExpect(status().isOk()); - mockMvc.perform(get) - .andExpect(status().isUnprocessableEntity()); + .andExpect(status().isOk()); + mockMvc.perform(get) + .andExpect(status().isOk()); } @Test