Skip to content

fix: seed OAuth IS_*_ENABLED flags individually instead of all-or-nothing #8740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rucoder wants to merge 1 commit into
base: preview
Choose a base branch
from
+168 −115
Open

fix: seed OAuth IS_*_ENABLED flags individually instead of all-or-nothing #8740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 51 additions & 109 deletions apps/api/plane/license/management/commands/configure_instance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,113 +40,55 @@ def handle(self, *args, **options):
else:
self.stdout.write(self.style.WARNING(f"{obj.key} configuration already exists"))

keys = ["IS_GOOGLE_ENABLED", "IS_GITHUB_ENABLED", "IS_GITLAB_ENABLED", "IS_GITEA_ENABLED"]
if not InstanceConfiguration.objects.filter(key__in=keys).exists():
for key in keys:
if key == "IS_GOOGLE_ENABLED":
GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET = get_configuration_value(
[
{
"key": "GOOGLE_CLIENT_ID",
"default": os.environ.get("GOOGLE_CLIENT_ID", ""),
},
{
"key": "GOOGLE_CLIENT_SECRET",
"default": os.environ.get("GOOGLE_CLIENT_SECRET", "0"),
},
]
)
if bool(GOOGLE_CLIENT_ID) and bool(GOOGLE_CLIENT_SECRET):
value = "1"
else:
value = "0"
InstanceConfiguration.objects.create(
key=key,
value=value,
category="AUTHENTICATION",
is_encrypted=False,
)
self.stdout.write(self.style.SUCCESS(f"{key} loaded with value from environment variable."))
if key == "IS_GITHUB_ENABLED":
GITHUB_CLIENT_ID, GITHUB_CLIENT_SECRET = get_configuration_value(
[
{
"key": "GITHUB_CLIENT_ID",
"default": os.environ.get("GITHUB_CLIENT_ID", ""),
},
{
"key": "GITHUB_CLIENT_SECRET",
"default": os.environ.get("GITHUB_CLIENT_SECRET", "0"),
},
]
)
if bool(GITHUB_CLIENT_ID) and bool(GITHUB_CLIENT_SECRET):
value = "1"
else:
value = "0"
InstanceConfiguration.objects.create(
key="IS_GITHUB_ENABLED",
value=value,
category="AUTHENTICATION",
is_encrypted=False,
)
self.stdout.write(self.style.SUCCESS(f"{key} loaded with value from environment variable."))
if key == "IS_GITLAB_ENABLED":
GITLAB_HOST, GITLAB_CLIENT_ID, GITLAB_CLIENT_SECRET = get_configuration_value(
[
{
"key": "GITLAB_HOST",
"default": os.environ.get("GITLAB_HOST", "https://gitlab.com"),
},
{
"key": "GITLAB_CLIENT_ID",
"default": os.environ.get("GITLAB_CLIENT_ID", ""),
},
{
"key": "GITLAB_CLIENT_SECRET",
"default": os.environ.get("GITLAB_CLIENT_SECRET", ""),
},
]
)
if bool(GITLAB_HOST) and bool(GITLAB_CLIENT_ID) and bool(GITLAB_CLIENT_SECRET):
value = "1"
else:
value = "0"
InstanceConfiguration.objects.create(
key="IS_GITLAB_ENABLED",
value=value,
category="AUTHENTICATION",
is_encrypted=False,
)
self.stdout.write(self.style.SUCCESS(f"{key} loaded with value from environment variable."))
if key == "IS_GITEA_ENABLED":
GITEA_HOST, GITEA_CLIENT_ID, GITEA_CLIENT_SECRET = get_configuration_value(
[
{
"key": "GITEA_HOST",
"default": os.environ.get("GITEA_HOST", ""),
},
{
"key": "GITEA_CLIENT_ID",
"default": os.environ.get("GITEA_CLIENT_ID", ""),
},
{
"key": "GITEA_CLIENT_SECRET",
"default": os.environ.get("GITEA_CLIENT_SECRET", ""),
},
]
)
if bool(GITEA_HOST) and bool(GITEA_CLIENT_ID) and bool(GITEA_CLIENT_SECRET):
value = "1"
else:
value = "0"
InstanceConfiguration.objects.create(
key="IS_GITEA_ENABLED",
value=value,
category="AUTHENTICATION",
is_encrypted=False,
)
self.stdout.write(self.style.SUCCESS(f"{key} loaded with value from environment variable."))
else:
for key in keys:
# Seed IS_*_ENABLED flags individually using get_or_create so that
# each missing key is created independently of the others.
oauth_enabled_keys = {
"IS_GOOGLE_ENABLED": lambda: all(
get_configuration_value(
[
{"key": "GOOGLE_CLIENT_ID", "default": os.environ.get("GOOGLE_CLIENT_ID", "")},
{"key": "GOOGLE_CLIENT_SECRET", "default": os.environ.get("GOOGLE_CLIENT_SECRET", "")},
]
)
),
"IS_GITHUB_ENABLED": lambda: all(
get_configuration_value(
[
{"key": "GITHUB_CLIENT_ID", "default": os.environ.get("GITHUB_CLIENT_ID", "")},
{"key": "GITHUB_CLIENT_SECRET", "default": os.environ.get("GITHUB_CLIENT_SECRET", "")},
]
)
),
"IS_GITLAB_ENABLED": lambda: all(
get_configuration_value(
[
{"key": "GITLAB_HOST", "default": os.environ.get("GITLAB_HOST", "")},
{"key": "GITLAB_CLIENT_ID", "default": os.environ.get("GITLAB_CLIENT_ID", "")},
{"key": "GITLAB_CLIENT_SECRET", "default": os.environ.get("GITLAB_CLIENT_SECRET", "")},
]
)
),
"IS_GITEA_ENABLED": lambda: all(
get_configuration_value(
[
{"key": "GITEA_HOST", "default": os.environ.get("GITEA_HOST", "")},
{"key": "GITEA_CLIENT_ID", "default": os.environ.get("GITEA_CLIENT_ID", "")},
{"key": "GITEA_CLIENT_SECRET", "default": os.environ.get("GITEA_CLIENT_SECRET", "")},
]
)
),
Comment on lines +71 to +79
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IS_GITEA_ENABLED entry in oauth_enabled_keys is redundant and its lambda will never actually be used to create a row. Because IS_GITEA_ENABLED is already included in gitea_config_variables inside instance_config_variables (see core.py line 116), it is always seeded by the first get_or_create loop (lines 29–41). By the time the second loop reaches IS_GITEA_ENABLED, the row already exists, so get_or_create will never call the lambda or create anything — it will always take the else branch and print "configuration already exists".

The IS_GITEA_ENABLED entry should be removed from oauth_enabled_keys to avoid confusion. If the seeding logic for IS_GITEA_ENABLED should instead mirror the credential-check approach used for the other three providers (checking GITEA_HOST, GITEA_CLIENT_ID, and GITEA_CLIENT_SECRET), then its entry in gitea_config_variables in core.py should be removed, and the oauth_enabled_keys entry kept. The current state creates a misleading impression that all four keys are handled consistently when IS_GITEA_ENABLED actually follows a completely different seeding path.

Copilot uses AI. Check for mistakes.
Copy link
Author

@rucoder rucoder Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Removed IS_GITEA_ENABLED from gitea_config_variables in core.py so all four flags are now handled consistently in configure_instance.py with category AUTHENTICATION. Also added a test (test_all_enabled_flags_use_authentication_category) that verifies this consistency.

}

