zulip / docker-zulip

Container configurations, images, and examples for Zulip.
https://zulip.com/
Apache License 2.0
550 stars 227 forks source link

OIDC Redirect URI incorrect behind reverse proxy #404

Closed mp-strachan closed 1 year ago

mp-strachan commented 1 year ago

After an upgrade to Zulip 7.0, the redirect URI is now being passed to the IdP as "http://" rather than "https://", so our OIDC (which requires using HTTPS) throws back an error. DISABLE_HTTPS: is set to true in the docker-compose, as it's behind a reverse proxy.

image

This error didn't occur in Zulip 6.... Not sure if this can be overridden in a config file?

tomkv commented 1 year ago

Same thing happening for SAML2: /saml/metadata.xml contains AssertionConsumerService Location="http://..." and IdP then complains about invalid redirect uri.

DISABLE_HTTPS is set to true, and reverse proxy is configured to provide X-Forwarded-Proto $scheme.

maltokyo commented 1 year ago

Does this fix it for you? https://github.com/zulip/docker-zulip/issues/403 It was the only thing I needed to do to upgrade to 7.0

jeehoonkang commented 1 year ago

@maltokyo I edited CSRF settings per #403 but the OIDC redirect uri problem remains after upgrading Zulip to 7.0.

Before editing CSRF settings, the website kept refreshing.

maltokyo commented 1 year ago

I can't replicate it, and I'm behind RP too... Any way I can try?

jeehoonkang commented 1 year ago

I don't think it's a proper fix, but I found a workaround: adding SETTING_SOCIAL_AUTH_REDIRECT_IS_HTTPS: 'True' in docker-compose.yml.

Relevant documentation: https://python-social-auth.readthedocs.io/en/latest/configuration/settings.html

tomkv commented 1 year ago

@maltokyo It doesn't seem to be CSFR problem; clients that have still valid saml token are using the service just fine. Only clients without token or with expired tokens cannot log in. It is only the login flow that fails.

@jeehoonkang setting this variable moved me further: now IdP doesn't complain anymore about invalid redirect uri, the /saml/metadata.xml does point to https:// and not http://, but zulip complains now:

/var/log/zulip/auth.log:

2023-06-06 19:36:02.310 INFO [zulip.auth.saml] AuthFailed: Authentication failed:
 SAML login failed: ['invalid_response'] (The response was received at http://$zulip-hostname/complete/saml/ instead of https://$zulip-hostname/complete/saml/)

For some reason (probably the samy why it put http into the metadata in the first place), it still considers incoming request to be http.

My config in docker-compose.yml is:

      DISABLE_HTTPS: "True"
      SSL_CERTIFICATE_GENERATION: "self-signed"
      MANUAL_CONFIGURATION: "True"
      LINK_SETTINGS_TO_DATA: "True"

and then relevant bits from settings.py:

USE_X_FORWARDED_HOST = True
...
AUTHENTICATION_BACKENDS = ('zproject.backends.SAMLAuthBackend',)
SOCIAL_AUTH_REDIRECT_IS_HTTPS = True
SOCIAL_AUTH_SAML_ORG_INFO = {"en-US": {"displayname": "Zulip", "name": "zulip", "url": "{}{}".format("https://", EXTERNAL_HOST),},}
SOCIAL_AUTH_SAML_SECURITY_CONFIG = {"authnRequestsSigned": True,}
SOCIAL_AUTH_SAML_SP_ENTITY_ID = "zulip"
SOCIAL_AUTH_SAML_TECHNICAL_CONTACT = {"givenName": "Technical team", "emailAddress": ZULIP_ADMINISTRATOR,}
SOCIAL_AUTH_SAML_SUPPORT_CONTACT = {"givenName": "Support team", "emailAddress": ZULIP_ADMINISTRATOR,}
SOCIAL_AUTH_SAML_ENABLED_IDPS = {
    "keycloak": {
        "entity_id": "https://$idphost/realms/myrealm",
        "url": "https://$idphostrealms/myrealm/protocol/saml",
        "attr_user_permanent_id": "email",
        "attr_first_name": "first_name",
        "attr_last_name": "last_name",
        "attr_username": "email",
        "attr_email": "email",
        "display_name": "SSO",
        "auto_signup": True,
    },
}

the nginx config is for zulip is:

server {
    listen 443 ssl;
    listen [::]:443 ssl;
    server_name zulip-hostname ;

    if ( $host !~ "(^zulip-hostname$)" ) { return 404; }
    include /usr/syno/etc/www/certificate/ReverseProxy_9ac99073-9b90-4457-9e92-6dc1dc03c89b/cert.conf*;
    include /usr/syno/etc/security-profile/tls-profile/config/ReverseProxy_9ac99073-9b90-4457-9e92-6dc1dc03c89b.conf*;

    add_header Strict-Transport-Security "max-age=15768000; includeSubdomains; preload" always;
    proxy_ssl_protocols TLSv1 TLSv1.1 TLSv1.2 TLSv1.3;
    include conf.d/.acl.e52ce0f1-33a5-4048-b970-8d92f93d9980.conf*;

    location / {
        proxy_connect_timeout 60;
        proxy_read_timeout 60;
        proxy_send_timeout 60;
        proxy_intercept_errors off;
        proxy_http_version 1.1;

        proxy_set_header        Connection                  \"upgrade\";
        proxy_set_header        Upgrade                     $http_upgrade;
        proxy_set_header        X-Forwarded-For             $proxy_add_x_forwarded_for;
        proxy_set_header        X-Forwarded-Host            $host;
        proxy_set_header        X-Real-IP                   $remote_addr;
        proxy_set_header        Host                        $host;
        proxy_set_header        X-Forwarded-Proto           $scheme;
        proxy_set_header        X-Forwarded-Protocol        $scheme;

        proxy_pass http://172.16.11.11:80;

    }
    error_page 403 404 500 502 503 504 /dsm_error_page;

    location /dsm_error_page {
        internal;
        root /usr/syno/share/nginx;
        rewrite (.*) /error.html break;
        allow all;
    }

}

I've put in both X-Forwarded-Proto and X-Forwarded-Protocol; first one is in zulip docs, the second one in wiki here, for docker-zulip; neither makes a difference. As you can see in the nginx config, the reverse proxy listens only on port 443. It is not available over 80/http at all.

alexmv commented 1 year ago

The fundamental error here is the same as #403 -- X-Forwarded-Proto is not set, or not trusted. Playing whack-a-mole with the other settings (SETTING_SOCIAL_AUTH_REDIRECT_IS_HTTPS / SETTING_CSRF_TRUSTED_ORIGINS) won't be helpful if Zulip still thinks requests are coming in from an untrusted connection.

X-Forwarded-Protocol is incorrect, and if you can link to where that's referenced, I'd be great so I can fix it. The rest of your location block should be trimmed down to match our documentation -- the upgrade pieces are counterproductive, as is trying to provide X-Real-IP.

alexmv commented 1 year ago

Since it's the same underlying issue, I'm going to resolve this issue and we can continue the discussion on #403.