From 0adc66e5481d55ad41388e99ae36d47c9aa76a96 Mon Sep 17 00:00:00 2001 From: Alex Mahabir Date: Fri, 21 Aug 2026 17:06:49 -0400 Subject: [PATCH] feat: add OAuth login by instance --- README.md | 27 +++++------- docs/COMMANDS.md | 22 ++++++---- docs/SSO_OAUTH.md | 14 ++++--- glean_code/auth/manager.py | 25 +++++------ glean_code/auth_commands.py | 41 ++++++++++++++----- glean_code/commands.py | 82 +++++++++++++++++++++---------------- glean_code/completion.py | 2 + glean_code/config.py | 51 +++++++++++++++-------- glean_code/help_docs.py | 26 ++++++------ tests/test_auth.py | 52 +++++++++++++++++++++++ tests/test_commands.py | 38 ++++++++++++----- tests/test_config.py | 23 ++++++++++- 12 files changed, 274 insertions(+), 129 deletions(-) diff --git a/README.md b/README.md index 3b0f61a..b777f3a 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ A local, terminal-first client for the Glean Client REST API. Inspired by Claude - **Indexing from local files** — `/index.document --path file.md` and `/index.bulk-documents --path ./docs/` walk a file or folder and synthesize the request body for you, with `--dry-run` to inspect it first - **Natural-language planner** — type `?login into acme-be.glean.com and search for "Q2 plan"` and Glean Assistant translates it into slash commands, validated locally and gated behind a single confirm for anything destructive - **Offline by default** — a real mock corpus of interlinked documents across five faux datasources, so every command is explorable without credentials. See [docs/MOCK_CORPUS.md](docs/MOCK_CORPUS.md) -- **Browser SSO or API token** — `/auth login` runs OAuth 2.1 + PKCE against your instance, or paste a Glean-issued token. Secure refs keep real secrets in environment variables, never on disk +- **Browser SSO or API token** — `/login ` starts OAuth 2.1 + PKCE, or `/login --token ...` uses a Glean-issued token. Secure refs keep real secrets in environment variables, never on disk - **MCP server** (`glean_mcp.py`) for Claude Code, Claude Desktop, and Cursor - Terminal niceties: `/help ` for every command, tab completion that cycles matches, a powerline-style status bar, and `/scaffold` to generate stdlib-only starter projects @@ -116,27 +116,16 @@ alias glean="PYTHONPATH=/glean-code-cli python3 -m glean_code" ### First run -**Browser SSO (no API token to paste)** — opens your browser for Glean → your company IdP, then stores OAuth tokens in `~/.gleancode/auth.json`: +**Sign in with Glean OAuth** using a backend hostname or instance ID: ```text -/auth login --instance acme-be.glean.com +/login /status /search "quarterly planning" /chat "summarise the Q2 plan" ``` -Details: [docs/SSO_OAUTH.md](docs/SSO_OAUTH.md). - -**API token** — paste a Glean-issued Client API token (same live API, different auth path): - -```text -/login --instance acme-be.glean.com --token -/status -/search "quarterly planning" -/chat "summarise the Q2 plan" -``` - -Without Client API credentials (no `/auth` session and no `/login` token) the CLI runs in **mock** mode. After `/auth login` or `/login`, it switches to live calls against `https:///rest/api/v1`. +For example, `/login acme` uses the instance ID `acme`. Run `/help login` for other login options. Without login, the CLI runs in **mock** mode. ## Coming soon @@ -208,7 +197,7 @@ Three ways to authenticate, in order of preference: | Method | How | Notes | | --- | --- | --- | -| Browser SSO | `/auth login --instance ` | OAuth 2.1 + PKCE, same SSO path as the web app. Tokens in `~/.gleancode/auth.json`. See [docs/SSO_OAUTH.md](docs/SSO_OAUTH.md) | +| Browser SSO | `/login ` | OAuth 2.1 + PKCE with DCR, same SSO path as the web app. IDs map to `-be.glean.com`; tokens live in `~/.gleancode/auth.json`. See [docs/SSO_OAUTH.md](docs/SSO_OAUTH.md) | | Secure ref | `/login --token token.secure.client` | Config stores the reference name; the real secret resolves from `$GLEAN_CLIENT_TOKEN` at request time. See [docs/SECURE_TOKENS.md](docs/SECURE_TOKENS.md) | | Literal token | `/login --token ` | Written to `~/.gleancode/config.json` with `0o600` perms, masked to `***1234` everywhere it displays | @@ -220,11 +209,15 @@ Tokens are stripped from the in-memory history buffer and masked on every displa | Key | Description | Values | | --- | --- | --- | -| `instance` | Glean backend host | e.g. `acme-be.glean.com` | +| `instance` | Glean backend hostname or instance ID | e.g. `acme-be.glean.com` or `acme` | | `api_token` | Client API bearer token | Glean-issued token, or a secure ref like `token.secure.client` | | `indexing_token` | Indexing API token | Glean-issued token, or `token.secure.indexing` | | `act_as` | Impersonate a user via `X-Glean-ActAs` | Email address | | `base_url` | Override the computed base URL | Full URL | +| `oauth_client_id` | Static OAuth client ID, or a DCR-generated ID | Optional; DCR is used when unset | +| `oauth_client_instance` | Instance bound to a DCR-generated client ID | Managed automatically | +| `oauth_scopes` | Space-separated OAuth scopes | Optional; defaults to Client API scopes | +| `redirect_port` | Fixed localhost callback port | Optional | | `mode` | API mode | `auto` (default), `live`, `mock` | | `theme` | Terminal colour theme | `glean` (default), `mono`, `neon` | | `default_page_size` | Default result count for search and entities | Integer, default `10` | diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index d8d1b7e..fcaa2b4 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -67,26 +67,32 @@ Run a full health check: validates config, tests DNS resolution, TCP connectivit #### /login -Store a Glean instance host and API token. Writes to `~/.gleancode/config.json` and immediately switches the session to live mode. +Sign in to Glean with browser OAuth, or store an API token. OAuth uses Authorization Code + PKCE and Dynamic Client Registration when no static client ID is configured. ```text -/login --instance --token [--act-as ] +/login [--client-id ] [--port ] [--no-browser] [--act-as ] ``` | Parameter | Description | | --- | --- | -| `--instance` | Full Glean backend host, e.g. `acme-be.glean.com`. Include the `-be` suffix — nothing is appended automatically. | -| `--token` | A Glean Client API token with the required scopes. | +| `hostname-or-instance-id` | A backend hostname such as `acme-be.glean.com`, or an instance ID such as `acme`. IDs map to `-be.glean.com`. | +| `--instance` | Alternative named form of the hostname or instance ID. | +| `--client-id` | Optional static OAuth client ID. DCR is used when omitted. | +| `--port` | Optional fixed localhost callback port for redirect-URI allowlisting. | +| `--no-browser` | Print the OAuth authorize URL instead of opening a browser. | +| `--token` | Legacy API-token login. Supplying it skips OAuth. | | `--act-as` | Optional. Email address to impersonate via `X-Glean-ActAs`. | ```text +/login acme +/login acme-be.glean.com +/login --instance acme-be.glean.com --no-browser /login --instance acme-be.glean.com --token glean_tok_xxx -/login --instance acme-be.glean.com --token glean_tok_xxx --act-as jane@acme.com ``` -**Output** — Confirms credentials saved and shows the resolved base URL. +**Output** — OAuth opens the browser and waits for the localhost callback, then stores OAuth tokens in `~/.gleancode/auth.json`. Token login stores the API token in `~/.gleancode/config.json`. -**Endpoint** — `(local, affects Authorization header)` +**Endpoint** — `(local, OAuth Authorization Code + PKCE or Authorization header)` --- @@ -104,7 +110,7 @@ Clear stored credentials and revert to mock mode. **Output** — Confirms credentials removed. -**Mock mode** — This is what `/logout` reverts *to*: with no token, searches answer from the [mock corpus](MOCK_CORPUS.md). OAuth tokens from `/auth login` are cleared separately with `/auth logout`. +**Mock mode** — This is what `/logout` reverts *to*: with no token, searches answer from the [mock corpus](MOCK_CORPUS.md). It clears both API-token and OAuth credentials. **Endpoint** — `(local)` diff --git a/docs/SSO_OAUTH.md b/docs/SSO_OAUTH.md index 3e42cc3..d8fadab 100644 --- a/docs/SSO_OAUTH.md +++ b/docs/SSO_OAUTH.md @@ -5,17 +5,21 @@ The CLI can sign in the same way as the Glean web app: **OAuth 2.1 authorization ## Usage ```text -/auth login --instance acme-be.glean.com +/login acme /auth status /search "quarterly planning" /auth logout ``` -`/auth login` flags: +The short form accepts a Glean instance ID (`acme`) or a backend hostname +(`acme-be.glean.com`). An ID resolves to `https://-be.glean.com`. +`/auth login` remains available as the explicit OAuth command. + +`/login` OAuth flags: | Flag | Purpose | | --- | --- | -| `--instance ` | Glean backend host (saved in config) | +| `--instance ` | Glean backend hostname or instance ID (saved in config) | | `--client-id ` | Static OAuth client id (optional if the tenant supports DCR) | | `--port ` | Fixed localhost callback port for redirect URI allowlisting | | `--no-browser` | Print the authorize URL instead of opening a browser | @@ -30,14 +34,14 @@ The CLI can sign in the same way as the Glean web app: **OAuth 2.1 authorization | Location | Content | | --- | --- | -| `~/.gleancode/config.json` | `oauth_client_id`, `oauth_scopes`, `redirect_port`, optional `oauth_*_url` overrides — never the access/refresh tokens | +| `~/.gleancode/config.json` | `oauth_client_id`, DCR instance binding, `oauth_scopes`, `redirect_port`, optional `oauth_*_url` overrides — never the access/refresh tokens | | `~/.gleancode/auth.json` | OAuth tokens only (0600, written atomically) | ## Scope errors (e.g. “not allowed to request scope 'AGENT'”) OAuth clients are often restricted to a fixed set of scopes. If authorization fails with an invalid / disallowed scope: -1. **Use a smaller scope string** in config, then run `/auth login` again (you may need `/auth logout` first if a half-login left state around): +1. **Use a smaller scope string** in config, then run `/login ` again (you may need `/auth logout` first if a half-login left state around): ```text /config set oauth_scopes "SEARCH CHAT DOCUMENTS TOOLS ENTITIES offline_access" diff --git a/glean_code/auth/manager.py b/glean_code/auth/manager.py index 423505f..4af9c6b 100644 --- a/glean_code/auth/manager.py +++ b/glean_code/auth/manager.py @@ -20,6 +20,7 @@ from . import oauth from . import pkce as _pkce from . import token_store +from ..config import normalize_instance_host from .callback_server import start_callback_server # Default scopes for Client API use. `AGENT` is omitted: many OAuth clients are @@ -51,15 +52,8 @@ def _server_root_from_instance(instance: Optional[str]) -> Optional[str]: Mirrors Config.effective_base_url's host handling, but returns just the root (no /rest/api/v1), since OAuth metadata lives at the host root. """ - if not instance: - return None - host = instance.strip().rstrip("/") - if "://" in host: - host = host.split("://", 1)[1] - host = host.split("/", 1)[0] - if not host: - return None - return f"https://{host}" + host = normalize_instance_host(instance) + return f"https://{host}" if host else None def _iso(ts: Optional[float]) -> Optional[str]: @@ -91,7 +85,7 @@ def login( server_root = _server_root_from_instance(self.config.instance) if not server_root: raise AuthError( - "No instance configured. Run: /auth login --instance " + "No instance configured. Run: /login " ) scopes = getattr(self.config, "oauth_scopes", None) or DEFAULT_SCOPES @@ -198,7 +192,9 @@ def current_access_token(self, allow_refresh: bool = True) -> Optional[str]: try: endpoints = self._resolve_endpoints(server_root) client_id = getattr(self.config, "oauth_client_id", None) - if not client_id: + client_instance = getattr(self.config, "oauth_client_instance", None) + current_instance = normalize_instance_host(self.config.instance) + if not client_id or (client_instance and client_instance != current_instance): return None scopes = getattr(self.config, "oauth_scopes", None) or DEFAULT_SCOPES payload = oauth.refresh_tokens( @@ -248,8 +244,12 @@ def _resolve_endpoints(self, server_root: str) -> oauth.Endpoints: def _resolve_client_id(self, endpoints: oauth.Endpoints, redirect_uri: str, scopes: str) -> str: client_id = getattr(self.config, "oauth_client_id", None) - if client_id: + client_instance = getattr(self.config, "oauth_client_instance", None) + current_instance = normalize_instance_host(self.config.instance) + if client_id and (not client_instance or client_instance == current_instance): return client_id + if client_instance and client_instance != current_instance: + client_id = None if not endpoints.registration_endpoint: raise AuthError( "No oauth_client_id configured and the tenant did not advertise a " @@ -262,6 +262,7 @@ def _resolve_client_id(self, endpoints: oauth.Endpoints, redirect_uri: str, scop # Persist the dynamically registered client so future logins reuse it. try: self.config.oauth_client_id = client_id + self.config.oauth_client_instance = normalize_instance_host(self.config.instance) self.config.save() except Exception: pass diff --git a/glean_code/auth_commands.py b/glean_code/auth_commands.py index f139332..6ccf8f7 100644 --- a/glean_code/auth_commands.py +++ b/glean_code/auth_commands.py @@ -14,23 +14,25 @@ from . import ui from .commands import register from .auth import AuthManager, AuthError +from .config import normalize_instance_host try: # Make `/help auth` and tab-completion aware of the new command. from .help_docs import DOCS DOCS["auth"] = { "summary": "Sign in to Glean via your browser/SSO using OAuth (Authorization Code + PKCE).", - "usage": "/auth [--instance ] [--client-id ] [--port ] [--no-browser]", + "usage": "/auth [] [--instance ] [--client-id ] [--port ] [--no-browser]", "params": [ ("login", "Open the browser and sign in through Glean -> your company SSO."), ("status", "Show authentication state, instance, and token expiry."), ("logout", "Delete the locally stored OAuth tokens."), - ("--instance", "Glean backend host, e.g. acme-be.glean.com (stored in config)."), + ("--instance", "Glean backend hostname or instance ID, e.g. acme-be.glean.com or acme (stored in config)."), ("--client-id", "Static OAuth client id (optional if the tenant supports DCR)."), ("--port", "Fixed localhost callback port (for redirect-URI allowlisting)."), ("--no-browser", "Print the authorize URL instead of opening a browser."), ], "examples": [ + "/auth login acme", "/auth login --instance acme-be.glean.com", "/auth login --instance acme-be.glean.com --client-id glean-code-cli --port 33389", "/auth status", @@ -42,21 +44,33 @@ pass -def _apply_login_flags(s, flags) -> None: +def _apply_login_flags(s, flags) -> bool: """Persist any connection settings passed on the /auth login line.""" instance = flags.get("instance") + if not instance: + instance = flags.get("hostname") client_id = flags.get("client-id") or flags.get("client_id") port = flags.get("port") + act_as = flags.get("act-as") or flags.get("act_as") changed = False if instance: - raw = str(instance).strip().rstrip("/") - if "://" in raw: - raw = raw.split("://", 1)[1] - raw = raw.split("/", 1)[0] + raw = normalize_instance_host(str(instance)) + if not raw: + ui.print_err(f"That does not look like a Glean hostname or instance ID: '{instance}'") + return False + previous = normalize_instance_host(s.config.instance) + if previous != raw and getattr(s.config, "oauth_client_instance", None) == previous: + s.config.oauth_client_id = None + s.config.oauth_client_instance = None s.config.instance = raw + s.config.base_url = None + changed = True + if act_as and act_as is not True: + s.config.act_as = str(act_as) changed = True if client_id and client_id is not True: s.config.oauth_client_id = str(client_id) + s.config.oauth_client_instance = None changed = True if port and port is not True: try: @@ -66,6 +80,7 @@ def _apply_login_flags(s, flags) -> None: ui.print_err("--port must be an integer") if changed: s.config.save() + return True def _print_status(s) -> None: @@ -83,7 +98,7 @@ def _print_status(s) -> None: rows = [ ("state", ui.style("Not logged in", ui.C.YELLOW)), ("instance", status.server_url or ui.style("(unset)", ui.C.GREY)), - ("hint", "Run /auth login --instance "), + ("hint", "Run /login "), ] print(ui.kv_table(rows)) print(ui.rule()) @@ -104,10 +119,14 @@ def cmd_auth(s, pos, flags): return if sub == "login": - _apply_login_flags(s, flags) + login_flags = dict(flags) + if len(pos) > 1 and not login_flags.get("instance"): + login_flags["instance"] = pos[1] + if not _apply_login_flags(s, login_flags): + return if not s.config.instance: - ui.print_err("No instance set. Run: /auth login --instance ") - ui.print_info("Example: /auth login --instance acme-be.glean.com") + ui.print_err("No instance set. Run: /login ") + ui.print_info("Example: /login acme") return manager = AuthManager(s.config) nb = bool(flags.get("no-browser") or flags.get("no_browser")) diff --git a/glean_code/commands.py b/glean_code/commands.py index 0eccd52..bb1c857 100644 --- a/glean_code/commands.py +++ b/glean_code/commands.py @@ -21,7 +21,13 @@ from . import ui from . import _indexing_walk as _walk from .client import GleanClient, GleanError -from .config import Config, SECURE_REFS, is_secure_ref, resolve_secure +from .config import ( + Config, + SECURE_REFS, + is_secure_ref, + normalize_instance_host, + resolve_secure, +) from .help_docs import DOCS, COMMAND_GROUPS from .scaffold import TEMPLATES, write_scaffold, default_dir as _scaffold_default_dir @@ -319,42 +325,43 @@ def cmd_open(s: Session, pos, flags): @register("login") def cmd_login(s: Session, pos, flags): - instance = flags.get("instance") - token = flags.get("token") - act_as = flags.get("act-as") or flags.get("act_as") - if not instance or not token: - ui.print_err("Usage: /login --instance --token ") - ui.print_info("Enter the full host, e.g. instance_name-be.glean.com or " - "https://instance_name-be.glean.com. No auto-append.") - return - - raw = str(instance).strip().rstrip("/") - - # Accept either a full URL or a bare host. No magic suffixes. - if "://" in raw: - scheme_host = raw.split("://", 1) - scheme = scheme_host[0] - rest = scheme_host[1] - else: - scheme = "https" - rest = raw - - host_and_path = rest.split("/", 1) - host = host_and_path[0] - path = host_and_path[1] if len(host_and_path) > 1 else "" - - if not host or "." not in host: - ui.print_err(f"That does not look like a hostname: '{raw}'") - ui.print_info("Expected something like instance_name-be.glean.com") - return - - if "/rest/api/" in path: - base_url = f"{scheme}://{host}/{path}".rstrip("/") - else: - base_url = f"{scheme}://{host}/rest/api/v1" + """Log in with OAuth, or preserve the legacy API-token login syntax.""" + instance = flags.get("instance") or (pos[0] if pos else None) + token = flags.get("token") + + if token is None or token is True: + if not instance: + ui.print_err("Usage: /login [--no-browser]") + ui.print_info("Example: /login acme or /login acme-be.glean.com") + return + # Import lazily so commands remain usable in tests and in deployments + # that only need the token-based path. + from .auth_commands import cmd_auth + + oauth_flags = dict(flags) + oauth_flags["instance"] = instance + cmd_auth(s, ["login"], oauth_flags) + return + + act_as = flags.get("act-as") or flags.get("act_as") + raw = str(instance).strip() if instance else "" + host = normalize_instance_host(raw) + if not host: + ui.print_err(f"That does not look like a Glean hostname or instance ID: '{raw}'") + ui.print_info("Expected something like acme, acme-be, or acme-be.glean.com") + return + + parsed = urllib.parse.urlsplit(raw if "://" in raw else f"//{raw}") + scheme = parsed.scheme or "https" + path = parsed.path.lstrip("/") + base_url = ( + f"{scheme}://{host}/{path}".rstrip("/") + if "/rest/api/" in path + else f"{scheme}://{host}/rest/api/v1" + ) - s.config.instance = host # store the literal host - s.config.base_url = base_url + s.config.instance = host + s.config.base_url = base_url s.config.api_token = str(token) if act_as: s.config.act_as = str(act_as) @@ -368,6 +375,9 @@ def cmd_login(s: Session, pos, flags): @register("logout") def cmd_logout(s: Session, pos, flags): + from .auth import AuthManager + + AuthManager(s.config).logout() s.config.api_token = None s.config.act_as = None s.config.save() diff --git a/glean_code/completion.py b/glean_code/completion.py index cbcd192..f79f420 100644 --- a/glean_code/completion.py +++ b/glean_code/completion.py @@ -25,6 +25,8 @@ _CONFIG_KEYS = [ "instance", "api_token", "indexing_token", "act_as", "base_url", + "oauth_client_id", "oauth_client_instance", "oauth_scopes", "redirect_port", + "oauth_authorize_url", "oauth_token_url", "oauth_registration_url", "mode", "theme", "default_page_size", "mock_corpus_path", ] diff --git a/glean_code/config.py b/glean_code/config.py index 5eb9c90..374cf50 100644 --- a/glean_code/config.py +++ b/glean_code/config.py @@ -6,6 +6,8 @@ import json import os +import re +import urllib.parse from dataclasses import dataclass, asdict, field from pathlib import Path from typing import Any, Dict, Optional @@ -21,6 +23,35 @@ "token.secure.indexing": "GLEAN_INDEXING_TOKEN", } +_INSTANCE_ID_SUFFIX = "-be.glean.com" + + +def normalize_instance_host(value: Optional[str]) -> Optional[str]: + """Return a backend hostname from a hostname, URL, or Glean instance ID. + + Glean instance IDs such as ``acme`` map to ``acme-be.glean.com``. Full + hostnames and URLs are preserved as hostnames, which keeps existing custom + tenant and development configurations working. + """ + if not value: + return None + raw = str(value).strip() + if not raw: + return None + try: + parsed = urllib.parse.urlsplit(raw if "://" in raw else f"//{raw}") + except ValueError: + return None + host = parsed.hostname + if not host or any(char.isspace() for char in host): + return None + if "." not in host: + if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9-]*", host): + return None + suffix = ".glean.com" if host.endswith("-be") else _INSTANCE_ID_SUFFIX + host = f"{host}{suffix}" + return host + def is_secure_ref(value: Optional[str]) -> bool: return isinstance(value, str) and value in SECURE_REFS @@ -54,6 +85,7 @@ class Config: # it is filled in automatically on first `/auth login`. The tokens # themselves are NEVER stored here; they live in ~/.gleancode/auth.json. oauth_client_id: Optional[str] = None + oauth_client_instance: Optional[str] = None # instance bound to a dynamically registered client oauth_scopes: Optional[str] = None # space-separated; falls back to a default set redirect_port: Optional[int] = None # fixed localhost callback port (optional) oauth_authorize_url: Optional[str] = None # override; else discovered from the tenant @@ -85,32 +117,19 @@ def save(self) -> None: def effective_base_url(self) -> Optional[str]: if self.base_url: return self.base_url.rstrip("/") - if self.instance: - # instance is now stored as a full host, never a bare subdomain - host = self.instance.strip().rstrip("/") - if "://" in host: - host = host.split("://", 1)[1] - host = host.split("/", 1)[0] + if host := normalize_instance_host(self.instance): return f"https://{host}/rest/api/v1" return None @property def effective_indexing_base_url(self) -> Optional[str]: - if self.instance: - host = self.instance.strip().rstrip("/") - if "://" in host: - host = host.split("://", 1)[1] - host = host.split("/", 1)[0] + if host := normalize_instance_host(self.instance): return f"https://{host}/api/index/v1" return None @property def effective_metadata_base_url(self) -> Optional[str]: - if self.instance: - host = self.instance.strip().rstrip("/") - if "://" in host: - host = host.split("://", 1)[1] - host = host.split("/", 1)[0] + if host := normalize_instance_host(self.instance): return f"https://{host}/rest/api/index" return None diff --git a/glean_code/help_docs.py b/glean_code/help_docs.py index de59820..7210eb5 100644 --- a/glean_code/help_docs.py +++ b/glean_code/help_docs.py @@ -75,22 +75,24 @@ "endpoint": "POST /chat (Glean Assistant is used as the planner; output is parsed locally)", }, "login": { - "summary": "Store a Glean host and API token for live calls.", - "usage": "/login --instance --token [--act-as ]", + "summary": "Sign in to Glean with browser OAuth, or store an API token for live calls.", + "usage": "/login [--client-id ] [--port ] [--no-browser] [--act-as ]", "params": [ - ("--instance", "The full Glean host, e.g. instance_name-be.glean.com, " - "or a full URL like https://instance_name-be.glean.com. " - "No suffix is auto-appended. You must include -be " - "(or whatever your tenant uses) yourself."), - ("--token", "A Glean API token with Client scopes."), + ("hostname-or-instance-id", "A Glean backend hostname such as acme-be.glean.com, or an instance ID such as acme. IDs map to -be.glean.com."), + ("--instance", "Alternative named form of the hostname or instance ID."), + ("--client-id", "Optional static OAuth client ID. DCR is used when omitted."), + ("--port", "Optional fixed localhost callback port for redirect-URI allowlisting."), + ("--no-browser", "Print the OAuth authorize URL instead of opening a browser."), + ("--token", "Legacy API-token login. When provided, OAuth is not started."), ("--act-as", "Optional user email to impersonate via X-Glean-ActAs."), ], "examples": [ - "/login --instance instance_name-be.glean.com --token glean_tok_xxx", - "/login --instance https://instance_name-be.glean.com --token glean_tok_xxx", - "/login --instance instance_name-be.glean.com --token glean_tok_xxx --act-as jane@example.com", + "/login acme", + "/login acme-be.glean.com", + "/login --instance acme-be.glean.com --no-browser", + "/login --instance acme-be.glean.com --token glean_tok_xxx", ], - "endpoint": "(local, affects Authorization header)", + "endpoint": "(local, OAuth Authorization Code + PKCE or Authorization header)", }, "logout": { "summary": "Remove stored credentials. Falls back to mock mode.", @@ -104,7 +106,7 @@ "usage": "/config [get | set | list]", "params": [ ("get", "Print the value of a single key."), - ("set", "Set a key. Valid keys: instance, api_token, act_as, base_url, mode, theme, default_page_size, mock_corpus_path."), + ("set", "Set a key. Valid keys include instance, api_token, act_as, base_url, oauth_client_id, oauth_client_instance, oauth_scopes, redirect_port, mode, theme, default_page_size, mock_corpus_path."), ("list", "Print the full config."), ], "examples": [ diff --git a/tests/test_auth.py b/tests/test_auth.py index 9d64b9c..68a716e 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -97,6 +97,25 @@ def test_save_load_clear(self): self.assertIsNone(token_store.load_tokens()) +class TestDynamicClientRegistration(unittest.TestCase): + def test_registers_public_pkce_client(self): + with mock.patch.object( + oauth, "_http_post_json", return_value={"client_id": "dcr-client"} + ) as post: + client_id = oauth.register_dynamic_client( + "https://acme-be.glean.com/register", + "http://127.0.0.1:33389/callback", + "SEARCH CHAT", + ) + + self.assertEqual(client_id, "dcr-client") + payload = post.call_args.args[1] + self.assertEqual(payload["client_name"], "Glean Code CLI") + self.assertEqual(payload["redirect_uris"], ["http://127.0.0.1:33389/callback"]) + self.assertEqual(payload["token_endpoint_auth_method"], "none") + self.assertEqual(payload["grant_types"], ["authorization_code", "refresh_token"]) + + class TestAuthorizeUrl(unittest.TestCase): def test_required_params_present(self): url = oauth.build_authorize_url( @@ -160,6 +179,39 @@ def test_login_stores_tokens(self): self.assertEqual(loaded.access_token, "AT") self.assertEqual(loaded.refresh_token, "RT") + def test_login_registers_client_with_dcr_when_no_client_id_is_configured(self): + cfg = Config(instance="acme") + fixed_state = "fixed-state-dcr" + + with mock.patch.object( + oauth, + "discover_endpoints", + return_value=oauth.Endpoints( + "https://a/authorize", "https://a/token", "https://a/register" + ), + ) as discover, mock.patch( + "glean_code.auth.manager._pkce.generate_state", return_value=fixed_state + ), mock.patch.object( + oauth, "register_dynamic_client", return_value="dcr-client" + ) as register, mock.patch.object( + oauth, + "exchange_code_for_tokens", + return_value={"access_token": "AT", "expires_in": 3600}, + ), mock.patch( + "glean_code.auth.manager.start_callback_server", + side_effect=lambda port: _FakeCallback("thecode", fixed_state), + ), mock.patch.object(cfg, "save"): + status = AuthManager(cfg).login(open_browser=False) + + self.assertTrue(status.authenticated) + discover.assert_called_once_with("https://acme-be.glean.com") + register.assert_called_once_with( + "https://a/register", + "http://127.0.0.1:33389/callback", + "SEARCH CHAT DOCUMENTS TOOLS ENTITIES offline_access", + ) + self.assertEqual(cfg.oauth_client_id, "dcr-client") + def test_state_mismatch_raises(self): cfg = Config(instance="acme-be.glean.com", oauth_client_id="cid") with mock.patch.object( diff --git a/tests/test_commands.py b/tests/test_commands.py index 18e02c7..031bc0a 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -203,44 +203,60 @@ def test_sets_act_as(self): }) self.assertEqual(s.config.act_as, "user@example.com") - def test_requires_instance(self): + def test_oauth_login_accepts_positional_instance_id(self): s = _mock_session() - with patch("builtins.print") as mock_print: - HANDLERS["login"](s, [], {"token": "tok"}) - output = " ".join(str(a) for call in mock_print.call_args_list for a in call[0]) - self.assertIn("Usage", output) + with patch("glean_code.auth_commands.cmd_auth") as auth_cmd, \ + patch.object(s.config, "save"), \ + patch("builtins.print"): + HANDLERS["login"](s, ["acme"], {"no-browser": True}) + auth_cmd.assert_called_once_with( + s, ["login"], {"no-browser": True, "instance": "acme"} + ) - def test_requires_token(self): + def test_oauth_login_accepts_instance_flag_without_token(self): s = _mock_session() - with patch("builtins.print") as mock_print: + with patch("glean_code.auth_commands.cmd_auth") as auth_cmd, \ + patch.object(s.config, "save"), \ + patch("builtins.print"): HANDLERS["login"](s, [], {"instance": "acme-be.glean.com"}) + auth_cmd.assert_called_once_with( + s, ["login"], {"instance": "acme-be.glean.com"} + ) + + def test_requires_instance_for_oauth(self): + s = _mock_session() + with patch("builtins.print") as mock_print: + HANDLERS["login"](s, [], {}) output = " ".join(str(a) for call in mock_print.call_args_list for a in call[0]) self.assertIn("Usage", output) def test_rejects_invalid_hostname(self): s = _mock_session() with patch("builtins.print") as mock_print: - HANDLERS["login"](s, [], {"instance": "notahost", "token": "tok"}) + HANDLERS["login"](s, [], {"instance": "not a host", "token": "tok"}) output = " ".join(str(a) for call in mock_print.call_args_list for a in call[0]) self.assertIn("hostname", output.lower()) class TestCmdLogout(unittest.TestCase): - def test_clears_token(self): + def test_clears_token_and_oauth_credentials(self): s = _mock_session() s.config.api_token = "tok" with patch("builtins.print"), \ patch.object(s.config, "save"), \ - patch.object(s, "refresh_client"): + patch.object(s, "refresh_client"), \ + patch("glean_code.auth.token_store.clear_tokens") as clear_tokens: HANDLERS["logout"](s, [], {}) self.assertIsNone(s.config.api_token) + clear_tokens.assert_called_once_with() def test_clears_act_as(self): s = _mock_session() s.config.act_as = "user@example.com" with patch("builtins.print"), \ patch.object(s.config, "save"), \ - patch.object(s, "refresh_client"): + patch.object(s, "refresh_client"), \ + patch("glean_code.auth.token_store.clear_tokens"): HANDLERS["logout"](s, [], {}) self.assertIsNone(s.config.act_as) diff --git a/tests/test_config.py b/tests/test_config.py index 8d7e5d5..333895a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -9,7 +9,7 @@ import sys sys.path.insert(0, str(Path(__file__).parent.parent)) -from glean_code.config import Config +from glean_code.config import Config, normalize_instance_host class TestConfigDefaults(unittest.TestCase): @@ -134,6 +134,23 @@ def test_roundtrip(self): self.assertEqual(loaded.default_page_size, 5) +class TestInstanceNormalization(unittest.TestCase): + def test_instance_id_maps_to_backend_host(self): + self.assertEqual(normalize_instance_host("acme"), "acme-be.glean.com") + + def test_backend_shorthand_maps_to_glean_host(self): + self.assertEqual(normalize_instance_host("acme-be"), "acme-be.glean.com") + + def test_hostname_and_url_are_reduced_to_host(self): + self.assertEqual( + normalize_instance_host("https://acme-be.glean.com/rest/api/v1"), + "acme-be.glean.com", + ) + + def test_invalid_instance_is_rejected(self): + self.assertIsNone(normalize_instance_host("not a host")) + + class TestConfigEffectiveBaseUrl(unittest.TestCase): def test_uses_base_url_when_set(self): c = Config(base_url="https://custom.example.com/api") @@ -147,6 +164,10 @@ def test_computes_from_bare_host(self): c = Config(instance="acme-be.glean.com") self.assertEqual(c.effective_base_url, "https://acme-be.glean.com/rest/api/v1") + def test_computes_from_instance_id(self): + c = Config(instance="acme") + self.assertEqual(c.effective_base_url, "https://acme-be.glean.com/rest/api/v1") + def test_computes_from_https_url(self): c = Config(instance="https://acme-be.glean.com") self.assertEqual(c.effective_base_url, "https://acme-be.glean.com/rest/api/v1")