Use zone filtering in approval store

[#142245925] https://www.pivotaltracker.com/story/show/142245925
cloudfoundry · Mar 22, 2017 · 50853fc · 50853fc
1 parent 76fdfbb
commit 50853fc
Show file tree

Hide file tree

Showing 6 changed files with 237 additions and 111 deletions.
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/approval/JdbcApprovalStore.java b/server/src/main/java/org/cloudfoundry/identity/uaa/approval/JdbcApprovalStore.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -12,23 +12,14 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.approval;
 
-import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
-
-import java.sql.PreparedStatement;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.sql.Timestamp;
-import java.util.Date;
-import java.util.List;
-import java.util.Map;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.cloudfoundry.identity.uaa.audit.event.ApprovalModifiedEvent;
 import org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus;
+import org.cloudfoundry.identity.uaa.audit.event.ApprovalModifiedEvent;
 import org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory;
 import org.cloudfoundry.identity.uaa.resources.jdbc.SearchQueryConverter;
 import org.cloudfoundry.identity.uaa.resources.jdbc.SearchQueryConverter.ProcessedFilter;
+import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
 import org.springframework.context.ApplicationEvent;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.ApplicationEventPublisherAware;
@@ -43,6 +34,17 @@
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.util.Assert;
 
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Timestamp;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
+
 public class JdbcApprovalStore implements ApprovalStore, ApplicationEventPublisherAware {
 
     private final JdbcTemplate jdbcTemplate;
@@ -143,17 +145,17 @@ public boolean revokeApprovals(String filter) {
         logger.debug(String.format("Filtering approvals with filter: [%s]", where));
 
         String sql;
-        Map<String, Object> sqlParams;
+        Map<String, Object> sqlParams = new HashMap<>(where.getParams());
         if (handleRevocationsAsExpiry) {
             // just expire all approvals matching the filter
             sql = EXPIRE_AUTHZ_SQL + " where " + where.getSql();
-            sqlParams = where.getParams();
             sqlParams.put("expiry", new Timestamp(new Date().getTime() - 1));
         } else {
             // delete the records
             sql = DELETE_AUTHZ_SQL + " where " + where.getSql();
-            sqlParams = where.getParams();
         }
+        sqlParams.put("__identity_zone_id", IdentityZoneHolder.get().getId());
+        sql = sql + " and user_id in (select id from users where identity_zone_id = :__identity_zone_id)";
 
         try {
             int revoked = new NamedParameterJdbcTemplate(jdbcTemplate).update(sql, sqlParams);
@@ -185,8 +187,14 @@ public List<Approval> getApprovals(String filter) {
         ProcessedFilter where = queryConverter.convert(filter, null, true);
         logger.debug(String.format("Filtering approvals with filter: [%s]", where));
         try {
-            return pagingListFactory.createJdbcPagingList(GET_AUTHZ_SQL + " where " +
-                            where.getSql(), where.getParams(), rowMapper, 200);
+            Map<String, Object> params = new HashMap(where.getParams());
+            params.put("__identity_zone_id", IdentityZoneHolder.get().getId());
+            return pagingListFactory.createJdbcPagingList(
+                GET_AUTHZ_SQL + " where " + where.getSql() + " and user_id in (select id from users where identity_zone_id = :__identity_zone_id)",
+                params,
+                rowMapper,
+                200
+            );
         } catch (DataAccessException e) {
             logger.error("Error filtering approvals with filter: " + where, e);
             throw new IllegalArgumentException("Invalid filter: " + filter);

diff --git a/...c/test/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandlerTests.java b/...c/test/java/org/cloudfoundry/identity/uaa/oauth/UserManagedAuthzApprovalHandlerTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -12,22 +12,6 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.oauth;
 
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashSet;
-import java.util.Set;
-
-import static java.util.Collections.singleton;
-import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
-import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
-import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-
 import org.cloudfoundry.identity.uaa.approval.Approval;
 import org.cloudfoundry.identity.uaa.approval.ApprovalStore;
 import org.cloudfoundry.identity.uaa.approval.JdbcApprovalStore;
@@ -49,6 +33,22 @@
 import org.springframework.security.oauth2.provider.ClientDetails;
 import org.springframework.security.oauth2.provider.client.BaseClientDetails;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.Set;
+
+import static java.util.Collections.singleton;
+import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
+import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+
 public class UserManagedAuthzApprovalHandlerTests extends JdbcTestBase {
 
     private final UserManagedAuthzApprovalHandler handler = new UserManagedAuthzApprovalHandler();
@@ -70,17 +70,18 @@ public void initUserManagedAuthzApprovalHandlerTests() {
         handler.setApprovalStore(approvalStore);
         handler.setClientDetailsService(
             mockClientDetailsService(
-                "foo", 
+                "foo",
                 new String[]{
                     "cloud_controller.read",
-                    "cloud_controller.write", 
+                    "cloud_controller.write",
                     "openid",
                     "space.*.developer"
-                }, 
+                },
                 Collections.emptySet()
             )
         );
         userId = new RandomValueStringGenerator().generate();
+        testAccounts.addRandomUser(jdbcTemplate, userId);
         userAuthentication = new TestAuthentication(userId, testAccounts.getUserName(), true);
     }
 
@@ -106,7 +107,7 @@ public void testNoScopeApproval() {
     @Test
     public void testNoPreviouslyApprovedScopes() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(
                 Arrays.asList("cloud_controller.read", "cloud_controller.write")
             )
@@ -121,7 +122,7 @@ public void testNoPreviouslyApprovedScopes() {
     @Test
     public void testAuthzApprovedButNoPreviouslyApprovedScopes() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(
                 Arrays.asList("cloud_controller.read", "cloud_controller.write")
             )
@@ -162,7 +163,7 @@ public void testNoRequestedScopesButSomeApprovedScopes() {
     @Test
     public void testRequestedScopesDontMatchApprovalsAtAll() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(
                 Arrays.asList("openid")
             )
@@ -193,7 +194,7 @@ public void testRequestedScopesDontMatchApprovalsAtAll() {
     @Test
     public void testOnlySomeRequestedScopeMatchesApproval() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(
                 Arrays.asList("openid", "cloud_controller.read")
             )
@@ -224,7 +225,7 @@ public void testOnlySomeRequestedScopeMatchesApproval() {
     @Test
     public void testOnlySomeRequestedScopeMatchesDeniedApprovalButScopeAutoApproved() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(
                 Arrays.asList("openid", "cloud_controller.read")
             )
@@ -236,10 +237,10 @@ public void testOnlySomeRequestedScopeMatchesDeniedApprovalButScopeAutoApproved(
 
         handler.setClientDetailsService(
             mockClientDetailsService(
-                "foo", 
+                "foo",
                 new String[]{
                     "cloud_controller.read",
-                    "cloud_controller.write", 
+                    "cloud_controller.write",
                     "openid"
                     },
                 singleton("true")
@@ -266,11 +267,11 @@ public void testOnlySomeRequestedScopeMatchesDeniedApprovalButScopeAutoApproved(
     @Test
     public void testRequestedScopesMatchApprovalButAdditionalScopesRequested() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(
                 Arrays.asList(
-                    "openid", 
-                    "cloud_controller.read", 
+                    "openid",
+                    "cloud_controller.read",
                     "cloud_controller.write"
                 )
             )
@@ -301,11 +302,11 @@ public void testRequestedScopesMatchApprovalButAdditionalScopesRequested() {
     @Test
     public void testAllRequestedScopesMatchApproval() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(
                 Arrays.asList(
-                    "openid", 
-                    "cloud_controller.read", 
+                    "openid",
+                    "cloud_controller.read",
                     "cloud_controller.write"
                 )
             )
@@ -342,11 +343,11 @@ public void testAllRequestedScopesMatchApproval() {
     @Test
     public void testRequestedScopesMatchApprovalButSomeDenied() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(
                 Arrays.asList(
-                    "openid", 
-                    "cloud_controller.read", 
+                    "openid",
+                    "cloud_controller.read",
                     "cloud_controller.write"
                 )
             )
@@ -383,11 +384,11 @@ public void testRequestedScopesMatchApprovalButSomeDenied() {
     @Test
     public void testRequestedScopesMatchApprovalSomeDeniedButDeniedScopesAutoApproved() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(
                 Arrays.asList(
-                    "openid", 
-                    "cloud_controller.read", 
+                    "openid",
+                    "cloud_controller.read",
                     "cloud_controller.write"
                 )
             )
@@ -400,7 +401,7 @@ public void testRequestedScopesMatchApprovalSomeDeniedButDeniedScopesAutoApprove
             "foo",
             new String[]{
                 "cloud_controller.read",
-                "cloud_controller.write", 
+                "cloud_controller.write",
                 "openid"
             },
             singleton("cloud_controller.write")));
@@ -561,7 +562,7 @@ public void testRequestedScopesMatchByWildcard() {
     @Test
     public void testSomeRequestedScopesMatchApproval() {
         AuthorizationRequest request = new AuthorizationRequest(
-            "foo", 
+            "foo",
             new HashSet<>(Arrays.asList("openid"))
         );
         request.setApproved(false);