only use sso redirection if sso is enabled #2

Open
moosemower wants to merge 1 commits from moosemower/element-web:use-sso into main
4 changed files with 5 additions and 2 deletions

View File

@ -7,3 +7,5 @@ LETS_ENCRYPT_ENV=production
HOMESERVER=matrix-synapse.example.com
SERVER_NAME=mymatrix
BRAND=mymatrix
USE_SSO=false

View File

@ -1 +1 @@
export CONFIG_JSON_VERSION=v3
export CONFIG_JSON_VERSION=v4

View File

@ -8,6 +8,7 @@ services:
- DOMAIN
- HOMESERVER
- SERVER_NAME
- USE_SSO=${USE_SSO:-true}
networks:
- proxy
configs:

View File

@ -6,7 +6,7 @@
}
},
"sso_redirect_options": {
"immediate": true
"immediate": {{ env "USE_SSO" }}
decentral1se marked this conversation as resolved Outdated

Thanks @moosemower 👏

If you run abra app run <app> app bash and then env | grep -i sso, do you see a value? I believe this is evaluating to false or some "non-truthy" value because it is not threaded through via the env configuration?

It's not present in the .env.sample: https://git.coopcloud.tech/coop-cloud/element-web/src/branch/main/.env.sample (#USE_SOO=)

And that needs to be threaded through into the container 👇

compose.yml Lines 8 to 10 in bb930287ed
- DOMAIN
- HOMESERVER
- SERVER_NAME

Then other operators can then customise the value. I believe to maintain backwards compatibility, you should set it to USE_SSO=${USE_SSO:-true} to ensure that operators who don't update their .env file will not have a broken upgrade. See below for more examples 👇

compose.yml Lines 78 to 83 in 8b7ed8142e
- PHP_MEMORY_LIMIT=${PHP_MEMORY_LIMIT:-1G}
- PHP_UPLOAD_LIMIT=${PHP_UPLOAD_LIMIT:-512M}
- FPM_MAX_CHILDREN=${FPM_MAX_CHILDREN:-131}
- FPM_START_SERVERS=${FPM_START_SERVERS:-32}
- FPM_MIN_SPARE_SERVERS=${FPM_MIN_SPARE_SERVERS:-32}
- FPM_MAX_SPARE_SERVERS=${FPM_MAX_SPARE_SERVERS:-98}

We wanted to document this slightly involved environment updating dance over on https://docs.coopcloud.tech/maintainers/upgrade/#backwards-compatible-environment-variable-changes but didn't get around to it 🙃 Docs patches welcome!

Thanks @moosemower 👏 If you run `abra app run <app> app bash` and then `env | grep -i sso`, do you see a value? I believe this is evaluating to `false` or some "non-truthy" value because it is not threaded through via the env configuration? It's not present in the `.env.sample`: https://git.coopcloud.tech/coop-cloud/element-web/src/branch/main/.env.sample (`#USE_SOO=`) And that needs to be threaded through into the container 👇 https://git.coopcloud.tech/coop-cloud/element-web/src/commit/bb930287ed89efbfc2f0822cf4393b8f3a92cc5a/compose.yml#L8-L10 Then other operators can then customise the value. I believe to maintain backwards compatibility, you should set it to `USE_SSO=${USE_SSO:-true}` to ensure that operators who don't update their `.env` file will not have a broken upgrade. See below for more examples 👇 https://git.coopcloud.tech/coop-cloud/nextcloud/src/commit/8b7ed8142e6a492fb107141a163bbec1107bb979/compose.yml#L78-L83 We wanted to document this slightly involved environment updating dance over on https://docs.coopcloud.tech/maintainers/upgrade/#backwards-compatible-environment-variable-changes but didn't get around to it 🙃 Docs patches welcome!

Oooh, good catch! Indeed, deploying a new element-web with my changes,

$ abra app run element-web.moose.garden app sh
/ $ env | grep -i sso
/ $

No SSO variables in there! Derp. Including my .env.sample changes in an amended commit.

Also, now that I plumb in environment: USE_SSO=${USE_SSO:-true} I get USE_SSO=true by default in the container's env, even if I don't set that variable to anything. Thanks for the tip! I won't promise a doc patch but I would like to :)


Changes to my commit:

  • plumbed in USE_SSO to compose.yml
  • default that var to true if not set for compat
  • add it to .env.sample
Oooh, good catch! Indeed, deploying a new element-web with my changes, ``` $ abra app run element-web.moose.garden app sh / $ env | grep -i sso / $ ``` No SSO variables in there! Derp. Including my .env.sample changes in an amended commit. Also, now that I plumb in `environment: USE_SSO=${USE_SSO:-true}` I get USE_SSO=true by default in the container's env, even if I don't set that variable to anything. Thanks for the tip! I won't promise a doc patch but I would like to :) ---- Changes to my commit: - plumbed in USE_SSO to compose.yml - default that var to `true` if not set for compat - add it to .env.sample

Great, thanks for the update!

Great, thanks for the update!
},
"disable_custom_urls": false,
"disable_guests": true,