coop-cloud/loomio
1
0
Fork 1
Code Issues 4 Pull Requests 3 Projects Releases Wiki Activity

add OAuth2 env variables #14

Merged
jmakdah2 merged 3 commits from jmakdah2/loomio:OAuth2 into main 2025-07-31 23:30:50 +00:00
Conversation 5 Commits 3 Files Changed 5 +47
jmakdah2 commented 2025-07-23 23:28:49 +00:00
Owner
Copy Link
No description provided.
❤️ 1
jmakdah2 added 1 commit 2025-07-23 23:28:50 +00:00
add OAuth2 env variables
Some checks failed
continuous-integration/drone/pr Build is failing
Details
e9ec238865
ammaratef45 reviewed 2025-07-23 23:45:45 +00:00
ammaratef45 left a comment
Owner
Copy Link

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
Owner
Copy Link

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
Owner
Copy Link

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.

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
Owner
Copy Link

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
Author
Owner
Copy Link

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

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
Owner
Copy Link

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
Author
Owner
Copy Link

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

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
Author
Owner
Copy Link

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
Some checks failed
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
Owner
Copy Link

Nearly there 🤸

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
Owner
Copy Link

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

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
Some checks failed
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
Owner
Copy Link

👏

👏
jmakdah2 changed title from WIP: add OAuth2 env variables to add OAuth2 env variables 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
Sign in to join this conversation.
Reviewers
No Reviewers
decentral1se
Labels
Clear labels
bug
Something is not working
duplicate
This issue or pull request already exists
enhancement
New feature
help wanted
Need some help
invalid
Something is wrong
question
More information is needed
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
3wordchant aadil abra-bot ammaratef45 Apfelwurm appletalk arjan basebuilder Brooke carla cas codegod100 coopcloud cyrnel decentral1se fauno flancian Frando iexos jade javielico jmakdah2 joe-irving kawaiipunk knoflook kolaente lambdabundesverband linnealovespie marlon mayel mirsal moritz nicksellen notplants p4u1 pau pharaohgraphy renovate-bot ripclap rix rscmbbng sef simon sixsmith stevensting tobias trav val virtualboys wolcen wykwit xynosis yksflip
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: coop-cloud/loomio#14
No description provided.
Delete Branch "jmakdah2/loomio:OAuth2"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?