Skip to content

Commit

Permalink
Add the ability for Feature Flags to work without user sessions.
Browse files Browse the repository at this point in the history
This requires some updates to how the Flask client creator for tests
works. This commit contains an intentionally broken test.
  • Loading branch information
aholmes committed Sep 23, 2024
1 parent 7704358 commit dc63f31
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 14 deletions.
49 changes: 38 additions & 11 deletions src/web/BL_Python/web/middleware/feature_flags/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,25 +106,52 @@ def _provide_caching_feature_flag_router(
return injector.get(self._t_feature_flag)


def get_feature_flag_blueprint(config: FeatureFlagConfig):
@inject
def get_feature_flag_blueprint(app: Flask, config: FeatureFlagConfig, log: Logger):
feature_flag_blueprint = Blueprint(
"feature_flag", __name__, url_prefix=f"{config.api_base_url}"
)

access_role = config.access_role_name

def _login_required(fn: Callable[..., Any]):
if access_role is False:
return fn
def _login_required(require_flask_login: bool):
"""
Decorate an API endpoint with the correct flask_login authentication
method given the requirements of the API endpoint.
# None means no roles were specified, but a session is still required
if access_role is None or access_role is True:
return login_required(fn)
require_flask_login is ignored if flask_login has been configured.
return login_required([access_role])(fn)
If flask_login has _not_ been configured:
* If require_flask_login is True, a warning is logged and a method returning False is returned, rather than returning the endpoint function
* If require_flask_login is False, the endpoint function is returned
:param bool require_flask_login: Determine whether flask_login must be configured for this endpoint to function
:return _type_: _description_
"""
if not hasattr(app, "login_manager"):
if require_flask_login:
log.warning(
"The Feature Flag module expects flask_login to be configured in order to control access to feature flag modifications. flask_login has not been configured, so the Feature Flag modification API is disabled."
)
return lambda *args, **kwargs: False
else:
return lambda fn: fn

def _login_required(fn: Callable[..., Any]):
if access_role is False:
return fn

# None means no roles were specified, but a session is still required
if access_role is None or access_role is True:
# FIXME feature flags needs a login_manager assigned to Flask
return login_required(fn)

return login_required([access_role])(fn)

return _login_required

@feature_flag_blueprint.route("/feature_flag", methods=("GET",)) # pyright: ignore[reportArgumentType,reportUntypedFunctionDecorator]
@_login_required
@_login_required(False)
@inject
def feature_flag(feature_flag_router: FeatureFlagRouter[FeatureFlag]): # pyright: ignore[reportUnusedFunction]
request_query_names: list[str] | None = request.query_params.getlist("name")
Expand Down Expand Up @@ -174,7 +201,7 @@ def feature_flag(feature_flag_router: FeatureFlagRouter[FeatureFlag]): # pyrigh
@feature_flag_blueprint.route("/feature_flag", methods=("PATCH",)) # pyright: ignore[reportArgumentType,reportUntypedFunctionDecorator]
# @login_required([UserRole.Operator])
# TODO assign a specific role ?
@_login_required
@_login_required(True)
@inject
async def feature_flag_patch(feature_flag_router: FeatureFlagRouter[FeatureFlag]): # pyright: ignore[reportUnusedFunction]
feature_flags_request: list[FeatureFlagPatchRequest] = await request.json()
Expand Down Expand Up @@ -256,7 +283,7 @@ async def wrapped_send(message: Any) -> None:

log.debug("Registering FeatureFlag blueprint.")
app.register_blueprint(
get_feature_flag_blueprint(injector.get(FeatureFlagConfig))
injector.call_with_injection(get_feature_flag_blueprint)
)
log.debug("FeatureFlag blueprint registered.")

Expand Down
2 changes: 0 additions & 2 deletions src/web/BL_Python/web/testing/create_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,6 @@ def __get_basic_flask_app(
application_configs = []
if application_modules is None:
application_modules = []
application_configs.append(SSOConfig)
application_modules.append(SAML2MiddlewareModule)
app = App[Flask].create("config.toml", application_configs, application_modules)
yield app
Expand Down Expand Up @@ -538,7 +537,6 @@ def _get_real_openapi_app(
application_configs = []
if application_modules is None:
application_modules = []
application_configs.append(SSOConfig)
application_modules.append(SAML2MiddlewareModule)
application_modules.append(
UserLoaderModule(
Expand Down
38 changes: 37 additions & 1 deletion src/web/test/unit/middleware/test_feature_flags_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
FeatureFlagConfig,
FeatureFlagMiddlewareModule,
)
from BL_Python.web.middleware.sso import SAML2MiddlewareModule
from BL_Python.web.testing.create_app import (
CreateOpenAPIApp,
OpenAPIClientInjectorConfigurable,
Expand Down Expand Up @@ -97,7 +98,7 @@ def _user_session_app_init_hook(
application_modules.append(CachingFeatureFlagRouterModule)
application_modules.append(FeatureFlagMiddlewareModule())

def test__FeatureFlagMiddleware__feature_flag_api_GET_requires_user_session(
def test__FeatureFlagMiddleware__feature_flag_api_GET_requires_user_session_when_flask_login_is_configured(
self,
openapi_config: Config,
openapi_client_configurable: OpenAPIClientInjectorConfigurable,
Expand All @@ -121,6 +122,41 @@ def app_init_hook(
# if SSO was broken, 500 would return
assert response.status_code == 401

def test__FeatureFlagMiddleware__feature_flag_api_GET_does_not_require_user_session_when_flask_login_is_not_configured(
self,
openapi_config: Config,
openapi_client_configurable: OpenAPIClientInjectorConfigurable,
openapi_mock_controller: OpenAPIMockController,
):
def app_init_hook(
application_configs: list[type[AbstractConfig]],
application_modules: list[Module | type[Module]],
):
# application_modules.remove(SAML2MiddlewareModule)
# application_modules.remove(UserLoaderModule)
application_modules.append(CachingFeatureFlagRouterModule)
application_modules.append(FeatureFlagMiddlewareModule())

def client_init_hook(app: CreateAppResult[FlaskApp]):
pass

openapi_mock_controller.begin()
app = next(
openapi_client_configurable(
openapi_config,
client_init_hook=client_init_hook,
app_init_hook=app_init_hook,
)
)

# del app.client.app.app.login_manager

response = app.client.get("/platform/feature_flag")

# 401 for now because no real auth is configured.
# if SSO was broken, 500 would return
assert response.status_code == 200

def test__FeatureFlagMiddleware__feature_flag_api_GET_gets_feature_flags_when_user_has_session(
self,
openapi_config: Config,
Expand Down

0 comments on commit dc63f31

Please sign in to comment.