add OAuth2 env variables #14

2025-07-23T23:28:49Z

jmakdah2 commented

2025-07-23 23:28:49 +00:00

No description provided.

❤️ 1

jmakdah2 added 1 commit 2025-07-23 23:28:50 +00:00

add OAuth2 env variables

continuous-integration/drone/pr Build is failing

Details

e9ec238865

ammaratef45 reviewed 2025-07-23 23:45:45 +00:00

ammaratef45 left a comment

Nice work!

Can we test if the recipe works fine if we created an app out of it but kept these env variables empty/commented out in the .env file?

If so, we might need to put them in their own compose file rather than extending the main compose file

Nice work! Can we test if the recipe works fine if we created an app out of it but kept these env variables empty/commented out in the .env file? If so, we might need to put them in their own compose file rather than extending the main compose file

❤️ 1

decentral1se reviewed 2025-07-24 08:51:16 +00:00

decentral1se left a comment

Great start 🎉

Thanks for the work 👏

Great start 🎉 Thanks for the work 👏

❤️ 1

.env.sample

						
				@@ -93,0 +99,4 @@

				# OAUTH_APP_KEY=

				# This is not a good practice, app secret should passed in via docker secret once support is added (https://github.com/loomio/loomio/issues/11671)

				# Remove this variable before publishing code to any shared repo!

decentral1se commented

2025-07-24 08:50:59 +00:00

Can the secret be configured as a file based secret using an entrypoint hack?

Several other recipes do this to get around the fact the upstream doesn't support it.

You store it in a secret and use the entrypoint to expose it from the FS.

		peertube/entrypoint.sh.tmpl
		Lines 5 to 28 in 2744684292
	
				file_env() {

				   local var="$1"

				   local fileVar="${var}_FILE"

				   local def="${2:-}"

				   if [ "${!var:-}" ] && [ "${!fileVar:-}" ]; then

				      echo >&2 "error: both $var and $fileVar are set (but are exclusive)"

				      exit 1

				   fi

				   local val="$def"

				   if [ "${!var:-}" ]; then

				      val="${!var}"

				   elif [ "${!fileVar:-}" ]; then

				      val="$(< "${!fileVar}")"

				   fi

				   export "$var"="$val"

				   unset "$fileVar"

				}

				file_env "PEERTUBE_DB_PASSWORD"

				file_env "PEERTUBE_SECRET"

Can the secret be configured as a file based secret using an entrypoint hack? Several other recipes do this to get around the fact the upstream doesn't support it. You store it in a secret and use the entrypoint to expose it from the FS. https://git.coopcloud.tech/coop-cloud/peertube/src/commit/2744684292d66053a9681ac57692b9f026863dde/entrypoint.sh.tmpl#L5-L28

👍 1

3wordchant commented

2025-07-24 13:57:57 +00:00

Yep, and no need to add file_env, it's already in use in the entrypoint: https://git.coopcloud.tech/coop-cloud/loomio/src/branch/main/entrypoint.sh#L24-L29

Yep, and no need to add `file_env`, it's already in use in the entrypoint: https://git.coopcloud.tech/coop-cloud/loomio/src/branch/main/entrypoint.sh#L24-L29

👍 1

jmakdah2 commented

2025-07-27 23:49:34 +00:00

oh this is a much nicer solution :) i've updated it to use that hack

❤️ 1

decentral1se marked this conversation as resolved

compose.yml Outdated

						
				@@ -29,6 +29,16 @@ x-environment: &default-env

				  SAML_IDP_METADATA_URL:

				  SAML_ISSUER:

				  FEATURES_DISABLE_EMAIL_LOGIN:

				  OAUTH_AUTH_URL:

decentral1se commented

2025-07-24 08:49:45 +00:00

I think these should rather go in a compose.oidc.yml like other recipes?

Not everyone will run OIDC and while it looks pretty harmless to have a bunch of empty env vars, you never know what can subtly break with configs/apps/etc. This becomes especially hard to overview when you run several apps. So, my intuition says to keep this separate.

Also, you may want to add a release note (see docs) to tell people that there is a new feature and they can add the env vars from the new .env.sample?

