This is an automated email from the ASF dual-hosted git repository.
jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 3493e8c GEODE-6185: management rest end point returns correct status
code and message
3493e8c is described below
commit 3493e8c8bfab0b7373f517f78762ea0921d7ef3c
Author: jinmeiliao <[email protected]>
AuthorDate: Tue Jan 29 10:31:17 2019 -0800
GEODE-6185: management rest end point returns correct status code and
message
Co-authored-by: Jens Deppe <[email protected]>
---
.../RegionManagementSecurityIntegrationTest.java | 21 +++++---
.../internal/web/security/RestSecurityService.java | 14 ++---
.../controllers/AbstractManagementController.java | 63 ----------------------
...roller.java => ManagementControllerAdvice.java} | 41 ++------------
.../rest/security/RestSecurityConfiguration.java | 36 +++++++++++--
.../rest/security/RestSecurityService.java | 14 ++---
6 files changed, 62 insertions(+), 127 deletions(-)
diff --git
a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementSecurityIntegrationTest.java
b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementSecurityIntegrationTest.java
index ed991d0..831c2e1 100644
---
a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementSecurityIntegrationTest.java
+++
b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementSecurityIntegrationTest.java
@@ -63,23 +63,32 @@ public class RegionManagementSecurityIntegrationTest {
@Test
public void sanityCheck_not_authorized() throws Exception {
ClusterManagementResult result =
- restClient.doPostAndAssert("/regions", json, "test", "test")
+ restClient.doPostAndAssert("/regions", json, "user", "user")
.hasStatusCode(403)
.getClusterManagementResult();
assertThat(result.isSuccessful()).isFalse();
- assertThat(result.getPersistenceStatus().getMessage()).isEqualTo("Access
is denied");
+ assertThat(result.getPersistenceStatus().getMessage())
+ .isEqualTo("user not authorized for DATA:MANAGE");
}
@Test
public void sanityCheckWithNoCredentials() throws Exception {
- restClient.doPostAndAssert("/regions", json, null, null)
- .hasStatusCode(401);
+ ClusterManagementResult result =
+ restClient.doPostAndAssert("/regions", json, null, null)
+ .hasStatusCode(401).getClusterManagementResult();
+ assertThat(result.getPersistenceStatus().getMessage())
+ .isEqualTo("Full authentication is required to access this resource");
+ assertThat(result.isSuccessful()).isFalse();
}
@Test
public void sanityCheckWithWrongCredentials() throws Exception {
- restClient.doPostAndAssert("/regions", json, "test", "invalid_pswd")
- .hasStatusCode(401);
+ ClusterManagementResult result =
+ restClient.doPostAndAssert("/regions", json, "test", "invalid_pswd")
+ .hasStatusCode(401).getClusterManagementResult();
+ assertThat(result.getPersistenceStatus().getMessage())
+ .isEqualTo("Authentication error. Please check your credentials.");
+ assertThat(result.isSuccessful()).isFalse();
}
@Test
diff --git
a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
index 4e9cad5..26fe8ec 100644
---
a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
+++
b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
@@ -21,7 +21,6 @@ import org.springframework.web.context.ServletContextAware;
import org.apache.geode.internal.cache.HttpService;
import org.apache.geode.internal.security.SecurityService;
-import org.apache.geode.security.GemFireSecurityException;
import org.apache.geode.security.ResourcePermission;
import org.apache.geode.security.ResourcePermission.Operation;
import org.apache.geode.security.ResourcePermission.Resource;
@@ -51,21 +50,18 @@ public class RestSecurityService implements
ServletContextAware {
* calls used in @PreAuthorize tag needs to return a boolean
*/
public boolean authorize(String resource, String operation, String region,
String key) {
- try {
- securityService.authorize(Resource.valueOf(resource),
Operation.valueOf(operation), region,
- key);
- return true;
- } catch (GemFireSecurityException ex) {
- return false;
- }
+ securityService.authorize(Resource.valueOf(resource),
Operation.valueOf(operation), region,
+ key);
+ return true;
}
public boolean authorize(String operation, String region, String[] keys) {
boolean authorized = false;
for (String key : keys) {
authorized = authorize("DATA", operation, region, key);
- if (!authorized)
+ if (!authorized) {
return false;
+ }
}
return true;
}
diff --git
a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/AbstractManagementController.java
b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/AbstractManagementController.java
index 7012ee3..4cba54a 100644
---
a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/AbstractManagementController.java
+++
b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/AbstractManagementController.java
@@ -15,27 +15,16 @@
package org.apache.geode.management.internal.rest.controllers;
-import javax.management.InstanceNotFoundException;
-import javax.management.MalformedObjectNameException;
import javax.servlet.ServletContext;
-import org.apache.logging.log4j.Logger;
import org.springframework.beans.propertyeditors.StringArrayPropertyEditor;
-import org.springframework.http.HttpStatus;
-import org.springframework.http.ResponseEntity;
-import org.springframework.security.access.AccessDeniedException;
import org.springframework.web.bind.WebDataBinder;
-import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.InitBinder;
import org.springframework.web.context.ServletContextAware;
import org.apache.geode.internal.cache.HttpService;
-import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.security.SecurityService;
-import org.apache.geode.management.internal.api.ClusterManagementResult;
import
org.apache.geode.management.internal.api.LocatorClusterManagementService;
-import org.apache.geode.security.AuthenticationFailedException;
-import org.apache.geode.security.NotAuthorizedException;
public class AbstractManagementController implements ServletContextAware {
@@ -51,58 +40,6 @@ public class AbstractManagementController implements
ServletContextAware {
.getAttribute(HttpService.CLUSTER_MANAGEMENT_SERVICE_CONTEXT_PARAM);
}
- private static final Logger logger = LogService.getLogger();
-
- @ExceptionHandler(Exception.class)
- public ResponseEntity<ClusterManagementResult> internalError(final Exception
e) {
- logger.error(e.getMessage(), e);
- return new ResponseEntity<>(new ClusterManagementResult(false,
e.getMessage()),
- HttpStatus.INTERNAL_SERVER_ERROR);
- }
-
- @ExceptionHandler(AuthenticationFailedException.class)
- public ResponseEntity<ClusterManagementResult>
unauthorized(AuthenticationFailedException e) {
- return new ResponseEntity<>(new ClusterManagementResult(false,
e.getMessage()),
- HttpStatus.UNAUTHORIZED);
- }
-
- @ExceptionHandler({NotAuthorizedException.class, SecurityException.class})
- public ResponseEntity<ClusterManagementResult> forbidden(Exception e) {
- logger.info(e.getMessage());
- return new ResponseEntity<>(new ClusterManagementResult(false,
e.getMessage()),
- HttpStatus.FORBIDDEN);
- }
-
- @ExceptionHandler(MalformedObjectNameException.class)
- public ResponseEntity<ClusterManagementResult> badRequest(final
MalformedObjectNameException e) {
- logger.info(e.getMessage(), e);
- return new ResponseEntity<>(new ClusterManagementResult(false,
e.getMessage()),
- HttpStatus.BAD_REQUEST);
- }
-
- @ExceptionHandler(InstanceNotFoundException.class)
- public ResponseEntity<ClusterManagementResult> notFound(final
InstanceNotFoundException e) {
- logger.info(e.getMessage(), e);
- return new ResponseEntity<>(new ClusterManagementResult(false,
e.getMessage()),
- HttpStatus.NOT_FOUND);
- }
-
- /**
- * Handles an AccessDenied Exception 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(AccessDeniedException.class)
- public ResponseEntity<ClusterManagementResult> handleException(
- final AccessDeniedException cause) {
- logger.info(cause.getMessage(), cause);
- return new ResponseEntity<>(new ClusterManagementResult(false,
cause.getMessage()),
- HttpStatus.FORBIDDEN);
- }
-
/**
* Initializes data bindings for various HTTP request handler method
parameter Java class types.
*
diff --git
a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/AbstractManagementController.java
b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
similarity index 68%
copy from
geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/AbstractManagementController.java
copy to
geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
index 7012ee3..1ecd13e 100644
---
a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/AbstractManagementController.java
+++
b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
@@ -12,45 +12,26 @@
* or implied. See the License for the specific language governing permissions
and limitations under
* the License.
*/
-
package org.apache.geode.management.internal.rest.controllers;
import javax.management.InstanceNotFoundException;
import javax.management.MalformedObjectNameException;
-import javax.servlet.ServletContext;
import org.apache.logging.log4j.Logger;
-import org.springframework.beans.propertyeditors.StringArrayPropertyEditor;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.AccessDeniedException;
-import org.springframework.web.bind.WebDataBinder;
+import org.springframework.security.core.AuthenticationException;
+import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
-import org.springframework.web.bind.annotation.InitBinder;
-import org.springframework.web.context.ServletContextAware;
-import org.apache.geode.internal.cache.HttpService;
import org.apache.geode.internal.logging.LogService;
-import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.management.internal.api.ClusterManagementResult;
-import
org.apache.geode.management.internal.api.LocatorClusterManagementService;
import org.apache.geode.security.AuthenticationFailedException;
import org.apache.geode.security.NotAuthorizedException;
-public class AbstractManagementController implements ServletContextAware {
-
- protected static final String MANAGEMENT_API_VERSION = "/v2";
- protected SecurityService securityService;
- protected LocatorClusterManagementService clusterManagementService;
-
- @Override
- public void setServletContext(ServletContext servletContext) {
- securityService = (SecurityService) servletContext
- .getAttribute(HttpService.SECURITY_SERVICE_SERVLET_CONTEXT_PARAM);
- clusterManagementService = (LocatorClusterManagementService) servletContext
- .getAttribute(HttpService.CLUSTER_MANAGEMENT_SERVICE_CONTEXT_PARAM);
- }
-
+@ControllerAdvice
+public class ManagementControllerAdvice {
private static final Logger logger = LogService.getLogger();
@ExceptionHandler(Exception.class)
@@ -60,7 +41,7 @@ public class AbstractManagementController implements
ServletContextAware {
HttpStatus.INTERNAL_SERVER_ERROR);
}
- @ExceptionHandler(AuthenticationFailedException.class)
+ @ExceptionHandler({AuthenticationFailedException.class,
AuthenticationException.class})
public ResponseEntity<ClusterManagementResult>
unauthorized(AuthenticationFailedException e) {
return new ResponseEntity<>(new ClusterManagementResult(false,
e.getMessage()),
HttpStatus.UNAUTHORIZED);
@@ -103,16 +84,4 @@ public class AbstractManagementController implements
ServletContextAware {
HttpStatus.FORBIDDEN);
}
- /**
- * Initializes data bindings for various HTTP request handler method
parameter Java class types.
- *
- * @param dataBinder the DataBinder implementation used for Web transactions.
- * @see WebDataBinder
- * @see InitBinder
- */
- @InitBinder
- public void initBinder(final WebDataBinder dataBinder) {
- dataBinder.registerCustomEditor(String[].class,
- new
StringArrayPropertyEditor(StringArrayPropertyEditor.DEFAULT_SEPARATOR, false));
- }
}
diff --git
a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/security/RestSecurityConfiguration.java
b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/security/RestSecurityConfiguration.java
index 6d28733..2bc6bc6 100644
---
a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/security/RestSecurityConfiguration.java
+++
b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/security/RestSecurityConfiguration.java
@@ -14,10 +14,19 @@
*/
package org.apache.geode.management.internal.rest.security;
+
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.annotation.Configuration;
+import org.springframework.http.MediaType;
import org.springframework.security.authentication.AuthenticationManager;
import
org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import
org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
@@ -25,6 +34,10 @@ import
org.springframework.security.config.annotation.web.builders.HttpSecurity;
import
org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import
org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.config.http.SessionCreationPolicy;
+import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.web.AuthenticationEntryPoint;
+
+import org.apache.geode.management.internal.api.ClusterManagementResult;
@Configuration
@EnableWebSecurity
@@ -37,6 +50,9 @@ public class RestSecurityConfiguration extends
WebSecurityConfigurerAdapter {
@Autowired
private GeodeAuthenticationProvider authProvider;
+ @Autowired
+ private ObjectMapper objectMapper;
+
@Override
protected void configure(AuthenticationManagerBuilder auth) throws Exception
{
auth.authenticationProvider(authProvider);
@@ -51,13 +67,25 @@ public class RestSecurityConfiguration extends
WebSecurityConfigurerAdapter {
protected void configure(HttpSecurity http) throws Exception {
http.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS).and()
.authorizeRequests()
- // .antMatchers("/ping", "/docs/**", "/swagger-ui.html",
"/v2/api-docs/**",
- // "/webjars/springfox-swagger-ui/**", "/swagger-resources/**")
- // .permitAll()
+ .antMatchers("/ping", "/docs/**", "/swagger-ui.html",
"/v2/api-docs/**",
+ "/webjars/springfox-swagger-ui/**", "/swagger-resources/**")
+ .permitAll()
.anyRequest().authenticated().and().csrf().disable();
if (this.authProvider.getSecurityService().isIntegratedSecurity()) {
- http.httpBasic();
+ http.httpBasic().authenticationEntryPoint(new AuthenticationEntryPoint()
{
+ @Override
+ public void commence(HttpServletRequest request, HttpServletResponse
response,
+ AuthenticationException authException)
+ throws IOException, ServletException {
+ response.addHeader("WWW-Authenticate", "Basic realm=\"GEODE\"");
+ response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+ response.setContentType(MediaType.APPLICATION_JSON_UTF8.getType());
+ ClusterManagementResult result =
+ new ClusterManagementResult(false, authException.getMessage());
+ objectMapper.writeValue(response.getWriter(), result);
+ }
+ });
} else {
http.authorizeRequests().anyRequest().permitAll();
}
diff --git
a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/security/RestSecurityService.java
b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/security/RestSecurityService.java
index 4860ab8..afe19f4 100644
---
a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/security/RestSecurityService.java
+++
b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/security/RestSecurityService.java
@@ -21,7 +21,6 @@ import org.springframework.web.context.ServletContextAware;
import org.apache.geode.internal.cache.HttpService;
import org.apache.geode.internal.security.SecurityService;
-import org.apache.geode.security.GemFireSecurityException;
import org.apache.geode.security.ResourcePermission;
import org.apache.geode.security.ResourcePermission.Operation;
import org.apache.geode.security.ResourcePermission.Resource;
@@ -51,21 +50,18 @@ public class RestSecurityService implements
ServletContextAware {
* calls used in @PreAuthorize tag needs to return a boolean
*/
public boolean authorize(String resource, String operation, String region,
String key) {
- try {
- securityService.authorize(Resource.valueOf(resource),
Operation.valueOf(operation), region,
- key);
- return true;
- } catch (GemFireSecurityException ex) {
- return false;
- }
+ securityService.authorize(Resource.valueOf(resource),
Operation.valueOf(operation), region,
+ key);
+ return true;
}
public boolean authorize(String operation, String region, String[] keys) {
boolean authorized = false;
for (String key : keys) {
authorized = authorize("DATA", operation, region, key);
- if (!authorized)
+ if (!authorized) {
return false;
+ }
}
return true;
}
