From 1a3047a50567b739a2b05906ad6767b2462d7234 Mon Sep 17 00:00:00 2001 From: Paul Warren Date: Fri, 20 Nov 2015 10:00:44 -0800 Subject: [PATCH] Add builder-style setters and remove complex constructors on Approval [#107504490] https://www.pivotaltracker.com/story/show/107504490 Signed-off-by: Jeremy Coffield --- .../UserManagedAuthzApprovalHandler.java | 27 ++- .../approval/ApprovalsAdminEndpoints.java | 7 +- .../uaa/oauth/approval/JdbcApprovalStore.java | 9 +- .../event/ApprovalModifiedEventTest.java | 9 +- .../uaa/oauth/CheckTokenEndpointTests.java | 61 +++-- .../UserManagedAuthzApprovalHandlerTests.java | 210 +++++++++++++++--- .../uaa/oauth/approval/ApprovalTests.java | 195 +++++++++++++--- .../ApprovalsAdminEndpointsTests.java | 147 ++++++++++-- .../approval/JdbcApprovalStoreTests.java | 78 +++++-- .../oauth/token/UaaTokenServicesTests.java | 187 +++++++++++++--- .../identity/uaa/login/DescribedApproval.java | 9 +- .../identity/uaa/oauth/approval/Approval.java | 83 +++---- .../endpoints/ScimUserEndpointsTests.java | 55 ++++- .../ClientAdminEndpointsIntegrationTests.java | 24 +- .../mock/audit/AuditCheckMockMvcTests.java | 9 +- .../ClientAdminEndpointsMockMvcTests.java | 46 ++-- 16 files changed, 929 insertions(+), 227 deletions(-) diff --git a/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandler.java b/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandler.java index 0eb5238027..4447227a1a 100644 --- a/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandler.java +++ b/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandler.java @@ -126,12 +126,22 @@ public boolean isApproved(AuthorizationRequest authorizationRequest, Authenticat for (String requestedScope : requestedScopes) { if (approvedScopes.contains(requestedScope)) { - approvalStore.addApproval(new Approval(getUserId(userAuthentication), authorizationRequest - .getClientId(), requestedScope, expiry, APPROVED)); + Approval approval = new Approval() + .setUserId(getUserId(userAuthentication)) + .setClientId(authorizationRequest.getClientId()) + .setScope(requestedScope) + .setExpiresAt(expiry) + .setStatus(APPROVED); + approvalStore.addApproval(approval); } else { - approvalStore.addApproval(new Approval(getUserId(userAuthentication), authorizationRequest - .getClientId(), requestedScope, expiry, DENIED)); + Approval approval = new Approval() + .setUserId(getUserId(userAuthentication)) + .setClientId(authorizationRequest.getClientId()) + .setScope(requestedScope) + .setExpiresAt(expiry) + .setStatus(DENIED); + approvalStore.addApproval(approval); } } @@ -141,8 +151,13 @@ public boolean isApproved(AuthorizationRequest authorizationRequest, Authenticat for (String requestedScope : requestedScopes) { if (!autoApprovedScopes.contains(requestedScope)) { - approvalStore.addApproval(new Approval(getUserId(userAuthentication), authorizationRequest - .getClientId(), requestedScope, expiry, DENIED)); + Approval approval = new Approval() + .setUserId(getUserId(userAuthentication)) + .setClientId(authorizationRequest.getClientId()) + .setScope(requestedScope) + .setExpiresAt(expiry) + .setStatus(DENIED); + approvalStore.addApproval(approval); } } } diff --git a/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalsAdminEndpoints.java b/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalsAdminEndpoints.java index 3d0241dd9a..6efeca06ee 100644 --- a/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalsAdminEndpoints.java +++ b/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalsAdminEndpoints.java @@ -42,6 +42,7 @@ import org.springframework.security.oauth2.provider.ClientDetailsService; import org.springframework.stereotype.Controller; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.*; import org.springframework.web.client.RestTemplate; import org.springframework.web.servlet.View; @@ -153,14 +154,13 @@ public List updateApprovals(@RequestBody Approval[] approvals) { logger.debug("Updating approvals for user: " + currentUserId); approvalStore.revokeApprovals(String.format(USER_FILTER_TEMPLATE, currentUserId)); for (Approval approval : approvals) { - if (approval.getUserId() !=null && !isValidUser(approval.getUserId())) { - logger.warn(String.format("Error[2] %s attemting to update approvals for %s", currentUserId, approval.getUserId())); + if (StringUtils.hasText(approval.getUserId()) && !isValidUser(approval.getUserId())) { + logger.warn(String.format("Error[2] %s attempting to update approvals for %s", currentUserId, approval.getUserId())); throw new UaaException("unauthorized_operation", "Cannot update approvals for another user. Set user_id to null to update for existing user.", HttpStatus.UNAUTHORIZED.value()); } else { approval.setUserId(currentUserId); } - approval.setLastUpdatedAt(new Date()); approvalStore.addApproval(approval); } return approvalStore.getApprovals(String.format(USER_FILTER_TEMPLATE, currentUserId)); @@ -181,7 +181,6 @@ public List updateClientApprovals(@PathVariable String clientId, @Requ } else { approval.setUserId(currentUserId); } - approval.setLastUpdatedAt(new Date()); approvalStore.addApproval(approval); } return approvalStore.getApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, currentUserId, clientId)); diff --git a/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/JdbcApprovalStore.java b/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/JdbcApprovalStore.java index 516704affa..1f2853b185 100644 --- a/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/JdbcApprovalStore.java +++ b/common/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/JdbcApprovalStore.java @@ -223,7 +223,14 @@ public Approval mapRow(ResultSet rs, int rowNum) throws SQLException { String status = rs.getString(5); Date lastUpdatedAt = rs.getTimestamp(6); - return new Approval(userName, clientId, scope, expiresAt, ApprovalStatus.valueOf(status), lastUpdatedAt); + Approval approval = new Approval() + .setUserId(userName) + .setClientId(clientId) + .setScope(scope) + .setExpiresAt(expiresAt) + .setStatus(ApprovalStatus.valueOf(status)) + .setLastUpdatedAt(lastUpdatedAt); + return approval; } } } diff --git a/common/src/test/java/org/cloudfoundry/identity/uaa/audit/event/ApprovalModifiedEventTest.java b/common/src/test/java/org/cloudfoundry/identity/uaa/audit/event/ApprovalModifiedEventTest.java index 7452701971..0031432b8c 100644 --- a/common/src/test/java/org/cloudfoundry/identity/uaa/audit/event/ApprovalModifiedEventTest.java +++ b/common/src/test/java/org/cloudfoundry/identity/uaa/audit/event/ApprovalModifiedEventTest.java @@ -7,6 +7,8 @@ import org.junit.Assert; import org.junit.Test; +import java.util.Date; + public class ApprovalModifiedEventTest { @Test(expected = IllegalArgumentException.class) @@ -16,7 +18,12 @@ public void testRaisesWithBadSource() throws Exception { @Test public void testAuditEvent() throws Exception { - Approval approval = new Approval("mruser", "app", "cloud_controller.read", 1000, Approval.ApprovalStatus.APPROVED); + Approval approval = new Approval() + .setUserId("mruser") + .setClientId("app") + .setScope("cloud_controller.read") + .setExpiresAt(Approval.timeFromNow(1000)) + .setStatus(Approval.ApprovalStatus.APPROVED); ApprovalModifiedEvent event = new ApprovalModifiedEvent(approval, null); diff --git a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/CheckTokenEndpointTests.java b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/CheckTokenEndpointTests.java index 24bbc3230a..a1027e03ed 100644 --- a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/CheckTokenEndpointTests.java +++ b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/CheckTokenEndpointTests.java @@ -230,10 +230,20 @@ public void setUp() { Date oneSecondAgo = new Date(System.currentTimeMillis() - 1000); Date thirtySecondsAhead = new Date(System.currentTimeMillis() + 30000); - approvalStore.addApproval(new Approval(userId, "client", "read", thirtySecondsAhead, ApprovalStatus.APPROVED, - oneSecondAgo)); - approvalStore.addApproval(new Approval(userId, "client", "write", thirtySecondsAhead, ApprovalStatus.APPROVED, - oneSecondAgo)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("client") + .setScope("read") + .setExpiresAt(thirtySecondsAhead) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(oneSecondAgo)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("client") + .setScope("write") + .setExpiresAt(thirtySecondsAhead) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(oneSecondAgo)); tokenServices.setApprovalStore(approvalStore); tokenServices.setTokenPolicy(new TokenPolicy(43200, 2592000)); @@ -561,8 +571,12 @@ public void testExpiredToken() throws Exception { @Test(expected = InvalidTokenException.class) public void testUpdatedApprovals() { Date thirtySecondsAhead = new Date(System.currentTimeMillis() + 30000); - approvalStore.addApproval(new Approval(userId, "client", "read", thirtySecondsAhead, ApprovalStatus.APPROVED, - new Date())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("client") + .setScope("read") + .setExpiresAt(thirtySecondsAhead) + .setStatus(ApprovalStatus.APPROVED)); Claims result = endpoint.checkToken(accessToken.getValue()); assertEquals(null, result.getAuthorities()); } @@ -571,21 +585,38 @@ public void testUpdatedApprovals() { public void testDeniedApprovals() { Date oneSecondAgo = new Date(System.currentTimeMillis() - 1000); Date thirtySecondsAhead = new Date(System.currentTimeMillis() + 30000); - approvalStore.revokeApproval(new Approval(userId, "client", "read", thirtySecondsAhead, - ApprovalStatus.APPROVED, - oneSecondAgo)); - approvalStore.addApproval(new Approval(userId, "client", "read", thirtySecondsAhead, ApprovalStatus.DENIED, - oneSecondAgo)); + approvalStore.revokeApproval(new Approval() + .setUserId(userId) + .setClientId("client") + .setScope("read") + .setExpiresAt(thirtySecondsAhead) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(oneSecondAgo)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("client") + .setScope("read") + .setExpiresAt(thirtySecondsAhead) + .setStatus(ApprovalStatus.DENIED) + .setLastUpdatedAt(oneSecondAgo)); Claims result = endpoint.checkToken(accessToken.getValue()); assertEquals(null, result.getAuthorities()); } @Test(expected = InvalidTokenException.class) public void testExpiredApprovals() { - approvalStore.revokeApproval(new Approval(userId, "client", "read", new Date(), ApprovalStatus.APPROVED, - new Date())); - approvalStore.addApproval(new Approval(userId, "client", "read", new Date(), ApprovalStatus.APPROVED, - new Date())); + approvalStore.revokeApproval(new Approval() + .setUserId(userId) + .setClientId("client") + .setScope("read") + .setExpiresAt(new Date()) + .setStatus(ApprovalStatus.APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("client") + .setScope("read") + .setExpiresAt(new Date()) + .setStatus(ApprovalStatus.APPROVED)); Claims result = endpoint.checkToken(accessToken.getValue()); assertEquals(null, result.getAuthorities()); } diff --git a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandlerTests.java b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandlerTests.java index 6855840a79..6905354143 100644 --- a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandlerTests.java +++ b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandlerTests.java @@ -139,8 +139,18 @@ public void testNoRequestedScopesButSomeApprovedScopes() { long theFuture = System.currentTimeMillis() + (86400 * 7 * 1000); Date nextWeek = new Date(theFuture); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "cloud_controller.write",nextWeek, DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); // The request is approved because the user has not requested any scopes assertTrue(handler.isApproved(request, userAuthentication)); @@ -160,8 +170,18 @@ public void testRequestedScopesDontMatchApprovalsAtAll() { long theFuture = System.currentTimeMillis() + (86400 * 7 * 1000); Date nextWeek = new Date(theFuture); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "cloud_controller.write",nextWeek, DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); // The request is not approved because the user has not yet approved the // scopes requested @@ -181,8 +201,18 @@ public void testOnlySomeRequestedScopeMatchesApproval() { long theFuture = System.currentTimeMillis() + (86400 * 7 * 1000); Date nextWeek = new Date(theFuture); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "cloud_controller.write",nextWeek, DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); // The request is not approved because the user has not yet approved all // the scopes requested @@ -214,8 +244,18 @@ public void testOnlySomeRequestedScopeMatchesDeniedApprovalButScopeAutoApproved( ) ); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "cloud_controller.read",nextWeek, DENIED)); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "openid", nextWeek, DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("openid") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); assertTrue(handler.isApproved(request, userAuthentication)); assertEquals(new HashSet<>(Arrays.asList(new String[]{"cloud_controller.read", "openid"})),request.getScope()); @@ -238,8 +278,18 @@ public void testRequestedScopesMatchApprovalButAdditionalScopesRequested() { long theFuture = System.currentTimeMillis() + (86400 * 7 * 1000); Date nextWeek = new Date(theFuture); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userAuthentication.getId(), "foo", "cloud_controller.write",nextWeek, DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userAuthentication.getId()) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); // The request is not approved because the user has not yet approved all // the scopes requested @@ -262,9 +312,24 @@ public void testAllRequestedScopesMatchApproval() { long theFuture = System.currentTimeMillis() + (86400 * 7 * 1000); Date nextWeek = new Date(theFuture); - approvalStore.addApproval(new Approval(userId, "foo", "openid", nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.write",nextWeek, APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("openid") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); // The request is approved because the user has approved all the scopes // requested @@ -288,9 +353,24 @@ public void testRequestedScopesMatchApprovalButSomeDenied() { long theFuture = System.currentTimeMillis() + (86400 * 7 * 1000); Date nextWeek = new Date(theFuture); - approvalStore.addApproval(new Approval(userId, "foo", "openid", nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.write",nextWeek, DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("openid") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); // The request is approved because the user has acted on all requested // scopes @@ -323,9 +403,24 @@ public void testRequestedScopesMatchApprovalSomeDeniedButDeniedScopesAutoApprove }, Collections.singletonMap(ClientConstants.AUTO_APPROVE,(Object) Collections.singletonList("cloud_controller.write")))); - approvalStore.addApproval(new Approval(userId, "foo", "openid", nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.write",nextWeek, DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("openid") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); // The request is not approved because the user has denied some of the // scopes requested @@ -364,10 +459,30 @@ public void testRequestedScopesMatchApprovalSomeDeniedButDeniedScopesAutoApprove }, Collections.singletonMap(ClientConstants.AUTO_APPROVE,(Object) Arrays.asList("space.*.developer", "cloud_controller.write")))); - approvalStore.addApproval(new Approval(userId, "foo", "openid", nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.write",nextWeek, DENIED)); - approvalStore.addApproval(new Approval(userId, "foo", "space.1.developer",nextWeek, DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("openid") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("space.1.developer") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); // The request is not approved because the user has denied some of the // scopes requested @@ -405,10 +520,30 @@ public void testRequestedScopesMatchByWildcard() { }, Collections.singletonMap(ClientConstants.AUTO_APPROVE, (Object) "true"))); - approvalStore.addApproval(new Approval(userId, "foo", "openid", nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.write",nextWeek, DENIED)); - approvalStore.addApproval(new Approval(userId, "foo", "space.1.developer",nextWeek, DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("openid") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("space.1.developer") + .setExpiresAt(nextWeek) + .setStatus(DENIED)); // The request is not approved because the user has denied some of the // scopes requested @@ -429,9 +564,24 @@ public void testSomeRequestedScopesMatchApproval() { long theFuture = System.currentTimeMillis() + (86400 * 7 * 1000); Date nextWeek = new Date(theFuture); - approvalStore.addApproval(new Approval(userId, "foo", "openid", nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.read",nextWeek, APPROVED)); - approvalStore.addApproval(new Approval(userId, "foo", "cloud_controller.write",nextWeek, APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("openid") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.read") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId("foo") + .setScope("cloud_controller.write") + .setExpiresAt(nextWeek) + .setStatus(APPROVED)); // The request is approved because the user has approved all the scopes // requested diff --git a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalTests.java b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalTests.java index 4dd90f21b9..71fb498646 100644 --- a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalTests.java +++ b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalTests.java @@ -16,6 +16,7 @@ import static org.junit.Assert.assertTrue; import java.util.Arrays; +import java.util.Date; import java.util.List; import org.junit.Test; @@ -24,45 +25,181 @@ public class ApprovalTests { @Test public void testHashCode() throws Exception { - assertTrue(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).hashCode() == new Approval("u1", - "c1", "s1", 500, Approval.ApprovalStatus.DENIED).hashCode()); - assertFalse(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).hashCode() == new Approval( - "u1", "c2", "s1", 100, Approval.ApprovalStatus.DENIED).hashCode()); - assertFalse(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).hashCode() == new Approval( - "u1", "c1", "s2", 100, Approval.ApprovalStatus.DENIED).hashCode()); - assertFalse(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).hashCode() == new Approval( - "u2", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).hashCode()); - assertFalse(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).hashCode() == new Approval( - "u1", "c1", "s1", 100, Approval.ApprovalStatus.APPROVED).hashCode()); + assertTrue(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).hashCode() == new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(500)) + .setStatus(Approval.ApprovalStatus.DENIED).hashCode()); + assertFalse(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).hashCode() == new Approval() + .setUserId("u1") + .setClientId("c2") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).hashCode()); + assertFalse(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).hashCode() == new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s2") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).hashCode()); + assertFalse(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).hashCode() == new Approval() + .setUserId("u2") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).hashCode()); + assertFalse(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).hashCode() == new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.APPROVED).hashCode()); } @Test public void testEquals() throws Exception { - assertTrue(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).equals(new Approval("u1", "c1", - "s1", 500, Approval.ApprovalStatus.DENIED))); - assertFalse(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).equals(new Approval("u1", "c2", - "s1", 100, Approval.ApprovalStatus.DENIED))); - assertFalse(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).equals(new Approval("u1", "c1", - "s2", 100, Approval.ApprovalStatus.DENIED))); - assertFalse(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).equals(new Approval("u2", "c1", - "s1", 100, Approval.ApprovalStatus.DENIED))); - assertFalse(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED).equals(new Approval("u1", "c1", - "s1", 100, Approval.ApprovalStatus.APPROVED))); + assertTrue(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).equals(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(500)) + .setStatus(Approval.ApprovalStatus.DENIED))); + assertFalse(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).equals(new Approval() + .setUserId("u1") + .setClientId("c2") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED))); + assertFalse(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).equals(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s2") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED))); + assertFalse(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).equals(new Approval() + .setUserId("u2") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED))); + assertFalse(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED).equals(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.APPROVED))); - List approvals = Arrays.asList(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.APPROVED), - new Approval("u1", "c1", "s2", 100, Approval.ApprovalStatus.APPROVED), - new Approval("u1", "c1", "s3", 100, Approval.ApprovalStatus.APPROVED), - new Approval("u1", "c2", "s1", 100, Approval.ApprovalStatus.APPROVED), - new Approval("u1", "c2", "s2", 100, Approval.ApprovalStatus.DENIED) + List approvals = Arrays.asList(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.APPROVED), + new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s2") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.APPROVED), + new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s3") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.APPROVED), + new Approval() + .setUserId("u1") + .setClientId("c2") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.APPROVED), + new Approval() + .setUserId("u1") + .setClientId("c2") + .setScope("s2") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED) ); - assertTrue(approvals.contains(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.APPROVED))); - assertFalse(approvals.contains(new Approval("u1", "c1", "s1", 100, Approval.ApprovalStatus.DENIED))); + assertTrue(approvals.contains(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.APPROVED))); + assertFalse(approvals.contains(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(100)) + .setStatus(Approval.ApprovalStatus.DENIED))); } @Test public void testExpiry() { int THIRTY_MINTUES = 30 * 60 * 1000; - assertTrue(new Approval("u1", "c1", "s1", THIRTY_MINTUES, Approval.ApprovalStatus.APPROVED).isCurrentlyActive()); - assertFalse(new Approval("u1", "c1", "s1", -1, Approval.ApprovalStatus.APPROVED).isCurrentlyActive()); + assertTrue(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(THIRTY_MINTUES)) + .setStatus(Approval.ApprovalStatus.APPROVED).isCurrentlyActive()); + int expiresIn = -1; + assertFalse(new Approval() + .setUserId("u1") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(expiresIn)) + .setStatus(Approval.ApprovalStatus.APPROVED).isCurrentlyActive()); } } diff --git a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalsAdminEndpointsTests.java b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalsAdminEndpointsTests.java index 7d17b50a27..7dc95dce97 100644 --- a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalsAdminEndpointsTests.java +++ b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/ApprovalsAdminEndpointsTests.java @@ -84,7 +84,12 @@ public void initApprovalsAdminEndpointsTests() { } private void addApproval(String userName, String clientId, String scope, int expiresIn, ApprovalStatus status) { - dao.addApproval(new Approval(userName, clientId, scope, expiresIn, status)); + dao.addApproval(new Approval() + .setUserId(userName) + .setClientId(clientId) + .setScope(scope) + .setExpiresAt(Approval.timeFromNow(expiresIn)) + .setStatus(status)); } private SecurityContextAccessor mockSecurityContextAccessor(String userName, String id) { @@ -116,7 +121,12 @@ public void canGetApprovals() { @Test public void testApprovalsDeserializationIsCaseInsensitive() throws Exception { Set approvals = new HashSet<>(); - approvals.add(new Approval("test-user-id", "testclientid", "scope", new Date(), Approval.ApprovalStatus.APPROVED)); + approvals.add(new Approval() + .setUserId("test-user-id") + .setClientId("testclientid") + .setScope("scope") + .setExpiresAt(new Date()) + .setStatus(ApprovalStatus.APPROVED)); Set deserializedApprovals = JsonUtils.readValue("[{\"userid\":\"test-user-id\",\"clientid\":\"testclientid\",\"scope\":\"scope\",\"status\":\"APPROVED\",\"expiresat\":\"2015-08-25T14:35:42.512Z\",\"lastupdatedat\":\"2015-08-25T14:35:42.512Z\"}]", new TypeReference>() { }); assertEquals(approvals, deserializedApprovals); @@ -143,23 +153,83 @@ public void canUpdateApprovals() { addApproval(marissa.getId(), "c1", "uaa.admin", 12000, DENIED); addApproval(marissa.getId(), "c1", "openid", 6000, APPROVED); - Approval[] app = new Approval[] { new Approval(marissa.getId(), "c1", "uaa.user", 2000, APPROVED), - new Approval(marissa.getId(), "c1", "dash.user", 2000, APPROVED), - new Approval(marissa.getId(), "c1", "openid", 2000, DENIED), - new Approval(marissa.getId(), "c1", "cloud_controller.read", 2000, APPROVED) }; + Approval[] app = new Approval[] {new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("uaa.user") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED), + new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("dash.user") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED), + new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("openid") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(DENIED), + new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("cloud_controller.read") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED)}; List response = endpoints.updateApprovals(app); assertEquals(4, response.size()); - assertTrue(response.contains(new Approval(marissa.getId(), "c1", "uaa.user", 2000, APPROVED))); - assertTrue(response.contains(new Approval(marissa.getId(), "c1", "dash.user", 2000, APPROVED))); - assertTrue(response.contains(new Approval(marissa.getId(), "c1", "openid", 2000, DENIED))); - assertTrue(response.contains(new Approval(marissa.getId(), "c1", "cloud_controller.read", 2000, APPROVED))); + assertTrue(response.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("uaa.user") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED))); + assertTrue(response.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("dash.user") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED))); + assertTrue(response.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("openid") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(DENIED))); + assertTrue(response.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("cloud_controller.read") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED))); List updatedApprovals = endpoints.getApprovals("user_id eq \""+marissa.getId()+"\"", 1, 100); assertEquals(4, updatedApprovals.size()); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "dash.user", 2000, APPROVED))); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "openid", 2000, DENIED))); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "cloud_controller.read", 2000, APPROVED))); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "uaa.user", 2000, APPROVED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("dash.user") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("openid") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(DENIED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("cloud_controller.read") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("uaa.user") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED))); } public void attemptingToCreateDuplicateApprovalsExtendsValidity() { @@ -171,9 +241,24 @@ public void attemptingToCreateDuplicateApprovalsExtendsValidity() { List updatedApprovals = endpoints.getApprovals("user_id eq \""+marissa.getId()+"\"", 1, 100); assertEquals(3, updatedApprovals.size()); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "uaa.user", 6000, APPROVED))); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "uaa.admin", 12000, DENIED))); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "openid", 10000, APPROVED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("uaa.user") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(APPROVED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("uaa.admin") + .setExpiresAt(Approval.timeFromNow(12000)) + .setStatus(DENIED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("openid") + .setExpiresAt(Approval.timeFromNow(10000)) + .setStatus(APPROVED))); } public void attemptingToCreateAnApprovalWithADifferentStatusUpdatesApproval() { @@ -185,9 +270,24 @@ public void attemptingToCreateAnApprovalWithADifferentStatusUpdatesApproval() { List updatedApprovals = endpoints.getApprovals("user_id eq \""+marissa.getId()+"\"", 1, 100); assertEquals(4, updatedApprovals.size()); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "uaa.user", 6000, APPROVED))); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "uaa.admin", 12000, DENIED))); - assertTrue(updatedApprovals.contains(new Approval(marissa.getId(), "c1", "openid", 18000, DENIED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("uaa.user") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(APPROVED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("uaa.admin") + .setExpiresAt(Approval.timeFromNow(12000)) + .setStatus(DENIED))); + assertTrue(updatedApprovals.contains(new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("openid") + .setExpiresAt(Approval.timeFromNow(18000)) + .setStatus(DENIED))); } @Test(expected = UaaException.class) @@ -196,7 +296,12 @@ public void userCannotUpdateApprovalsForAnotherUser() { addApproval(marissa.getId(), "c1", "uaa.admin", 12000, DENIED); addApproval(marissa.getId(), "c1", "openid", 6000, APPROVED); endpoints.setSecurityContextAccessor(mockSecurityContextAccessor("vidya", "123456")); - endpoints.updateApprovals(new Approval[] { new Approval(marissa.getId(), "c1", "uaa.user", 2000, APPROVED) }); + endpoints.updateApprovals(new Approval[] {new Approval() + .setUserId(marissa.getId()) + .setClientId("c1") + .setScope("uaa.user") + .setExpiresAt(Approval.timeFromNow(2000)) + .setStatus(APPROVED)}); } @Test diff --git a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/JdbcApprovalStoreTests.java b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/JdbcApprovalStoreTests.java index f95125bb4a..a11a89c477 100644 --- a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/JdbcApprovalStoreTests.java +++ b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/approval/JdbcApprovalStoreTests.java @@ -59,7 +59,13 @@ public void initJdbcApprovalStoreTests() { private void addApproval(String userName, String clientId, String scope, long expiresIn, ApprovalStatus status) { Date expiresAt = new Timestamp(new Date().getTime() + expiresIn); Date lastUpdatedAt = new Date(); - Approval newApproval = new Approval(userName, clientId, scope, expiresAt, status, lastUpdatedAt); + Approval newApproval = new Approval() + .setUserId(userName) + .setClientId(clientId) + .setScope(scope) + .setExpiresAt(expiresAt) + .setStatus(status) + .setLastUpdatedAt(lastUpdatedAt); dao.addApproval(newApproval); } @@ -79,18 +85,23 @@ public void testAddAndGetApproval() { ApprovalStatus status = APPROVED; Date expiresAt = new Timestamp(new Date().getTime() + expiresIn); - Approval newApproval = new Approval(userName, clientId, scope, expiresAt, status, lastUpdatedAt); + Approval newApproval = new Approval() + .setUserId(userName) + .setClientId(clientId) + .setScope(scope) + .setExpiresAt(expiresAt) + .setStatus(status) + .setLastUpdatedAt(lastUpdatedAt); dao.addApproval(newApproval); List approvals = dao.getApprovals(userName, clientId); - Approval approval = approvals.get(0); - assertEquals(clientId, approval.getClientId()); - assertEquals(userName, approval.getUserId()); - assertEquals(Math.round(expiresAt.getTime() / 1000), Math.round(approval.getExpiresAt().getTime() / 1000)); + assertEquals(clientId, approvals.get(0).getClientId()); + assertEquals(userName, approvals.get(0).getUserId()); + assertEquals(Math.round(expiresAt.getTime() / 1000), Math.round(approvals.get(0).getExpiresAt().getTime() / 1000)); assertEquals(Math.round(lastUpdatedAt.getTime() / 1000), - Math.round(approval.getLastUpdatedAt().getTime() / 1000)); - assertEquals(scope, approval.getScope()); - assertEquals(status, approval.getStatus()); + Math.round(approvals.get(0).getLastUpdatedAt().getTime() / 1000)); + assertEquals(scope, approvals.get(0).getScope()); + assertEquals(status, approvals.get(0).getStatus()); } @Test @@ -103,7 +114,12 @@ public void canGetApprovals() { @Test public void canAddApproval() { - assertTrue(dao.addApproval(new Approval("u2", "c2", "dash.user", 12000, APPROVED))); + assertTrue(dao.addApproval(new Approval() + .setUserId("u2") + .setClientId("c2") + .setScope("dash.user") + .setExpiresAt(Approval.timeFromNow(12000)) + .setStatus(APPROVED))); List apps = dao.getApprovals("u2", "c2"); assertEquals(1, apps.size()); Approval app = apps.iterator().next(); @@ -134,11 +150,21 @@ public void canRevokeSingleApproval() { @Test public void addSameApprovalRepeatedlyUpdatesExpiry() { - assertTrue(dao.addApproval(new Approval("u2", "c2", "dash.user", 6000, APPROVED))); + assertTrue(dao.addApproval(new Approval() + .setUserId("u2") + .setClientId("c2") + .setScope("dash.user") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(APPROVED))); Approval app = dao.getApprovals("u2", "c2").iterator().next(); assertEquals(Math.round(app.getExpiresAt().getTime() / 1000), Math.round((new Date().getTime() + 6000) / 1000)); - assertTrue(dao.addApproval(new Approval("u2", "c2", "dash.user", 8000, APPROVED))); + assertTrue(dao.addApproval(new Approval() + .setUserId("u2") + .setClientId("c2") + .setScope("dash.user") + .setExpiresAt(Approval.timeFromNow(8000)) + .setStatus(APPROVED))); app = dao.getApprovals("u2", "c2").iterator().next(); assertEquals(Math.round(app.getExpiresAt().getTime() / 1000), Math.round((new Date().getTime() + 8000) / 1000)); } @@ -146,11 +172,21 @@ public void addSameApprovalRepeatedlyUpdatesExpiry() { @Test @Ignore //this test has issues public void addSameApprovalDifferentStatusRepeatedlyOnlyUpdatesStatus() { - assertTrue(dao.addApproval(new Approval("u2", "c2", "dash.user", 6000, APPROVED))); + assertTrue(dao.addApproval(new Approval() + .setUserId("u2") + .setClientId("c2") + .setScope("dash.user") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(APPROVED))); Approval app = dao.getApprovals("u2", "c2").iterator().next(); assertEquals(Math.round(app.getExpiresAt().getTime() / 1000), Math.round((new Date().getTime() + 6000) / 1000)); - assertTrue(dao.addApproval(new Approval("u2", "c2", "dash.user", 8000, DENIED))); + assertTrue(dao.addApproval(new Approval() + .setUserId("u2") + .setClientId("c2") + .setScope("dash.user") + .setExpiresAt(Approval.timeFromNow(8000)) + .setStatus(DENIED))); app = dao.getApprovals("u2", "c2").iterator().next(); assertEquals(Math.round(app.getExpiresAt().getTime() / 1000), Math.round((new Date().getTime() + 6000) / 1000)); assertEquals(DENIED, app.getStatus()); @@ -161,7 +197,12 @@ public void canRefreshApproval() { Approval app = dao.getApprovals("u1", "c1").iterator().next(); Date now = new Date(); - dao.refreshApproval(new Approval(app.getUserId(), app.getClientId(), app.getScope(), now, APPROVED)); + dao.refreshApproval(new Approval() + .setUserId(app.getUserId()) + .setClientId(app.getClientId()) + .setScope(app.getScope()) + .setExpiresAt(now) + .setStatus(APPROVED)); app = dao.getApprovals("u1", "c1").iterator().next(); assertEquals(Math.round(now.getTime() / 1000), Math.round(app.getExpiresAt().getTime() / 1000)); } @@ -188,7 +229,12 @@ public void canPurgeExpiredApprovals() throws InterruptedException { public void testAddingAndUpdatingAnApprovalPublishesEvents() throws Exception { UaaTestAccounts testAccounts = UaaTestAccounts.standard(null); - Approval approval = new Approval(testAccounts.getUserName(), "app", "cloud_controller.read", 1000, ApprovalStatus.APPROVED); + Approval approval = new Approval() + .setUserId(testAccounts.getUserName()) + .setClientId("app") + .setScope("cloud_controller.read") + .setExpiresAt(Approval.timeFromNow(1000)) + .setStatus(ApprovalStatus.APPROVED); eventPublisher.clearEvents(); diff --git a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/UaaTokenServicesTests.java b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/UaaTokenServicesTests.java index 4d9b547b48..74e57c205a 100644 --- a/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/UaaTokenServicesTests.java +++ b/common/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/UaaTokenServicesTests.java @@ -451,8 +451,20 @@ private OAuth2AccessToken getOAuth2AccessToken() { Calendar updatedAt = Calendar.getInstance(); updatedAt.add(Calendar.MILLISECOND, -1000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED, updatedAt.getTime())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED, updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); authorizationRequest.setResourceIds(new HashSet<>(resourceIds)); @@ -571,7 +583,13 @@ public void testCreateAccessTokenRefreshGrantSomeScopesAutoApproved() throws Int Calendar updatedAt = Calendar.getInstance(); updatedAt.add(Calendar.MILLISECOND, -1000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); authorizationRequest.setResourceIds(new HashSet<>(resourceIds)); @@ -623,7 +641,13 @@ public void testCreateAccessTokenRefreshGrantNoScopesAutoApprovedIncompleteAppro Calendar updatedAt = Calendar.getInstance(); updatedAt.add(Calendar.MILLISECOND, -1000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); authorizationRequest.setResourceIds(new HashSet<>(resourceIds)); @@ -668,8 +692,20 @@ public void testCreateAccessTokenRefreshGrantAllScopesAutoApprovedButApprovalDen Calendar updatedAt = Calendar.getInstance(); updatedAt.add(Calendar.MILLISECOND, -1000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,updatedAt.getTime())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.DENIED,updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.DENIED) + .setLastUpdatedAt(updatedAt.getTime())); AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); authorizationRequest.setResourceIds(new HashSet<>(resourceIds)); @@ -850,8 +886,20 @@ public void testCreateAccessTokenAuthcodeGrantNarrowerScopes() { Calendar updatedAt = Calendar.getInstance(); updatedAt.add(Calendar.MILLISECOND, -1000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,updatedAt.getTime())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); // First Request AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); @@ -891,8 +939,18 @@ public void testCreateAccessTokenAuthcodeGrantExpandedScopes() { Calendar expiresAt = Calendar.getInstance(); expiresAt.add(Calendar.MILLISECOND, 3000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED, new Date())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED, new Date())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); // First Request AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); authorizationRequest.setResourceIds(new HashSet<>(resourceIds)); @@ -949,8 +1007,18 @@ public void testUserUpdatedAfterRefreshTokenIssued() { Calendar expiresAt = Calendar.getInstance(); expiresAt.add(Calendar.MILLISECOND, 3000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,new Date())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,new Date())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); authorizationRequest.setResourceIds(new HashSet<>(resourceIds)); Map azParameters = new HashMap<>(authorizationRequest.getRequestParameters()); @@ -979,8 +1047,18 @@ public void testRefreshTokenExpiry() { Calendar expiresAt = Calendar.getInstance(); expiresAt.add(Calendar.MILLISECOND, 3000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,new Date())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,new Date())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); BaseClientDetails clientDetails = cloneClient(defaultClient); // Back date the refresh token. Crude way to do this but i'm not sure of @@ -1022,8 +1100,18 @@ public void testRefreshTokenAfterApprovalsChanged() { Calendar expiresAt = Calendar.getInstance(); expiresAt.add(Calendar.MILLISECOND, 3000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,new Date())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,new Date())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); AuthorizationRequest refreshAuthorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); refreshAuthorizationRequest.setResourceIds(new HashSet<>(resourceIds)); @@ -1039,8 +1127,18 @@ public void testRefreshTokenAfterApprovalsExpired() { Calendar expiresAt = Calendar.getInstance(); expiresAt.add(Calendar.MILLISECOND, -3000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,new Date())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,new Date())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); authorizationRequest.setResourceIds(new HashSet<>(resourceIds)); @@ -1066,8 +1164,18 @@ public void testRefreshTokenAfterApprovalsDenied() { Calendar expiresAt = Calendar.getInstance(); expiresAt.add(Calendar.MILLISECOND, -3000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.DENIED, new Date())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,new Date())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.DENIED)); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED)); AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); authorizationRequest.setResourceIds(new HashSet<>(resourceIds)); @@ -1093,7 +1201,12 @@ public void testRefreshTokenAfterApprovalsMissing() { Calendar expiresAt = Calendar.getInstance(); expiresAt.add(Calendar.MILLISECOND, -3000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.DENIED,new Date())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.DENIED)); AuthorizationRequest authorizationRequest = new AuthorizationRequest(CLIENT_ID,requestedAuthScopes); authorizationRequest.setResourceIds(new HashSet<>(resourceIds)); @@ -1149,8 +1262,20 @@ public void testReadAccessToken() { Calendar updatedAt = Calendar.getInstance(); updatedAt.add(Calendar.MILLISECOND, -1000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,updatedAt.getTime())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED,updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); OAuth2Authentication authentication = new OAuth2Authentication(authorizationRequest.createOAuth2Request(), userAuthentication); OAuth2AccessToken accessToken = tokenServices.createAccessToken(authentication); @@ -1171,8 +1296,20 @@ public void testReadAccessTokenForDeletedUserId() { Calendar updatedAt = Calendar.getInstance(); updatedAt.add(Calendar.MILLISECOND, -1000); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, readScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED, updatedAt.getTime())); - approvalStore.addApproval(new Approval(userId, CLIENT_ID, writeScope.get(0), expiresAt.getTime(), ApprovalStatus.APPROVED, updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(readScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); + approvalStore.addApproval(new Approval() + .setUserId(userId) + .setClientId(CLIENT_ID) + .setScope(writeScope.get(0)) + .setExpiresAt(expiresAt.getTime()) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(updatedAt.getTime())); OAuth2Authentication authentication = new OAuth2Authentication(authorizationRequest.createOAuth2Request(), userAuthentication); OAuth2AccessToken accessToken = tokenServices.createAccessToken(authentication); diff --git a/login/src/main/java/org/cloudfoundry/identity/uaa/login/DescribedApproval.java b/login/src/main/java/org/cloudfoundry/identity/uaa/login/DescribedApproval.java index 0320c67ac9..62f88cfef3 100644 --- a/login/src/main/java/org/cloudfoundry/identity/uaa/login/DescribedApproval.java +++ b/login/src/main/java/org/cloudfoundry/identity/uaa/login/DescribedApproval.java @@ -21,8 +21,15 @@ public class DescribedApproval extends Approval { public DescribedApproval() { } + public DescribedApproval(Approval approval) { - super(approval); + this + .setLastUpdatedAt(approval.getLastUpdatedAt()) + .setUserId(approval.getUserId()) + .setStatus(approval.getStatus()) + .setExpiresAt(approval.getExpiresAt()) + .setScope(approval.getScope()) + .setClientId(approval.getClientId()); } @JsonIgnore diff --git a/payload/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/Approval.java b/payload/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/Approval.java index ac052851d9..569ee593cb 100644 --- a/payload/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/Approval.java +++ b/payload/src/main/java/org/cloudfoundry/identity/uaa/oauth/approval/Approval.java @@ -17,6 +17,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import org.cloudfoundry.identity.uaa.impl.JsonDateDeserializer; @@ -27,71 +28,48 @@ @JsonDeserialize(using = ApprovalsJsonDeserializer.class) public class Approval { + public Approval() { + } + public enum ApprovalStatus { APPROVED, - DENIED; + DENIED } - private String userId; + private String userId = ""; - private String clientId; + private String clientId = ""; - private String scope; + private String scope = ""; private ApprovalStatus status; private Date expiresAt; - private Date lastUpdatedAt; - - public Approval(String userId, String clientId, String scope, int expiresIn, ApprovalStatus status) { - this(userId, clientId, scope, new Date(), status, new Date()); - Calendar expiresAt = Calendar.getInstance(); - expiresAt.add(Calendar.MILLISECOND, expiresIn); - setExpiresAt(expiresAt.getTime()); - } - - public Approval(String userId, String clientId, String scope, Date expiresAt, ApprovalStatus status) { - this(userId, clientId, scope, expiresAt, status, new Date()); - } + private Date lastUpdatedAt = new Date(); - public Approval(String userId, String clientId, String scope, Date expiresAt, ApprovalStatus status, - Date lastUpdatedAt) { - this.userId = userId; - this.clientId = clientId; - this.scope = scope; - this.expiresAt = expiresAt; - this.status = status; - this.lastUpdatedAt = lastUpdatedAt; - } - - public Approval() { - } - - public Approval(Approval approval) { - this(approval.getUserId(), - approval.getClientId(), - approval.getScope(), - approval.getExpiresAt(), - approval.getStatus(), - approval.getLastUpdatedAt() - ); + public static Date timeFromNow(int timeTill) { + Calendar timeOf = Calendar.getInstance(); + timeOf.add(Calendar.MILLISECOND, timeTill); + return timeOf.getTime(); } public String getUserId() { return userId; } - public void setUserId(String userId) { + public Approval setUserId(String userId) { this.userId = userId == null ? "" : userId; + return this; } public String getClientId() { return clientId; } - public void setClientId(String clientId) { + public Approval setClientId(String clientId) { this.clientId = clientId == null ? "" : clientId; + return this; } public ApprovalStatus getStatus() { @@ -102,33 +80,39 @@ public String getScope() { return scope; } - public void setScope(String scope) { + public Approval setScope(String scope) { this.scope = scope == null ? "" : scope; + return this; } - @JsonSerialize(using = JsonDateSerializer.class, include = JsonSerialize.Inclusion.NON_NULL) + @JsonSerialize(using = JsonDateSerializer.class) + @JsonProperty("expiresAt") public Date getExpiresAt() { - return expiresAt; - } - - @JsonDeserialize(using = JsonDateDeserializer.class) - public void setExpiresAt(Date expiresAt) { if (expiresAt == null) { Calendar thirtyMinFromNow = Calendar.getInstance(); thirtyMinFromNow.add(Calendar.MINUTE, 30); expiresAt = thirtyMinFromNow.getTime(); } + return expiresAt; + } + + @JsonDeserialize(using = JsonDateDeserializer.class) + @JsonProperty("expiresAt") + public Approval setExpiresAt(Date expiresAt) { this.expiresAt = expiresAt; + return this; } - @JsonSerialize(using = JsonDateSerializer.class, include = JsonSerialize.Inclusion.NON_NULL) + @JsonSerialize(using = JsonDateSerializer.class) public Date getLastUpdatedAt() { return lastUpdatedAt; } @JsonDeserialize(using = JsonDateDeserializer.class) - public void setLastUpdatedAt(Date lastUpdatedAt) { + public Approval setLastUpdatedAt(Date lastUpdatedAt) { + if (lastUpdatedAt == null) throw new IllegalArgumentException("lastUpdatedAt cannot be null"); this.lastUpdatedAt = lastUpdatedAt; + return this; } @JsonIgnore @@ -163,8 +147,9 @@ public String toString() { lastUpdatedAt); } - public void setStatus(ApprovalStatus status) { + public Approval setStatus(ApprovalStatus status) { this.status = status; + return this; } } diff --git a/scim/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsTests.java b/scim/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsTests.java index f2844931ca..1d2dc9057f 100644 --- a/scim/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsTests.java +++ b/scim/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsTests.java @@ -67,6 +67,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -244,8 +245,12 @@ public void groupsIsSyncedCorrectlyOnGet() { public void approvalsIsSyncedCorrectlyOnCreate() { ScimUser user = new ScimUser(null, "vidya", "Vidya", "V"); user.addEmail("vidya@vmware.com"); - user.setApprovals(Collections.singleton(new Approval("vidya", "c1", "s1", 6000, - Approval.ApprovalStatus.APPROVED))); + user.setApprovals(Collections.singleton(new Approval() + .setUserId("vidya") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(Approval.ApprovalStatus.APPROVED))); ScimUser created = endpoints.createUser(user, new MockHttpServletResponse()); assertNotNull(created.getApprovals()); @@ -258,14 +263,32 @@ public void approvalsIsSyncedCorrectlyOnUpdate() { ScimUser user = new ScimUser(null, "vidya", "Vidya", "V"); user.addEmail("vidya@vmware.com"); - user.setApprovals(Collections.singleton(new Approval("vidya", "c1", "s1", 6000, - Approval.ApprovalStatus.APPROVED))); + user.setApprovals(Collections.singleton(new Approval() + .setUserId("vidya") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(Approval.ApprovalStatus.APPROVED))); ScimUser created = endpoints.createUser(user, new MockHttpServletResponse()); - am.addApproval(new Approval(created.getId(), "c1", "s1", 6000, Approval.ApprovalStatus.APPROVED)); - am.addApproval(new Approval(created.getId(), "c1", "s2", 6000, Approval.ApprovalStatus.DENIED)); - - created.setApprovals(Collections.singleton(new Approval("vidya", "c1", "s1", 6000, - Approval.ApprovalStatus.APPROVED))); + am.addApproval(new Approval() + .setUserId(created.getId()) + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(Approval.ApprovalStatus.APPROVED)); + am.addApproval(new Approval() + .setUserId(created.getId()) + .setClientId("c1") + .setScope("s2") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(Approval.ApprovalStatus.DENIED)); + + created.setApprovals(Collections.singleton(new Approval() + .setUserId("vidya") + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(Approval.ApprovalStatus.APPROVED))); ScimUser updated = endpoints.updateUser(created, created.getId(), "*", new MockHttpServletResponse()); assertEquals(2, updated.getApprovals().size()); } @@ -274,8 +297,18 @@ public void approvalsIsSyncedCorrectlyOnUpdate() { public void approvalsIsSyncedCorrectlyOnGet() { assertEquals(0, endpoints.getUser(joel.getId(), new MockHttpServletResponse()).getApprovals().size()); - am.addApproval(new Approval(joel.getId(), "c1", "s1", 6000, Approval.ApprovalStatus.APPROVED)); - am.addApproval(new Approval(joel.getId(), "c1", "s2", 6000, Approval.ApprovalStatus.DENIED)); + am.addApproval(new Approval() + .setUserId(joel.getId()) + .setClientId("c1") + .setScope("s1") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(Approval.ApprovalStatus.APPROVED)); + am.addApproval(new Approval() + .setUserId(joel.getId()) + .setClientId("c1") + .setScope("s2") + .setExpiresAt(Approval.timeFromNow(6000)) + .setStatus(Approval.ApprovalStatus.DENIED)); assertEquals(2, endpoints.getUser(joel.getId(), new MockHttpServletResponse()).getApprovals().size()); } diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java index f28d3e45b9..941cdb5304 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/ClientAdminEndpointsIntegrationTests.java @@ -539,9 +539,27 @@ private Approval[] addApprovals(String token, String clientId) throws Exception Date oneMinuteAgo = new Date(System.currentTimeMillis() - 60000); Date expiresAt = new Date(System.currentTimeMillis() + 60000); Approval[] approvals = new Approval[] { - new Approval(null, clientId, "cloud_controller.read", expiresAt, Approval.ApprovalStatus.APPROVED,oneMinuteAgo), - new Approval(null, clientId, "openid", expiresAt, Approval.ApprovalStatus.APPROVED,oneMinuteAgo), - new Approval(null, clientId, "password.write", expiresAt, Approval.ApprovalStatus.APPROVED,oneMinuteAgo) + new Approval() + .setUserId(null) + .setClientId(clientId) + .setScope("cloud_controller.read") + .setExpiresAt(expiresAt) + .setStatus(Approval.ApprovalStatus.APPROVED) + .setLastUpdatedAt(oneMinuteAgo), + new Approval() + .setUserId(null) + .setClientId(clientId) + .setScope("openid") + .setExpiresAt(expiresAt) + .setStatus(Approval.ApprovalStatus.APPROVED) + .setLastUpdatedAt(oneMinuteAgo), + new Approval() + .setUserId(null) + .setClientId(clientId) + .setScope("password.write") + .setExpiresAt(expiresAt) + .setStatus(Approval.ApprovalStatus.APPROVED) + .setLastUpdatedAt(oneMinuteAgo) }; HttpHeaders headers = getAuthenticatedHeaders(token); diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/audit/AuditCheckMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/audit/AuditCheckMockMvcTests.java index 50da8d6236..252fae5fdf 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/audit/AuditCheckMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/audit/AuditCheckMockMvcTests.java @@ -66,6 +66,7 @@ import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import java.util.Arrays; +import java.util.Date; import java.util.List; import java.util.Map; @@ -487,12 +488,18 @@ public void clientAuthenticationFailureClientNotFound() throws Exception { ClientAuthenticationFailureEvent event1 = (ClientAuthenticationFailureEvent)captor.getAllValues().get(1); assertEquals("login", event1.getClientId()); } + @Test public void testUserApprovalAdded() throws Exception { clientRegistrationService.updateClientDetails(new BaseClientDetails("login", "oauth", "oauth.approvals", "password", "oauth.login")); String marissaToken = testClient.getUserOAuthAccessToken("login", "loginsecret", testUser.getUserName(), testPassword, "oauth.approvals"); - Approval[] approvals = {new Approval(null, "app", "cloud_controller.read", 1000, Approval.ApprovalStatus.APPROVED)}; + Approval[] approvals = {new Approval() + .setUserId(null) + .setClientId("app") + .setScope("cloud_controller.read") + .setExpiresAt(Approval.timeFromNow(1000)) + .setStatus(Approval.ApprovalStatus.APPROVED)}; MockHttpServletRequestBuilder approvalsPut = put("/approvals") .accept(MediaType.APPLICATION_JSON_VALUE) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java index 7ef2145b28..06085b4d7c 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java @@ -253,7 +253,7 @@ public void testCreate_RestrictedClient_Succeeds() throws Exception { getMockMvc().perform(createClientPost).andExpect(status().isOk()); client.setScope(new UaaScopes().getUaaScopes()); - createClientPost = put("/oauth/clients/restricted/"+id) + createClientPost = put("/oauth/clients/restricted/" + id) .header("Authorization", "Bearer " + adminToken) .accept(APPLICATION_JSON) .contentType(APPLICATION_JSON) @@ -321,7 +321,7 @@ public void test_InZone_ClientWrite_Using_ZonesDotClientsDotAdmin() throws Excep client = MockMvcUtils.utils().createClient(getMockMvc(), adminToken, client); client.setClientSecret("secret"); - String zonesClientsAdminToken = MockMvcUtils.utils().getClientOAuthAccessToken(getMockMvc(), client.getClientId(), client.getClientSecret(), "zones."+id+".clients.admin"); + String zonesClientsAdminToken = MockMvcUtils.utils().getClientOAuthAccessToken(getMockMvc(), client.getClientId(), client.getClientSecret(), "zones." + id + ".clients.admin"); BaseClientDetails newclient = new BaseClientDetails(clientId, "", "openid","authorization_code",""); newclient.setClientSecret("secret"); @@ -740,11 +740,11 @@ public void testModifyApprovalsAreDeleted() throws Exception { ClientDetails approvalsClient = createApprovalsLoginClient(adminToken); String loginToken = testClient.getUserOAuthAccessToken( - approvalsClient.getClientId(), - "secret", - testUser.getUserName(), - testPassword, - "oauth.approvals"); + approvalsClient.getClientId(), + "secret", + testUser.getUserName(), + testPassword, + "oauth.approvals"); approvals = getApprovals(loginToken, details.getClientId()); assertEquals(0, approvals.length); } @@ -1133,7 +1133,7 @@ public void testCreateAsWritePermissions() throws Exception { ClientDetails adminsClient = createReadWriteClient(adminToken); //create clients - ClientDetailsModification[] clients = createBaseClients(1, Arrays.asList("client_credentials","refresh_token")); + ClientDetailsModification[] clients = createBaseClients(1, Arrays.asList("client_credentials", "refresh_token")); for (ClientDetailsModification c : clients) { c.setScope(Collections.singletonList("oauth.approvals")); c.setAction(c.ADD); @@ -1221,9 +1221,27 @@ private Approval[] addApprovals(String token, String clientId) throws Exception Date oneMinuteAgo = new Date(System.currentTimeMillis() - 60000); Date expiresAt = new Date(System.currentTimeMillis() + 60000); Approval[] approvals = new Approval[] { - new Approval(null, clientId, "cloud_controller.read", expiresAt, ApprovalStatus.APPROVED,oneMinuteAgo), - new Approval(null, clientId, "openid", expiresAt, ApprovalStatus.APPROVED,oneMinuteAgo), - new Approval(null, clientId, "password.write", expiresAt, ApprovalStatus.APPROVED,oneMinuteAgo)}; + new Approval() + .setUserId(null) + .setClientId(clientId) + .setScope("cloud_controller.read") + .setExpiresAt(expiresAt) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(oneMinuteAgo), + new Approval() + .setUserId(null) + .setClientId(clientId) + .setScope("openid") + .setExpiresAt(expiresAt) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(oneMinuteAgo), + new Approval() + .setUserId(null) + .setClientId(clientId) + .setScope("password.write") + .setExpiresAt(expiresAt) + .setStatus(ApprovalStatus.APPROVED) + .setLastUpdatedAt(oneMinuteAgo)}; MockHttpServletRequestBuilder put = put("/approvals/"+clientId) .header("Authorization", "Bearer " + token) @@ -1263,8 +1281,8 @@ private ClientDetails getClient(String id) throws Exception { } private ClientDetails createClientAdminsClient(String token) throws Exception { - List scopes = Arrays.asList("oauth.approvals","clients.admin"); - BaseClientDetails client = createBaseClient(null, Arrays.asList("password","client_credentials"), scopes, scopes); + List scopes = Arrays.asList("oauth.approvals", "clients.admin"); + BaseClientDetails client = createBaseClient(null, Arrays.asList("password", "client_credentials"), scopes, scopes); MockHttpServletRequestBuilder createClientPost = post("/oauth/clients") .header("Authorization", "Bearer " + token) .accept(APPLICATION_JSON) @@ -1316,7 +1334,7 @@ private ClientDetails createApprovalsLoginClient(String token) throws Exception private ClientDetailsModification createBaseClient(String id, Collection grantTypes) { - return createBaseClient(id, grantTypes, Collections.singletonList("uaa.none"), Arrays.asList("foo","bar","oauth.approvals")); + return createBaseClient(id, grantTypes, Collections.singletonList("uaa.none"), Arrays.asList("foo", "bar", "oauth.approvals")); } private ClientDetailsModification createBaseClient(String id, Collection grantTypes, List authorities, List scopes) {