-
Notifications
You must be signed in to change notification settings - Fork 157
Fix/pages apikey #214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix/pages apikey #214
Changes from all commits
2092ecd
d967595
1850aae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,14 +1,33 @@ | ||||||||||||||||
| """Pages, at workspace or project scope, their hierarchy, and their links to work items. | ||||||||||||||||
|
|
||||||||||||||||
| Every page action is scoped by whether project_id is supplied: with it the page | ||||||||||||||||
| is a project page, without it a workspace page. The SDK has a separate endpoint | ||||||||||||||||
| pair for each, so the branch is explicit rather than a default. | ||||||||||||||||
| NOTE ON PLANE CE 1.4.2 | ||||||||||||||||
| ---------------------- | ||||||||||||||||
| In Plane Community Edition 1.4.2 the Pages REST API is served ONLY under the | ||||||||||||||||
| legacy ``/api/`` prefix (``/api/workspaces/<slug>/projects/<uuid>/pages/``). The | ||||||||||||||||
| ``plane`` SDK hard-codes ``/api/v1`` in ``plane.config.Configuration``, and the | ||||||||||||||||
| Pages routes are not registered under ``/api/v1/`` at all, so every SDK call to | ||||||||||||||||
| Pages 404s. | ||||||||||||||||
|
|
||||||||||||||||
| This module talks to the legacy ``/api/`` endpoint directly so it can reuse the | ||||||||||||||||
| SAME ``PLANE_API_KEY`` as the rest of the connector (the ``X-Api-Key`` header), | ||||||||||||||||
| instead of a browser session cookie. | ||||||||||||||||
|
|
||||||||||||||||
| IMPORTANT — server-side prerequisite | ||||||||||||||||
| ------------------------------------- | ||||||||||||||||
| Out of the box, the legacy ``/api/`` routes only use ``SessionAuthentication``, | ||||||||||||||||
| so an ``X-Api-Key`` request returns 401. To let the API key reach Pages, add | ||||||||||||||||
| ``APIKeyAuthentication`` to DRF's ``DEFAULT_AUTHENTICATION_CLASSES`` in | ||||||||||||||||
| ``apps/api/plane/settings/common.py`` on the Plane server (one-line change, then | ||||||||||||||||
| restart the API). With that in place this module works using only | ||||||||||||||||
| ``PLANE_API_KEY`` — no session cookie required. | ||||||||||||||||
| """ | ||||||||||||||||
|
|
||||||||||||||||
| from __future__ import annotations | ||||||||||||||||
|
|
||||||||||||||||
| import os | ||||||||||||||||
| from typing import Any, Literal | ||||||||||||||||
|
|
||||||||||||||||
| import requests | ||||||||||||||||
| from fastmcp import FastMCP | ||||||||||||||||
| from plane.models.collections import AddCollectionPages, UpdateCollectionPage | ||||||||||||||||
| from plane.models.pages import CreatePage, Page, UpdatePage | ||||||||||||||||
|
|
@@ -21,6 +40,71 @@ | |||||||||||||||
| NAME = "page" | ||||||||||||||||
| TITLE = "Pages" | ||||||||||||||||
|
|
||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||
| # Legacy /api/ client for Pages (Plane CE 1.4.2) — authenticated with PLANE_API_KEY | ||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||
| _PLANE_BASE_URL = os.getenv("PLANE_BASE_URL", "").rstrip("/") | ||||||||||||||||
| _PLANE_WORKSPACE_SLUG = os.getenv("PLANE_WORKSPACE_SLUG", "") | ||||||||||||||||
| _PLANE_API_KEY = os.getenv("PLANE_API_KEY", "") | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def _require_api_key() -> None: | ||||||||||||||||
| """Fail fast with a clear message if the API key is missing.""" | ||||||||||||||||
| from plane.errors.errors import HttpError | ||||||||||||||||
|
|
||||||||||||||||
| if not _PLANE_API_KEY: | ||||||||||||||||
| raise HttpError( | ||||||||||||||||
| "Pages require PLANE_API_KEY (X-Api-Key). The legacy /api/ Pages route must " | ||||||||||||||||
| "also accept the API key on the server side — see the module docstring.", | ||||||||||||||||
| 401, | ||||||||||||||||
| "missing PLANE_API_KEY", | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def _page_url(project_id: str, page_id: str = "", extra: str = "") -> str: | ||||||||||||||||
| """Build a legacy Pages URL. Workspace scope when project_id is empty. | ||||||||||||||||
|
|
||||||||||||||||
| A trailing slash is always appended: Django's URLs are slash-terminated and | ||||||||||||||||
| a missing slash triggers a 301 that drops the auth header on redirect. | ||||||||||||||||
| """ | ||||||||||||||||
| if project_id: | ||||||||||||||||
| url = f"{_PLANE_BASE_URL}/api/workspaces/{_PLANE_WORKSPACE_SLUG}/projects/{project_id}/pages" | ||||||||||||||||
| else: | ||||||||||||||||
| url = f"{_PLANE_BASE_URL}/api/workspaces/{_PLANE_WORKSPACE_SLUG}/pages" | ||||||||||||||||
| if page_id: | ||||||||||||||||
| url = f"{url}/{page_id}" | ||||||||||||||||
| if extra: | ||||||||||||||||
| url = f"{url}/{extra}" | ||||||||||||||||
| return url + "/" | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def _legacy_headers() -> dict[str, str]: | ||||||||||||||||
| return { | ||||||||||||||||
| "Content-Type": "application/json", | ||||||||||||||||
| "X-Api-Key": _PLANE_API_KEY, | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def _legacy_request(method: str, url: str, json: dict[str, Any] | None = None) -> Any: | ||||||||||||||||
| from plane.errors.errors import HttpError | ||||||||||||||||
|
|
||||||||||||||||
| resp = requests.request(method, url, headers=_legacy_headers(), json=json, timeout=30) | ||||||||||||||||
| if resp.status_code == 204: | ||||||||||||||||
| return None | ||||||||||||||||
| if 200 <= resp.status_code < 300: | ||||||||||||||||
| if not resp.content: | ||||||||||||||||
| return None | ||||||||||||||||
| try: | ||||||||||||||||
| return resp.json() | ||||||||||||||||
| except Exception: | ||||||||||||||||
| return resp.text | ||||||||||||||||
| try: | ||||||||||||||||
| payload = resp.json() | ||||||||||||||||
| except Exception: | ||||||||||||||||
| payload = resp.text | ||||||||||||||||
| raise HttpError(f"HTTP {resp.status_code}: {resp.reason}", resp.status_code, payload) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| ACTIONS = ( | ||||||||||||||||
| Action( | ||||||||||||||||
| "list", (), ("project_id", "cursor", "per_page"), note="workspace pages unless project_id is given", read=True | ||||||||||||||||
|
|
@@ -138,77 +222,83 @@ def page( | |||||||||||||||
| ) -> Page | WorkItemPage | list[WorkItemPage] | dict[str, Any] | str | None: | ||||||||||||||||
| client, workspace_slug = get_plane_client_context() | ||||||||||||||||
|
|
||||||||||||||||
| # ----- Core Page CRUD: legacy /api/ + PLANE_API_KEY (X-Api-Key) ----- | ||||||||||||||||
| _require_api_key() | ||||||||||||||||
|
Comment on lines
+225
to
+226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Do not require
Move the guard into the legacy branches. 🐛 Proposed fix- # ----- Core Page CRUD: legacy /api/ + PLANE_API_KEY (X-Api-Key) -----
- _require_api_key()
- if action == "list":
+ # ----- Core Page CRUD: legacy /api/ + PLANE_API_KEY (X-Api-Key) -----
+ if action in ("list", "retrieve", "create", "update", "archive", "delete"):
+ _require_api_key()
+
+ if action == "list":📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| if action == "list": | ||||||||||||||||
| params = as_params(PaginatedQueryParams, cursor=cursor, per_page=per_page) | ||||||||||||||||
| if project_id: | ||||||||||||||||
| response = client.pages.list_project_pages( | ||||||||||||||||
| workspace_slug=workspace_slug, project_id=project_id, params=params | ||||||||||||||||
| ) | ||||||||||||||||
| else: | ||||||||||||||||
| response = client.pages.list_workspace_pages(workspace_slug=workspace_slug, params=params) | ||||||||||||||||
| return envelope(response) | ||||||||||||||||
| return _legacy_request("GET", _page_url(project_id)) | ||||||||||||||||
| return _legacy_request("GET", _page_url("")) | ||||||||||||||||
|
Comment on lines
227
to
+230
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Preserve list pagination and use the method supported by the legacy update endpoint. The 📍 Affects 1 file
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||
|
|
||||||||||||||||
| if action == "retrieve": | ||||||||||||||||
| if not page_id: | ||||||||||||||||
| return missing(action, "page_id") | ||||||||||||||||
| if project_id: | ||||||||||||||||
| return client.pages.retrieve_project_page( | ||||||||||||||||
| workspace_slug=workspace_slug, project_id=project_id, page_id=page_id | ||||||||||||||||
| ) | ||||||||||||||||
| return client.pages.retrieve_workspace_page(workspace_slug=workspace_slug, page_id=page_id) | ||||||||||||||||
| return _legacy_request("GET", _page_url(project_id, page_id)) | ||||||||||||||||
| return _legacy_request("GET", _page_url("", page_id)) | ||||||||||||||||
|
|
||||||||||||||||
| if action == "create": | ||||||||||||||||
| if error := needs(action, name=name, description_html=description_html): | ||||||||||||||||
| return error | ||||||||||||||||
| if parent_id and collection_id: | ||||||||||||||||
| return "Error: pass parent_id or collection_id, not both. A nested page takes its parent's collection." | ||||||||||||||||
| if collection_id and project_id: | ||||||||||||||||
| return "Error: collections hold workspace pages only. Omit project_id, or omit collection_id." | ||||||||||||||||
| data: dict[str, Any] = {"name": name, "description_html": description_html} | ||||||||||||||||
| if access is not None: | ||||||||||||||||
| data["access"] = access | ||||||||||||||||
| if color: | ||||||||||||||||
| data["color"] = color | ||||||||||||||||
| if is_locked is not None: | ||||||||||||||||
| data["is_locked"] = is_locked | ||||||||||||||||
| if parent_id: | ||||||||||||||||
| data["parent_id"] = parent_id | ||||||||||||||||
| if external_id: | ||||||||||||||||
| data["external_id"] = external_id | ||||||||||||||||
| if external_source: | ||||||||||||||||
| data["external_source"] = external_source | ||||||||||||||||
| if collection_id: | ||||||||||||||||
| data["collection_id"] = collection_id | ||||||||||||||||
| if project_id: | ||||||||||||||||
| return _legacy_request("POST", _page_url(project_id), json=data) | ||||||||||||||||
| return _legacy_request("POST", _page_url(""), json=data) | ||||||||||||||||
|
|
||||||||||||||||
| if action == "update": | ||||||||||||||||
| if not page_id: | ||||||||||||||||
| return missing(action, "page_id") | ||||||||||||||||
| if not (name or description_html): | ||||||||||||||||
| return missing(action, "name or description_html") | ||||||||||||||||
| data = {} | ||||||||||||||||
| if name: | ||||||||||||||||
| data["name"] = name | ||||||||||||||||
| if description_html: | ||||||||||||||||
| data["description_html"] = description_html | ||||||||||||||||
| if project_id: | ||||||||||||||||
| return _legacy_request("PUT", _page_url(project_id, page_id), json=data) | ||||||||||||||||
| return _legacy_request("PUT", _page_url("", page_id), json=data) | ||||||||||||||||
|
|
||||||||||||||||
| if action == "archive": | ||||||||||||||||
| if not page_id: | ||||||||||||||||
| return missing(action, "page_id") | ||||||||||||||||
| if project_id: | ||||||||||||||||
| mover = client.pages.archive_project_page if archive else client.pages.unarchive_project_page | ||||||||||||||||
| mover(workspace_slug=workspace_slug, project_id=project_id, page_id=page_id) | ||||||||||||||||
| url = _page_url(project_id, page_id, "archive") | ||||||||||||||||
| else: | ||||||||||||||||
| mover = client.pages.archive_workspace_page if archive else client.pages.unarchive_workspace_page | ||||||||||||||||
| mover(workspace_slug=workspace_slug, page_id=page_id) | ||||||||||||||||
| # Plane answers nothing, and delete depends on this having happened. | ||||||||||||||||
| url = _page_url("", page_id, "archive") | ||||||||||||||||
| if archive: | ||||||||||||||||
| _legacy_request("POST", url) | ||||||||||||||||
| else: | ||||||||||||||||
| _legacy_request("DELETE", url) | ||||||||||||||||
| return {"page_id": page_id, "archived": archive} | ||||||||||||||||
|
|
||||||||||||||||
| if action in ("update", "delete"): | ||||||||||||||||
| if action == "delete": | ||||||||||||||||
| if not page_id: | ||||||||||||||||
| return missing(action, "page_id") | ||||||||||||||||
| scope = {"project_id": project_id} if project_id else {} | ||||||||||||||||
| if action == "delete": | ||||||||||||||||
| deleter = client.pages.delete_project_page if project_id else client.pages.delete_workspace_page | ||||||||||||||||
| deleter(workspace_slug=workspace_slug, page_id=page_id, **scope) | ||||||||||||||||
| return None | ||||||||||||||||
| if not (name or description_html): | ||||||||||||||||
| return missing(action, "name or description_html") | ||||||||||||||||
| updater = client.pages.update_project_page if project_id else client.pages.update_workspace_page | ||||||||||||||||
| return updater( | ||||||||||||||||
| workspace_slug=workspace_slug, | ||||||||||||||||
| page_id=page_id, | ||||||||||||||||
| **scope, | ||||||||||||||||
| data=UpdatePage(name=opt(name), description_html=opt(description_html)), | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| if action == "create": | ||||||||||||||||
| if error := needs(action, name=name, description_html=description_html): | ||||||||||||||||
| return error | ||||||||||||||||
| if parent_id and collection_id: | ||||||||||||||||
| return "Error: pass parent_id or collection_id, not both. A nested page takes its parent's collection." | ||||||||||||||||
| if collection_id and project_id: | ||||||||||||||||
| return "Error: collections hold workspace pages only. Omit project_id, or omit collection_id." | ||||||||||||||||
| data = CreatePage( | ||||||||||||||||
| name=name, | ||||||||||||||||
| description_html=description_html, | ||||||||||||||||
| access=access, | ||||||||||||||||
| color=opt(color), | ||||||||||||||||
| is_locked=is_locked, | ||||||||||||||||
| parent_id=opt(parent_id), | ||||||||||||||||
| collection_id=opt(collection_id), | ||||||||||||||||
| external_id=opt(external_id), | ||||||||||||||||
| external_source=opt(external_source), | ||||||||||||||||
| ) | ||||||||||||||||
| if project_id: | ||||||||||||||||
| return client.pages.create_project_page(workspace_slug=workspace_slug, project_id=project_id, data=data) | ||||||||||||||||
| return client.pages.create_workspace_page(workspace_slug=workspace_slug, data=data) | ||||||||||||||||
| _legacy_request("DELETE", _page_url(project_id, page_id)) | ||||||||||||||||
| else: | ||||||||||||||||
| _legacy_request("DELETE", _page_url("", page_id)) | ||||||||||||||||
| return None | ||||||||||||||||
|
|
||||||||||||||||
| # ----- Set collection / work-item links: still via SDK (/api/v1/) ----- | ||||||||||||||||
| if action == "set_collection": | ||||||||||||||||
| if error := needs(action, page_id=page_id, collection_id=collection_id): | ||||||||||||||||
| return error | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Resolve the Plane base URL, workspace slug, and API key the same way the shared client does.
get_plane_client_contextinplane_mcp/client.pyreadsPLANE_INTERNAL_BASE_URLfirst, falls back toPLANE_BASE_URL, and defaults tohttps://api.plane.so. It also takes the workspace slug and API key from OAuth or header claims when they are present. This module reads onlyPLANE_BASE_URL,PLANE_WORKSPACE_SLUG, andPLANE_API_KEYat import time.Consequences:
PLANE_INTERNAL_BASE_URLproduces_PLANE_BASE_URL == ""._page_urlthen returns/api/workspaces//pages/, andrequests.requestraisesrequests.exceptions.MissingSchemainstead of a Plane error.x-api-keyandx-workspace-slugheaders or with OAuth has noPLANE_API_KEYorPLANE_WORKSPACE_SLUGin the environment, so every Pages call fails.Read the values per request and reuse the resolved workspace slug returned by
get_plane_client_context.♻️ Proposed direction
_page_urlthen takes the workspace slug as a parameter, supplied by the caller fromget_plane_client_context().🤖 Prompt for AI Agents