GEODE-1570: improve rest security framework * use annotations for authorization * consolidate tests * delete unused classes
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/29e49480 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/29e49480 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/29e49480 Branch: refs/heads/develop Commit: 29e4948037b0f8066953efb7abafb51c77639493 Parents: 280d2d8 Author: Jinmei Liao <jil...@pivotal.io> Authored: Mon Oct 10 15:00:33 2016 -0700 Committer: Jinmei Liao <jil...@pivotal.io> Committed: Tue Oct 11 14:22:33 2016 -0700 ---------------------------------------------------------------------- .../internal/web/RestSecurityDUnitTest.java | 409 +++++++++++++++++- .../web/RestSecurityEndpointsDUnitTest.java | 422 ------------------- .../security/IntegratedSecurityService.java | 27 +- .../internal/security/SecurityService.java | 7 +- .../geode/security/ResourcePermission.java | 4 +- .../web/controllers/AbstractBaseController.java | 32 +- .../web/controllers/BaseControllerAdvice.java | 43 +- .../web/controllers/CommonCrudController.java | 55 +-- .../controllers/FunctionAccessController.java | 56 ++- .../web/controllers/PdxBasedCrudController.java | 53 +-- .../web/controllers/QueryAccessController.java | 73 ++-- .../web/security/GeodeAuthentication.java | 37 -- .../security/GeodeAuthenticationProvider.java | 21 +- .../internal/web/security/GeodeAuthority.java | 47 --- .../web/security/RestSecurityService.java | 56 +++ 15 files changed, 607 insertions(+), 735 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java b/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java index 7d232ce..59e00c8 100644 --- a/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java +++ b/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java @@ -23,7 +23,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.net.MalformedURLException; -import java.net.URL; import java.nio.charset.StandardCharsets; import org.apache.http.HttpEntity; @@ -47,7 +46,10 @@ import org.apache.http.impl.client.BasicAuthCache; import org.apache.http.impl.client.BasicCredentialsProvider; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; +import org.json.JSONArray; +import org.json.JSONObject; import org.json.JSONTokener; +import org.junit.Test; import org.junit.experimental.categories.Category; import org.apache.geode.internal.AvailablePortHelper; @@ -63,15 +65,390 @@ public class RestSecurityDUnitTest extends AbstractSecureServerDUnitTest { public final static String HOSTNAME = "localhost"; public final static String CONTEXT = "/geode/v1"; - private final String endPoint; - private final URL url; - public RestSecurityDUnitTest() throws MalformedURLException { int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2); this.jmxPort = ports[0]; this.restPort = ports[1]; - endPoint = PROTOCOL + "://" + HOSTNAME + ":" + restPort + CONTEXT; - url = new URL(endPoint); + } + + @Test + public void testFunctions() { + client1.invoke(() -> { + String json = "{\"@type\":\"double\",\"@value\":210}"; + + HttpResponse response = doGet("/functions", "unknown-user", "1234567"); + assertEquals(401, getCode(response)); + response = doGet("/functions", "stranger", "1234567"); + assertEquals(403, getCode(response)); + response = doGet("/functions", "dataReader", "1234567"); + assertTrue(isOK(response)); + + response = doPost("/functions/AddFreeItemsToOrder", "unknown-user", "1234567", json); + assertEquals(401, getCode(response)); + response = doPost("/functions/AddFreeItemsToOrder", "dataReader", "1234567", json); + assertEquals(403, getCode(response)); + response = doPost("/functions/AddFreeItemsToOrder?onRegion=" + REGION_NAME, "dataWriter", "1234567", json); + // because we're only testing the security of the endpoint, not the endpoint functionality, a 500 is acceptable + assertEquals(500, getCode(response)); + }); + } + + @Test + public void testQueries() { + client1.invoke(() -> { + HttpResponse response = doGet("/queries", "unknown-user", "1234567"); + assertEquals(401, getCode(response)); + response = doGet("/queries", "stranger", "1234567"); + assertEquals(403, getCode(response)); + response = doGet("/queries", "dataReader", "1234567"); + assertEquals(200, getCode(response)); + }); + } + + @Test + public void testAdhocQuery() { + client1.invoke(() -> { + HttpResponse response = doGet("/queries/adhoc?q=", "unknown-user", "1234567"); + assertEquals(401, getCode(response)); + response = doGet("/queries/adhoc?q=", "stranger", "1234567"); + assertEquals(403, getCode(response)); + response = doGet("/queries/adhoc?q=", "dataReader", "1234567"); + // because we're only testing the security of the endpoint, not the endpoint functionality, a 500 is acceptable + assertEquals(500, getCode(response)); + }); + } + + @Test + public void testPostQuery() { + client1.invoke(() -> { + HttpResponse response = doPost("/queries?id=0&q=", "unknown-user", "1234567", ""); + assertEquals(401, getCode(response)); + response = doPost("/queries?id=0&q=", "stranger", "1234567", ""); + assertEquals(403, getCode(response)); + response = doPost("/queries?id=0&q=", "dataWriter", "1234567", ""); + // because we're only testing the security of the endpoint, not the endpoint functionality, a 500 is acceptable + assertEquals(500, getCode(response)); + }); + } + + @Test + public void testPostQuery2() { + client1.invoke(() -> { + HttpResponse response = doPost("/queries/id", "unknown-user", "1234567", "{\"id\" : \"foo\"}"); + assertEquals(401, getCode(response)); + response = doPost("/queries/id", "stranger", "1234567", "{\"id\" : \"foo\"}"); + assertEquals(403, getCode(response)); + response = doPost("/queries/id", "dataWriter", "1234567", "{\"id\" : \"foo\"}"); + // because we're only testing the security of the endpoint, not the endpoint functionality, a 500 is acceptable + assertEquals(500, getCode(response)); + }); + } + + @Test + public void testPutQuery() { + client1.invoke(() -> { + HttpResponse response = doPut("/queries/id", "unknown-user", "1234567", "{\"id\" : \"foo\"}"); + assertEquals(401, getCode(response)); + response = doPut("/queries/id", "stranger", "1234567", "{\"id\" : \"foo\"}"); + assertEquals(403, getCode(response)); + response = doPut("/queries/id", "dataWriter", "1234567", "{\"id\" : \"foo\"}"); + // We should get a 404 because we're trying to update a query that doesn't exist + assertEquals(404, getCode(response)); + }); + } + + @Test + public void testDeleteQuery() { + client1.invoke(() -> { + HttpResponse response = doDelete("/queries/id", "unknown-user", "1234567"); + assertEquals(401, getCode(response)); + response = doDelete("/queries/id", "stranger", "1234567"); + assertEquals(403, getCode(response)); + response = doDelete("/queries/id", "dataWriter", "1234567"); + // We should get a 404 because we're trying to delete a query that doesn't exist + assertEquals(404, getCode(response)); + }); + } + + @Test + public void testServers() { + client1.invoke(() -> { + HttpResponse response = doGet("/servers", "unknown-user", "1234567"); + assertEquals(401, getCode(response)); + response = doGet("/servers", "stranger", "1234567"); + assertEquals(403, getCode(response)); + response = doGet("/servers", "super-user", "1234567"); + assertTrue(isOK(response)); + }); + } + + /** + * This test should always return an OK, whether the user is known or unknown. A phishing script should not be + * able to determine whether a user/password combination is good + */ + @Test + public void testPing() { + client1.invoke(() -> { + HttpResponse response = doHEAD("/ping", "stranger", "1234567"); + assertTrue(isOK(response)); + response = doGet("/ping", "stranger", "1234567"); + assertTrue(isOK(response)); + + response = doHEAD("/ping", "super-user", "1234567"); + assertTrue(isOK(response)); + response = doGet("/ping", "super-user", "1234567"); + assertTrue(isOK(response)); + + // TODO - invalid username/password should still respond, but doesn't + // response = doHEAD("/ping", "unknown-user", "badpassword"); + // assertTrue(isOK(response)); + // response = doGet("/ping", "unknown-user", "badpassword"); + // assertTrue(isOK(response)); + + // TODO - credentials are currently required and shouldn't be for this endpoint + // response = doHEAD("/ping", null, null); + // assertTrue(isOK(response)); + // response = doGet("/ping", null, null); + // assertTrue(isOK(response)); + }); + } + + /** + * Test permissions on retrieving a list of regions. + */ + @Test + public void getRegions() { + client1.invoke(() -> { + HttpResponse response = doGet("", "dataReader", "1234567"); + assertEquals("A '200 - OK' was expected", 200, getCode(response)); + + assertTrue(isOK(response)); + JSONObject jsonObject = new JSONObject(getResponseBody(response)); + JSONArray regions = jsonObject.getJSONArray("regions"); + assertNotNull(regions); + assertTrue(regions.length() > 0); + JSONObject region = regions.getJSONObject(0); + assertEquals("AuthRegion", region.get("name")); + assertEquals("REPLICATE", region.get("type")); + }); + + // List regions with an unknown user - 401 + client1.invoke(() -> { + HttpResponse response = doGet("", "unknown-user", "badpassword"); + assertEquals(401, getCode(response)); + }); + + // list regions with insufficent rights - 403 + client1.invoke(() -> { + HttpResponse response = doGet("", "authRegionReader", "1234567"); + assertEquals(403, getCode(response)); + }); + } + + /** + * Test permissions on getting a region + */ + @Test + public void getRegion() { + // Test an unknown user - 401 error + client1.invoke(() -> { + HttpResponse response = doGet("/" + REGION_NAME, "unknown-user", "1234567"); + assertEquals(401, getCode(response)); + }); + + // Test a user with insufficient rights - 403 + client1.invoke(() -> { + HttpResponse response = doGet("/" + REGION_NAME, "stranger", "1234567"); + assertEquals(403, getCode(response)); + }); + + // Test an authorized user - 200 + client1.invoke(() -> { + HttpResponse response = doGet("/" + REGION_NAME, "super-user", "1234567"); + assertTrue(isOK(response)); + }); + } + + /** + * Test permissions on HEAD region + */ + @Test + public void headRegion() { + // Test an unknown user - 401 error + client1.invoke(() -> { + HttpResponse response = doHEAD("/" + REGION_NAME, "unknown-user", "1234567"); + assertEquals(401, getCode(response)); + }); + + // Test a user with insufficient rights - 403 + client1.invoke(() -> { + HttpResponse response = doHEAD("/" + REGION_NAME, "stranger", "1234567"); + assertEquals(403, getCode(response)); + }); + + // Test an authorized user - 200 + client1.invoke(() -> { + HttpResponse response = doHEAD("/" + REGION_NAME, "super-user", "1234567"); + assertTrue(isOK(response)); + }); + } + + /** + * Test permissions on deleting a region + */ + @Test + public void deleteRegion() { + // Test an unknown user - 401 error + client1.invoke(() -> { + HttpResponse response = doDelete("/" + REGION_NAME, "unknown-user", "1234567"); + assertEquals(401, getCode(response)); + }); + + // Test a user with insufficient rights - 403 + client1.invoke(() -> { + HttpResponse response = doDelete("/" + REGION_NAME, "dataReader", "1234567"); + assertEquals(403, getCode(response)); + }); + + // Test an authorized user - 200 + client1.invoke(() -> { + HttpResponse response = doDelete("/" + REGION_NAME, "super-user", "1234567"); + assertTrue(isOK(response)); + }); + } + + /** + * Test permissions on getting a region's keys + */ + @Test + public void getRegionKeys() { + // Test an authorized user + client1.invoke(() -> { + HttpResponse response = doGet("/" + REGION_NAME + "/keys", "super-user", "1234567"); + assertTrue(isOK(response)); + }); + // Test an unauthorized user + client1.invoke(() -> { + HttpResponse response = doGet("/" + REGION_NAME + "/keys", "dataWriter", "1234567"); + assertEquals(403, getCode(response)); + }); + } + + /** + * Test permissions on retrieving a key from a region + */ + @Test + public void getRegionKey() { + // Test an authorized user + client1.invoke(() -> { + HttpResponse response = doGet("/" + REGION_NAME + "/key1", "key1User", "1234567"); + assertTrue(isOK(response)); + }); + // Test an unauthorized user + client1.invoke(() -> { + HttpResponse response = doGet("/" + REGION_NAME + "/key1", "dataWriter", "1234567"); + assertEquals(403, getCode(response)); + }); + } + + /** + * Test permissions on deleting a region's key(s) + */ + @Test + public void deleteRegionKey() { + // Test an unknown user - 401 error + client1.invoke(() -> { + HttpResponse response = doDelete("/" + REGION_NAME + "/key1", "unknown-user", "1234567"); + assertEquals(401, getCode(response)); + }); + + // Test a user with insufficient rights - 403 + client1.invoke(() -> { + HttpResponse response = doDelete("/" + REGION_NAME + "/key1", "dataReader", "1234567"); + assertEquals(403, getCode(response)); + }); + + // Test an authorized user - 200 + client1.invoke(() -> { + HttpResponse response = doDelete("/" + REGION_NAME + "/key1", "key1User", "1234567"); + assertTrue(isOK(response)); + }); + } + + /** + * Test permissions on deleting a region's key(s) + */ + @Test + public void postRegionKey() { + // Test an unknown user - 401 error + client1.invoke(() -> { + HttpResponse response = doPost("/" + REGION_NAME + "?key9", "unknown", "1234567", "{ \"key9\" : \"foo\" }"); + assertEquals(401, getCode(response)); + }); + + // Test a user with insufficient rights - 403 + client1.invoke(() -> { + HttpResponse response = doPost("/" + REGION_NAME + "?key9", "dataReader", "1234567", "{ \"key9\" : \"foo\" }"); + assertEquals(403, getCode(response)); + }); + + // Test an authorized user - 200 + client1.invoke(() -> { + HttpResponse response = doPost("/" + REGION_NAME + "?key9", "dataWriter", "1234567", "{ \"key9\" : \"foo\" }"); + assertEquals(201, getCode(response)); + assertTrue(isOK(response)); + }); + } + + /** + * Test permissions on deleting a region's key(s) + */ + @Test + public void putRegionKey() { + + String json = "{\"@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.b...@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}"; + String casJSON = "{\"@old\":{\"@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.b...@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225},\"@new \":{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1013,\"description\":\"Order for New Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/25/2014\",\"contact\":\"Vanilla Bean\",\"email\":\"vanillab...@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":12345,\"description\":\"part 123\",\"quantity\":12,\"unitPrice\":29.99,\"totalPrice\":149.95}],\"totalPrice\":149.95}}"; + // Test an unknown user - 401 error + client1.invoke(() -> { + HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=PUT", "unknown-user", "1234567", "{ \"key9\" : \"foo\" }"); + assertEquals(401, getCode(response)); + }); + + client1.invoke(() -> { + HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=CAS", "unknown-user", "1234567", "{ \"key9\" : \"foo\" }"); + assertEquals(401, getCode(response)); + }); + + client1.invoke(() -> { + HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=REPLACE", "unknown-user", "1234567", "{ \"@old\" : \"value1\", \"@new\" : \"CASvalue\" }"); + assertEquals(401, getCode(response)); + }); + + // Test a user with insufficient rights - 403 + client1.invoke(() -> { + HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=PUT", "dataReader", "1234567", "{ \"key1\" : \"foo\" }"); + assertEquals(403, getCode(response)); + }); + client1.invoke(() -> { + HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=REPLACE", "dataReader", "1234567", "{ \"key1\" : \"foo\" }"); + assertEquals(403, getCode(response)); + }); + client1.invoke(() -> { + HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=CAS", "dataReader", "1234567", casJSON); + assertEquals(403, getCode(response)); + }); + + // Test an authorized user - 200 + client1.invoke(() -> { + HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=PUT", "key1User", "1234567", "{ \"key1\" : \"foo\" }"); + assertEquals(200, getCode(response)); + assertTrue(isOK(response)); + }); + client1.invoke(() -> { + HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=REPLACE", "key1User", "1234567", json); + assertEquals(200, getCode(response)); + assertTrue(isOK(response)); + }); } protected HttpResponse doHEAD(String query, String username, String password) throws MalformedURLException { @@ -95,6 +472,16 @@ public class RestSecurityDUnitTest extends AbstractSecureServerDUnitTest { return doRequest(httpPut, username, password); } + protected HttpResponse doGet(String uri, String username, String password) throws MalformedURLException { + HttpGet getRequest = new HttpGet(CONTEXT + uri); + return doRequest(getRequest, username, password); + } + + protected HttpResponse doDelete(String uri, String username, String password) throws MalformedURLException { + HttpDelete httpDelete = new HttpDelete(CONTEXT + uri); + return doRequest(httpDelete, username, password); + } + /** * Check the HTTP status of the response and return if it's within the OK range * @param response The HttpResponse message received from the server @@ -140,16 +527,6 @@ public class RestSecurityDUnitTest extends AbstractSecureServerDUnitTest { return new JSONTokener(str.toString()); } - protected HttpResponse doGet(String uri, String username, String password) throws MalformedURLException { - HttpGet getRequest = new HttpGet(CONTEXT + uri); - return doRequest(getRequest, username, password); - } - - protected HttpResponse doDelete(String uri, String username, String password) throws MalformedURLException { - HttpDelete httpDelete = new HttpDelete(CONTEXT + uri); - return doRequest(httpDelete, username, password); - } - private HttpResponse doRequest(HttpRequestBase request, String username, String password) throws MalformedURLException { HttpHost targetHost = new HttpHost(HOSTNAME, this.restPort, PROTOCOL); CloseableHttpClient httpclient = HttpClients.custom().build(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityEndpointsDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityEndpointsDUnitTest.java b/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityEndpointsDUnitTest.java deleted file mode 100644 index 149a905..0000000 --- a/geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityEndpointsDUnitTest.java +++ /dev/null @@ -1,422 +0,0 @@ -/* - * 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.junit.Assert.*; - -import java.net.MalformedURLException; - -import org.apache.geode.test.junit.categories.DistributedTest; -import org.apache.geode.test.junit.categories.SecurityTest; -import org.apache.http.HttpResponse; -import org.json.JSONArray; -import org.json.JSONObject; -import org.junit.Test; -import org.junit.experimental.categories.Category; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -@SuppressWarnings("serial") -@Category({ DistributedTest.class, SecurityTest.class }) -public class RestSecurityEndpointsDUnitTest extends RestSecurityDUnitTest { - - Logger logger = LoggerFactory.getLogger(RestSecurityEndpointsDUnitTest.class); - - public RestSecurityEndpointsDUnitTest() throws MalformedURLException { - super(); - } - - @Test - public void testFunctions() { - client1.invoke(() -> { - String json = "{\"@type\":\"double\",\"@value\":210}"; - - HttpResponse response = doGet("/functions", "unknown-user", "1234567"); - assertEquals(401, getCode(response)); - response = doGet("/functions", "stranger", "1234567"); - assertEquals(403, getCode(response)); - response = doGet("/functions", "dataReader", "1234567"); - assertTrue(isOK(response)); - - response = doPost("/functions/AddFreeItemsToOrder", "unknown-user", "1234567", json); - assertEquals(401, getCode(response)); - response = doPost("/functions/AddFreeItemsToOrder", "dataReader", "1234567", json); - assertEquals(403, getCode(response)); - response = doPost("/functions/AddFreeItemsToOrder?onRegion=" + REGION_NAME, "dataWriter", "1234567", json); - // because we're only testing the security of the endpoint, not the endpoint functionality, a 500 is acceptable - assertEquals(500, getCode(response)); - }); - } - - @Test - public void testQueries() { - client1.invoke(() -> { - HttpResponse response = doGet("/queries", "unknown-user", "1234567"); - assertEquals(401, getCode(response)); - response = doGet("/queries", "stranger", "1234567"); - assertEquals(403, getCode(response)); - response = doGet("/queries", "dataReader", "1234567"); - assertEquals(200, getCode(response)); - }); - } - - @Test - public void testAdhocQuery() { - client1.invoke(() -> { - HttpResponse response = doGet("/queries/adhoc?q=", "unknown-user", "1234567"); - assertEquals(401, getCode(response)); - response = doGet("/queries/adhoc?q=", "stranger", "1234567"); - assertEquals(403, getCode(response)); - response = doGet("/queries/adhoc?q=", "dataReader", "1234567"); - // because we're only testing the security of the endpoint, not the endpoint functionality, a 500 is acceptable - assertEquals(500, getCode(response)); - }); - } - - @Test - public void testPostQuery() { - client1.invoke(() -> { - HttpResponse response = doPost("/queries?id=0&q=", "unknown-user", "1234567", ""); - assertEquals(401, getCode(response)); - response = doPost("/queries?id=0&q=", "stranger", "1234567", ""); - assertEquals(403, getCode(response)); - response = doPost("/queries?id=0&q=", "dataWriter", "1234567", ""); - // because we're only testing the security of the endpoint, not the endpoint functionality, a 500 is acceptable - assertEquals(500, getCode(response)); - }); - } - - @Test - public void testPostQuery2() { - client1.invoke(() -> { - HttpResponse response = doPost("/queries/id", "unknown-user", "1234567", "{\"id\" : \"foo\"}"); - assertEquals(401, getCode(response)); - response = doPost("/queries/id", "stranger", "1234567", "{\"id\" : \"foo\"}"); - assertEquals(403, getCode(response)); - response = doPost("/queries/id", "dataWriter", "1234567", "{\"id\" : \"foo\"}"); - // because we're only testing the security of the endpoint, not the endpoint functionality, a 500 is acceptable - assertEquals(500, getCode(response)); - }); - } - - @Test - public void testPutQuery() { - client1.invoke(() -> { - HttpResponse response = doPut("/queries/id", "unknown-user", "1234567", "{\"id\" : \"foo\"}"); - assertEquals(401, getCode(response)); - response = doPut("/queries/id", "stranger", "1234567", "{\"id\" : \"foo\"}"); - assertEquals(403, getCode(response)); - response = doPut("/queries/id", "dataWriter", "1234567", "{\"id\" : \"foo\"}"); - // We should get a 404 because we're trying to update a query that doesn't exist - assertEquals(404, getCode(response)); - }); - } - - @Test - public void testDeleteQuery() { - client1.invoke(() -> { - HttpResponse response = doDelete("/queries/id", "unknown-user", "1234567"); - assertEquals(401, getCode(response)); - response = doDelete("/queries/id", "stranger", "1234567"); - assertEquals(403, getCode(response)); - response = doDelete("/queries/id", "dataWriter", "1234567"); - // We should get a 404 because we're trying to delete a query that doesn't exist - assertEquals(404, getCode(response)); - }); - } - - @Test - public void testServers() { - client1.invoke(() -> { - HttpResponse response = doGet("/servers", "unknown-user", "1234567"); - assertEquals(401, getCode(response)); - response = doGet("/servers", "stranger", "1234567"); - assertEquals(403, getCode(response)); - response = doGet("/servers", "super-user", "1234567"); - assertTrue(isOK(response)); - }); - } - - /** - * This test should always return an OK, whether the user is known or unknown. A phishing script should not be - * able to determine whether a user/password combination is good - */ - @Test - public void testPing() { - client1.invoke(() -> { - HttpResponse response = doHEAD("/ping", "stranger", "1234567"); - assertTrue(isOK(response)); - response = doGet("/ping", "stranger", "1234567"); - assertTrue(isOK(response)); - - response = doHEAD("/ping", "super-user", "1234567"); - assertTrue(isOK(response)); - response = doGet("/ping", "super-user", "1234567"); - assertTrue(isOK(response)); - - // TODO - invalid username/password should still respond, but doesn't - // response = doHEAD("/ping", "unknown-user", "badpassword"); - // assertTrue(isOK(response)); - // response = doGet("/ping", "unknown-user", "badpassword"); - // assertTrue(isOK(response)); - - // TODO - credentials are currently required and shouldn't be for this endpoint - // response = doHEAD("/ping", null, null); - // assertTrue(isOK(response)); - // response = doGet("/ping", null, null); - // assertTrue(isOK(response)); - }); - } - - /** - * Test permissions on retrieving a list of regions. - */ - @Test - public void getRegions() { - client1.invoke(() -> { - HttpResponse response = doGet("", "dataReader", "1234567"); - assertEquals("A '200 - OK' was expected", 200, getCode(response)); - - assertTrue(isOK(response)); - JSONObject jsonObject = new JSONObject(getResponseBody(response)); - JSONArray regions = jsonObject.getJSONArray("regions"); - assertNotNull(regions); - assertTrue(regions.length() > 0); - JSONObject region = regions.getJSONObject(0); - assertEquals("AuthRegion", region.get("name")); - assertEquals("REPLICATE", region.get("type")); - }); - - // List regions with an unknown user - 401 - client1.invoke(() -> { - HttpResponse response = doGet("", "unknown-user", "badpassword"); - assertEquals(401, getCode(response)); - }); - - // list regions with insufficent rights - 403 - client1.invoke(() -> { - HttpResponse response = doGet("", "authRegionReader", "1234567"); - assertEquals(403, getCode(response)); - }); - } - - /** - * Test permissions on getting a region - */ - @Test - public void getRegion() { - // Test an unknown user - 401 error - client1.invoke(() -> { - HttpResponse response = doGet("/" + REGION_NAME, "unknown-user", "1234567"); - assertEquals(401, getCode(response)); - }); - - // Test a user with insufficient rights - 403 - client1.invoke(() -> { - HttpResponse response = doGet("/" + REGION_NAME, "stranger", "1234567"); - assertEquals(403, getCode(response)); - }); - - // Test an authorized user - 200 - client1.invoke(() -> { - HttpResponse response = doGet("/" + REGION_NAME, "super-user", "1234567"); - assertTrue(isOK(response)); - }); - } - - /** - * Test permissions on HEAD region - */ - @Test - public void headRegion() { - // Test an unknown user - 401 error - client1.invoke(() -> { - HttpResponse response = doHEAD("/" + REGION_NAME, "unknown-user", "1234567"); - assertEquals(401, getCode(response)); - }); - - // Test a user with insufficient rights - 403 - client1.invoke(() -> { - HttpResponse response = doHEAD("/" + REGION_NAME, "stranger", "1234567"); - assertEquals(403, getCode(response)); - }); - - // Test an authorized user - 200 - client1.invoke(() -> { - HttpResponse response = doHEAD("/" + REGION_NAME, "super-user", "1234567"); - assertTrue(isOK(response)); - }); - } - - /** - * Test permissions on deleting a region - */ - @Test - public void deleteRegion() { - // Test an unknown user - 401 error - client1.invoke(() -> { - HttpResponse response = doDelete("/" + REGION_NAME, "unknown-user", "1234567"); - assertEquals(401, getCode(response)); - }); - - // Test a user with insufficient rights - 403 - client1.invoke(() -> { - HttpResponse response = doDelete("/" + REGION_NAME, "dataReader", "1234567"); - assertEquals(403, getCode(response)); - }); - - // Test an authorized user - 200 - client1.invoke(() -> { - HttpResponse response = doDelete("/" + REGION_NAME, "super-user", "1234567"); - assertTrue(isOK(response)); - }); - } - - /** - * Test permissions on getting a region's keys - */ - @Test - public void getRegionKeys() { - // Test an authorized user - client1.invoke(() -> { - HttpResponse response = doGet("/" + REGION_NAME + "/keys", "super-user", "1234567"); - assertTrue(isOK(response)); - }); - // Test an unauthorized user - client1.invoke(() -> { - HttpResponse response = doGet("/" + REGION_NAME + "/keys", "dataWriter", "1234567"); - assertEquals(403, getCode(response)); - }); - } - - /** - * Test permissions on retrieving a key from a region - */ - @Test - public void getRegionKey() { - // Test an authorized user - client1.invoke(() -> { - HttpResponse response = doGet("/" + REGION_NAME + "/key1", "key1User", "1234567"); - assertTrue(isOK(response)); - }); - // Test an unauthorized user - client1.invoke(() -> { - HttpResponse response = doGet("/" + REGION_NAME + "/key1", "dataWriter", "1234567"); - assertEquals(403, getCode(response)); - }); - } - - /** - * Test permissions on deleting a region's key(s) - */ - @Test - public void deleteRegionKey() { - // Test an unknown user - 401 error - client1.invoke(() -> { - HttpResponse response = doDelete("/" + REGION_NAME + "/key1", "unknown-user", "1234567"); - assertEquals(401, getCode(response)); - }); - - // Test a user with insufficient rights - 403 - client1.invoke(() -> { - HttpResponse response = doDelete("/" + REGION_NAME + "/key1", "dataReader", "1234567"); - assertEquals(403, getCode(response)); - }); - - // Test an authorized user - 200 - client1.invoke(() -> { - HttpResponse response = doDelete("/" + REGION_NAME + "/key1", "key1User", "1234567"); - assertTrue(isOK(response)); - }); - } - - /** - * Test permissions on deleting a region's key(s) - */ - @Test - public void postRegionKey() { - // Test an unknown user - 401 error - client1.invoke(() -> { - HttpResponse response = doPost("/" + REGION_NAME + "?key9", "unknown", "1234567", "{ \"key9\" : \"foo\" }"); - assertEquals(401, getCode(response)); - }); - - // Test a user with insufficient rights - 403 - client1.invoke(() -> { - HttpResponse response = doPost("/" + REGION_NAME + "?key9", "dataReader", "1234567", "{ \"key9\" : \"foo\" }"); - assertEquals(403, getCode(response)); - }); - - // Test an authorized user - 200 - client1.invoke(() -> { - HttpResponse response = doPost("/" + REGION_NAME + "?key9", "dataWriter", "1234567", "{ \"key9\" : \"foo\" }"); - assertEquals(201, getCode(response)); - assertTrue(isOK(response)); - }); - } - - /** - * Test permissions on deleting a region's key(s) - */ - @Test - public void putRegionKey() { - - String json = "{\"@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.b...@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225}"; - String casJSON = "{\"@old\":{\"@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.b...@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":1,\"description\":\"Product-100\",\"quantity\":12,\"unitPrice\":5,\"totalPrice\":60}],\"totalPrice\":225},\"@new \":{\"@type\":\"com.gemstone.gemfire.web.rest.domain.Order\",\"purchaseOrderNo\":1121,\"customerId\":1013,\"description\":\"Order for New Corp\",\"orderDate\":\"02/10/2014\",\"deliveryDate\":\"02/25/2014\",\"contact\":\"Vanilla Bean\",\"email\":\"vanillab...@example.com\",\"phone\":\"01-2048096\",\"items\":[{\"itemNo\":12345,\"description\":\"part 123\",\"quantity\":12,\"unitPrice\":29.99,\"totalPrice\":149.95}],\"totalPrice\":149.95}}"; - // Test an unknown user - 401 error - client1.invoke(() -> { - HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=PUT", "unknown-user", "1234567", "{ \"key9\" : \"foo\" }"); - assertEquals(401, getCode(response)); - }); - - client1.invoke(() -> { - HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=CAS", "unknown-user", "1234567", "{ \"key9\" : \"foo\" }"); - assertEquals(401, getCode(response)); - }); - - client1.invoke(() -> { - HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=REPLACE", "unknown-user", "1234567", "{ \"@old\" : \"value1\", \"@new\" : \"CASvalue\" }"); - assertEquals(401, getCode(response)); - }); - - // Test a user with insufficient rights - 403 - client1.invoke(() -> { - HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=PUT", "dataReader", "1234567", "{ \"key1\" : \"foo\" }"); - assertEquals(403, getCode(response)); - }); - client1.invoke(() -> { - HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=REPLACE", "dataReader", "1234567", "{ \"key1\" : \"foo\" }"); - assertEquals(403, getCode(response)); - }); - client1.invoke(() -> { - HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=CAS", "dataReader", "1234567", casJSON); - assertEquals(403, getCode(response)); - }); - - // Test an authorized user - 200 - client1.invoke(() -> { - HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=PUT", "key1User", "1234567", "{ \"key1\" : \"foo\" }"); - assertEquals(200, getCode(response)); - assertTrue(isOK(response)); - }); - client1.invoke(() -> { - HttpResponse response = doPut("/" + REGION_NAME + "/key1?op=REPLACE", "key1User", "1234567", json); - assertEquals(200, getCode(response)); - assertTrue(isOK(response)); - }); - } -} http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java index a515de5..79b70f8 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java @@ -27,6 +27,18 @@ import java.util.concurrent.Callable; import org.apache.commons.lang.SerializationException; import org.apache.commons.lang.StringUtils; +import org.apache.logging.log4j.Logger; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.ShiroException; +import org.apache.shiro.config.Ini.Section; +import org.apache.shiro.config.IniSecurityManagerFactory; +import org.apache.shiro.mgt.DefaultSecurityManager; +import org.apache.shiro.realm.Realm; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.subject.support.SubjectThreadState; +import org.apache.shiro.util.ThreadContext; +import org.apache.shiro.util.ThreadState; + import org.apache.geode.GemFireIOException; import org.apache.geode.internal.cache.EntryEventImpl; import org.apache.geode.internal.logging.LogService; @@ -44,17 +56,6 @@ import org.apache.geode.security.ResourcePermission; import org.apache.geode.security.ResourcePermission.Operation; import org.apache.geode.security.ResourcePermission.Resource; import org.apache.geode.security.SecurityManager; -import org.apache.logging.log4j.Logger; -import org.apache.shiro.SecurityUtils; -import org.apache.shiro.ShiroException; -import org.apache.shiro.config.Ini.Section; -import org.apache.shiro.config.IniSecurityManagerFactory; -import org.apache.shiro.mgt.DefaultSecurityManager; -import org.apache.shiro.realm.Realm; -import org.apache.shiro.subject.Subject; -import org.apache.shiro.subject.support.SubjectThreadState; -import org.apache.shiro.util.ThreadContext; -import org.apache.shiro.util.ThreadState; public class IntegratedSecurityService implements SecurityService{ @@ -270,11 +271,11 @@ public class IntegratedSecurityService implements SecurityService{ authorize(resource, operation, null); } - private void authorize(String resource, String operation, String regionName){ + public void authorize(String resource, String operation, String regionName){ authorize(resource, operation, regionName, null); } - private void authorize(String resource, String operation, String regionName, String key) { + public void authorize(String resource, String operation, String regionName, String key) { regionName = StringUtils.stripStart(regionName, "/"); authorize(new ResourcePermission(resource, operation, regionName, key)); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java index d645bbf..7f5a34e 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java +++ b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java @@ -20,6 +20,9 @@ import java.lang.reflect.Method; import java.util.Properties; import java.util.concurrent.Callable; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadState; + import org.apache.geode.internal.ClassLoadUtil; import org.apache.geode.management.internal.security.ResourceConstants; import org.apache.geode.management.internal.security.ResourceOperation; @@ -27,8 +30,6 @@ import org.apache.geode.security.GemFireSecurityException; import org.apache.geode.security.PostProcessor; import org.apache.geode.security.ResourcePermission; import org.apache.geode.security.SecurityManager; -import org.apache.shiro.subject.Subject; -import org.apache.shiro.util.ThreadState; public interface SecurityService { @@ -52,6 +53,8 @@ public interface SecurityService { void authorizeRegionRead(String regionName); void authorizeRegionRead(String regionName, String key); void authorize(String resource, String operation); + void authorize(String resource, String operation, String regionName); + void authorize(String resource, String operation, String regionName, String key); void authorize(ResourcePermission context); void initSecurity(Properties securityProps); void close(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java b/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java index d6be540..0112e39 100644 --- a/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java +++ b/geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java @@ -60,8 +60,8 @@ public class ResourcePermission extends WildcardPermission { } public ResourcePermission(String resource, String operation, String regionName, String key) { - this((resource==null) ? Resource.NULL : Resource.valueOf(resource), - (operation == null) ? Operation.NULL : Operation.valueOf(operation), + this((resource==null) ? Resource.NULL : Resource.valueOf(resource.toUpperCase()), + (operation == null) ? Operation.NULL : Operation.valueOf(operation.toUpperCase()), regionName, key); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java index ee5d714..ffa0b13 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java @@ -39,6 +39,21 @@ import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.logging.log4j.Logger; +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; +import org.json.JSONTokener; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; +import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; +import org.springframework.web.servlet.support.ServletUriComponentsBuilder; + import org.apache.geode.SerializationException; import org.apache.geode.cache.Cache; import org.apache.geode.cache.CacheLoaderException; @@ -54,8 +69,6 @@ import org.apache.geode.distributed.internal.DistributionManager; import org.apache.geode.distributed.internal.membership.InternalDistributedMember; import org.apache.geode.internal.cache.GemFireCacheImpl; import org.apache.geode.internal.logging.LogService; -import org.apache.geode.internal.security.IntegratedSecurityService; -import org.apache.geode.internal.security.SecurityService; import org.apache.geode.pdx.JSONFormatter; import org.apache.geode.pdx.JSONFormatterException; import org.apache.geode.pdx.PdxInstance; @@ -71,20 +84,6 @@ import org.apache.geode.rest.internal.web.util.IdentifiableUtils; import org.apache.geode.rest.internal.web.util.JSONUtils; import org.apache.geode.rest.internal.web.util.NumberUtils; import org.apache.geode.rest.internal.web.util.ValidationUtils; -import org.apache.logging.log4j.Logger; -import org.json.JSONArray; -import org.json.JSONException; -import org.json.JSONObject; -import org.json.JSONTokener; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; -import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; -import org.springframework.util.CollectionUtils; -import org.springframework.util.StringUtils; -import org.springframework.web.servlet.support.ServletUriComponentsBuilder; /** @@ -104,7 +103,6 @@ public abstract class AbstractBaseController { protected static final String UTF_8 = "UTF-8"; protected static final String DEFAULT_ENCODING = UTF_8; private static final AtomicLong ID_SEQUENCE = new AtomicLong(0l); - protected SecurityService securityService = IntegratedSecurityService.getSecurityService(); //private Cache cache = GemFireCacheImpl.getExisting(null); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java index 54dd9e5..cbc19a7 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java @@ -20,16 +20,7 @@ package org.apache.geode.rest.internal.web.controllers; import java.io.PrintWriter; import java.io.StringWriter; -import org.apache.geode.internal.logging.LogService; -import org.apache.geode.rest.internal.web.exception.DataTypeNotSupportedException; -import org.apache.geode.rest.internal.web.exception.GemfireRestException; -import org.apache.geode.rest.internal.web.exception.MalformedJsonException; -import org.apache.geode.rest.internal.web.exception.RegionNotFoundException; -import org.apache.geode.rest.internal.web.exception.ResourceNotFoundException; -import org.apache.geode.security.AuthenticationFailedException; -import org.apache.geode.security.NotAuthorizedException; import org.apache.logging.log4j.Logger; -import org.apache.shiro.authc.AuthenticationException; import org.springframework.http.HttpStatus; import org.springframework.security.access.AccessDeniedException; import org.springframework.web.HttpRequestMethodNotSupportedException; @@ -38,6 +29,14 @@ import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.ResponseStatus; +import org.apache.geode.internal.logging.LogService; +import org.apache.geode.rest.internal.web.exception.DataTypeNotSupportedException; +import org.apache.geode.rest.internal.web.exception.GemfireRestException; +import org.apache.geode.rest.internal.web.exception.MalformedJsonException; +import org.apache.geode.rest.internal.web.exception.RegionNotFoundException; +import org.apache.geode.rest.internal.web.exception.ResourceNotFoundException; +import org.apache.geode.security.NotAuthorizedException; + /** * The CrudControllerAdvice class handles exception thrown while serving the REST request @@ -123,32 +122,6 @@ public class BaseControllerAdvice extends AbstractBaseController { public String handleException(final HttpRequestMethodNotSupportedException e) { return convertErrorAsJson(e.getMessage()); } - - /** - * Handles an AuthenticationFailedException thrown by a REST API web service endpoint, HTTP request handler method. - * <p/> - * @param cause the Exception causing the error. - * @return a ResponseEntity with an appropriate HTTP status code (403 - Forbidden) - */ - @ExceptionHandler(AuthenticationFailedException.class) - @ResponseBody - @ResponseStatus(HttpStatus.UNAUTHORIZED) - public String handleException(final AuthenticationFailedException cause) { - return convertErrorAsJson(cause.getMessage()); - } - - /** - * Handles an AuthenticationException thrown by a REST API web service endpoint, HTTP request handler method. - * <p/> - * @param cause the Exception causing the error. - * @return a ResponseEntity with an appropriate HTTP status code (403 - Forbidden) - */ - @ExceptionHandler(AuthenticationException.class) - @ResponseBody - @ResponseStatus(HttpStatus.UNAUTHORIZED) - public String handleException(final AuthenticationException cause) { - return convertErrorAsJson(cause.getMessage()); - } /** * Handles an AccessDenied Exception thrown by a REST API web service endpoint, HTTP request handler method. http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java index 232f034..35d8fb4 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java @@ -24,6 +24,17 @@ import java.util.Set; import com.wordnik.swagger.annotations.ApiOperation; import com.wordnik.swagger.annotations.ApiResponse; import com.wordnik.swagger.annotations.ApiResponses; +import org.apache.logging.log4j.Logger; +import org.json.JSONException; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; + import org.apache.geode.cache.LowMemoryException; import org.apache.geode.cache.Region; import org.apache.geode.cache.execute.Execution; @@ -36,15 +47,6 @@ import org.apache.geode.rest.internal.web.controllers.support.RestServersResultC import org.apache.geode.rest.internal.web.exception.GemfireRestException; import org.apache.geode.rest.internal.web.util.ArrayUtils; import org.apache.geode.rest.internal.web.util.JSONUtils; -import org.apache.logging.log4j.Logger; -import org.json.JSONException; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpStatus; -import org.springframework.http.MediaType; -import org.springframework.http.ResponseEntity; -import org.springframework.web.bind.annotation.PathVariable; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestMethod; /** @@ -75,11 +77,9 @@ public abstract class CommonCrudController extends AbstractBaseController { @ApiResponse( code = 403, message = "Insufficient privileges for operation." ), @ApiResponse( code = 500, message = "GemFire throws an error or exception." ) } ) + @PreAuthorize("@securityService.authorize('DATA', 'READ')") public ResponseEntity<?> regions() { - securityService.authorizeDataRead(); - if(logger.isDebugEnabled()){ - logger.debug("Listing all resources (Regions) in GemFire..."); - } + logger.debug("Listing all resources (Regions) in GemFire..."); final HttpHeaders headers = new HttpHeaders(); headers.setLocation(toUri()); final Set<Region<?, ?>> regions = getCache().rootRegions(); @@ -106,12 +106,10 @@ public abstract class CommonCrudController extends AbstractBaseController { @ApiResponse( code = 404, message = "Region does not exist" ), @ApiResponse( code = 500, message = "GemFire throws an error or exception" ) } ) + @PreAuthorize("@securityService.authorize('DATA', 'READ', #region)") public ResponseEntity<?> keys(@PathVariable("region") String region){ - securityService.authorizeRegionRead(region); - if(logger.isDebugEnabled()){ - logger.debug("Reading all Keys in Region ({})...", region); - } - + logger.debug("Reading all Keys in Region ({})...", region); + region = decode(region); Object[] keys = getKeys(region, null); @@ -142,14 +140,11 @@ public abstract class CommonCrudController extends AbstractBaseController { @ApiResponse( code = 404, message = "Region or key(s) does not exist" ), @ApiResponse( code = 500, message = "GemFire throws an error or exception" ) } ) + @PreAuthorize("@securityService.authorizeKeys('WRITE', #region, #keys)") public ResponseEntity<?> delete(@PathVariable("region") String region, @PathVariable("keys") final String[] keys){ - for (String key : keys) - securityService.authorizeRegionWrite(region, key); - if(logger.isDebugEnabled()){ - logger.debug("Delete data for key {} on region {}", ArrayUtils.toString((Object[])keys), region); - } - + logger.debug("Delete data for key {} on region {}", ArrayUtils.toString((Object[])keys), region); + region = decode(region); deleteValues(region, (Object[])keys); @@ -174,11 +169,9 @@ public abstract class CommonCrudController extends AbstractBaseController { @ApiResponse( code = 404, message = "Region does not exist" ), @ApiResponse( code = 500, message = "if GemFire throws an error or exception" ) } ) + @PreAuthorize("@securityService.authorize('DATA', 'WRITE', #regon)") public ResponseEntity<?> delete(@PathVariable("region") String region) { - securityService.authorizeRegionWrite(region); - if(logger.isDebugEnabled()){ - logger.debug("Deleting all data in Region ({})...", region); - } + logger.debug("Deleting all data in Region ({})...", region); region = decode(region); @@ -216,11 +209,9 @@ public abstract class CommonCrudController extends AbstractBaseController { @ApiResponse( code = 403, message = "Insufficient privileges for operation." ), @ApiResponse( code = 500, message = "if GemFire throws an error or exception" ) } ) + @PreAuthorize("@securityService.authorize('CLUSTER', 'READ')") public ResponseEntity<?> servers() { - securityService.authorizeClusterRead(); - if(logger.isDebugEnabled()){ - logger.debug("Executing function to get REST enabled gemfire nodes in the DS!"); - } + logger.debug("Executing function to get REST enabled gemfire nodes in the DS!"); Execution function; try { http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java index 8cec110..e1ea1ad 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java @@ -26,22 +26,13 @@ import com.wordnik.swagger.annotations.Api; import com.wordnik.swagger.annotations.ApiOperation; import com.wordnik.swagger.annotations.ApiResponse; import com.wordnik.swagger.annotations.ApiResponses; -import org.apache.geode.cache.LowMemoryException; -import org.apache.geode.cache.execute.Execution; -import org.apache.geode.cache.execute.Function; -import org.apache.geode.cache.execute.FunctionException; -import org.apache.geode.cache.execute.FunctionService; -import org.apache.geode.cache.execute.ResultCollector; -import org.apache.geode.internal.logging.LogService; -import org.apache.geode.rest.internal.web.exception.GemfireRestException; -import org.apache.geode.rest.internal.web.util.ArrayUtils; -import org.apache.geode.rest.internal.web.util.JSONUtils; import org.apache.logging.log4j.Logger; import org.json.JSONException; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Controller; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.PathVariable; @@ -52,6 +43,17 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.ResponseStatus; +import org.apache.geode.cache.LowMemoryException; +import org.apache.geode.cache.execute.Execution; +import org.apache.geode.cache.execute.Function; +import org.apache.geode.cache.execute.FunctionException; +import org.apache.geode.cache.execute.FunctionService; +import org.apache.geode.cache.execute.ResultCollector; +import org.apache.geode.internal.logging.LogService; +import org.apache.geode.rest.internal.web.exception.GemfireRestException; +import org.apache.geode.rest.internal.web.util.ArrayUtils; +import org.apache.geode.rest.internal.web.util.JSONUtils; + /** * The FunctionsController class serving REST Requests related to the function execution * @see org.springframework.stereotype.Controller @@ -98,11 +100,9 @@ public class FunctionAccessController extends AbstractBaseController { }) @ResponseBody @ResponseStatus(HttpStatus.OK) + @PreAuthorize("@securityService.authorize('DATA', 'READ')") public ResponseEntity<?> list() { - securityService.authorizeDataRead(); - if (logger.isDebugEnabled()) { - logger.debug("Listing all registered Functions in GemFire..."); - } + logger.debug("Listing all registered Functions in GemFire..."); final Map<String, Function> registeredFunctions = FunctionService.getRegisteredFunctions(); String listFunctionsAsJson = JSONUtils.formulateJsonForListFunctionsCall(registeredFunctions.keySet()); @@ -138,6 +138,7 @@ public class FunctionAccessController extends AbstractBaseController { }) @ResponseBody @ResponseStatus(HttpStatus.OK) + @PreAuthorize("@securityService.authorize('DATA', 'WRITE')") public ResponseEntity<String> execute(@PathVariable("functionId") String functionId, @RequestParam(value = "onRegion", required = false) String region, @RequestParam(value = "onMembers", required = false) final String[] members, @@ -145,15 +146,13 @@ public class FunctionAccessController extends AbstractBaseController { @RequestParam(value = "filter", required = false) final String[] filter, @RequestBody(required = false) final String argsInBody ) { - securityService.authorizeDataWrite(); Execution function = null; functionId = decode(functionId); if (StringUtils.hasText(region)) { - if (logger.isDebugEnabled()) { - logger.debug("Executing Function ({}) with arguments ({}) on Region ({})...", functionId, - ArrayUtils.toString(argsInBody), region); - } + logger.debug("Executing Function ({}) with arguments ({}) on Region ({})...", functionId, + ArrayUtils.toString(argsInBody), region); + region = decode(region); try { @@ -162,20 +161,18 @@ public class FunctionAccessController extends AbstractBaseController { throw new GemfireRestException(String.format("The Region identified by name (%1$s) could not found!", region), fe); } } else if (ArrayUtils.isNotEmpty(members)) { - if (logger.isDebugEnabled()) { - logger.debug("Executing Function ({}) with arguments ({}) on Member ({})...", functionId, + logger.debug("Executing Function ({}) with arguments ({}) on Member ({})...", functionId, ArrayUtils.toString(argsInBody), ArrayUtils.toString(members)); - } + try { function = FunctionService.onMembers(getMembers(members)); } catch (FunctionException fe) { throw new GemfireRestException("Could not found the specified members in distributed system!", fe); } } else if (ArrayUtils.isNotEmpty(groups)) { - if (logger.isDebugEnabled()) { - logger.debug("Executing Function ({}) with arguments ({}) on Groups ({})...", functionId, + logger.debug("Executing Function ({}) with arguments ({}) on Groups ({})...", functionId, ArrayUtils.toString(argsInBody), ArrayUtils.toString(groups)); - } + try { function = FunctionService.onMembers(groups); } catch (FunctionException fe) { @@ -183,10 +180,8 @@ public class FunctionAccessController extends AbstractBaseController { } } else { //Default case is to execute function on all existing data node in DS, document this. - if (logger.isDebugEnabled()) { - logger.debug("Executing Function ({}) with arguments ({}) on all Members...", functionId, + logger.debug("Executing Function ({}) with arguments ({}) on all Members...", functionId, ArrayUtils.toString(argsInBody)); - } try { function = FunctionService.onMembers(getAllMembersInDS()); @@ -196,10 +191,9 @@ public class FunctionAccessController extends AbstractBaseController { } if (!ArrayUtils.isEmpty(filter)) { - if (logger.isDebugEnabled()) { - logger.debug("Executing Function ({}) with filter ({})", functionId, + logger.debug("Executing Function ({}) with filter ({})", functionId, ArrayUtils.toString(filter)); - } + Set filter1 = ArrayUtils.asSet(filter); function = function.withFilter(filter1); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java index c05845a..3003b3d 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java @@ -24,17 +24,12 @@ import com.wordnik.swagger.annotations.Api; import com.wordnik.swagger.annotations.ApiOperation; import com.wordnik.swagger.annotations.ApiResponse; import com.wordnik.swagger.annotations.ApiResponses; -import org.apache.geode.internal.logging.LogService; -import org.apache.geode.rest.internal.web.controllers.support.JSONTypes; -import org.apache.geode.rest.internal.web.controllers.support.RegionData; -import org.apache.geode.rest.internal.web.controllers.support.RegionEntryData; -import org.apache.geode.rest.internal.web.exception.ResourceNotFoundException; -import org.apache.geode.rest.internal.web.util.ArrayUtils; import org.apache.logging.log4j.Logger; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Controller; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.PathVariable; @@ -43,6 +38,13 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestParam; +import org.apache.geode.internal.logging.LogService; +import org.apache.geode.rest.internal.web.controllers.support.JSONTypes; +import org.apache.geode.rest.internal.web.controllers.support.RegionData; +import org.apache.geode.rest.internal.web.controllers.support.RegionEntryData; +import org.apache.geode.rest.internal.web.exception.ResourceNotFoundException; +import org.apache.geode.rest.internal.web.util.ArrayUtils; + /** * The PdxBasedCrudController class serving REST Requests related to the REST CRUD operation on region * <p/> @@ -93,16 +95,15 @@ public class PdxBasedCrudController extends CommonCrudController { @ApiResponse( code = 409, message = "Key already exist in region."), @ApiResponse( code = 500, message = "GemFire throws an error or exception.") } ) + @PreAuthorize("@securityService.authorize('DATA', 'WRITE', #region)") public ResponseEntity<?> create(@PathVariable("region") String region, @RequestParam(value = "key", required = false) String key, @RequestBody final String json) { - securityService.authorizeRegionWrite(region); key = generateKey(key); - - if(logger.isDebugEnabled()){ - logger.debug("Posting (creating/putIfAbsent) JSON document ({}) to Region ({}) with Key ({})...", + + logger.debug("Posting (creating/putIfAbsent) JSON document ({}) to Region ({}) with Key ({})...", json, region, key); - } + region = decode(region); Object existingPdxObj = null; @@ -147,12 +148,11 @@ public class PdxBasedCrudController extends CommonCrudController { @ApiResponse( code = 404, message = "Region does not exist." ), @ApiResponse( code = 500, message = "GemFire throws an error or exception.") } ) + @PreAuthorize("@securityService.authorize('DATA', 'READ', #region)") public ResponseEntity<?> read(@PathVariable("region") String region, @RequestParam(value = "limit", defaultValue = DEFAULT_GETALL_RESULT_LIMIT) final String limit) { - securityService.authorizeRegionRead(region); - if(logger.isDebugEnabled()){ - logger.debug("Reading all data in Region ({})...", region); - } + logger.debug("Reading all data in Region ({})...", region); + region = decode(region); Map<Object, Object> valueObjs = null; @@ -227,17 +227,13 @@ public class PdxBasedCrudController extends CommonCrudController { @ApiResponse( code = 404, message = "Region does not exist." ), @ApiResponse( code = 500, message = "GemFire throws an error or exception.") } ) + @PreAuthorize("@securityService.authorizeKeys('READ', #region, #keys)") public ResponseEntity<?> read( @PathVariable("region") String region, @PathVariable("keys") final String[] keys, @RequestParam(value = "ignoreMissingKey", required = false ) final String ignoreMissingKey) { - - for (String key : keys) - securityService.authorizeRegionRead(region, key); - if(logger.isDebugEnabled()){ - logger.debug("Reading data for keys ({}) in Region ({})", + logger.debug("Reading data for keys ({}) in Region ({})", ArrayUtils.toString(keys), region); - } final HttpHeaders headers = new HttpHeaders(); region = decode(region); @@ -315,15 +311,13 @@ public class PdxBasedCrudController extends CommonCrudController { @ApiResponse( code = 409, message = "For CAS, @old value does not match to the current value in region" ), @ApiResponse( code = 500, message = "GemFire throws an error or exception.") } ) + @PreAuthorize("@securityService.authorizeKeys('WRITE', #region, #keys)") public ResponseEntity<?> update(@PathVariable("region") String region, @PathVariable("keys") final String[] keys, @RequestParam(value = "op", defaultValue = "PUT") final String opValue, @RequestBody final String json) { - for (String key : keys) - securityService.authorizeRegionWrite(region, key); - if(logger.isDebugEnabled()){ - logger.debug("updating key(s) for region ({}) ", region); - } + logger.debug("updating key(s) for region ({}) ", region); + region = decode(region); if(keys.length > 1){ @@ -349,11 +343,10 @@ public class PdxBasedCrudController extends CommonCrudController { @ApiResponse( code = 404, message = "Region does not exist." ), @ApiResponse( code = 500, message = "GemFire throws an error or exception.") } ) + @PreAuthorize("@securityService.authorize('DATA', 'READ', #region)") public ResponseEntity<?> size(@PathVariable("region") String region) { - securityService.authorizeRegionRead(region); - if(logger.isDebugEnabled()){ - logger.debug("Determining the number of entries in Region ({})...", region); - } + logger.debug("Determining the number of entries in Region ({})...", region); + region = decode(region); final HttpHeaders headers = new HttpHeaders(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java index 2df95aa..e43e5e6 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java @@ -23,6 +23,21 @@ import com.wordnik.swagger.annotations.Api; import com.wordnik.swagger.annotations.ApiOperation; import com.wordnik.swagger.annotations.ApiResponse; import com.wordnik.swagger.annotations.ApiResponses; +import org.apache.logging.log4j.Logger; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.ResponseStatus; + import org.apache.geode.cache.Region; import org.apache.geode.cache.query.FunctionDomainException; import org.apache.geode.cache.query.NameResolutionException; @@ -38,19 +53,6 @@ import org.apache.geode.rest.internal.web.exception.GemfireRestException; import org.apache.geode.rest.internal.web.exception.ResourceNotFoundException; import org.apache.geode.rest.internal.web.util.JSONUtils; import org.apache.geode.rest.internal.web.util.ValidationUtils; -import org.apache.logging.log4j.Logger; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpStatus; -import org.springframework.http.MediaType; -import org.springframework.http.ResponseEntity; -import org.springframework.stereotype.Controller; -import org.springframework.web.bind.annotation.PathVariable; -import org.springframework.web.bind.annotation.RequestBody; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestMethod; -import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.bind.annotation.ResponseBody; -import org.springframework.web.bind.annotation.ResponseStatus; /** @@ -103,12 +105,10 @@ public class QueryAccessController extends AbstractBaseController { } ) @ResponseBody @ResponseStatus(HttpStatus.OK) + @PreAuthorize("@securityService.authorize('DATA', 'READ')") public ResponseEntity<?> list() { - securityService.authorizeDataRead(); - if (logger.isDebugEnabled()) { - logger.debug("Listing all parametrized Queries in GemFire..."); - } - + logger.debug("Listing all parametrized Queries in GemFire..."); + final Region<String, String> parametrizedQueryRegion = getQueryStore(PARAMETERIZED_QUERIES_REGION); String queryListAsJson = JSONUtils.formulateJsonForListQueriesCall(parametrizedQueryRegion); @@ -137,16 +137,14 @@ public class QueryAccessController extends AbstractBaseController { @ApiResponse( code = 409, message = "QueryId already assigned to other query." ), @ApiResponse( code = 500, message = "GemFire throws an error or exception." ) } ) + @PreAuthorize("@securityService.authorize('DATA', 'WRITE')") public ResponseEntity<?> create(@RequestParam("id") final String queryId, @RequestParam(value = "q", required = false) String oqlInUrl, @RequestBody(required = false) final String oqlInBody) { - securityService.authorizeDataWrite(); final String oqlStatement = validateQuery(oqlInUrl, oqlInBody); - - if (logger.isDebugEnabled()) { - logger.debug("Creating a named, parametrized Query ({}) with ID ({})...", oqlStatement, queryId); - } + logger.debug("Creating a named, parametrized Query ({}) with ID ({})...", oqlStatement, queryId); + // store the compiled OQL statement with 'queryId' as the Key into the hidden, ParameterizedQueries Region... final String existingOql = createNamedQuery(PARAMETERIZED_QUERIES_REGION, queryId, oqlStatement); @@ -181,11 +179,10 @@ public class QueryAccessController extends AbstractBaseController { } ) @ResponseBody @ResponseStatus(HttpStatus.OK) + @PreAuthorize("@securityService.authorize('DATA', 'READ')") public ResponseEntity<String> runAdhocQuery(@RequestParam("q") String oql) { - securityService.authorizeDataRead(); - if (logger.isDebugEnabled()) { - logger.debug("Running an adhoc Query ({})...", oql); - } + logger.debug("Running an adhoc Query ({})...", oql); + oql = decode(oql); final Query query = getQueryService().newQuery(oql); @@ -237,13 +234,12 @@ public class QueryAccessController extends AbstractBaseController { } ) @ResponseBody @ResponseStatus(HttpStatus.OK) + @PreAuthorize("@securityService.authorize('DATA', 'WRITE')") public ResponseEntity<String> runNamedQuery(@PathVariable("query") String queryId, @RequestBody String arguments) { - securityService.authorizeDataWrite(); - if (logger.isDebugEnabled()) { - logger.debug("Running named Query with ID ({})...", queryId); - } + logger.debug("Running named Query with ID ({})...", queryId); + queryId = decode(queryId); if (arguments != null) { @@ -314,15 +310,14 @@ public class QueryAccessController extends AbstractBaseController { @ApiResponse( code = 404, message = "queryId does not exist." ), @ApiResponse( code = 500, message = "GemFire throws an error or exception." ) } ) + @PreAuthorize("@securityService.authorize('DATA', 'WRITE')") public ResponseEntity<?> update( @PathVariable("query") final String queryId, @RequestParam(value = "q", required = false) String oqlInUrl, @RequestBody(required = false) final String oqlInBody) { - securityService.authorizeDataWrite(); final String oqlStatement = validateQuery(oqlInUrl, oqlInBody); - if (logger.isDebugEnabled()) { - logger.debug("Updating a named, parametrized Query ({}) with ID ({})...", oqlStatement, queryId); - } + logger.debug("Updating a named, parametrized Query ({}) with ID ({})...", oqlStatement, queryId); + // update the OQL statement with 'queryId' as the Key into the hidden, ParameterizedQueries Region... checkForQueryIdExist(PARAMETERIZED_QUERIES_REGION, queryId); @@ -350,12 +345,10 @@ public class QueryAccessController extends AbstractBaseController { @ApiResponse( code = 404, message = "queryId does not exist." ), @ApiResponse( code = 500, message = "GemFire throws an error or exception" ) } ) + @PreAuthorize("@securityService.authorize('DATA', 'WRITE')") public ResponseEntity<?> delete(@PathVariable("query") final String queryId) { - securityService.authorizeDataWrite(); - if (logger.isDebugEnabled()) { - logger.debug("Deleting a named, parametrized Query with ID ({}).", queryId); - } - + logger.debug("Deleting a named, parametrized Query with ID ({}).", queryId); + //delete the OQL statement with 'queryId' as the Key into the hidden, // ParameterizedQueries Region... deleteNamedQuery(PARAMETERIZED_QUERIES_REGION, queryId); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthentication.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthentication.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthentication.java deleted file mode 100644 index c4226f6..0000000 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthentication.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * 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.security; - -import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; -import org.springframework.security.core.authority.AuthorityUtils; - -class GeodeAuthentication extends UsernamePasswordAuthenticationToken { - /** - * This constructor should only be used by <code>AuthenticationManager</code> or <code>AuthenticationProvider</code> - * implementations that are satisfied with producing a trusted (i.e. {@link #isAuthenticated()} = <code>true</code>) - * authentication token. - * @param principal - * @param credentials - */ - public GeodeAuthentication(final Object principal, - final Object credentials) { - super(principal, credentials, AuthorityUtils.NO_AUTHORITIES); - } - -} http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java index c482047..79fcc8f 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java @@ -18,20 +18,21 @@ package org.apache.geode.rest.internal.web.security; -import org.apache.shiro.subject.Subject; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.stereotype.Component; -import org.apache.geode.internal.security.IntegratedSecurityService; -import org.apache.geode.security.AuthenticationFailedException; +import org.apache.geode.internal.security.SecurityService; +import org.apache.geode.security.GemFireSecurityException; @Component public class GeodeAuthenticationProvider implements AuthenticationProvider { + private SecurityService securityService = SecurityService.getSecurityService(); @Override public Authentication authenticate(Authentication authentication) throws AuthenticationException { @@ -39,18 +40,16 @@ public class GeodeAuthenticationProvider implements AuthenticationProvider { String password = authentication.getCredentials().toString(); try { - Subject subject = IntegratedSecurityService.getSecurityService().login(username, password); - if (subject != null) { - return new GeodeAuthentication(subject.getPrincipal(), authentication.getCredentials()); - } - } catch (AuthenticationFailedException authFailedEx) { - throw new BadCredentialsException("Invalid username or password"); + securityService.login(username, password); + return new UsernamePasswordAuthenticationToken(username, password, AuthorityUtils.NO_AUTHORITIES); + } + catch (GemFireSecurityException e){ + throw new BadCredentialsException(e.getLocalizedMessage(), e); } - return authentication; } @Override public boolean supports(Class<?> authentication) { - return authentication.equals(UsernamePasswordAuthenticationToken.class); + return authentication.isAssignableFrom(UsernamePasswordAuthenticationToken.class); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/29e49480/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthority.java ---------------------------------------------------------------------- diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthority.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthority.java deleted file mode 100644 index fd21628..0000000 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthority.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * 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.security; - -import org.springframework.security.core.GrantedAuthority; - -public class GeodeAuthority implements GrantedAuthority { - - private String authority; - - GeodeAuthority(String authority) { - this.authority = authority; - } - - /** - * If the <code>GrantedAuthority</code> can be represented as a <code>String</code> and that - * <code>String</code> is sufficient in precision to be relied upon for an access control decision by an {@link - * AccessDecisionManager} (or delegate), this method should return such a <code>String</code>. - * <p> - * If the <code>GrantedAuthority</code> cannot be expressed with sufficient precision as a <code>String</code>, - * <code>null</code> should be returned. Returning <code>null</code> will require an - * <code>AccessDecisionManager</code> (or delegate) to specifically support the <code>GrantedAuthority</code> - * implementation, so returning <code>null</code> should be avoided unless actually required. - * @return a representation of the granted authority (or <code>null</code> if the granted authority cannot be - * expressed as a <code>String</code> with sufficient precision). - */ - @Override - public String getAuthority() { - return authority; - } -}