Skip to content

csp: allow backend passthrough for /oidc/callback #1478

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.

Sign up for GitHub

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

Merged
alxndrsn merged 5 commits into from
Dec 2, 2025
Merged

csp: allow backend passthrough for /oidc/callback #1478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
534aac6
csp: allow backend passthrough for /oidc/callback
Nov 5, 2025
1acbe3d
Merge branch 'next' into oidc-csp
alxndrsn Nov 18, 2025
30b1cf4
Merge branch 'next' into oidc-csp
alxndrsn Nov 20, 2025
96a2910
Merge branch 'next' into oidc-csp
alxndrsn Dec 2, 2025
1a406b4
resinstate top-level headers
Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions files/nginx/backend.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
proxy_set_header X-Forwarded-Proto $scheme;
proxy_pass http://service:8383;
proxy_redirect off;

# buffer requests, but not responses, so streaming out works.
proxy_request_buffering on;
proxy_buffering off;
proxy_read_timeout 2m;
17 changes: 10 additions & 7 deletions files/nginx/odk.conf.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ server {

# More lax CSP for enketo-express:
# Google Maps API: https://developers.google.com/maps/documentation/javascript/content-security-policy
proxy_hide_header Content-Security-Policy-Report-Only;
add_header Content-Security-Policy-Report-Only "default-src 'none'; connect-src 'self' blob: https://maps.googleapis.com/ https://maps.google.com/ https://maps.gstatic.com/mapfiles/ https://fonts.gstatic.com/ https://fonts.googleapis.com/ https://translate.google.com https://translate.googleapis.com; font-src 'self' https://fonts.gstatic.com/; frame-src 'none'; img-src data: blob: jr: 'self' https://maps.google.com/maps/ https://maps.gstatic.com/mapfiles/ https://maps.googleapis.com/maps/ https://tile.openstreetmap.org/ https://translate.google.com; manifest-src 'none'; media-src blob: jr: 'self'; object-src 'none'; script-src 'unsafe-inline' 'self' https://maps.googleapis.com/maps/api/js/ https://maps.google.com/maps/ https://maps.google.com/maps-api-v3/api/js/; style-src 'unsafe-inline' 'self' https://fonts.googleapis.com/css; style-src-attr 'unsafe-inline'; report-uri /csp-report";
#
# Rules set to 'none' here would fallback to default-src if excluded.
Expand All @@ -154,15 +155,17 @@ server {
}
# End of Enketo Configuration.

location ~ ^/v\d+/oidc/callback$ {
include /usr/share/odk/nginx/common-headers.conf;
include /usr/share/odk/nginx/backend.conf;
}

location ~ ^/v\d {
proxy_set_header X-Forwarded-Proto $scheme;
proxy_pass http://service:8383;
proxy_redirect off;
proxy_hide_header Content-Security-Policy-Report-Only;
add_header Content-Security-Policy-Report-Only "default-src 'none'; connect-src https://translate.google.com https://translate.googleapis.com; img-src https://translate.google.com; report-uri /csp-report";
Copy link
Member

@matthew-white matthew-white Dec 2, 2025

Choose a reason for hiding this comment

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

This is kind of an aside/tangent, but why do we include these Google Translate URLs for the Backend CSP? Backend almost never returns HTML, so I feel like the Google Translate bit isn't needed. Google Translate isn't going to translate JSON returned from Backend. I know this has been the default value of this header, but I wonder whether it could be further simplified for Backend.

Or is the attitude more like, "it never hurts to include Google Translate, so let's just always include it"?

Copy link
Contributor Author

@alxndrsn alxndrsn Dec 3, 2025

Choose a reason for hiding this comment

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

I think this is a mistake - API services should not be loaded in the browser as the main document, so it should be safe to completely lock down their CSPs. On the other hand, the blank.html file may be loaded as the main document, so should probably be allowing google translate.

Copy link
Contributor Author

@alxndrsn alxndrsn Dec 3, 2025

Choose a reason for hiding this comment

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

Filed at:


# buffer requests, but not responses, so streaming out works.
proxy_request_buffering on;
proxy_buffering off;
proxy_read_timeout 2m;
include /usr/share/odk/nginx/common-headers.conf;
include /usr/share/odk/nginx/backend.conf;
}

location @blank.html {
Expand Down
1 change: 1 addition & 0 deletions nginx.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ COPY files/nginx/setup-odk.sh \
/scripts/

COPY files/nginx/redirector.conf /usr/share/odk/nginx/
COPY files/nginx/backend.conf /usr/share/odk/nginx/
COPY files/nginx/common-headers.conf /usr/share/odk/nginx/
COPY files/nginx/robots.txt /usr/share/nginx/html
COPY --from=intermediate client/dist/ /usr/share/nginx/html
Expand Down
4 changes: 4 additions & 0 deletions test/nginx/mock-http-server/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ const app = express();

app.use((req, res, next) => {
console.log(new Date(), req.method, req.originalUrl);

// always set CSP header to detect (or allow) leaks from backend through to the client
res.set('Content-Security-Policy-Report-Only', 'default-src NOTE:FROM-BACKEND');

next();
});

Expand Down
13 changes: 13 additions & 0 deletions test/nginx/test-nginx.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const contentSecurityPolicies = {
'img-src': 'https://translate.google.com',
'report-uri': '/csp-report',
},
'backend-unmodified': {
'default-src': 'NOTE:FROM-BACKEND',
},
'central-frontend': {
'default-src': none,
'connect-src': [
Expand Down Expand Up @@ -353,6 +356,16 @@ describe('nginx config', () => {
);
});

it('/oidc/callback should serve Content-Security-Policy from backend', async () => {
// when
const res = await fetchHttps('/v1/oidc/callback');

// then
assert.equal(res.status, 200);
assert.equal(await res.text(), 'OK');
assertSecurityHeaders(res, { csp:'backend-unmodified' });
});

it('should set x-forwarded-proto header to "https"', async () => {
// when
const res = await fetchHttps('/v1/reflect-headers');
Expand Down