Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nginx configuration change to deny access to active_storage blobs #1081

Merged
merged 6 commits into from
Apr 18, 2019

Conversation

pgwillia
Copy link
Member

@pgwillia pgwillia commented Apr 12, 2019

Should be handled in the background and not grant access to the average user.

The equivalent for apache (jupiter.conf) is

 RewriteEngine On
 RewriteCond %{REQUEST_METHOD} ^TRACE
 RewriteRule .* - [F]
 RewriteCond %{REQUEST_URI} /active_storage/blobs [NC]
 RewriteRule ^.*$ - [F,G]

Use the docker deployment images

docker-compose -f docker-compose.deployment.yml up -d
docker-compose -f docker-compose.deployment.yml run web rails db:migrate
docker-compose -f docker-compose.deployment.yml run web rails db:setup
docker-compose -f docker-compose.deployment.yml run web rails c

Visit localhost and pick an item with download. Make note of the id, for example '8927cb4f-9d9c-4cd1-b2cb-78aad9cc536a'

In the rails console find a blob url

include Rails.application.routes.url_helpers
item = JupiterCore::LockedLdpObject.find('8927cb4f-9d9c-4cd1-b2cb-78aad9cc536a', types: [Item, Thesis])
file = item.files.first
url_for(file)

Make note of the response. It will be something like http://uat.library.ualberta.ca/rails/active_storage/blobs/qaqoPUYCMvwp9GPoNBeaT8DK/840559b8-e953-4606-b417-546b0fb27f16

From this branch you should not be able to visit the url
Screenshot from 2019-04-12 15-24-53
But all the variants for thumbnails should still work
Screenshot from 2019-04-12 15-42-37
Screenshot from 2019-04-12 15-42-24

From the master branch

# add --force-recreate to update the volumes for the nginx config change
docker-compose -f docker-compose.deployment.yml up -d --force-recreate

Screenshot from 2019-04-12 15-34-31

Should be handled in the background and not grant access to the average 
user.
@pgwillia pgwillia requested a review from pbinkley April 12, 2019 21:39
@pgwillia
Copy link
Member Author

Thumbnails etc appear to continue to work with this change.

@pbinkley
Copy link
Member

We should configure Apache to return 510 Gone for these, so that any search engine that has already harvested them will know to delete them (and not to punish us for 404s).

@pgwillia
Copy link
Member Author

We should configure Apache to return 510 Gone for these, so that any search engine that has already harvested them will know to delete them (and not to punish us for 404s).

We should configure Apache to return 510 Gone for these, so that any search engine that has already harvested them will know to delete them (and not to punish us for 404s).

510 Not Extended
Further extensions to the request are required for the server to fulfill it.

410 Gone
This response would be sent when the requested content has been permanently deleted from server, with no forwarding address. Clients are expected to remove their caches and links to the resource. The HTTP specification intends this status code to be used for "limited-time, promotional services". APIs should not feel compelled to indicate resources that have been deleted with this status code.

403 Forbidden
The client does not have access rights to the content, i.e. they are unauthorized, so server is rejecting to give proper response. Unlike 401, the client's identity is known to the server.

@weiweishi
Copy link
Contributor

I wonder if 403 is a more proper response? The content is not gone, but just not premitted to be viewed through centain routes?

We'd be able to use .htaccess file:

RewriteEngine On
RewriteCond %{REQUEST_URI} /active_storage/blobs [NC]
RewriteRule ^.*$ - [F,L]

Or in apache configuration:

<Location />
RewriteEngine On
RewriteCond %{REQUEST_URI} ^/active_storage/blobs [NC]
RewriteRule ^.*$ - [F,L]
</Location>

@pbinkley
Copy link
Member

As discussed this morning: since we'll never serve to the public from this URI again, I think 410 is appropriate: there's no point in any external system remembering this URI for any kind of future access.

(And sorry about the 510 Can't Remember Response Code error)

@pbinkley
Copy link
Member

pbinkley commented Apr 15, 2019

I suppose the fully correct response would be a 301 Moved Permanently, redirecting to the helper-based url, which would then issue the 403. That would have to be implemented in the application, though, so that it could calculate the target url. I think 410 is good enough, given the small exposure of the blob urls.

@pbinkley
Copy link
Member

I've emailed @henryzhang87 asking for the configuration change as outlined in @weiweishi 's comment above, with 410 response codes.

@pgwillia
Copy link
Member Author

Since the equivalent is already in production can we merge this to catch up uat?

@pgwillia pgwillia requested a review from weiweishi April 16, 2019 15:52
Copy link
Contributor

@weiweishi weiweishi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

@pgwillia pgwillia merged commit d8796ba into ualbertalib:master Apr 18, 2019
@pgwillia pgwillia deleted the nginx_deny_activestorage_blobs branch April 18, 2019 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants