refactor(penpot-mcp): drop Authelia/cookie/password auth; access-token only #68

Merged
code-lead merged 2 commits from refactor/penpot-access-token-auth into main 2026-04-18 22:49:41 +00:00
Collaborator

Summary

Penpot is now reachable directly on the LAN (no Authelia forward-auth proxy
in front) with access tokens enabled in the server config. That undoes the
reason this fork of penpot-mcp diverged from upstream on auth: versions
0.1–0.3 carried an OIDC-cookie path + Authelia-basic-auth path + a
password-login fallback because the instance was proxy-fronted and
OIDC-only. None of that is needed anymore.

  • penpot-mcp 0.4.0 — removes AUTHELIA_BASIC_AUTH,
    PENPOT_AUTH_TOKEN_COOKIE, PENPOT_EMAIL, PENPOT_PASSWORD, and the
    _login() helper. Adds PENPOT_ACCESS_TOKEN as the sole auth path
    (sent as Authorization: Token <value> on every RPC call). Fork now
    only diverges from upstream on design-token writes (v0.3); auth is
    upstream-compatible again.
  • Service startup (src/main.ts) — replaces the triple-secret load
    with a single ~/.config/claude-hooks/penpot-token read.
  • DocsCLAUDE.md § Penpot MCP auth rewritten; fork README.md
    and CHANGELOG.md updated with the 0.4.0 rationale.

Surfaced while running the #56 operational smoke on #62 — the OIDC cookie
kept returning authentication-required from Penpot (session not
reissuable without the Authelia proxy flow). Simpler to remove the whole
stack than patch around the edges.

Migration for operators

# One-time cleanup + new secret
rm -f ~/.config/claude-hooks/{authelia-creds,penpot-cookie,penpot-creds}
# Create one in the Penpot UI: Profile → Access tokens
echo '<token-value>' > ~/.config/claude-hooks/penpot-token
chmod 600 ~/.config/claude-hooks/penpot-token
# Rebuild designer/design-reviewer containers, restart service

Test plan

  • just qa — 234 tests, fmt/lint clean.
  • Upstream-facing Python tests (test_get_design_tokens.py) still
    pass in-container. test_tokens.py fails pre-existing on main
    (FastMCP library drift — separate issue).
  • After merge + deploy: get_file_info against
    689d7fa4-f94b-81d4-8007-e39c5c82f66c returns JSON (auth works).
    Will be checked once the access-token value is in place on the
    host.

