Fix connector OAuth configuration lookup

[ishaanxgupta]

Summary

• allow Notion connector OAuth to read common client ID env aliases
• allow Google Drive connector OAuth to read common client ID env aliases
• default connector callback URLs to the public API base instead of localhost while preserving explicit redirect overrides
• include accepted env var names in missing-client-ID errors without exposing values

Validation

• python -m py_compile src/api/routes/connectors.py

Production note: Notion OAuth still requires a configured Notion OAuth app/client ID in the environment and a matching redirect URI registered in Notion.

[github-actions]

	Warnings
⚠️	🧪 Source files in src/ were modified but no test files changed. Please add or update tests to cover your changes.

	Messages
📖	📝 No CHANGELOG.md update detected. If this PR introduces a user-visible change, please add an entry.
📖	✅ Targeting main. Please squash commits before merging to keep the git history clean.

Generated by 🚫 dangerJS against 251c0ae

[gemini-code-assist]

Code Review

This pull request refactors how connector client IDs and redirect URIs are resolved. It introduces a helper function to retrieve the first available environment variable from a list, maps environment variables for each connector type, and updates redirect URI construction to support a dynamic base URL. The review feedback suggests refactoring _redirect_uri to use a dictionary mapping and dynamic path construction to prevent silent fallbacks to Google Drive if new connectors are added in the future.

[gemini-code-assist]

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"

[github-actions]

✅ Staging Deployment Report

Item	Value
Branch	fix-notion-oauth-config
Commit	90088b1
Environment	Staging
Health	http://3.6.255.148:8001/health
API Docs	http://3.6.255.148:8001/docs
Smoke Tests	success

🟢 Staging is live and healthy! Test your changes at the staging URL above.

Ready to ship? Comment /promote on this PR to merge to main and deploy to production.

[greptile-apps]

Greptile Summary

This PR improves OAuth connector configuration by accepting multiple common env var aliases for client IDs (e.g. NOTION_OAUTH_CLIENT_ID alongside NOTION_CLIENT_ID), switching redirect URI defaults from localhost:8000 to the public API base (https://api.xmem.in), and adding a helpful error message that names the accepted env vars when a client ID is missing.

• _first_env helper walks a priority-ordered list of env var names and returns the first non-empty value; used for both client ID and redirect URI lookups.
• Redirect URI now falls back to CONNECTOR_BASE_URL → XMEM_SERVER_URL → DEFAULT_PUBLIC_API_BASE_URL, preserving any explicit NOTION_REDIRECT_URI / GOOGLE_DRIVE_REDIRECT_URI override.
• The 503 error message now includes the accepted env var names to aid operators in diagnosing missing configuration, without exposing actual values.

Confidence Score: 4/5

The change is safe for production deployments that set connector-specific env vars; operators relying on the generic GOOGLE_CLIENT_ID fallback may hit silent OAuth failures if that credential belongs to a different Google service.

The redirect URI default change fixes a real production gap, and the multi-alias client ID resolution is a clear improvement. The main concern is that GOOGLE_CLIENT_ID is a common name for Google Sign-In clients and could silently resolve to the wrong credential if GOOGLE_DRIVE_CLIENT_ID is not explicitly set. Additionally, direct dict indexing on CLIENT_ID_ENV_NAMES without a fallback will raise an unhandled KeyError if a new connector is added to CONNECTORS without a corresponding entry in the map.

src/api/routes/connectors.py — specifically the GOOGLE_CLIENT_ID fallback entry and the CLIENT_ID_ENV_NAMES dict indexing in both _client_id and _build_authorization_url.

Important Files Changed

Filename	Overview
src/api/routes/connectors.py	Adds multi-alias env var resolution for OAuth client IDs, switches redirect URI default from localhost to the public API base URL, and improves the missing-config error message. Two minor robustness concerns: GOOGLE_CLIENT_ID is too generic (can collide with Sign-In clients), and direct dict indexing into CLIENT_ID_ENV_NAMES will KeyError for any future connector not in the map.

Sequence Diagram

sequenceDiagram
    participant Client
    participant API as FastAPI (connectors.py)
    participant Env as Environment Variables
    participant OAuthProvider as OAuth Provider (Notion/Google)

    Client->>API: "POST /{connector_id}/oauth/start"
    API->>API: _get_connector(connector_id)
    API->>Env: "_first_env(*CLIENT_ID_ENV_NAMES[connector_id])"
    alt client_id found
        Env-->>API: client_id value
        API->>Env: _first_env(NOTION/GOOGLE_DRIVE_REDIRECT_URI)
        alt explicit redirect URI set
            Env-->>API: explicit redirect URI
        else no explicit override
            API->>Env: _first_env(CONNECTOR_BASE_URL, XMEM_SERVER_URL)
            alt base URL env var set
                Env-->>API: custom base URL
            else fallback
                API->>API: use DEFAULT_PUBLIC_API_BASE_URL
            end
            API->>API: build redirect URI from base + path
        end
        API->>API: build authorization URL with params + state
        API-->>Client: ConnectorStartResponse (authorization_url)
        Client->>OAuthProvider: redirect to authorization_url
        OAuthProvider-->>API: "GET /{connector_id}/oauth/callback?code=...&state=..."
        API->>API: validate state token
        API-->>Client: pending status
    else client_id missing
        API-->>Client: 503 Set one of: ENV_NAME_1, ENV_NAME_2, ...
    end

Loading

_{Reviews (1): Last reviewed commit: "Fix connector OAuth configuration lookup" | Re-trigger Greptile}

[greptile-apps]

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.

[greptile-apps]

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.