@@ -145,6 +145,12 @@ func PATAuthMiddleware(cfg PATConfig) func(http.Handler) http.Handler { |
| 145 | 145 | }() |
| 146 | 146 | } |
| 147 | 147 | |
| 148 | + // X-OAuth-Scopes echoes the token's scopes on every response — |
| 149 | + // even errors emitted further down the chain — so the CLI's |
| 150 | + // error path (shithub-cli/internal/api/errors.go) can report |
| 151 | + // provided scopes alongside the required scope on 403. |
| 152 | + w.Header().Set("X-OAuth-Scopes", strings.Join(row.Scopes, ", ")) |
| 153 | + |
| 148 | 154 | ctx := context.WithValue(r.Context(), patAuthKey, PATAuth{ |
| 149 | 155 | UserID: row.UserID, |
| 150 | 156 | Username: user.Username, |
@@ -161,6 +167,11 @@ func PATAuthMiddleware(cfg PATConfig) func(http.Handler) http.Handler { |
| 161 | 167 | // RequireScope rejects with 403 if the request was authenticated via PAT |
| 162 | 168 | // and the token's scopes don't include required. Pure-session callers |
| 163 | 169 | // (PATAuth zero) pass through — sessions have implicit full scope. |
| 170 | +// |
| 171 | +// The 403 response is the canonical /api/v1 JSON envelope |
| 172 | +// `{"error": "token lacks required scope: <scope>"}` and carries |
| 173 | +// X-Accepted-OAuth-Scopes so the CLI can format an actionable error |
| 174 | +// without parsing the message body. |
| 164 | 175 | func RequireScope(required pat.Scope) func(http.Handler) http.Handler { |
| 165 | 176 | return func(next http.Handler) http.Handler { |
| 166 | 177 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
@@ -170,9 +181,8 @@ func RequireScope(required pat.Scope) func(http.Handler) http.Handler { |
| 170 | 181 | return |
| 171 | 182 | } |
| 172 | 183 | if !pat.HasScope(auth.Scopes, required) { |
| 173 | | - w.Header().Set("Content-Type", "text/plain; charset=utf-8") |
| 174 | | - w.WriteHeader(http.StatusForbidden) |
| 175 | | - _, _ = w.Write([]byte("token lacks required scope: " + string(required) + "\n")) |
| 184 | + w.Header().Set("X-Accepted-OAuth-Scopes", string(required)) |
| 185 | + writeAPIJSONError(w, http.StatusForbidden, "token lacks required scope: "+string(required)) |
| 176 | 186 | return |
| 177 | 187 | } |
| 178 | 188 | next.ServeHTTP(w, r) |
@@ -180,6 +190,54 @@ func RequireScope(required pat.Scope) func(http.Handler) http.Handler { |
| 180 | 190 | } |
| 181 | 191 | } |
| 182 | 192 | |
| 193 | +// writeAPIJSONError writes the canonical /api/v1 error envelope. Kept |
| 194 | +// inline so package middleware doesn't depend on the api handler package |
| 195 | +// (which would import-cycle). |
| 196 | +func writeAPIJSONError(w http.ResponseWriter, status int, msg string) { |
| 197 | + w.Header().Set("Content-Type", "application/json; charset=utf-8") |
| 198 | + w.Header().Set("Cache-Control", "no-store") |
| 199 | + w.WriteHeader(status) |
| 200 | + // Escape the inner message just enough for embedding in a JSON |
| 201 | + // string literal. The reason strings here are package-controlled — |
| 202 | + // no user input — but defensive escaping keeps this honest if a |
| 203 | + // caller ever passes through external data. |
| 204 | + _, _ = w.Write([]byte(`{"error":` + jsonString(msg) + `}` + "\n")) |
| 205 | +} |
| 206 | + |
| 207 | +// jsonString returns s wrapped in JSON-string quoting, escaping the |
| 208 | +// minimum required characters. Avoiding encoding/json here keeps |
| 209 | +// allocations down on the hot challenge path. |
| 210 | +func jsonString(s string) string { |
| 211 | + var b strings.Builder |
| 212 | + b.Grow(len(s) + 2) |
| 213 | + b.WriteByte('"') |
| 214 | + for _, r := range s { |
| 215 | + switch r { |
| 216 | + case '"', '\\': |
| 217 | + b.WriteByte('\\') |
| 218 | + b.WriteRune(r) |
| 219 | + case '\n': |
| 220 | + b.WriteString(`\n`) |
| 221 | + case '\r': |
| 222 | + b.WriteString(`\r`) |
| 223 | + case '\t': |
| 224 | + b.WriteString(`\t`) |
| 225 | + default: |
| 226 | + if r < 0x20 { |
| 227 | + // Control chars: emit as \u00XX. |
| 228 | + const hex = "0123456789abcdef" |
| 229 | + b.WriteString(`\u00`) |
| 230 | + b.WriteByte(hex[byte(r)>>4]) |
| 231 | + b.WriteByte(hex[byte(r)&0x0f]) |
| 232 | + continue |
| 233 | + } |
| 234 | + b.WriteRune(r) |
| 235 | + } |
| 236 | + } |
| 237 | + b.WriteByte('"') |
| 238 | + return b.String() |
| 239 | +} |
| 240 | + |
| 183 | 241 | // errNoCredentials is the sentinel that says "no Authorization header at |
| 184 | 242 | // all" — distinct from "Authorization present but malformed." |
| 185 | 243 | var errNoCredentials = errors.New("middleware: no credentials") |
@@ -216,11 +274,11 @@ func extractPAT(r *http.Request) (string, error) { |
| 216 | 274 | } |
| 217 | 275 | |
| 218 | 276 | // writePATChallenge writes the canonical 401 with a Bearer challenge. |
| 277 | +// Body is the /api/v1 JSON error envelope so the shithub-cli client and |
| 278 | +// any other JSON consumer can decode the failure uniformly. |
| 219 | 279 | func writePATChallenge(w http.ResponseWriter, realm, reason string) { |
| 220 | 280 | w.Header().Set("WWW-Authenticate", `Bearer realm="`+realm+`", error="invalid_token", error_description="`+reason+`"`) |
| 221 | | - w.Header().Set("Content-Type", "text/plain; charset=utf-8") |
| 222 | | - w.WriteHeader(http.StatusUnauthorized) |
| 223 | | - _, _ = w.Write([]byte(reason + "\n")) |
| 281 | + writeAPIJSONError(w, http.StatusUnauthorized, reason) |
| 224 | 282 | } |
| 225 | 283 | |
| 226 | 284 | // remoteAddrFromRequest pulls the client IP for last_used_ip. Reuses the |