Skip to content

Fix connector OAuth configuration lookup #207

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.

Sign up for GitHub

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

Open
ishaanxgupta wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix connector OAuth configuration lookup #207

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions src/api/routes/connectors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@

STATE_TTL_MINUTES = 10
MAX_PENDING_STATES = 1000
DEFAULT_PUBLIC_API_BASE_URL = "https://api.xmem.in"

CLIENT_ID_ENV_NAMES: Dict[ConnectorId, tuple[str, ...]] = {
"notion": (
"NOTION_OAUTH_CLIENT_ID",
"NOTION_CLIENT_ID",
"NOTION_INTEGRATION_CLIENT_ID",
"NOTION_PUBLIC_CLIENT_ID",
),
"google-drive": (
"GOOGLE_DRIVE_CLIENT_ID",
"GOOGLE_OAUTH_CLIENT_ID",
"GOOGLE_CLIENT_ID",
),
Comment on lines +31 to +35
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 GOOGLE_CLIENT_ID fallback too broad

GOOGLE_CLIENT_ID is a commonly-used name for Google Sign-In (user authentication) clients, which are typically scoped to openid/profile/email and have different redirect URIs than a Drive integration. If an operator has GOOGLE_CLIENT_ID set for their user-auth flow and hasn't yet set GOOGLE_DRIVE_CLIENT_ID, the Drive connector will silently pick up the wrong client, and Google will reject the authorization request with a scope or redirect_uri mismatch — with no indication from this service that the wrong credential was used.

Fix in Cursor Fix in Codex Fix in Claude Code

}
Comment on lines +24 to +36
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 KeyError if a new connector is added without updating CLIENT_ID_ENV_NAMES

_client_id and _build_authorization_url both index CLIENT_ID_ENV_NAMES[connector.id] without a guard. If ConnectorId is ever extended (or a connector is added to CONNECTORS) but this dict is not updated, a KeyError bubbles up as an unhandled 500 instead of a clean HTTPException. A CLIENT_ID_ENV_NAMES.get(connector_id, ()) or a .get with a fallback empty tuple would make the failure explicit and graceful.

Fix in Cursor Fix in Codex Fix in Claude Code



class ConnectorDefinition(BaseModel):
Expand Down Expand Up @@ -93,22 +108,30 @@ def _now() -> datetime:
return datetime.now(timezone.utc)


def _first_env(*names: str) -> Optional[str]:
for name in names:
value = os.getenv(name)
if value:
return value
return None


def _client_id(connector_id: ConnectorId) -> Optional[str]:
if connector_id == "notion":
return os.getenv("NOTION_CLIENT_ID")
return os.getenv("GOOGLE_DRIVE_CLIENT_ID") or os.getenv("GOOGLE_CLIENT_ID")
return _first_env(*CLIENT_ID_ENV_NAMES[connector_id])


def _redirect_uri(connector_id: ConnectorId) -> str:
if connector_id == "notion":
return os.getenv(
"NOTION_REDIRECT_URI",
"http://localhost:8000/api/connectors/notion/oauth/callback",
)
return os.getenv(
"GOOGLE_DRIVE_REDIRECT_URI",
"http://localhost:8000/api/connectors/google-drive/oauth/callback",
explicit_redirect_uri = _first_env(
"NOTION_REDIRECT_URI" if connector_id == "notion" else "GOOGLE_DRIVE_REDIRECT_URI"
)
if explicit_redirect_uri:
return explicit_redirect_uri

public_api_base_url = _first_env("CONNECTOR_BASE_URL", "XMEM_SERVER_URL")
base_url = (public_api_base_url or DEFAULT_PUBLIC_API_BASE_URL).rstrip("/")
if connector_id == "notion":
return f"{base_url}/api/connectors/notion/oauth/callback"
return f"{base_url}/api/connectors/google-drive/oauth/callback"
Comment on lines +124 to +134
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of _redirect_uri uses a ternary expression and conditional branches that default to Google Drive. If a new connector is added in the future, it will silently fall back to Google Drive's redirect URI and callback path, which can lead to subtle bugs.

We can make this fully generic and robust by mapping the redirect environment variables in a dictionary typed with ConnectorId (enabling static type checkers to enforce completeness) and dynamically constructing the callback URL using the connector_id.

Suggested change
explicit_redirect_uri = _first_env(
"NOTION_REDIRECT_URI" if connector_id == "notion" else "GOOGLE_DRIVE_REDIRECT_URI"
)
if explicit_redirect_uri:
return explicit_redirect_uri
public_api_base_url = _first_env("CONNECTOR_BASE_URL", "XMEM_SERVER_URL")
base_url = (public_api_base_url or DEFAULT_PUBLIC_API_BASE_URL).rstrip("/")
if connector_id == "notion":
return f"{base_url}/api/connectors/notion/oauth/callback"
return f"{base_url}/api/connectors/google-drive/oauth/callback"
redirect_env_map: Dict[ConnectorId, str] = {
"notion": "NOTION_REDIRECT_URI",
"google-drive": "GOOGLE_DRIVE_REDIRECT_URI",
}
explicit_redirect_uri = os.getenv(redirect_env_map[connector_id])
if explicit_redirect_uri:
return explicit_redirect_uri
public_api_base_url = _first_env("CONNECTOR_BASE_URL", "XMEM_SERVER_URL")
base_url = (public_api_base_url or DEFAULT_PUBLIC_API_BASE_URL).rstrip("/")
return f"{base_url}/api/connectors/{connector_id}/oauth/callback"



def _get_connector(connector_id: str) -> ConnectorDefinition:
Expand Down Expand Up @@ -148,9 +171,10 @@ def _status_for(user_id: str, connector: ConnectorDefinition) -> ConnectorStatus
def _build_authorization_url(connector: ConnectorDefinition, state: str) -> str:
client_id = _client_id(connector.id)
if not client_id:
accepted_env_names = ", ".join(CLIENT_ID_ENV_NAMES[connector.id])
raise HTTPException(
status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
detail=f"{connector.name} OAuth client ID is not configured",
detail=f"{connector.name} OAuth client ID is not configured. Set one of: {accepted_env_names}",
)

params = {
Expand Down
Loading