From 95d7838011748e06b301dde3d71db06e06896bcf Mon Sep 17 00:00:00 2001 From: Rene Orozco Martinez Date: Wed, 6 May 2026 14:55:28 -0600 Subject: [PATCH 1/6] feat(server): allow charm to provide oidc variables --- server/charm/charmcraft.yaml | 16 ++++++ server/charm/src/charm.py | 4 ++ server/charm/src/config.py | 37 ++++++++++++ server/charm/tests/unit/test_charm.py | 27 +++++++++ server/charm/tests/unit/test_config.py | 80 ++++++++++++++++++++++++++ 5 files changed, 164 insertions(+) diff --git a/server/charm/charmcraft.yaml b/server/charm/charmcraft.yaml index b3113b983..44e9dcc27 100644 --- a/server/charm/charmcraft.yaml +++ b/server/charm/charmcraft.yaml @@ -120,6 +120,22 @@ config: type: string default: "" description: Optional bearer token for authenticating with the webhook endpoint + web_secret_key: + type: string + default: "" + description: The secret key used for signing web sessions and cookies + oidc_client_id: + type: string + default: "" + description: The OIDC client ID used for authenticating with the OIDC provider + oidc_client_secret: + type: string + default: "" + description: The OIDC client secret used for authenticating with the OIDC provider + oidc_provider_issuer: + type: string + default: "" + description: The OIDC provider issuer URL for authenticating users parts: diff --git a/server/charm/src/charm.py b/server/charm/src/charm.py index d74aa1a7b..037100edd 100755 --- a/server/charm/src/charm.py +++ b/server/charm/src/charm.py @@ -496,6 +496,10 @@ def app_environment(self) -> dict: "NO_PROXY": self.typed_config.no_proxy, "WEBHOOK_URL": self.typed_config.webhook_url, "WEBHOOK_AUTH": self.typed_config.webhook_auth, + "WEB_SECRET_KEY": self.typed_config.web_secret_key, + "OIDC_CLIENT_ID": self.typed_config.oidc_client_id, + "OIDC_CLIENT_SECRET": self.typed_config.oidc_client_secret, + "OIDC_PROVIDER_ISSUER": self.typed_config.oidc_provider_issuer, } return env diff --git a/server/charm/src/config.py b/server/charm/src/config.py index 07b39d8e8..58b873165 100644 --- a/server/charm/src/config.py +++ b/server/charm/src/config.py @@ -20,6 +20,10 @@ class TestflingerServerConfig(pydantic.BaseModel): no_proxy: str = "localhost,127.0.0.1,::1" webhook_url: str = "http://test-observer-api.local/" webhook_auth: str = "" + web_secret_key: str = "" + oidc_client_id: str = "" + oidc_client_secret: str = "" + oidc_provider_issuer: str = "" @pydantic.field_validator("external_hostname") @classmethod @@ -47,3 +51,36 @@ def validate_webhook_url(cls, value): raise ValueError("webhook_url must include a host") return value + + @pydantic.field_validator("oidc_provider_issuer") + @classmethod + def validate_oidc_provider_issuer(cls, value): + """Validate oidc_provider_issuer includes a protocol and no paths.""" + if not value: + return value + + parsed_oidc = urlparse(value) + if parsed_oidc.scheme not in {"http", "https"}: + raise ValueError( + "oidc_provider_issuer must include protocol (http:// or https://)" + ) + if not parsed_oidc.netloc: + raise ValueError("oidc_provider_issuer must include a host") + if parsed_oidc.path != "": + raise ValueError("oidc_provider_issuer must not include a path") + return value + + @pydantic.model_validator(mode="after") + def validate_oidc_config(self): + """Validate that all OIDC parameters are set if any are configured.""" + oidc_params = [ + self.oidc_client_id, + self.oidc_client_secret, + self.oidc_provider_issuer, + self.web_secret_key, + ] + if any(oidc_params) and not all(oidc_params): + raise ValueError( + "All OIDC parameters must be set if any are configured" + ) + return self diff --git a/server/charm/tests/unit/test_charm.py b/server/charm/tests/unit/test_charm.py index 7fe7f27ae..cc1d84e77 100644 --- a/server/charm/tests/unit/test_charm.py +++ b/server/charm/tests/unit/test_charm.py @@ -531,3 +531,30 @@ def test_retry_key_rotation_action_exec_error(_, ctx, make_state): with pytest.raises(testing.ActionFailed): ctx.run(ctx.on.action("retry-key-rotation"), state_in) + + +def test_oidc_config_set_on_charm(ctx, make_state): + """Test charm is active when OIDC config is set and valid.""" + oidc_config = { + "oidc_client_id": "client-id", + "oidc_client_secret": "client-secret", + "oidc_provider_issuer": "https://oidc-provider.local", + "web_secret_key": "web-secret-key", + } + state_in = make_state(config=oidc_config) + state_out = ctx.run(ctx.on.config_changed(), state_in) + assert state_out.unit_status == testing.ActiveStatus() + + +def test_oidc_config_invalid_on_charm(ctx, make_state): + """Test charm is blocked when OIDC config is invalid.""" + oidc_config = { + "oidc_client_id": "client-id", + "oidc_client_secret": "client-secret", + } + state_in = make_state(config=oidc_config) + + # Pydantic validation runs in __init__ via load_config(errors="blocked"), + # which sets BlockedStatus and raises _Abort before the hook handler runs. + with pytest.raises(testing.errors.UncaughtCharmError): + _ = ctx.run(ctx.on.config_changed(), state_in) diff --git a/server/charm/tests/unit/test_config.py b/server/charm/tests/unit/test_config.py index eff1c84a8..4926a86d6 100644 --- a/server/charm/tests/unit/test_config.py +++ b/server/charm/tests/unit/test_config.py @@ -51,3 +51,83 @@ def test_invalid_webhook_url(): # Reject webhook url that does not include hostname with pytest.raises(ValueError): TestflingerServerConfig(webhook_url="https:///v1/test-executions/") + + +def test_valid_oidc_configuration(): + """Test that OIDC configuration is validated correctly.""" + # Valid OIDC config with all parameters set + config = TestflingerServerConfig( + oidc_client_id="client-id", + oidc_client_secret="client-secret", # noqa: S106 + oidc_provider_issuer="https://oidc-provider.local", + web_secret_key="web-secret-key", # noqa: S106 + ) + assert config.oidc_client_id == "client-id" + assert config.oidc_client_secret == "client-secret" # noqa: S105 + assert config.oidc_provider_issuer == "https://oidc-provider.local" + assert config.web_secret_key == "web-secret-key" # noqa: S105 + + +def test_invalid_oidc_configuration(): + """Test that invalid OIDC configuration raises validation error.""" + # Missing web_secret_key + with pytest.raises(ValueError): + TestflingerServerConfig( + oidc_client_id="client-id", + oidc_client_secret="client-secret", # noqa: S106 + oidc_provider_issuer="https://oidc-provider.local", + ) + + # Missing oidc_provider_issuer + with pytest.raises(ValueError): + TestflingerServerConfig( + oidc_client_id="client-id", + oidc_client_secret="client-secret", # noqa: S106 + web_secret_key="web-secret-key", # noqa: S106 + ) + + # Missing oidc_secret_key + with pytest.raises(ValueError): + TestflingerServerConfig( + oidc_client_id="client-id", + oidc_provider_issuer="https://oidc-provider.local", + web_secret_key="web-secret-key", # noqa: S106 + ) + + # Missing oidc_client_id + with pytest.raises(ValueError): + TestflingerServerConfig( + oidc_client_secret="client-secret", # noqa: S106 + oidc_provider_issuer="https://oidc-provider.local", + web_secret_key="web-secret-key", # noqa: S106 + ) + + +def test_invalid_oidc_provider_issuer(): + """Test that invalid OIDC provider issuer raises validation error.""" + # Reject oidc_provider_issuer that do not include protocol + with pytest.raises(ValueError): + TestflingerServerConfig( + oidc_client_id="client-id", + oidc_client_secret="client-secret", # noqa: S106 + oidc_provider_issuer="oidc-provider.local", + web_secret_key="web-secret-key", # noqa: S106 + ) + + # Reject oidc_provider_issuer that include path + with pytest.raises(ValueError): + TestflingerServerConfig( + oidc_client_id="client-id", + oidc_client_secret="client-secret", # noqa: S106 + oidc_provider_issuer="https://oidc-provider.local/issuer", + web_secret_key="web-secret-key", # noqa: S106 + ) + + # Reject oidc_provider_issuer that does not include hostname + with pytest.raises(ValueError): + TestflingerServerConfig( + oidc_client_id="client-id", + oidc_client_secret="client-secret", # noqa: S106 + oidc_provider_issuer="https:///issuer", + web_secret_key="web-secret-key", # noqa: S106 + ) From 797850aebfc72e4b881e959547129902c4d37109 Mon Sep 17 00:00:00 2001 From: Rene Orozco Martinez Date: Thu, 7 May 2026 08:25:59 -0600 Subject: [PATCH 2/6] fix(server): tell flask is behind a proxy --- server/src/testflinger/application.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/src/testflinger/application.py b/server/src/testflinger/application.py index cd7e4fcca..89581196b 100644 --- a/server/src/testflinger/application.py +++ b/server/src/testflinger/application.py @@ -21,6 +21,7 @@ from flask import request from pymongo.errors import ConnectionFailure from werkzeug.exceptions import NotFound +from werkzeug.middleware.proxy_fix import ProxyFix from testflinger.api.v1 import LogTypeConverter, v1 from testflinger.database import setup_mongodb @@ -103,6 +104,11 @@ def inject_oidc_status(): def inject_vanilla_framework_version(): return {"vanilla_framework_version": VANILLA_FRAMEWORK_VERSION} + # Tell Flask it's behind a proxy so it can properly handle redirects + tf_app.wsgi_app = ProxyFix( + tf_app.wsgi_app, x_for=1, x_proto=1, x_host=1, x_prefix=1 + ) + tf_app.register_blueprint(views) tf_app.register_blueprint(v1, url_prefix="/v1") if tf_app.oauth: From 00dd5b7943d74a62de8779c51a772ec437362e1d Mon Sep 17 00:00:00 2001 From: Rene Orozco Martinez Date: Thu, 21 May 2026 12:25:08 -0600 Subject: [PATCH 3/6] refactor: make ProxyFix conditional --- server/charm/charmcraft.yaml | 7 +++++++ server/charm/src/charm.py | 1 + server/charm/src/config.py | 1 + server/src/testflinger/application.py | 8 +++++--- server/tests/test_app.py | 13 +++++++++++++ 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/server/charm/charmcraft.yaml b/server/charm/charmcraft.yaml index 44e9dcc27..77598c61f 100644 --- a/server/charm/charmcraft.yaml +++ b/server/charm/charmcraft.yaml @@ -112,6 +112,13 @@ config: type: string default: "localhost,127.0.0.1,::1" description: Resources that we should be able to access bypassing proxy + behind_proxy: + type: boolean + default: false + description: | + Whether the application is running behind a proxy. + Required if OIDC is enabled and the application is running behind a proxy, + so that redirects from the OIDC provider can be properly handled. webhook_url: type: string default: "http://test-observer-api.local/" diff --git a/server/charm/src/charm.py b/server/charm/src/charm.py index 037100edd..9b762a23a 100755 --- a/server/charm/src/charm.py +++ b/server/charm/src/charm.py @@ -494,6 +494,7 @@ def app_environment(self) -> dict: "HTTP_PROXY": self.typed_config.http_proxy, "HTTPS_PROXY": self.typed_config.https_proxy, "NO_PROXY": self.typed_config.no_proxy, + "BEHIND_PROXY": str(self.typed_config.behind_proxy), "WEBHOOK_URL": self.typed_config.webhook_url, "WEBHOOK_AUTH": self.typed_config.webhook_auth, "WEB_SECRET_KEY": self.typed_config.web_secret_key, diff --git a/server/charm/src/config.py b/server/charm/src/config.py index 58b873165..37765d004 100644 --- a/server/charm/src/config.py +++ b/server/charm/src/config.py @@ -18,6 +18,7 @@ class TestflingerServerConfig(pydantic.BaseModel): http_proxy: str = "" https_proxy: str = "" no_proxy: str = "localhost,127.0.0.1,::1" + behind_proxy: bool = False webhook_url: str = "http://test-observer-api.local/" webhook_auth: str = "" web_secret_key: str = "" diff --git a/server/src/testflinger/application.py b/server/src/testflinger/application.py index 89581196b..f0a41036f 100644 --- a/server/src/testflinger/application.py +++ b/server/src/testflinger/application.py @@ -16,6 +16,7 @@ """Setup the Testflinger web application.""" import logging +import os from apiflask import APIFlask from flask import request @@ -105,9 +106,10 @@ def inject_vanilla_framework_version(): return {"vanilla_framework_version": VANILLA_FRAMEWORK_VERSION} # Tell Flask it's behind a proxy so it can properly handle redirects - tf_app.wsgi_app = ProxyFix( - tf_app.wsgi_app, x_for=1, x_proto=1, x_host=1, x_prefix=1 - ) + if os.environ.get("BEHIND_PROXY", "false").lower() == "true": + tf_app.wsgi_app = ProxyFix( + tf_app.wsgi_app, x_for=1, x_proto=1, x_host=1, x_prefix=1 + ) tf_app.register_blueprint(views) tf_app.register_blueprint(v1, url_prefix="/v1") diff --git a/server/tests/test_app.py b/server/tests/test_app.py index 588579c30..964b21474 100644 --- a/server/tests/test_app.py +++ b/server/tests/test_app.py @@ -16,6 +16,7 @@ """Unit tests for Testflinger flask app.""" import pytest +from werkzeug.middleware.proxy_fix import ProxyFix from testflinger.application import create_flask_app @@ -31,3 +32,15 @@ def test_setup_mongo_fails_without_config(): with pytest.raises(SystemExit) as exc: create_flask_app() assert exc.value.code == "No MongoDB URI configured!" + + +def test_proxyfix_enabled(monkeypatch): + """Ensure ProxyFix is enabled when BEHIND_PROXY is true.""" + monkeypatch.setenv("BEHIND_PROXY", "true") + app = create_flask_app(type("", (), {"TESTING": True})()) + assert isinstance(app.wsgi_app, ProxyFix) + + +def test_proxyfix_disabled(testapp): + """Ensure ProxyFix is disabled when BEHIND_PROXY is not set.""" + assert not isinstance(testapp.wsgi_app, ProxyFix) From 8b40916a4902f2d00acd29387374c4499637fdca Mon Sep 17 00:00:00 2001 From: Rene Orozco Martinez Date: Thu, 21 May 2026 13:48:44 -0600 Subject: [PATCH 4/6] mongodb hotfix --- server/src/testflinger/database.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/testflinger/database.py b/server/src/testflinger/database.py index ac292712d..5bb1eea06 100644 --- a/server/src/testflinger/database.py +++ b/server/src/testflinger/database.py @@ -95,10 +95,10 @@ def create_indexes(): ) # Remove artifacts after 7 days - mongo.db.fs.chunks.create_index( + mongo.db["fs.chunks"].create_index( "uploadDate", expireAfterSeconds=DEFAULT_EXPIRATION ) - mongo.db.fs.files.create_index( + mongo.db["fs.files"].create_index( "uploadDate", expireAfterSeconds=DEFAULT_EXPIRATION ) @@ -134,8 +134,8 @@ def save_file(data: Any, filename: str): storage = GridFS(mongo.db) file_id = storage.put(data, filename=filename) # Add a timestamp to the chunks - do this so we can set a TTL for them - timestamp = mongo.db.fs.files.find_one({"_id": file_id})["uploadDate"] - mongo.db.fs.chunks.update_many( + timestamp = mongo.db["fs.files"].find_one({"_id": file_id})["uploadDate"] + mongo.db["fs.chunks"].update_many( {"files_id": file_id}, {"$set": {"uploadDate": timestamp}} ) From 1d84f048cbdf94116a7cea380f9e3422ddb73809 Mon Sep 17 00:00:00 2001 From: Rene Orozco Martinez Date: Thu, 21 May 2026 14:40:18 -0600 Subject: [PATCH 5/6] add copilot suggestions --- server/charm/src/config.py | 6 ++---- server/charm/tests/unit/test_config.py | 25 ++++++++++++++++--------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/server/charm/src/config.py b/server/charm/src/config.py index 37765d004..675560d0d 100644 --- a/server/charm/src/config.py +++ b/server/charm/src/config.py @@ -56,7 +56,7 @@ def validate_webhook_url(cls, value): @pydantic.field_validator("oidc_provider_issuer") @classmethod def validate_oidc_provider_issuer(cls, value): - """Validate oidc_provider_issuer includes a protocol and no paths.""" + """Validate oidc_provider_issuer includes a scheme and host.""" if not value: return value @@ -67,9 +67,7 @@ def validate_oidc_provider_issuer(cls, value): ) if not parsed_oidc.netloc: raise ValueError("oidc_provider_issuer must include a host") - if parsed_oidc.path != "": - raise ValueError("oidc_provider_issuer must not include a path") - return value + return value.rstrip("/") @pydantic.model_validator(mode="after") def validate_oidc_config(self): diff --git a/server/charm/tests/unit/test_config.py b/server/charm/tests/unit/test_config.py index 4926a86d6..be6935b61 100644 --- a/server/charm/tests/unit/test_config.py +++ b/server/charm/tests/unit/test_config.py @@ -86,7 +86,7 @@ def test_invalid_oidc_configuration(): web_secret_key="web-secret-key", # noqa: S106 ) - # Missing oidc_secret_key + # Missing oidc_client_secret with pytest.raises(ValueError): TestflingerServerConfig( oidc_client_id="client-id", @@ -114,14 +114,21 @@ def test_invalid_oidc_provider_issuer(): web_secret_key="web-secret-key", # noqa: S106 ) - # Reject oidc_provider_issuer that include path - with pytest.raises(ValueError): - TestflingerServerConfig( - oidc_client_id="client-id", - oidc_client_secret="client-secret", # noqa: S106 - oidc_provider_issuer="https://oidc-provider.local/issuer", - web_secret_key="web-secret-key", # noqa: S106 - ) + config = TestflingerServerConfig( + oidc_client_id="client-id", + oidc_client_secret="client-secret", # noqa: S106 + oidc_provider_issuer="https://keycloak.example.com/realms/myrealm", + web_secret_key="web-secret-key", # noqa: S106 + ) + assert config.oidc_provider_issuer == "https://keycloak.example.com/realms/myrealm" + + config = TestflingerServerConfig( + oidc_client_id="client-id", + oidc_client_secret="client-secret", # noqa: S106 + oidc_provider_issuer="https://oidc-provider.local/", + web_secret_key="web-secret-key", # noqa: S106 + ) + assert config.oidc_provider_issuer == "https://oidc-provider.local" # Reject oidc_provider_issuer that does not include hostname with pytest.raises(ValueError): From 8e5f2ba7d4eaeaa937ba8132dee29075e7ca2fe5 Mon Sep 17 00:00:00 2001 From: Rene Orozco Martinez Date: Thu, 21 May 2026 14:42:01 -0600 Subject: [PATCH 6/6] add formatting --- server/charm/tests/unit/test_config.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/charm/tests/unit/test_config.py b/server/charm/tests/unit/test_config.py index be6935b61..0a1d31768 100644 --- a/server/charm/tests/unit/test_config.py +++ b/server/charm/tests/unit/test_config.py @@ -120,7 +120,10 @@ def test_invalid_oidc_provider_issuer(): oidc_provider_issuer="https://keycloak.example.com/realms/myrealm", web_secret_key="web-secret-key", # noqa: S106 ) - assert config.oidc_provider_issuer == "https://keycloak.example.com/realms/myrealm" + assert ( + config.oidc_provider_issuer + == "https://keycloak.example.com/realms/myrealm" + ) config = TestflingerServerConfig( oidc_client_id="client-id",