l4rm4nd / VoucherVault

Django web application to store and manage vouchers, coupons, loyalty and gift cards digitally. Supports expiry notifications, transaction histories, file uploads and OIDC SSO.
https://github.com/l4rm4nd/VoucherVault/wiki
GNU General Public License v3.0
125 stars 2 forks source link

CSRF Logic on alternate port #7

Closed michaelkrieger closed 2 months ago

michaelkrieger commented 2 months ago

If one sets DOMAIN=vouchervault.mydomain.com, a 403 CSRF error takes place when using a hostname as the hostname is not allowed.

If one sets DOMAIN=vouchervault.mydomain.com:1234, Django returns a Invalid HTTP_HOST header: 'vouchervault.mydomain.com:1234'. You may need to add 'vouchervault.mydomain.com:1234' to ALLOWED_HOSTS..

Trusted_origins should include the full https://hostname:port/ and allowed_hosts should just be 'hostname' Allowed Hosts is "case-insensitive, not including port", but CSRF_TRUSTED_ORIGINS requires the host header to match with a port number.

Proposed #6 to remedy gracefully.

l4rm4nd commented 2 months ago

Hi @michaelkrieger ,

many thanks for pointing out these issues.

During the initial developing phase, I did not notice those issues as localhost/127.0.0.1 bypasses every security check. Later on, I only added support to define a custom domain behind SSL/TLS.

The actual problem arises when you run VoucherVault with a custom domain but still do not use SSL/TLS. The settings.py declared the secure cookie flag for the CSRF cookie. Therefore, if you browse Vouchervault over unencrypted HTTP, the cookie will not be set by the server due to the insecure connection.

I've reworked the settings.py now and introduced two new environment variables to handle these cases:

See https://github.com/l4rm4nd/VoucherVault/commit/2ba76d7eacb66f27f879223fdd4224b1a3924a5b and https://github.com/l4rm4nd/VoucherVault/commit/b8f87955bd67adbb8b205b6c061b3cbda19cc0e8.

I've refrained from merging your pull request as the fundamental issue was a bit deeper than just the trusted origins.

Nonetheless, many thanks for pointing this out. Hope the commit works for you now.

michaelkrieger commented 2 months ago

Thank you. Not a problem. glad I was able to help.