Skip to content
This repository has been archived by the owner on Jan 21, 2022. It is now read-only.

Updated to provide read acl permission on the bucket created for the use... #2

Merged
merged 1 commit into from
Feb 26, 2015

Conversation

VenkateshSub
Copy link
Contributor

...r

This is required to use the RiakCS BlobStore using JCloud Blobstore libraries. The library requires permission to access the acls on the bucket before it can put a new object in it.

Below is the exception thrown by JCloud's BlobStore library indicating the same. This fix resolves it.

2015-02-11T21:34:28.92-0800 [App/0] OUT 2015-02-12 05:34:28.920 ERROR 31 --- [o-61082-exec-10] o.a.c.c.C.[.[.[/].[dispatcherServlet] : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is com.google.common.util.concurrent.UncheckedExecutionException: org.jclouds.rest.AuthorizationException: Access Denied] with root cause
2015-02-11T21:34:28.92-0800 [App/0] OUT org.jclouds.aws.AWSResponseException: request GET https://p-riakcs.10.244.0.34.xip.io/service-instance-9329d598-4c8a-470b-8eba-89dd9759cf0a?acl HTTP/1.1 failed with code 403, error: AWSError{requestId='', requestToken='null', code='AccessDenied', message='Access Denied', context='{Resource=/service-instance-9329d598-4c8a-470b-8eba-89dd9759cf0a}'}
2015-02-11T21:34:28.92-0800 [App/0] OUT at org.jclouds.aws.handlers.ParseAWSErrorFromXmlContent.handleError(ParseAWSErrorFromXmlContent.java:77)
2015-02-11T21:34:28.92-0800 [App/0] OUT at org.jclouds.http.handlers.DelegatingErrorHandler.handleError(DelegatingErrorHandler.java:67)
2015-02-11T21:34:28.92-0800 [App/0] OUT at org.jclouds.http.internal.BaseHttpCommandExecutorService.shouldContinue(BaseHttpCommandExecutorService.java:180)
2015-02-11T21:34:28.92-0800 [App/0] OUT at org.jclouds.http.internal.BaseHttpCommandExecutorService.invoke(BaseHttpCommandExecutorService.java:150)
2015-02-11T21:34:28.92-0800 [App/0] OUT at org.jclouds.rest.internal.InvokeSyncToAsyncHttpMethod.invoke(InvokeSyncToAsyncHttpMethod.java:129)
2015-02-11T21:34:28.92-0800 [App/0] OUT at org.jclouds.rest.internal.InvokeSyncToAsyncHttpMethod.apply(InvokeSyncToAsyncHttpMethod.java:95)
2015-02-11T21:34:28.92-0800 [App/0] OUT at org.jclouds.rest.internal.InvokeSyncToAsyncHttpMethod.apply(InvokeSyncToAsyncHttpMethod.java:56)
2015-02-11T21:34:28.92-0800 [App/0] OUT at org.jclouds.rest.internal.DelegatesToInvocationFunction.handle(DelegatesToInvocationFunction.java:156)
2015-02-11T21:34:28.92-0800 [App/0] OUT at org.jclouds.rest.internal.DelegatesToInvocationFunction.invoke(DelegatesToInvocationFunction.java:123)
2015-02-11T21:34:28.92-0800 [App/0] OUT at com.sun.proxy.$Proxy70.getBucketACL(Unknown Source)

…user

This is required to use the RiakCS BlobStore using JCloud Blobstore libraries. The library requires permission to access the acls on the bucket before it can put a new object in it.
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/88214440.

@cfdreddbot
Copy link

Hey VenkateshSub!

Thanks for submitting this pull request!

All pull request authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

@jbayer
Copy link

jbayer commented Feb 17, 2015

we have received the CLA, thank you!

@shalako
Copy link
Contributor

shalako commented Feb 20, 2015

Thank you for the fix! We'll review asap.

@karlkfi
Copy link
Contributor

karlkfi commented Feb 24, 2015

Why does JClouds require read access to the access control list for a bucket?
Doesn't that allow someone to list all users (ids & emails) that have access to a bucket?

Looking at the JCloud source on github I don't see any particular reason why you would need READ_ACP unless you were using getBlobAccess, getObjectAcl, getBucketAcl or getContainerAccess. You should be able to just use getBlob and putBlob and catch an AuthorizationException (wraps 401 and 403 status code responses).

UPDATE: on closer inspection it looks like S3BlobStore.putBlob requires READ_ACL to read the bucket permission before putting an object (getBlob does not). This is a rather questionable implementation choice. Why would you want to make two http calls when one would do? It seems only to set the object read permissions to PUBLIC_READ if the bucket is also PUBLIC_READ. It also seems to ignore the user's PutOptions overrides, ignoring what access the user specifies....

I'd recommend a pull request to jclouds to make it not require READ_ACL, rather than exposing user access lists to everyone.

@poblin-orange
Copy link

my understanding is that this fix would grant READ_ACL to the bucket binding authkey, in the riakcs CF broker context (not everyone), which seems quite reasonable, unless this introduces any other security issue.

@VenkateshSub
Copy link
Contributor Author

Thanks @karlkfi and @poblin-orange for looking into this!

Yes, like @poblin-orange mentioned, in CF, we do create a new bucket in RIAKCS for every create-service call. The idea here is to provide READ_ACL access for that bucket to that authkey only.

@karlkfi
Copy link
Contributor

karlkfi commented Feb 26, 2015

Since service instances are shared within a CF org/space, multiple users in the same space can have apps that bind to a shared service. If those users are given READ_ACP, they will be able to see the RiakCS users and permissions of all other apps bound to that service instance in that space.

Given that all users in a space already have access to view service bindings for all apps in that space, it's probably ok to let their apps have similar access.

@rainmaker rainmaker merged commit 263bac7 into cloudfoundry-attic:master Feb 26, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants