Add additional CSRF setting

[knabar]

Following #471, this PR adds another required setting to make CSRF requests work:

omero config set omero.web.csrf_trusted_origins '[".example.com"]'

Without this setting, it is not possible to perform cross-domain unsafe HTTP requests (POST, PUT, and DELETE), even if cookies etc. are configured correctly.

Reference:

• https://docs.djangoproject.com/en/3.2/ref/settings/#csrf-trusted-origins

[chris-allan]

Similar to SECURE_PROXY_SSL_HEADER below, might be a good idea to link to the Django documentation for CSRF_TRUSTED_ORIGINS:

• https://docs.djangoproject.com/en/4.2/ref/settings/#csrf-trusted-origins

[will-moore]

With this PR included on merge-ci I was able to get Django 4.0 working (avoided the CSRF errors I was seeing previously) by setting:

omero config append omero.web.csrf_trusted_origins '"https://merge-ci.openmicroscopy.org"'

This appears to be a requirement for Django 4.0+ to avoid CSRF errors.
Also, there is a breaking change in upgrade from 3.2 - previously the origins needed to have no schema (as in the your docstring examples) but in Django 4.0 you need to have the schema.
So the update of that setting would be a required step in upgrade from 3.2 to 4.0+.

Just a thought: would it make sense to hold off on this change until we bump Django to 4.0 (or 4.2) to avoid those upgrade issues?

https://docs.djangoproject.com/en/4.0/ref/settings/#csrf-trusted-origins

[will-moore]

I don't really understand why Django 4.0 requires the setting in order for login to work (on merge-ci) but that wasn't needed for Django 3.2. The docs above don't say anything about Django 4.0 being more strict. The only change mentioned for 3.2 -> 4.0 is the addition of the schema to origins.

Although we're not the first to see this https://stackoverflow.com/questions/73338124/csrf-token-issue-when-upgrading-django-to-version-4

[sbesson]

@will-moore do you have a link for the error you were receiving? Scanning quickly the docs, this might be related to the changes described in the CSRF section and bear some relationship with the configuration of the OME CI architecture and the way requests are proxied from merge-ci.openmicroscopy.org to the underlying OMERO.web application.
If correct, I would expect other deployment would be affected and we might need to work out best recommendations in terms of the headers that should be set on requests.

[will-moore]

I'm just seeing the csrf_failure response

omero-web/omeroweb/feedback/views.py

Line 175 in ba072f1

def csrf_failure(request, reason=""):

as configured at

omero-web/omeroweb/settings.py

Line 1668 in ba072f1

CSRF_FAILURE_VIEW = "omeroweb.feedback.views.csrf_failure"

on login.

But yes, that CSRF section of the docs seems to match what we're seeing.

It might be nice if omero-web could automatically add it's own origin to the CSRF_TRUSTED_ORIGINS setting, so that users didn't have to do it?

[chris-allan]

At least for our systems we have no requirement to set CSRF_TRUSTED_ORIGINS with Django 4.0 in a semi-production setting. This includes scenarios there are SSL certificates.

If you are having CSRF errors you'll definitely want to look at the OMEROweb.log for the full reason. As @sbesson has implied, problems with merge-ci are likely due to Host, Origin or Referrer header mismatch which is more strict in 4.0. This suggests that actually other things in this OMERO.web deployment, including fully qualified URL generation, may not be working correctly if that's the case.

[will-moore]

Ah - that reminds me that on merge-ci Django doesn't know it is running under https.
E.g. build_absolute_uri() returns http://... - e.g. https://merge-ci.openmicroscopy.org/web/api/ has "url:base": "http://merge-ci.openmicroscopy.org/web/api/v0/".
So it's probably failing because of that mismatch.
It would be good to fix that on merge-ci, as it's causing issues elsewhere (e.g. ome/omero-web-zarr#7)

[chris-allan]

If that's true then OMERO.web is indeed misconfigured. See also:

• https://forum.image.sc/t/qupath-omero-login-problem/71577/9
• https://github.com/qupath/qupath-extension-omero/issues/22#issuecomment-1488673902

[will-moore]

Tried looking into this some more with no luck...

I found a couple of examples of others seeing similar issues, but can't tell how to apply their fixes to merge-ci:

https://stackoverflow.com/questions/11441832/request-is-secure-always-returns-false-with-uwsgi-server
https://stackoverflow.com/questions/66176881/django-request-is-secure-always-returns-false

