From 699b0340cf9977eb0afe6f6176f6797b24a312de Mon Sep 17 00:00:00 2001 From: VACInc <3279061+VACInc@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:04:29 -0400 Subject: [PATCH 1/6] feat(auth): add Spotify OAuth PKCE --- CHANGELOG.md | 4 + README.md | 27 +- cmd/spogo/main.go | 2 +- cmd/spogo/main_test.go | 36 ++ docs/agents.md | 11 +- docs/auth.md | 205 +++++-- docs/commands.md | 10 +- docs/engines.md | 25 +- docs/quickstart.md | 13 +- docs/spec.md | 26 +- docs/troubleshooting.md | 28 + go.mod | 5 +- go.sum | 2 + internal/app/context.go | 33 +- internal/app/context_factory.go | 47 +- internal/app/context_init.go | 9 + internal/app/context_test.go | 77 ++- internal/app/errors.go | 2 +- internal/app/errors_test.go | 3 + internal/cli/auth.go | 20 + internal/cli/auth_oauth.go | 287 +++++++++ internal/cli/auth_oauth_test.go | 435 +++++++++++++ internal/cli/cli.go | 62 +- internal/config/config.go | 28 +- internal/config/config_test.go | 42 ++ internal/spotify/connect.go | 2 + internal/spotify/connect_webclient_test.go | 21 + internal/spotify/oauth.go | 528 ++++++++++++++++ internal/spotify/oauth_replace_unix.go | 9 + internal/spotify/oauth_replace_windows.go | 21 + internal/spotify/oauth_test.go | 683 +++++++++++++++++++++ 31 files changed, 2547 insertions(+), 156 deletions(-) create mode 100644 internal/cli/auth_oauth.go create mode 100644 internal/cli/auth_oauth_test.go create mode 100644 internal/spotify/oauth.go create mode 100644 internal/spotify/oauth_replace_unix.go create mode 100644 internal/spotify/oauth_replace_windows.go create mode 100644 internal/spotify/oauth_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b8aaac6..07f6e4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Added + +- Add official Spotify Authorization Code with PKCE for Web API requests, including automatic refresh and a secure per-profile token cache while preserving cookie-based Connect behavior + ### Changed - Refresh Go dependencies, including Kong, SweetCookie, SQLite, crypto, and formatting tools, and select Go 1.26.8 while retaining Go 1.26.7 support diff --git a/README.md b/README.md index 41044e8..2a79fe6 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ ![spogo banner](docs/assets/readme-banner.jpg) - Power CLI using web cookies. Search, control playback, manage library/playlists, and script with JSON/plain output. + Power CLI using Spotify browser cookies or official OAuth. Search, control playback, manage library/playlists, and script with JSON/plain output. Product direction and compatibility policy: [VISION.md](VISION.md). @@ -17,13 +17,15 @@ Product direction and compatibility policy: [VISION.md](VISION.md). - Playlist management (create/add/remove/list) - Device selection and status - Browser cookie import via `sweetcookie` +- Official Spotify Authorization Code OAuth with PKCE, refresh tokens, and a secure per-profile token cache +- Explicit `--auth cookies|oauth` selection for Web API requests - `--json` and `--plain` for scripting - Colorized human output (respects `NO_COLOR`, `TERM=dumb`, `--no-color`) - Engine switch: `auto` (connect → web → local Spotify.app for playback on macOS), `connect` (internal endpoints), `web` (Web API endpoints; search/info/playback fall back to connect on rate limit) -## Why Cookies? +## Cookies and OAuth -Spotify's official Web API has strict rate limits that can make it impractical for agents and automation. Browser cookies let spogo use the same internal endpoints as the Spotify web player for catalog search, item lookup, library listing, listening history, and most playback and playlist operations: +Cookie auth remains the default because Spotify's official Web API has strict rate limits that can make it impractical for agents and automation. Browser cookies let spogo use the same internal endpoints as the Spotify web player for catalog search, item lookup, library listing, listening history, and most playback and playlist operations: - **Fewer public-API rate limits** - Most reads and playback use the same internal endpoints as open.spotify.com - **No app registration** - No need to create a Spotify Developer app @@ -34,6 +36,15 @@ Import your cookies once with `sweetcookie` and you're good to go (defaults to C Some operations still require Spotify's public Web API: saving/removing library tracks or albums, following/unfollowing artists, creating playlists, artist-top-track lookups used by artist playback, and certain device transfers or playback fallbacks. Explicit `--engine web` also uses the public API. These paths can return `429`; when Spotify supplies a cooldown, spogo reports its `retry-after hint`, which can be several hours. +For a cookie-free Web API setup, spogo also supports Spotify's official Authorization Code flow with PKCE: + +```bash +spogo auth oauth login --client-id YOUR_SPOTIFY_CLIENT_ID +spogo --engine web --auth oauth search track "weezer" +``` + +OAuth never uses a client secret. Connect and internal endpoints still require browser cookies; selecting OAuth changes the Web API token provider, not the Connect protocol. + ## Install ### Homebrew @@ -73,6 +84,9 @@ Global flags: - `--language ` language/locale (default `en`) - `--device ` target device - `--engine ` API engine (default `connect`, `applescript` is macOS-only) +- `--auth ` Web API authentication (default `cookies`) +- `--spotify-client-id ` public Spotify application client ID +- `--spotify-redirect-uri ` registered loopback OAuth redirect URI - `--json` / `--plain` - `--no-color` - `-q, --quiet` / `-v, --verbose` / `-d, --debug` @@ -86,6 +100,7 @@ Commands: - `completion bash|zsh|fish` - `auth status|import|paste|clear` +- `auth oauth login|status|clear` - `search track|album|artist|playlist|show|episode` - `track info`, `album info`, `artist info`, `playlist info`, `show info`, `episode info` - `play [] [--type ...] [--shuffle]`, `pause`, `next`, `prev`, `seek`, `volume`, `shuffle`, `repeat`, `status` @@ -97,9 +112,9 @@ Commands: Full spec: `docs/spec.md`. -## Cookies +## Authentication -`spogo` uses browser cookies (via `sweetcookie`) to fetch a web access token. Import cookies once: +Cookie auth is the default. Import cookies once: ```bash spogo auth import --browser chrome @@ -126,6 +141,8 @@ Non-interactive: printf '%s\n%s\n' "sp_dc=..." "sp_t=..." | spogo auth paste --no-input ``` +Official OAuth is available for the Web API client. Register `http://127.0.0.1:8888/callback` in a Spotify developer application, then run `spogo auth oauth login --client-id ...`. See [Auth](docs/auth.md) for scopes, storage, environment variables, and the exact Connect/OAuth interaction. + ## Auto engine notes - `auto` tries connect first, then falls back to web on unsupported features or rate limits. diff --git a/cmd/spogo/main.go b/cmd/spogo/main.go index 34a048a..b82bec3 100644 --- a/cmd/spogo/main.go +++ b/cmd/spogo/main.go @@ -23,7 +23,7 @@ func run(args []string, out io.Writer, errOut io.Writer) int { parser, err := kong.New( command, kong.Name("spogo"), - kong.Description("Spotify power CLI using web cookies."), + kong.Description("Spotify power CLI using browser cookies or official OAuth."), kong.UsageOnError(), kong.Writers(out, errOut), kong.Vars(cli.VersionVars()), diff --git a/cmd/spogo/main_test.go b/cmd/spogo/main_test.go index 0c3c864..39368eb 100644 --- a/cmd/spogo/main_test.go +++ b/cmd/spogo/main_test.go @@ -9,8 +9,11 @@ import ( "path/filepath" "strings" "testing" + "time" + "github.com/steipete/spogo/internal/config" "github.com/steipete/spogo/internal/cookies" + "github.com/steipete/spogo/internal/spotify" "github.com/steipete/sweetcookie" ) @@ -193,6 +196,39 @@ func TestRunAuthStatusWithoutCookiesReturnsAuthExitCode(t *testing.T) { } } +func TestRunOAuthStatusCommandName(t *testing.T) { + out := &bytes.Buffer{} + errOut := &bytes.Buffer{} + configPath := filepath.Join(t.TempDir(), "config.toml") + code := run([]string{"--config", configPath, "--plain", "auth", "oauth", "status"}, out, errOut) + if code != 0 { + t.Fatalf("expected 0, got %d; out=%q err=%q", code, out.String(), errOut.String()) + } +} + +func TestRunOAuthStatusMismatchReturnsAuthExitCode(t *testing.T) { + out := &bytes.Buffer{} + errOut := &bytes.Buffer{} + configPath := filepath.Join(t.TempDir(), "config.toml") + cfg := config.Default() + cfg.SetProfile("default", config.Profile{Auth: "oauth", SpotifyClientID: "configured-client"}) + if err := config.Save(configPath, cfg); err != nil { + t.Fatalf("save config: %v", err) + } + if err := spotify.SaveOAuthToken(config.OAuthTokenPath(configPath, "default"), spotify.OAuthToken{ + AccessToken: "access", + RefreshToken: "refresh", + ExpiresAt: time.Now().Add(time.Hour), + ClientID: "other-client", + }); err != nil { + t.Fatal("save token:", err) + } + code := run([]string{"--config", configPath, "--plain", "auth", "oauth", "status"}, out, errOut) + if code != 3 { + t.Fatalf("expected 3, got %d; out=%q err=%q", code, out.String(), errOut.String()) + } +} + func TestNormalizeArgsMovesNoInput(t *testing.T) { got := normalizeArgs([]string{"auth", "paste", "--no-input", "--cookie-path", "cookies.json"}) want := []string{"--no-input", "auth", "paste", "--cookie-path", "cookies.json"} diff --git a/docs/agents.md b/docs/agents.md index 6d5dec4..90680f7 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -25,12 +25,15 @@ See [Output](output.md) for the full contract. #!/usr/bin/env bash set -euo pipefail -# Make sure auth still works +# Make sure the configured cookie store exists if ! spogo auth status >/dev/null 2>&1; then echo "spogo: cookies missing or stale; re-run 'spogo auth import'" >&2 exit 3 fi +# For an OAuth-only Web API profile, inspect local OAuth state instead: +# spogo --engine web --auth oauth auth oauth status --json + # Capture the currently playing track ID track_id=$(spogo status --json | jq -r '.item.id // empty') if [[ -z "$track_id" ]]; then @@ -89,7 +92,7 @@ spogo writes nothing to stdout that isn't useful and nothing to stderr unless so 0 4 * * * /usr/local/bin/spogo library tracks list --limit 1000 --json > "$HOME/snapshots/tracks-$(date +\%F).json" 2>&1 ``` -For headless servers / CI runners, copy a working cookie jar (from a machine where you ran `auth import`) into the runner's spogo config directory rather than trying to import from a browser that doesn't exist. +For headless servers / CI runners, either copy a working cookie jar (from a machine where you ran `auth import`) into the runner's spogo config directory, or provision an OAuth token cache created by `auth oauth login` for `--engine web --auth oauth`. Both files are credentials. Do not print them or commit them. ## CI @@ -118,7 +121,7 @@ spogo is a good fit for AI coding agents (Claude Code, Codex, Cursor) because: - **Self-documenting.** `spogo --help` and `spogo --help` describe the entire surface. The [Spec](spec.md) is short and stable. - **Deterministic.** Stable JSON keys mean the agent's parsing doesn't drift across releases. -- **Safe-ish.** The destructive surface is small (`library tracks remove`, `playlist remove`, `auth clear`). Wrap those behind explicit confirmation in your agent prompt. +- **Safe-ish.** The destructive surface is small (`library tracks remove`, `playlist remove`, `auth clear`, `auth oauth clear`). Wrap those behind explicit confirmation in your agent prompt. Recommended agent rules: @@ -129,7 +132,7 @@ Recommended agent rules: A starter system prompt fragment for an agent: -> You can use the `spogo` CLI to control Spotify. Always pass `--json` and `--no-input`. Read `spogo --help` and `spogo --help` before invoking unfamiliar commands. Treat exit code `3` as "needs auth" — surface that to the user, don't try to recover automatically. +> You can use the `spogo` CLI to control Spotify. Always pass `--json` and `--no-input`. Read `spogo --help` and `spogo --help` before invoking unfamiliar commands. Treat exit code `3` as "needs auth". Surface that to the user; do not launch an interactive cookie import or OAuth login automatically. ## Safety diff --git a/docs/auth.md b/docs/auth.md index 7c7c450..59d6fb5 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -1,132 +1,219 @@ --- title: Auth -description: "How spogo authenticates with Spotify using your browser cookies — import, paste, status, troubleshooting." +description: "Authenticate spogo with browser cookies or Spotify Authorization Code OAuth with PKCE." --- # Auth -spogo does not use the Spotify Developer API. It reads the cookies your browser already has for `open.spotify.com` and uses them to fetch a web access token. That means **no app registration, no client ID, no redirect URI** — just log in to Spotify in your browser, then import. +spogo supports two authentication mechanisms: -The cookie machinery comes from [steipete/sweetcookie](https://github.com/steipete/sweetcookie). +- **Browser cookies** are the default and preserve the existing Connect/internal-endpoint behavior. +- **Spotify OAuth** uses the official Authorization Code flow with PKCE for Web API requests. It does not use or store a client secret. -## What spogo needs +Choose Web API authentication with `--auth cookies|oauth`, `SPOGO_AUTH`, or the profile's `auth` setting. The default is `cookies`, so existing profiles keep their current behavior. -The minimum cookies for authentication: +## Which auth should I use? -- `sp_dc` — required. Long-lived web session cookie. -- `sp_key` — optional, helps with rotation. -- `sp_t` — recommended for `connect` engine playback control. +| Goal | Engine | Auth | Credentials required | +| --- | --- | --- | --- | +| Existing internal endpoints and Connect playback | `connect` or `auto` | `cookies` | Spotify browser cookies | +| Official Spotify Web API only | `web` | `oauth` | OAuth token cache + Spotify client ID | +| Connect first, official Web API for public-API fallbacks | `connect` or `auto` | `oauth` | Both browser cookies and OAuth | -These cookies live in your browser's cookie store and rotate on their own; spogo refreshes its cached access token using them as needed. +OAuth cannot replace cookies for Spotify's internal Connect protocol. If you want a cookie-free setup, use `--engine web --auth oauth`. -## Importing from a browser +## Official Spotify OAuth + +spogo implements Spotify's [Authorization Code with PKCE flow](https://developer.spotify.com/documentation/web-api/tutorials/code-pkce-flow). PKCE is designed for installed/public clients that cannot safely hold a client secret. + +### 1. Register a Spotify application + +Create an application in the Spotify Developer Dashboard and add this redirect URI exactly: + +```text +http://127.0.0.1:8888/callback +``` + +Spotify requires an explicit loopback IP. `localhost` is not accepted. A different loopback URI is fine, but it must match the dashboard entry and the value passed to spogo exactly. + +### 2. Log in ```bash -spogo auth import --browser chrome +spogo auth oauth login --client-id YOUR_SPOTIFY_CLIENT_ID ``` -Defaults: Chrome + `Default` profile + `spotify.com` domain. Cookies are stored under your config directory keyed by profile. +spogo starts a loopback-only callback server, generates a cryptographically random state value and PKCE verifier, opens the Spotify authorization page, validates the callback state, exchanges the code, and stores the resulting access and refresh tokens. -### Pick a different browser +If the machine cannot open a browser automatically: ```bash -spogo auth import --browser brave -spogo auth import --browser edge -spogo auth import --browser firefox -spogo auth import --browser safari +spogo auth oauth login --client-id YOUR_SPOTIFY_CLIENT_ID --no-open ``` -### Pick a non-default profile +Open the URL printed to stderr. The command waits up to five minutes by default; change that with `--wait-timeout`. -Chrome / Brave / Edge keep profiles in directories like `Default`, `Profile 1`, `Profile 2`. Pass the directory name: +For a custom registered callback: ```bash -spogo auth import --browser chrome --browser-profile "Profile 1" +spogo auth oauth login \ + --client-id YOUR_SPOTIFY_CLIENT_ID \ + --redirect-uri http://127.0.0.1:9999/callback ``` -### Use a specific cookie store file +The login stores only the non-secret client ID, redirect URI, and `auth = "oauth"` in the profile config. **Client secrets are unsupported and are never read from or written to config.** -If you have an exported cookie jar already: +### 3. Use the Web API client ```bash -spogo auth import --cookie-path /path/to/cookies.sqlite +spogo --engine web --auth oauth search track "weezer" --limit 5 +spogo --engine web --auth oauth library tracks list --limit 20 ``` -### Limit the cookie scope +After login, `auth = "oauth"` is saved for the profile, so `--auth oauth` is optional. `--engine web` remains explicit because the default `connect` engine still requires cookies. + +### OAuth scopes + +spogo requests the scopes needed by its existing Web API surface: + +- playback state read/write and currently-playing access +- saved-library read/write +- followed-artist read/write +- private/collaborative playlist read and public/private playlist write +- top tracks and recently played history +- private account data required by Spotify search/profile endpoints + +It does not request email access, streaming, image upload, or Web Playback SDK scopes. + +### OAuth status and clearing ```bash -spogo auth import --domain spotify.com +spogo auth oauth status +spogo auth oauth clear ``` -When the browser-store read returns nothing, spogo now surfaces the underlying warning (locked keychain, missing profile, decryption failure) instead of just printing `no cookies found`. +`status` reads only local metadata. It never prints access or refresh token values and does not call Spotify. `clear` removes the token cache and returns the profile to the default cookie auth selection while retaining the non-secret client ID and redirect URI for a future login. -## Manual paste (WSL fallback) +Access tokens refresh automatically before expiry. Spotify refresh responses that omit a replacement refresh token retain the previous refresh token. If Spotify rejects or expires the refresh token, run `auth oauth login` again. -If WSL cookie decryption is broken, or you need to copy cookies from a Chromium DevTools session, paste the values straight in: +### OAuth config and environment -1. In Chrome, open DevTools → Application → Cookies → `https://open.spotify.com`. -2. Copy the values for `sp_dc` (required), `sp_key` (optional), `sp_t` (recommended). -3. Run: +Profile config example: + +```toml +[profile.default] +auth = "oauth" +spotify_client_id = "your-public-client-id" +spotify_redirect_uri = "http://127.0.0.1:8888/callback" +engine = "web" +``` + +Equivalent environment overrides: ```bash -spogo auth paste +export SPOGO_AUTH=oauth +export SPOGO_SPOTIFY_CLIENT_ID=your-public-client-id +export SPOGO_SPOTIFY_REDIRECT_URI=http://127.0.0.1:8888/callback +export SPOGO_ENGINE=web +``` + +There is intentionally no client-secret config key, flag, or environment variable. + +### OAuth token storage + +OAuth tokens are stored per profile under the config directory: + +```text +/spogo/oauth/.json ``` -spogo prompts for each cookie. To skip the prompts (CI, scripts): +The OAuth directory is mode `0700` and token file is mode `0600` on POSIX systems. Writes use a same-directory temporary file, file sync, and atomic rename. spogo refuses to load a token file that is readable or writable by group/other users. + +Treat the token cache as a credential. Do not copy it into source control, logs, shell history, or CI artifacts. + +## Browser cookie auth + +Cookie auth remains the default. spogo reads the cookies your browser already has for `open.spotify.com` and uses them for the internal Web Player and Connect protocols. The cookie machinery comes from [steipete/sweetcookie](https://github.com/steipete/sweetcookie). + +### What spogo needs + +- `sp_dc` is required. +- `sp_key` is optional and helps with rotation. +- `sp_t` is recommended for Connect playback control. + +### Importing from a browser ```bash -printf '%s\n%s\n' "sp_dc=..." "sp_t=..." | spogo auth paste --no-input +spogo auth import --browser chrome ``` -Other paste flags: +Supported browser names are `chrome`, `brave`, `edge`, `firefox`, and `safari`. -- `--cookie-path ` — write the resulting cookie jar to a custom path. -- `--domain ` — override the cookie domain (default `spotify.com`). -- `--path ` — override the cookie path (default `/`). +For a non-default browser profile: + +```bash +spogo auth import --browser chrome --browser-profile "Profile 1" +``` -## Status & clearing +For a specific cookie store file: ```bash -spogo auth status # which profile, when imported, what cookies exist -spogo auth clear # delete the stored cookies for the current profile +spogo auth import --cookie-path /path/to/cookies.sqlite ``` -`auth status` does not call Spotify; it only inspects the local store. To verify cookies actually work, run any read command: +When the browser-store read returns nothing, spogo surfaces the underlying warning, such as a locked keychain, missing profile, or decryption failure. + +### Manual paste (WSL fallback) + +1. In Chrome, open DevTools, then Application, Cookies, `https://open.spotify.com`. +2. Copy `sp_dc` and, preferably, `sp_t`. `sp_key` is optional. +3. Run: ```bash -spogo status -spogo search track "test" --limit 1 +spogo auth paste ``` -A `401`/`403` from those means the cookies are stale — re-import. +For non-interactive input: + +```bash +printf '%s\n%s\n' "sp_dc=..." "sp_t=..." | spogo auth paste --no-input +``` + +### Cookie status and clearing + +```bash +spogo auth status +spogo auth clear +``` -## Where cookies are stored +These existing commands remain cookie-specific. They do not inspect or clear OAuth tokens. -- macOS: `~/Library/Application Support/spogo//cookies.json` -- Linux: `~/.config/spogo//cookies.json` -- Windows: `%APPDATA%\spogo\\cookies.json` +`auth status` does not call Spotify. To verify cookies actually work, run a command that uses them: -`` defaults to `default` — override with `--profile ` or `SPOGO_PROFILE`. +```bash +spogo status +spogo search track "test" --limit 1 +``` ## Multiple accounts -Use profiles to keep multiple Spotify logins side by side: +Profiles keep cookie jars, OAuth token caches, and settings separate: ```bash spogo --profile work auth import --browser chrome --browser-profile "Profile 1" -spogo --profile personal auth import --browser chrome --browser-profile "Default" +spogo --profile personal auth oauth login --client-id YOUR_SPOTIFY_CLIENT_ID spogo --profile work status -spogo --profile personal play spotify:track:... +spogo --profile personal --engine web search track "test" ``` -Set the default for a shell with `export SPOGO_PROFILE=work`. - ## Troubleshooting -- **"no cookies found"** — pass `--browser-profile`, double-check you're logged in to `open.spotify.com` in that browser, and check the warning spogo prints (it now surfaces the real reason). -- **Locked keychain (macOS Chrome)** — unlock the login keychain, then re-run `auth import`. -- **WSL Chrome** — cookie decryption is unreliable; use [paste](#manual-paste-wsl-fallback). -- **Auth works locally but not in CI** — copy the cookie jar file (the path printed by `auth status`) into the CI runner before running spogo. +- **`no cookies found`**: choose the correct browser profile or use `auth paste`. +- **OAuth callback bind failure**: another process is using the registered port. Stop it or register and pass a different loopback redirect URI. +- **OAuth state mismatch**: leave the command running and use the newest authorization URL it printed. Invalid callbacks are rejected. +- **OAuth token cache permissions error**: change the token file to owner-only mode (`0600`) and its directory to `0700`, or clear and log in again. +- **OAuth works with `web` but `connect` fails**: expected. Connect still requires Spotify browser cookies. +- **OAuth refresh rejected**: run `spogo auth oauth login` again. +- **`401`/`403` with cookies**: re-import after logging back into `open.spotify.com`. See [Troubleshooting](troubleshooting.md) for more. diff --git a/docs/commands.md b/docs/commands.md index 8545101..fc1ea0a 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -24,6 +24,9 @@ Apply to every command. | `--language ` | `en` | Language/locale. | | `--device ` | active | Target a specific Connect device. | | `--engine ` | `connect` | `auto` / `connect` / `web` / `applescript`. | +| `--auth ` | `cookies` | Web API auth: `cookies` / `oauth`. | +| `--spotify-client-id ` | profile | Public Spotify application client ID. | +| `--spotify-redirect-uri ` | profile | Registered loopback OAuth redirect URI. | | `--json` | off | JSON output. | | `--plain` | off | Plain (TSV) output. | | `--no-color` | auto | Disable color in human output. | @@ -67,7 +70,7 @@ To enable them permanently, add the appropriate command to `~/.bashrc`, `~/.zshr ## auth -Cookie management. See [Auth](auth.md). +Cookie and official Spotify OAuth management. See [Auth](auth.md). | Command | Purpose | | --- | --- | @@ -75,6 +78,9 @@ Cookie management. See [Auth](auth.md). | `spogo auth import [--browser ] [--browser-profile ] [--cookie-path ] [--domain ]` | Pull cookies from a browser store. | | `spogo auth paste [--cookie-path ] [--domain ] [--path ]` | Read cookies from stdin (interactive prompts unless `--no-input`). | | `spogo auth clear` | Delete stored cookies for the current profile. | +| `spogo auth oauth login [--client-id ] [--redirect-uri ] [--no-open] [--wait-timeout ]` | Run Authorization Code with PKCE and cache refresh credentials. | +| `spogo auth oauth status` | Show non-secret local OAuth cache metadata. | +| `spogo auth oauth clear` | Delete the OAuth token cache and restore cookie auth selection. | ## search @@ -189,7 +195,7 @@ Connect devices. See [Devices](devices.md). | `0` | Success | | `1` | Generic failure | | `2` | Invalid usage / validation | -| `3` | Auth / cookies missing or invalid | +| `3` | Auth credentials missing or invalid | | `4` | Network / timeouts | See [Output](output.md) for the full output contract. diff --git a/docs/engines.md b/docs/engines.md index 7b8abcb..d4f43e7 100644 --- a/docs/engines.md +++ b/docs/engines.md @@ -16,7 +16,7 @@ spogo can talk to Spotify through one of four engines. Pick whichever matches wh | "I just want it to work" | `auto` | | Drive Spotify.app on macOS, no network needed | `applescript` | -Set with `--engine ` per call, or globally with `SPOGO_ENGINE`. +Set with `--engine ` per call, or globally with `SPOGO_ENGINE`. Engine selection is separate from Web API authentication, selected with `--auth cookies|oauth` or `SPOGO_AUTH`. ## connect (default) @@ -30,6 +30,10 @@ Talks to Spotify's internal Connect endpoints — the same ones the official des - Search and item info via the internal GraphQL surface, including episode lookup. - Listing followed artists, saved albums/tracks, and playlists; user top tracks and recently played history also use internal endpoints. +**Authentication** + +Connect always requires Spotify browser cookies. When Connect delegates an operation to the public Web API, that fallback uses the selected `--auth cookies|oauth` provider. Therefore `--engine connect --auth oauth` requires both cookies for Connect and an OAuth login for Web API fallbacks. + **Tradeoffs** - Saving/removing library tracks or albums, following/unfollowing artists, creating playlists, and artist-top-track lookups used by artist playback still require the public Web API. @@ -40,6 +44,15 @@ Talks to Spotify's internal Connect endpoints — the same ones the official des The public Spotify Web API. Slower, lower throughput, and rate-limited according to Spotify's account- and application-specific policies; cookie-derived tokens can encounter aggressive cooldowns, including retry hints measured in hours. +**Authentication** + +Use the existing cookie-derived Web API token with `--auth cookies` (the default), or the official Authorization Code with PKCE token with `--auth oauth`. A cookie-free setup is: + +```bash +spogo auth oauth login --client-id YOUR_SPOTIFY_CLIENT_ID +spogo --engine web --auth oauth search track "weezer" +``` + **Best for** - Accounts that can't use Connect (rare — usually corporate or family-restricted). @@ -53,7 +66,7 @@ The public Spotify Web API. Slower, lower throughput, and rate-limited according ## auto -Try `connect` first, then fall back to `web` for unsupported features or rate limits. On macOS, playback status and controls get one final fallback to the already-local Spotify.app through AppleScript after both remote engines fail, including when cookies are missing. +Try `connect` first, then fall back to `web` for unsupported features or rate limits. Because Connect is first, `auto` still requires browser cookies even when `--auth oauth` selects OAuth for the Web API fallback. On macOS, playback status and controls get one final fallback to the already-local Spotify.app through AppleScript after both remote engines fail, including when cookies are missing. ```bash spogo --engine auto play spotify:playlist:... @@ -92,20 +105,22 @@ Per command: ```bash spogo --engine connect play -spogo --engine web search track "weezer" +spogo --engine web --auth oauth search track "weezer" spogo --engine applescript pause ``` Per shell: ```bash -export SPOGO_ENGINE=connect +export SPOGO_ENGINE=web +export SPOGO_AUTH=oauth ``` In a config profile (`~/.config/spogo//config.toml` or platform equivalent): ```toml -engine = "connect" +engine = "web" +auth = "oauth" ``` ## Diagnosing engine issues diff --git a/docs/quickstart.md b/docs/quickstart.md index 9168cee..e857a19 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -31,7 +31,16 @@ spogo auth import --browser chrome --browser-profile "Profile 1" If something goes wrong (locked keychain, weird WSL setup), see [Auth](auth.md) for `auth paste` and other fallbacks. -Verify: +For an official, cookie-free Web API setup instead, register `http://127.0.0.1:8888/callback` in a Spotify developer application and run: + +```bash +spogo auth oauth login --client-id YOUR_SPOTIFY_CLIENT_ID +spogo --engine web --auth oauth search track "test" --limit 1 +``` + +OAuth supports the Web API client. Connect and `auto` still require browser cookies. + +Verify cookie auth: ```bash spogo auth status @@ -80,7 +89,7 @@ spogo search track "lo-fi" --limit 5 --plain | ## Where to next -- [Auth](auth.md) — cookie details, manual paste, troubleshooting. +- [Auth](auth.md) — cookie import, official OAuth, token storage, and troubleshooting. - [Engines](engines.md) — when to choose `connect`, `web`, `auto`, or `applescript`. - [Output](output.md) — the JSON / plain contract. - [Agents](agents.md) — end-to-end automation patterns. diff --git a/docs/spec.md b/docs/spec.md index 402b2b2..e7147aa 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -1,8 +1,9 @@ # spogo CLI spec -One-liner: Spotify power CLI using web cookies; search + playback control. +One-liner: Spotify power CLI using browser cookies or official OAuth; search + playback control. Parser: Kong. Cookies: steipete/sweetcookie (local sweetcookie). +OAuth: Spotify Authorization Code with PKCE; no client secret. Output: human by default; `--plain` or `--json`. Color: on by default; respects `NO_COLOR`, `TERM=dumb`, `--no-color`. Platforms: macOS, Linux, Windows. @@ -30,6 +31,9 @@ spogo [global flags] [args] - `--language ` default: `en` - `--device ` default: active device - `--engine ` default: `connect` (`applescript` is macOS-only) +- `--auth ` default: `cookies`; selects the Web API token provider +- `--spotify-client-id ` / `SPOGO_SPOTIFY_CLIENT_ID` +- `--spotify-redirect-uri ` / `SPOGO_SPOTIFY_REDIRECT_URI` - `--no-input` ## Commands @@ -57,6 +61,15 @@ spogo [global flags] [args] - `--domain ` default `spotify.com` - `--path ` default `/` - `spogo auth clear` +- `spogo auth oauth login` + - Authorization Code with PKCE and state validation + - flags: `--client-id`, `--redirect-uri`, `--no-open`, `--wait-timeout` + - callback must be an explicit IPv4/IPv6 loopback URI with a port + - stores no client secret +- `spogo auth oauth status` + - local-only; never emits token values +- `spogo auth oauth clear` + - removes the OAuth token cache and restores cookie auth selection ### search @@ -143,8 +156,8 @@ spogo [global flags] [args] ## Engines - `auto`: connect first; fall back to web for unsupported features or rate limits; on macOS, playback status/control can finally fall back to Spotify.app through AppleScript after both remote engines fail. -- `connect`: internal connect-state endpoints for playback; GraphQL for search/info. Auth/session data and the last active playback route are cached per profile. -- `web`: Web API endpoints; search/info/playback auto-fallback to connect when rate limited. +- `connect`: internal connect-state endpoints for playback; GraphQL for search/info. Auth/session data and the last active playback route are cached per profile. Connect always requires browser cookies. Its public Web API fallbacks use the selected `cookies` or `oauth` provider. +- `web`: Web API endpoints authenticated by the selected `cookies` or `oauth` provider; search/info/playback can fall back to Connect when rate limited, which requires cookies. ## Exit codes @@ -158,7 +171,10 @@ spogo [global flags] [args] - Env prefix: `SPOGO_` - Precedence: flags > env > config -- Secrets: never via flags; use browser cookies only. +- OAuth precedence: command/global flags > env > profile config. +- Non-secret profile keys: `auth`, `spotify_client_id`, `spotify_redirect_uri`. +- OAuth token cache: `/oauth/.json`, atomic owner-only writes (`0600`; parent `0700` on POSIX). +- Client secrets are unsupported: no config key, flag, or environment variable exists. - Overrides: - `SPOGO_TOTP_SECRET_URL` (http(s) or `file://...`) - `SPOGO_CONNECT_VERSION` (connect playback client version) @@ -166,6 +182,8 @@ spogo [global flags] [args] ## Examples - `spogo auth import --browser chrome` +- `spogo auth oauth login --client-id YOUR_SPOTIFY_CLIENT_ID` +- `spogo --engine web --auth oauth search track "weezer" --limit 5 --plain` - `spogo search track "weezer" --limit 5 --plain` - `spogo play spotify:track:7hQJA50XrCWABAu5v6QZ4i` - `spogo device list --json` diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 1edc6ff..9b56d53 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -36,6 +36,34 @@ spogo auth status If that doesn't help, your browser's session may have expired. Visit `https://open.spotify.com`, log back in, then re-import. +### OAuth login says the callback address is already in use + +The registered loopback port is occupied. Close the process using it, or add a different loopback redirect URI to the Spotify application and pass the exact same URI to `auth oauth login --redirect-uri`. + +### OAuth works with `--engine web` but Connect fails + +This is expected when cookies are missing. OAuth authenticates the public Web API client only. The internal `connect` protocol and the Connect-first `auto` engine still require Spotify browser cookies. + +For OAuth without cookies: + +```bash +spogo --engine web --auth oauth status +``` + +### OAuth token cache permissions are rejected + +spogo requires owner-only OAuth credentials. On POSIX systems the token file must be `0600` and its directory must be `0700`. Fix those permissions, or run `spogo auth oauth clear` and log in again. + +### OAuth refresh is rejected + +The refresh token may have been revoked or expired. Run: + +```bash +spogo auth oauth login +``` + +The client ID and redirect URI remain in the profile after `auth oauth clear`, so a client ID flag is only required when it is not already configured. + ### macOS Chrome keychain prompt The first cookie import will trigger a "Chrome wants to use your confidential information from your keychain" dialog. Click **Always Allow**. If you mis-click **Deny**, fix it via: diff --git a/go.mod b/go.mod index b81e94b..6ddf7e9 100644 --- a/go.mod +++ b/go.mod @@ -9,10 +9,12 @@ require ( github.com/coder/websocket v1.8.15 github.com/daixiang0/gci v0.14.0 github.com/fatih/color v1.19.0 + github.com/gofrs/flock v0.13.1 github.com/jotaen/kong-completion v0.0.14 github.com/mattn/go-isatty v0.0.24 github.com/pelletier/go-toml/v2 v2.4.3 github.com/steipete/sweetcookie v0.0.2 + golang.org/x/sys v0.47.0 mvdan.cc/gofumpt v0.11.0 ) @@ -35,7 +37,7 @@ require ( github.com/rogpeppe/go-internal v1.16.0 // indirect github.com/spf13/cobra v1.10.2 // indirect github.com/spf13/pflag v1.0.10 // indirect - github.com/stretchr/testify v1.12.1 // indirect + github.com/stretchr/objx v0.5.3 // indirect github.com/zalando/go-keyring v0.2.8 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.28.0 // indirect @@ -43,7 +45,6 @@ require ( golang.org/x/crypto v0.56.0 // indirect golang.org/x/mod v0.40.0 // indirect golang.org/x/sync v0.22.0 // indirect - golang.org/x/sys v0.47.0 // indirect golang.org/x/tools v0.49.0 // indirect gopkg.in/ini.v1 v1.67.3 // indirect modernc.org/libc v1.75.6 // indirect diff --git a/go.sum b/go.sum index 5133d12..1ff3133 100644 --- a/go.sum +++ b/go.sum @@ -21,6 +21,8 @@ github.com/go-quicktest/qt v1.102.0 h1:HSQxCeh5YZH3EL3W39ixjtyaEhcWSXQHtHnMBzSs4 github.com/go-quicktest/qt v1.102.0/go.mod h1:p4lGIVX+8Wa6ZPNDvqcxq36XpUDLh42FLetFU7odllI= github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= +github.com/gofrs/flock v0.13.1 h1:jjREztyBeSKBZYAC+mgc1laB+xsgy4kYMf3FbKF2UBo= +github.com/gofrs/flock v0.13.1/go.mod h1:sf4BFiHwnvgxa25DlQoDqXQnwRMEOwqxRq37P6MzzmE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/pprof v0.0.0-20260903180319-d6c3cb2f37ec h1:ZhR4jlKRUmzPEPd2RLiJRUi25rUrdKBrhQNVIbqpTNE= diff --git a/internal/app/context.go b/internal/app/context.go index 09e1f1c..516d81e 100644 --- a/internal/app/context.go +++ b/internal/app/context.go @@ -13,19 +13,22 @@ import ( ) type Settings struct { - ConfigPath string - Profile string - Timeout time.Duration - Market string - Language string - Device string - Engine string - Format output.Format - NoColor bool - Quiet bool - Verbose bool - Debug bool - NoInput bool + ConfigPath string + Profile string + Timeout time.Duration + Market string + Language string + Device string + Engine string + Auth string + SpotifyClientID string + SpotifyRedirectURI string + Format output.Format + NoColor bool + Quiet bool + Verbose bool + Debug bool + NoInput bool } type Context struct { @@ -72,6 +75,10 @@ func (c *Context) ResolveCachePath() string { return config.CachePath(c.ConfigPath, c.ProfileKey) } +func (c *Context) ResolveOAuthTokenPath() string { + return config.OAuthTokenPath(c.ConfigPath, c.ProfileKey) +} + func (c *Context) ClearCache() error { path := c.ResolveCachePath() if path == "" { diff --git a/internal/app/context_factory.go b/internal/app/context_factory.go index 8c5f567..e761ebf 100644 --- a/internal/app/context_factory.go +++ b/internal/app/context_factory.go @@ -3,6 +3,7 @@ package app import ( "errors" "fmt" + "net/http" "os" "strings" @@ -17,6 +18,9 @@ const ( engineWeb engineName = "web" engineAuto engineName = "auto" engineAppleScript engineName = "applescript" + + authCookies = "cookies" + authOAuth = "oauth" ) func (c *Context) Spotify() (spotify.API, error) { @@ -41,7 +45,11 @@ func (c *Context) Spotify() (spotify.API, error) { func (c *Context) buildSpotifyClient(source cookies.Source) (spotify.API, error) { switch c.engine() { case engineConnect: - return c.newConnectClient(source) + webClient, err := c.newWebClient(source) + if err != nil { + return nil, err + } + return c.newConnectClient(source, webClient) case engineWeb: return c.newPlaybackFallbackClient(source) case engineAuto: @@ -59,7 +67,7 @@ func (c *Context) newPlaybackFallbackClient(source cookies.Source) (spotify.API, return nil, err } client := spotify.API(webClient) - if connectClient, connectErr := c.newConnectClient(source); connectErr == nil { + if connectClient, connectErr := c.newConnectClient(source, webClient); connectErr == nil { client = spotify.NewPlaybackFallbackClient(webClient, connectClient) } return client, nil @@ -71,7 +79,7 @@ func (c *Context) newAutoClient(source cookies.Source) (spotify.API, error) { return nil, err } client := spotify.API(webClient) - if connectClient, connectErr := c.newConnectClient(source); connectErr == nil { + if connectClient, connectErr := c.newConnectClient(source, webClient); connectErr == nil { if localClient, localErr := spotify.NewAppleScriptClient(spotify.AppleScriptOptions{}); localErr == nil { client = spotify.NewAutoClient(connectClient, webClient, localClient) } else { @@ -85,14 +93,14 @@ func (c *Context) newAppleScriptClient(source cookies.Source) (spotify.API, erro var fallback spotify.API if webClient, webErr := c.newWebClient(source); webErr == nil { fallback = webClient - if connectClient, connectErr := c.newConnectClient(source); connectErr == nil { + if connectClient, connectErr := c.newConnectClient(source, webClient); connectErr == nil { fallback = spotify.NewPlaybackFallbackClient(webClient, connectClient) } } return spotify.NewAppleScriptClient(spotify.AppleScriptOptions{Fallback: fallback}) } -func (c *Context) newConnectClient(source cookies.Source) (*spotify.ConnectClient, error) { +func (c *Context) newConnectClient(source cookies.Source, webClient *spotify.Client) (*spotify.ConnectClient, error) { return spotify.NewConnectClient(spotify.ConnectOptions{ Source: source, Market: c.Profile.Market, @@ -100,12 +108,31 @@ func (c *Context) newConnectClient(source cookies.Source) (*spotify.ConnectClien Device: c.Profile.Device, Timeout: c.Settings.Timeout, CachePath: c.ResolveCachePath(), + WebClient: webClient, }) } func (c *Context) newWebClient(source cookies.Source) (*spotify.Client, error) { + var provider spotify.TokenProvider + switch c.auth() { + case authCookies: + provider = spotify.CookieTokenProvider{Source: source, Timeout: c.Settings.Timeout} + case authOAuth: + oauthProvider, err := spotify.NewOAuthTokenProvider(spotify.OAuthOptions{ + ClientID: c.Profile.SpotifyClientID, + RedirectURI: c.Profile.SpotifyRedirectURI, + CachePath: c.ResolveOAuthTokenPath(), + HTTPClient: &http.Client{Timeout: c.EnsureTimeout()}, + }) + if err != nil { + return nil, err + } + provider = oauthProvider + default: + return nil, fmt.Errorf("unknown auth %q (use cookies or oauth)", c.auth()) + } return spotify.NewClient(spotify.Options{ - TokenProvider: spotify.CookieTokenProvider{Source: source, Timeout: c.Settings.Timeout}, + TokenProvider: provider, Market: c.Profile.Market, Language: c.Profile.Language, Device: c.Profile.Device, @@ -121,6 +148,14 @@ func (c *Context) engine() engineName { return engine } +func (c *Context) auth() string { + auth := strings.ToLower(strings.TrimSpace(c.Profile.Auth)) + if auth == "" { + return authCookies + } + return auth +} + func (c *Context) cookieSource() (cookies.Source, error) { if c.Profile.CookiePath != "" { return cookies.FileSource{Path: c.Profile.CookiePath}, nil diff --git a/internal/app/context_init.go b/internal/app/context_init.go index 3576f7a..c2ef857 100644 --- a/internal/app/context_init.go +++ b/internal/app/context_init.go @@ -62,5 +62,14 @@ func applySettings(profile config.Profile, settings Settings) config.Profile { if settings.Engine != "" { profile.Engine = settings.Engine } + if settings.Auth != "" { + profile.Auth = settings.Auth + } + if settings.SpotifyClientID != "" { + profile.SpotifyClientID = settings.SpotifyClientID + } + if settings.SpotifyRedirectURI != "" { + profile.SpotifyRedirectURI = settings.SpotifyRedirectURI + } return profile } diff --git a/internal/app/context_test.go b/internal/app/context_test.go index af65604..d8276d0 100644 --- a/internal/app/context_test.go +++ b/internal/app/context_test.go @@ -42,6 +42,14 @@ func TestResolveCookiePath(t *testing.T) { } } +func TestResolveOAuthTokenPath(t *testing.T) { + ctx := &Context{ConfigPath: "/tmp/spogo/config.toml", ProfileKey: "work"} + path := ctx.ResolveOAuthTokenPath() + if filepath.Base(path) != "work.json" || filepath.Base(filepath.Dir(path)) != "oauth" { + t.Fatalf("oauth token path: %s", path) + } +} + func TestClearCache(t *testing.T) { dir := t.TempDir() ctx := &Context{ConfigPath: filepath.Join(dir, "config.toml"), ProfileKey: "default"} @@ -198,6 +206,32 @@ func TestSpotifyUnknownEngine(t *testing.T) { } } +func TestSpotifyOAuthWebEngine(t *testing.T) { + ctx := &Context{ + ConfigPath: "/tmp/spogo/config.toml", + ProfileKey: "default", + Profile: config.Profile{ + Engine: "web", + Auth: "oauth", + SpotifyClientID: "client-id", + }, + } + client, err := ctx.Spotify() + if err != nil { + t.Fatalf("spotify: %v", err) + } + if client == nil { + t.Fatalf("expected oauth web client") + } +} + +func TestSpotifyUnknownAuth(t *testing.T) { + ctx := &Context{Profile: config.Profile{CookiePath: "/tmp/cookies.json", Engine: "web", Auth: "nope"}} + if _, err := ctx.Spotify(); err == nil { + t.Fatalf("expected error") + } +} + func TestIsColorEnabled(t *testing.T) { if isColorEnabled(output.FormatJSON, false) { t.Fatalf("expected false") @@ -285,23 +319,29 @@ func TestNewContextAppliesRequestedProfileAndSettings(t *testing.T) { path := filepath.Join(dir, "config.toml") cfg := config.Default() cfg.SetProfile("work", config.Profile{ - Market: "US", - Language: "en", - Device: "speaker", - Engine: "web", + Market: "US", + Language: "en", + Device: "speaker", + Engine: "web", + Auth: "cookies", + SpotifyClientID: "stored-client", + SpotifyRedirectURI: "http://127.0.0.1:8888/callback", }) if err := config.Save(path, cfg); err != nil { t.Fatalf("save: %v", err) } ctx, err := NewContext(Settings{ - ConfigPath: path, - Profile: "work", - Market: "DE", - Language: "de", - Device: "desktop", - Engine: "auto", - Format: output.FormatPlain, + ConfigPath: path, + Profile: "work", + Market: "DE", + Language: "de", + Device: "desktop", + Engine: "auto", + Auth: "oauth", + SpotifyClientID: "override-client", + SpotifyRedirectURI: "http://[::1]:9999/callback", + Format: output.FormatPlain, }) if err != nil { t.Fatalf("new context: %v", err) @@ -309,7 +349,7 @@ func TestNewContextAppliesRequestedProfileAndSettings(t *testing.T) { if ctx.ProfileKey != "work" { t.Fatalf("profile key: %s", ctx.ProfileKey) } - if ctx.Profile.Market != "DE" || ctx.Profile.Language != "de" || ctx.Profile.Device != "desktop" || ctx.Profile.Engine != "auto" { + if ctx.Profile.Market != "DE" || ctx.Profile.Language != "de" || ctx.Profile.Device != "desktop" || ctx.Profile.Engine != "auto" || ctx.Profile.Auth != "oauth" || ctx.Profile.SpotifyClientID != "override-client" || ctx.Profile.SpotifyRedirectURI != "http://[::1]:9999/callback" { t.Fatalf("profile overrides not applied: %+v", ctx.Profile) } } @@ -328,13 +368,16 @@ func TestResolveProfileKeyFallbacks(t *testing.T) { func TestApplySettingsKeepsEmptyValues(t *testing.T) { profile := applySettings(config.Profile{ - Market: "US", - Language: "en", - Device: "speaker", - Engine: "web", + Market: "US", + Language: "en", + Device: "speaker", + Engine: "web", + Auth: "oauth", + SpotifyClientID: "client-id", + SpotifyRedirectURI: "http://127.0.0.1:8888/callback", }, Settings{}) - if profile.Market != "US" || profile.Language != "en" || profile.Device != "speaker" || profile.Engine != "web" { + if profile.Market != "US" || profile.Language != "en" || profile.Device != "speaker" || profile.Engine != "web" || profile.Auth != "oauth" || profile.SpotifyClientID != "client-id" || profile.SpotifyRedirectURI == "" { t.Fatalf("profile changed unexpectedly: %+v", profile) } } diff --git a/internal/app/errors.go b/internal/app/errors.go index e7f7071..e15a1db 100644 --- a/internal/app/errors.go +++ b/internal/app/errors.go @@ -47,7 +47,7 @@ func ExitCode(err error) int { if errors.As(err, &parseErr) { return 2 } - if errors.Is(err, cookies.ErrNoCookies) { + if errors.Is(err, cookies.ErrNoCookies) || errors.Is(err, spotify.ErrOAuthAuthentication) { return 3 } var apiErr spotify.APIError diff --git a/internal/app/errors_test.go b/internal/app/errors_test.go index b92d2b0..6f8fcf4 100644 --- a/internal/app/errors_test.go +++ b/internal/app/errors_test.go @@ -49,6 +49,9 @@ func TestExitCode(t *testing.T) { if ExitCode(cookies.ErrNoCookies) != 3 { t.Fatalf("expected 3") } + if ExitCode(spotify.ErrOAuthAuthentication) != 3 { + t.Fatalf("expected 3") + } if ExitCode(spotify.APIError{Status: 500}) != 1 { t.Fatalf("expected 1") } diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 1414b30..12a9ce9 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -1,10 +1,13 @@ package cli +import "time" + type AuthCmd struct { Status AuthStatusCmd `kong:"cmd,help='Show cookie status.'"` Import AuthImportCmd `kong:"cmd,help='Import browser cookies.'"` Paste AuthPasteCmd `kong:"cmd,help='Paste cookie values from the browser.'"` Clear AuthClearCmd `kong:"cmd,help='Clear stored cookies.'"` + OAuth AuthOAuthCmd `kong:"cmd,name='oauth',help='Official Spotify OAuth for Web API requests.'"` } type AuthStatusCmd struct{} @@ -24,6 +27,23 @@ type AuthPasteCmd struct { type AuthClearCmd struct{} +type AuthOAuthCmd struct { + Login AuthOAuthLoginCmd `kong:"cmd,help='Authorize with Spotify using Authorization Code with PKCE.'"` + Status AuthOAuthStatusCmd `kong:"cmd,help='Show local OAuth token status.'"` + Clear AuthOAuthClearCmd `kong:"cmd,help='Clear the local OAuth token cache.'"` +} + +type AuthOAuthLoginCmd struct { + ClientID string `name:"client-id" help:"Spotify application client ID."` + RedirectURI string `name:"redirect-uri" help:"Registered loopback redirect URI (default http://127.0.0.1:8888/callback)."` + NoOpen bool `name:"no-open" help:"Print the authorization URL without opening a browser."` + WaitTimeout time.Duration `name:"wait-timeout" help:"Maximum time to wait for the OAuth callback." default:"5m"` +} + +type AuthOAuthStatusCmd struct{} + +type AuthOAuthClearCmd struct{} + type authStatusPayload struct { CookieCount int `json:"cookie_count"` HasSPDC bool `json:"has_sp_dc"` diff --git a/internal/cli/auth_oauth.go b/internal/cli/auth_oauth.go new file mode 100644 index 0000000..d1cabed --- /dev/null +++ b/internal/cli/auth_oauth.go @@ -0,0 +1,287 @@ +package cli + +import ( + "context" + "crypto/subtle" + "errors" + "fmt" + "net" + "net/http" + "net/url" + "os" + "os/exec" + "runtime" + "strings" + "time" + + "github.com/steipete/spogo/internal/app" + "github.com/steipete/spogo/internal/spotify" +) + +type oauthCallback struct { + code string + err error +} + +type oauthStatusPayload struct { + Authenticated bool `json:"authenticated"` + Auth string `json:"auth"` + ClientID string `json:"client_id,omitempty"` + ExpiresAt string `json:"expires_at,omitempty"` + Expired bool `json:"expired"` + HasRefresh bool `json:"has_refresh_token"` + Scopes []string `json:"scopes,omitempty"` + TokenPath string `json:"token_path"` + FileMode string `json:"file_mode,omitempty"` +} + +const defaultOAuthRedirectURI = "http://127.0.0.1:8888/callback" + +var ( + openOAuthBrowser = openBrowserURL + newOAuthTokenProvider = spotify.NewOAuthTokenProvider +) + +func (cmd *AuthOAuthLoginCmd) Run(ctx *app.Context) error { + clientID := firstNonEmpty(cmd.ClientID, ctx.Profile.SpotifyClientID) + if clientID == "" { + return fmt.Errorf("%w: pass --client-id, --spotify-client-id, or SPOGO_SPOTIFY_CLIENT_ID", spotify.ErrOAuthAuthentication) + } + redirectURI := firstNonEmpty(cmd.RedirectURI, ctx.Profile.SpotifyRedirectURI, defaultOAuthRedirectURI) + if err := spotify.ValidateOAuthRedirectURI(redirectURI); err != nil { + return err + } + provider, err := newOAuthTokenProvider(spotify.OAuthOptions{ + ClientID: clientID, + RedirectURI: redirectURI, + CachePath: ctx.ResolveOAuthTokenPath(), + HTTPClient: &http.Client{Timeout: ctx.EnsureTimeout()}, + }) + if err != nil { + return err + } + verifier, challenge, err := spotify.GenerateOAuthPKCE() + if err != nil { + return err + } + state, err := spotify.GenerateOAuthState() + if err != nil { + return err + } + authorizationURL, err := provider.AuthorizationURL(state, challenge) + if err != nil { + return err + } + parsedRedirect, err := url.Parse(redirectURI) + if err != nil { + return err + } + listener, err := net.Listen("tcp", parsedRedirect.Host) + if err != nil { + return fmt.Errorf("listen for spotify oauth callback on %s: %w", parsedRedirect.Host, err) + } + defer func() { _ = listener.Close() }() + + callbackCh := make(chan oauthCallback, 1) + mux := http.NewServeMux() + callbackPath := parsedRedirect.Path + if callbackPath == "" { + callbackPath = "/" + } + mux.HandleFunc(callbackPath, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'") + w.Header().Set("X-Content-Type-Options", "nosniff") + if r.Host != parsedRedirect.Host { + http.Error(w, "invalid callback host", http.StatusBadRequest) + return + } + if r.Method != http.MethodGet { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + query := r.URL.Query() + if subtle.ConstantTimeCompare([]byte(query.Get("state")), []byte(state)) != 1 { + http.Error(w, "invalid oauth state", http.StatusBadRequest) + return + } + if oauthErr := query.Get("error"); oauthErr != "" { + http.Error(w, "Spotify authorization was not granted. You can close this window.", http.StatusBadRequest) + select { + case callbackCh <- oauthCallback{err: fmt.Errorf("%w: spotify authorization failed: %s", spotify.ErrOAuthAuthentication, oauthErr)}: + default: + } + return + } + code := query.Get("code") + if code == "" { + http.Error(w, "missing authorization code", http.StatusBadRequest) + select { + case callbackCh <- oauthCallback{err: fmt.Errorf("%w: callback is missing the authorization code", spotify.ErrOAuthAuthentication)}: + default: + } + return + } + w.Header().Set("Content-Type", "text/html; charset=utf-8") + _, _ = fmt.Fprint(w, "spogo authorized

