diff --git a/.env.example b/.env.example index 0a452c9f..0bb6e4cb 100644 --- a/.env.example +++ b/.env.example @@ -1 +1,5 @@ -GHGLANCE_TOKEN= +# "Sign in with GitHub" (a GitHub OAuth App); all three are required. +GHGLANCE_OAUTH_CLIENT_ID= +GHGLANCE_OAUTH_CLIENT_SECRET= +# External origin; the OAuth App's callback URL is /auth/callback. +GHGLANCE_PUBLIC_URL= diff --git a/README.md b/README.md index e83eb9ab..6ee5f1c3 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,7 @@ ghglance -user tiennm99 -themes dracula -include-org-repos -out output | Flag | Default | Description | | ------------------- | --------------- | ---------------------------------------------------------------------- | | `-user` | *(required)* | GitHub username | -| `-token` | `$GITHUB_TOKEN` | Personal access token | +| `-token` | `$GITHUB_TOKEN` | Personal access token (not used by `-serve`) | | `-out` | `output` | Output directory (`//…svg`) | | `-themes` | `dracula` | Comma-separated theme ids, or `all` | | `-tz` | `Local` | IANA timezone for productive-time cards | @@ -177,23 +177,32 @@ ghglance -user tiennm99 -themes dracula -include-org-repos -out output | `-list-themes` | | Print available theme ids and exit | | `-serve` | | Run the [web UI](#run-the-web-ui) on this address (e.g. `:8080`) instead of generating once | | `-data-dir` | `data` | Web UI only: directory holding generated cards | -| `-cooldown` | `6h` | Web UI only: minimum age of a user's cards before a token-less submission regenerates them | +| `-cooldown` | `6h` | Web UI only: minimum age of a user's cards before someone signed in as another account regenerates them | | `-retention` | `24h` | Web UI only: delete a user's cards this long after they were generated, `0` = keep forever | | `-workers` | `2` | Web UI only: concurrent generation jobs | +| `-oauth-client-id` | `$GHGLANCE_OAUTH_CLIENT_ID` | Web UI only, required: GitHub OAuth App client ID for [Sign in with GitHub](#sign-in-with-github) | +| `-oauth-client-secret` | `$GHGLANCE_OAUTH_CLIENT_SECRET` | Web UI only, required: that OAuth App's client secret (never printed by `-help`) | +| `-public-url` | `$GHGLANCE_PUBLIC_URL` | Web UI only, required: the site's external origin; the OAuth callback is `/auth/callback` | -The five web UI flags are server-only: the Action (`action.yml`, -`entrypoint.sh`) does not expose them. +The web UI flags above (`-serve` through `-public-url`) are server-only: +the Action (`action.yml`, `entrypoint.sh`) does not expose them. ## Run the web UI `-serve` turns the binary into a small web app: a form takes a GitHub -username plus options, a background job renders all sixteen cards in every -theme, and `/u/` shows them again with a theme picker and -copyable embed URLs. Cards are stored on disk and survive restarts. +username plus options, the visitor signs in with GitHub, a background job +renders all sixteen cards in every theme on that sign-in's token, and +`/u/` shows them again with a theme picker and copyable embed +URLs. Cards are stored on disk and survive restarts. + +[Sign in with GitHub](#sign-in-with-github) is required: the server has no +GitHub token of its own, and `-serve` refuses to start until the OAuth +client ID, client secret and public URL are all set. ```sh -export GITHUB_TOKEN=ghp_xxx -ghglance -serve :8080 -data-dir data -retention 24h +export GHGLANCE_OAUTH_CLIENT_SECRET=xxxx # keep the secret out of the command line +ghglance -serve :8080 -data-dir data -retention 24h \ + -oauth-client-id Ov23xxxx -public-url http://localhost:8080 # open http://localhost:8080 ``` @@ -203,32 +212,23 @@ ghglance -serve :8080 -data-dir data -retention 24h | `/u/` | The user's cards (`?theme=` picks the theme), or job progress while one runs | | `/u///.svg` | One card, embeddable in a README | | `/u//status` | Job status as JSON, polled by the progress page | +| `/auth/start` | Validates the form and redirects to GitHub's consent page | +| `/auth/callback` | Finishes the sign-in and queues the job | | `/healthz` | Liveness probe | How submissions are handled: -- **Server token.** Submissions without a token use the server's - `GITHUB_TOKEN`, with private repos and org repos forced off. That alone - does not hide private work: GitHub counts every private contribution a - token can see in the totals and the calendar. So the server token must be - public-only (a classic PAT with just `read:user`, or a fine-grained token - with public repositories only); a token with `repo` scope, or one that can - list any private repository, is refused for token-less jobs and logged at - startup. The token owner's own username is refused without a token too. -- **Submitter's token.** An optional token in the form is used for that one - job, then dropped: never logged, never written to disk. When it belongs to - the username being generated, private repos count by default and the - cooldown is skipped. A token that belongs to someone else renders public - data only, does not skip the cooldown, and is refused outright if it can - read private repositories. The cards it renders are public on the site +- **Every job runs on a sign-in token.** The token is used for that one + job, then revoked: never logged, never written to disk. Signed in as the + username being generated, the ticked private and org repos count and the + cooldown is skipped. Signed in as someone else, the job renders public + data only, does not skip the cooldown, and is refused outright if the + token can read private repositories. The cards are public on the site like any other. - A "Create a token on GitHub" button beside the field opens GitHub's - new-token page with a classic token's `repo` and `read:user` scopes - pre-ticked. - **Failures.** A job that fails or times out at any fetch stage publishes nothing, so an earlier complete set stays in place. -- **Cooldown.** Without a token, cards younger than `-cooldown` are shown - instead of regenerated. +- **Cooldown.** Cards younger than `-cooldown` are not regenerated by a + sign-in as another account; the owner's own sign-in skips the wait. - **Retention.** Cards are deleted `-retention` (default `24h`) after they were generated, checked at startup and hourly. The user's page then offers a fresh generation, and embedded card URLs return 404 until @@ -239,6 +239,52 @@ How submissions are handled: reverse proxy on a private or loopback address, the client address comes from the last `X-Forwarded-For` hop. +### Sign in with GitHub + +A visitor ticks their options, clicks **Sign in with GitHub & generate**, +approves once on GitHub, and lands on their cards page while the job runs. + +- **Scopes follow the ticks**, never more: `read:user` for public data; + `repo read:user` when private repos are ticked (GitHub has no read-only + private scope); plus `read:org` when org repos are ticked too. The page + shows the list next to the button and updates it as boxes change. + Private repos start unticked. +- **One token per generation.** The server exchanges GitHub's code (with + `state` bound to a short-lived cookie and PKCE `S256`) for a token, runs + one job with it, then revokes it through GitHub's API, whether the job + succeeded, failed or was dropped at shutdown. The token is never logged, + stored or sent to the browser. +- **Fewer permissions granted than asked** downgrade the job to what was + granted, and the user's page says so. Cancelling on GitHub returns to the + form with the options kept. +- **More permissions granted than asked** stop the sign-in: GitHub folds + every scope a user once granted the app into each new token, so a + public-only sign-in after an earlier private one comes back with `repo`. + The token is revoked, nothing is generated, and the form explains how to + match the ticks or revoke the app under GitHub **Settings > Applications + > Authorized OAuth Apps**. +- **Another account.** Signing in as `alice` to generate `bob` renders + public data only, and is refused if private repos were ticked (the token + would then read private repos); untick them to generate someone else. + +The browser binding cookie is `__Host-ghglance_oauth` when the public URL +is https, so another site on a sibling subdomain cannot plant it. A plain +`http://` public URL cannot use that prefix, which leaves the binding +weaker; use https outside local testing. + +To set it up, register an OAuth App: GitHub **Settings > Developer +settings > OAuth Apps > New OAuth App**, with + +- **Homepage URL**: the site's address, e.g. `https://ghglance.example.com` +- **Authorization callback URL**: `/auth/callback`, e.g. + `https://ghglance.example.com/auth/callback` + +Then generate a client secret and give the server all three values +(`-oauth-client-id`, `-oauth-client-secret`, `-public-url`, or the +`GHGLANCE_OAUTH_CLIENT_ID`, `GHGLANCE_OAUTH_CLIENT_SECRET` and +`GHGLANCE_PUBLIC_URL` environment variables). With any of them missing, +`-serve` exits at startup with an error naming the missing settings. + Each user takes about 9 MB on disk for an active profile (16 cards × every theme). ### Deploy with Docker Compose or Coolify @@ -253,17 +299,17 @@ In Coolify: 1. Create a resource from this Git repository with the **Docker Compose** build pack and compose file `/compose.yml`. -2. Set `GHGLANCE_TOKEN` (see [`.env.example`](./.env.example)) to a - public-only token: a classic PAT with only `read:user`, never `repo`. - `compose.yml` requires it and passes it to the container as - `GITHUB_TOKEN`. The distinct name keeps a `GITHUB_TOKEN` exported in your - shell from silently replacing it during `docker compose up`. +2. Register the [OAuth App](#sign-in-with-github) and set + `GHGLANCE_OAUTH_CLIENT_ID`, `GHGLANCE_OAUTH_CLIENT_SECRET` and + `GHGLANCE_PUBLIC_URL` (see [`.env.example`](./.env.example)); the public + URL is the domain from step 3, e.g. `https://ghglance.sg.miti99.com`. + `compose.yml` requires all three and refuses to start without them. 3. Keep the generated domain or set your own on the `ghglance` service, then deploy. -On a plain Docker host, copy `.env.example` to `.env`, fill in the token, -add a `ports: ["8080:8080"]` entry to the service, and run (`.dockerignore` -keeps `.env` and `data/` out of the image context): +On a plain Docker host, copy `.env.example` to `.env`, fill in the three +sign-in values, add a `ports: ["8080:8080"]` entry to the service, and run +(`.dockerignore` keeps `.env` and `data/` out of the image context): ```sh docker compose up -d --build diff --git a/compose.yml b/compose.yml index f6d8d99e..437b8f66 100644 --- a/compose.yml +++ b/compose.yml @@ -11,7 +11,11 @@ services: - "8080" environment: - SERVICE_FQDN_GHGLANCE_8080 - - GITHUB_TOKEN=${GHGLANCE_TOKEN:?set a public-only GitHub token} + # "Sign in with GitHub" (a GitHub OAuth App) is required: every + # generation runs on the visitor's own sign-in token. + - GHGLANCE_OAUTH_CLIENT_ID=${GHGLANCE_OAUTH_CLIENT_ID:?set the GitHub OAuth App client ID} + - GHGLANCE_OAUTH_CLIENT_SECRET=${GHGLANCE_OAUTH_CLIENT_SECRET:?set the GitHub OAuth App client secret} + - GHGLANCE_PUBLIC_URL=${GHGLANCE_PUBLIC_URL:?set the external origin, e.g. https://ghglance.example.com} volumes: - ghglance-data:/data healthcheck: diff --git a/docs/deployment-guide.md b/docs/deployment-guide.md index b37ade56..82df52a6 100644 --- a/docs/deployment-guide.md +++ b/docs/deployment-guide.md @@ -108,10 +108,21 @@ the `ghglance-data` volume, and health-checks `/healthz` with busybox | Variable | Needed for | | --- | --- | -| `GHGLANCE_TOKEN` | Required by `compose.yml`, which passes it to the container as `GITHUB_TOKEN` for token-less submissions. Must be public-only: a classic PAT with just `read:user`. A token with `repo` scope or any private-repo access is refused for token-less jobs (GitHub would count private contributions in totals and calendars). | +| `GHGLANCE_OAUTH_CLIENT_ID` | Required. Client ID of the GitHub OAuth App behind "Sign in with GitHub" (`-oauth-client-id`). | +| `GHGLANCE_OAUTH_CLIENT_SECRET` | Required. That OAuth App's client secret (`-oauth-client-secret`). Never logged or printed. | +| `GHGLANCE_PUBLIC_URL` | Required. The site's external origin, e.g. `https://ghglance.sg.miti99.com` (`-public-url`). The OAuth App's callback URL must be exactly `/auth/callback`. | -Coolify: create a Docker Compose resource from this repo, compose file -`/compose.yml`, set `GHGLANCE_TOKEN`, assign the domain, deploy. Server flags +The web UI is sign-in only: the server holds no GitHub token, and every +generation runs on the visitor's OAuth token, revoked when the job ends. +`compose.yml` refuses to start while any of the three variables is empty, +and `-serve` exits with an error naming the missing settings. + +Coolify: register an OAuth App (GitHub **Settings > Developer settings > +OAuth Apps > New OAuth App**; homepage URL = the domain, authorization +callback URL = `/auth/callback`) and generate a client secret. +Then create a Docker Compose resource from this repo, compose file +`/compose.yml`, set the three variables, assign the domain, and deploy; +the startup log prints the callback URL it uses. Server flags (`-cooldown`, `-retention`, `-workers`, `-timeout`) are changed by editing `command:` in `compose.yml`. Steps for a plain Docker host and the request-handling rules are in the README's "Run the web UI" section. diff --git a/docs/system-architecture.md b/docs/system-architecture.md index 6188ac20..7b2948b5 100644 --- a/docs/system-architecture.md +++ b/docs/system-architecture.md @@ -152,13 +152,18 @@ Light themes (`default`, `github`, `nord_bright`, etc.) use `StrokeOpacity: 1` w CLI path. ``` -POST /generate ─► validate ─► rate limit / cooldown ─► Queue (dedup per user) - │ -workers goroutines - ▼ - github.Collect ─► Store.Publish (every theme) - │ -GET /u/{user} ◄── meta.json + card list ◄─────────────────┘ +POST /auth/start ─► validate ─► rate limit ─► pending sign-in (state, PKCE verifier; memory, 10 min) + ─► 302 github.com/login/oauth/authorize (scope from ticks, state, S256 challenge) +GET /auth/callback ─► state == cookie, single use ─► POST /login/oauth/access_token + ─► narrow options to granted scopes (wider than ticked: revoke, refuse) + ─► Queue (dedup per user) ─► -workers goroutines + │ token owner check, cooldown + ▼ + github.Collect ─► Store.Publish (every theme) + │ +GET /u/{user} ◄── meta.json + card list ◄──────────┘ GET /u/{user}/{theme}/{card}.svg ◄── os.Root read +job ends ─► DELETE api.github.com/applications/{client_id}/token ``` - **Storage.** `/` (lowercased login) is a symlink into @@ -176,24 +181,42 @@ GET /u/{user}/{theme}/{card}.svg ◄── os.Root read `-workers` concurrent, `-timeout` each, capacity 64. `SIGTERM` stops the HTTP server, cancels running jobs and drops queued ones; nothing is published mid-render. -- **Tokens.** GitHub folds every private contribution a token can see into - totals and calendars, so repo filters alone cannot keep cards public. Each - job first identifies its token (`viewer` query: login, a one-repo - `privacy: PRIVATE` probe, and a classic token's `X-OAuth-Scopes`). - Token-less jobs use the server's `GITHUB_TOKEN` (identified once and - cached) with private and org-repo scope forced off; they are refused when - that token can read private repos, and for the token owner's own login. A - submitter's token keeps its scope and skips the cooldown only for its own - login; for anyone else it is refused if private-capable, otherwise forced - to public scope under the cooldown. It lives only on the job and is - cleared when it ends. +- **Tokens.** The server has no GitHub token of its own: every job runs on + the token of the visitor's sign-in. GitHub folds every private + contribution a token can see into totals and calendars, so repo filters + alone cannot keep cards public. Each job first identifies its token + (`viewer` query: login, a one-repo `privacy: PRIVATE` probe, and a + classic token's `X-OAuth-Scopes`). Signed in as the target login, the job + keeps the ticked scope and skips the cooldown; as anyone else it is + refused if private-capable, otherwise forced to public scope under the + cooldown. The token lives only on the job and is cleared when it ends. +- **Sign in with GitHub** (`internal/web/oauth.go`). OAuth App web flow, + required: `-serve` exits at startup unless `-oauth-client-id`, + `-oauth-client-secret` and `-public-url` are all set. Scopes come from + the ticked options (`read:user`; `repo` for private; `read:org` on top + for org repos). `/auth/start` parks the validated submission under a + 256-bit random `state` (10-minute TTL, 1,000 entries max) and binds it to + the browser with an `HttpOnly`, `SameSite=Lax` cookie on `/`, named + `__Host-ghglance_oauth` and `Secure` when the public URL is https so a + sibling subdomain cannot plant it (plain `ghglance_oauth` over http, a + weaker binding). The callback compares state and cookie in constant time, + consumes the entry, exchanges the code with the PKCE verifier (15 s + timeout), drops options whose scope GitHub did not grant, revokes and + refuses a token carrying scopes the ticks did not ask for (GitHub folds + earlier grants into new tokens), and hands the token to the queue. The + worker revokes it before it reports the job finished; `Stop` revokes the + tokens of queued jobs it drops, and a callback whose job is not queued + revokes at once. Revocation is best effort and logs only the status. + GitHub endpoints come from `OAuthConfig.WebURL`/`APIURL`, so tests run + against `httptest`. The page CSP adds GitHub's origin to `form-action`, + since browsers check a form submission's redirect against it. - **Partial fetches.** The web path runs `github.Collect` with `Strict`, so a failed all-time or commit-history stage fails the job, and a job whose deadline passed is failed even if the fetch returned. The CLI keeps rendering partial data with warnings. - **HTTP hardening.** Strict CSP on pages, `default-src 'none'` + `sandbox` on SVGs, `nosniff`, 16 KiB form limit, `http.CrossOriginProtection` on the - POST, per-client token bucket (burst 5, +1 per 2 min) keyed by IPv4 + POSTs, per-client token bucket (burst 5, +1 per 2 min) keyed by IPv4 address or IPv6 /64, capped at 10,000 tracked clients. ## Extension points diff --git a/internal/web/jobs.go b/internal/web/jobs.go index 6f71f98a..978a6359 100644 --- a/internal/web/jobs.go +++ b/internal/web/jobs.go @@ -30,11 +30,10 @@ const finishedJobTTL = time.Hour const queueCapacity = 64 var ( - errQueueFull = errors.New("the generation queue is full, try again in a few minutes") - errStopped = errors.New("server is shutting down") - errOwner = errors.New("this account owns the server's token, so its cards need your own token") - errServerPrivate = errors.New("this server's GitHub token can read private repositories, so it cannot make public cards; add your own token under Options") - errFresh = errors.New("these cards are recent; your token belongs to another account, so it does not skip the wait") + errQueueFull = errors.New("the generation queue is full, try again in a few minutes") + errStopped = errors.New("server is shutting down") + errNoToken = errors.New("this generation has no sign-in token; sign in with GitHub again") + errFresh = errors.New("these cards are recent; you signed in as another account, so it does not skip the wait") ) // Fetcher runs the GitHub fetch. Tests swap in a fake; the server uses @@ -54,8 +53,9 @@ func (githubFetcher) TokenInfo(ctx context.Context, token string) (github.TokenI return github.NewClient(token).TokenInfo(ctx) } -// job is one queued generation. token is the submitter's own token, held -// only until the job ends and never logged or persisted. +// job is one queued generation. token is the submitter's sign-in token, +// held only until the job ends, never logged or persisted, and revoked on +// GitHub then. type job struct { key string login string @@ -84,12 +84,13 @@ func (s JobStatus) Active() bool { return s.State == stateQueued || s.State == s // Queue runs generation jobs on a fixed worker pool, with at most one queued // or running job per user. type Queue struct { - store *Store - fetcher Fetcher - serverToken string - timeout time.Duration - cooldown time.Duration - now func() time.Time + store *Store + fetcher Fetcher + timeout time.Duration + cooldown time.Duration + now func() time.Time + // revoke deletes a sign-in token on GitHub once its job is over. + revoke func(token string) mu sync.Mutex jobs map[string]*job @@ -97,30 +98,27 @@ type Queue struct { wake chan struct{} closed bool - serverMu sync.Mutex - serverInfo *github.TokenInfo - ctx context.Context cancel context.CancelFunc wg sync.WaitGroup } -func newQueue(store *Store, fetcher Fetcher, serverToken string, timeout, cooldown time.Duration, workers int) *Queue { +func newQueue(store *Store, fetcher Fetcher, revoke func(token string), timeout, cooldown time.Duration, workers int) *Queue { if workers < 1 { workers = 1 } ctx, cancel := context.WithCancel(context.Background()) q := &Queue{ - store: store, - fetcher: fetcher, - serverToken: serverToken, - timeout: timeout, - cooldown: cooldown, - now: time.Now, - jobs: map[string]*job{}, - wake: make(chan struct{}, queueCapacity), - ctx: ctx, - cancel: cancel, + store: store, + fetcher: fetcher, + timeout: timeout, + cooldown: cooldown, + now: time.Now, + revoke: revoke, + jobs: map[string]*job{}, + wake: make(chan struct{}, queueCapacity), + ctx: ctx, + cancel: cancel, } for range workers { q.wg.Add(1) @@ -184,19 +182,35 @@ func (q *Queue) Status(login string) JobStatus { } // Stop cancels running jobs, drops queued ones and waits for the workers. +// Tokens of dropped jobs are revoked before it returns; running jobs revoke +// theirs as their workers wind down. func (q *Queue) Stop() { q.mu.Lock() q.closed = true + var dropped []string for _, j := range q.pending { + dropped = append(dropped, j.token) j.token = "" j.state, j.err = stateFailed, errStopped.Error() } q.pending = nil q.mu.Unlock() q.cancel() + var revokes sync.WaitGroup + for _, token := range dropped { + revokes.Go(func() { q.revokeToken(token) }) + } + revokes.Wait() q.wg.Wait() } +// revokeToken revokes a sign-in token; an empty one has nothing to revoke. +func (q *Queue) revokeToken(token string) { + if q.revoke != nil && token != "" { + q.revoke(token) + } +} + // pruneLocked forgets finished jobs older than finishedJobTTL. func (q *Queue) pruneLocked() { cutoff := q.now().Add(-finishedJobTTL) @@ -227,8 +241,15 @@ func (q *Queue) worker() { err := q.run(j) + // Revoke before reporting the job over, so a finished job never + // leaves a live sign-in token behind. q.mu.Lock() + token := j.token j.token = "" + q.mu.Unlock() + q.revokeToken(token) + + q.mu.Lock() j.finished = q.now() if err != nil { j.state, j.stage, j.err = stateFailed, "", err.Error() @@ -262,35 +283,23 @@ func (q *Queue) run(j *job) error { q.mu.Lock() token := j.token q.mu.Unlock() + if token == "" { + return errNoToken + } opts := j.opts - ownToken := token != "" - if ownToken { - info, err := q.fetcher.TokenInfo(ctx, token) - if err != nil { - return errors.New("GitHub did not accept your token") - } - if !strings.EqualFold(info.Login, j.login) { - if info.CanReadPrivate { - return fmt.Errorf("your token belongs to %s and can read private repositories, so it can only generate cards for %s", info.Login, info.Login) - } - ownToken = false - opts.IncludePrivate, opts.IncludeOrgRepos = false, false - if q.store.cooldownLeft(j.login, q.cooldown, q.now()) > 0 { - return errFresh - } - } - } else { - info, err := q.server(ctx) - if err != nil { - return errors.New("could not verify the server token, try again later") - } + info, err := q.fetcher.TokenInfo(ctx, token) + if err != nil { + return errors.New("GitHub did not accept your sign-in token") + } + own := strings.EqualFold(info.Login, j.login) + if !own { if info.CanReadPrivate { - return errServerPrivate + return fmt.Errorf("you signed in as %s with access to private repositories, so you can only generate cards for %s", info.Login, info.Login) } - if info.Login != "" && strings.EqualFold(info.Login, j.login) { - return errOwner + opts.IncludePrivate, opts.IncludeOrgRepos = false, false + if q.store.cooldownLeft(j.login, q.cooldown, q.now()) > 0 { + return errFresh } - token = q.serverToken } cfg := opts.collectConfig() @@ -317,7 +326,7 @@ func (q *Queue) run(j *job) error { q.mu.Unlock() scope := "public" - if ownToken && opts.IncludePrivate { + if own && opts.IncludePrivate { scope = "private" } return q.store.Publish(profile, Meta{ @@ -328,29 +337,6 @@ func (q *Queue) run(j *job) error { }) } -// server identifies the server token, cached after the first successful -// lookup. An empty server token has no owner and no reach to protect. -func (q *Queue) server(ctx context.Context) (github.TokenInfo, error) { - if q.serverToken == "" { - return github.TokenInfo{}, nil - } - q.serverMu.Lock() - defer q.serverMu.Unlock() - if q.serverInfo != nil { - return *q.serverInfo, nil - } - info, err := q.fetcher.TokenInfo(ctx, q.serverToken) - if err != nil { - log.Printf("server token lookup failed: %v", err) - return github.TokenInfo{}, err - } - if info.CanReadPrivate { - log.Printf("warn: GITHUB_TOKEN can read private repositories; token-less submissions are refused until it is replaced with a public-only token") - } - q.serverInfo = &info - return info, nil -} - // publicError trims a fetch error to something fit for the status page. func publicError(err error) error { if errors.Is(err, context.DeadlineExceeded) { diff --git a/internal/web/oauth.go b/internal/web/oauth.go new file mode 100644 index 00000000..637d2634 --- /dev/null +++ b/internal/web/oauth.go @@ -0,0 +1,451 @@ +package web + +import ( + "bytes" + "cmp" + "context" + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "io" + "log" + "net/http" + "net/url" + "slices" + "strconv" + "strings" + "sync" + "time" +) + +// GitHub's OAuth endpoints. Tests point OAuthConfig at an httptest server. +const ( + defaultOAuthWebURL = "https://github.com" + defaultOAuthAPIURL = "https://api.github.com" +) + +const ( + // loginTTL is how long a started sign-in waits for GitHub's callback; + // GitHub's own authorization codes expire after ten minutes too. + loginTTL = 10 * time.Minute + // maxPendingLogins caps sign-ins waiting for a callback. The submission + // rate limit already bounds one client; this bounds them all. + maxPendingLogins = 1000 + // oauthCookie binds the browser that started a sign-in to its state. + // Over https it carries the __Host- prefix, so a sibling subdomain + // cannot plant one; a plain-http public URL cannot use the prefix and + // gets the weaker unprefixed name. + oauthCookie = "__Host-ghglance_oauth" + oauthCookieInsecure = "ghglance_oauth" + // oauthTimeout bounds one call to GitHub's token or revoke endpoint. + oauthTimeout = 15 * time.Second +) + +// OAuthConfig configures "Sign in with GitHub" through a GitHub OAuth App. +// ClientID, ClientSecret and PublicURL are all required. +type OAuthConfig struct { + ClientID string + ClientSecret string + // PublicURL is the site's external origin; the callback GitHub + // redirects to is PublicURL + "/auth/callback" and must match the one + // registered on the OAuth App exactly. + PublicURL string + // WebURL and APIURL override github.com and api.github.com. + WebURL string + APIURL string +} + +// missing names the required values that are empty, in the order client +// ID, client secret, public URL. +func (c OAuthConfig) missing() []string { + var missing []string + if c.ClientID == "" { + missing = append(missing, "client ID") + } + if c.ClientSecret == "" { + missing = append(missing, "client secret") + } + if c.PublicURL == "" { + missing = append(missing, "public URL") + } + return missing +} + +// oauthApp talks to GitHub on behalf of the OAuth App. It never logs a +// token, a code or the client secret. +type oauthApp struct { + clientID string + clientSecret string + redirectURI string + secureCookie bool + cookieName string + webURL string + apiURL string + client *http.Client +} + +func newOAuthApp(c OAuthConfig) (*oauthApp, error) { + if missing := c.missing(); len(missing) > 0 { + return nil, fmt.Errorf("sign in with GitHub is required but not configured: missing OAuth %s", strings.Join(missing, ", ")) + } + pub, err := url.Parse(strings.TrimRight(c.PublicURL, "/")) + if err != nil || (pub.Scheme != "http" && pub.Scheme != "https") || pub.Host == "" || pub.RawQuery != "" || pub.Fragment != "" { + return nil, fmt.Errorf("public URL %q must be an absolute http(s) URL such as https://ghglance.example.com", c.PublicURL) + } + app := &oauthApp{ + clientID: c.ClientID, + clientSecret: c.ClientSecret, + redirectURI: pub.String() + "/auth/callback", + secureCookie: pub.Scheme == "https", + cookieName: oauthCookieInsecure, + webURL: strings.TrimRight(cmp.Or(c.WebURL, defaultOAuthWebURL), "/"), + apiURL: strings.TrimRight(cmp.Or(c.APIURL, defaultOAuthAPIURL), "/"), + client: &http.Client{Timeout: oauthTimeout}, + } + if app.secureCookie { + app.cookieName = oauthCookie + } + return app, nil +} + +// webOrigin is the scheme and host of the authorize page, which the page +// CSP must allow as a form-submission redirect target. +func (a *oauthApp) webOrigin() string { + u, err := url.Parse(a.webURL) + if err != nil { + return defaultOAuthWebURL + } + return u.Scheme + "://" + u.Host +} + +// oauthScopes derives the scopes a sign-in asks for from the ticked +// options, never more: public data needs only read:user, private repos +// need repo (GitHub has no read-only private scope), and org repos add +// read:org on top of private access. +func oauthScopes(o Options) string { + if !o.IncludePrivate { + return "read:user" + } + if o.IncludeOrgRepos { + return "repo read:user read:org" + } + return "repo read:user" +} + +// scopeSet splits GitHub's comma- or space-separated scope list. +func scopeSet(scopes string) map[string]bool { + has := map[string]bool{} + for _, s := range strings.FieldsFunc(scopes, func(r rune) bool { return r == ',' || r == ' ' }) { + has[s] = true + } + return has +} + +// grantScopes narrows o to the scopes GitHub actually granted, which the +// user may have reduced on the consent screen. It reports which option was +// dropped ("" when none): "private" or "org". +func grantScopes(o Options, granted string) (Options, string) { + has := scopeSet(granted) + if o.IncludePrivate && !has["repo"] { + o.IncludePrivate, o.IncludeOrgRepos = false, false + return o, "private" + } + if o.IncludePrivate && o.IncludeOrgRepos && !has["read:org"] && !has["write:org"] && !has["admin:org"] { + o.IncludeOrgRepos = false + return o, "org" + } + return o, "" +} + +// extraScopes lists the granted scopes the ticked options did not ask +// for. GitHub folds every scope a user ever granted the app into each new +// token and skips the consent screen, so a public-only sign-in can come +// back with repo from an earlier private one. A job never runs with a +// token wider than the ticks. +func extraScopes(o Options, granted string) []string { + asked := scopeSet(oauthScopes(o)) + var extra []string + for s := range scopeSet(granted) { + if !asked[s] { + extra = append(extra, s) + } + } + slices.Sort(extra) + return extra +} + +// authorizeURL is GitHub's consent page for one sign-in. +func (a *oauthApp) authorizeURL(state, challenge, scope, login string) string { + q := url.Values{ + "client_id": {a.clientID}, + "redirect_uri": {a.redirectURI}, + "scope": {scope}, + "state": {state}, + "code_challenge": {challenge}, + "code_challenge_method": {"S256"}, + "login": {login}, + } + return a.webURL + "/login/oauth/authorize?" + q.Encode() +} + +var errExchange = errors.New("GitHub sign-in failed, try again") + +// exchange trades an authorization code and its PKCE verifier for a token +// and the scopes GitHub granted. +func (a *oauthApp) exchange(ctx context.Context, code, verifier string) (token, scope string, err error) { + ctx, cancel := context.WithTimeout(ctx, oauthTimeout) + defer cancel() + form := url.Values{ + "client_id": {a.clientID}, + "client_secret": {a.clientSecret}, + "code": {code}, + "redirect_uri": {a.redirectURI}, + "code_verifier": {verifier}, + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, a.webURL+"/login/oauth/access_token", strings.NewReader(form.Encode())) + if err != nil { + return "", "", err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Accept", "application/json") + resp, err := a.client.Do(req) + if err != nil { + log.Printf("oauth: token exchange failed: %v", err) + return "", "", errExchange + } + defer resp.Body.Close() + var body struct { + AccessToken string `json:"access_token"` + Scope string `json:"scope"` + Error string `json:"error"` + } + if err := json.NewDecoder(io.LimitReader(resp.Body, 64<<10)).Decode(&body); err != nil || resp.StatusCode != http.StatusOK { + log.Printf("oauth: token exchange failed: status %d", resp.StatusCode) + return "", "", errExchange + } + if body.Error != "" || !tokenRE.MatchString(body.AccessToken) { + log.Printf("oauth: token exchange refused: %s", strconv.Quote(body.Error)) + return "", "", errExchange + } + return body.AccessToken, body.Scope, nil +} + +// revoke deletes the grant's token on GitHub. It is best effort: a token +// that cannot be revoked still never leaves the server, and the user can +// revoke it under Settings > Applications. +func (a *oauthApp) revoke(token string) { + if token == "" { + return + } + ctx, cancel := context.WithTimeout(context.Background(), oauthTimeout) + defer cancel() + body, _ := json.Marshal(map[string]string{"access_token": token}) + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, + a.apiURL+"/applications/"+url.PathEscape(a.clientID)+"/token", bytes.NewReader(body)) + if err != nil { + log.Printf("oauth: revoke failed: %v", err) + return + } + req.SetBasicAuth(a.clientID, a.clientSecret) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + resp, err := a.client.Do(req) + if err != nil { + log.Printf("oauth: revoke failed: %v", err) + return + } + io.Copy(io.Discard, io.LimitReader(resp.Body, 64<<10)) + resp.Body.Close() + if resp.StatusCode != http.StatusNoContent { + log.Printf("oauth: revoke returned status %d", resp.StatusCode) + } +} + +// pendingLogin is a submission waiting for GitHub's callback. +type pendingLogin struct { + sub submission + form formValues + verifier string + expires time.Time +} + +// pendingLogins holds started sign-ins by state, in memory only. +type pendingLogins struct { + now func() time.Time + + mu sync.Mutex + entries map[string]*pendingLogin +} + +func newPendingLogins() *pendingLogins { + return &pendingLogins{now: time.Now, entries: map[string]*pendingLogin{}} +} + +var errTooManyLogins = errors.New("too many sign-ins are in progress, try again in a few minutes") + +// add stores p under a fresh random state and returns the state. +func (l *pendingLogins) add(p *pendingLogin) (string, error) { + state, err := randomString() + if err != nil { + return "", err + } + l.mu.Lock() + defer l.mu.Unlock() + now := l.now() + if len(l.entries) >= maxPendingLogins { + for k, e := range l.entries { + if !now.Before(e.expires) { + delete(l.entries, k) + } + } + } + if len(l.entries) >= maxPendingLogins { + return "", errTooManyLogins + } + p.expires = now.Add(loginTTL) + l.entries[state] = p + return state, nil +} + +// take removes and returns the sign-in for state, or nil when it is +// unknown, already used or expired. A state is good for one callback. +func (l *pendingLogins) take(state string) *pendingLogin { + l.mu.Lock() + defer l.mu.Unlock() + p, ok := l.entries[state] + if !ok { + return nil + } + delete(l.entries, state) + if !l.now().Before(p.expires) { + return nil + } + return p +} + +// randomString is 256 random bits, base64url-encoded: 43 characters, which +// also fits PKCE's 43–128 character verifier. +func randomString() (string, error) { + b := make([]byte, 32) + if _, err := rand.Read(b); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(b), nil +} + +// pkceChallenge is the S256 code challenge for verifier. +func pkceChallenge(verifier string) string { + sum := sha256.Sum256([]byte(verifier)) + return base64.RawURLEncoding.EncodeToString(sum[:]) +} + +// handleAuthStart validates the generation form, parks it under a random +// state and sends the browser to GitHub's consent page. +func (s *Server) handleAuthStart(w http.ResponseWriter, r *http.Request) { + sub, form, ok := s.readSubmission(w, r) + if !ok { + return + } + if s.queue.Status(sub.Login).Active() { + http.Redirect(w, r, "/u/"+url.PathEscape(userKey(sub.Login))+"?notice=pending", http.StatusSeeOther) + return + } + verifier, err := randomString() + if err != nil { + s.render(w, http.StatusInternalServerError, "index", pageData{Title: "ghglance", Error: capitalize(errExchange.Error()) + ".", Form: form}) + return + } + state, err := s.logins.add(&pendingLogin{sub: sub, form: form, verifier: verifier}) + if err != nil { + status, msg := http.StatusInternalServerError, errExchange + if errors.Is(err, errTooManyLogins) { + status, msg = http.StatusServiceUnavailable, errTooManyLogins + } + s.render(w, status, "index", pageData{Title: "ghglance", Error: capitalize(msg.Error()) + ".", Form: form}) + return + } + http.SetCookie(w, &http.Cookie{ + Name: s.oauth.cookieName, + Value: state, + Path: "/", + MaxAge: int(loginTTL.Seconds()), + HttpOnly: true, + Secure: s.oauth.secureCookie, + SameSite: http.SameSiteLaxMode, + }) + w.Header().Set("Cache-Control", "no-store") + http.Redirect(w, r, s.oauth.authorizeURL(state, pkceChallenge(verifier), oauthScopes(sub.Options), sub.Login), http.StatusFound) +} + +// handleAuthCallback finishes a sign-in: it checks the state against the +// browser's cookie, trades the code for a token and queues the job, which +// enforces the ownership, privacy and cooldown rules once it knows whose +// token it holds. The token is revoked when the job ends. +func (s *Server) handleAuthCallback(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-store") + http.SetCookie(w, &http.Cookie{ + Name: s.oauth.cookieName, Value: "", Path: "/", MaxAge: -1, + HttpOnly: true, Secure: s.oauth.secureCookie, SameSite: http.SameSiteLaxMode, + }) + q := r.URL.Query() + state := q.Get("state") + c, err := r.Cookie(s.oauth.cookieName) + if state == "" || err != nil || subtle.ConstantTimeCompare([]byte(c.Value), []byte(state)) != 1 { + s.renderMessage(w, http.StatusBadRequest, "Sign-in failed", + "This sign-in was not started in this browser, or it was already used. Start again from the form.") + return + } + p := s.logins.take(state) + if p == nil { + s.renderMessage(w, http.StatusBadRequest, "Sign-in expired", + "This sign-in expired or was already used. Start again from the form.") + return + } + + if e := q.Get("error"); e != "" { + msg := "GitHub sign-in failed. Your options are kept below; try again." + if e == "access_denied" { + msg = "GitHub sign-in was cancelled, so nothing was generated. Your options are kept below." + } else { + log.Printf("oauth: callback error %s", strconv.Quote(e)) + } + s.render(w, http.StatusOK, "index", pageData{Title: "ghglance", Error: msg, Form: p.form}) + return + } + code := q.Get("code") + if code == "" { + s.render(w, http.StatusBadRequest, "index", pageData{Title: "ghglance", Error: capitalize(errExchange.Error()) + ".", Form: p.form}) + return + } + token, granted, err := s.oauth.exchange(r.Context(), code, p.verifier) + if err != nil { + s.render(w, http.StatusBadGateway, "index", pageData{Title: "ghglance", Error: capitalize(err.Error()) + ".", Form: p.form}) + return + } + + if extra := extraScopes(p.sub.Options, granted); len(extra) > 0 { + s.oauth.revoke(token) + s.render(w, http.StatusOK, "index", pageData{Title: "ghglance", Form: p.form, Error: fmt.Sprintf( + "GitHub returned a token with more access than your ticks ask for (%s), because you granted it to ghglance before. "+ + "Nothing was generated and the token was revoked. Tick the matching options, or revoke ghglance under GitHub "+ + "Settings > Applications > Authorized OAuth Apps and sign in again.", strings.Join(extra, ", "))}) + return + } + + sub := p.sub + sub.Token = token + var dropped string + sub.Options, dropped = grantScopes(sub.Options, granted) + notice := "" + if dropped != "" { + notice = "granted-" + dropped + } + if !s.enqueue(w, r, sub, p.form, notice) { + s.oauth.revoke(token) + } +} diff --git a/internal/web/oauth_test.go b/internal/web/oauth_test.go new file mode 100644 index 00000000..81c340ff --- /dev/null +++ b/internal/web/oauth_test.go @@ -0,0 +1,576 @@ +package web + +import ( + "bytes" + "cmp" + "encoding/base64" + "encoding/json" + "io/fs" + "log" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "slices" + "strings" + "sync" + "testing" + "time" + + "github.com/tiennm99/ghglance/internal/github" +) + +const ( + testClientID = "Iv1testclientid" + testClientSecret = "test-client-secret-value" + testPublicURL = "https://ghglance.example" + testCode = "good-code" + // oauthToken is what the fake GitHub issues for testCode. + oauthToken = "gho_oauthTOKENvalue0123456789abcdef" +) + +// fakeGitHub plays GitHub's OAuth token and revoke endpoints. +type fakeGitHub struct { + srv *httptest.Server + + mu sync.Mutex + scope string // scopes granted with the token; "" grants what was asked + asked string // scopes the last sign-in asked for + exchanges []url.Values + revoked []string +} + +func newFakeGitHub(t *testing.T) *fakeGitHub { + t.Helper() + g := &fakeGitHub{} + mux := http.NewServeMux() + mux.HandleFunc("POST /login/oauth/access_token", func(w http.ResponseWriter, r *http.Request) { + r.ParseForm() + g.mu.Lock() + g.exchanges = append(g.exchanges, r.PostForm) + scope := cmp.Or(g.scope, strings.ReplaceAll(g.asked, " ", ",")) + g.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + f := r.PostForm + if r.Header.Get("Accept") != "application/json" || f.Get("client_id") != testClientID || + f.Get("client_secret") != testClientSecret || f.Get("code") != testCode || + f.Get("redirect_uri") != testPublicURL+"/auth/callback" || f.Get("code_verifier") == "" { + json.NewEncoder(w).Encode(map[string]string{"error": "bad_verification_code"}) + return + } + json.NewEncoder(w).Encode(map[string]string{"access_token": oauthToken, "token_type": "bearer", "scope": scope}) + }) + mux.HandleFunc("DELETE /applications/{id}/token", func(w http.ResponseWriter, r *http.Request) { + id, secret, ok := r.BasicAuth() + if !ok || id != testClientID || secret != testClientSecret || r.PathValue("id") != testClientID { + w.WriteHeader(http.StatusUnauthorized) + return + } + var body struct { + AccessToken string `json:"access_token"` + } + json.NewDecoder(r.Body).Decode(&body) + g.mu.Lock() + g.revoked = append(g.revoked, body.AccessToken) + g.mu.Unlock() + w.WriteHeader(http.StatusNoContent) + }) + g.srv = httptest.NewServer(mux) + t.Cleanup(g.srv.Close) + return g +} + +func (g *fakeGitHub) exchangeCount() int { + g.mu.Lock() + defer g.mu.Unlock() + return len(g.exchanges) +} + +func (g *fakeGitHub) revokedTokens() []string { + g.mu.Lock() + defer g.mu.Unlock() + return append([]string(nil), g.revoked...) +} + +func newOAuthTestServer(t *testing.T, f *fakeFetcher, g *fakeGitHub) *Server { + t.Helper() + return newServerWith(t, f, g, time.Minute) +} + +// startSignIn posts the form to /auth/start from ip. +func startSignIn(h http.Handler, v url.Values, ip string) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodPost, "/auth/start", strings.NewReader(v.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.RemoteAddr = ip + ":1" + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec +} + +// signIn starts a sign-in and returns GitHub's authorize query and the +// state cookie. The fake GitHub remembers the scopes asked for. +func signIn(t *testing.T, h http.Handler, g *fakeGitHub, v url.Values) (url.Values, *http.Cookie) { + t.Helper() + rec := startSignIn(h, v, "203.0.113.50") + if rec.Code != http.StatusFound { + t.Fatalf("auth start = %d: %s", rec.Code, rec.Body.String()) + } + loc, err := url.Parse(rec.Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + g.mu.Lock() + g.asked = loc.Query().Get("scope") + g.mu.Unlock() + for _, c := range rec.Result().Cookies() { + if c.Name == oauthCookie { + return loc.Query(), c + } + } + t.Fatal("no state cookie set") + return nil, nil +} + +// callback calls /auth/callback with query and an optional cookie. +func callback(h http.Handler, query url.Values, cookie *http.Cookie) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodGet, "/auth/callback?"+query.Encode(), nil) + if cookie != nil { + req.AddCookie(cookie) + } + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec +} + +// lockedBuffer collects log output written from several goroutines. +type lockedBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *lockedBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *lockedBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +func captureLog(t *testing.T) *lockedBuffer { + t.Helper() + buf := &lockedBuffer{} + log.SetOutput(buf) + t.Cleanup(func() { log.SetOutput(os.Stderr) }) + return buf +} + +func TestOAuthScopesFollowTicks(t *testing.T) { + cases := []struct { + private, org bool + want string + }{ + {false, false, "read:user"}, + {false, true, "read:user"}, + {true, false, "repo read:user"}, + {true, true, "repo read:user read:org"}, + } + for _, c := range cases { + if got := oauthScopes(Options{IncludePrivate: c.private, IncludeOrgRepos: c.org}); got != c.want { + t.Errorf("oauthScopes(private=%v, org=%v) = %q, want %q", c.private, c.org, got, c.want) + } + } +} + +func TestGrantScopesDowngrades(t *testing.T) { + both := Options{IncludePrivate: true, IncludeOrgRepos: true} + cases := []struct { + asked Options + granted string + want Options + dropped string + }{ + {both, "repo,read:user,read:org", both, ""}, + {both, "read:org, repo, user", both, ""}, + {both, "repo,read:user", Options{IncludePrivate: true}, "org"}, + {both, "admin:org,repo", both, ""}, + {both, "read:user", Options{}, "private"}, + {Options{IncludePrivate: true}, "", Options{}, "private"}, + {Options{IncludeOrgRepos: true}, "read:user", Options{IncludeOrgRepos: true}, ""}, + } + for _, c := range cases { + got, dropped := grantScopes(c.asked, c.granted) + if got != c.want || dropped != c.dropped { + t.Errorf("grantScopes(%+v, %q) = %+v, %q; want %+v, %q", c.asked, c.granted, got, dropped, c.want, c.dropped) + } + } +} + +func TestOAuthStartRedirectsToGitHub(t *testing.T) { + g := newFakeGitHub(t) + s := newOAuthTestServer(t, &fakeFetcher{}, g) + h := s.Handler() + + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + if body := rec.Body.String(); !strings.Contains(body, `action="/auth/start"`) || !strings.Contains(body, "Sign in with GitHub") { + t.Error("index lacks the sign-in form") + } + if csp := rec.Header().Get("Content-Security-Policy"); !strings.Contains(csp, "form-action 'self' "+g.srv.URL+";") { + t.Errorf("CSP does not allow the authorize redirect: %s", csp) + } + + if body := rec.Body.String(); strings.Contains(body, `name="include_private" value="1" checked`) { + t.Error("private repos start ticked") + } + + q, c := signIn(t, h, g, url.Values{ + "user": {"octocat"}, "include_private": {"1"}, "include_org_repos": {"1"}, + "token": {testToken}, // not a form field: ignored, still a sign-in + }) + want := map[string]string{ + "client_id": testClientID, + "redirect_uri": testPublicURL + "/auth/callback", + "scope": "repo read:user read:org", + "code_challenge_method": "S256", + "login": "octocat", + } + for k, v := range want { + if q.Get(k) != v { + t.Errorf("authorize %s = %q, want %q", k, q.Get(k), v) + } + } + if raw, err := base64.RawURLEncoding.DecodeString(q.Get("state")); err != nil || len(raw) < 16 { + t.Errorf("state %q is not at least 128 random bits", q.Get("state")) + } + if q.Get("code_challenge") == "" || q.Get("client_secret") != "" { + t.Errorf("authorize query = %v", q) + } + if c.Value != q.Get("state") || !c.HttpOnly || !c.Secure || c.SameSite != http.SameSiteLaxMode || c.Path != "/" || c.Domain != "" || c.MaxAge != 600 { + t.Errorf("state cookie = %+v", c) + } + + // Public data only asks for read:user, whatever the org box says. + q, _ = signIn(t, h, g, url.Values{"user": {"octocat"}, "include_org_repos": {"1"}}) + if q.Get("scope") != "read:user" { + t.Errorf("public sign-in scope = %q", q.Get("scope")) + } + + // The form is validated and rate limited before anything goes to GitHub. + if rec := startSignIn(h, url.Values{"user": {"--bad"}}, "203.0.113.51"); rec.Code != http.StatusBadRequest { + t.Errorf("invalid user = %d", rec.Code) + } + var last int + for range submitBurst + 1 { + last = startSignIn(h, url.Values{"user": {"octocat"}}, "203.0.113.52").Code + } + if last != http.StatusTooManyRequests { + t.Errorf("sign-in past burst = %d, want 429", last) + } + if g.exchangeCount() != 0 { + t.Error("starting a sign-in talked to the token endpoint") + } +} + +func TestOAuthFlowRevokesAfterJob(t *testing.T) { + logs := captureLog(t) + g := newFakeGitHub(t) + f := &fakeFetcher{tokens: map[string]github.TokenInfo{oauthToken: {Login: "octocat", CanReadPrivate: true}}} + s := newOAuthTestServer(t, f, g) + h := s.Handler() + + publishTest(t, s.store, "octocat") // in cooldown: a sign-in skips it + q, c := signIn(t, h, g, url.Values{"user": {"octocat"}, "tz": {"Asia/Saigon"}, "include_private": {"1"}, "commits_per_repo": {"0"}}) + rec := callback(h, url.Values{"code": {testCode}, "state": {q.Get("state")}}, c) + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/u/octocat" { + t.Fatalf("callback = %d %q: %s", rec.Code, rec.Header().Get("Location"), rec.Body.String()) + } + if st := waitIdle(t, s.queue, "octocat"); st.State != stateDone { + t.Fatalf("job = %+v", st) + } + + // PKCE: the verifier sent to the token endpoint hashes to the challenge. + g.mu.Lock() + verifier := g.exchanges[0].Get("code_verifier") + g.mu.Unlock() + if len(verifier) < 43 || pkceChallenge(verifier) != q.Get("code_challenge") { + t.Errorf("verifier %q does not match challenge %q", verifier, q.Get("code_challenge")) + } + + call := f.lastCall() + if call.token != oauthToken || !call.cfg.Options.IncludePrivate || call.cfg.CommitsPerRepo != 0 { + t.Errorf("fetch = %+v", call) + } + if m, err := s.store.Meta("octocat"); err != nil || m.Scope != "private" || m.Options.TZ != "Asia/Saigon" { + t.Errorf("meta = %+v, %v", m, err) + } + if got := g.revokedTokens(); len(got) != 1 || got[0] != oauthToken { + t.Errorf("revoked = %q, want the sign-in token once", got) + } + + // The token, code and client secret never reach disk or the log. + filepath.WalkDir(s.store.dir, func(path string, d fs.DirEntry, err error) error { + if err != nil || !d.Type().IsRegular() { + return err + } + raw, _ := os.ReadFile(path) + if bytes.Contains(raw, []byte(oauthToken)) || bytes.Contains(raw, []byte(testClientSecret)) { + t.Errorf("secret written to %s", path) + } + return nil + }) + out := logs.String() + for _, secret := range []string{oauthToken, testClientSecret, testCode, verifier} { + if strings.Contains(out, secret) { + t.Errorf("log contains %q:\n%s", secret, out) + } + } +} + +func TestOAuthStateChecks(t *testing.T) { + g := newFakeGitHub(t) + f := &fakeFetcher{tokens: map[string]github.TokenInfo{oauthToken: {Login: "octocat"}}} + s := newOAuthTestServer(t, f, g) + h := s.Handler() + + q, c := signIn(t, h, g, url.Values{"user": {"octocat"}}) + state := q.Get("state") + other := &http.Cookie{Name: oauthCookie, Value: state[:len(state)-1] + "x"} + planted := &http.Cookie{Name: oauthCookieInsecure, Value: state} + for name, rec := range map[string]*httptest.ResponseRecorder{ + "no cookie": callback(h, url.Values{"code": {testCode}, "state": {state}}, nil), + "other cookie": callback(h, url.Values{"code": {testCode}, "state": {state}}, other), + "unprefixed name": callback(h, url.Values{"code": {testCode}, "state": {state}}, planted), + "no state": callback(h, url.Values{"code": {testCode}}, c), + "unknown state": callback(h, url.Values{"code": {testCode}, "state": {other.Value}}, other), + "error, no state": callback(h, url.Values{"error": {"access_denied"}}, nil), + } { + if rec.Code != http.StatusBadRequest { + t.Errorf("%s: callback = %d, want 400", name, rec.Code) + } + } + if g.exchangeCount() != 0 { + t.Fatal("a rejected callback reached the token endpoint") + } + + // The real state still works once, then is spent. + if rec := callback(h, url.Values{"code": {testCode}, "state": {state}}, c); rec.Code != http.StatusSeeOther { + t.Fatalf("valid callback = %d", rec.Code) + } + waitIdle(t, s.queue, "octocat") + if rec := callback(h, url.Values{"code": {testCode}, "state": {state}}, c); rec.Code != http.StatusBadRequest { + t.Errorf("replayed callback = %d, want 400", rec.Code) + } + if n := g.exchangeCount(); n != 1 { + t.Errorf("token endpoint called %d times, want 1", n) + } +} + +func TestOAuthStateExpires(t *testing.T) { + g := newFakeGitHub(t) + s := newOAuthTestServer(t, &fakeFetcher{}, g) + h := s.Handler() + q, c := signIn(t, h, g, url.Values{"user": {"octocat"}}) + s.logins.now = func() time.Time { return time.Now().Add(loginTTL + time.Second) } + if rec := callback(h, url.Values{"code": {testCode}, "state": {q.Get("state")}}, c); rec.Code != http.StatusBadRequest { + t.Errorf("expired callback = %d, want 400", rec.Code) + } + if g.exchangeCount() != 0 { + t.Error("expired sign-in reached the token endpoint") + } +} + +func TestPendingLoginsCap(t *testing.T) { + l := newPendingLogins() + now := time.Now() + l.now = func() time.Time { return now } + for range maxPendingLogins { + if _, err := l.add(&pendingLogin{}); err != nil { + t.Fatal(err) + } + } + if _, err := l.add(&pendingLogin{}); err != errTooManyLogins { + t.Fatalf("add past the cap = %v", err) + } + now = now.Add(loginTTL) + if _, err := l.add(&pendingLogin{}); err != nil { + t.Errorf("expired entries were not swept: %v", err) + } + if len(l.entries) != 1 { + t.Errorf("%d entries left, want 1", len(l.entries)) + } +} + +func TestOAuthAccessDeniedKeepsOptions(t *testing.T) { + g := newFakeGitHub(t) + f := &fakeFetcher{} + s := newOAuthTestServer(t, f, g) + h := s.Handler() + q, c := signIn(t, h, g, url.Values{"user": {"octocat"}, "tz": {"Asia/Saigon"}, "commits_per_repo": {"123"}}) + rec := callback(h, url.Values{"error": {"access_denied"}, "state": {q.Get("state")}}, c) + body := rec.Body.String() + if rec.Code != http.StatusOK || !strings.Contains(body, "cancelled") { + t.Fatalf("access denied = %d: %s", rec.Code, body) + } + if !strings.Contains(body, `value="octocat"`) || !strings.Contains(body, "Asia/Saigon") || !strings.Contains(body, `value="123"`) { + t.Error("options were not kept") + } + if g.exchangeCount() != 0 || s.queue.Status("octocat").State != stateNone { + t.Error("a cancelled sign-in exchanged a code or queued a job") + } +} + +func TestOAuthScopeDowngrade(t *testing.T) { + g := newFakeGitHub(t) + g.scope = "read:user" + f := &fakeFetcher{tokens: map[string]github.TokenInfo{oauthToken: {Login: "octocat"}}} + s := newOAuthTestServer(t, f, g) + h := s.Handler() + q, c := signIn(t, h, g, url.Values{"user": {"octocat"}, "include_private": {"1"}, "include_org_repos": {"1"}}) + if q.Get("scope") != "repo read:user read:org" { + t.Fatalf("scope = %q", q.Get("scope")) + } + rec := callback(h, url.Values{"code": {testCode}, "state": {q.Get("state")}}, c) + if loc := rec.Header().Get("Location"); loc != "/u/octocat?notice=granted-private" { + t.Fatalf("callback redirected to %q", loc) + } + waitIdle(t, s.queue, "octocat") + if o := f.lastCall().cfg.Options; o.IncludePrivate || o.IncludeOrgRepos { + t.Errorf("job kept ungranted scope: %+v", o) + } + if m, err := s.store.Meta("octocat"); err != nil || m.Scope != "public" { + t.Errorf("meta = %+v, %v", m, err) + } + page := httptest.NewRecorder() + h.ServeHTTP(page, httptest.NewRequest(http.MethodGet, "/u/octocat?notice=granted-private", nil)) + if !strings.Contains(page.Body.String(), "did not grant access to private repositories") { + t.Error("user page does not explain the downgrade") + } +} + +func TestOAuthRevokesFailedAndForeignJobs(t *testing.T) { + g := newFakeGitHub(t) + f := &fakeFetcher{tokens: map[string]github.TokenInfo{oauthToken: {Login: "alice"}}} + s := newOAuthTestServer(t, f, g) + h := s.Handler() + + // Signed in as alice for bob: public data only, then revoked. + q, c := signIn(t, h, g, url.Values{"user": {"bob"}, "include_private": {"1"}}) + callback(h, url.Values{"code": {testCode}, "state": {q.Get("state")}}, c) + if st := waitIdle(t, s.queue, "bob"); st.State != stateDone { + t.Fatalf("foreign job = %+v", st) + } + if o := f.lastCall().cfg.Options; o.IncludePrivate { + t.Errorf("foreign sign-in kept private scope: %+v", o) + } + + // A failing job revokes too. + f.mu.Lock() + f.err = errFresh + f.mu.Unlock() + q, c = signIn(t, h, g, url.Values{"user": {"alice"}}) + callback(h, url.Values{"code": {testCode}, "state": {q.Get("state")}}, c) + if st := waitIdle(t, s.queue, "alice"); st.State != stateFailed { + t.Fatalf("failing job = %+v", st) + } + if got := g.revokedTokens(); len(got) != 2 { + t.Errorf("revoked %d tokens, want 2", len(got)) + } +} + +func TestOAuthRevokesWhenNotQueued(t *testing.T) { + g := newFakeGitHub(t) + f := &fakeFetcher{gate: make(chan struct{})} + s := newOAuthTestServer(t, f, g) + h := s.Handler() + + q, c := signIn(t, h, g, url.Values{"user": {"octocat"}}) + s.queue.Submit(signedIn(t, testToken, "user", "octocat")) // a job starts while the user is on GitHub + rec := callback(h, url.Values{"code": {testCode}, "state": {q.Get("state")}}, c) + if loc := rec.Header().Get("Location"); loc != "/u/octocat?notice=pending" { + t.Errorf("callback redirected to %q", loc) + } + if got := g.revokedTokens(); len(got) != 1 || got[0] != oauthToken { + t.Errorf("unused token not revoked: %q", got) + } + close(f.gate) + waitIdle(t, s.queue, "octocat") +} + +func TestOAuthRevokesOnShutdown(t *testing.T) { + g := newFakeGitHub(t) + f := &fakeFetcher{gate: make(chan struct{})} + s := newOAuthTestServer(t, f, g) + for _, login := range []string{"a1", "a2", "a3"} { + if _, err := s.queue.Submit(submission{Login: login, Token: oauthToken}); err != nil { + t.Fatal(err) + } + } + s.queue.Stop() // two running, one still queued + if got := g.revokedTokens(); len(got) != 3 { + t.Errorf("revoked %d tokens on shutdown, want 3", len(got)) + } +} + +func TestExtraScopes(t *testing.T) { + cases := []struct { + asked Options + granted string + want []string + }{ + {Options{}, "read:user", nil}, + {Options{}, "repo,read:user", []string{"repo"}}, + {Options{IncludeOrgRepos: true}, "read:org,read:user", []string{"read:org"}}, + {Options{IncludePrivate: true}, "read:user,repo", nil}, + {Options{IncludePrivate: true}, "read:org,read:user,repo", []string{"read:org"}}, + {Options{IncludePrivate: true, IncludeOrgRepos: true}, "admin:org,repo", []string{"admin:org"}}, + } + for _, c := range cases { + if got := extraScopes(c.asked, c.granted); !slices.Equal(got, c.want) { + t.Errorf("extraScopes(%+v, %q) = %q, want %q", c.asked, c.granted, got, c.want) + } + } +} + +func TestOAuthWiderGrantIsRefused(t *testing.T) { + g := newFakeGitHub(t) + g.scope = "read:user,repo" // granted to the app by an earlier private sign-in + f := &fakeFetcher{tokens: map[string]github.TokenInfo{oauthToken: {Login: "alice", CanReadPrivate: true}}} + s := newOAuthTestServer(t, f, g) + h := s.Handler() + + for _, login := range []string{"alice", "bob"} { + q, c := signIn(t, h, g, url.Values{"user": {login}, "tz": {"Asia/Saigon"}}) + if q.Get("scope") != "read:user" { + t.Fatalf("scope = %q", q.Get("scope")) + } + rec := callback(h, url.Values{"code": {testCode}, "state": {q.Get("state")}}, c) + body := rec.Body.String() + if rec.Code != http.StatusOK || !strings.Contains(body, "more access than your ticks ask for (repo)") { + t.Fatalf("%s: wider grant = %d: %s", login, rec.Code, body) + } + if !strings.Contains(body, `value="`+login+`"`) || !strings.Contains(body, "Asia/Saigon") { + t.Errorf("%s: options were not kept", login) + } + if s.queue.Status(login).State != stateNone { + t.Errorf("%s: a job was queued with a token wider than the ticks", login) + } + } + if got := g.revokedTokens(); len(got) != 2 { + t.Errorf("revoked %d tokens, want 2", len(got)) + } +} + +func TestOAuthPlainHTTPCookie(t *testing.T) { + app, err := newOAuthApp(OAuthConfig{ClientID: testClientID, ClientSecret: testClientSecret, PublicURL: "http://localhost:8080"}) + if err != nil { + t.Fatal(err) + } + if app.secureCookie || app.cookieName != oauthCookieInsecure { + t.Errorf("plain-http cookie = %q secure=%v", app.cookieName, app.secureCookie) + } +} diff --git a/internal/web/server.go b/internal/web/server.go index c0b18483..30cdffe1 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -1,5 +1,6 @@ -// Package web serves the ghglance web UI: a form that queues card -// generation for any GitHub user, and pages that re-show the stored cards. +// Package web serves the ghglance web UI: a form that signs the visitor in +// with GitHub and queues card generation for any GitHub user on that +// sign-in's token, and pages that re-show the stored cards. package web import ( @@ -34,8 +35,11 @@ const ( ) const ( + // pageCSP takes GitHub's origin as an extra form-action source: the + // sign-in form is redirected to GitHub's consent page, and browsers + // check redirects of a form submission against form-action too. pageCSP = "default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self'; " + - "connect-src 'self'; form-action 'self'; base-uri 'none'; frame-ancestors 'none'" + "connect-src 'self'; form-action 'self' %s; base-uri 'none'; frame-ancestors 'none'" // Cards are static drawings: no scripts, no external fetches. Inline // style attributes are the only thing they need. svgCSP = "default-src 'none'; style-src 'unsafe-inline'; sandbox" @@ -52,11 +56,11 @@ type Config struct { Workers int // JobTimeout bounds one generation job (0 = no limit). JobTimeout time.Duration - // Token is the server's own GitHub token, used for submissions that - // bring none. It never renders private or org-administered data. - Token string // Fetcher overrides the GitHub fetch; nil uses the real API. Fetcher Fetcher + // OAuth configures "Sign in with GitHub", which every generation runs + // through. It is required. + OAuth OAuthConfig } // Server holds the store, job queue and handlers. @@ -67,6 +71,9 @@ type Server struct { limiter *rateLimiter pages map[string]*template.Template static http.Handler + csp string + oauth *oauthApp + logins *pendingLogins } // New opens the data directory and starts the job workers. @@ -74,6 +81,10 @@ func New(cfg Config) (*Server, error) { if cfg.Fetcher == nil { cfg.Fetcher = githubFetcher{} } + oauth, err := newOAuthApp(cfg.OAuth) + if err != nil { + return nil, err + } store, err := OpenStore(cfg.DataDir) if err != nil { return nil, err @@ -88,14 +99,18 @@ func New(cfg Config) (*Server, error) { store.Close() return nil, err } - return &Server{ + s := &Server{ cfg: cfg, store: store, - queue: newQueue(store, cfg.Fetcher, cfg.Token, cfg.JobTimeout, cfg.Cooldown, cfg.Workers), limiter: newRateLimiter(submitBurst, submitInterval), pages: pages, static: http.FileServerFS(staticFS), - }, nil + csp: fmt.Sprintf(pageCSP, oauth.webOrigin()), + oauth: oauth, + logins: newPendingLogins(), + } + s.queue = newQueue(store, cfg.Fetcher, oauth.revoke, cfg.JobTimeout, cfg.Cooldown, cfg.Workers) + return s, nil } // Close stops the workers and releases the data directory. @@ -113,9 +128,7 @@ func Run(ctx context.Context, cfg Config) error { return err } defer s.Close() - if cfg.Token == "" { - log.Printf("warn: GITHUB_TOKEN is empty; only submissions with their own token will succeed") - } + log.Printf("sign in with GitHub callback %s", s.oauth.redirectURI) ln, err := net.Listen("tcp", cfg.Addr) if err != nil { @@ -131,9 +144,6 @@ func Run(ctx context.Context, cfg Config) error { errc := make(chan error, 1) go func() { errc <- srv.Serve(ln) }() log.Printf("serving on %s, data in %s", ln.Addr(), s.store.dir) - // Check the server token up front so a private-capable token is - // reported at startup, not on the first submission. - go s.queue.server(ctx) go s.expireLoop(ctx) select { @@ -151,7 +161,8 @@ func Run(ctx context.Context, cfg Config) error { func (s *Server) Handler() http.Handler { mux := http.NewServeMux() mux.HandleFunc("GET /{$}", s.handleIndex) - mux.HandleFunc("POST /generate", s.handleGenerate) + mux.HandleFunc("POST /auth/start", s.handleAuthStart) + mux.HandleFunc("GET /auth/callback", s.handleAuthCallback) mux.HandleFunc("GET /u/{user}", s.handleUser) mux.HandleFunc("GET /u/{user}/status", s.handleStatus) mux.HandleFunc("GET /u/{user}/{theme}/{card}", s.handleCard) @@ -166,15 +177,15 @@ func (s *Server) Handler() http.Handler { }) cop := http.NewCrossOriginProtection() - return commonHeaders(cop.Handler(mux)) + return commonHeaders(cop.Handler(mux), s.csp) } -func commonHeaders(next http.Handler) http.Handler { +func commonHeaders(next http.Handler, csp string) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { h := w.Header() h.Set("X-Content-Type-Options", "nosniff") h.Set("Referrer-Policy", "no-referrer") - h.Set("Content-Security-Policy", pageCSP) + h.Set("Content-Security-Policy", csp) next.ServeHTTP(w, r) }) } @@ -217,11 +228,13 @@ func (s *Server) handleIndex(w http.ResponseWriter, r *http.Request) { s.render(w, http.StatusOK, "index", pageData{Title: "ghglance", Form: defaultForm()}) } -func (s *Server) handleGenerate(w http.ResponseWriter, r *http.Request) { +// readSubmission parses, rate-limits and validates a generation form. On +// failure it has already written the response. +func (s *Server) readSubmission(w http.ResponseWriter, r *http.Request) (submission, formValues, bool) { r.Body = http.MaxBytesReader(w, r.Body, maxFormBytes) if err := r.ParseForm(); err != nil { s.render(w, http.StatusBadRequest, "index", pageData{Title: "ghglance", Error: "The form could not be read.", Form: defaultForm()}) - return + return submission{}, formValues{}, false } sub, form, err := parseSubmission(r.PostForm.Get) if !s.limiter.Allow(clientKey(r)) { @@ -231,40 +244,39 @@ func (s *Server) handleGenerate(w http.ResponseWriter, r *http.Request) { Error: "Too many submissions from your address. Wait a couple of minutes and try again.", Form: form, }) - return + return submission{}, formValues{}, false } if err != nil { s.render(w, http.StatusBadRequest, "index", pageData{Title: "ghglance", Error: capitalize(err.Error()) + ".", Form: form}) - return - } - - if sub.Token == "" && s.cfg.Token == "" { - s.render(w, http.StatusBadRequest, "index", pageData{ - Title: "ghglance", - Error: "This server has no GitHub token of its own. Add yours under Options.", - Form: form, - }) - return + return submission{}, formValues{}, false } + return sub, form, true +} +// enqueue queues sub and redirects to the user's page, adding notice when +// one is given. It reports whether a new job took the submission's token; +// when not, the caller still owns it. The cooldown is checked by the job, +// once it knows whose token it holds: a sign-in as the target account +// skips it. +func (s *Server) enqueue(w http.ResponseWriter, r *http.Request, sub submission, form formValues, notice string) bool { target := "/u/" + url.PathEscape(userKey(sub.Login)) if s.queue.Status(sub.Login).Active() { http.Redirect(w, r, target+"?notice=pending", http.StatusSeeOther) - return - } - if sub.Token == "" && s.store.cooldownLeft(sub.Login, s.cfg.Cooldown, time.Now()) > 0 { - http.Redirect(w, r, target+"?notice=fresh", http.StatusSeeOther) - return + return false } created, err := s.queue.Submit(sub) if err != nil { s.render(w, http.StatusServiceUnavailable, "index", pageData{Title: "ghglance", Error: capitalize(err.Error()) + ".", Form: form}) - return + return false } - if !created { + switch { + case !created: target += "?notice=pending" + case notice != "": + target += "?notice=" + url.QueryEscape(notice) } http.Redirect(w, r, target, http.StatusSeeOther) + return created } func (s *Server) handleUser(w http.ResponseWriter, r *http.Request) { @@ -306,12 +318,14 @@ func (s *Server) handleUser(w http.ResponseWriter, r *http.Request) { d.Theme = t } switch r.URL.Query().Get("notice") { - case "fresh": - d.Notice = "These cards are recent, so they were not regenerated." case "pending": if job.Active() { d.Notice = "A generation for this user is already in progress." } + case "granted-private": + d.Notice = "GitHub did not grant access to private repositories, so this generation counts public data only." + case "granted-org": + d.Notice = "GitHub did not grant read:org, so this generation leaves out org repos." } if meta != nil { d.Login = meta.Login @@ -403,7 +417,8 @@ func parsePages() (map[string]*template.Template, error) { } // formFromOptions pre-fills the regenerate form with a set's last options. -// Private scope stays ticked: it only takes effect with a token anyway. +// Private and org scope are ticked only when the last generation used them, +// so a regenerate asks GitHub for no more than the set already shows. func formFromOptions(login string, o Options) formValues { return formValues{ User: login, @@ -411,7 +426,7 @@ func formFromOptions(login string, o Options) formValues { StartOfWeek: o.StartOfWeek, IncludeForks: o.IncludeForks, IncludeOrgRepos: o.IncludeOrgRepos, - IncludePrivate: true, + IncludePrivate: o.IncludePrivate, CommitsPerRepo: strconv.Itoa(o.CommitsPerRepo), } } diff --git a/internal/web/static/app.js b/internal/web/static/app.js index 457a26e7..ea11dc24 100644 --- a/internal/web/static/app.js +++ b/internal/web/static/app.js @@ -1,6 +1,6 @@ // ghglance web UI enhancements. Every page works without JavaScript; this -// adds browser-timezone detection, token-aware checkboxes, copy buttons and -// job-status polling. +// adds browser-timezone detection, a live list of the GitHub permissions a +// sign-in asks for, copy buttons and job-status polling. 'use strict'; /** @@ -17,24 +17,44 @@ function detectTimezone(input) { } /** - * Enables the private/org checkboxes only while a token is entered, and - * ticks private repos the first time a token appears. + * Lists the GitHub scopes a sign-in will request for the ticked options. + * Mirrors oauthScopes in internal/web/oauth.go. + * @param {boolean} includePrivate + * @param {boolean} includeOrgs + * @returns {string[]} + */ +function oauthScopes(includePrivate, includeOrgs) { + if (!includePrivate) return ['read:user']; + return includeOrgs ? ['repo', 'read:user', 'read:org'] : ['repo', 'read:user']; +} + +/** + * Replaces the sign-in hint's general rule with the exact scopes for the + * current ticks, updated as they change. * @param {HTMLFormElement} form */ -function wireTokenScope(form) { - const token = /** @type {HTMLInputElement|null} */ (form.querySelector('input[name="token"]')); - if (!token) return; - const scoped = /** @type {NodeListOf} */ (form.querySelectorAll('input[data-needs-token]')); - let hadToken = false; +function wireOAuthScopes(form) { + const hint = /** @type {HTMLElement|null} */ (form.querySelector('[data-oauth-scopes]')); + const priv = /** @type {HTMLInputElement|null} */ (form.querySelector('input[name="include_private"]')); + const orgs = /** @type {HTMLInputElement|null} */ (form.querySelector('input[name="include_org_repos"]')); + if (!hint || !priv || !orgs) return; const sync = () => { - const hasToken = token.value.trim() !== ''; - scoped.forEach((box) => { - box.disabled = !hasToken; - if (hasToken && !hadToken && box.name === 'include_private') box.checked = true; + const scopes = oauthScopes(priv.checked, orgs.checked); + hint.textContent = ''; + hint.append('GitHub will ask for: '); + scopes.forEach((scope, i) => { + if (i > 0) hint.append(', '); + const code = document.createElement('code'); + code.textContent = scope; + hint.append(code); }); - hadToken = hasToken; + hint.append(scopes.includes('repo') + ? '. repo is GitHub\'s only private-repository scope and includes write access; ' + : '. Public data only; '); + hint.append('the token is used for this one generation, then revoked: never stored, never logged.'); }; - token.addEventListener('input', sync); + priv.addEventListener('change', sync); + orgs.addEventListener('change', sync); sync(); } @@ -117,10 +137,10 @@ function pollStatus(box) { }) .then((/** @type {{state: string, stage?: string, position?: number, elapsed_seconds?: number}} */ st) => { if (st.state !== 'queued' && st.state !== 'running') { - // Drop one-off notices such as ?notice=pending, which no longer - // describe the page once the job is over. + // Drop ?notice=pending, which no longer describes the page once + // the job is over; a granted-scope notice still does. const next = new URL(window.location.href); - next.searchParams.delete('notice'); + if (next.searchParams.get('notice') === 'pending') next.searchParams.delete('notice'); window.location.replace(next.toString()); return; } @@ -144,7 +164,7 @@ document.documentElement.classList.add('js'); document.addEventListener('DOMContentLoaded', () => { document.querySelectorAll('input[data-autotz]').forEach((el) => detectTimezone(/** @type {HTMLInputElement} */ (el))); - document.querySelectorAll('form.gen').forEach((el) => wireTokenScope(/** @type {HTMLFormElement} */ (el))); + document.querySelectorAll('form.gen').forEach((el) => wireOAuthScopes(/** @type {HTMLFormElement} */ (el))); document.querySelectorAll('button[data-copy], button[data-copy-target]').forEach((el) => wireCopy(/** @type {HTMLButtonElement} */ (el))); const progress = document.getElementById('progress'); if (progress) pollStatus(progress); diff --git a/internal/web/static/style.css b/internal/web/static/style.css index 71cdbbe0..13660f36 100644 --- a/internal/web/static/style.css +++ b/internal/web/static/style.css @@ -84,7 +84,6 @@ h2 { font-size: 1.2rem; margin: 0 0 12px; } .hero { max-width: 720px; } .lead { color: var(--muted); font-size: 1.05rem; margin: 0 0 24px; } .hint { color: var(--muted); font-size: 0.85rem; margin: 6px 0 0; } -.optional { color: var(--muted); font-weight: 400; } .alert { border: 1px solid var(--info-border); @@ -135,7 +134,6 @@ textarea { font-family: ui-monospace, SFMono-Regular, Menlo, Consolas, monospace details.options { margin-bottom: 18px; } details.options summary { cursor: pointer; font-weight: 600; padding: 4px 0; width: max-content; } .options-grid { display: grid; grid-template-columns: repeat(auto-fit, minmax(240px, 1fr)); gap: 0 20px; margin-top: 14px; } -.field-wide { grid-column: 1 / -1; } fieldset.checks { border: 0; padding: 0; margin: 0 0 16px; display: flex; flex-direction: column; gap: 6px; } fieldset.checks label { font-weight: 400; display: flex; gap: 8px; align-items: center; } @@ -157,10 +155,7 @@ button, .button { button:hover, .button:hover { border-color: var(--accent); } button.primary, .button { background: var(--accent); color: var(--accent-text); border-color: var(--accent); } button.primary:hover { filter: brightness(1.08); } -.token-create { display: flex; flex-wrap: wrap; align-items: center; gap: 6px 12px; margin: 8px 0 0; } -.token-create .button { background: var(--surface); color: var(--text); border-color: var(--border); } -.token-create .button:hover { border-color: var(--accent); } -.token-create .hint { margin: 0; flex: 1 1 240px; } +.signin .hint { max-width: 640px; } .user-head .meta { color: var(--muted); margin: 0 0 20px; overflow-wrap: anywhere; } diff --git a/internal/web/templates/form.html b/internal/web/templates/form.html index b812bd23..86625be4 100644 --- a/internal/web/templates/form.html +++ b/internal/web/templates/form.html @@ -1,5 +1,5 @@ {{define "form"}} -
+
Commits sampled per repo -

