fix(mcp): explain the LinkedIn callback URL instead of "Missing code or state"
Watched a real operator copy the callback URL — as the MCP page tells them to — and then open it in a browser tab, landing here with no query string and getting "Missing code or state." Accurate, and completely useless to the person reading it. Only LinkedIn should ever call this URL, carrying ?code=...&state=... after a real login, so a bare visit is a strong signal that somebody misread the copy step. The response is the last place left to tell them, so it now says what the URL is for and where it actually belongs (Developer Portal -> app -> Auth -> Authorized redirect URLs), and that Authenticate LinkedIn is the button that starts a real login. LinkedIn's own errors are untouched: a genuine refusal still shows LinkedIn's wording, not this guidance, since that is a different failure and the operator needs LinkedIn's own words. Pinned by tests, including one asserting the old bare symptom message cannot come back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1113,8 +1113,23 @@ async def linkedin_oauth_callback(
|
||||
status_code=400,
|
||||
)
|
||||
if not code or not state:
|
||||
# Reaching this with no code and no state almost always means a
|
||||
# person pasted this URL into their browser by hand, having been
|
||||
# told to "copy the callback URL" — it is LinkedIn that is supposed
|
||||
# to call it, carrying ?code=...&state=..., after a real login.
|
||||
# "Missing code or state" is true but tells that person nothing, so
|
||||
# name what actually happened and what to do instead.
|
||||
return HTMLResponse(
|
||||
"<h2>LinkedIn OAuth failed</h2><p>Missing code or state.</p>",
|
||||
"<h2>Nothing to complete here</h2>"
|
||||
"<p>This page is not meant to be opened directly. It is the "
|
||||
"address LinkedIn sends you back to <em>after</em> you log in, "
|
||||
"so opening it yourself arrives with no login attached.</p>"
|
||||
"<p><strong>If you were copying this URL:</strong> it belongs in "
|
||||
"the LinkedIn Developer Portal, under your app → Auth tab "
|
||||
"→ Authorized redirect URLs. Paste it into that field, save, "
|
||||
"then return to MaskanX and click <strong>Authenticate "
|
||||
"LinkedIn</strong> — that button opens the real LinkedIn "
|
||||
"login.</p>",
|
||||
status_code=400,
|
||||
)
|
||||
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
"""The LinkedIn OAuth callback endpoint's guidance when opened by a person.
|
||||
|
||||
Real incident: the Agent -> MCP page tells the operator to copy the
|
||||
callback URL so it can be registered in the LinkedIn Developer Portal.
|
||||
Someone copied it and opened it in a browser tab instead, landing on this
|
||||
endpoint with no query string at all, and got "Missing code or state." —
|
||||
accurate, and useless to the person reading it.
|
||||
|
||||
Only LinkedIn is supposed to call this URL, carrying ?code=...&state=...
|
||||
after a real login. A bare visit is therefore a strong signal that
|
||||
somebody misunderstood the copy step, and the response is the only place
|
||||
left to tell them so.
|
||||
"""
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from adclaw.app.routers import mcp as mcp_router
|
||||
|
||||
|
||||
def _client() -> TestClient:
|
||||
app = FastAPI()
|
||||
app.include_router(mcp_router.router, prefix="/api")
|
||||
return TestClient(app)
|
||||
|
||||
|
||||
def test_opening_the_callback_directly_explains_what_it_is_for():
|
||||
response = _client().get("/api/mcp/linkedin/oauth/callback")
|
||||
|
||||
assert response.status_code == 400
|
||||
body = response.text
|
||||
# Names what the URL is actually for, rather than the raw symptom.
|
||||
assert "not meant to be opened directly" in body
|
||||
assert "Authorized redirect URLs" in body
|
||||
assert "Authenticate" in body
|
||||
|
||||
|
||||
def test_the_old_bare_symptom_message_is_gone():
|
||||
""""Missing code or state" told the operator nothing they could act
|
||||
on. If it ever comes back, this endpoint has lost the only guidance a
|
||||
misdirected person gets."""
|
||||
body = _client().get("/api/mcp/linkedin/oauth/callback").text
|
||||
|
||||
assert "Missing code or state" not in body
|
||||
|
||||
|
||||
def test_a_real_linkedin_error_is_still_shown_verbatim():
|
||||
"""LinkedIn's own refusal (user denied, app misconfigured) must not be
|
||||
replaced by the copy-step guidance — that is a different failure and
|
||||
the operator needs LinkedIn's own words."""
|
||||
response = _client().get(
|
||||
"/api/mcp/linkedin/oauth/callback",
|
||||
params={
|
||||
"error": "user_cancelled_login",
|
||||
"error_description": "The user cancelled the login.",
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert "The user cancelled the login." in response.text
|
||||
assert "not meant to be opened directly" not in response.text
|
||||
|
||||
|
||||
def test_a_code_without_state_is_not_treated_as_a_stray_visit():
|
||||
"""Half a callback is a real OAuth problem, not someone pasting a URL,
|
||||
but it lands in the same branch — the guidance is still the most
|
||||
useful thing to say, so this only pins that it does not crash."""
|
||||
response = _client().get(
|
||||
"/api/mcp/linkedin/oauth/callback",
|
||||
params={"code": "abc123"},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
Reference in New Issue
Block a user