for key, check_configured in oauth_enabled_keys.items():
obj, created = InstanceConfiguration.objects.get_or_create(
key=key,
defaults={
"value": "1" if check_configured() else "0",
"category": "AUTHENTICATION",
"is_encrypted": False,
},
)
if created:
self.stdout.write(self.style.SUCCESS(f"{key} loaded with value from environment variable."))
else:
self.stdout.write(self.style.WARNING(f"{key} configuration already exists"))
Empty file added apps/api/plane/tests/unit/management/__init__.py
View file
Open in desktop
Empty file.
117 changes: 117 additions & 0 deletions apps/api/plane/tests/unit/management/test_configure_instance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
# Copyright (c) 2023-present Plane Software, Inc. and contributors
# SPDX-License-Identifier: AGPL-3.0-only
# See the LICENSE file for details.

import pytest
from io import StringIO
from unittest.mock import patch

from django.core.management import call_command

from plane.license.models import InstanceConfiguration


@pytest.mark.unit
class TestConfigureInstanceOAuthSeeding:
"""Test that configure_instance seeds IS_*_ENABLED flags individually."""

@pytest.mark.django_db
def test_creates_all_enabled_flags_when_none_exist(self):
"""All four IS_*_ENABLED flags should be created on a fresh database."""
out = StringIO()
with patch.dict("os.environ", {"SECRET_KEY": "test-secret"}):
call_command("configure_instance", stdout=out)

keys = [
"IS_GOOGLE_ENABLED",
"IS_GITHUB_ENABLED",
"IS_GITLAB_ENABLED",
"IS_GITEA_ENABLED",
]
for key in keys:
assert InstanceConfiguration.objects.filter(key=key).exists(), (
f"{key} should have been created"
)

@pytest.mark.django_db
def test_creates_missing_flags_when_some_already_exist(self):
"""Missing IS_*_ENABLED flags should be created even if others already exist.

Each flag uses get_or_create so the presence of one should never
prevent creation of the others.
"""
InstanceConfiguration.objects.create(
key="IS_GITHUB_ENABLED",
value="1",
category="AUTHENTICATION",
is_encrypted=False,
)
InstanceConfiguration.objects.create(
key="IS_GITEA_ENABLED",
value="0",
category="AUTHENTICATION",
is_encrypted=False,
)