0 samples every commit and needs your own token.

+

0 samples every commit.

Repositories - - -

Private and org repos are only counted with your own token.

+ + +

Private and org repos are only counted when you sign in as the username's own account. + The resulting cards are public on this site, including totals drawn from private repos.

- -
- - -

- Create a token on GitHub - Opens a classic token with the repo and read:user scopes already ticked. Pick a short expiration, generate it, and paste it here. -

-

- Used only for this one generation, then dropped: never stored, never logged. - With a token for your own account, private repos count unless you untick them, and the regenerate cooldown is skipped. - A token for another account renders public data only. - The resulting cards are public on this site, including totals drawn from private repos. -

-
- +
{{end}} diff --git a/internal/web/templates/user.html b/internal/web/templates/user.html index acb10740..bb7faa30 100644 --- a/internal/web/templates/user.html +++ b/internal/web/templates/user.html @@ -64,7 +64,7 @@

{{if .Meta}}Regenerate{{else}}Try again{{end}}

{{if .ExpiresIn}}

These cards are deleted in about {{.ExpiresIn}}; regenerate to keep embedded links working.

{{end}} - {{if .CooldownLeft}}

Without a token these cards can be regenerated in {{.CooldownLeft}}. With your own token you can regenerate now.

{{end}} + {{if .CooldownLeft}}

