Optional authentication token for API and Web UI - #93
Conversation
Shelly Manager had no login of its own, so anyone reaching the host had full control of stored device credentials, firmware updates and backups (jfmlima#90). Set SHELLY_AUTH_TOKEN to require it - off by default. - API: a Litestar guard checks Authorization: Bearer <token> against SHELLY_AUTH_TOKEN on every route except /api/health, /api/auth/config and /docs, which stay public. New GET /api/auth/config and GET /api/auth/verify endpoints back the Web UI's login flow. - Web UI: a login page collects the token, stores it in localStorage (Remember me) or sessionStorage otherwise, attaches it to every request, and bounces back to /login on a 401. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016kaiVXDHedKfqLgfdGNMwx
valentinocossar
changed the title
jfmlima
left a comment
jfmlima
left a comment
There was a problem hiding this comment.
Hey again @valentinocossar, thanks for this and appreciate the minimal solution.
Left some comments, let me know what you think.
| // The login form's own token check 401s on a wrong password - that's | ||
| // expected and handled inline, not a session being kicked out. | ||
| const isAuthVerify = error.config?.url?.includes("/auth/verify"); | ||
| if (error.response?.status === 401 && !isAuthVerify) { |
There was a problem hiding this comment.
The API also returns 401 for DeviceAuthenticationError (a password protected device with missing or wrong stored credentials), and this treats any non-verify 401 as an expired session, clears the token and redirects to /login. It does that even when SHELLY_AUTH_TOKEN is unset, since the interceptor is unconditional, so opening the detail page of a password protected device kicks the user to the login screen. What do you think about adding WWW-Authenticate: Bearer to the guard's 401 (RFC 7235 wants it there anyway) and only treating 401s carrying that header as a manager logout?
|
|
||
| header = connection.headers.get("authorization", "") | ||
| presented = header.removeprefix("Bearer ") if header.startswith("Bearer ") else "" | ||
| if not presented or not hmac.compare_digest(presented, token): |
There was a problem hiding this comment.
hmac.compare_digest on str raises TypeError when either side contains non-ASCII, so a garbage Authorization header becomes a logged 500 instead of a 401. Can we compare presented.encode() against token.encode()?
| header = connection.headers.get("authorization", "") | ||
| presented = header.removeprefix("Bearer ") if header.startswith("Bearer ") else "" | ||
| if not presented or not hmac.compare_digest(presented, token): | ||
| raise UnauthorizedError() |
There was a problem hiding this comment.
Nothing in core raises this, bearer transport is purely an API concern, so I'd rather keep it out of the domain layer. If the guard raises Litestar's NotAuthorizedException instead, the existing HTTPException handler already produces the same envelope, and both the core exception and the new EXCEPTION_HANDLERS entry can go.
|
|
||
| const form = useForm<LoginFormData>({ | ||
| resolver: zodResolver(loginFormSchema), | ||
| defaultValues: { token: "", rememberMe: true }, |
There was a problem hiding this comment.
Can we default rememberMe to false? Checked by default makes localStorage the default home for a token that grants full control.
| export SHELLY_AUTH_TOKEN="a-strong-random-token" | ||
| ``` | ||
|
|
||
| When set, every API request (except `/api/health` and `/api/auth/config`) must include `Authorization: Bearer <token>`, and the Web UI shows a login page asking for the token before it will load. Leave it unset to keep the zero-configuration default. Unlike `SHELLY_SECRET_KEY`, this is optional and unrelated to credential encryption. |
There was a problem hiding this comment.
/docs and /docs/openapi.json also stay public, can we list them here? Worth also recommending a long random token, since there's no rate limiting on /auth/verify.
2 participants
Summary
Closes #90. Adds an optional, off-by-default admin token that gates both the API and the Web UI, as a shared-secret token rather than HTTP Basic Auth or cookies (see the issue for why: cross-origin API/Web UI deployments break both of those cleanly).
SHELLY_AUTH_TOKEN(unset by default = no auth, same zero-config philosophy as the rest of the app). A Litestar guard checksAuthorization: Bearer <token>on every route exceptGET /api/health,GET /api/auth/configand/docs, which stay public. NewGET /api/auth/config({"enabled": bool}) andGET /api/auth/verifyback the Web UI's login flow./api/auth/verify, and stores it inlocalStorageif "Remember me" is checked orsessionStorageotherwise. Every request attaches it via an axios interceptor; a 401 anywhere clears it and bounces back to/login.Open to feedback on the approach (env var placement/naming,
/docsstaying public, the shared-secret + verify-endpoint model) - happy to adjust based on how you'd want this to look.Breaking change (opt-in only)
Nothing changes for anyone who leaves
SHELLY_AUTH_TOKENunset - default behavior is identical to before this PR. But for anyone who sets it: every existing integration that talks to the API directly (Home Assistant, scripts, curl in a cron job, etc.) will start getting 401s until it's updated to sendAuthorization: Bearer <token>. Worth calling out explicitly since it is not purely additive once someone opts in.Test plan
make lint(black, ruff, mypy) - cleanmake test-core- 996 passedmake test-api- 129 passed (13 new: guard behavior,/auth/config+/auth/verify, app-level router composition with/without a token set)npm run type-check,npm run lint,npm run format:check,npm run build- clean/api/health,/api/auth/configand/docsstay public, everything else 401s without the header and works withAuthorization: Bearer <token>Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_016kaiVXDHedKfqLgfdGNMwx