feat: add HTTP Basic Auth support for nginx reverse proxy#81
Conversation
Add --auth-user and --auth-password CLI args (also configurable via TOML) for servers behind nginx with auth_basic. Same approach as aw-watcher-window: replace aw-client's _get/_post/_delete methods to inject Authorization headers.
Greptile SummaryThis PR adds HTTP Basic Auth support for connecting through an nginx reverse proxy, following the same pattern as aw-watcher-window#127. Auth credentials are read from the TOML config or CLI args and injected into
Confidence Score: 3/5The core auth logic works when both credentials are supplied, but a misconfigured install silently drops auth, leaving the watcher to fail with unexplained 401s. When only one credential is configured the guard quietly disables auth with no log warning, so a user who makes a partial config mistake will see mysterious 401 errors with no clue the credentials were ignored. The monkey-patch also relies on three undocumented private symbols from aw_client, so any upstream refactor can silently break auth. aw_watcher_afk/afk.py — the silent credential skip and the dependency on private aw_client internals both live here.
|
| Filename | Overview |
|---|---|
| aw_watcher_afk/afk.py | Adds _patch_client_auth to monkey-patch aw_client private HTTP methods with Basic Auth; silent skip when only one credential is set, incorrect charset header, and fragile dependency on private aw_client internals. |
| aw_watcher_afk/config.py | Adds auth_user/auth_password TOML defaults and CLI args; --auth-password is exposed in process listings without any warning in the help text. |
Reviews (1): Last reviewed commit: "feat: add HTTP Basic Auth support for ng..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if args.auth_user and args.auth_password: | ||
| _patch_client_auth(self.client, args.auth_user, args.auth_password) | ||
| logger.info("HTTP Basic Auth enabled for user: %s", args.auth_user) |
There was a problem hiding this comment.
When a user sets auth_user but forgets auth_password (or vice versa), the condition evaluates to False and auth is silently disabled — no warning, no error. This is especially confusing because the config write was intentional, but the watcher will connect without credentials and fail with an HTTP 401 from nginx, with no indication the auth config was ignored.
|
|
||
| @always_raise_for_request_errors | ||
| def _post(self_ref, endpoint, data, params=None): | ||
| headers = {"Content-type": "application/json", "charset": "utf-8"} |
There was a problem hiding this comment.
"charset" key is set as a standalone HTTP header rather than as a parameter inside the Content-Type value. Per RFC 7231, charset belongs in the media type parameter: application/json; charset=utf-8. As a separate header named charset, most servers will ignore it entirely.
| headers = {"Content-type": "application/json", "charset": "utf-8"} | |
| headers = {"Content-type": "application/json; charset=utf-8"} |
| parser.add_argument( | ||
| "--auth-password", | ||
| dest="auth_password", | ||
| default=default_auth_password, | ||
| help="Password for HTTP Basic Auth (for nginx-proxied servers)", | ||
| ) |
There was a problem hiding this comment.
--auth-password is visible in process listings
Passing a password via CLI argument makes it readable to any user on the system via ps aux or /proc/<pid>/cmdline. The help text doesn't warn about this. Consider noting in the help string that using the TOML config file is more secure, or supporting an environment variable fallback (e.g. AW_AUTH_PASSWORD).
| def _patch_client_auth(client, user, password): | ||
| """Replace aw-client HTTP methods to inject Basic Auth credentials.""" | ||
| from aw_client.client import always_raise_for_request_errors | ||
|
|
||
| auth = requests.auth.HTTPBasicAuth(user, password) | ||
| _url = client._url | ||
|
|
||
| @always_raise_for_request_errors | ||
| def _get(self_ref, endpoint, params=None): | ||
| return requests.get(_url(endpoint), params=params, auth=auth) | ||
|
|
||
| @always_raise_for_request_errors | ||
| def _post(self_ref, endpoint, data, params=None): | ||
| headers = {"Content-type": "application/json", "charset": "utf-8"} | ||
| return requests.post(_url(endpoint), data=bytes(json.dumps(data), "utf8"), headers=headers, params=params, auth=auth) | ||
|
|
||
| @always_raise_for_request_errors | ||
| def _delete(self_ref, endpoint, data=None): | ||
| if data is None: | ||
| data = {} | ||
| headers = {"Content-type": "application/json"} | ||
| return requests.delete(_url(endpoint), data=json.dumps(data), headers=headers, auth=auth) | ||
|
|
||
| client._get = lambda endpoint, params=None: _get(client, endpoint, params) | ||
| client._post = lambda endpoint, data, params=None: _post(client, endpoint, data, params) | ||
| client._delete = lambda endpoint, data=None: _delete(client, endpoint, data) |
There was a problem hiding this comment.
aw_client internals
_patch_client_auth imports always_raise_for_request_errors from aw_client.client and replaces the private _get/_post/_delete instance methods, and also closes over client._url. All of these are undocumented private APIs. Any refactor of aw-client can silently break auth with no clear diagnostic.
1 participant
Summary
Add HTTP Basic Auth support for connecting to ActivityWatch servers behind an nginx reverse proxy with
auth_basic.Changes
auth_user/auth_passwordto TOML config defaults--auth-user/--auth-passwordCLI args_get,_post,_deletemethods with implementations that injectauth=HTTPBasicAuth(user, password)Usage
Same implementation approach as aw-watcher-window#127.