Follow-ups

  • Option C from the issue thread — extend the fork with
    create_file / create_page / create_frame / create_text /
    export_frame_png so the designer can actually satisfy #62's AC
    (the operational smoke's remaining blocker).
## Summary Penpot is now reachable directly on the LAN (no Authelia forward-auth proxy in front) with access tokens enabled in the server config. That undoes the reason this fork of `penpot-mcp` diverged from upstream on auth: versions 0.1–0.3 carried an OIDC-cookie path + Authelia-basic-auth path + a password-login fallback because the instance was proxy-fronted and OIDC-only. None of that is needed anymore. - **`penpot-mcp` 0.4.0** — removes `AUTHELIA_BASIC_AUTH`, `PENPOT_AUTH_TOKEN_COOKIE`, `PENPOT_EMAIL`, `PENPOT_PASSWORD`, and the `_login()` helper. Adds `PENPOT_ACCESS_TOKEN` as the sole auth path (sent as `Authorization: Token <value>` on every RPC call). Fork now only diverges from upstream on design-token writes (v0.3); auth is upstream-compatible again. - **Service startup** (`src/main.ts`) — replaces the triple-secret load with a single `~/.config/claude-hooks/penpot-token` read. - **Docs** — `CLAUDE.md` § _Penpot MCP auth_ rewritten; fork `README.md` and `CHANGELOG.md` updated with the 0.4.0 rationale. Surfaced while running the #56 operational smoke on #62 — the OIDC cookie kept returning `authentication-required` from Penpot (session not reissuable without the Authelia proxy flow). Simpler to remove the whole stack than patch around the edges. ## Migration for operators ```bash # One-time cleanup + new secret rm -f ~/.config/claude-hooks/{authelia-creds,penpot-cookie,penpot-creds} # Create one in the Penpot UI: Profile → Access tokens echo '<token-value>' > ~/.config/claude-hooks/penpot-token chmod 600 ~/.config/claude-hooks/penpot-token # Rebuild designer/design-reviewer containers, restart service ``` ## Test plan - [x] `just qa` — 234 tests, fmt/lint clean. - [x] Upstream-facing Python tests (`test_get_design_tokens.py`) still pass in-container. `test_tokens.py` fails pre-existing on main (FastMCP library drift — separate issue). - [ ] After merge + deploy: `get_file_info` against `689d7fa4-f94b-81d4-8007-e39c5c82f66c` returns JSON (auth works). Will be checked once the access-token value is in place on the host. ## Follow-ups - Option C from the issue thread — extend the fork with `create_file` / `create_page` / `create_frame` / `create_text` / `export_frame_png` so the designer can actually satisfy #62's AC (the operational smoke's remaining blocker).
refactor(penpot-mcp): drop Authelia/cookie/password auth; access-token only
All checks were successful
qa / qa (pull_request) Successful in 2m29s
qa / dockerfile (pull_request) Successful in 9s
50ffaeac1b
Penpot is now reachable directly on the LAN (no Authelia forward-auth
proxy in front) with access tokens enabled in the server config. That
undoes the original reason this fork diverged from upstream on auth:
versions 0.1–0.3 carried an OIDC-cookie path + Authelia-basic-auth
path + a password-login fallback because the instance was
proxy-fronted and OIDC-only. None of that is needed anymore.

penpot-mcp 0.4.0:

- Remove AUTHELIA_BASIC_AUTH, PENPOT_AUTH_TOKEN_COOKIE, PENPOT_EMAIL,
  PENPOT_PASSWORD env vars + their handling in config.py.
- Remove the _login() helper and the priority-ordered auth selection
  inside PenpotAPI._get_client.
- Add PENPOT_ACCESS_TOKEN as the sole auth path; sent as
  `Authorization: Token <value>` on every RPC call. Matches upstream's
  auth contract, so the fork no longer diverges here — only the
  design-token writes (v0.3) remain as custom patches.
- Bump version to 0.4.0; CHANGELOG explains the rationale.

Service-side (src/main.ts):

- Replace the triple-secret load (authelia-creds + penpot-cookie +
  penpot-creds) with a single-file load: ~/.config/claude-hooks/penpot-token.
- Startup log changes from "Penpot MCP secrets loaded" to "Penpot
  access token loaded".

Docs: CLAUDE.md § "Penpot MCP auth" rewritten; README.md of the fork
documents the new single-env-var auth; history tables updated.

Operators upgrading: delete ~/.config/claude-hooks/{authelia-creds,
penpot-cookie,penpot-creds} and write the new access token to
~/.config/claude-hooks/penpot-token before restarting the service.
fix(penpot-mcp): force Accept: application/json on every RPC response
All checks were successful
qa / qa (pull_request) Successful in 2m29s
qa / dockerfile (pull_request) Successful in 9s
d694f2694a
Penpot's default response encoding is ``application/transit+json`` — the
Clojure-transit form ``["^ ","~:id","~uUUID","~:name",...,"~:revn",42]``.
``httpx.Response.json()`` decodes that as a Python list, not a dict, so
every call site downstream that does ``data.get("…")`` raises
``AttributeError: 'list' object has no attribute 'get'`` as soon as the
MCP actually talks to a live server. The token tools ship with unit
tests that mock at the module level above the HTTP layer, so the bug
was invisible until we verified v0.4.0 auth end-to-end against the
production instance.

Fix: ask for ``application/json`` in the session headers. Penpot honours
the ``Accept`` header and emits camelCase JSON, which httpx decodes
natively.

Side effects from the camelCase rename:

- ``files.get_file_info``: Penpot's JSON key is ``modifiedAt``; keep the
  MCP's public field name as ``modified-at`` to preserve the historical
  contract, and pull the value from ``modifiedAt`` internally.
- ``changes.get_design_tokens``: prefer ``data.tokensLib`` but fall back
  to the historical ``tokens-lib`` so a future server build that emits
  transit-shaped JSON doesn't silently return an empty dict.

Verified against the live Penpot file from #55 handoff comment:

    POST /api/rpc/command/get-file
    → {"id":"…","name":"claude-hooks — dashboard","revn":631,…}

The same bug applied to ``apply_changes`` (reads ``revn`` off the
``get_file`` response before issuing ``update-file``); the fix there is
the same shared session header.
reviewer approved these changes 2026-04-18 22:49:09 +00:00
reviewer left a comment

Review

CI: run #1581success on d694f26.

Summary

Clean, well-motivated refactor. The triple-secret auth stack (Authelia basic-auth → OIDC cookie → password login) is replaced with a single PENPOT_ACCESS_TOKEN bearer header. Every layer of the stack is updated consistently:

  • config.py — old four vars dropped, single PENPOT_ACCESS_TOKEN added.
  • api.py_get_client fails fast with a clear error if the token is unset; _login() helper removed; Accept: application/json added (needed for the camelCase key fixes in changes.py / tools/files.py).
  • changes.pytokensLibtokens-lib fallback is the correct defensive fix given the JSON key format change from transit-encoded to plain JSON.
  • tools/files.pymodifiedAt key fix is consistent with the same Accept: application/json switch.
  • src/main.ts — three-file parallel read replaced with a single penpot-token read; error message is specific and actionable.
  • All docs (CLAUDE.md, README.md, CHANGELOG.md) updated and accurate.

Nit (non-blocking)

_get_client in penpot-mcp-server/src/penpot_mcp/services/api.py is declared async but now contains no await. Callers that await self._get_client() still work correctly, so this is harmless — but it could be simplified to a plain def. Not worth a re-review cycle; fine to fix in a follow-up or leave as-is.

Acceptance criteria check (issue #56)

The relevant AC item — Penpot MCP credentials passed in as env from authelia-creds, penpot-cookie, penpot-creds — is superseded by this PR's simpler approach, which is a better outcome than the original AC anticipated. All other AC items from #56 are out of scope for this refactor PR and tracked separately.

LGTM — approved.

## Review CI: ✅ run #1581 — `success` on `d694f26`. ### Summary Clean, well-motivated refactor. The triple-secret auth stack (Authelia basic-auth → OIDC cookie → password login) is replaced with a single `PENPOT_ACCESS_TOKEN` bearer header. Every layer of the stack is updated consistently: - `config.py` — old four vars dropped, single `PENPOT_ACCESS_TOKEN` added. - `api.py` — `_get_client` fails fast with a clear error if the token is unset; `_login()` helper removed; `Accept: application/json` added (needed for the camelCase key fixes in `changes.py` / `tools/files.py`). - `changes.py` — `tokensLib` → `tokens-lib` fallback is the correct defensive fix given the JSON key format change from transit-encoded to plain JSON. - `tools/files.py` — `modifiedAt` key fix is consistent with the same `Accept: application/json` switch. - `src/main.ts` — three-file parallel read replaced with a single `penpot-token` read; error message is specific and actionable. - All docs (`CLAUDE.md`, `README.md`, `CHANGELOG.md`) updated and accurate. ### Nit (non-blocking) `_get_client` in `penpot-mcp-server/src/penpot_mcp/services/api.py` is declared `async` but now contains no `await`. Callers that `await self._get_client()` still work correctly, so this is harmless — but it could be simplified to a plain `def`. Not worth a re-review cycle; fine to fix in a follow-up or leave as-is. ### Acceptance criteria check (issue #56) The relevant AC item — Penpot MCP credentials passed in as env from `authelia-creds`, `penpot-cookie`, `penpot-creds` — is superseded by this PR's simpler approach, which is a better outcome than the original AC anticipated. All other AC items from #56 are out of scope for this refactor PR and tracked separately. LGTM — approved.
@ -43,44 +45,28 @@ class PenpotAPI:
if self._client is not None:
Collaborator

Nit: _get_client is now synchronous in behaviour — no await in the body. Declaring it def _get_client(self) instead of async def would make the intent clearer. All callers already handle both (await coroutine is a no-op when the function is made sync). Non-blocking; fine to fix in a follow-up.

Nit: `_get_client` is now synchronous in behaviour — no `await` in the body. Declaring it `def _get_client(self)` instead of `async def` would make the intent clearer. All callers already handle both (`await coroutine` is a no-op when the function is made sync). Non-blocking; fine to fix in a follow-up.
code-lead deleted branch refactor/penpot-access-token-auth 2026-04-18 22:49:41 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!68
No description provided.