Skip to content

Commit 484227c

Browse files
ekanshulclaude
andcommitted
Make REDASH_DISABLE_PUBLIC_URLS win over stale DB org settings
disable_public_urls is not editable in the admin UI, but the settings form posts back every value it received from GET /api/settings/organization, so the env-derived value got persisted into organizations.settings. From then on the stored copy shadowed the environment variable, and flipping REDASH_DISABLE_PUBLIC_URLS had no effect. Introduce an "environment only" set of org settings (currently just disable_public_urls): - Organization.get_setting() ignores stored values for these keys, so existing stale rows written by older versions are neutralized without requiring DB surgery. - The settings endpoint skips persisting them and reports the effective (environment) value. Fixes #7630 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ca79fe9 commit 484227c

4 files changed

Lines changed: 44 additions & 2 deletions

File tree

redash/handlers/settings.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
from redash.handlers.base import BaseResource
44
from redash.models import Organization, db
55
from redash.permissions import require_admin
6+
from redash.settings.organization import (
7+
environment_only as environment_only_org_settings,
8+
)
69
from redash.settings.organization import settings as org_settings
710

811

@@ -11,7 +14,7 @@ def get_settings_with_defaults(defaults, org):
1114
settings = {}
1215

1316
for setting, default_value in defaults.items():
14-
current_value = values.get(setting)
17+
current_value = values.get(setting) if setting not in environment_only_org_settings else None
1518
if current_value is None and default_value is None:
1619
continue
1720

@@ -41,6 +44,9 @@ def post(self):
4144

4245
previous_values = {}
4346
for k, v in new_values.items():
47+
if k in environment_only_org_settings:
48+
continue
49+
4450
if k == "auth_google_apps_domains":
4551
previous_values[k] = self.current_org.google_apps_domains
4652
self.current_org.settings[Organization.SETTING_GOOGLE_APPS_DOMAINS] = v

redash/models/organizations.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
from sqlalchemy.orm.attributes import flag_modified
33
from sqlalchemy_utils.models import generic_repr
44

5+
from redash.settings.organization import (
6+
environment_only as environment_only_org_settings,
7+
)
58
from redash.settings.organization import settings as org_settings
69

710
from .base import Column, db, primary_key
@@ -66,7 +69,7 @@ def set_setting(self, key, value):
6669
flag_modified(self, "settings")
6770

6871
def get_setting(self, key, raise_on_missing=True):
69-
if key in self.settings.get("settings", {}):
72+
if key in self.settings.get("settings", {}) and key not in environment_only_org_settings:
7073
return self.settings["settings"][key]
7174

7275
if key in org_settings:

redash/settings/organization.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,10 @@
8181
"hide_plotly_mode_bar": HIDE_PLOTLY_MODE_BAR,
8282
"disable_public_urls": DISABLE_PUBLIC_URLS,
8383
}
84+
85+
# Settings that are controlled exclusively through environment variables and are
86+
# not editable from the admin UI. Values for these keys that were persisted into
87+
# the organizations table (e.g. by the admin settings form of older versions
88+
# posting back every setting it received) are ignored, so the environment
89+
# variable remains the source of truth.
90+
environment_only = frozenset(["disable_public_urls"])

tests/handlers/test_settings.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,32 @@
33

44

55
class TestOrganizationSettings(BaseTestCase):
6+
def test_environment_only_settings_ignore_stored_values(self):
7+
# A value persisted into the organizations table by an older version
8+
# must not shadow the environment variable (#7630).
9+
self.factory.org.settings["settings"] = {"disable_public_urls": True}
10+
11+
self.assertEqual(self.factory.org.get_setting("disable_public_urls"), False)
12+
13+
def test_post_does_not_persist_environment_only_settings(self):
14+
admin = self.factory.create_admin()
15+
rv = self.make_request(
16+
"post",
17+
"/api/settings/organization",
18+
data={"disable_public_urls": True, "auth_password_login_enabled": True},
19+
user=admin,
20+
)
21+
self.assertEqual(rv.status_code, 200)
22+
self.assertEqual(rv.json["settings"]["disable_public_urls"], False)
23+
self.assertNotIn("disable_public_urls", self.factory.org.settings["settings"])
24+
25+
def test_get_returns_effective_environment_only_settings(self):
26+
admin = self.factory.create_admin()
27+
self.factory.org.settings["settings"] = {"disable_public_urls": True}
28+
29+
rv = self.make_request("get", "/api/settings/organization", user=admin)
30+
self.assertEqual(rv.json["settings"]["disable_public_urls"], False)
31+
632
def test_post(self):
733
admin = self.factory.create_admin()
834
rv = self.make_request(

0 commit comments

Comments
 (0)