Add basic password auth to the reduction dashboard #468
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds basic password authentication to the reduction dashboard to provide minimal security before implementing ESS SSO integration. The authentication uses environment variables for configuration and provides a simple login screen.
- Added basic authentication parameters to dashboard initialization chain
- Integrated authentication configuration through command-line arguments and environment variables
- Updated panel server configuration to enable basic authentication
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/ess/livedata/dashboard/reduction.py | Added basic auth parameters to ReductionApp constructor and enabled dashboard auth in argument parser |
| src/ess/livedata/dashboard/dashboard.py | Added basic auth support to DashboardBase class and panel server configuration |
| src/ess/livedata/core/service.py | Added command-line argument parsing for basic authentication credentials and minor formatting cleanup |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| basic_auth=self._basic_auth_password, | ||
| cookie_secret=self._basic_auth_cookie_secret, |
There was a problem hiding this comment.
Basic authentication credentials are being passed directly to the panel server. Consider validating that both basic_auth_password and basic_auth_cookie_secret are provided together, as partial authentication configuration could lead to unexpected behavior or security issues.
Copilot uses AI. Check for mistakes.
| parser.add_argument( | ||
| '--basic-auth-password', | ||
| default=None, | ||
| help='Basic authentication password for the dashboard', |
There was a problem hiding this comment.
Command-line arguments for passwords can be visible in process lists and shell history. Consider adding a warning in the help text to recommend using the environment variable LIVEDATA_BASIC_AUTH_PASSWORD instead for better security.
| help='Basic authentication password for the dashboard', | |
| help='Basic authentication password for the dashboard. ' | |
| 'WARNING: Command-line arguments can be visible in process lists and shell history. ' | |
| 'For better security, use the environment variable LIVEDATA_BASIC_AUTH_PASSWORD instead.', |
Copilot uses AI. Check for mistakes.
| def get_arg_parser() -> argparse.ArgumentParser: | ||
| return Service.setup_arg_parser(description='ESSlivedata Dashboard') | ||
| return Service.setup_arg_parser( | ||
| description='ESSlivedata Dashboard', dashboard_auth=True | ||
| ) |
There was a problem hiding this comment.
Since auth is only for the dashboard, it might make more sense to add the options here before returning, instead of adding a flag to the to the generic Service, which is used by the backend workers as well?
|
Making this draft for now. Let's wait to see how we can setup auth that makes DST happy too. |
2 participants
This adds some minimal auth in front of the panel dashboard. Running the dashboard with
LIVEDATA_BASIC_AUTH_COOKIE_SECRETandLIVEDATA_BASIC_AUTH_PASSWORDenv variable should set a password (and login with any random username).It will give a screen like this
Before adding ESS SSO I want to test this out with DST infra.