I think these should rather go in a `compose.oidc.yml` like other recipes? Not everyone will run OIDC and while it looks pretty harmless to have a bunch of empty env vars, you never know what can subtly break with configs/apps/etc. This becomes especially hard to overview when you run several apps. So, my intuition says to keep this separate. Also, you may want to add a release note (see docs) to tell people that there is a new feature and they can add the env vars from the new `.env.sample`?

👍 2

jmakdah2 commented

2025-07-27 23:48:25 +00:00

Thanks for the suggestion! I created a separate compose file and will also add a release note

👍 1

decentral1se marked this conversation as resolved

jmakdah2 commented

2025-07-27 23:53:46 +00:00

Nice work!

Can we test if the recipe works fine if we created an app out of it but kept these env variables empty/commented out in the .env file?

If so, we might need to put them in their own compose file rather than extending the main compose file

@ammaratef45 I've created a separate compose file as suggested by @decentral1se. So I don't think this should be a concern anymore?

By the way, I see that continuous-integration/drone/pr check failed. Is this a concern? It looks like in the last merged PR it was also failing, and i couldnt determine if the issue was from my change or not (https://build.coopcloud.tech/coop-cloud/loomio/61/1/2)

> Nice work! > > Can we test if the recipe works fine if we created an app out of it but kept these env variables empty/commented out in the .env file? > > If so, we might need to put them in their own compose file rather than extending the main compose file @ammaratef45 I've created a separate compose file as suggested by @decentral1se. So I don't think this should be a concern anymore? By the way, I see that continuous-integration/drone/pr check failed. Is this a concern? It looks like in the last merged PR it was also failing, and i couldnt determine if the issue was from my change or not (https://build.coopcloud.tech/coop-cloud/loomio/61/1/2)

🚀 1

jmakdah2 added 1 commit 2025-07-28 00:48:58 +00:00

add compose.oauth.yml file

continuous-integration/drone/pr Build is failing

Details

ef26599b5e

jmakdah2 force-pushed OAuth2 from ef26599b5e to 9531f4fe40

2025-07-28 00:56:41 +00:00

Compare

decentral1se reviewed 2025-07-28 07:07:54 +00:00

decentral1se left a comment

Nearly there 🤸

entrypoint.sh Outdated

						
				@@ -25,6 +25,7 @@ file_env "DEVISE_SECRET"

				file_env "SECRET_COOKIE_TOKEN"

				file_env "POSTGRES_PASSWORD"

				file_env "SMTP_PASSWORD"

				file_env "OAUTH_APP_SECRET"

decentral1se commented

2025-07-28 07:06:22 +00:00

The last trick you're missing now is to wrap this in a "if OAUTH_IS_ENABLED" kinda thing...

		peertube/entrypoint.sh.tmpl
		Lines 30 to 32 in 2744684292
	
				{{ if eq (env "PEERTUBE_SMTP_ENABLED") "1" }}

				file_env "PEERTUBE_SMTP_PASSWORD"

				{{ end }}

As not everyone will enable this and then their deployment will blow up asking them for this secret.

The last trick you're missing now is to wrap this in a "if OAUTH_IS_ENABLED" kinda thing... https://git.coopcloud.tech/coop-cloud/peertube/src/commit/2744684292d66053a9681ac57692b9f026863dde/entrypoint.sh.tmpl#L30-L32 As not everyone will enable this and then their deployment will blow up asking them for this secret.

decentral1se marked this conversation as resolved

jmakdah2 added 1 commit 2025-07-29 05:42:35 +00:00

add OAUTH_ENABLED env variable

continuous-integration/drone/pr Build is failing

Details

8e577ae8af

decentral1se approved these changes 2025-07-29 07:48:54 +00:00

decentral1se left a comment

👏

jmakdah2 marked the pull request as ready for review 2025-07-31 23:28:50 +00:00

jmakdah2 merged commit 5deafb6ed5 into main

2025-07-31 23:30:50 +00:00

jmakdah2 deleted branch OAuth2

2025-07-31 23:30:50 +00:00

jmakdah2 referenced this issue from a commit

2025-07-31 23:30:50 +00:00

Merge pull request 'add OAuth2 env variables' (#14) from jmakdah2/loomio:OAuth2 into main

Sign in to join this conversation.

4 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: coop-cloud/loomio#14

Allow edits from maintainers

	{{ if eq (env "PEERTUBE_SMTP_ENABLED") "1" }}
	file_env "PEERTUBE_SMTP_PASSWORD"
	{{ end }}