out = StringIO()
with patch.dict("os.environ", {"SECRET_KEY": "test-secret"}):
call_command("configure_instance", stdout=out)

for key in ["IS_GOOGLE_ENABLED", "IS_GITLAB_ENABLED"]:
assert InstanceConfiguration.objects.filter(key=key).exists(), (
f"{key} should have been created even though other IS_*_ENABLED flags already existed"
)

@pytest.mark.django_db
def test_does_not_overwrite_existing_enabled_flags(self):
"""Existing IS_*_ENABLED values should not be overwritten on re-run."""
InstanceConfiguration.objects.create(
key="IS_GITHUB_ENABLED",
value="1",
category="AUTHENTICATION",
is_encrypted=False,
)

out = StringIO()
with patch.dict("os.environ", {"SECRET_KEY": "test-secret"}):
call_command("configure_instance", stdout=out)

config = InstanceConfiguration.objects.get(key="IS_GITHUB_ENABLED")
assert config.value == "1", "Existing IS_GITHUB_ENABLED=1 should not be overwritten"

@pytest.mark.django_db
def test_enabled_flags_default_to_zero_without_credentials(self):
"""Without OAuth env vars, IS_*_ENABLED flags should default to '0'."""
out = StringIO()
env = {
"SECRET_KEY": "test-secret",
"GITHUB_CLIENT_ID": "",
"GITHUB_CLIENT_SECRET": "",
"GOOGLE_CLIENT_ID": "",
"GOOGLE_CLIENT_SECRET": "",
"GITLAB_HOST": "",
"GITLAB_CLIENT_ID": "",
"GITLAB_CLIENT_SECRET": "",
"GITEA_HOST": "",
"GITEA_CLIENT_ID": "",
"GITEA_CLIENT_SECRET": "",
}
with patch.dict("os.environ", env):
call_command("configure_instance", stdout=out)

for key in ["IS_GOOGLE_ENABLED", "IS_GITHUB_ENABLED", "IS_GITLAB_ENABLED", "IS_GITEA_ENABLED"]:
config = InstanceConfiguration.objects.get(key=key)
assert config.value == "0", f"{key} should be '0' without credentials"

@pytest.mark.django_db
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test suite covers the negative path (no credentials → IS_*_ENABLED="0") but is missing a test for the positive path: when OAuth credentials are provided, the corresponding IS_*_ENABLED flag should be set to "1". Since SKIP_ENV_VAR defaults to True (DB mode), such a test would need to pre-create the credential rows (e.g., GITHUB_CLIENT_ID and GITHUB_CLIENT_SECRET) with non-empty values in the DB, then call configure_instance and assert that IS_GITHUB_ENABLED has value "1". Without this test, the credential-presence check logic in the lambda (line 86) is not exercised end-to-end.

Suggested change
@pytest.mark.django_db
@pytest.mark.django_db
def test_github_flag_set_to_one_with_db_credentials(self):
"""With GitHub OAuth credentials in DB, IS_GITHUB_ENABLED should be '1'."""
# Pre-create non-empty GitHub credential rows in the database (DB mode).
InstanceConfiguration.objects.create(
key="GITHUB_CLIENT_ID",
value="github-client-id",
category="AUTHENTICATION",
is_encrypted=False,
)
InstanceConfiguration.objects.create(
key="GITHUB_CLIENT_SECRET",
value="github-client-secret",
category="AUTHENTICATION",
is_encrypted=False,
)
out = StringIO()
# SKIP_ENV_VAR defaults to True, so the command should read credentials from DB.
with patch.dict("os.environ", {"SECRET_KEY": "test-secret"}):
call_command("configure_instance", stdout=out)
config = InstanceConfiguration.objects.get(key="IS_GITHUB_ENABLED")
assert config.value == "1", "IS_GITHUB_ENABLED should be '1' when DB credentials are present"
@pytest.mark.django_db

Copilot uses AI. Check for mistakes.
def test_all_enabled_flags_use_authentication_category(self):
"""All IS_*_ENABLED flags should be created with category AUTHENTICATION."""
out = StringIO()
with patch.dict("os.environ", {"SECRET_KEY": "test-secret"}):
call_command("configure_instance", stdout=out)

for key in ["IS_GOOGLE_ENABLED", "IS_GITHUB_ENABLED", "IS_GITLAB_ENABLED", "IS_GITEA_ENABLED"]:
config = InstanceConfiguration.objects.get(key=key)
assert config.category == "AUTHENTICATION", (
f"{key} should have category AUTHENTICATION, got {config.category}"
)
6 changes: 0 additions & 6 deletions apps/api/plane/utils/instance_config_variables/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,6 @@
]

gitea_config_variables = [
{
"key": "IS_GITEA_ENABLED",
"value": os.environ.get("IS_GITEA_ENABLED", "0"),
"category": "GITEA",
"is_encrypted": False,
},
{
"key": "GITEA_HOST",
"value": os.environ.get("GITEA_HOST"),
Expand Down
Loading