Signed in as another account, these cards can be regenerated in {{.CooldownLeft}}. Signed in as {{.Login}}, you can regenerate now.

{{end}} {{template "form" .}}
{{end}} diff --git a/internal/web/validate.go b/internal/web/validate.go index 1237ca88..f45aa9fe 100644 --- a/internal/web/validate.go +++ b/internal/web/validate.go @@ -18,8 +18,9 @@ import ( // path segment, which is what lets it name a directory under the data dir. var usernameRE = regexp.MustCompile(`^[A-Za-z0-9]+(-[A-Za-z0-9]+)*$`) -// Tokens are only ever forwarded in an Authorization header; restricting -// the charset rules out header injection and pasted whitespace. +// Sign-in tokens are only ever forwarded in an Authorization header; +// restricting the charset of what GitHub's token endpoint returns rules out +// header injection. var tokenRE = regexp.MustCompile(`^[A-Za-z0-9_]{20,255}$`) // IANA zone names: letters, digits and _ + - / only. time.LoadLocation @@ -59,7 +60,7 @@ func validCard(name string) bool { } // Options are the generation settings recorded in meta.json. They never -// include the submitter's token. +// include the sign-in token. type Options struct { TZ string `json:"tz"` StartOfWeek string `json:"start_of_week"` @@ -69,7 +70,8 @@ type Options struct { CommitsPerRepo int `json:"commits_per_repo"` } -// submission is a validated form post. +// submission is a validated form post. Token is the sign-in token GitHub +// issues at the callback; it is revoked when the job ends. type submission struct { Login string Token string @@ -77,7 +79,6 @@ type submission struct { } // formValues is the raw form, kept so a rejected post re-renders as typed. -// It never carries the token back to the page. type formValues struct { User string TZ string @@ -88,19 +89,21 @@ type formValues struct { CommitsPerRepo string } +// defaultForm is the blank generation form. Private repos start unticked: a +// sign-in asks for exactly what is ticked, and repo is a broad grant that +// only helps when the username is the visitor's own account. func defaultForm() formValues { return formValues{ TZ: "UTC", StartOfWeek: "sunday", IncludeForks: true, - IncludePrivate: true, CommitsPerRepo: strconv.Itoa(defaultCommitsPerRepo), } } -// parseSubmission validates a submitted form. Without the submitter's own -// token, private and org-repo scope are forced off: the server token must -// never surface its owner's private repos or the orgs it administers. +// parseSubmission validates a generation form. The ticked scope is kept: +// it picks the scopes the sign-in asks for, and the job still enforces who +// the resulting token belongs to. func parseSubmission(get func(string) string) (submission, formValues, error) { f := formValues{ User: strings.TrimSpace(get("user")), @@ -111,14 +114,10 @@ func parseSubmission(get func(string) string) (submission, formValues, error) { IncludePrivate: get("include_private") != "", CommitsPerRepo: strings.TrimSpace(get("commits_per_repo")), } - token := strings.TrimSpace(get("token")) if !validUsername(f.User) { return submission{}, f, errors.New("enter a valid GitHub username: letters, digits and single hyphens, up to 39 characters") } - if token != "" && !tokenRE.MatchString(token) { - return submission{}, f, errors.New("that does not look like a GitHub token") - } tz := f.TZ if tz == "" { @@ -144,23 +143,15 @@ func parseSubmission(get func(string) string) (submission, formValues, error) { } perRepo = n } - if perRepo == 0 && token == "" { - return submission{}, f, errors.New("sampling every commit (0) needs your own token") - } - opts := Options{ + return submission{Login: f.User, Options: Options{ TZ: tz, StartOfWeek: strings.ToLower(wd.String()), IncludeForks: f.IncludeForks, IncludeOrgRepos: f.IncludeOrgRepos, IncludePrivate: f.IncludePrivate, CommitsPerRepo: perRepo, - } - if token == "" { - opts.IncludePrivate = false - opts.IncludeOrgRepos = false - } - return submission{Login: f.User, Token: token, Options: opts}, f, nil + }}, f, nil } // collectConfig maps validated options onto the shared fetch pipeline. diff --git a/internal/web/web_test.go b/internal/web/web_test.go index 694ef31f..ec0b655d 100644 --- a/internal/web/web_test.go +++ b/internal/web/web_test.go @@ -92,14 +92,31 @@ func newTestServer(t *testing.T, f *fakeFetcher) *Server { } func newTestServerTimeout(t *testing.T, f *fakeFetcher, timeout time.Duration) *Server { + t.Helper() + return newServerWith(t, f, newFakeGitHub(t), timeout) +} + +// testOAuth points sign-in at the fake GitHub g, so no test ever revokes a +// token on the real one. +func testOAuth(g *fakeGitHub) OAuthConfig { + return OAuthConfig{ + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL + "/", + WebURL: g.srv.URL, + APIURL: g.srv.URL, + } +} + +func newServerWith(t *testing.T, f *fakeFetcher, g *fakeGitHub, timeout time.Duration) *Server { t.Helper() s, err := New(Config{ DataDir: t.TempDir(), Cooldown: time.Hour, Workers: 2, JobTimeout: timeout, - Token: "server-token", Fetcher: f, + OAuth: testOAuth(g), }) if err != nil { t.Fatal(err) @@ -130,6 +147,17 @@ func form(kv ...string) func(string) string { return v.Get } +// signedIn parses a form and attaches token, as the sign-in callback does. +func signedIn(t *testing.T, token string, kv ...string) submission { + t.Helper() + sub, _, err := parseSubmission(form(kv...)) + if err != nil { + t.Fatal(err) + } + sub.Token = token + return sub +} + func TestValidUsername(t *testing.T) { good := []string{"a", "octocat", "tiennm99", "a-b", "A1-b2-C3", strings.Repeat("a", 39)} bad := []string{"", "-a", "a-", "a--b", strings.Repeat("a", 40), "..", "../etc", "a/b", `a\b`, "a.b", "a_b", ".gen", "a b", "%2e%2e", "é"} @@ -164,52 +192,37 @@ func TestValidThemeAndCard(t *testing.T) { } } -func TestParseSubmissionForcesPrivacyWithoutToken(t *testing.T) { +func TestParseSubmissionKeepsTicks(t *testing.T) { sub, _, err := parseSubmission(form( "user", "octocat", "tz", "Asia/Saigon", "start_of_week", "Mon", "include_private", "1", "include_org_repos", "1", "include_forks", "1", + "token", testToken, // no such field any more: ignored )) if err != nil { t.Fatal(err) } - if sub.Options.IncludePrivate || sub.Options.IncludeOrgRepos { - t.Errorf("server-token submission kept private=%v org=%v", sub.Options.IncludePrivate, sub.Options.IncludeOrgRepos) + o := sub.Options + if !o.IncludePrivate || !o.IncludeOrgRepos || !o.IncludeForks || o.StartOfWeek != "monday" || o.TZ != "Asia/Saigon" { + t.Errorf("options = %+v", o) } - if !sub.Options.IncludeForks || sub.Options.StartOfWeek != "monday" || sub.Options.TZ != "Asia/Saigon" { - t.Errorf("options = %+v", sub.Options) + if o.CommitsPerRepo != defaultCommitsPerRepo || sub.Token != "" { + t.Errorf("submission = %+v", sub) } - if sub.Options.CommitsPerRepo != defaultCommitsPerRepo { - t.Errorf("commits per repo = %d, want default", sub.Options.CommitsPerRepo) - } -} - -func TestParseSubmissionHonorsOwnToken(t *testing.T) { - sub, _, err := parseSubmission(form( - "user", "octocat", "token", testToken, "include_private", "1", "include_org_repos", "1", "commits_per_repo", "0", - )) - if err != nil { - t.Fatal(err) - } - if !sub.Options.IncludePrivate || !sub.Options.IncludeOrgRepos || sub.Token != testToken { - t.Errorf("own-token submission = %+v", sub) - } - if sub.Options.CommitsPerRepo != 0 { - t.Errorf("commits per repo = %d, want 0", sub.Options.CommitsPerRepo) + if sub, _, err := parseSubmission(form("user", "octocat", "commits_per_repo", "0")); err != nil || sub.Options.CommitsPerRepo != 0 { + t.Errorf("every commit = %+v, %v", sub.Options, err) } } func TestParseSubmissionRejects(t *testing.T) { cases := map[string]func(string) string{ - "bad user": form("user", "../etc"), - "empty user": form("user", ""), - "bad tz": form("user", "a", "tz", "../../etc/passwd"), - "local tz": form("user", "a", "tz", "Local"), - "unknown tz": form("user", "a", "tz", "Mars/Olympus"), - "bad week": form("user", "a", "start_of_week", "moonday"), - "negative commits": form("user", "a", "commits_per_repo", "-1"), - "huge commits": form("user", "a", "commits_per_repo", "999999"), - "every commit, no t": form("user", "a", "commits_per_repo", "0"), - "header injection": form("user", "a", "token", "ghp_abcdefghijklmnopqrstuvwxyz\r\nX: y"), + "bad user": form("user", "../etc"), + "empty user": form("user", ""), + "bad tz": form("user", "a", "tz", "../../etc/passwd"), + "local tz": form("user", "a", "tz", "Local"), + "unknown tz": form("user", "a", "tz", "Mars/Olympus"), + "bad week": form("user", "a", "start_of_week", "moonday"), + "negative commits": form("user", "a", "commits_per_repo", "-1"), + "huge commits": form("user", "a", "commits_per_repo", "999999"), } for name, get := range cases { if _, _, err := parseSubmission(get); err == nil { @@ -355,11 +368,11 @@ func TestQueueDedupsPerUser(t *testing.T) { f := &fakeFetcher{gate: make(chan struct{})} s := newTestServer(t, f) - sub, _, _ := parseSubmission(form("user", "octocat")) + sub := signedIn(t, testToken, "user", "octocat") if created, err := s.queue.Submit(sub); !created || err != nil { t.Fatalf("first submit = %v, %v", created, err) } - again, _, _ := parseSubmission(form("user", "OctoCat")) + again := signedIn(t, testToken, "user", "OctoCat") if created, err := s.queue.Submit(again); created || err != nil { t.Fatalf("duplicate submit = %v, %v; want deduped", created, err) } @@ -376,40 +389,30 @@ func TestQueueDedupsPerUser(t *testing.T) { waitIdle(t, s.queue, "octocat") } -func TestQueueTokenHandling(t *testing.T) { - f := &fakeFetcher{viewer: "Boss", tokens: map[string]github.TokenInfo{ - testToken: {Login: "boss", CanReadPrivate: true}, - }} - s := newTestServer(t, f) +func TestQueueOwnSignIn(t *testing.T) { + g := newFakeGitHub(t) + f := &fakeFetcher{tokens: map[string]github.TokenInfo{testToken: {Login: "boss", CanReadPrivate: true}}} + s := newServerWith(t, f, g, time.Minute) - // The server token's owner is refused without their own token. - sub, _, _ := parseSubmission(form("user", "boss")) + // A job without a sign-in token never fetches. + sub := signedIn(t, "", "user", "octocat") s.queue.Submit(sub) - if st := waitIdle(t, s.queue, "boss"); st.State != stateFailed || st.Error != errOwner.Error() { - t.Fatalf("owner job = %+v", st) + if st := waitIdle(t, s.queue, "octocat"); st.State != stateFailed || st.Error != errNoToken.Error() { + t.Fatalf("token-less job = %+v", st) } if f.callCount() != 0 { - t.Fatal("owner was fetched with the server token") + t.Fatal("fetched without a sign-in token") } - // Anyone else without a token uses the server token, public scope only. - sub, _, _ = parseSubmission(form("user", "octocat", "include_private", "1")) - s.queue.Submit(sub) - waitIdle(t, s.queue, "octocat") - c := f.lastCall() - if c.token != "server-token" || c.cfg.Options.IncludePrivate || c.cfg.Options.IncludeOrgRepos { - t.Errorf("server-token call = %+v", c) - } - - // A submitter's token is used for their job and never persisted. - sub, _, _ = parseSubmission(form("user", "boss", "token", testToken, "include_private", "1")) - s.queue.Submit(sub) + // A sign-in as the target account keeps private scope; the token is + // used for the job, revoked, and never persisted. + s.queue.Submit(signedIn(t, testToken, "user", "boss", "include_private", "1")) if st := waitIdle(t, s.queue, "boss"); st.State != stateDone { - t.Fatalf("own-token job = %+v", st) + t.Fatalf("own sign-in job = %+v", st) } - c = f.lastCall() + c := f.lastCall() if c.token != testToken || !c.cfg.Options.IncludePrivate || !c.cfg.Strict { - t.Errorf("own-token call = %+v", c) + t.Errorf("own sign-in call = %+v", c) } m, err := s.store.Meta("boss") if err != nil || m.Scope != "private" { @@ -425,47 +428,8 @@ func TestQueueTokenHandling(t *testing.T) { if leftover != "" { t.Error("token kept in memory after the job ended") } -} - -func TestCooldownAndTokenBypass(t *testing.T) { - f := &fakeFetcher{tokens: map[string]github.TokenInfo{testToken: {Login: "octocat"}}} - s := newTestServer(t, f) - h := s.Handler() - post := func(v url.Values, ip string) *httptest.ResponseRecorder { - req := httptest.NewRequest(http.MethodPost, "/generate", strings.NewReader(v.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.RemoteAddr = ip + ":1234" - rec := httptest.NewRecorder() - h.ServeHTTP(rec, req) - return rec - } - - rec := post(url.Values{"user": {"octocat"}}, "203.0.113.1") - if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/u/octocat" { - t.Fatalf("first post = %d %q", rec.Code, rec.Header().Get("Location")) - } - waitIdle(t, s.queue, "octocat") - - rec = post(url.Values{"user": {"octocat"}}, "203.0.113.2") - if loc := rec.Header().Get("Location"); loc != "/u/octocat?notice=fresh" { - t.Fatalf("cooldown post redirected to %q", loc) - } - if n := f.callCount(); n != 1 { - t.Fatalf("cooldown still fetched: %d calls", n) - } - - rec = post(url.Values{"user": {"octocat"}, "token": {testToken}}, "203.0.113.3") - if loc := rec.Header().Get("Location"); loc != "/u/octocat" { - t.Fatalf("token post redirected to %q", loc) - } - waitIdle(t, s.queue, "octocat") - if n := f.callCount(); n != 2 { - t.Fatalf("token bypass did not fetch: %d calls", n) - } - - // Cooldown expiry lets a token-less submission through again. - if left := s.store.cooldownLeft("octocat", time.Hour, time.Now().Add(2*time.Hour)); left != 0 { - t.Errorf("cooldown after expiry = %s", left) + if got := g.revokedTokens(); len(got) != 1 || got[0] != testToken { + t.Errorf("revoked = %q, want the sign-in token once", got) } } @@ -480,9 +444,12 @@ func TestHandlers(t *testing.T) { } rec := get("/") - if rec.Code != 200 || !strings.Contains(rec.Body.String(), `action="/generate"`) { + if rec.Code != 200 || !strings.Contains(rec.Body.String(), `action="/auth/start"`) { t.Fatalf("index = %d", rec.Code) } + if body := rec.Body.String(); strings.Contains(body, `name="token"`) || strings.Count(body, `type="submit"`) != 1 { + t.Error("index offers something besides signing in") + } if rec.Header().Get("Content-Security-Policy") == "" || rec.Header().Get("X-Content-Type-Options") != "nosniff" { t.Error("index missing security headers") } @@ -542,11 +509,11 @@ func TestHandlers(t *testing.T) { } } -func TestGenerateValidationAndLimits(t *testing.T) { +func TestSubmitValidationAndLimits(t *testing.T) { s := newTestServer(t, &fakeFetcher{}) h := s.Handler() post := func(body, ip string) *httptest.ResponseRecorder { - req := httptest.NewRequest(http.MethodPost, "/generate", strings.NewReader(body)) + req := httptest.NewRequest(http.MethodPost, "/auth/start", strings.NewReader(body)) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.RemoteAddr = ip + ":1" rec := httptest.NewRecorder() @@ -557,8 +524,8 @@ func TestGenerateValidationAndLimits(t *testing.T) { if rec := post("user=..%2Fetc", "198.51.100.1"); rec.Code != 400 { t.Errorf("invalid user = %d", rec.Code) } - if rec := post("user=a&token="+testToken, "198.51.100.2"); strings.Contains(rec.Body.String(), testToken) { - t.Error("token echoed back into the page") + if rec := post("user=a", "198.51.100.2"); rec.Code != http.StatusFound { + t.Errorf("valid post = %d, want a redirect to GitHub", rec.Code) } big := "user=octocat&pad=" + strings.Repeat("x", maxFormBytes) if rec := post(big, "198.51.100.3"); rec.Code != 400 { @@ -616,19 +583,33 @@ func TestClientKey(t *testing.T) { } } -func TestGenerateNeedsSomeToken(t *testing.T) { - f := &fakeFetcher{} - s, err := New(Config{DataDir: t.TempDir(), Workers: 1, Fetcher: f}) - if err != nil { - t.Fatal(err) +func TestNewRequiresOAuth(t *testing.T) { + g := newFakeGitHub(t) + full := testOAuth(g) + cases := map[string]struct { + cfg OAuthConfig + want string + }{ + "unset": {OAuthConfig{}, "client ID, client secret, public URL"}, + "no public url": {OAuthConfig{ClientID: full.ClientID, ClientSecret: full.ClientSecret}, "missing OAuth public URL"}, + "no secret": {OAuthConfig{ClientID: full.ClientID, PublicURL: full.PublicURL}, "missing OAuth client secret"}, + "no client id": {OAuthConfig{ClientSecret: full.ClientSecret, PublicURL: full.PublicURL}, "missing OAuth client ID"}, + "bad url": {OAuthConfig{ClientID: full.ClientID, ClientSecret: full.ClientSecret, PublicURL: "ghglance.example"}, "absolute http(s) URL"}, } - defer s.Close() - req := httptest.NewRequest(http.MethodPost, "/generate", strings.NewReader("user=octocat")) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - rec := httptest.NewRecorder() - s.Handler().ServeHTTP(rec, req) - if rec.Code != http.StatusBadRequest || f.callCount() != 0 { - t.Errorf("token-less post on a token-less server = %d, %d fetches", rec.Code, f.callCount()) + for name, c := range cases { + dir := t.TempDir() + s, err := New(Config{DataDir: dir, Workers: 1, Fetcher: &fakeFetcher{}, OAuth: c.cfg}) + if err == nil { + s.Close() + t.Errorf("%s: server started", name) + continue + } + if !strings.Contains(err.Error(), c.want) { + t.Errorf("%s: error %q does not mention %q", name, err, c.want) + } + if entries, _ := os.ReadDir(dir); len(entries) != 0 { + t.Errorf("%s: data directory touched before the config was checked", name) + } } } @@ -655,24 +636,6 @@ func TestRateLimiterHardCap(t *testing.T) { } } -func TestServerTokenThatReadsPrivateIsRefused(t *testing.T) { - f := &fakeFetcher{tokens: map[string]github.TokenInfo{ - "server-token": {Login: "operator", CanReadPrivate: true}, - }} - s := newTestServer(t, f) - sub, _, _ := parseSubmission(form("user", "coworker")) - s.queue.Submit(sub) - if st := waitIdle(t, s.queue, "coworker"); st.State != stateFailed || st.Error != errServerPrivate.Error() { - t.Fatalf("job = %+v", st) - } - if f.callCount() != 0 { - t.Error("fetched with a private-capable server token") - } - if _, err := s.store.Meta("coworker"); !isNotExist(err) { - t.Errorf("cards published: %v", err) - } -} - func TestForeignTokenScope(t *testing.T) { const publicToken = "ghp_publicONLYvalue0123456789abcdef" f := &fakeFetcher{tokens: map[string]github.TokenInfo{ @@ -681,18 +644,17 @@ func TestForeignTokenScope(t *testing.T) { }} s := newTestServer(t, f) - // A private-capable token is only good for its own account. - sub, _, _ := parseSubmission(form("user", "xavier", "token", testToken, "include_private", "1")) - s.queue.Submit(sub) - if st := waitIdle(t, s.queue, "xavier"); st.State != stateFailed || !strings.Contains(st.Error, "belongs to alice") { + // A private-capable sign-in is only good for its own account. + s.queue.Submit(signedIn(t, testToken, "user", "xavier", "include_private", "1")) + if st := waitIdle(t, s.queue, "xavier"); st.State != stateFailed || !strings.Contains(st.Error, "signed in as alice") { t.Fatalf("private-capable foreign token job = %+v", st) } if f.callCount() != 0 { t.Fatal("fetched another user with a private-capable token") } - // A public-only token renders public scope for someone else. - sub, _, _ = parseSubmission(form("user", "xavier", "token", publicToken, "include_private", "1", "include_org_repos", "1")) + // A public-only sign-in renders public scope for someone else. + sub := signedIn(t, publicToken, "user", "xavier", "include_private", "1", "include_org_repos", "1") s.queue.Submit(sub) if st := waitIdle(t, s.queue, "xavier"); st.State != stateDone { t.Fatalf("public foreign token job = %+v", st) @@ -704,14 +666,23 @@ func TestForeignTokenScope(t *testing.T) { t.Errorf("meta = %+v, %v", m, err) } - // ...and does not skip the cooldown on that account. + // ...and does not skip the cooldown on that account... s.queue.Submit(sub) if st := waitIdle(t, s.queue, "xavier"); st.State != stateFailed || st.Error != errFresh.Error() { - t.Fatalf("foreign token during cooldown = %+v", st) + t.Fatalf("foreign sign-in during cooldown = %+v", st) } if n := f.callCount(); n != 1 { t.Errorf("fetched %d times, want 1", n) } + + // ...until the cooldown has passed. + s.queue.mu.Lock() + s.queue.now = func() time.Time { return time.Now().Add(2 * time.Hour) } + s.queue.mu.Unlock() + s.queue.Submit(sub) + if st := waitIdle(t, s.queue, "xavier"); st.State != stateDone { + t.Fatalf("foreign sign-in after the cooldown = %+v", st) + } } func TestPartialFetchIsNotPublished(t *testing.T) { @@ -726,8 +697,7 @@ func TestPartialFetchIsNotPublished(t *testing.T) { f.mu.Lock() f.stall = true f.mu.Unlock() - sub, _, _ := parseSubmission(form("user", "octocat", "token", testToken)) - s.queue.Submit(sub) + s.queue.Submit(signedIn(t, testToken, "user", "octocat")) if st := waitIdle(t, s.queue, "octocat"); st.State != stateFailed || !strings.Contains(st.Error, "timed out") { t.Fatalf("stalled job = %+v", st) } @@ -749,8 +719,7 @@ func TestPendingNoticeOnlyWhileActive(t *testing.T) { return rec.Body.String() } - sub, _, _ := parseSubmission(form("user", "octocat", "token", testToken)) - s.queue.Submit(sub) + s.queue.Submit(signedIn(t, testToken, "user", "octocat")) if !strings.Contains(get(), notice) { t.Error("pending notice missing while the job runs") } @@ -766,8 +735,8 @@ func TestRateLimitedPostKeepsForm(t *testing.T) { h := s.Handler() var rec *httptest.ResponseRecorder for range submitBurst + 1 { - req := httptest.NewRequest(http.MethodPost, "/generate", - strings.NewReader("user=--bad&tz=Asia%2FSaigon&commits_per_repo=123&token="+testToken)) + req := httptest.NewRequest(http.MethodPost, "/auth/start", + strings.NewReader("user=--bad&tz=Asia%2FSaigon&commits_per_repo=123")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.RemoteAddr = "198.51.100.20:1" rec = httptest.NewRecorder() @@ -780,7 +749,4 @@ func TestRateLimitedPostKeepsForm(t *testing.T) { if !strings.Contains(body, "--bad") || !strings.Contains(body, "Asia/Saigon") || !strings.Contains(body, `value="123"`) { t.Error("429 page dropped the submitted form") } - if strings.Contains(body, testToken) { - t.Error("token echoed back into the 429 page") - } } diff --git a/main.go b/main.go index 1fff6e53..91cafd0a 100644 --- a/main.go +++ b/main.go @@ -2,6 +2,7 @@ package main import ( + "cmp" "context" "flag" "fmt" @@ -20,7 +21,7 @@ import ( func main() { var ( user = flag.String("user", "", "GitHub username (required)") - token = flag.String("token", os.Getenv("GITHUB_TOKEN"), "GitHub token (or env GITHUB_TOKEN)") + token = flag.String("token", os.Getenv("GITHUB_TOKEN"), "GitHub token (or env GITHUB_TOKEN); not used by -serve, which runs every generation on the visitor's sign-in") out = flag.String("out", "output", "output directory") themesFlag = flag.String("themes", "dracula", "comma-separated theme ids, or 'all'") tzName = flag.String("tz", "Local", "timezone for productive-time card (IANA name, e.g. Asia/Saigon)") @@ -34,9 +35,14 @@ func main() { listThemes = flag.Bool("list-themes", false, "print available theme ids and exit") serve = flag.String("serve", "", "run the web UI on this address (e.g. :8080) instead of generating once") dataDir = flag.String("data-dir", "data", "web UI: directory holding generated cards") - cooldown = flag.Duration("cooldown", 6*time.Hour, "web UI: minimum age of a user's cards before they can be regenerated without the submitter's own token") + cooldown = flag.Duration("cooldown", 6*time.Hour, "web UI: minimum age of a user's cards before someone signed in as another account can regenerate them") retention = flag.Duration("retention", 24*time.Hour, "web UI: delete a user's generated cards this long after they were generated (0 = keep forever)") workers = flag.Int("workers", 2, "web UI: concurrent generation jobs") + // Empty defaults keep the secret out of -help; the env fallback is + // applied after parsing. + oauthID = flag.String("oauth-client-id", "", "web UI, required: GitHub OAuth App client ID for \"Sign in with GitHub\" (or env GHGLANCE_OAUTH_CLIENT_ID)") + oauthSecret = flag.String("oauth-client-secret", "", "web UI, required: GitHub OAuth App client secret (or env GHGLANCE_OAUTH_CLIENT_SECRET)") + publicURL = flag.String("public-url", "", "web UI, required: external origin, e.g. https://ghglance.example.com; the OAuth callback is /auth/callback (or env GHGLANCE_PUBLIC_URL)") ) flag.Parse() @@ -48,6 +54,15 @@ func main() { } if *serve != "" { + oauth := web.OAuthConfig{ + ClientID: cmp.Or(*oauthID, os.Getenv("GHGLANCE_OAUTH_CLIENT_ID")), + ClientSecret: cmp.Or(*oauthSecret, os.Getenv("GHGLANCE_OAUTH_CLIENT_SECRET")), + PublicURL: cmp.Or(*publicURL, os.Getenv("GHGLANCE_PUBLIC_URL")), + } + if missing := missingOAuthSettings(oauth); len(missing) > 0 { + fmt.Fprintf(os.Stderr, "error: -serve requires sign in with GitHub; set %s\n", strings.Join(missing, ", ")) + os.Exit(2) + } ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() err := web.Run(ctx, web.Config{ @@ -57,7 +72,7 @@ func main() { Retention: *retention, Workers: *workers, JobTimeout: *timeout, - Token: *token, + OAuth: oauth, }) if err != nil { fmt.Fprintf(os.Stderr, "error: %v\n", err) @@ -135,6 +150,22 @@ func main() { } } +// missingOAuthSettings names each empty sign-in setting as its flag and +// environment variable. +func missingOAuthSettings(c web.OAuthConfig) []string { + var missing []string + if c.ClientID == "" { + missing = append(missing, "-oauth-client-id (GHGLANCE_OAUTH_CLIENT_ID)") + } + if c.ClientSecret == "" { + missing = append(missing, "-oauth-client-secret (GHGLANCE_OAUTH_CLIENT_SECRET)") + } + if c.PublicURL == "" { + missing = append(missing, "-public-url (GHGLANCE_PUBLIC_URL)") + } + return missing +} + func resolveThemes(spec string) ([]theme.Theme, error) { spec = strings.TrimSpace(spec) if spec == "" { diff --git a/plans/reports/research-261007-1138-github-login-for-web-ui-token.md b/plans/reports/research-261007-1138-github-login-for-web-ui-token.md new file mode 100644 index 00000000..18403b69 --- /dev/null +++ b/plans/reports/research-261007-1138-github-login-for-web-ui-token.md @@ -0,0 +1,81 @@ +# GitHub login for the web UI token + +Researched 2026-10-07 11:38 (Asia/Saigon). + +## Answer + +Yes. A GitHub **OAuth App** web flow can replace manual token creation. +The user clicks "Sign in with GitHub" and approves once, and the server gets +a token for that one generation. Use an OAuth App rather than a GitHub App: +ghglance's private stats depend on the token seeing every repo the user can +see, and only an OAuth App's `repo` scope gives that. + +## Options compared + +| Option | Private stats | UX | Depends most on | Fails first when | +| --- | --- | --- | --- | --- | +| **A. OAuth App, token used for one job then revoked** (recommended) | Complete: `repo` covers every repo the user can access | One click and one consent screen | Users accepting the `repo` consent ("full control of private repositories") | A user's org enforces OAuth App access restrictions; org repos drop out until an owner approves the app | +| B. GitHub App user token | Partial: only repos where the app is installed | Install step, then pick repos | Users installing the app on every repo and org | Any org repo without an install, which is common for work repos | +| C. Keep the manual PAT (today) | Complete | Five manual steps on GitHub | Users being willing to create a PAT | Most visitors give up | + +Better approaches: none. The requested direction (login to get a token) is +the right one. B looks safer because of fine-grained permissions, but it +cannot see private repos where it isn't installed, so it fails the feature's +purpose. + +## Recommended design (A) + +``` +form ─► POST /auth/start (options saved server-side under a random state, 10 min TTL) + ─► 302 github.com/login/oauth/authorize?client_id&redirect_uri&scope&state&code_challenge(S256) + ─► user approves + ─► GET /auth/callback?code&state ─► check state and PKCE ─► POST /login/oauth/access_token + ─► token + viewer login ─► existing job queue (same rules as a pasted token) + ─► job ends ─► DELETE /applications/{client_id}/token (revoke) ─► token gone +``` + +- **Two buttons.** "Sign in (public only)" asks for `read:user`. "Sign in + with private repos" asks for `repo read:user`. Public-only login still + helps: it proves the user owns the account, skips the cooldown, and runs + on the user's own API quota instead of the server's. +- **No sessions or database.** The token lives only on the job, as a pasted + token does today, and is revoked when the job ends, so nothing long-lived + is left behind. The only new state is the pending options keyed by + `state`, kept in memory with a short TTL. +- **Security.** Use a random `state` bound to a cookie, PKCE `S256`, and an + exact callback URL. The client secret comes from env and is never logged. + The login is checked against the target username; signing in as A and + generating B renders public data only, which the current code already + enforces. +- **Optional by config.** The feature is off unless + `GHGLANCE_OAUTH_CLIENT_ID` and `GHGLANCE_OAUTH_CLIENT_SECRET` are set. The + manual token field stays as a fallback. +- **One-time setup.** Register an OAuth App under the account with callback + `https://ghglance.sg.miti99.com/auth/callback` and put both values in + Coolify. + +## Facts the design rests on + +- The web flow supports `state` and PKCE (`code_challenge`, `S256`). Codes + expire after 10 minutes. GitHub allows 10 tokens created per hour per + user, app and scope. ([docs][flow]) +- OAuth App tokens are long-lived by default. GitHub App user tokens expire + after 8 hours. A GitHub App only reaches the repos it is installed on. + Organization OAuth policies can block OAuth Apps until an owner approves. + ([docs][diff]) +- `repo` grants read and write access to every repo the user can reach. + There is no read-only private-repo scope for OAuth Apps. ([docs][scopes]) + That is why the design revokes the token after each job. + +[flow]: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps +[diff]: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/differences-between-github-apps-and-oauth-apps +[scopes]: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/scopes-for-oauth-apps + +## Unresolved questions + +- Should the private login also ask for `read:org`? Today's PAT + instructions do not use it, and `-include-org-repos` works without it for + repos the user administers. +- Whether `contributionsCollection` counts private contributions for a + GitHub App user token without an install is not documented. It does not + change the recommendation.