diff --git a/server/apiv1/api.go b/server/apiv1/api.go index 76926d738..c8e25618f 100644 --- a/server/apiv1/api.go +++ b/server/apiv1/api.go @@ -1,6 +1,7 @@ package apiv1 import ( + "cmp" "errors" "net/http" "runtime/debug" @@ -126,9 +127,10 @@ func headAsGet(mux chi.Routes) func(http.Handler) http.Handler { } } +// routePath must pick the same path chi's routeHTTP dispatches on, or the gate could vet a different route. func routePath(req *http.Request) string { if rctx := chi.RouteContext(req.Context()); rctx != nil && rctx.RoutePath != "" { return rctx.RoutePath } - return req.URL.Path + return cmp.Or(req.URL.RawPath, req.URL.Path, "/") } diff --git a/server/apiv1/gate.go b/server/apiv1/gate.go index 19234413b..d125eb846 100644 --- a/server/apiv1/gate.go +++ b/server/apiv1/gate.go @@ -218,22 +218,29 @@ func (g *gate) authorize(w http.ResponseWriter, r *http.Request, op *gateOp) (*h writeProblem(w, r, err) return r, false case errors.Is(err, apiauth.ErrInsufficientScope): - w.Header().Set("WWW-Authenticate", `Bearer error="insufficient_scope"`) - writeProblem(w, r, err) + insufficientScope(w, r, op, err) return r, false case err != nil: writeProblem(w, r, err) return r, false } if op.scope != "" && !apiauth.Satisfies(p.Scopes, op.scope) { - w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer error="insufficient_scope", scope=%q`, op.scope)) - writeProblem(w, r, apiauth.ErrInsufficientScope) + insufficientScope(w, r, op, apiauth.ErrInsufficientScope) return r, false } ctx := apiauth.WithPrincipal(request.WithUser(r.Context(), p.User), p) return r.WithContext(ctx), true } +func insufficientScope(w http.ResponseWriter, r *http.Request, op *gateOp, err error) { + challenge := `Bearer error="insufficient_scope"` + if op.scope != "" { + challenge += fmt.Sprintf(`, scope=%q`, op.scope) + } + w.Header().Set("WWW-Authenticate", challenge) + writeProblem(w, r, err) +} + func bearerToken(r *http.Request) (string, bool) { scheme, token, ok := strings.Cut(strings.TrimSpace(r.Header.Get("Authorization")), " ") token = strings.TrimSpace(token) diff --git a/server/apiv1/gate_test.go b/server/apiv1/gate_test.go index a74f76b26..6fb7daa6c 100644 --- a/server/apiv1/gate_test.go +++ b/server/apiv1/gate_test.go @@ -209,6 +209,26 @@ var _ = Describe("spec gate", func() { Expect(decodeProblem(w).Code).To(Equal(ProblemCodeInsufficientScope)) }) + It("names the operation's scope when Authenticate reports an insufficient scope", func() { + fa.err = apiauth.ErrInsufficientScope + w := do(http.MethodGet, "/things/1", "Bearer x", "") + Expect(w.Code).To(Equal(http.StatusForbidden)) + Expect(w.Header().Get("WWW-Authenticate")).To(Equal(`Bearer error="insufficient_scope", scope="read"`)) + }) + + It("looks routes up on the raw path, as chi dispatches them", func() { + w := do(http.MethodGet, "/things/a%2Fb", "", "") + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + Expect(reached).To(BeEmpty()) + + root := chi.NewRouter() + root.Mount("/music/api/v1", mux) + w = httptest.NewRecorder() + root.ServeHTTP(w, httptest.NewRequestWithContext(ctx, http.MethodGet, "/music/api/v1/things/a%2Fb", nil)) + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + Expect(reached).To(BeEmpty()) + }) + It("works when mounted under a base path", func() { root := chi.NewRouter() root.Mount("/music/api/v1", mux)