Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 19 additions & 10 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ smorg is a keyboard-driven terminal dashboard: each connected integration is a
tab, nothing is enabled by default, and the app is read-plus-safe-actions — it
shows what's on your plate and opens things, it never writes to a service.

This document explains the load-bearing decisions. How to *add* an integration
This document explains the load-bearing decisions. How to _add_ an integration
is covered in [CONTRIBUTING.md](../CONTRIBUTING.md).

## Three layers, two seams
Expand Down Expand Up @@ -65,15 +65,14 @@ easiest way to misread this design:
The current integrations occupy diagonal corners, which makes the two axes
look like one:

| | MCP transport | REST transport |
| ---------------- | ------------- | -------------- |
| **OAuth** | Linear | — |
| **Pasted token** | — | GitHub |
| | MCP transport | REST transport |
| ---------------- | ------------- | ------------------------ |
| **OAuth** | Linear | Spotify, Google Calendar |
| **Pasted token** | — | GitHub |

The empty corners are circumstance, not design. OAuth + REST is where any
classic OAuth provider with no token alternative lands (Spotify is the
roadmap's first candidate); token + MCP would be an MCP server reached with a
static bearer token.
The empty corner is circumstance, not design. OAuth + REST is where a classic
OAuth provider with no token alternative lands; token + MCP would be an MCP
server reached with a static bearer token.

### The auth axis: OAuth where it is cheap, a pasted token where it is not

Expand All @@ -90,6 +89,16 @@ carries the endpoints, and the user creates the OAuth app themselves and
pastes its client id. That per-user setup step is worth it only when the
provider issues no token a user could paste instead.

A provider that offers neither discovery nor a pasteable token, and whose own
console makes per-user app creation a chore, is declared as a
`BundledProvider`: smorg's maintainer registers the app once and the client id
and secret ship with the build, the pattern every desktop calendar client uses.
The secret is not treated as confidential, since an installed app cannot keep
one, and it never reaches output. Config records no client id for a bundled
tab; the id belongs to the build. Google Calendar is the first bundled
provider: Google rejects a secretless code exchange for a desktop client,
issues refresh tokens without extra parameters, and accepts any loopback port.

GitHub publishes no metadata document and registers no clients, so an OAuth
tab there would need an app somebody registered by hand and a client id
configured before the first login — a setup step per user, to arrive where one
Expand Down Expand Up @@ -119,7 +128,7 @@ own source module, and why the allowlist is a feature: when a server changes
its output, exactly one source breaks, its tab shows the failure, and every
other tab keeps working.

Nothing versions MCP tool output, so every source treats response *shape* as
Nothing versions MCP tool output, so every source treats response _shape_ as
untrusted alongside content: a field that should be an object may be a string,
and that must degrade one tab (`Malformed`), never crash the app. The protocol
revision we speak, and the upgrade path, live in [mcp-protocol.md](mcp-protocol.md).
Expand Down
11 changes: 10 additions & 1 deletion src/smorg/auth/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,14 @@ def perform_login(
try:
redirect_uri = f"http://127.0.0.1:{server.server_port}/callback"
metadata = oauth.resolve_metadata(client, method)
client_id = oauth.client_id_for(method, client_id)
if client_id is None:
provider = method.provider
if isinstance(provider, oauth.StaticProvider):
raise oauth.OAuthError(
"this provider cannot register clients; connect with a client id"
)
assert isinstance(provider, oauth.DiscoveredProvider)
client_id = oauth.register_client(
client, metadata, provider, oauth.REGISTERED_REDIRECT_URI
)
Expand Down Expand Up @@ -135,8 +137,15 @@ def perform_login(
if "error" in received:
raise oauth.OAuthError(f"authorization was refused: {sanitize_line(received['error'])}")

client_secret = oauth.client_secret_of(method)
credentials = oauth.exchange_code(
client, metadata, client_id, received["code"], verifier, redirect_uri
client,
metadata,
client_id,
received["code"],
verifier,
redirect_uri,
client_secret=client_secret,
)
return client_id, credentials
finally:
Expand Down
43 changes: 39 additions & 4 deletions src/smorg/auth/oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@
__all__ = [
"REGISTERED_REDIRECT_URI",
"REGISTRATION_PORT",
"BundledProvider",
"DiscoveredProvider",
"OAuthError",
"OAuthMethod",
"ServerMetadata",
"StaticProvider",
"build_authorize_url",
"callback_port",
"client_id_for",
"client_secret_of",
"discover",
"exchange_code",
"extra_scopes_warning",
Expand Down Expand Up @@ -73,18 +76,41 @@ class StaticProvider:
setup_hint: str


@dataclass(frozen=True)
class BundledProvider:
"""smorg registered the OAuth app itself; the client id and secret ship with the build."""

metadata: ServerMetadata
client_id: str
client_secret: str


@dataclass(frozen=True)
class OAuthMethod:
"""Authorize in the browser against provider, requesting scopes."""

provider: DiscoveredProvider | StaticProvider
provider: DiscoveredProvider | StaticProvider | BundledProvider
scopes: tuple[str, ...]


class OAuthError(Exception):
"""A registration, token, or discovery request failed. Never carries a token."""


def client_secret_of(method: OAuthMethod) -> str | None:
if isinstance(method.provider, BundledProvider):
return method.provider.client_secret
return None


def client_id_for(method: OAuthMethod, recorded: str | None) -> str | None:
"""The client id a login, refresh, or revocation uses: the build's own for a bundled provider,
otherwise whatever config recorded."""
if isinstance(method.provider, BundledProvider):
return method.provider.client_id
return recorded


def _json_object(response: httpx.Response, source: str) -> _JsonObject:
"""Decode a response body that must be a JSON object."""
try:
Expand Down Expand Up @@ -130,7 +156,7 @@ def discover(client: httpx.Client, provider: DiscoveredProvider) -> ServerMetada


def resolve_metadata(client: httpx.Client, method: OAuthMethod) -> ServerMetadata:
if isinstance(method.provider, StaticProvider):
if isinstance(method.provider, StaticProvider | BundledProvider):
return method.provider.metadata
return discover(client, method.provider)

Expand Down Expand Up @@ -166,7 +192,7 @@ def register_client(

def callback_port(method: OAuthMethod) -> int:
# A hand-registered app pins its redirect URI, so a static provider binds the registered
# port exactly; discovered providers accept any loopback port (RFC 8252 §7.3).
# port exactly; discovered and bundled providers accept any loopback port (RFC 8252 §7.3).
if isinstance(method.provider, StaticProvider):
return REGISTRATION_PORT
return 0
Expand Down Expand Up @@ -240,8 +266,13 @@ def _credentials_from_token_response(


def _post_token(
client: httpx.Client, metadata: ServerMetadata, form: dict[str, str]
client: httpx.Client,
metadata: ServerMetadata,
form: dict[str, str],
client_secret: str | None = None,
) -> _JsonObject:
if client_secret is not None:
form = form | {"client_secret": client_secret}
# Binds the issued token to the protected resource; omit it and the token
# carries the wrong audience — rejected later at the API, not here.
if metadata.resource:
Expand All @@ -266,6 +297,7 @@ def exchange_code(
code: str,
verifier: str,
redirect_uri: str,
client_secret: str | None = None,
) -> Credentials:
payload = _post_token(
client,
Expand All @@ -277,6 +309,7 @@ def exchange_code(
"client_id": client_id,
"code_verifier": verifier,
},
client_secret,
)
return _credentials_from_token_response(payload, fallback_refresh=None)

Expand All @@ -286,6 +319,7 @@ def refresh_credentials(
metadata: ServerMetadata,
client_id: str,
credentials: Credentials,
client_secret: str | None = None,
) -> Credentials:
if credentials.refresh_token is None:
raise OAuthError("no refresh token available; re-run smorg connect")
Expand All @@ -297,6 +331,7 @@ def refresh_credentials(
"refresh_token": credentials.refresh_token,
"client_id": client_id,
},
client_secret,
)
return _credentials_from_token_response(payload, fallback_refresh=credentials.refresh_token)

Expand Down
6 changes: 5 additions & 1 deletion src/smorg/auth/refresh.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ def fresh_credentials(
credentials = get_credentials(integration_id)
if credentials is None or not _expiring(credentials):
return credentials
client_id = oauth.client_id_for(method, client_id)
if credentials.refresh_token is None or client_id is None:
return credentials
lock = _lock_for(integration_id)
Expand All @@ -75,7 +76,10 @@ def fresh_credentials(
return credentials
try:
metadata = oauth.resolve_metadata(http, method)
refreshed = oauth.refresh_credentials(http, metadata, client_id, credentials)
client_secret = oauth.client_secret_of(method)
refreshed = oauth.refresh_credentials(
http, metadata, client_id, credentials, client_secret=client_secret
)
except OAuthError as error:
raise AuthExpired(f"token refresh failed ({error})") from error
# A store failure here propagates as CredentialStoreError on purpose:
Expand Down
12 changes: 11 additions & 1 deletion src/smorg/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@ def on_authorize_url(url: str) -> None:
)


def _tab_config_for(integration_id: str, path: AuthPath, client_id: str) -> TabConfig:
"""The config entry a connect records; a bundled client's id belongs to the build, not the
config."""
if isinstance(path.method, oauth.OAuthMethod) and isinstance(
path.method.provider, oauth.BundledProvider
):
return TabConfig(integration=integration_id, connection=path.id)
return TabConfig(integration=integration_id, client_id=client_id, connection=path.id)


def _connect(integration_id: str) -> int:
try:
integration = get_integration(integration_id)
Expand Down Expand Up @@ -106,7 +116,7 @@ def _connect(integration_id: str) -> int:
print(str(error), file=sys.stderr)
return 1

tab_config = TabConfig(integration=integration_id, client_id=client_id, connection=path.id)
tab_config = _tab_config_for(integration_id, path, client_id)
save_config(add_tab(config, tab_config))
print(f"connected {integration.manifest.display_name} (scope: {credentials.scope})")
return 0
Expand Down
7 changes: 4 additions & 3 deletions src/smorg/core/removal.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,16 @@ def remove_integration(integration_id: str) -> RemovalResult:
get_integration(integration_id)

revoked = False
if integration is not None and credentials is not None and tab is not None and tab.client_id:
if integration is not None and credentials is not None and tab is not None:
try:
path = integration.manifest.connection(tab.connection)
except ValueError:
path = None # a stale connection id must not block deletion

# Only try to revoke OAuth tokens: a pasted token has no provider to ask
if path is not None and isinstance(path.method, oauth.OAuthMethod):
revoked = revoke_best_effort(path.method, tab.client_id, credentials)
client_id = oauth.client_id_for(path.method, tab.client_id)
if client_id is not None:
revoked = revoke_best_effort(path.method, client_id, credentials)

# Credentials before config: dropping the tab first could strand credentials with nothing left
# pointing at them.
Expand Down
17 changes: 14 additions & 3 deletions src/smorg/shell/menu/connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from smorg.auth.login import LoginCancelled, perform_login
from smorg.auth.oauth import (
REGISTERED_REDIRECT_URI,
BundledProvider,
OAuthError,
OAuthMethod,
StaticProvider,
Expand Down Expand Up @@ -140,6 +141,13 @@ def connect_screen_for(integration: AddableIntegration, path: AuthPath) -> Manag
return TokenModal(integration.integration_id, integration.display_name, path)
if isinstance(path.method.provider, StaticProvider):
return ClientIdModal(integration.integration_id, integration.display_name, path)
if isinstance(path.method.provider, BundledProvider):
return ConnectModal(
integration.integration_id,
integration.display_name,
path,
client_id=path.method.provider.client_id,
)
return ConnectModal(integration.integration_id, integration.display_name, path)


Expand Down Expand Up @@ -376,9 +384,12 @@ def revoke_token() -> None:
# nothing could revoke it later.
revoke_best_effort(self.method, client_id, credentials)

tab_config = TabConfig(
integration=self.integration_id, client_id=client_id, connection=self.path.id
)
if isinstance(self.method.provider, BundledProvider):
tab_config = TabConfig(integration=self.integration_id, connection=self.path.id)
else:
tab_config = TabConfig(
integration=self.integration_id, client_id=client_id, connection=self.path.id
)
warning = extra_scopes_warning(
self.integration_id, self.display_name, self.method, credentials
)
Expand Down
38 changes: 38 additions & 0 deletions tests/auth/test_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,3 +242,41 @@ def test_a_static_login_without_a_client_id_raises():
port=0,
timeout=10,
)


BUNDLED = oauth.OAuthMethod(
provider=oauth.BundledProvider(
metadata=oauth.ServerMetadata(
authorization_endpoint=METADATA["authorization_endpoint"],
token_endpoint=METADATA["token_endpoint"],
),
client_id="client-bundled",
client_secret="secret-bundled",
),
scopes=("read",),
)


def test_a_bundled_login_uses_the_bundled_id_posts_the_secret_and_never_registers(monkeypatch):
browser_sending(monkeypatch, "/callback?code=code-1&state={state}")
bodies = []

def handler(request: httpx.Request) -> httpx.Response:
if request.url.path == "/token":
bodies.append(urllib.parse.parse_qs(request.content.decode()))
return httpx.Response(200, json=TOKEN)
raise AssertionError(f"unexpected request to {request.url}")

client_id, credentials = perform_login(
httpx.Client(transport=httpx.MockTransport(handler)),
BUNDLED,
None,
on_authorize_url=lambda url: None,
port=0,
timeout=10,
)

assert client_id == "client-bundled"
assert credentials.access_token == "at-1"
assert bodies[0]["client_id"] == ["client-bundled"]
assert bodies[0]["client_secret"] == ["secret-bundled"]
Loading