You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by kjduling <gi...@git.apache.org> on 2016/10/28 20:30:59 UTC

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

GitHub user kjduling opened a pull request:

    https://github.com/apache/incubator-geode/pull/276

    GEODE-1993: postprocess region/key

    Add post processing to the GET {region}/{key..key} endpoint
    precheckin running

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kjduling/incubator-geode feature/GEODE-1993

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-geode/pull/276.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #276
    
----
commit 41559682c95f844018fa285f35a1adf2f7fcf0ba
Author: Kevin Duling <kd...@pivotal.io>
Date:   2016-10-28T20:27:56Z

    GEODE-1993: postprocess region/key

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on the issue:

    https://github.com/apache/incubator-geode/pull/276
  
    A few changes requested:
    
    1. use Autowire to auto wire the RestSecurityService.
    2. In your RestSecurityPostProcessorTest, considering only using "customers" region for readability. You don't needs all these "/" + REGION_NAME +"/key1" at all.
    3. Then you can use a customer domain object specific PostProcessor to change the field values and assert for the value returned.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86836287
  
    --- Diff: geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java ---
    @@ -87,6 +89,12 @@
       Object postProcess(Object principal, String regionPath, Object key, Object value,
           boolean valueIsSerialized);
     
    +  Collection<Object> postProcess(Query query, Collection<String> regionNames,
    +      Collection<Object> results);
    +
    +  Collection<Object> postProcess(Object principal, Query query, Collection<String> regionNames,
    --- End diff --
    
    remove this


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on the issue:

    https://github.com/apache/incubator-geode/pull/276
  
    Precheckin successful except for one flaky test, QueueCommandsDUnitTest.testCreateUpdatesSharedConfig which doesn't appear to be related to this change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86821510
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java ---
    @@ -0,0 +1,185 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
    +import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getCode;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getContentType;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonArray;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonObject;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import org.apache.commons.io.IOUtils;
    +import org.apache.geode.cache.Region;
    +import org.apache.geode.cache.RegionShortcut;
    +import org.apache.geode.cache.execute.FunctionService;
    +import org.apache.geode.internal.AvailablePortHelper;
    +import org.apache.geode.rest.internal.web.controllers.GetRegions;
    +import org.apache.geode.security.templates.SamplePostProcessor;
    +import org.apache.geode.security.templates.SampleSecurityManager;
    +import org.apache.geode.test.dunit.rules.ServerStarterRule;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +import org.apache.geode.test.junit.categories.SecurityTest;
    +import org.apache.http.HttpResponse;
    +import org.json.JSONArray;
    +import org.json.JSONObject;
    +import org.junit.BeforeClass;
    +import org.junit.ClassRule;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +import org.springframework.http.MediaType;
    +
    +import java.net.URLEncoder;
    +import java.util.Properties;
    +
    +
    +@Category({IntegrationTest.class, SecurityTest.class})
    +public class RestSecurityPostProcessorTest {
    +
    +  static final String REGION_NAME = "AuthRegion";
    +
    +  static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
    +  static Properties properties = new Properties() {
    +    {
    +      setProperty(SampleSecurityManager.SECURITY_JSON,
    +          "org/apache/geode/management/internal/security/clientServer.json");
    +      setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
    +      setProperty(START_DEV_REST_API, "true");
    +      setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
    +      setProperty(HTTP_SERVICE_PORT, restPort + "");
    +      setProperty(SECURITY_POST_PROCESSOR, SamplePostProcessor.class.getName());
    +    }
    +  };
    +
    +  @ClassRule
    +  public static ServerStarterRule serverStarter = new ServerStarterRule(properties);
    +  private final GeodeRestClient restClient = new GeodeRestClient("localhost", restPort);
    +
    +  @BeforeClass
    +  public static void before() throws Exception {
    +    serverStarter.startServer();
    --- End diff --
    
    I think I just made a change in the rule that you don't need to call startServer anymore if you are using is as a rule.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86920671
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java ---
    @@ -0,0 +1,185 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
    +import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getCode;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getContentType;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonArray;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonObject;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import org.apache.commons.io.IOUtils;
    +import org.apache.geode.cache.Region;
    +import org.apache.geode.cache.RegionShortcut;
    +import org.apache.geode.cache.execute.FunctionService;
    +import org.apache.geode.internal.AvailablePortHelper;
    +import org.apache.geode.rest.internal.web.controllers.GetRegions;
    +import org.apache.geode.security.templates.SamplePostProcessor;
    +import org.apache.geode.security.templates.SampleSecurityManager;
    +import org.apache.geode.test.dunit.rules.ServerStarterRule;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +import org.apache.geode.test.junit.categories.SecurityTest;
    +import org.apache.http.HttpResponse;
    +import org.json.JSONArray;
    +import org.json.JSONObject;
    +import org.junit.BeforeClass;
    +import org.junit.ClassRule;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +import org.springframework.http.MediaType;
    +
    +import java.net.URLEncoder;
    +import java.util.Properties;
    +
    +
    +@Category({IntegrationTest.class, SecurityTest.class})
    +public class RestSecurityPostProcessorTest {
    +
    +  static final String REGION_NAME = "AuthRegion";
    +
    +  static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
    +  static Properties properties = new Properties() {
    +    {
    +      setProperty(SampleSecurityManager.SECURITY_JSON,
    +          "org/apache/geode/management/internal/security/clientServer.json");
    +      setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
    +      setProperty(START_DEV_REST_API, "true");
    +      setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
    +      setProperty(HTTP_SERVICE_PORT, restPort + "");
    +      setProperty(SECURITY_POST_PROCESSOR, SamplePostProcessor.class.getName());
    +    }
    +  };
    +
    +  @ClassRule
    +  public static ServerStarterRule serverStarter = new ServerStarterRule(properties);
    +  private final GeodeRestClient restClient = new GeodeRestClient("localhost", restPort);
    +
    +  @BeforeClass
    +  public static void before() throws Exception {
    +    serverStarter.startServer();
    +    Region region =
    +        serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME);
    +    region.put("key1",
    +        "{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1012,\"description\":\"Order for  XYZ Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/20/2014\",\"contact\":\"Jelly Bean\",\"email\":\"jelly.bean@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}");
    +    region.put("key2", "bar");
    +    serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create("customers");
    +    FunctionService.registerFunction(new GetRegions());
    +  }
    +
    +  /**
    +   * Test post-processing of a retrieved key from the server.
    +   */
    +  @Test
    +  public void getRegionKey() throws Exception {
    +
    +    // Test a single key
    +    HttpResponse response = restClient.doGet("/" + REGION_NAME + "/key1", "key1User", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    String body = IOUtils.toString(response.getEntity().getContent(), "UTF-8");
    +    assertTrue(body.startsWith("\"key1User/" + REGION_NAME + "/key1/"));
    --- End diff --
    
    Please verify if without the postProcessor, what  it is returning? Does it still contain the quotes? This might be because you are using SamplePostProcessor which turns any object into a String result. Consider use a different post processor, that is specific to your "customers" region data.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86900842
  
    --- Diff: geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java ---
    @@ -87,6 +89,12 @@
       Object postProcess(Object principal, String regionPath, Object key, Object value,
           boolean valueIsSerialized);
     
    +  Collection<Object> postProcess(Query query, Collection<String> regionNames,
    +      Collection<Object> results);
    +
    +  Collection<Object> postProcess(Object principal, Query query, Collection<String> regionNames,
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86822375
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java ---
    @@ -0,0 +1,185 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
    +import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getCode;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getContentType;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonArray;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonObject;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import org.apache.commons.io.IOUtils;
    +import org.apache.geode.cache.Region;
    +import org.apache.geode.cache.RegionShortcut;
    +import org.apache.geode.cache.execute.FunctionService;
    +import org.apache.geode.internal.AvailablePortHelper;
    +import org.apache.geode.rest.internal.web.controllers.GetRegions;
    +import org.apache.geode.security.templates.SamplePostProcessor;
    +import org.apache.geode.security.templates.SampleSecurityManager;
    +import org.apache.geode.test.dunit.rules.ServerStarterRule;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +import org.apache.geode.test.junit.categories.SecurityTest;
    +import org.apache.http.HttpResponse;
    +import org.json.JSONArray;
    +import org.json.JSONObject;
    +import org.junit.BeforeClass;
    +import org.junit.ClassRule;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +import org.springframework.http.MediaType;
    +
    +import java.net.URLEncoder;
    +import java.util.Properties;
    +
    +
    +@Category({IntegrationTest.class, SecurityTest.class})
    +public class RestSecurityPostProcessorTest {
    +
    +  static final String REGION_NAME = "AuthRegion";
    +
    +  static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
    +  static Properties properties = new Properties() {
    +    {
    +      setProperty(SampleSecurityManager.SECURITY_JSON,
    +          "org/apache/geode/management/internal/security/clientServer.json");
    +      setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
    +      setProperty(START_DEV_REST_API, "true");
    +      setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
    +      setProperty(HTTP_SERVICE_PORT, restPort + "");
    +      setProperty(SECURITY_POST_PROCESSOR, SamplePostProcessor.class.getName());
    +    }
    +  };
    +
    +  @ClassRule
    +  public static ServerStarterRule serverStarter = new ServerStarterRule(properties);
    +  private final GeodeRestClient restClient = new GeodeRestClient("localhost", restPort);
    +
    +  @BeforeClass
    +  public static void before() throws Exception {
    +    serverStarter.startServer();
    +    Region region =
    +        serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME);
    +    region.put("key1",
    +        "{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1012,\"description\":\"Order for  XYZ Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/20/2014\",\"contact\":\"Jelly Bean\",\"email\":\"jelly.bean@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}");
    +    region.put("key2", "bar");
    +    serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create("customers");
    +    FunctionService.registerFunction(new GetRegions());
    +  }
    +
    +  /**
    +   * Test post-processing of a retrieved key from the server.
    +   */
    +  @Test
    +  public void getRegionKey() throws Exception {
    +
    +    // Test a single key
    +    HttpResponse response = restClient.doGet("/" + REGION_NAME + "/key1", "key1User", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    String body = IOUtils.toString(response.getEntity().getContent(), "UTF-8");
    +    assertTrue(body.startsWith("\"key1User/" + REGION_NAME + "/key1/"));
    +
    +    // Test multiple keys
    +    response = restClient.doGet("/" + REGION_NAME + "/key1,key2", "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    JSONObject jsonObject = getJsonObject(response);
    +    JSONArray jsonArray = jsonObject.getJSONArray(REGION_NAME);
    +    final int length = jsonArray.length();
    +    for (int index = 0; index < length; ++index) {
    +      String data = jsonArray.getString(index);
    +      assertTrue(data.contains("dataReader/" + REGION_NAME + "/"));
    +    }
    +  }
    +
    +  @Test
    +  public void getRegion() throws Exception {
    +    HttpResponse response = restClient.doGet("/" + REGION_NAME, "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    JSONObject jsonObject = getJsonObject(response);
    +    JSONArray jsonArray = jsonObject.getJSONArray(REGION_NAME);
    +    final int length = jsonArray.length();
    +    for (int index = 0; index < length; ++index) {
    +      String data = jsonArray.getString(index);
    +      assertTrue(data.contains("dataReader/" + REGION_NAME + "/"));
    +    }
    +  }
    +
    +  @Test
    +  public void adhocQuery() throws Exception {
    +    String query =
    +        "/queries/adhoc?q=" + URLEncoder.encode("SELECT * FROM /" + REGION_NAME, "UTF-8");
    +    HttpResponse response = restClient.doGet(query, "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    JSONArray jsonArray = getJsonArray(response);
    +    final int length = jsonArray.length();
    +    for (int index = 0; index < length; ++index) {
    +      String data = jsonArray.getString(index);
    +      assertTrue(data.contains("dataReader/" + REGION_NAME + "/"));
    +    }
    +  }
    +
    +  @Test
    +  public void namedQuery() throws Exception {
    +    String namedQuery = "SELECT c FROM /customers c WHERE c.customerId = $1";
    +
    +    HttpResponse response =
    +        restClient.doPost("/queries?id=selectCustomer&q=" + URLEncoder.encode(namedQuery, "UTF-8"),
    +            "dataReader", "1234567", "");
    +    assertEquals(201, getCode(response));
    +
    +    String query = "/queries";
    +    response = restClient.doGet(query, "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    String customerJson =
    --- End diff --
    
    use java api to put the data in the regions instead of using rest api.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86836374
  
    --- Diff: geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java ---
    @@ -409,6 +435,34 @@ public Object postProcess(Object principal, String regionPath, Object key, Objec
         return newValue;
       }
     
    +  private Object getPrincipal(Object principal) {
    +    if (principal == null) {
    +      Subject subject = getSubject();
    +      if (subject == null)
    +        return null;
    +      principal = subject.getPrincipal();
    +    }
    +    return principal;
    +  }
    +
    +  @Override
    +  public Collection<Object> postProcess(Query query, Collection<String> regionNames,
    --- End diff --
    
    remove this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on the issue:

    https://github.com/apache/incubator-geode/pull/276
  
    Don't accept this PR yet.  This is just a checkin to preserve work.
    
    Addressed all of the issues listed above.  Added test for adhoc queries.  Still need a test for named queries.  Also need to verify functions won't return any region data without being post-processed.
    
    Have not run precheckin yet, either.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86847492
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java ---
    @@ -0,0 +1,185 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
    +import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getCode;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getContentType;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonArray;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonObject;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import org.apache.commons.io.IOUtils;
    +import org.apache.geode.cache.Region;
    +import org.apache.geode.cache.RegionShortcut;
    +import org.apache.geode.cache.execute.FunctionService;
    +import org.apache.geode.internal.AvailablePortHelper;
    +import org.apache.geode.rest.internal.web.controllers.GetRegions;
    +import org.apache.geode.security.templates.SamplePostProcessor;
    +import org.apache.geode.security.templates.SampleSecurityManager;
    +import org.apache.geode.test.dunit.rules.ServerStarterRule;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +import org.apache.geode.test.junit.categories.SecurityTest;
    +import org.apache.http.HttpResponse;
    +import org.json.JSONArray;
    +import org.json.JSONObject;
    +import org.junit.BeforeClass;
    +import org.junit.ClassRule;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +import org.springframework.http.MediaType;
    +
    +import java.net.URLEncoder;
    +import java.util.Properties;
    +
    +
    +@Category({IntegrationTest.class, SecurityTest.class})
    +public class RestSecurityPostProcessorTest {
    +
    +  static final String REGION_NAME = "AuthRegion";
    +
    +  static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
    +  static Properties properties = new Properties() {
    +    {
    +      setProperty(SampleSecurityManager.SECURITY_JSON,
    +          "org/apache/geode/management/internal/security/clientServer.json");
    +      setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
    +      setProperty(START_DEV_REST_API, "true");
    +      setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
    +      setProperty(HTTP_SERVICE_PORT, restPort + "");
    +      setProperty(SECURITY_POST_PROCESSOR, SamplePostProcessor.class.getName());
    +    }
    +  };
    +
    +  @ClassRule
    +  public static ServerStarterRule serverStarter = new ServerStarterRule(properties);
    +  private final GeodeRestClient restClient = new GeodeRestClient("localhost", restPort);
    +
    +  @BeforeClass
    +  public static void before() throws Exception {
    +    serverStarter.startServer();
    +    Region region =
    +        serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME);
    +    region.put("key1",
    +        "{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1012,\"description\":\"Order for  XYZ Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/20/2014\",\"contact\":\"Jelly Bean\",\"email\":\"jelly.bean@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}");
    --- End diff --
    
    True.  The tests don't use the object, so I'll remove it for clarity.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on the issue:

    https://github.com/apache/incubator-geode/pull/276
  
    A few things:
    1. In your RestSecurityPostProcessorTest, in the before method, you can get ahold of the region you created and pre populate the regions with a few keys and values, this way, you don't need to worry about putting in values in your tests.
    2. Looks like /{regions} end point also retrieves the values of the entire region, you need to post process the values there as well. 
    3. query end points definitely return region values as well.
    4. SecurityUtils.getSecurityService will definitely get the security service, but let's do it the spring way, I have a component called RestSecurityService, use that instead.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86920722
  
    --- Diff: geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java ---
    @@ -50,6 +52,11 @@
     
       protected static final String REST_API_VERSION = "/v1";
     
    +  public BaseControllerAdvice(final RestSecurityService securityService,
    --- End diff --
    
    use Autowire inside AbstractBaseController to auto wire the RestSecurityService instead of using constructors. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on the issue:

    https://github.com/apache/incubator-geode/pull/276
  
    precheckin running


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86900849
  
    --- Diff: geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java ---
    @@ -87,6 +89,12 @@
       Object postProcess(Object principal, String regionPath, Object key, Object value,
           boolean valueIsSerialized);
     
    +  Collection<Object> postProcess(Query query, Collection<String> regionNames,
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on the issue:

    https://github.com/apache/incubator-geode/pull/276
  
    precheckin successful


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86836714
  
    --- Diff: geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java ---
    @@ -50,6 +52,11 @@
     
       protected static final String REST_API_VERSION = "/v1";
     
    +  public BaseControllerAdvice(final RestSecurityService securityService,
    --- End diff --
    
    do we need this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on the issue:

    https://github.com/apache/incubator-geode/pull/276
  
    Precheckin successful


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86848834
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java ---
    @@ -0,0 +1,185 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
    +import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getCode;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getContentType;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonArray;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonObject;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import org.apache.commons.io.IOUtils;
    +import org.apache.geode.cache.Region;
    +import org.apache.geode.cache.RegionShortcut;
    +import org.apache.geode.cache.execute.FunctionService;
    +import org.apache.geode.internal.AvailablePortHelper;
    +import org.apache.geode.rest.internal.web.controllers.GetRegions;
    +import org.apache.geode.security.templates.SamplePostProcessor;
    +import org.apache.geode.security.templates.SampleSecurityManager;
    +import org.apache.geode.test.dunit.rules.ServerStarterRule;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +import org.apache.geode.test.junit.categories.SecurityTest;
    +import org.apache.http.HttpResponse;
    +import org.json.JSONArray;
    +import org.json.JSONObject;
    +import org.junit.BeforeClass;
    +import org.junit.ClassRule;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +import org.springframework.http.MediaType;
    +
    +import java.net.URLEncoder;
    +import java.util.Properties;
    +
    +
    +@Category({IntegrationTest.class, SecurityTest.class})
    +public class RestSecurityPostProcessorTest {
    +
    +  static final String REGION_NAME = "AuthRegion";
    +
    +  static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
    +  static Properties properties = new Properties() {
    +    {
    +      setProperty(SampleSecurityManager.SECURITY_JSON,
    +          "org/apache/geode/management/internal/security/clientServer.json");
    +      setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
    +      setProperty(START_DEV_REST_API, "true");
    +      setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
    +      setProperty(HTTP_SERVICE_PORT, restPort + "");
    +      setProperty(SECURITY_POST_PROCESSOR, SamplePostProcessor.class.getName());
    +    }
    +  };
    +
    +  @ClassRule
    +  public static ServerStarterRule serverStarter = new ServerStarterRule(properties);
    +  private final GeodeRestClient restClient = new GeodeRestClient("localhost", restPort);
    +
    +  @BeforeClass
    +  public static void before() throws Exception {
    +    serverStarter.startServer();
    +    Region region =
    +        serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME);
    +    region.put("key1",
    +        "{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1012,\"description\":\"Order for  XYZ Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/20/2014\",\"contact\":\"Jelly Bean\",\"email\":\"jelly.bean@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}");
    +    region.put("key2", "bar");
    +    serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create("customers");
    +    FunctionService.registerFunction(new GetRegions());
    +  }
    +
    +  /**
    +   * Test post-processing of a retrieved key from the server.
    +   */
    +  @Test
    +  public void getRegionKey() throws Exception {
    +
    +    // Test a single key
    +    HttpResponse response = restClient.doGet("/" + REGION_NAME + "/key1", "key1User", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    String body = IOUtils.toString(response.getEntity().getContent(), "UTF-8");
    +    assertTrue(body.startsWith("\"key1User/" + REGION_NAME + "/key1/"));
    --- End diff --
    
    Because PdxBasedCrudController.read(region, keys, boolean) returns the value as a quoted string.  E.g., "key1User/AuthRegion/key1/key1Value"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86822046
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java ---
    @@ -0,0 +1,185 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
    +import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getCode;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getContentType;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonArray;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonObject;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import org.apache.commons.io.IOUtils;
    +import org.apache.geode.cache.Region;
    +import org.apache.geode.cache.RegionShortcut;
    +import org.apache.geode.cache.execute.FunctionService;
    +import org.apache.geode.internal.AvailablePortHelper;
    +import org.apache.geode.rest.internal.web.controllers.GetRegions;
    +import org.apache.geode.security.templates.SamplePostProcessor;
    +import org.apache.geode.security.templates.SampleSecurityManager;
    +import org.apache.geode.test.dunit.rules.ServerStarterRule;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +import org.apache.geode.test.junit.categories.SecurityTest;
    +import org.apache.http.HttpResponse;
    +import org.json.JSONArray;
    +import org.json.JSONObject;
    +import org.junit.BeforeClass;
    +import org.junit.ClassRule;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +import org.springframework.http.MediaType;
    +
    +import java.net.URLEncoder;
    +import java.util.Properties;
    +
    +
    +@Category({IntegrationTest.class, SecurityTest.class})
    +public class RestSecurityPostProcessorTest {
    +
    +  static final String REGION_NAME = "AuthRegion";
    +
    +  static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
    +  static Properties properties = new Properties() {
    +    {
    +      setProperty(SampleSecurityManager.SECURITY_JSON,
    +          "org/apache/geode/management/internal/security/clientServer.json");
    +      setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
    +      setProperty(START_DEV_REST_API, "true");
    +      setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
    +      setProperty(HTTP_SERVICE_PORT, restPort + "");
    +      setProperty(SECURITY_POST_PROCESSOR, SamplePostProcessor.class.getName());
    +    }
    +  };
    +
    +  @ClassRule
    +  public static ServerStarterRule serverStarter = new ServerStarterRule(properties);
    +  private final GeodeRestClient restClient = new GeodeRestClient("localhost", restPort);
    +
    +  @BeforeClass
    +  public static void before() throws Exception {
    +    serverStarter.startServer();
    +    Region region =
    +        serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME);
    +    region.put("key1",
    +        "{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1012,\"description\":\"Order for  XYZ Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/20/2014\",\"contact\":\"Jelly Bean\",\"email\":\"jelly.bean@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}");
    +    region.put("key2", "bar");
    +    serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create("customers");
    +    FunctionService.registerFunction(new GetRegions());
    +  }
    +
    +  /**
    +   * Test post-processing of a retrieved key from the server.
    +   */
    +  @Test
    +  public void getRegionKey() throws Exception {
    +
    +    // Test a single key
    +    HttpResponse response = restClient.doGet("/" + REGION_NAME + "/key1", "key1User", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    String body = IOUtils.toString(response.getEntity().getContent(), "UTF-8");
    +    assertTrue(body.startsWith("\"key1User/" + REGION_NAME + "/key1/"));
    +
    +    // Test multiple keys
    +    response = restClient.doGet("/" + REGION_NAME + "/key1,key2", "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    JSONObject jsonObject = getJsonObject(response);
    +    JSONArray jsonArray = jsonObject.getJSONArray(REGION_NAME);
    +    final int length = jsonArray.length();
    +    for (int index = 0; index < length; ++index) {
    +      String data = jsonArray.getString(index);
    +      assertTrue(data.contains("dataReader/" + REGION_NAME + "/"));
    +    }
    +  }
    +
    +  @Test
    +  public void getRegion() throws Exception {
    +    HttpResponse response = restClient.doGet("/" + REGION_NAME, "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    JSONObject jsonObject = getJsonObject(response);
    +    JSONArray jsonArray = jsonObject.getJSONArray(REGION_NAME);
    +    final int length = jsonArray.length();
    +    for (int index = 0; index < length; ++index) {
    +      String data = jsonArray.getString(index);
    +      assertTrue(data.contains("dataReader/" + REGION_NAME + "/"));
    +    }
    +  }
    +
    +  @Test
    +  public void adhocQuery() throws Exception {
    +    String query =
    +        "/queries/adhoc?q=" + URLEncoder.encode("SELECT * FROM /" + REGION_NAME, "UTF-8");
    +    HttpResponse response = restClient.doGet(query, "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    JSONArray jsonArray = getJsonArray(response);
    +    final int length = jsonArray.length();
    +    for (int index = 0; index < length; ++index) {
    +      String data = jsonArray.getString(index);
    +      assertTrue(data.contains("dataReader/" + REGION_NAME + "/"));
    +    }
    +  }
    +
    +  @Test
    +  public void namedQuery() throws Exception {
    +    String namedQuery = "SELECT c FROM /customers c WHERE c.customerId = $1";
    +
    +    HttpResponse response =
    +        restClient.doPost("/queries?id=selectCustomer&q=" + URLEncoder.encode(namedQuery, "UTF-8"),
    +            "dataReader", "1234567", "");
    +    assertEquals(201, getCode(response));
    --- End diff --
    
    is there anyway to crate the named query using java api instead of rest api?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86852018
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java ---
    @@ -0,0 +1,185 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
    +import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getCode;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getContentType;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonArray;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonObject;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import org.apache.commons.io.IOUtils;
    +import org.apache.geode.cache.Region;
    +import org.apache.geode.cache.RegionShortcut;
    +import org.apache.geode.cache.execute.FunctionService;
    +import org.apache.geode.internal.AvailablePortHelper;
    +import org.apache.geode.rest.internal.web.controllers.GetRegions;
    +import org.apache.geode.security.templates.SamplePostProcessor;
    +import org.apache.geode.security.templates.SampleSecurityManager;
    +import org.apache.geode.test.dunit.rules.ServerStarterRule;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +import org.apache.geode.test.junit.categories.SecurityTest;
    +import org.apache.http.HttpResponse;
    +import org.json.JSONArray;
    +import org.json.JSONObject;
    +import org.junit.BeforeClass;
    +import org.junit.ClassRule;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +import org.springframework.http.MediaType;
    +
    +import java.net.URLEncoder;
    +import java.util.Properties;
    +
    +
    +@Category({IntegrationTest.class, SecurityTest.class})
    +public class RestSecurityPostProcessorTest {
    +
    +  static final String REGION_NAME = "AuthRegion";
    +
    +  static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
    +  static Properties properties = new Properties() {
    +    {
    +      setProperty(SampleSecurityManager.SECURITY_JSON,
    +          "org/apache/geode/management/internal/security/clientServer.json");
    +      setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
    +      setProperty(START_DEV_REST_API, "true");
    +      setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
    +      setProperty(HTTP_SERVICE_PORT, restPort + "");
    +      setProperty(SECURITY_POST_PROCESSOR, SamplePostProcessor.class.getName());
    +    }
    +  };
    +
    +  @ClassRule
    +  public static ServerStarterRule serverStarter = new ServerStarterRule(properties);
    +  private final GeodeRestClient restClient = new GeodeRestClient("localhost", restPort);
    +
    +  @BeforeClass
    +  public static void before() throws Exception {
    +    serverStarter.startServer();
    +    Region region =
    +        serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME);
    +    region.put("key1",
    +        "{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1012,\"description\":\"Order for  XYZ Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/20/2014\",\"contact\":\"Jelly Bean\",\"email\":\"jelly.bean@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}");
    +    region.put("key2", "bar");
    +    serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create("customers");
    +    FunctionService.registerFunction(new GetRegions());
    +  }
    +
    +  /**
    +   * Test post-processing of a retrieved key from the server.
    +   */
    +  @Test
    +  public void getRegionKey() throws Exception {
    +
    +    // Test a single key
    +    HttpResponse response = restClient.doGet("/" + REGION_NAME + "/key1", "key1User", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    String body = IOUtils.toString(response.getEntity().getContent(), "UTF-8");
    +    assertTrue(body.startsWith("\"key1User/" + REGION_NAME + "/key1/"));
    +
    +    // Test multiple keys
    +    response = restClient.doGet("/" + REGION_NAME + "/key1,key2", "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    JSONObject jsonObject = getJsonObject(response);
    +    JSONArray jsonArray = jsonObject.getJSONArray(REGION_NAME);
    +    final int length = jsonArray.length();
    +    for (int index = 0; index < length; ++index) {
    +      String data = jsonArray.getString(index);
    +      assertTrue(data.contains("dataReader/" + REGION_NAME + "/"));
    +    }
    +  }
    +
    +  @Test
    +  public void getRegion() throws Exception {
    +    HttpResponse response = restClient.doGet("/" + REGION_NAME, "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    JSONObject jsonObject = getJsonObject(response);
    +    JSONArray jsonArray = jsonObject.getJSONArray(REGION_NAME);
    +    final int length = jsonArray.length();
    +    for (int index = 0; index < length; ++index) {
    +      String data = jsonArray.getString(index);
    +      assertTrue(data.contains("dataReader/" + REGION_NAME + "/"));
    +    }
    +  }
    +
    +  @Test
    +  public void adhocQuery() throws Exception {
    +    String query =
    +        "/queries/adhoc?q=" + URLEncoder.encode("SELECT * FROM /" + REGION_NAME, "UTF-8");
    +    HttpResponse response = restClient.doGet(query, "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    JSONArray jsonArray = getJsonArray(response);
    +    final int length = jsonArray.length();
    +    for (int index = 0; index < length; ++index) {
    +      String data = jsonArray.getString(index);
    +      assertTrue(data.contains("dataReader/" + REGION_NAME + "/"));
    +    }
    +  }
    +
    +  @Test
    +  public void namedQuery() throws Exception {
    +    String namedQuery = "SELECT c FROM /customers c WHERE c.customerId = $1";
    +
    +    HttpResponse response =
    +        restClient.doPost("/queries?id=selectCustomer&q=" + URLEncoder.encode(namedQuery, "UTF-8"),
    +            "dataReader", "1234567", "");
    +    assertEquals(201, getCode(response));
    +
    +    String query = "/queries";
    +    response = restClient.doGet(query, "dataReader", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    String customerJson =
    --- End diff --
    
    This breaks from the pattern RestAPIsQueryAndFEJUnitTest follows.  But as you said in an earlier comment about the Orders, it will read cleaner.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86900866
  
    --- Diff: geode-core/src/main/java/org/apache/geode/security/PostProcessor.java ---
    @@ -44,6 +47,9 @@ default void init(Properties securityProps) {}
        */
       Object processRegionValue(Object principal, String regionName, Object key, Object value);
     
    +  Collection<Object> processQueryResult(Object principal, Query query, Collection<String> regions,
    --- End diff --
    
    removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86836336
  
    --- Diff: geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java ---
    @@ -409,6 +435,34 @@ public Object postProcess(Object principal, String regionPath, Object key, Objec
         return newValue;
       }
     
    +  private Object getPrincipal(Object principal) {
    +    if (principal == null) {
    +      Subject subject = getSubject();
    +      if (subject == null)
    +        return null;
    +      principal = subject.getPrincipal();
    +    }
    +    return principal;
    +  }
    +
    +  @Override
    +  public Collection<Object> postProcess(Query query, Collection<String> regionNames,
    +      Collection<Object> results) {
    +    return postProcess(null, query, regionNames, results);
    +  }
    +
    +  @Override
    +  public Collection<Object> postProcess(Object principal, Query query,
    --- End diff --
    
    remove this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86819415
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java ---
    @@ -0,0 +1,185 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
    +import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getCode;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getContentType;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonArray;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonObject;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import org.apache.commons.io.IOUtils;
    +import org.apache.geode.cache.Region;
    +import org.apache.geode.cache.RegionShortcut;
    +import org.apache.geode.cache.execute.FunctionService;
    +import org.apache.geode.internal.AvailablePortHelper;
    +import org.apache.geode.rest.internal.web.controllers.GetRegions;
    +import org.apache.geode.security.templates.SamplePostProcessor;
    +import org.apache.geode.security.templates.SampleSecurityManager;
    +import org.apache.geode.test.dunit.rules.ServerStarterRule;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +import org.apache.geode.test.junit.categories.SecurityTest;
    +import org.apache.http.HttpResponse;
    +import org.json.JSONArray;
    +import org.json.JSONObject;
    +import org.junit.BeforeClass;
    +import org.junit.ClassRule;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +import org.springframework.http.MediaType;
    +
    +import java.net.URLEncoder;
    +import java.util.Properties;
    +
    +
    +@Category({IntegrationTest.class, SecurityTest.class})
    +public class RestSecurityPostProcessorTest {
    +
    +  static final String REGION_NAME = "AuthRegion";
    +
    +  static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
    +  static Properties properties = new Properties() {
    +    {
    +      setProperty(SampleSecurityManager.SECURITY_JSON,
    +          "org/apache/geode/management/internal/security/clientServer.json");
    +      setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
    +      setProperty(START_DEV_REST_API, "true");
    +      setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
    +      setProperty(HTTP_SERVICE_PORT, restPort + "");
    +      setProperty(SECURITY_POST_PROCESSOR, SamplePostProcessor.class.getName());
    +    }
    +  };
    +
    +  @ClassRule
    +  public static ServerStarterRule serverStarter = new ServerStarterRule(properties);
    +  private final GeodeRestClient restClient = new GeodeRestClient("localhost", restPort);
    +
    +  @BeforeClass
    +  public static void before() throws Exception {
    +    serverStarter.startServer();
    +    Region region =
    +        serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME);
    +    region.put("key1",
    +        "{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1012,\"description\":\"Order for  XYZ Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/20/2014\",\"contact\":\"Jelly Bean\",\"email\":\"jelly.bean@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}");
    --- End diff --
    
    You could create an Order object and put it as an java object instead of using json. This would be much readable.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86836201
  
    --- Diff: geode-core/src/main/java/org/apache/geode/security/PostProcessor.java ---
    @@ -44,6 +47,9 @@ default void init(Properties securityProps) {}
        */
       Object processRegionValue(Object principal, String regionName, Object key, Object value);
     
    +  Collection<Object> processQueryResult(Object principal, Query query, Collection<String> regions,
    --- End diff --
    
    I don't think we want to add this method in the interface. For query,  essentially it's still getting value from region. Customers should not be burdened with another method to implement if they just want to manipulate the data. See how query result get post processed in gfsh queries. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86836263
  
    --- Diff: geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java ---
    @@ -87,6 +89,12 @@
       Object postProcess(Object principal, String regionPath, Object key, Object value,
           boolean valueIsSerialized);
     
    +  Collection<Object> postProcess(Query query, Collection<String> regionNames,
    --- End diff --
    
    remove this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-geode/pull/276


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on the issue:

    https://github.com/apache/incubator-geode/pull/276
  
    1. I am using Autowire, the annotation is on the base class's constructor.  But as for the injection via the constructor, please review https://spring.io/blog/2015/11/29/how-not-to-hate-spring-in-2016 which references http://olivergierke.de/2013/11/why-field-injection-is-evil/ and http://docs.spring.io/spring-framework/docs/current/spring-framework-reference/htmlsingle/#beans-constructor-injection
    
    Do you think it would be better to only add the RestSecurityService to the controllers that actually need it instead of adding it to the base class?  I think that would complicate the constructors.
    
    2 & 3. Agreed, that'll make the test cleaner.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86852092
  
    --- Diff: geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java ---
    @@ -50,6 +52,11 @@
     
       protected static final String REST_API_VERSION = "/v1";
     
    +  public BaseControllerAdvice(final RestSecurityService securityService,
    --- End diff --
    
    BaseControllerAdvice extends AbstractBaseController.  Since the RestSecurityService lives in AbstractBaseController, yes.  Otherwise, we define separate member variables in PdxBasedCrudController, CommonCrudController, and QueryAccessContoller.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86900814
  
    --- Diff: geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java ---
    @@ -409,6 +435,34 @@ public Object postProcess(Object principal, String regionPath, Object key, Objec
         return newValue;
       }
     
    +  private Object getPrincipal(Object principal) {
    +    if (principal == null) {
    +      Subject subject = getSubject();
    +      if (subject == null)
    +        return null;
    +      principal = subject.getPrincipal();
    +    }
    +    return principal;
    +  }
    +
    +  @Override
    +  public Collection<Object> postProcess(Query query, Collection<String> regionNames,
    +      Collection<Object> results) {
    +    return postProcess(null, query, regionNames, results);
    +  }
    +
    +  @Override
    +  public Collection<Object> postProcess(Object principal, Query query,
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #276: GEODE-1993: postprocess region/key

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/276#discussion_r86820194
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java ---
    @@ -0,0 +1,185 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    + * agreements. See the NOTICE file distributed with this work for additional information regarding
    + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance with the License. You may obtain a
    + * copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software distributed under the License
    + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
    + * or implied. See the License for the specific language governing permissions and limitations under
    + * the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
    +import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
    +import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
    +import static org.apache.geode.distributed.ConfigurationProperties.START_DEV_REST_API;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getCode;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getContentType;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonArray;
    +import static org.apache.geode.rest.internal.web.GeodeRestClient.getJsonObject;
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import org.apache.commons.io.IOUtils;
    +import org.apache.geode.cache.Region;
    +import org.apache.geode.cache.RegionShortcut;
    +import org.apache.geode.cache.execute.FunctionService;
    +import org.apache.geode.internal.AvailablePortHelper;
    +import org.apache.geode.rest.internal.web.controllers.GetRegions;
    +import org.apache.geode.security.templates.SamplePostProcessor;
    +import org.apache.geode.security.templates.SampleSecurityManager;
    +import org.apache.geode.test.dunit.rules.ServerStarterRule;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +import org.apache.geode.test.junit.categories.SecurityTest;
    +import org.apache.http.HttpResponse;
    +import org.json.JSONArray;
    +import org.json.JSONObject;
    +import org.junit.BeforeClass;
    +import org.junit.ClassRule;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +import org.springframework.http.MediaType;
    +
    +import java.net.URLEncoder;
    +import java.util.Properties;
    +
    +
    +@Category({IntegrationTest.class, SecurityTest.class})
    +public class RestSecurityPostProcessorTest {
    +
    +  static final String REGION_NAME = "AuthRegion";
    +
    +  static int restPort = AvailablePortHelper.getRandomAvailableTCPPort();
    +  static Properties properties = new Properties() {
    +    {
    +      setProperty(SampleSecurityManager.SECURITY_JSON,
    +          "org/apache/geode/management/internal/security/clientServer.json");
    +      setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
    +      setProperty(START_DEV_REST_API, "true");
    +      setProperty(HTTP_SERVICE_BIND_ADDRESS, "localhost");
    +      setProperty(HTTP_SERVICE_PORT, restPort + "");
    +      setProperty(SECURITY_POST_PROCESSOR, SamplePostProcessor.class.getName());
    +    }
    +  };
    +
    +  @ClassRule
    +  public static ServerStarterRule serverStarter = new ServerStarterRule(properties);
    +  private final GeodeRestClient restClient = new GeodeRestClient("localhost", restPort);
    +
    +  @BeforeClass
    +  public static void before() throws Exception {
    +    serverStarter.startServer();
    +    Region region =
    +        serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create(REGION_NAME);
    +    region.put("key1",
    +        "{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1012,\"description\":\"Order for  XYZ Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/20/2014\",\"contact\":\"Jelly Bean\",\"email\":\"jelly.bean@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}");
    +    region.put("key2", "bar");
    +    serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create("customers");
    +    FunctionService.registerFunction(new GetRegions());
    +  }
    +
    +  /**
    +   * Test post-processing of a retrieved key from the server.
    +   */
    +  @Test
    +  public void getRegionKey() throws Exception {
    +
    +    // Test a single key
    +    HttpResponse response = restClient.doGet("/" + REGION_NAME + "/key1", "key1User", "1234567");
    +    assertEquals(200, getCode(response));
    +    assertEquals(MediaType.APPLICATION_JSON_UTF8_VALUE, getContentType(response));
    +
    +    String body = IOUtils.toString(response.getEntity().getContent(), "UTF-8");
    +    assertTrue(body.startsWith("\"key1User/" + REGION_NAME + "/key1/"));
    --- End diff --
    
    Why the key1User is quoted?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---