Spotify authorization complete. You can close this window.

") + select { + case callbackCh <- oauthCallback{code: code}: + default: + } + }) + server := &http.Server{ + Handler: mux, + ReadHeaderTimeout: 5 * time.Second, + } + serveErrCh := make(chan error, 1) + go func() { + if serveErr := server.Serve(listener); serveErr != nil && !errors.Is(serveErr, http.ErrServerClosed) { + serveErrCh <- serveErr + } + }() + defer func() { _ = server.Close() }() + + ctx.Output.Errorf("Spotify authorization URL: %s", authorizationURL) + if !cmd.NoOpen { + if err := openOAuthBrowser(authorizationURL); err != nil { + ctx.Output.Errorf("Could not open a browser: %v", err) + } + } + + waitTimeout := cmd.WaitTimeout + if waitTimeout <= 0 { + waitTimeout = 5 * time.Minute + } + waitCtx, cancel := context.WithTimeout(ctx.CommandContext(), waitTimeout) + defer cancel() + var callback oauthCallback + select { + case callback = <-callbackCh: + case serveErr := <-serveErrCh: + return fmt.Errorf("spotify oauth callback server: %w", serveErr) + case <-waitCtx.Done(): + return fmt.Errorf("spotify oauth callback wait failed: %w", waitCtx.Err()) + } + if callback.err != nil { + return callback.err + } + if _, err := provider.ExchangeCode(ctx.CommandContext(), callback.code, verifier); err != nil { + return err + } + profile := ctx.Profile + profile.Auth = "oauth" + profile.SpotifyClientID = clientID + profile.SpotifyRedirectURI = redirectURI + if err := ctx.SaveProfile(profile); err != nil { + return fmt.Errorf("oauth token saved but profile update failed: %w", err) + } + payload := map[string]any{ + "status": "ok", + "auth": "oauth", + "client_id": clientID, + "redirect_uri": redirectURI, + "token_path": ctx.ResolveOAuthTokenPath(), + } + return ctx.Output.Emit(payload, []string{"ok\toauth"}, []string{ + "Spotify OAuth login complete.", + "Use --engine web for OAuth without browser cookies; Connect still requires cookies.", + }) +} + +func (cmd *AuthOAuthStatusCmd) Run(ctx *app.Context) error { + status, err := spotify.OAuthStatus(ctx.ResolveOAuthTokenPath()) + if err != nil { + return fmt.Errorf("%w: invalid oauth token cache: %w", spotify.ErrOAuthAuthentication, err) + } + effectiveClientID := firstNonEmpty(ctx.Profile.SpotifyClientID, status.ClientID) + if status.Exists && status.ClientID != "" && effectiveClientID != "" && status.ClientID != effectiveClientID { + return fmt.Errorf("%w: cached token belongs to a different Spotify client ID", spotify.ErrOAuthAuthentication) + } + payload := oauthStatusPayload{ + Authenticated: status.Exists, + Auth: selectedAuth(ctx.Profile.Auth), + ClientID: effectiveClientID, + Expired: status.Expired, + HasRefresh: status.HasRefresh, + Scopes: status.Scopes, + TokenPath: ctx.ResolveOAuthTokenPath(), + } + if !status.ExpiresAt.IsZero() { + payload.ExpiresAt = status.ExpiresAt.UTC().Format(time.RFC3339) + } + if status.Exists { + payload.FileMode = fmt.Sprintf("%04o", status.FileMode.Perm()) + } + plain := []string{fmt.Sprintf("%t\t%s\t%s\t%t", payload.Authenticated, payload.Auth, payload.ExpiresAt, payload.HasRefresh)} + human := []string{fmt.Sprintf("OAuth: %s", enabledLabel(payload.Authenticated))} + if payload.Authenticated { + human = append( + human, + fmt.Sprintf("Client ID: %s", payload.ClientID), + fmt.Sprintf("Access token expires: %s (expired: %t)", payload.ExpiresAt, payload.Expired), + fmt.Sprintf("Refresh token: %t", payload.HasRefresh), + fmt.Sprintf("Token cache permissions: %s", payload.FileMode), + ) + } + return ctx.Output.Emit(payload, plain, human) +} + +func (cmd *AuthOAuthClearCmd) Run(ctx *app.Context) error { + path := ctx.ResolveOAuthTokenPath() + if err := spotify.ClearOAuthToken(path); err != nil { + return err + } + profile := ctx.Profile + if selectedAuth(profile.Auth) == "oauth" { + profile.Auth = "" + if err := ctx.SaveProfile(profile); err != nil { + return err + } + } + payload := map[string]string{"status": "ok", "token_path": path} + return ctx.Output.Emit(payload, []string{"ok"}, []string{"Cleared Spotify OAuth token cache."}) +} + +func selectedAuth(auth string) string { + auth = strings.ToLower(strings.TrimSpace(auth)) + if auth == "" { + return "cookies" + } + return auth +} + +func enabledLabel(enabled bool) string { + if enabled { + return "authenticated" + } + return "not authenticated" +} + +func firstNonEmpty(values ...string) string { + for _, value := range values { + if value = strings.TrimSpace(value); value != "" { + return value + } + } + return "" +} + +func openBrowserURL(rawURL string) error { + var command string + var args []string + switch runtime.GOOS { + case "darwin": + command = "open" + args = []string{rawURL} + case "windows": + command = "rundll32" + args = []string{"url.dll,FileProtocolHandler", rawURL} + default: + command = "xdg-open" + args = []string{rawURL} + } + cmd := exec.Command(command, args...) + cmd.Stdout = os.Stderr + cmd.Stderr = os.Stderr + return cmd.Start() +} diff --git a/internal/cli/auth_oauth_test.go b/internal/cli/auth_oauth_test.go new file mode 100644 index 0000000..3b59308 --- /dev/null +++ b/internal/cli/auth_oauth_test.go @@ -0,0 +1,435 @@ +package cli + +import ( + "encoding/json" + "errors" + "net" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/steipete/spogo/internal/config" + "github.com/steipete/spogo/internal/output" + "github.com/steipete/spogo/internal/spotify" + "github.com/steipete/spogo/internal/testutil" +) + +func TestAuthOAuthLoginCmd(t *testing.T) { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + t.Errorf("parse form: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + if r.Form.Get("grant_type") != "authorization_code" || r.Form.Get("code") != "test-code" { + t.Errorf("unexpected token form: %v", r.Form) + w.WriteHeader(http.StatusBadRequest) + return + } + if r.Header.Get("Authorization") != "" { + t.Errorf("PKCE exchange sent Authorization header") + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "access", + "refresh_token": "refresh", + "token_type": "Bearer", + "scope": "user-library-read", + "expires_in": 3600, + }) + })) + defer tokenServer.Close() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("reserve port: %v", err) + } + redirectURI := "http://" + listener.Addr().String() + "/callback" + _ = listener.Close() + + oldProvider := newOAuthTokenProvider + newOAuthTokenProvider = func(opts spotify.OAuthOptions) (*spotify.OAuthTokenProvider, error) { + opts.AccountsURL = tokenServer.URL + return spotify.NewOAuthTokenProvider(opts) + } + t.Cleanup(func() { newOAuthTokenProvider = oldProvider }) + + callbackErr := make(chan error, 1) + oldOpen := openOAuthBrowser + openOAuthBrowser = func(raw string) error { + parsed, err := url.Parse(raw) + if err != nil { + return err + } + state := parsed.Query().Get("state") + go func() { + callbackURL := redirectURI + "?code=test-code&state=" + url.QueryEscape(state) + var lastErr error + for range 50 { + resp, getErr := http.Get(callbackURL) //nolint:gosec // loopback test callback + if getErr == nil { + _ = resp.Body.Close() + callbackErr <- nil + return + } + lastErr = getErr + time.Sleep(10 * time.Millisecond) + } + callbackErr <- lastErr + }() + return nil + } + t.Cleanup(func() { openOAuthBrowser = oldOpen }) + + ctx, out, _ := testutil.NewTestContext(t, output.FormatJSON) + ctx.Config = config.Default() + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + cmd := AuthOAuthLoginCmd{ + ClientID: "client-id", + RedirectURI: redirectURI, + WaitTimeout: 2 * time.Second, + } + if err := cmd.Run(ctx); err != nil { + t.Fatalf("login: %v", err) + } + if err := <-callbackErr; err != nil { + t.Fatalf("callback: %v", err) + } + if ctx.Profile.Auth != "oauth" || ctx.Profile.SpotifyClientID != "client-id" { + t.Fatalf("profile not updated: %+v", ctx.Profile) + } + if _, err := spotify.LoadOAuthToken(ctx.ResolveOAuthTokenPath()); err != nil { + t.Fatalf("load cached token: %v", err) + } + var loginPayload map[string]any + if err := json.Unmarshal(out.Bytes(), &loginPayload); err != nil { + t.Fatalf("decode output: %v", err) + } + if loginPayload["status"] != "ok" { + t.Fatalf("unexpected output: %s", out.String()) + } +} + +func TestAuthOAuthStatusAndClearCmd(t *testing.T) { + ctx, out, _ := testutil.NewTestContext(t, output.FormatJSON) + ctx.Config = config.Default() + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + ctx.Profile = config.Profile{Auth: "oauth", SpotifyClientID: "client-id"} + if err := spotify.SaveOAuthToken(ctx.ResolveOAuthTokenPath(), spotify.OAuthToken{ + AccessToken: "access", + RefreshToken: "refresh", + Scope: "scope-a scope-b", + ExpiresAt: time.Date(2026, 8, 27, 21, 0, 0, 0, time.UTC), + ClientID: "client-id", + }); err != nil { + t.Fatalf("save token: %v", err) + } + if err := (&AuthOAuthStatusCmd{}).Run(ctx); err != nil { + t.Fatalf("status: %v", err) + } + var statusPayload map[string]any + if err := json.Unmarshal(out.Bytes(), &statusPayload); err != nil { + t.Fatalf("decode status: %v", err) + } + if statusPayload["authenticated"] != true { + t.Fatalf("status omitted authentication: %s", out.String()) + } + if _, ok := statusPayload["refresh_token"]; ok { + t.Fatalf("status leaked refresh token: %s", out.String()) + } + if _, ok := statusPayload["access_token"]; ok { + t.Fatalf("status leaked access token: %s", out.String()) + } + out.Reset() + if err := (&AuthOAuthClearCmd{}).Run(ctx); err != nil { + t.Fatalf("clear: %v", err) + } + if _, err := os.Stat(ctx.ResolveOAuthTokenPath()); !os.IsNotExist(err) { + t.Fatalf("expected token cache removed, got %v", err) + } + if ctx.Profile.Auth != "" { + t.Fatalf("expected cookie auth restored, got %q", ctx.Profile.Auth) + } +} + +func TestAuthOAuthStatusRejectsClientIDMismatch(t *testing.T) { + ctx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + ctx.Profile = config.Profile{Auth: "oauth", SpotifyClientID: "configured-client"} + if err := spotify.SaveOAuthToken(ctx.ResolveOAuthTokenPath(), spotify.OAuthToken{ + AccessToken: "access", + RefreshToken: "refresh", + ExpiresAt: time.Now().Add(time.Hour), + ClientID: "other-client", + }); err != nil { + t.Fatalf("save token: %v", err) + } + err := (&AuthOAuthStatusCmd{}).Run(ctx) + if !errors.Is(err, spotify.ErrOAuthAuthentication) { + t.Fatalf("expected OAuth authentication error, got %v", err) + } +} + +func TestAuthOAuthStatusRejectsInvalidCache(t *testing.T) { + ctx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + path := ctx.ResolveOAuthTokenPath() + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(path, []byte(`{`), 0o600); err != nil { + t.Fatalf("write invalid cache: %v", err) + } + err := (&AuthOAuthStatusCmd{}).Run(ctx) + if !errors.Is(err, spotify.ErrOAuthAuthentication) { + t.Fatalf("expected OAuth authentication error, got %v", err) + } +} + +func TestAuthOAuthLoginRequiresClientID(t *testing.T) { + ctx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + err := (&AuthOAuthLoginCmd{RedirectURI: defaultOAuthRedirectURI}).Run(ctx) + if !errors.Is(err, spotify.ErrOAuthAuthentication) { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestAuthOAuthStatusMissing(t *testing.T) { + ctx, out, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + if err := (&AuthOAuthStatusCmd{}).Run(ctx); err != nil { + t.Fatalf("status: %v", err) + } + if got := strings.TrimSpace(out.String()); got != "false\tcookies\t\tfalse" { + t.Fatalf("status output = %q", got) + } +} + +func TestAuthOAuthClearMissingKeepsCookieSelection(t *testing.T) { + ctx, out, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + ctx.Profile = config.Profile{Auth: "cookies"} + if err := (&AuthOAuthClearCmd{}).Run(ctx); err != nil { + t.Fatalf("clear missing: %v", err) + } + if ctx.Profile.Auth != "cookies" || strings.TrimSpace(out.String()) != "ok" { + t.Fatalf("unexpected clear result: profile=%+v output=%q", ctx.Profile, out.String()) + } +} + +func TestAuthOAuthLoginTimeoutAndBrowserError(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("reserve port: %v", err) + } + redirectURI := "http://" + listener.Addr().String() + "/callback" + _ = listener.Close() + + oldOpen := openOAuthBrowser + openOAuthBrowser = func(string) error { return errors.New("browser unavailable") } + t.Cleanup(func() { openOAuthBrowser = oldOpen }) + + ctx, _, errOut := testutil.NewTestContext(t, output.FormatPlain) + ctx.Config = config.Default() + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + err = (&AuthOAuthLoginCmd{ + ClientID: "client-id", + RedirectURI: redirectURI, + WaitTimeout: 20 * time.Millisecond, + }).Run(ctx) + if err == nil || !strings.Contains(err.Error(), "deadline exceeded") { + t.Fatalf("expected callback timeout, got %v", err) + } + if !strings.Contains(errOut.String(), "browser unavailable") { + t.Fatalf("expected browser warning: %s", errOut.String()) + } +} + +func TestAuthOAuthLoginNoOpenTimeout(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("reserve port: %v", err) + } + redirectURI := "http://" + listener.Addr().String() + "/callback" + _ = listener.Close() + + oldOpen := openOAuthBrowser + openOAuthBrowser = func(string) error { + t.Fatal("browser opener must not run with --no-open") + return nil + } + t.Cleanup(func() { openOAuthBrowser = oldOpen }) + + ctx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + err = (&AuthOAuthLoginCmd{ + ClientID: "client-id", + RedirectURI: redirectURI, + NoOpen: true, + WaitTimeout: 20 * time.Millisecond, + }).Run(ctx) + if err == nil || !strings.Contains(err.Error(), "deadline exceeded") { + t.Fatalf("expected callback timeout, got %v", err) + } +} + +func TestAuthOAuthLoginRejectsDeniedCallback(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("reserve port: %v", err) + } + redirectURI := "http://" + listener.Addr().String() + "/callback" + _ = listener.Close() + + oldOpen := openOAuthBrowser + openOAuthBrowser = func(raw string) error { + parsed, err := url.Parse(raw) + if err != nil { + return err + } + state := parsed.Query().Get("state") + go func() { + callbackURL := redirectURI + "?error=access_denied&state=" + url.QueryEscape(state) + for range 50 { + resp, getErr := http.Get(callbackURL) //nolint:gosec // loopback test callback + if getErr == nil { + _ = resp.Body.Close() + return + } + time.Sleep(10 * time.Millisecond) + } + }() + return nil + } + t.Cleanup(func() { openOAuthBrowser = oldOpen }) + + ctx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + err = (&AuthOAuthLoginCmd{ + ClientID: "client-id", + RedirectURI: redirectURI, + WaitTimeout: time.Second, + }).Run(ctx) + if err == nil || !strings.Contains(err.Error(), "access_denied") || !errors.Is(err, spotify.ErrOAuthAuthentication) { + t.Fatalf("expected denial authentication error, got %v", err) + } +} + +func TestAuthOAuthLoginRedirectAndListenerValidation(t *testing.T) { + ctx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + if err := (&AuthOAuthLoginCmd{ClientID: "client-id", RedirectURI: "http://localhost:8888/callback"}).Run(ctx); err == nil { + t.Fatalf("expected invalid redirect error") + } + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer func() { _ = listener.Close() }() + redirectURI := "http://" + listener.Addr().String() + "/callback" + if err := (&AuthOAuthLoginCmd{ClientID: "client-id", RedirectURI: redirectURI}).Run(ctx); err == nil || !strings.Contains(err.Error(), "listen for spotify oauth callback") { + t.Fatalf("expected listener error, got %v", err) + } +} + +func TestOpenBrowserURLLinux(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("Linux xdg-open command test") + } + dir := t.TempDir() + marker := filepath.Join(dir, "opened") + script := filepath.Join(dir, "xdg-open") + contents := "#!/bin/sh\nprintf '%s' \"$1\" > \"" + marker + "\"\n" + if err := os.WriteFile(script, []byte(contents), 0o755); err != nil { + t.Fatalf("write xdg-open: %v", err) + } + t.Setenv("PATH", dir) + if err := openBrowserURL("https://example.test/authorize"); err != nil { + t.Fatalf("open browser: %v", err) + } + for range 50 { + data, err := os.ReadFile(marker) + if err == nil { + if string(data) != "https://example.test/authorize" { + t.Fatalf("opened URL = %q", data) + } + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("xdg-open helper did not run") +} + +func TestAuthOAuthLoginRejectsMalformedCallback(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("reserve port: %v", err) + } + redirectURI := "http://" + listener.Addr().String() + "/callback" + _ = listener.Close() + + oldOpen := openOAuthBrowser + openOAuthBrowser = func(raw string) error { + parsed, err := url.Parse(raw) + if err != nil { + return err + } + state := parsed.Query().Get("state") + go func() { + callbackURL := redirectURI + "?state=" + url.QueryEscape(state) + for range 50 { + wrongStateURL := redirectURI + "?state=wrong" + wrongResp, wrongErr := http.Get(wrongStateURL) //nolint:gosec // loopback test callback + if wrongErr == nil { + _ = wrongResp.Body.Close() + } + req, reqErr := http.NewRequest(http.MethodPost, callbackURL, nil) + if reqErr != nil { + return + } + resp, doErr := http.DefaultClient.Do(req) //nolint:gosec // loopback test callback + if doErr == nil { + _ = resp.Body.Close() + resp, doErr = http.Get(callbackURL) //nolint:gosec // loopback test callback + if doErr == nil { + _ = resp.Body.Close() + } + return + } + time.Sleep(10 * time.Millisecond) + } + }() + return nil + } + t.Cleanup(func() { openOAuthBrowser = oldOpen }) + + ctx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + err = (&AuthOAuthLoginCmd{ + ClientID: "client-id", + RedirectURI: redirectURI, + WaitTimeout: time.Second, + }).Run(ctx) + if err == nil || !strings.Contains(err.Error(), "missing the authorization code") { + t.Fatalf("expected missing code error, got %v", err) + } +} diff --git a/internal/cli/cli.go b/internal/cli/cli.go index cc08f3a..c99efca 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -44,21 +44,24 @@ type CLI struct { } type Globals struct { - Config string `help:"Config file path." env:"SPOGO_CONFIG"` - Profile string `help:"Profile name." env:"SPOGO_PROFILE"` - Timeout time.Duration `help:"HTTP timeout." env:"SPOGO_TIMEOUT" default:"10s"` - Market string `help:"Market country code." env:"SPOGO_MARKET"` - Language string `help:"Language/locale." env:"SPOGO_LANGUAGE"` - Device string `help:"Device name or id." env:"SPOGO_DEVICE"` - Engine string `help:"Engine (auto|web|connect|applescript)." env:"SPOGO_ENGINE"` - JSON bool `help:"JSON output." env:"SPOGO_JSON"` - Plain bool `help:"Plain output." env:"SPOGO_PLAIN"` - NoColor bool `help:"Disable color output." env:"SPOGO_NO_COLOR"` - Quiet bool `short:"q" help:"Quiet output." env:"SPOGO_QUIET"` - Verbose bool `short:"v" help:"Verbose output." env:"SPOGO_VERBOSE"` - Debug bool `short:"d" help:"Debug output." env:"SPOGO_DEBUG"` - NoInput bool `help:"Disable prompts." env:"SPOGO_NO_INPUT"` - Version kong.VersionFlag `help:"Print version."` + Config string `help:"Config file path." env:"SPOGO_CONFIG"` + Profile string `help:"Profile name." env:"SPOGO_PROFILE"` + Timeout time.Duration `help:"HTTP timeout." env:"SPOGO_TIMEOUT" default:"10s"` + Market string `help:"Market country code." env:"SPOGO_MARKET"` + Language string `help:"Language/locale." env:"SPOGO_LANGUAGE"` + Device string `help:"Device name or id." env:"SPOGO_DEVICE"` + Engine string `help:"Engine (auto|web|connect|applescript)." env:"SPOGO_ENGINE"` + Auth string `help:"Web API authentication (cookies|oauth)." env:"SPOGO_AUTH"` + SpotifyClientID string `name:"spotify-client-id" help:"Spotify application client ID." env:"SPOGO_SPOTIFY_CLIENT_ID"` + SpotifyRedirectURI string `name:"spotify-redirect-uri" help:"Spotify OAuth redirect URI." env:"SPOGO_SPOTIFY_REDIRECT_URI"` + JSON bool `help:"JSON output." env:"SPOGO_JSON"` + Plain bool `help:"Plain output." env:"SPOGO_PLAIN"` + NoColor bool `help:"Disable color output." env:"SPOGO_NO_COLOR"` + Quiet bool `short:"q" help:"Quiet output." env:"SPOGO_QUIET"` + Verbose bool `short:"v" help:"Verbose output." env:"SPOGO_VERBOSE"` + Debug bool `short:"d" help:"Debug output." env:"SPOGO_DEBUG"` + NoInput bool `help:"Disable prompts." env:"SPOGO_NO_INPUT"` + Version kong.VersionFlag `help:"Print version."` } func (g Globals) Settings() (app.Settings, error) { @@ -67,19 +70,22 @@ func (g Globals) Settings() (app.Settings, error) { return app.Settings{}, err } return app.Settings{ - ConfigPath: g.Config, - Profile: g.Profile, - Timeout: g.Timeout, - Market: g.Market, - Language: g.Language, - Device: g.Device, - Engine: g.Engine, - Format: format, - NoColor: g.NoColor, - Quiet: g.Quiet, - Verbose: g.Verbose, - Debug: g.Debug, - NoInput: g.NoInput, + ConfigPath: g.Config, + Profile: g.Profile, + Timeout: g.Timeout, + Market: g.Market, + Language: g.Language, + Device: g.Device, + Engine: g.Engine, + Auth: g.Auth, + SpotifyClientID: g.SpotifyClientID, + SpotifyRedirectURI: g.SpotifyRedirectURI, + Format: format, + NoColor: g.NoColor, + Quiet: g.Quiet, + Verbose: g.Verbose, + Debug: g.Debug, + NoInput: g.NoInput, }, nil } diff --git a/internal/config/config.go b/internal/config/config.go index ce24829..a88c913 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -19,13 +19,16 @@ type Config struct { } type Profile struct { - Browser string `toml:"browser"` - BrowserProfile string `toml:"browser_profile"` - CookiePath string `toml:"cookie_path"` - Market string `toml:"market"` - Language string `toml:"language"` - Device string `toml:"device"` - Engine string `toml:"engine"` + Browser string `toml:"browser"` + BrowserProfile string `toml:"browser_profile"` + CookiePath string `toml:"cookie_path"` + Auth string `toml:"auth"` + SpotifyClientID string `toml:"spotify_client_id"` + SpotifyRedirectURI string `toml:"spotify_redirect_uri"` + Market string `toml:"market"` + Language string `toml:"language"` + Device string `toml:"device"` + Engine string `toml:"engine"` } func DefaultPath() (string, error) { @@ -139,6 +142,17 @@ func CachePath(configPath, profile string) string { return filepath.Join(base, "cache", profile+".json") } +func OAuthTokenPath(configPath, profile string) string { + if profile == "" { + profile = DefaultProfile + } + if configPath == "" { + return "" + } + base := filepath.Dir(configPath) + return filepath.Join(base, "oauth", profile+".json") +} + func (c *Config) normalize() { if c.DefaultProfile == "" { c.DefaultProfile = DefaultProfile diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7cc0896..20cef61 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "strings" "testing" ) @@ -55,6 +56,37 @@ func TestSaveLoadRoundTrip(t *testing.T) { } } +func TestSaveLoadOAuthSettingsWithoutClientSecret(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + cfg := Default() + cfg.SetProfile("default", Profile{ + Auth: "oauth", + SpotifyClientID: "client-id", + SpotifyRedirectURI: "http://127.0.0.1:8888/callback", + }) + if err := Save(path, cfg); err != nil { + t.Fatalf("save: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read: %v", err) + } + if string(data) == "" || !strings.Contains(string(data), "spotify_client_id") { + t.Fatalf("oauth settings missing: %s", data) + } + if strings.Contains(strings.ToLower(string(data)), "client_secret") { + t.Fatalf("config must not contain a client secret field: %s", data) + } + loaded, err := Load(path) + if err != nil { + t.Fatalf("load: %v", err) + } + if got := loaded.Profile("default"); got.Auth != "oauth" || got.SpotifyClientID != "client-id" { + t.Fatalf("oauth profile mismatch: %+v", got) + } +} + func TestCookiePath(t *testing.T) { path := CookiePath("/tmp/spogo/config.toml", "default") if filepath.Base(path) != "default.json" { @@ -87,6 +119,16 @@ func TestCachePathEmptyConfig(t *testing.T) { } } +func TestOAuthTokenPath(t *testing.T) { + path := OAuthTokenPath("/tmp/spogo/config.toml", "work") + if filepath.Base(path) != "work.json" || filepath.Base(filepath.Dir(path)) != "oauth" { + t.Fatalf("oauth token path: %s", path) + } + if OAuthTokenPath("", "default") != "" { + t.Fatalf("expected empty oauth token path") + } +} + func TestLoadInvalid(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "bad.toml") diff --git a/internal/spotify/connect.go b/internal/spotify/connect.go index bd2d245..f27d89a 100644 --- a/internal/spotify/connect.go +++ b/internal/spotify/connect.go @@ -17,6 +17,7 @@ type ConnectOptions struct { Device string Timeout time.Duration CachePath string + WebClient *Client } type ConnectClient struct { @@ -64,6 +65,7 @@ func NewConnectClient(opts ConnectOptions) (*ConnectClient, error) { session: session, hashes: newHashResolver(httpClient, session), cache: cache, + web: opts.WebClient, }, nil } diff --git a/internal/spotify/connect_webclient_test.go b/internal/spotify/connect_webclient_test.go index e56c1af..066dcfc 100644 --- a/internal/spotify/connect_webclient_test.go +++ b/internal/spotify/connect_webclient_test.go @@ -23,3 +23,24 @@ func TestConnectWebClientCaches(t *testing.T) { t.Fatalf("expected cached web client") } } + +func TestConnectWebClientUsesInjectedClient(t *testing.T) { + web, err := NewClient(Options{TokenProvider: staticTokenProvider{}}) + if err != nil { + t.Fatalf("new web client: %v", err) + } + connect, err := NewConnectClient(ConnectOptions{ + Source: cookieSourceStub{cookies: []*http.Cookie{{Name: "sp_dc", Value: "cookie"}}}, + WebClient: web, + }) + if err != nil { + t.Fatalf("new connect client: %v", err) + } + got, err := connect.webClient() + if err != nil { + t.Fatalf("web client: %v", err) + } + if got != web { + t.Fatalf("expected injected Web API client") + } +} diff --git a/internal/spotify/oauth.go b/internal/spotify/oauth.go new file mode 100644 index 0000000..34a335f --- /dev/null +++ b/internal/spotify/oauth.go @@ -0,0 +1,528 @@ +package spotify + +import ( + "context" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "time" + + "github.com/gofrs/flock" +) + +const ( + defaultSpotifyAccountsURL = "https://accounts.spotify.com" + oauthExpirySkew = time.Minute + oauthLockRetryDelay = 25 * time.Millisecond +) + +var ErrOAuthAuthentication = errors.New("spotify oauth authentication required") + +var DefaultOAuthScopes = []string{ + "playlist-modify-private", + "playlist-modify-public", + "playlist-read-collaborative", + "playlist-read-private", + "user-follow-modify", + "user-follow-read", + "user-library-modify", + "user-library-read", + "user-modify-playback-state", + "user-read-currently-playing", + "user-read-playback-state", + "user-read-private", + "user-read-recently-played", + "user-top-read", +} + +type OAuthToken struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + TokenType string `json:"token_type"` + Scope string `json:"scope"` + ExpiresAt time.Time `json:"expires_at"` + ClientID string `json:"client_id"` +} + +type OAuthTokenStatus struct { + Exists bool + ClientID string + Scopes []string + ExpiresAt time.Time + Expired bool + HasRefresh bool + FileMode os.FileMode +} + +type OAuthOptions struct { + ClientID string + RedirectURI string + Scopes []string + CachePath string + HTTPClient *http.Client + AccountsURL string + Now func() time.Time +} + +type OAuthTokenProvider struct { + opts OAuthOptions + mu sync.Mutex +} + +type oauthTokenResponse struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + TokenType string `json:"token_type"` + Scope string `json:"scope"` + ExpiresIn int `json:"expires_in"` + Error string `json:"error"` + Description string `json:"error_description"` +} + +func NewOAuthTokenProvider(opts OAuthOptions) (*OAuthTokenProvider, error) { + opts.ClientID = strings.TrimSpace(opts.ClientID) + if opts.ClientID == "" { + return nil, fmt.Errorf("%w: spotify client ID is required", ErrOAuthAuthentication) + } + if opts.CachePath == "" { + return nil, fmt.Errorf("%w: oauth token cache path is required", ErrOAuthAuthentication) + } + if opts.HTTPClient == nil { + opts.HTTPClient = &http.Client{Timeout: defaultHTTPClientTimeout} + } + httpClient := *opts.HTTPClient + if httpClient.CheckRedirect == nil { + httpClient.CheckRedirect = func(_ *http.Request, _ []*http.Request) error { + return http.ErrUseLastResponse + } + } + opts.HTTPClient = &httpClient + if opts.AccountsURL == "" { + opts.AccountsURL = defaultSpotifyAccountsURL + } + if opts.Now == nil { + opts.Now = time.Now + } + if len(opts.Scopes) == 0 { + opts.Scopes = append([]string(nil), DefaultOAuthScopes...) + } + return &OAuthTokenProvider{opts: opts}, nil +} + +func (p *OAuthTokenProvider) Token(ctx context.Context) (Token, error) { + p.mu.Lock() + defer p.mu.Unlock() + cacheLock, err := acquireOAuthCacheLock(ctx, p.opts.CachePath) + if err != nil { + return Token{}, err + } + defer releaseOAuthCacheLock(cacheLock) + + cached, err := loadOAuthTokenUnlocked(p.opts.CachePath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return Token{}, fmt.Errorf("%w: run 'spogo auth oauth login'", ErrOAuthAuthentication) + } + return Token{}, fmt.Errorf("%w: invalid oauth token cache: %w", ErrOAuthAuthentication, err) + } + if cached.ClientID != "" && cached.ClientID != p.opts.ClientID { + return Token{}, fmt.Errorf("%w: cached token belongs to a different Spotify client ID", ErrOAuthAuthentication) + } + if cached.AccessToken != "" && cached.ExpiresAt.After(p.opts.Now().Add(oauthExpirySkew)) { + return oauthAPIToken(cached), nil + } + if cached.RefreshToken == "" { + return Token{}, fmt.Errorf("%w: cached token has no refresh token; run 'spogo auth oauth login'", ErrOAuthAuthentication) + } + refreshed, err := p.refresh(ctx, cached) + if err != nil { + return Token{}, err + } + if err := saveOAuthTokenUnlocked(p.opts.CachePath, refreshed); err != nil { + return Token{}, err + } + return oauthAPIToken(refreshed), nil +} + +func (p *OAuthTokenProvider) AuthorizationURL(state, codeChallenge string) (string, error) { + if state == "" || codeChallenge == "" { + return "", errors.New("oauth state and PKCE challenge are required") + } + if err := ValidateOAuthRedirectURI(p.opts.RedirectURI); err != nil { + return "", err + } + params := url.Values{ + "client_id": {p.opts.ClientID}, + "code_challenge": {codeChallenge}, + "code_challenge_method": {"S256"}, + "redirect_uri": {p.opts.RedirectURI}, + "response_type": {"code"}, + "scope": {strings.Join(p.opts.Scopes, " ")}, + "state": {state}, + } + return strings.TrimRight(p.opts.AccountsURL, "/") + "/authorize?" + params.Encode(), nil +} + +func (p *OAuthTokenProvider) ExchangeCode(ctx context.Context, code, verifier string) (OAuthToken, error) { + if strings.TrimSpace(code) == "" || strings.TrimSpace(verifier) == "" { + return OAuthToken{}, errors.New("authorization code and PKCE verifier are required") + } + form := url.Values{ + "client_id": {p.opts.ClientID}, + "code": {code}, + "code_verifier": {verifier}, + "grant_type": {"authorization_code"}, + "redirect_uri": {p.opts.RedirectURI}, + } + cacheLock, err := acquireOAuthCacheLock(ctx, p.opts.CachePath) + if err != nil { + return OAuthToken{}, err + } + defer releaseOAuthCacheLock(cacheLock) + response, err := p.requestToken(ctx, form) + if err != nil { + return OAuthToken{}, err + } + token := p.tokenFromResponse(response) + if token.RefreshToken == "" { + return OAuthToken{}, errors.New("spotify oauth response did not include a refresh token") + } + if err := saveOAuthTokenUnlocked(p.opts.CachePath, token); err != nil { + return OAuthToken{}, err + } + return token, nil +} + +func (p *OAuthTokenProvider) refresh(ctx context.Context, previous OAuthToken) (OAuthToken, error) { + form := url.Values{ + "client_id": {p.opts.ClientID}, + "grant_type": {"refresh_token"}, + "refresh_token": {previous.RefreshToken}, + } + response, err := p.requestToken(ctx, form) + if err != nil { + return OAuthToken{}, fmt.Errorf("token refresh failed: %w", err) + } + token := p.tokenFromResponse(response) + if token.RefreshToken == "" { + token.RefreshToken = previous.RefreshToken + } + if response.Scope == "" { + token.Scope = previous.Scope + } + if token.TokenType == "" { + token.TokenType = previous.TokenType + } + return token, nil +} + +func (p *OAuthTokenProvider) requestToken(ctx context.Context, form url.Values) (oauthTokenResponse, error) { + endpoint := strings.TrimRight(p.opts.AccountsURL, "/") + "/api/token" + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(form.Encode())) + if err != nil { + return oauthTokenResponse{}, err + } + req.Header.Set("Accept", "application/json") + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + resp, err := p.opts.HTTPClient.Do(req) + if err != nil { + return oauthTokenResponse{}, err + } + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return oauthTokenResponse{}, err + } + var payload oauthTokenResponse + if len(body) > 0 { + if err := json.Unmarshal(body, &payload); err != nil { + return oauthTokenResponse{}, fmt.Errorf("decode spotify oauth response: %w", err) + } + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + message := payload.Description + if message == "" { + message = payload.Error + } + if message == "" { + message = http.StatusText(resp.StatusCode) + } + if resp.StatusCode == http.StatusBadRequest || resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden { + return oauthTokenResponse{}, fmt.Errorf("%w: spotify oauth error (%d): %s", ErrOAuthAuthentication, resp.StatusCode, message) + } + return oauthTokenResponse{}, APIError{ + Status: resp.StatusCode, + Message: message, + Body: string(body), + RetryAfter: retryAfterFromResponse(resp), + } + } + if payload.AccessToken == "" || payload.ExpiresIn <= 0 { + return oauthTokenResponse{}, errors.New("spotify oauth response is missing access_token or expires_in") + } + return payload, nil +} + +func (p *OAuthTokenProvider) tokenFromResponse(response oauthTokenResponse) OAuthToken { + scope := response.Scope + if scope == "" { + scope = strings.Join(p.opts.Scopes, " ") + } + return OAuthToken{ + AccessToken: response.AccessToken, + RefreshToken: response.RefreshToken, + TokenType: response.TokenType, + Scope: scope, + ExpiresAt: p.opts.Now().Add(time.Duration(response.ExpiresIn) * time.Second), + ClientID: p.opts.ClientID, + } +} + +func oauthAPIToken(token OAuthToken) Token { + return Token{AccessToken: token.AccessToken, ExpiresAt: token.ExpiresAt, ClientID: token.ClientID} +} + +func GenerateOAuthPKCE() (verifier, challenge string, err error) { + verifier, err = randomURLSafe(64) + if err != nil { + return "", "", err + } + digest := sha256.Sum256([]byte(verifier)) + return verifier, base64.RawURLEncoding.EncodeToString(digest[:]), nil +} + +func GenerateOAuthState() (string, error) { + return randomURLSafe(32) +} + +func randomURLSafe(size int) (string, error) { + buf := make([]byte, size) + if _, err := rand.Read(buf); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(buf), nil +} + +func ValidateOAuthRedirectURI(raw string) error { + parsed, err := url.Parse(raw) + if err != nil { + return fmt.Errorf("invalid spotify redirect URI: %w", err) + } + if parsed.Scheme != "http" { + return errors.New("spotify CLI redirect URI must use http on a loopback IP") + } + host := parsed.Hostname() + if host != "127.0.0.1" && host != "::1" { + return errors.New("spotify CLI redirect URI must use 127.0.0.1 or [::1], not localhost or a non-loopback host") + } + if parsed.Port() == "" { + return errors.New("spotify CLI redirect URI must include a port") + } + if parsed.RawQuery != "" || parsed.Fragment != "" || parsed.User != nil { + return errors.New("spotify CLI redirect URI cannot include userinfo, a query, or a fragment") + } + return nil +} + +func LoadOAuthToken(path string) (OAuthToken, error) { + if _, err := os.Stat(path); err != nil { + return OAuthToken{}, err + } + cacheLock, err := acquireOAuthCacheLock(context.Background(), path) + if err != nil { + return OAuthToken{}, err + } + defer releaseOAuthCacheLock(cacheLock) + return loadOAuthTokenUnlocked(path) +} + +func loadOAuthTokenUnlocked(path string) (OAuthToken, error) { + info, err := os.Stat(path) + if err != nil { + return OAuthToken{}, err + } + if runtime.GOOS != "windows" { + dirInfo, err := os.Stat(filepath.Dir(path)) + if err != nil { + return OAuthToken{}, err + } + if dirInfo.Mode().Perm()&0o077 != 0 { + return OAuthToken{}, fmt.Errorf("oauth token cache directory permissions are %04o; require 0700", dirInfo.Mode().Perm()) + } + } + if !info.Mode().IsRegular() { + return OAuthToken{}, errors.New("oauth token cache is not a regular file") + } + if runtime.GOOS != "windows" && info.Mode().Perm()&0o077 != 0 { + return OAuthToken{}, fmt.Errorf("oauth token cache permissions are %04o; require 0600", info.Mode().Perm()) + } + data, err := os.ReadFile(path) + if err != nil { + return OAuthToken{}, err + } + var token OAuthToken + if err := json.Unmarshal(data, &token); err != nil { + return OAuthToken{}, fmt.Errorf("decode oauth token cache: %w", err) + } + if token.RefreshToken == "" && token.AccessToken == "" { + return OAuthToken{}, errors.New("oauth token cache contains no tokens") + } + return token, nil +} + +func SaveOAuthToken(path string, token OAuthToken) error { + cacheLock, err := acquireOAuthCacheLock(context.Background(), path) + if err != nil { + return err + } + defer releaseOAuthCacheLock(cacheLock) + return saveOAuthTokenUnlocked(path, token) +} + +func saveOAuthTokenUnlocked(path string, token OAuthToken) error { + if path == "" { + return errors.New("oauth token cache path is required") + } + if token.RefreshToken == "" { + return errors.New("refusing to cache oauth token without refresh token") + } + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return err + } + if runtime.GOOS != "windows" { + if err := os.Chmod(dir, 0o700); err != nil { + return err + } + } + data, err := json.MarshalIndent(token, "", " ") + if err != nil { + return err + } + data = append(data, '\n') + tmp, err := os.CreateTemp(dir, ".oauth-token-*") + if err != nil { + return err + } + tmpPath := tmp.Name() + committed := false + defer func() { + _ = tmp.Close() + if !committed { + _ = os.Remove(tmpPath) + } + }() + if err := tmp.Chmod(0o600); err != nil { + return err + } + if _, err := tmp.Write(data); err != nil { + return err + } + if err := tmp.Sync(); err != nil { + return err + } + if err := tmp.Close(); err != nil { + return err + } + if err := replaceOAuthTokenFile(tmpPath, path); err != nil { + return err + } + committed = true + return nil +} + +func OAuthStatus(path string) (OAuthTokenStatus, error) { + if _, err := os.Stat(path); err != nil { + if errors.Is(err, os.ErrNotExist) { + return OAuthTokenStatus{}, nil + } + return OAuthTokenStatus{}, err + } + cacheLock, err := acquireOAuthCacheLock(context.Background(), path) + if err != nil { + return OAuthTokenStatus{}, err + } + defer releaseOAuthCacheLock(cacheLock) + info, err := os.Stat(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return OAuthTokenStatus{}, nil + } + return OAuthTokenStatus{}, err + } + token, err := loadOAuthTokenUnlocked(path) + if err != nil { + return OAuthTokenStatus{}, err + } + scopes := strings.Fields(token.Scope) + return OAuthTokenStatus{ + Exists: true, + ClientID: token.ClientID, + Scopes: scopes, + ExpiresAt: token.ExpiresAt, + Expired: !token.ExpiresAt.IsZero() && !token.ExpiresAt.After(time.Now()), + HasRefresh: token.RefreshToken != "", + FileMode: info.Mode().Perm(), + }, nil +} + +func ClearOAuthToken(path string) error { + if path == "" { + return errors.New("oauth token cache path is required") + } + cacheLock, err := acquireOAuthCacheLock(context.Background(), path) + if err != nil { + return err + } + defer releaseOAuthCacheLock(cacheLock) + if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + return nil +} + +func acquireOAuthCacheLock(ctx context.Context, path string) (*flock.Flock, error) { + if path == "" { + return nil, errors.New("oauth token cache path is required") + } + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, err + } + cacheLock := flock.New(path+".lock", flock.SetPermissions(0o600)) + locked, err := cacheLock.TryLockContext(ctx, oauthLockRetryDelay) + if err != nil { + _ = cacheLock.Close() + return nil, fmt.Errorf("lock oauth token cache: %w", err) + } + if !locked { + _ = cacheLock.Close() + return nil, fmt.Errorf("lock oauth token cache: %w", ctx.Err()) + } + if runtime.GOOS != "windows" { + if err := os.Chmod(cacheLock.Path(), 0o600); err != nil { + releaseOAuthCacheLock(cacheLock) + return nil, err + } + } + return cacheLock, nil +} + +func releaseOAuthCacheLock(cacheLock *flock.Flock) { + if cacheLock == nil { + return + } + _ = cacheLock.Unlock() + _ = cacheLock.Close() +} diff --git a/internal/spotify/oauth_replace_unix.go b/internal/spotify/oauth_replace_unix.go new file mode 100644 index 0000000..dc851fa --- /dev/null +++ b/internal/spotify/oauth_replace_unix.go @@ -0,0 +1,9 @@ +//go:build !windows + +package spotify + +import "os" + +func replaceOAuthTokenFile(source, destination string) error { + return os.Rename(source, destination) +} diff --git a/internal/spotify/oauth_replace_windows.go b/internal/spotify/oauth_replace_windows.go new file mode 100644 index 0000000..32c56f6 --- /dev/null +++ b/internal/spotify/oauth_replace_windows.go @@ -0,0 +1,21 @@ +//go:build windows + +package spotify + +import "golang.org/x/sys/windows" + +func replaceOAuthTokenFile(source, destination string) error { + sourcePtr, err := windows.UTF16PtrFromString(source) + if err != nil { + return err + } + destinationPtr, err := windows.UTF16PtrFromString(destination) + if err != nil { + return err + } + return windows.MoveFileEx( + sourcePtr, + destinationPtr, + windows.MOVEFILE_REPLACE_EXISTING|windows.MOVEFILE_WRITE_THROUGH, + ) +} diff --git a/internal/spotify/oauth_test.go b/internal/spotify/oauth_test.go new file mode 100644 index 0000000..3276597 --- /dev/null +++ b/internal/spotify/oauth_test.go @@ -0,0 +1,683 @@ +package spotify + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestGenerateOAuthPKCE(t *testing.T) { + verifier, challenge, err := GenerateOAuthPKCE() + if err != nil { + t.Fatalf("generate PKCE: %v", err) + } + if len(verifier) < 43 || len(verifier) > 128 { + t.Fatalf("verifier length = %d", len(verifier)) + } + if verifier == challenge || strings.ContainsAny(verifier+challenge, "+/=") { + t.Fatalf("invalid URL-safe PKCE values") + } + second, _, err := GenerateOAuthPKCE() + if err != nil { + t.Fatalf("generate second PKCE: %v", err) + } + if second == verifier { + t.Fatalf("expected unique verifier") + } +} + +func TestOAuthAuthorizationURL(t *testing.T) { + provider, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + RedirectURI: "http://127.0.0.1:8888/callback", + CachePath: filepath.Join(t.TempDir(), "token.json"), + AccountsURL: "https://accounts.example", + Scopes: []string{"scope-b", "scope-a"}, + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + raw, err := provider.AuthorizationURL("state", "challenge") + if err != nil { + t.Fatalf("authorization URL: %v", err) + } + parsed, err := url.Parse(raw) + if err != nil { + t.Fatalf("parse: %v", err) + } + query := parsed.Query() + if parsed.Path != "/authorize" || query.Get("response_type") != "code" || query.Get("code_challenge_method") != "S256" { + t.Fatalf("unexpected authorization URL: %s", raw) + } + if query.Get("client_id") != "client-id" || query.Get("redirect_uri") != "http://127.0.0.1:8888/callback" { + t.Fatalf("missing client settings: %s", raw) + } + if query.Get("scope") != "scope-b scope-a" || query.Get("state") != "state" { + t.Fatalf("unexpected scope/state: %s", raw) + } +} + +func TestValidateOAuthRedirectURI(t *testing.T) { + valid := []string{ + "http://127.0.0.1:8888/callback", + "http://[::1]:8888/callback", + } + for _, raw := range valid { + if err := ValidateOAuthRedirectURI(raw); err != nil { + t.Errorf("ValidateOAuthRedirectURI(%q): %v", raw, err) + } + } + invalid := []string{ + "http://localhost:8888/callback", + "https://127.0.0.1:8888/callback", + "http://127.0.0.1/callback", + "http://192.168.1.2:8888/callback", + "http://127.0.0.1:8888/callback?token=x", + } + for _, raw := range invalid { + if err := ValidateOAuthRedirectURI(raw); err == nil { + t.Errorf("expected %q to be rejected", raw) + } + } +} + +func TestOAuthExchangeAndRefresh(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/token" || r.Method != http.MethodPost { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + if got := r.Header.Get("Authorization"); got != "" { + t.Fatalf("PKCE request must not send client secret authorization: %q", got) + } + if err := r.ParseForm(); err != nil { + t.Fatalf("parse form: %v", err) + } + switch requests.Add(1) { + case 1: + if r.Form.Get("grant_type") != "authorization_code" || r.Form.Get("code_verifier") != "verifier" { + t.Fatalf("unexpected exchange form: %v", r.Form) + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "access-one", + "refresh_token": "refresh-one", + "token_type": "Bearer", + "scope": "user-library-read", + "expires_in": 3600, + }) + case 2: + if r.Form.Get("grant_type") != "refresh_token" || r.Form.Get("refresh_token") != "refresh-one" { + t.Fatalf("unexpected refresh form: %v", r.Form) + } + if r.Form.Get("client_id") != "client-id" { + t.Fatalf("missing PKCE client ID: %v", r.Form) + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "access-two", + "token_type": "Bearer", + "expires_in": 3600, + }) + default: + t.Fatalf("unexpected token request") + } + })) + defer server.Close() + + now := time.Date(2026, 8, 27, 20, 0, 0, 0, time.UTC) + path := filepath.Join(t.TempDir(), "oauth", "default.json") + provider, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + RedirectURI: "http://127.0.0.1:8888/callback", + CachePath: path, + AccountsURL: server.URL, + Now: func() time.Time { return now }, + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + stored, err := provider.ExchangeCode(context.Background(), "code", "verifier") + if err != nil { + t.Fatalf("exchange: %v", err) + } + if stored.AccessToken != "access-one" || stored.RefreshToken != "refresh-one" { + t.Fatalf("unexpected exchange token: %+v", stored) + } + cached, err := provider.Token(context.Background()) + if err != nil { + t.Fatalf("cached token: %v", err) + } + if cached.AccessToken != "access-one" || requests.Load() != 1 { + t.Fatalf("expected cached access token, got %+v with %d requests", cached, requests.Load()) + } + + now = now.Add(2 * time.Hour) + refreshed, err := provider.Token(context.Background()) + if err != nil { + t.Fatalf("refresh: %v", err) + } + if refreshed.AccessToken != "access-two" || requests.Load() != 2 { + t.Fatalf("unexpected refreshed token: %+v with %d requests", refreshed, requests.Load()) + } + persisted, err := LoadOAuthToken(path) + if err != nil { + t.Fatalf("load refreshed: %v", err) + } + if persisted.RefreshToken != "refresh-one" { + t.Fatalf("refresh token rotation fallback failed: %+v", persisted) + } + if persisted.Scope != "user-library-read" { + t.Fatalf("refresh scope fallback failed: %+v", persisted) + } + if persisted.TokenType != "Bearer" { + t.Fatalf("refresh token type fallback failed: %+v", persisted) + } +} + +func TestOAuthRefreshIsLockedAcrossProviders(t *testing.T) { + var requests atomic.Int32 + releaseRefresh := make(chan struct{}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if requests.Add(1) != 1 { + t.Errorf("expected one refresh request") + } + <-releaseRefresh + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "fresh-access", + "refresh_token": "fresh-refresh", + "token_type": "Bearer", + "expires_in": 3600, + }) + })) + defer server.Close() + + path := filepath.Join(t.TempDir(), "oauth", "default.json") + if err := SaveOAuthToken(path, OAuthToken{ + AccessToken: "expired-access", + RefreshToken: "initial-refresh", + ExpiresAt: time.Now().Add(-time.Hour), + ClientID: "client-id", + }); err != nil { + t.Fatalf("save expired token: %v", err) + } + newProvider := func() *OAuthTokenProvider { + provider, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + CachePath: path, + AccountsURL: server.URL, + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + return provider + } + providers := []*OAuthTokenProvider{newProvider(), newProvider()} + results := make(chan Token, len(providers)) + errorsCh := make(chan error, len(providers)) + var started sync.WaitGroup + started.Add(len(providers)) + for _, provider := range providers { + go func(provider *OAuthTokenProvider) { + started.Done() + token, err := provider.Token(context.Background()) + if err != nil { + errorsCh <- err + return + } + results <- token + }(provider) + } + started.Wait() + close(releaseRefresh) + for range providers { + select { + case err := <-errorsCh: + t.Fatalf("token: %v", err) + case token := <-results: + if token.AccessToken != "fresh-access" { + t.Fatalf("access token = %q", token.AccessToken) + } + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for providers") + } + } + if requests.Load() != 1 { + t.Fatalf("refresh requests = %d, want 1", requests.Load()) + } +} + +func TestOAuthCacheLockHonorsContextCancellation(t *testing.T) { + path := filepath.Join(t.TempDir(), "oauth", "default.json") + first, err := acquireOAuthCacheLock(context.Background(), path) + if err != nil { + t.Fatalf("first lock: %v", err) + } + defer releaseOAuthCacheLock(first) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := acquireOAuthCacheLock(ctx, path); !errors.Is(err, context.Canceled) { + t.Fatalf("expected context cancellation, got %v", err) + } + releaseOAuthCacheLock(nil) +} + +func TestOAuthProviderPropagatesCacheLockCancellation(t *testing.T) { + path := filepath.Join(t.TempDir(), "oauth", "default.json") + cacheLock, err := acquireOAuthCacheLock(context.Background(), path) + if err != nil { + t.Fatalf("hold lock: %v", err) + } + defer releaseOAuthCacheLock(cacheLock) + provider, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + RedirectURI: "http://127.0.0.1:8888/callback", + CachePath: path, + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := provider.Token(ctx); !errors.Is(err, context.Canceled) { + t.Fatalf("token cancellation = %v", err) + } + if _, err := provider.ExchangeCode(ctx, "code", "verifier"); !errors.Is(err, context.Canceled) { + t.Fatalf("exchange cancellation = %v", err) + } +} + +func TestOAuthProviderMissingAndMismatchedCache(t *testing.T) { + path := filepath.Join(t.TempDir(), "token.json") + provider, err := NewOAuthTokenProvider(OAuthOptions{ClientID: "client-id", CachePath: path}) + if err != nil { + t.Fatalf("provider: %v", err) + } + if _, err := provider.Token(context.Background()); !errors.Is(err, ErrOAuthAuthentication) { + t.Fatalf("expected oauth auth error, got %v", err) + } + if err := SaveOAuthToken(path, OAuthToken{RefreshToken: "refresh", ClientID: "other"}); err != nil { + t.Fatalf("save: %v", err) + } + if _, err := provider.Token(context.Background()); !errors.Is(err, ErrOAuthAuthentication) { + t.Fatalf("expected client mismatch auth error, got %v", err) + } +} + +func TestLoadOAuthTokenMissing(t *testing.T) { + _, err := LoadOAuthToken(filepath.Join(t.TempDir(), "missing.json")) + if !errors.Is(err, os.ErrNotExist) { + t.Fatalf("expected missing token cache, got %v", err) + } +} + +func TestOAuthTokenCachePermissionsStatusAndClear(t *testing.T) { + path := filepath.Join(t.TempDir(), "oauth", "default.json") + token := OAuthToken{ + AccessToken: "access", + RefreshToken: "refresh", + Scope: "scope-a scope-b", + ExpiresAt: time.Now().Add(time.Hour), + ClientID: "client-id", + } + if err := SaveOAuthToken(path, token); err != nil { + t.Fatalf("save: %v", err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + if runtime.GOOS != "windows" && info.Mode().Perm() != 0o600 { + t.Fatalf("file mode = %04o", info.Mode().Perm()) + } + dirInfo, err := os.Stat(filepath.Dir(path)) + if err != nil { + t.Fatalf("stat dir: %v", err) + } + if runtime.GOOS != "windows" && dirInfo.Mode().Perm() != 0o700 { + t.Fatalf("directory mode = %04o", dirInfo.Mode().Perm()) + } + status, err := OAuthStatus(path) + if err != nil { + t.Fatalf("status: %v", err) + } + if !status.Exists || !status.HasRefresh || len(status.Scopes) != 2 { + t.Fatalf("unexpected status: %+v", status) + } + if err := ClearOAuthToken(path); err != nil { + t.Fatalf("clear: %v", err) + } + status, err = OAuthStatus(path) + if err != nil || status.Exists { + t.Fatalf("status after clear: %+v, %v", status, err) + } +} + +func TestLoadOAuthTokenRejectsLoosePermissions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX permissions are not available") + } + dir := t.TempDir() + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatalf("chmod dir: %v", err) + } + path := filepath.Join(dir, "token.json") + if err := os.WriteFile(path, []byte(`{"refresh_token":"refresh"}`), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + if _, err := LoadOAuthToken(path); err == nil || !strings.Contains(err.Error(), "require 0600") { + t.Fatalf("expected permission error, got %v", err) + } +} + +func TestOAuthTokenEndpointErrorIsBounded(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"invalid_grant","error_description":"expired code"}`)) + })) + defer server.Close() + provider, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + RedirectURI: "http://127.0.0.1:8888/callback", + CachePath: filepath.Join(t.TempDir(), "token.json"), + AccountsURL: server.URL, + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + if _, err := provider.ExchangeCode(context.Background(), "code", "verifier"); err == nil || !strings.Contains(err.Error(), "expired code") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestOAuthTokenEndpointTransientErrorsRemainAPIError(t *testing.T) { + for _, status := range []int{http.StatusTooManyRequests, http.StatusServiceUnavailable} { + t.Run(http.StatusText(status), func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if status == http.StatusTooManyRequests { + w.Header().Set("Retry-After", "42") + } + w.WriteHeader(status) + _, _ = w.Write([]byte(`{"error":"temporarily_unavailable","error_description":"try later"}`)) + })) + defer server.Close() + provider, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + RedirectURI: "http://127.0.0.1:8888/callback", + CachePath: filepath.Join(t.TempDir(), "token.json"), + AccountsURL: server.URL, + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + _, err = provider.ExchangeCode(context.Background(), "code", "verifier") + var apiErr APIError + if !errors.As(err, &apiErr) || errors.Is(err, ErrOAuthAuthentication) || apiErr.Status != status { + t.Fatalf("unexpected error: %v", err) + } + if status == http.StatusTooManyRequests && apiErr.RetryAfter != 42*time.Second { + t.Fatalf("retry after = %s", apiErr.RetryAfter) + } + }) + } +} + +func TestGenerateOAuthState(t *testing.T) { + first, err := GenerateOAuthState() + if err != nil { + t.Fatalf("generate state: %v", err) + } + second, err := GenerateOAuthState() + if err != nil { + t.Fatalf("generate second state: %v", err) + } + if first == "" || first == second || strings.ContainsAny(first+second, "+/=") { + t.Fatalf("invalid OAuth states: %q %q", first, second) + } +} + +func TestNewOAuthTokenProviderValidation(t *testing.T) { + if _, err := NewOAuthTokenProvider(OAuthOptions{CachePath: "token.json"}); !errors.Is(err, ErrOAuthAuthentication) { + t.Fatalf("expected missing client ID error, got %v", err) + } + if _, err := NewOAuthTokenProvider(OAuthOptions{ClientID: "client-id"}); !errors.Is(err, ErrOAuthAuthentication) { + t.Fatalf("expected missing cache path error, got %v", err) + } +} + +func TestOAuthAuthorizationURLValidation(t *testing.T) { + provider, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + RedirectURI: "http://localhost:8888/callback", + CachePath: filepath.Join(t.TempDir(), "token.json"), + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + if _, err := provider.AuthorizationURL("", "challenge"); err == nil { + t.Fatalf("expected missing state error") + } + if _, err := provider.AuthorizationURL("state", ""); err == nil { + t.Fatalf("expected missing challenge error") + } + if _, err := provider.AuthorizationURL("state", "challenge"); err == nil { + t.Fatalf("expected invalid redirect error") + } +} + +func TestOAuthExchangeValidationAndMalformedResponses(t *testing.T) { + provider, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + RedirectURI: "http://127.0.0.1:8888/callback", + CachePath: filepath.Join(t.TempDir(), "token.json"), + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + if _, err := provider.ExchangeCode(context.Background(), "", "verifier"); err == nil { + t.Fatalf("expected missing code error") + } + if _, err := provider.ExchangeCode(context.Background(), "code", ""); err == nil { + t.Fatalf("expected missing verifier error") + } + + tests := []struct { + name string + body string + }{ + {name: "invalid json", body: `{`}, + {name: "missing fields", body: `{}`}, + {name: "missing refresh", body: `{"access_token":"access","expires_in":3600}`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(tt.body)) + })) + defer server.Close() + p, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + RedirectURI: "http://127.0.0.1:8888/callback", + CachePath: filepath.Join(t.TempDir(), "token.json"), + AccountsURL: server.URL, + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + if _, err := p.ExchangeCode(context.Background(), "code", "verifier"); err == nil { + t.Fatalf("expected malformed response error") + } + }) + } +} + +func TestOAuthTokenCacheValidationErrors(t *testing.T) { + if err := SaveOAuthToken("", OAuthToken{RefreshToken: "refresh"}); err == nil { + t.Fatalf("expected empty path error") + } + if err := SaveOAuthToken(filepath.Join(t.TempDir(), "token.json"), OAuthToken{}); err == nil { + t.Fatalf("expected missing refresh token error") + } + + t.Run("not regular", func(t *testing.T) { + dir := t.TempDir() + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatalf("chmod: %v", err) + } + if _, err := LoadOAuthToken(dir); err == nil { + t.Fatalf("expected non-regular error") + } + }) + + t.Run("invalid json", func(t *testing.T) { + dir := t.TempDir() + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatalf("chmod: %v", err) + } + path := filepath.Join(dir, "token.json") + if err := os.WriteFile(path, []byte(`{`), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + if _, err := LoadOAuthToken(path); err == nil { + t.Fatalf("expected decode error") + } + }) + + t.Run("no tokens", func(t *testing.T) { + dir := t.TempDir() + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatalf("chmod: %v", err) + } + path := filepath.Join(dir, "token.json") + if err := os.WriteFile(path, []byte(`{}`), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + if _, err := LoadOAuthToken(path); err == nil { + t.Fatalf("expected empty token error") + } + }) + + t.Run("loose directory", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX permissions are not available") + } + dir := t.TempDir() + if err := os.Chmod(dir, 0o755); err != nil { + t.Fatalf("chmod: %v", err) + } + path := filepath.Join(dir, "token.json") + if err := os.WriteFile(path, []byte(`{"refresh_token":"refresh"}`), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + if _, err := LoadOAuthToken(path); err == nil || !strings.Contains(err.Error(), "require 0700") { + t.Fatalf("expected directory permission error, got %v", err) + } + }) + + t.Run("save parent is file", func(t *testing.T) { + parent := filepath.Join(t.TempDir(), "file") + if err := os.WriteFile(parent, []byte("x"), 0o600); err != nil { + t.Fatalf("write parent: %v", err) + } + if err := SaveOAuthToken(filepath.Join(parent, "token.json"), OAuthToken{RefreshToken: "refresh"}); err == nil { + t.Fatalf("expected parent error") + } + }) +} + +func TestOAuthStatusAndClearErrors(t *testing.T) { + dir := t.TempDir() + if _, err := OAuthStatus(dir); err == nil { + t.Fatalf("expected directory status error") + } + if err := ClearOAuthToken(""); err == nil { + t.Fatalf("expected empty clear path error") + } + if err := ClearOAuthToken(filepath.Join(t.TempDir(), "missing.json")); err != nil { + t.Fatalf("clear missing: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "keep"), []byte("x"), 0o600); err != nil { + t.Fatalf("write directory entry: %v", err) + } + if err := ClearOAuthToken(dir); err == nil { + t.Fatalf("expected non-empty directory clear error") + } +} + +func TestOAuthProviderExpiredTokenWithoutRefresh(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "oauth", "default.json") + if err := SaveOAuthToken(path, OAuthToken{ + AccessToken: "expired", + RefreshToken: "temporary", + ExpiresAt: time.Now().Add(-time.Hour), + ClientID: "client-id", + }); err != nil { + t.Fatalf("save: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read: %v", err) + } + var token OAuthToken + if err := json.Unmarshal(data, &token); err != nil { + t.Fatalf("decode: %v", err) + } + token.RefreshToken = "" + data, err = json.Marshal(token) + if err != nil { + t.Fatalf("encode: %v", err) + } + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatalf("rewrite: %v", err) + } + provider, err := NewOAuthTokenProvider(OAuthOptions{ClientID: "client-id", CachePath: path}) + if err != nil { + t.Fatalf("provider: %v", err) + } + if _, err := provider.Token(context.Background()); !errors.Is(err, ErrOAuthAuthentication) || !strings.Contains(err.Error(), "no refresh token") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestOAuthTokenEndpointDoesNotFollowRedirects(t *testing.T) { + var followed atomic.Bool + sink := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + followed.Store(true) + w.WriteHeader(http.StatusOK) + })) + defer sink.Close() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, sink.URL, http.StatusTemporaryRedirect) + })) + defer server.Close() + provider, err := NewOAuthTokenProvider(OAuthOptions{ + ClientID: "client-id", + RedirectURI: "http://127.0.0.1:8888/callback", + CachePath: filepath.Join(t.TempDir(), "token.json"), + AccountsURL: server.URL, + }) + if err != nil { + t.Fatalf("provider: %v", err) + } + if _, err := provider.ExchangeCode(context.Background(), "code", "verifier"); err == nil { + t.Fatalf("expected redirect rejection") + } else { + var apiErr APIError + if !errors.As(err, &apiErr) || apiErr.Status != http.StatusTemporaryRedirect { + t.Fatalf("expected redirect API error, got %v", err) + } + } + if followed.Load() { + t.Fatalf("token endpoint redirect was followed") + } +} From a865ccfb36eb42467a12a6726f89b37f341e2daa Mon Sep 17 00:00:00 2001 From: roboclaw-bot <309084314+roboclaw-bot@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:18:11 +0000 Subject: [PATCH 2/6] fix(auth): make OAuth clear failure-safe --- internal/cli/auth_oauth.go | 6 +++--- internal/cli/auth_oauth_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/internal/cli/auth_oauth.go b/internal/cli/auth_oauth.go index d1cabed..ce96a23 100644 --- a/internal/cli/auth_oauth.go +++ b/internal/cli/auth_oauth.go @@ -228,9 +228,6 @@ func (cmd *AuthOAuthStatusCmd) Run(ctx *app.Context) error { func (cmd *AuthOAuthClearCmd) Run(ctx *app.Context) error { path := ctx.ResolveOAuthTokenPath() - if err := spotify.ClearOAuthToken(path); err != nil { - return err - } profile := ctx.Profile if selectedAuth(profile.Auth) == "oauth" { profile.Auth = "" @@ -238,6 +235,9 @@ func (cmd *AuthOAuthClearCmd) Run(ctx *app.Context) error { return err } } + if err := spotify.ClearOAuthToken(path); err != nil { + return err + } payload := map[string]string{"status": "ok", "token_path": path} return ctx.Output.Emit(payload, []string{"ok"}, []string{"Cleared Spotify OAuth token cache."}) } diff --git a/internal/cli/auth_oauth_test.go b/internal/cli/auth_oauth_test.go index 3b59308..465b7ac 100644 --- a/internal/cli/auth_oauth_test.go +++ b/internal/cli/auth_oauth_test.go @@ -230,6 +230,37 @@ func TestAuthOAuthClearMissingKeepsCookieSelection(t *testing.T) { } } +func TestAuthOAuthClearKeepsTokenWhenProfileUpdateFails(t *testing.T) { + ctx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + root := t.TempDir() + ctx.Config = config.Default() + ctx.ConfigPath = filepath.Join(root, "config.toml") + ctx.ProfileKey = "default" + ctx.Profile = config.Profile{Auth: "oauth", SpotifyClientID: "client-id"} + tokenPath := ctx.ResolveOAuthTokenPath() + if err := spotify.SaveOAuthToken(tokenPath, spotify.OAuthToken{ + AccessToken: "access", + RefreshToken: "refresh", + ExpiresAt: time.Now().Add(time.Hour), + ClientID: "client-id", + }); err != nil { + t.Fatalf("save token: %v", err) + } + if err := os.Mkdir(ctx.ConfigPath, 0o755); err != nil { + t.Fatalf("block config save: %v", err) + } + + if err := (&AuthOAuthClearCmd{}).Run(ctx); err == nil { + t.Fatal("expected profile update failure") + } + if ctx.Profile.Auth != "oauth" { + t.Fatalf("auth changed despite failed profile update: %q", ctx.Profile.Auth) + } + if _, err := spotify.LoadOAuthToken(tokenPath); err != nil { + t.Fatalf("token removed before profile update succeeded: %v", err) + } +} + func TestAuthOAuthLoginTimeoutAndBrowserError(t *testing.T) { listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { From e26100c1b3f32c59a5aa7c57b2f5854279caf74b Mon Sep 17 00:00:00 2001 From: roboclaw-bot <309084314+roboclaw-bot@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:12:20 +0000 Subject: [PATCH 3/6] fix(auth): serialize OAuth lifecycle transitions Co-authored-by: VACInc <3279061+VACInc@users.noreply.github.com> Worked on by: - @VACInc --- internal/cli/auth_oauth.go | 61 +++++++++---- internal/cli/auth_oauth_test.go | 156 ++++++++++++++++++++++++++++++++ internal/spotify/oauth.go | 27 +++++- internal/spotify/oauth_test.go | 44 +++++++++ 4 files changed, 267 insertions(+), 21 deletions(-) diff --git a/internal/cli/auth_oauth.go b/internal/cli/auth_oauth.go index ce96a23..6165ce2 100644 --- a/internal/cli/auth_oauth.go +++ b/internal/cli/auth_oauth.go @@ -15,6 +15,7 @@ import ( "time" "github.com/steipete/spogo/internal/app" + "github.com/steipete/spogo/internal/config" "github.com/steipete/spogo/internal/spotify" ) @@ -38,8 +39,9 @@ type oauthStatusPayload struct { const defaultOAuthRedirectURI = "http://127.0.0.1:8888/callback" var ( - openOAuthBrowser = openBrowserURL - newOAuthTokenProvider = spotify.NewOAuthTokenProvider + openOAuthBrowser = openBrowserURL + newOAuthTokenProvider = spotify.NewOAuthTokenProvider + afterOAuthTokenExchange = func() {} ) func (cmd *AuthOAuthLoginCmd) Run(ctx *app.Context) error { @@ -165,22 +167,32 @@ func (cmd *AuthOAuthLoginCmd) Run(ctx *app.Context) error { if callback.err != nil { return callback.err } - if _, err := provider.ExchangeCode(ctx.CommandContext(), callback.code, verifier); err != nil { + path := ctx.ResolveOAuthTokenPath() + if err := spotify.WithOAuthLifecycleLock(ctx.CommandContext(), path, func() error { + profile, err := reloadOAuthProfile(ctx) + if err != nil { + return err + } + if _, err := provider.ExchangeCode(ctx.CommandContext(), callback.code, verifier); err != nil { + return err + } + afterOAuthTokenExchange() + profile.Auth = "oauth" + profile.SpotifyClientID = clientID + profile.SpotifyRedirectURI = redirectURI + if err := ctx.SaveProfile(profile); err != nil { + return fmt.Errorf("oauth token saved but profile update failed: %w", err) + } + return nil + }); err != nil { return err } - profile := ctx.Profile - profile.Auth = "oauth" - profile.SpotifyClientID = clientID - profile.SpotifyRedirectURI = redirectURI - if err := ctx.SaveProfile(profile); err != nil { - return fmt.Errorf("oauth token saved but profile update failed: %w", err) - } payload := map[string]any{ "status": "ok", "auth": "oauth", "client_id": clientID, "redirect_uri": redirectURI, - "token_path": ctx.ResolveOAuthTokenPath(), + "token_path": path, } return ctx.Output.Emit(payload, []string{"ok\toauth"}, []string{ "Spotify OAuth login complete.", @@ -228,20 +240,35 @@ func (cmd *AuthOAuthStatusCmd) Run(ctx *app.Context) error { func (cmd *AuthOAuthClearCmd) Run(ctx *app.Context) error { path := ctx.ResolveOAuthTokenPath() - profile := ctx.Profile - if selectedAuth(profile.Auth) == "oauth" { - profile.Auth = "" - if err := ctx.SaveProfile(profile); err != nil { + if err := spotify.WithOAuthLifecycleLock(ctx.CommandContext(), path, func() error { + profile, err := reloadOAuthProfile(ctx) + if err != nil { return err } - } - if err := spotify.ClearOAuthToken(path); err != nil { + if selectedAuth(profile.Auth) == "oauth" { + profile.Auth = "" + if err := ctx.SaveProfile(profile); err != nil { + return err + } + } + return spotify.ClearOAuthToken(path) + }); err != nil { return err } payload := map[string]string{"status": "ok", "token_path": path} return ctx.Output.Emit(payload, []string{"ok"}, []string{"Cleared Spotify OAuth token cache."}) } +func reloadOAuthProfile(ctx *app.Context) (config.Profile, error) { + cfg, err := config.Load(ctx.ConfigPath) + if err != nil { + return config.Profile{}, fmt.Errorf("reload oauth profile: %w", err) + } + ctx.Config = cfg + ctx.Profile = cfg.Profile(ctx.ProfileKey) + return ctx.Profile, nil +} + func selectedAuth(auth string) string { auth = strings.ToLower(strings.TrimSpace(auth)) if auth == "" { diff --git a/internal/cli/auth_oauth_test.go b/internal/cli/auth_oauth_test.go index 465b7ac..bd7c5c1 100644 --- a/internal/cli/auth_oauth_test.go +++ b/internal/cli/auth_oauth_test.go @@ -116,6 +116,157 @@ func TestAuthOAuthLoginCmd(t *testing.T) { } } +func TestAuthOAuthLoginAndClearSerializeLifecycle(t *testing.T) { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "access", + "refresh_token": "refresh", + "token_type": "Bearer", + "scope": "user-library-read", + "expires_in": 3600, + }) + })) + defer tokenServer.Close() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("reserve port: %v", err) + } + redirectURI := "http://" + listener.Addr().String() + "/callback" + _ = listener.Close() + + oldProvider := newOAuthTokenProvider + newOAuthTokenProvider = func(opts spotify.OAuthOptions) (*spotify.OAuthTokenProvider, error) { + opts.AccountsURL = tokenServer.URL + return spotify.NewOAuthTokenProvider(opts) + } + t.Cleanup(func() { newOAuthTokenProvider = oldProvider }) + + oldOpen := openOAuthBrowser + openOAuthBrowser = func(raw string) error { + parsed, parseErr := url.Parse(raw) + if parseErr != nil { + return parseErr + } + state := parsed.Query().Get("state") + go func() { + callbackURL := redirectURI + "?code=***&state=" + url.QueryEscape(state) + for range 50 { + resp, getErr := http.Get(callbackURL) //nolint:gosec // loopback test callback + if getErr == nil { + _ = resp.Body.Close() + return + } + time.Sleep(10 * time.Millisecond) + } + }() + return nil + } + t.Cleanup(func() { openOAuthBrowser = oldOpen }) + + root := t.TempDir() + configPath := filepath.Join(root, "config.toml") + initialConfig := config.Default() + initialConfig.SetProfile("default", config.Profile{Auth: "cookies"}) + if err := config.Save(configPath, initialConfig); err != nil { + t.Fatalf("save initial config: %v", err) + } + + loginConfig, err := config.Load(configPath) + if err != nil { + t.Fatalf("load login config: %v", err) + } + loginCtx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + loginCtx.Config = loginConfig + loginCtx.ConfigPath = configPath + loginCtx.ProfileKey = "default" + loginCtx.Profile = loginConfig.Profile("default") + + // Load clear's context before login commits so it holds the stale cookie profile + // that triggered the original token/profile race. + clearConfig, err := config.Load(configPath) + if err != nil { + t.Fatalf("load clear config: %v", err) + } + clearCtx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + clearCtx.Config = clearConfig + clearCtx.ConfigPath = configPath + clearCtx.ProfileKey = "default" + clearCtx.Profile = clearConfig.Profile("default") + + tokenSaved := make(chan struct{}) + releaseLogin := make(chan struct{}) + oldAfterExchange := afterOAuthTokenExchange + afterOAuthTokenExchange = func() { + close(tokenSaved) + <-releaseLogin + } + t.Cleanup(func() { + afterOAuthTokenExchange = oldAfterExchange + select { + case <-releaseLogin: + default: + close(releaseLogin) + } + }) + + loginDone := make(chan error, 1) + go func() { + loginDone <- (&AuthOAuthLoginCmd{ + ClientID: "client-id", + RedirectURI: redirectURI, + WaitTimeout: 2 * time.Second, + }).Run(loginCtx) + }() + select { + case <-tokenSaved: + case <-time.After(2 * time.Second): + t.Fatal("login did not reach the token/profile transition") + } + + clearStarted := make(chan struct{}) + clearDone := make(chan error, 1) + go func() { + close(clearStarted) + clearDone <- (&AuthOAuthClearCmd{}).Run(clearCtx) + }() + <-clearStarted + select { + case clearErr := <-clearDone: + t.Fatalf("clear bypassed the login lifecycle lock: %v", clearErr) + case <-time.After(100 * time.Millisecond): + } + close(releaseLogin) + + select { + case loginErr := <-loginDone: + if loginErr != nil { + t.Fatalf("login: %v", loginErr) + } + case <-time.After(2 * time.Second): + t.Fatal("login did not finish") + } + select { + case clearErr := <-clearDone: + if clearErr != nil { + t.Fatalf("clear: %v", clearErr) + } + case <-time.After(2 * time.Second): + t.Fatal("clear did not finish") + } + + finalConfig, err := config.Load(configPath) + if err != nil { + t.Fatalf("load final config: %v", err) + } + if auth := finalConfig.Profile("default").Auth; auth != "" { + t.Fatalf("final auth = %q; want cookie fallback", auth) + } + if _, err := os.Stat(loginCtx.ResolveOAuthTokenPath()); !os.IsNotExist(err) { + t.Fatalf("final token cache still exists: %v", err) + } +} + func TestAuthOAuthStatusAndClearCmd(t *testing.T) { ctx, out, _ := testutil.NewTestContext(t, output.FormatJSON) ctx.Config = config.Default() @@ -222,6 +373,11 @@ func TestAuthOAuthClearMissingKeepsCookieSelection(t *testing.T) { ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") ctx.ProfileKey = "default" ctx.Profile = config.Profile{Auth: "cookies"} + ctx.Config = config.Default() + ctx.Config.SetProfile("default", ctx.Profile) + if err := config.Save(ctx.ConfigPath, ctx.Config); err != nil { + t.Fatalf("save config: %v", err) + } if err := (&AuthOAuthClearCmd{}).Run(ctx); err != nil { t.Fatalf("clear missing: %v", err) } diff --git a/internal/spotify/oauth.go b/internal/spotify/oauth.go index 34a335f..7a49858 100644 --- a/internal/spotify/oauth.go +++ b/internal/spotify/oauth.go @@ -492,23 +492,42 @@ func ClearOAuthToken(path string) error { return nil } +func WithOAuthLifecycleLock(ctx context.Context, path string, fn func() error) error { + if path == "" { + return errors.New("oauth token cache path is required") + } + lifecycleLock, err := acquireOAuthLifecycleLock(ctx, path) + if err != nil { + return err + } + defer releaseOAuthCacheLock(lifecycleLock) + return fn() +} + func acquireOAuthCacheLock(ctx context.Context, path string) (*flock.Flock, error) { if path == "" { return nil, errors.New("oauth token cache path is required") } - dir := filepath.Dir(path) + return acquireOAuthFileLock(ctx, path+".lock", filepath.Dir(path), "oauth token cache") +} + +func acquireOAuthLifecycleLock(ctx context.Context, path string) (*flock.Flock, error) { + return acquireOAuthFileLock(ctx, path+".lifecycle.lock", filepath.Dir(path), "oauth lifecycle") +} + +func acquireOAuthFileLock(ctx context.Context, lockPath, dir, label string) (*flock.Flock, error) { if err := os.MkdirAll(dir, 0o700); err != nil { return nil, err } - cacheLock := flock.New(path+".lock", flock.SetPermissions(0o600)) + cacheLock := flock.New(lockPath, flock.SetPermissions(0o600)) locked, err := cacheLock.TryLockContext(ctx, oauthLockRetryDelay) if err != nil { _ = cacheLock.Close() - return nil, fmt.Errorf("lock oauth token cache: %w", err) + return nil, fmt.Errorf("lock %s: %w", label, err) } if !locked { _ = cacheLock.Close() - return nil, fmt.Errorf("lock oauth token cache: %w", ctx.Err()) + return nil, fmt.Errorf("lock %s: %w", label, ctx.Err()) } if runtime.GOOS != "windows" { if err := os.Chmod(cacheLock.Path(), 0o600); err != nil { diff --git a/internal/spotify/oauth_test.go b/internal/spotify/oauth_test.go index 3276597..4f53171 100644 --- a/internal/spotify/oauth_test.go +++ b/internal/spotify/oauth_test.go @@ -271,6 +271,50 @@ func TestOAuthCacheLockHonorsContextCancellation(t *testing.T) { releaseOAuthCacheLock(nil) } +func TestOAuthLifecycleLockSerializesAndPropagates(t *testing.T) { + if err := WithOAuthLifecycleLock(context.Background(), "", func() error { return nil }); err == nil { + t.Fatal("expected empty path error") + } + + path := filepath.Join(t.TempDir(), "token.json") + entered := make(chan struct{}) + release := make(chan struct{}) + t.Cleanup(func() { + select { + case <-release: + default: + close(release) + } + }) + firstDone := make(chan error, 1) + go func() { + firstDone <- WithOAuthLifecycleLock(context.Background(), path, func() error { + close(entered) + <-release + return nil + }) + }() + <-entered + + waitCtx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + if err := WithOAuthLifecycleLock(waitCtx, path, func() error { + t.Fatal("second lifecycle transition entered while the first held the lock") + return nil + }); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected lifecycle lock timeout, got %v", err) + } + close(release) + if err := <-firstDone; err != nil { + t.Fatalf("first lifecycle transition: %v", err) + } + + wantErr := errors.New("transition failed") + if err := WithOAuthLifecycleLock(context.Background(), path, func() error { return wantErr }); !errors.Is(err, wantErr) { + t.Fatalf("expected callback error, got %v", err) + } +} + func TestOAuthProviderPropagatesCacheLockCancellation(t *testing.T) { path := filepath.Join(t.TempDir(), "oauth", "default.json") cacheLock, err := acquireOAuthCacheLock(context.Background(), path) From ad054dfd7873a0a785c16755381dae4552b08634 Mon Sep 17 00:00:00 2001 From: VACInc <3279061+VACInc@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:46:45 -0400 Subject: [PATCH 4/6] fix(auth): harden OAuth and Web API integration --- internal/cli/auth_oauth_test.go | 25 +++++++ internal/spotify/connect.go | 1 - internal/spotify/connect_pathfinder_test.go | 16 +++-- internal/spotify/connect_test_helpers_test.go | 6 +- internal/spotify/connect_web.go | 65 +------------------ internal/spotify/connect_webclient_test.go | 25 +++++++ internal/spotify/oauth.go | 8 ++- internal/spotify/oauth_test.go | 1 + 8 files changed, 74 insertions(+), 73 deletions(-) diff --git a/internal/cli/auth_oauth_test.go b/internal/cli/auth_oauth_test.go index bd7c5c1..5831259 100644 --- a/internal/cli/auth_oauth_test.go +++ b/internal/cli/auth_oauth_test.go @@ -417,6 +417,31 @@ func TestAuthOAuthClearKeepsTokenWhenProfileUpdateFails(t *testing.T) { } } +func TestAuthOAuthClearResetsStoredOAuthSelectionDespiteRuntimeOverride(t *testing.T) { + ctx, _, _ := testutil.NewTestContext(t, output.FormatPlain) + ctx.Config = config.Default() + ctx.ConfigPath = filepath.Join(t.TempDir(), "config.toml") + ctx.ProfileKey = "default" + stored := config.Profile{Auth: "oauth", SpotifyClientID: "client-id"} + ctx.Config.SetProfile(ctx.ProfileKey, stored) + if err := config.Save(ctx.ConfigPath, ctx.Config); err != nil { + t.Fatalf("save config: %v", err) + } + ctx.Profile = stored + ctx.Profile.Auth = "cookies" + + if err := (&AuthOAuthClearCmd{}).Run(ctx); err != nil { + t.Fatalf("clear with runtime override: %v", err) + } + loaded, err := config.Load(ctx.ConfigPath) + if err != nil { + t.Fatalf("load saved config: %v", err) + } + if got := loaded.Profile(ctx.ProfileKey).Auth; got != "" { + t.Fatalf("expected stored OAuth selection cleared, got %q", got) + } +} + func TestAuthOAuthLoginTimeoutAndBrowserError(t *testing.T) { listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { diff --git a/internal/spotify/connect.go b/internal/spotify/connect.go index f27d89a..974dd42 100644 --- a/internal/spotify/connect.go +++ b/internal/spotify/connect.go @@ -30,7 +30,6 @@ type ConnectClient struct { hashes *hashResolver webMu sync.Mutex web *Client - searchURL string searchClient *http.Client cache *connectCacheStore diff --git a/internal/spotify/connect_pathfinder_test.go b/internal/spotify/connect_pathfinder_test.go index b574891..349c48f 100644 --- a/internal/spotify/connect_pathfinder_test.go +++ b/internal/spotify/connect_pathfinder_test.go @@ -139,11 +139,12 @@ func TestPathfinderFallbackToWeb(t *testing.T) { "total": 1, }, } - return jsonResponse(http.StatusOK, payload), nil + response := jsonResponse(http.StatusOK, payload) + response.ContentLength = -1 + return response, nil }) client := newConnectClientForTests(transport) client.hashes.hashes["searchDesktop"] = "hash" - client.searchURL = "https://search.local/search" result, err := client.Search(context.Background(), "track", "song", 1, 0) if err != nil { @@ -168,11 +169,11 @@ func TestSearchViaWebAPIDefaultClient(t *testing.T) { "total": 1, }, } - return jsonResponse(http.StatusOK, payload), nil + response := jsonResponse(http.StatusOK, payload) + response.ContentLength = -1 + return response, nil }) client := newConnectClientForTests(transport) - client.searchURL = "" - client.searchClient = nil result, err := client.searchViaWebAPI(context.Background(), "track", "song", 1, 0) if err != nil { @@ -186,10 +187,11 @@ func TestSearchViaWebAPIDefaultClient(t *testing.T) { func TestSearchViaWebAPIMissingKind(t *testing.T) { transport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { payload := map[string]any{"album": map[string]any{}} - return jsonResponse(http.StatusOK, payload), nil + response := jsonResponse(http.StatusOK, payload) + response.ContentLength = -1 + return response, nil }) client := newConnectClientForTests(transport) - client.searchURL = "https://search.local/search" if _, err := client.searchViaWebAPI(context.Background(), "track", "song", 1, 0); err == nil { t.Fatalf("expected error") diff --git a/internal/spotify/connect_test_helpers_test.go b/internal/spotify/connect_test_helpers_test.go index e90ee39..e66ba09 100644 --- a/internal/spotify/connect_test_helpers_test.go +++ b/internal/spotify/connect_test_helpers_test.go @@ -7,6 +7,10 @@ import ( func newConnectClientForTests(transport http.RoundTripper) *ConnectClient { client := &http.Client{Transport: transport} + web, err := NewClient(Options{TokenProvider: staticTokenProvider{}, HTTPClient: client}) + if err != nil { + panic(err) + } session := &connectSession{ client: client, token: Token{AccessToken: "access", ExpiresAt: time.Now().Add(time.Hour), ClientID: "client"}, @@ -16,7 +20,7 @@ func newConnectClientForTests(transport http.RoundTripper) *ConnectClient { deviceID: "device", } hashes := &hashResolver{client: client, session: session, hashes: map[string]string{}} - return &ConnectClient{client: client, session: session, hashes: hashes} + return &ConnectClient{client: client, session: session, hashes: hashes, web: web} } func newRegisteredConnectClientForTests(transport http.RoundTripper) *ConnectClient { diff --git a/internal/spotify/connect_web.go b/internal/spotify/connect_web.go index d7d2e18..4380ade 100644 --- a/internal/spotify/connect_web.go +++ b/internal/spotify/connect_web.go @@ -2,75 +2,14 @@ package spotify import ( "context" - "encoding/json" - "fmt" - "net/http" - "net/url" ) func (c *ConnectClient) searchViaWebAPI(ctx context.Context, kind, query string, limit, offset int) (SearchResult, error) { - auth, err := c.session.auth(ctx) + web, err := c.webClient() if err != nil { return SearchResult{}, err } - params := url.Values{} - params.Set("q", query) - params.Set("type", kind) - params.Set("limit", fmt.Sprint(limit)) - params.Set("offset", fmt.Sprint(offset)) - if c.market != "" && params.Get("market") == "" { - params.Set("market", c.market) - } - if c.language != "" && params.Get("locale") == "" { - params.Set("locale", c.language) - } - searchURL := c.searchURL - if searchURL == "" { - searchURL = "https://api.spotify.com/v1/search" - } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, searchURL+"?"+params.Encode(), nil) - if err != nil { - return SearchResult{}, err - } - applyRequestHeaders(req, requestHeaders{ - AccessToken: auth.AccessToken, - ClientToken: auth.ClientToken, - ClientVersion: auth.ClientVersion, - Accept: "application/json", - Language: c.language, - AppPlatform: defaultSpotifyAppPlatform, - }) - resp, err := c.client.Do(req) - if err != nil { - return SearchResult{}, err - } - defer func() { _ = resp.Body.Close() }() - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return SearchResult{}, apiErrorFromResponse(resp) - } - var response map[string]searchContainer - if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { - return SearchResult{}, err - } - container, ok := response[kind+"s"] - if !ok { - return SearchResult{}, fmt.Errorf("missing %s result", kind) - } - items := make([]Item, 0, len(container.Items)) - for _, raw := range container.Items { - item, err := mapSearchItem(kind, raw) - if err != nil { - return SearchResult{}, err - } - items = append(items, item) - } - return SearchResult{ - Type: kind, - Limit: container.Limit, - Offset: container.Offset, - Total: container.Total, - Items: items, - }, nil + return web.Search(ctx, kind, query, limit, offset) } func (c *ConnectClient) webClient() (*Client, error) { diff --git a/internal/spotify/connect_webclient_test.go b/internal/spotify/connect_webclient_test.go index 066dcfc..5e17990 100644 --- a/internal/spotify/connect_webclient_test.go +++ b/internal/spotify/connect_webclient_test.go @@ -1,7 +1,9 @@ package spotify import ( + "context" "net/http" + "net/http/httptest" "testing" ) @@ -44,3 +46,26 @@ func TestConnectWebClientUsesInjectedClient(t *testing.T) { t.Fatalf("expected injected Web API client") } } + +func TestConnectSearchViaWebAPIUsesInjectedClient(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "Bearer token" { + t.Fatalf("authorization header = %q", got) + } + _, _ = w.Write([]byte(`{"tracks":{"items":[{"id":"t1","uri":"spotify:track:t1","name":"Song"}],"limit":1,"offset":0,"total":1}}`)) + })) + t.Cleanup(server.Close) + web, err := NewClient(Options{TokenProvider: staticTokenProvider{}, BaseURL: server.URL}) + if err != nil { + t.Fatalf("new web client: %v", err) + } + connect := &ConnectClient{web: web} + + result, err := connect.searchViaWebAPI(context.Background(), "track", "song", 1, 0) + if err != nil { + t.Fatalf("search via injected web client: %v", err) + } + if len(result.Items) != 1 || result.Items[0].ID != "t1" { + t.Fatalf("unexpected result: %#v", result) + } +} diff --git a/internal/spotify/oauth.go b/internal/spotify/oauth.go index 7a49858..22f806d 100644 --- a/internal/spotify/oauth.go +++ b/internal/spotify/oauth.go @@ -14,6 +14,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "strings" "sync" "time" @@ -326,9 +327,14 @@ func ValidateOAuthRedirectURI(raw string) error { if host != "127.0.0.1" && host != "::1" { return errors.New("spotify CLI redirect URI must use 127.0.0.1 or [::1], not localhost or a non-loopback host") } - if parsed.Port() == "" { + port := parsed.Port() + if port == "" { return errors.New("spotify CLI redirect URI must include a port") } + portNumber, err := strconv.Atoi(port) + if err != nil || portNumber < 1 || portNumber > 65535 { + return errors.New("spotify CLI redirect URI must use a port from 1 to 65535") + } if parsed.RawQuery != "" || parsed.Fragment != "" || parsed.User != nil { return errors.New("spotify CLI redirect URI cannot include userinfo, a query, or a fragment") } diff --git a/internal/spotify/oauth_test.go b/internal/spotify/oauth_test.go index 4f53171..3c475bb 100644 --- a/internal/spotify/oauth_test.go +++ b/internal/spotify/oauth_test.go @@ -82,6 +82,7 @@ func TestValidateOAuthRedirectURI(t *testing.T) { "http://localhost:8888/callback", "https://127.0.0.1:8888/callback", "http://127.0.0.1/callback", + "http://127.0.0.1:0/callback", "http://192.168.1.2:8888/callback", "http://127.0.0.1:8888/callback?token=x", } From 2c01000a6aa1e3660afa58d9059bfe07320dda27 Mon Sep 17 00:00:00 2001 From: VACInc <3279061+VACInc@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:53:54 -0400 Subject: [PATCH 5/6] fix(config): serialize profile updates --- internal/app/context.go | 11 +-- internal/config/config.go | 54 ++++++++++++++ internal/config/config_test.go | 127 +++++++++++++++++++++++++++++++++ 3 files changed, 188 insertions(+), 4 deletions(-) diff --git a/internal/app/context.go b/internal/app/context.go index 516d81e..32c66c2 100644 --- a/internal/app/context.go +++ b/internal/app/context.go @@ -57,12 +57,15 @@ func (c *Context) SaveProfile(profile config.Profile) error { if c.Config == nil { return errors.New("nil config") } - cfg := c.Config - cfg.SetProfile(c.ProfileKey, profile) - cfg.DefaultProfile = c.ProfileKey - if err := config.Save(c.ConfigPath, cfg); err != nil { + cfg, err := config.Update(c.CommandContext(), c.ConfigPath, func(cfg *config.Config) error { + cfg.SetProfile(c.ProfileKey, profile) + cfg.DefaultProfile = c.ProfileKey + return nil + }) + if err != nil { return err } + c.Config = cfg c.Profile = profile return nil } diff --git a/internal/config/config.go b/internal/config/config.go index a88c913..857243f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,16 +1,22 @@ package config import ( + "context" "errors" + "fmt" "os" "path/filepath" + "runtime" + "time" + "github.com/gofrs/flock" "github.com/pelletier/go-toml/v2" ) const ( DefaultProfile = "default" DefaultConfig = "config.toml" + updateLockWait = 25 * time.Millisecond ) type Config struct { @@ -84,6 +90,54 @@ func Save(path string, cfg *Config) error { return os.WriteFile(path, data, 0o644) } +// Update serializes a load-modify-save transaction for the shared config file. +func Update(ctx context.Context, path string, fn func(*Config) error) (*Config, error) { + if fn == nil { + return nil, errors.New("nil config update") + } + if path == "" { + var err error + path, err = DefaultPath() + if err != nil { + return nil, err + } + } + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return nil, err + } + configLock := flock.New(path+".lock", flock.SetPermissions(0o600)) + locked, err := configLock.TryLockContext(ctx, updateLockWait) + if err != nil { + _ = configLock.Close() + return nil, fmt.Errorf("lock config: %w", err) + } + if !locked { + _ = configLock.Close() + return nil, fmt.Errorf("lock config: %w", ctx.Err()) + } + defer func() { + _ = configLock.Unlock() + _ = configLock.Close() + }() + if runtime.GOOS != "windows" { + if err := os.Chmod(configLock.Path(), 0o600); err != nil { + return nil, err + } + } + + cfg, err := Load(path) + if err != nil { + return nil, err + } + if err := fn(cfg); err != nil { + return nil, err + } + if err := Save(path, cfg); err != nil { + return nil, err + } + return cfg, nil +} + func Default() *Config { return &Config{ DefaultProfile: DefaultProfile, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 20cef61..9523bee 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,10 +1,13 @@ package config import ( + "context" + "errors" "os" "path/filepath" "strings" "testing" + "time" ) func isolateConfigHome(t *testing.T) string { @@ -173,6 +176,130 @@ func TestSaveInvalidDir(t *testing.T) { } } +func TestUpdateErrorsAndDefaultPath(t *testing.T) { + if _, err := Update(context.Background(), filepath.Join(t.TempDir(), "config.toml"), nil); err == nil { + t.Fatal("expected nil update error") + } + + isolateConfigHome(t) + wantErr := context.Canceled + if _, err := Update(context.Background(), "", func(*Config) error { return wantErr }); !errors.Is(err, wantErr) { + t.Fatalf("callback error = %v, want %v", err, wantErr) + } + updated, err := Update(context.Background(), "", func(cfg *Config) error { + cfg.SetProfile("default", Profile{Market: "US"}) + return nil + }) + if err != nil { + t.Fatalf("default-path update: %v", err) + } + if got := updated.Profile("default").Market; got != "US" { + t.Fatalf("updated market = %q", got) + } +} + +func TestUpdateLoadAndSaveErrors(t *testing.T) { + dir := t.TempDir() + invalidPath := filepath.Join(dir, "invalid.toml") + if err := os.WriteFile(invalidPath, []byte("not=toml=\""), 0o644); err != nil { + t.Fatalf("write invalid config: %v", err) + } + if _, err := Update(context.Background(), invalidPath, func(*Config) error { return nil }); err == nil { + t.Fatal("expected load error") + } + + savePath := filepath.Join(dir, "save-error.toml") + if _, err := Update(context.Background(), savePath, func(*Config) error { + if err := os.Mkdir(savePath, 0o755); err != nil { + return err + } + return nil + }); err == nil { + t.Fatal("expected save error") + } +} + +func TestUpdateHonorsLockCancellation(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.toml") + firstEntered := make(chan struct{}) + releaseFirst := make(chan struct{}) + firstDone := make(chan error, 1) + go func() { + _, err := Update(context.Background(), path, func(*Config) error { + close(firstEntered) + <-releaseFirst + return nil + }) + firstDone <- err + }() + <-firstEntered + + waitCtx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) + defer cancel() + if _, err := Update(waitCtx, path, func(*Config) error { return nil }); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("lock wait error = %v", err) + } + close(releaseFirst) + if err := <-firstDone; err != nil { + t.Fatalf("first update: %v", err) + } +} + +func TestUpdateSerializesDifferentProfileWrites(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.toml") + if err := Save(path, Default()); err != nil { + t.Fatalf("save initial config: %v", err) + } + + firstEntered := make(chan struct{}) + releaseFirst := make(chan struct{}) + firstDone := make(chan error, 1) + go func() { + _, err := Update(context.Background(), path, func(cfg *Config) error { + cfg.SetProfile("personal", Profile{Auth: "oauth", SpotifyClientID: "personal-client"}) + close(firstEntered) + <-releaseFirst + return nil + }) + firstDone <- err + }() + <-firstEntered + + secondEntered := make(chan struct{}) + secondDone := make(chan error, 1) + go func() { + _, err := Update(context.Background(), path, func(cfg *Config) error { + close(secondEntered) + cfg.SetProfile("work", Profile{Auth: "oauth", SpotifyClientID: "work-client"}) + return nil + }) + secondDone <- err + }() + select { + case <-secondEntered: + t.Fatal("second profile update bypassed the config lock") + case <-time.After(100 * time.Millisecond): + } + close(releaseFirst) + if err := <-firstDone; err != nil { + t.Fatalf("first update: %v", err) + } + if err := <-secondDone; err != nil { + t.Fatalf("second update: %v", err) + } + + loaded, err := Load(path) + if err != nil { + t.Fatalf("load final config: %v", err) + } + if got := loaded.Profile("personal").SpotifyClientID; got != "personal-client" { + t.Fatalf("personal profile lost: %q", got) + } + if got := loaded.Profile("work").SpotifyClientID; got != "work-client" { + t.Fatalf("work profile lost: %q", got) + } +} + func TestProfileNilConfig(t *testing.T) { var cfg *Config if p := cfg.Profile("default"); p != (Profile{}) { From 62f34e81700bb1d705dc1e56735aa24af319605a Mon Sep 17 00:00:00 2001 From: VACInc <3279061+VACInc@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:13:24 -0400 Subject: [PATCH 6/6] fix(auth): contain profile-derived token paths --- docs/auth.md | 2 ++ internal/config/config.go | 33 +++++++++++++++++++++++++- internal/config/config_test.go | 42 ++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/docs/auth.md b/docs/auth.md index 59d6fb5..8552e2c 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -126,6 +126,8 @@ OAuth tokens are stored per profile under the config directory: /spogo/oauth/.json ``` +Profile names that are not portable lowercase filename segments are encoded before deriving the token and lock filenames, so separators, traversal components, Windows-reserved names, and case variants cannot escape or alias within the OAuth directory. + The OAuth directory is mode `0700` and token file is mode `0600` on POSIX systems. Writes use a same-directory temporary file, file sync, and atomic rename. spogo refuses to load a token file that is readable or writable by group/other users. Treat the token cache as a credential. Do not copy it into source control, logs, shell history, or CI artifacts. diff --git a/internal/config/config.go b/internal/config/config.go index 857243f..0c5a551 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,11 +2,13 @@ package config import ( "context" + "encoding/hex" "errors" "fmt" "os" "path/filepath" "runtime" + "strings" "time" "github.com/gofrs/flock" @@ -204,7 +206,36 @@ func OAuthTokenPath(configPath, profile string) string { return "" } base := filepath.Dir(configPath) - return filepath.Join(base, "oauth", profile+".json") + return filepath.Join(base, "oauth", oauthProfileFilename(profile)) +} + +func oauthProfileFilename(profile string) string { + if isPortableProfileFilename(profile) { + return profile + ".json" + } + return "~" + hex.EncodeToString([]byte(profile)) + ".json" +} + +func isPortableProfileFilename(profile string) bool { + if profile == "" || profile == "." || profile == ".." || strings.HasSuffix(profile, ".") { + return false + } + for _, char := range profile { + if (char >= 'a' && char <= 'z') || (char >= '0' && char <= '9') || + char == '-' || char == '_' || char == '.' { + continue + } + return false + } + stem := strings.ToUpper(strings.SplitN(profile, ".", 2)[0]) + if stem == "CON" || stem == "PRN" || stem == "AUX" || stem == "NUL" { + return false + } + if len(stem) == 4 && (strings.HasPrefix(stem, "COM") || strings.HasPrefix(stem, "LPT")) && + stem[3] >= '1' && stem[3] <= '9' { + return false + } + return true } func (c *Config) normalize() { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9523bee..7613aaf 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -132,6 +132,48 @@ func TestOAuthTokenPath(t *testing.T) { } } +func TestOAuthTokenPathContainsUnsafeProfiles(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "spogo", "config.toml") + oauthDir := filepath.Join(filepath.Dir(configPath), "oauth") + unsafeProfiles := []string{ + "../other", + "work/personal", + `work\personal`, + ".", + "..", + "profile.", + "CON", + "com1.txt", + "personal account", + "00a ", + "00G ", + "WORK", + "Work", + } + seen := map[string]string{} + for _, profile := range unsafeProfiles { + path := OAuthTokenPath(configPath, profile) + if filepath.Dir(path) != oauthDir { + t.Fatalf("profile %q escaped OAuth directory: %s", profile, path) + } + name := filepath.Base(path) + if !strings.HasPrefix(name, "~") || filepath.Ext(name) != ".json" { + t.Fatalf("profile %q was not safely encoded: %s", profile, name) + } + if strings.ContainsAny(name, `/\`) { + t.Fatalf("profile %q retained a path separator: %s", profile, name) + } + collisionKey := strings.ToLower(name) + if previous, ok := seen[collisionKey]; ok { + t.Fatalf("profiles %q and %q collided at %s", previous, profile, name) + } + seen[collisionKey] = profile + } + if got := filepath.Base(OAuthTokenPath(configPath, "work.prod-1")); got != "work.prod-1.json" { + t.Fatalf("portable profile path changed: %s", got) + } +} + func TestLoadInvalid(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "bad.toml")