diff --git a/server/charm/charmcraft.yaml b/server/charm/charmcraft.yaml index b3113b983..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/" @@ -120,6 +127,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..9b762a23a 100755 --- a/server/charm/src/charm.py +++ b/server/charm/src/charm.py @@ -494,8 +494,13 @@ 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, + "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..675560d0d 100644 --- a/server/charm/src/config.py +++ b/server/charm/src/config.py @@ -18,8 +18,13 @@ 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 = "" + oidc_client_id: str = "" + oidc_client_secret: str = "" + oidc_provider_issuer: str = "" @pydantic.field_validator("external_hostname") @classmethod @@ -47,3 +52,34 @@ 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 scheme and host.""" + 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") + return value.rstrip("/") + + @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..0a1d31768 100644 --- a/server/charm/tests/unit/test_config.py +++ b/server/charm/tests/unit/test_config.py @@ -51,3 +51,93 @@ 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_client_secret + 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 + ) + + 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): + 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 + ) diff --git a/server/src/testflinger/application.py b/server/src/testflinger/application.py index cd7e4fcca..f0a41036f 100644 --- a/server/src/testflinger/application.py +++ b/server/src/testflinger/application.py @@ -16,11 +16,13 @@ """Setup the Testflinger web application.""" import logging +import os from apiflask import APIFlask 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 +105,12 @@ 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 + 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") if tf_app.oauth: 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}} ) 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)