I added an output of the nginx conf on merge-ci, see
https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-web/213/console

15:47:41 upstream omeroweb_web {
15:47:41     server web:4080 fail_timeout=0;
15:47:41 }
15:47:41 
15:47:41 server {
15:47:41     listen 80;
15:47:41     server_name $hostname;
15:47:41 
15:47:41     sendfile on;
15:47:41     client_max_body_size 0;
15:47:41 
15:47:41 
15:47:41 
15:47:41     # maintenance page serve from here
15:47:41     location @maintenance_web {
15:47:41         root /home/omero-web/workspace/OMERO-web/OMERO.web/etc/templates/error;
15:47:41         try_files $uri /maintainance.html =502;
15:47:41     }
15:47:41 
15:47:41     # weblitz django apps serve media from here
15:47:41     location /web/static {
15:47:41         alias /home/omero-web/static/web;
15:47:41     }
15:47:41 
15:47:41     location @proxy_to_app_web {
15:47:41         proxy_set_header X-Forwarded-Proto $scheme;
15:47:41         proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
15:47:41         proxy_set_header Host $http_host;
15:47:41         proxy_redirect off;
15:47:41         proxy_buffering off;
15:47:41 
15:47:41         proxy_pass [http://omeroweb_web;](http://omeroweb_web%3B/)
15:47:41     }
15:47:41 
15:47:41     location /web {
15:47:41 
15:47:41         error_page 502 @maintenance_web;
15:47:41         # checks for static file, if not found proxy to app
15:47:41         try_files $uri @proxy_to_app_web;
15:47:41     }
15:47:41 
15:47:41 }

which has the line mentioned in one of those posts: proxy_set_header X-Forwarded-Proto $scheme.

[chris-allan]

That's the HTTP rather than HTTPS configuration, no?

[will-moore]

I don't know.
I don't see 2 different nginx confs. This is what's generated at https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-web/configure
by

omero web config nginx --http "$OMERO_WEB_PORT" >$OMERO_DIST/nginx.conf.tmp

sudo cp $OMERO_DIST/nginx.conf.tmp $HOME/nginx/$NODE_NAME.conf

[chris-allan]

Then you'll need to find out as it's the HTTPS request pipeline that is in question here. If TLS is being terminated further up the chain there is further work to do ¹².

Footnotes

1. https://omero.readthedocs.io/en/stable/sysadmins/config.html#omero.web.secure ↩

2. https://omero.readthedocs.io/en/stable/sysadmins/config.html#omero.web.secure_proxy_ssl_header ↩

[will-moore]

@jburel - do you have any idea how https is getting set-up on merge-ci?

It looks like we are already doing:

omero config set omero.web.secure_proxy_ssl_header '["HTTP_X_FORWARDED_PROTO", "https"]'

as suggested fix for this mentions at
https://stackoverflow.com/questions/66176881/django-request-is-secure-always-returns-false

But reading the warning at https://docs.djangoproject.com/en/3.2/ref/settings/#secure-proxy-ssl-header I don't know if we're satisfying all these conditions on merge-ci.
Does NGINX do all that for us?

[jburel]

See also https://github.com/openmicroscopy/management_tools/blob/master/ansible/inventory/host_vars/web-proxy.openmicroscopy.org.yml

[will-moore]

@jburel - Thanks for that. I'm not sure exactly what it's doing! Been reading https://realpython.com/django-nginx-gunicorn/ but management_tools and ansible is outside of the scope of that...

It seems like this line is relevant:

nginx_proxy_forward_scheme_header: X-Forwarded-Proto-Web-Proxy

The nginx config generated by omero-web contains:

proxy_set_header X-Forwarded-Proto $scheme;

and this is not configurable since it's in the template:

omero-web/omeroweb/templates/nginx.conf.template

Line 26 in ba072f1

proxy_set_header X-Forwarded-Proto $scheme;

The setting passed to Django in merge-ci is currently:

omero config set omero.web.secure_proxy_ssl_header '["HTTP_X_FORWARDED_PROTO", "https"]'

I'm not sure if all 3 need to be the same, or if the ansible setting overrides the one in the nginx config generated by omero-web?

[will-moore]

This change is working as expected.
I don't think this PR needs to be held up by the ongoing discussion on merge-ci http vv https since that is not closely related to this setting.

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/release-of-omero-server-5-6-8-and-omero-web-5-22-0/84157/1

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/docker-image-of-omero-web-5-23-0-still-with-v5-22-1/88699/13