USERGRID-1266: check permissions at REST layer to avoid incorrect response codes
also block user token exchange via GET
/org/app/users/{user}/token?access_token={usertoken}
Project: http://git-wip-us.apache.org/repos/asf/usergrid/repo
Commit: http://git-wip-us.apache.org/repos/asf/usergrid/commit/10e2be7b
Tree: http://git-wip-us.apache.org/repos/asf/usergrid/tree/10e2be7b
Diff: http://git-wip-us.apache.org/repos/asf/usergrid/diff/10e2be7b
Branch: refs/heads/release-2.1.1
Commit: 10e2be7b405e1031abf74082efa8dce2ca9cb1fc
Parents: 8435783
Author: Mike Dunker <[email protected]>
Authored: Tue Mar 15 12:30:25 2016 -0700
Committer: Mike Dunker <[email protected]>
Committed: Tue Mar 15 12:30:25 2016 -0700
----------------------------------------------------------------------
.../rest/applications/ServiceResource.java | 24 ++++++++--------
.../notifiers/NotifierResource.java | 3 +-
.../notifiers/NotifiersResource.java | 3 +-
.../rest/applications/users/UserResource.java | 26 ++++++++++++++----
.../rest/applications/users/UsersResource.java | 3 +-
.../ServiceResourceNotFoundExceptionMapper.java | 9 +-----
.../security/SecuredResourceFilterFactory.java | 29 ++++++++++++++------
.../applications/ApplicationResourceIT.java | 2 +-
.../collection/users/PermissionsResourceIT.java | 4 +--
9 files changed, 62 insertions(+), 41 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/usergrid/blob/10e2be7b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java
----------------------------------------------------------------------
diff --git
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java
index a600bf1..4c92fef 100644
---
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java
+++
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java
@@ -31,6 +31,7 @@ import org.apache.usergrid.rest.AbstractContextResource;
import org.apache.usergrid.rest.ApiResponse;
import org.apache.usergrid.rest.RootResource;
import org.apache.usergrid.rest.applications.assets.AssetsResource;
+import org.apache.usergrid.rest.security.annotations.CheckPermissionsForPath;
import org.apache.usergrid.rest.security.annotations.RequireApplicationAccess;
import org.apache.usergrid.security.oauth.AccessInfo;
import org.apache.usergrid.services.*;
@@ -39,7 +40,6 @@ import
org.apache.usergrid.services.assets.data.AwsSdkS3BinaryStore;
import org.apache.usergrid.services.assets.data.BinaryStore;
import org.apache.usergrid.services.assets.data.LocalFileBinaryStore;
import org.apache.usergrid.services.exceptions.AwsPropertiesNotFoundException;
-import org.apache.usergrid.utils.InflectionUtils;
import org.apache.usergrid.utils.JsonUtils;
import org.glassfish.jersey.media.multipart.BodyPart;
import org.glassfish.jersey.media.multipart.BodyPartEntity;
@@ -292,8 +292,7 @@ public class ServiceResource extends
AbstractContextResource {
boolean collectionGet = false;
if ( action == ServiceAction.GET ) {
- collectionGet = (getServiceParameters().size() == 1 &&
InflectionUtils
- .isPlural(getServiceParameters().get(0)));
+ collectionGet = getServiceParameters().size() == 1;
}
addQueryParams( getServiceParameters(), ui );
ServiceRequest r = services.newRequest( action, tree,
getServiceParameters(), payload,
@@ -330,9 +329,9 @@ public class ServiceResource extends
AbstractContextResource {
}
+ @CheckPermissionsForPath
@GET
@Produces({MediaType.APPLICATION_JSON, MediaType.TEXT_HTML,
"application/javascript"})
- @RequireApplicationAccess
@JSONP
public ApiResponse executeGet( @Context UriInfo ui,
@QueryParam("callback")
@DefaultValue("callback") String callback )
@@ -430,8 +429,8 @@ public class ServiceResource extends
AbstractContextResource {
}
+ @CheckPermissionsForPath
@POST
- @RequireApplicationAccess
@Consumes(MediaType.APPLICATION_JSON)
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
@@ -465,8 +464,8 @@ public class ServiceResource extends
AbstractContextResource {
+ @CheckPermissionsForPath
@PUT
- @RequireApplicationAccess
@Consumes(MediaType.APPLICATION_JSON)
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
@@ -485,8 +484,8 @@ public class ServiceResource extends
AbstractContextResource {
}
+ @CheckPermissionsForPath
@DELETE
- @RequireApplicationAccess
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
public ApiResponse executeDelete(
@@ -605,8 +604,8 @@ public class ServiceResource extends
AbstractContextResource {
/** ************** the following is file attachment (Asset) support
********************* */
+ @CheckPermissionsForPath
@POST
- @RequireApplicationAccess
@Consumes(MediaType.MULTIPART_FORM_DATA)
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
@@ -621,8 +620,8 @@ public class ServiceResource extends
AbstractContextResource {
}
+ @CheckPermissionsForPath
@PUT
- @RequireApplicationAccess
@Consumes(MediaType.MULTIPART_FORM_DATA)
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
@@ -709,16 +708,16 @@ public class ServiceResource extends
AbstractContextResource {
}
+ @CheckPermissionsForPath
@PUT
- @RequireApplicationAccess
@Consumes(MediaType.APPLICATION_OCTET_STREAM)
public Response uploadDataStreamPut( @Context UriInfo ui, InputStream
uploadedInputStream ) throws Exception {
return uploadDataStream( ui, uploadedInputStream );
}
+ @CheckPermissionsForPath
@POST
- @RequireApplicationAccess
@Consumes(MediaType.APPLICATION_OCTET_STREAM)
public Response uploadDataStream( @Context UriInfo ui, InputStream
uploadedInputStream ) throws Exception {
@@ -752,9 +751,8 @@ public class ServiceResource extends
AbstractContextResource {
return Response.status( 200 ).build();
}
-
+ @CheckPermissionsForPath
@GET
- @RequireApplicationAccess
@Produces(MediaType.WILDCARD)
public Response executeStreamGet( @Context UriInfo ui,
@PathParam("entityId") PathSegment entityId,
@HeaderParam("range") String rangeHeader,
http://git-wip-us.apache.org/repos/asf/usergrid/blob/10e2be7b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifierResource.java
----------------------------------------------------------------------
diff --git
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifierResource.java
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifierResource.java
index 2049983..22fbe0b 100644
---
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifierResource.java
+++
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifierResource.java
@@ -21,6 +21,7 @@ import org.apache.commons.io.IOUtils;
import org.apache.usergrid.persistence.index.query.Identifier;
import org.apache.usergrid.rest.ApiResponse;
import org.apache.usergrid.rest.applications.ServiceResource;
+import org.apache.usergrid.rest.security.annotations.CheckPermissionsForPath;
import org.apache.usergrid.rest.security.annotations.RequireApplicationAccess;
import org.apache.usergrid.rest.utils.CertificateUtils;
import org.apache.usergrid.services.ServiceAction;
@@ -55,8 +56,8 @@ public class NotifierResource extends ServiceResource {
}
/* Multipart PUT update with uploaded p12Certificate */
+ @CheckPermissionsForPath
@PUT
- @RequireApplicationAccess
@Consumes(MediaType.MULTIPART_FORM_DATA)
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
http://git-wip-us.apache.org/repos/asf/usergrid/blob/10e2be7b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifiersResource.java
----------------------------------------------------------------------
diff --git
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifiersResource.java
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifiersResource.java
index b92e775..c99be8e 100644
---
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifiersResource.java
+++
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/notifiers/NotifiersResource.java
@@ -23,6 +23,7 @@ import org.apache.usergrid.persistence.index.query.Identifier;
import org.apache.usergrid.rest.AbstractContextResource;
import org.apache.usergrid.rest.ApiResponse;
import org.apache.usergrid.rest.applications.ServiceResource;
+import org.apache.usergrid.rest.security.annotations.CheckPermissionsForPath;
import org.apache.usergrid.rest.security.annotations.RequireApplicationAccess;
import org.apache.usergrid.rest.utils.CertificateUtils;
import org.apache.usergrid.services.ServiceAction;
@@ -101,8 +102,8 @@ public class NotifiersResource extends ServiceResource {
}
/* Multipart POST create with uploaded p12Certificate */
+ @CheckPermissionsForPath
@POST
- @RequireApplicationAccess
@Consumes(MediaType.MULTIPART_FORM_DATA)
@Override
@JSONP
http://git-wip-us.apache.org/repos/asf/usergrid/blob/10e2be7b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UserResource.java
----------------------------------------------------------------------
diff --git
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UserResource.java
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UserResource.java
index 751113f..373bf09 100644
---
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UserResource.java
+++
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UserResource.java
@@ -36,6 +36,7 @@ import javax.ws.rs.core.PathSegment;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
+import org.apache.usergrid.rest.security.annotations.CheckPermissionsForPath;
import org.glassfish.jersey.server.mvc.Viewable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -103,8 +104,8 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@PUT
- @RequireApplicationAccess
@Consumes(MediaType.APPLICATION_JSON)
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
@@ -123,7 +124,7 @@ public class UserResource extends ServiceResource {
return super.executePutWithMap( ui, json, callback );
}
-
+ // no access token needed
@PUT
@Path("password")
@JSONP
@@ -253,6 +254,7 @@ public class UserResource extends ServiceResource {
}
+ // no access token needed
@POST
@Path("password")
@JSONP
@@ -264,6 +266,7 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@POST
@Path("deactivate")
@JSONP
@@ -282,6 +285,7 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@GET
@Path("sendpin")
@JSONP
@@ -308,6 +312,7 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@POST
@Path("sendpin")
@JSONP
@@ -319,9 +324,9 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@GET
@Path("setpin")
- @RequireApplicationAccess
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
public ApiResponse setPin( @Context UriInfo ui, @QueryParam("pin") String
pin,
@@ -346,10 +351,10 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@POST
@Path("setpin")
@Consumes("application/x-www-form-urlencoded")
- @RequireApplicationAccess
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
public ApiResponse postPin( @Context UriInfo ui, @FormParam("pin") String
pin,
@@ -374,10 +379,10 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@POST
@Path("setpin")
@Consumes(MediaType.APPLICATION_JSON)
- @RequireApplicationAccess
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
public ApiResponse jsonPin( @Context UriInfo ui, JsonNode json,
@@ -402,6 +407,7 @@ public class UserResource extends ServiceResource {
}
+ // no access token needed
@GET
@Path("resetpw")
@Produces(MediaType.TEXT_HTML)
@@ -429,6 +435,7 @@ public class UserResource extends ServiceResource {
}
+ // no access token needed, reset token required
@POST
@Path("resetpw")
@Consumes("application/x-www-form-urlencoded")
@@ -526,6 +533,7 @@ public class UserResource extends ServiceResource {
}
+ // no access token needed, activation token required
@GET
@Path("activate")
@Produces(MediaType.TEXT_HTML)
@@ -547,6 +555,7 @@ public class UserResource extends ServiceResource {
}
+ // no access token needed, confirmation token required
@GET
@Path("confirm")
@Produces(MediaType.TEXT_HTML)
@@ -614,6 +623,7 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@PUT
@Path("revoketokens")
@JSONP
@@ -625,6 +635,7 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@POST
@Path("revoketoken")
@JSONP
@@ -646,6 +657,7 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@PUT
@Path("revoketoken")
@JSONP
@@ -657,6 +669,7 @@ public class UserResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@GET
@Path("token")
@RequireApplicationAccess
@@ -669,7 +682,8 @@ public class UserResource extends ServiceResource {
try {
- if ( isApplicationUser() && !getUserUuid().equals(
getSubjectUserId() ) ) {
+ // don't allow application user tokens to be exchanged for new
tokens (possibly increasing ttl)
+ if ( isApplicationUser() ) {
OAuthResponse res = OAuthResponse.errorResponse( SC_FORBIDDEN
).buildJSONMessage();
return Response.status( res.getResponseStatus() ).type(
jsonMediaType( callback ) )
.entity( wrapWithCallback( res.getBody(),
callback ) ).build();
http://git-wip-us.apache.org/repos/asf/usergrid/blob/10e2be7b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java
----------------------------------------------------------------------
diff --git
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java
index 76614f8..34c6915 100644
---
a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java
+++
b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java
@@ -30,6 +30,7 @@ import org.apache.usergrid.rest.ApiResponse;
import org.apache.usergrid.rest.RootResource;
import org.apache.usergrid.rest.applications.ServiceResource;
import org.apache.usergrid.rest.exceptions.RedirectionException;
+import org.apache.usergrid.rest.security.annotations.CheckPermissionsForPath;
import org.apache.usergrid.rest.security.annotations.RequireApplicationAccess;
import org.glassfish.jersey.server.mvc.Viewable;
import org.slf4j.Logger;
@@ -203,9 +204,9 @@ public class UsersResource extends ServiceResource {
}
+ @CheckPermissionsForPath
@POST
@Override
- @RequireApplicationAccess
@JSONP
@Produces({MediaType.APPLICATION_JSON, "application/javascript"})
public ApiResponse executePost( @Context UriInfo ui, String body,
http://git-wip-us.apache.org/repos/asf/usergrid/blob/10e2be7b/stack/rest/src/main/java/org/apache/usergrid/rest/exceptions/ServiceResourceNotFoundExceptionMapper.java
----------------------------------------------------------------------
diff --git
a/stack/rest/src/main/java/org/apache/usergrid/rest/exceptions/ServiceResourceNotFoundExceptionMapper.java
b/stack/rest/src/main/java/org/apache/usergrid/rest/exceptions/ServiceResourceNotFoundExceptionMapper.java
index 64eb5d9..4d92811 100644
---
a/stack/rest/src/main/java/org/apache/usergrid/rest/exceptions/ServiceResourceNotFoundExceptionMapper.java
+++
b/stack/rest/src/main/java/org/apache/usergrid/rest/exceptions/ServiceResourceNotFoundExceptionMapper.java
@@ -20,11 +20,9 @@ package org.apache.usergrid.rest.exceptions;
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.Provider;
-import org.apache.usergrid.security.shiro.utils.SubjectUtils;
import
org.apache.usergrid.services.exceptions.ServiceResourceNotFoundException;
import static javax.ws.rs.core.Response.Status.NOT_FOUND;
-import static javax.ws.rs.core.Response.Status.UNAUTHORIZED;
@Provider
@@ -32,11 +30,6 @@ public class ServiceResourceNotFoundExceptionMapper extends
AbstractExceptionMap
@Override
public Response toResponse( ServiceResourceNotFoundException e ) {
- if ( SubjectUtils.getSubjectUserId() == null ) {
- return toResponse( UNAUTHORIZED, e );
- }
- else {
- return toResponse( NOT_FOUND, e );
- }
+ return toResponse( NOT_FOUND, e );
}
}
http://git-wip-us.apache.org/repos/asf/usergrid/blob/10e2be7b/stack/rest/src/main/java/org/apache/usergrid/rest/security/SecuredResourceFilterFactory.java
----------------------------------------------------------------------
diff --git
a/stack/rest/src/main/java/org/apache/usergrid/rest/security/SecuredResourceFilterFactory.java
b/stack/rest/src/main/java/org/apache/usergrid/rest/security/SecuredResourceFilterFactory.java
index 0514dca..4c7d26a 100644
---
a/stack/rest/src/main/java/org/apache/usergrid/rest/security/SecuredResourceFilterFactory.java
+++
b/stack/rest/src/main/java/org/apache/usergrid/rest/security/SecuredResourceFilterFactory.java
@@ -123,7 +123,6 @@ public class SecuredResourceFilterFactory implements
DynamicFeature {
featureContext.register( ApplicationFilter.class );
}
else if ( am.isAnnotationPresent( RequireOrganizationAccess.class ) ) {
-
featureContext.register( OrganizationFilter.class );
}
else if ( am.isAnnotationPresent( RequireSystemAccess.class ) ) {
@@ -414,12 +413,32 @@ public class SecuredResourceFilterFactory implements
DynamicFeature {
"---- Checked permissions for path
--------------------------------------------\n" + "Requested path: {} \n"
+ "Requested action: {} \n" + "Requested permission: {}
\n" + "Permitted: {} \n";
- ApplicationInfo application;
+ ApplicationInfo application = null;
try {
application = management.getApplicationInfo(
getApplicationIdentifier() );
EntityManager em = emf.getEntityManager( application.getId() );
+
+ if ( SubjectUtils.isAnonymous() ) {
+ Map<String, String> roles = null;
+ try {
+ roles = em.getRoles();
+ if (logger.isTraceEnabled()) {
+ logger.trace("found roles {}", roles);
+ }
+ }
+ catch ( Exception e ) {
+ logger.error( "Unable to retrieve roles", e );
+ }
+ if ( ( roles != null ) && roles.containsKey( "guest" ) ) {
+ loginApplicationGuest( application );
+ }
+ else {
+ throw mappableSecurityException( "unauthorized", "No
application guest access authorized" );
+ }
+ }
+
Subject currentUser = SubjectUtils.getSubject();
if ( currentUser == null ) {
@@ -435,12 +454,6 @@ public class SecuredResourceFilterFactory implements
DynamicFeature {
logger.debug( PATH_MSG, path, operation, perm, permitted );
}
- if(!permitted){
- // throwing this so we can raise a proper mapped REST
exception
- throw new Exception("Subject not permitted");
- }
-
-
SubjectUtils.checkPermission( perm );
Subject subject = SubjectUtils.getSubject();
http://git-wip-us.apache.org/repos/asf/usergrid/blob/10e2be7b/stack/rest/src/test/java/org/apache/usergrid/rest/applications/ApplicationResourceIT.java
----------------------------------------------------------------------
diff --git
a/stack/rest/src/test/java/org/apache/usergrid/rest/applications/ApplicationResourceIT.java
b/stack/rest/src/test/java/org/apache/usergrid/rest/applications/ApplicationResourceIT.java
index e822f66..2dd5090 100644
---
a/stack/rest/src/test/java/org/apache/usergrid/rest/applications/ApplicationResourceIT.java
+++
b/stack/rest/src/test/java/org/apache/usergrid/rest/applications/ApplicationResourceIT.java
@@ -517,7 +517,7 @@ public class ApplicationResourceIT extends AbstractRestIT {
//Set the default TTL of the application to a date far in the future
final MapUtils.HashMapBuilder<String, String> map =
new MapUtils.HashMapBuilder<String, String>().map(
"accesstokenttl", "31536000000" );
- this.app().getTarget( false )
+ this.app().getTarget(true, clientSetup.getSuperuserToken())
.queryParam( "access_token", token )
.request()
.accept( MediaType.APPLICATION_JSON )
http://git-wip-us.apache.org/repos/asf/usergrid/blob/10e2be7b/stack/rest/src/test/java/org/apache/usergrid/rest/applications/collection/users/PermissionsResourceIT.java
----------------------------------------------------------------------
diff --git
a/stack/rest/src/test/java/org/apache/usergrid/rest/applications/collection/users/PermissionsResourceIT.java
b/stack/rest/src/test/java/org/apache/usergrid/rest/applications/collection/users/PermissionsResourceIT.java
index 340b75f..973de55 100644
---
a/stack/rest/src/test/java/org/apache/usergrid/rest/applications/collection/users/PermissionsResourceIT.java
+++
b/stack/rest/src/test/java/org/apache/usergrid/rest/applications/collection/users/PermissionsResourceIT.java
@@ -193,7 +193,7 @@ public class PermissionsResourceIT extends AbstractRestIT {
}
@Test
- public void getNonExistantEntityReturns404() throws Exception {
+ public void getNonExistentEntityReturns404() throws Exception {
// Call a get on a existing entity with no access token and check if
we get a 401
try {
@@ -409,7 +409,7 @@ public class PermissionsResourceIT extends AbstractRestIT {
"get,put,post:/reviews/**" );
// allow access to all user's connections excluding delete
addPermission( "reviewer",
- "get,put,post:/users/${user}/**" );
+ "get,put,post:/users/me/**" );
// allow access to the review relationship excluding delete
addPermission( "reviewer",
"get,put,post:/books/*/review/*" );
