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

Restrict access to uploads #2088

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

pixeltrix
Copy link
Member

Description of change

Use CloudFront signed cookies to prevent premature access to planning application documents.

Story Link

https://trello.com/c/1VP9wgYz

@pixeltrix pixeltrix force-pushed the restrict-access-to-uploads branch 4 times, most recently from 10541d9 to 7711eea Compare December 20, 2024 03:31
@pixeltrix pixeltrix force-pushed the restrict-access-to-uploads branch 5 times, most recently from 5fb2b97 to d23f4a0 Compare January 14, 2025 09:21
@pixeltrix pixeltrix marked this pull request as ready for review January 14, 2025 09:21
@@ -209,12 +226,11 @@ class NotArchiveableError < StandardError; end
validate :numbered
validate :created_date_is_in_the_past

default_scope -> { no_owner.or(not_excluded_owners) }

Copy link
Member Author

Choose a reason for hiding this comment

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

This needed to be removed so the uploads engine could find all documents for checking the blob key

.or(where(file_preview_variant_blobs: {key:}))
.distinct.sole
end

Copy link
Member Author

Choose a reason for hiding this comment

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

This huge block is so we can find a document through a local authority and prevent someone using access in one local authority to access a document in another authority.

Copy link
Member Author

Choose a reason for hiding this comment

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

Doing an EXPLAIN on the query shows it's not great so continuing to look at other options.

@@ -6,3 +7,4 @@ GROUPED_PROPOSAL_DETAILS_FEATURE=enabled
GOOGLE_TAG_MANAGER_ID=GTM-XXXXXXXX
UPLOADS_HOSTNAME=uploads.bops.localhost
UPLOADS_BASE_URL=http://uploads.bops.localhost:3000
USE_SIGNED_URLS=false
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to deploy this to staging so we can test it but don't want to break the app so this controls whether it uses the current urls or the new urls.

This is so they can be reused in other engines.
This is the first step in adding signed cookies.
The default scope prevents the bops_uploads module from finding the
correct document for a blob key if it is a site visit, press notice, etc.
@pixeltrix pixeltrix force-pushed the restrict-access-to-uploads branch from d23f4a0 to 159f73d Compare January 14, 2025 12:09
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.

2 participants