Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 4 additions & 25 deletions server/device/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ type Handler struct {

// Mount registers the device authorization routes.
func (h *Handler) Mount(m router.Mux) {
m.HandleFunc("/device", h.handleDeviceExchange)
m.HandleFunc("/device/auth/verify_code", h.verifyUserCode)
m.HandleFunc("/device/code", h.handleDeviceCode)
m.HandleFunc(oauth2.DeviceCallbackURI, h.handleDeviceCallback)
m.HandleFunc("/device", h.handleDeviceExchange, http.MethodGet)
m.HandleFunc("/device/auth/verify_code", h.verifyUserCode, http.MethodPost)
m.HandleFunc("/device/code", h.handleDeviceCode, http.MethodPost)
m.HandleFunc(oauth2.DeviceCallbackURI, h.handleDeviceCallback, http.MethodGet)
}

// deviceFlowError is a failed step in the flow. A non-empty OAuth2 code makes the
Expand Down Expand Up @@ -99,11 +99,6 @@ func (h *Handler) getDeviceVerificationURI() string {

// handleDeviceExchange serves the /device user-code entry page.
func (h *Handler) handleDeviceExchange(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet {
h.renderError(r, w, http.StatusBadRequest, "Requested resource does not exist.")
return
}

// If "user_code" is set, pre-populate the user code field. If "invalid" is
// set, show a message that the code was invalid or expired.
userCode := r.URL.Query().Get("user_code")
Expand Down Expand Up @@ -214,12 +209,6 @@ func (h *Handler) createDeviceAuthorization(ctx context.Context, req deviceCodeR
}

func (h *Handler) handleDeviceCode(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
h.renderError(r, w, http.StatusBadRequest, "Invalid device code request type")
h.writeError(w, oauth2.InvalidRequest, "", http.StatusBadRequest)
return
}

req, ferr := h.parseDeviceCodeRequest(r)
if ferr != nil {
h.writeFlowError(r, w, ferr)
Expand All @@ -244,10 +233,6 @@ func writeDeviceCodeResponse(w http.ResponseWriter, resp *DeviceCodeResponse) {
}

func (h *Handler) verifyUserCode(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
h.renderError(r, w, http.StatusBadRequest, "Requested resource does not exist.")
return
}
ctx := r.Context()

if err := r.ParseForm(); err != nil {
Expand Down Expand Up @@ -293,12 +278,6 @@ func (h *Handler) verifyUserCode(w http.ResponseWriter, r *http.Request) {
}

func (h *Handler) handleDeviceCallback(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet {
h.Logger.ErrorContext(r.Context(), "unsupported method in device callback", "method", r.Method)
h.renderError(r, w, http.StatusBadRequest, "Method not allowed.")
return
}

clientName, ferr := h.completeDeviceAuthorization(w, r)
if ferr != nil {
h.writeFlowError(r, w, ferr)
Expand Down
9 changes: 6 additions & 3 deletions server/device_authorize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,15 @@ func TestHandleDeviceCode(t *testing.T) {
expectedContentType: "application/json",
},
{
testName: "Invalid request Type (GET)",
// The router restricts /device/code to POST, so a GET is rejected
// with the shared 405 handler before reaching the handler. That
// handler renders the HTML error page, which sets no content type.
testName: "Method not allowed (GET)",
clientID: "test",
Comment on lines +43 to 47
requestType: "GET",
scopes: []string{"openid", "profile", "email"},
expectedResponseCode: http.StatusBadRequest,
expectedContentType: "application/json",
expectedResponseCode: http.StatusMethodNotAllowed,
expectedContentType: "",
},
{
testName: "New Code with valid PKCE",
Expand Down
5 changes: 3 additions & 2 deletions server/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,9 @@ func TestDeviceCallbackMethodError(t *testing.T) {
body, _ := io.ReadAll(resp.Body)
bodyStr := string(body)

// Should not expose the method name in error
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
// The router now rejects the wrong method with the shared 405 handler, which
// must not expose the method name in the error.
require.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode)
require.NotContains(t, bodyStr, "PUT")
require.NotContains(t, bodyStr, "method not implemented")
}
Expand Down
6 changes: 1 addition & 5 deletions server/grants/grants.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,18 +196,14 @@ func (e *Endpoint) register(supported []string, gs ...Grant) {

// Mount registers the token route.
func (e *Endpoint) Mount(m router.Mux) {
m.HandleCORS("/token", e.handleToken)
m.HandleCORS("/token", e.handleToken, http.MethodPost)
}

// handleToken serves /token: it validates the request shape and dispatches to the
// grant for its grant_type.
func (e *Endpoint) handleToken(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
w.Header().Set("Content-Type", "application/json")
if r.Method != http.MethodPost {
e.writeError(ctx, w, &oauth2.Error{Type: oauth2.InvalidRequest, Description: "method not allowed", Status: http.StatusBadRequest})
return
}
if err := r.ParseForm(); err != nil {
e.logger.ErrorContext(ctx, "could not parse request body", "err", err)
e.writeError(ctx, w, &oauth2.Error{Type: oauth2.InvalidRequest, Status: http.StatusBadRequest})
Expand Down
6 changes: 2 additions & 4 deletions server/introspection/introspection.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ type Handler struct {

// Mount registers the introspection route.
func (h *Handler) Mount(m router.Mux) {
m.HandleCORS("/token/introspect", h.handle)
m.HandleCORS("/token/introspect", h.handle, http.MethodPost)
}

func (h *Handler) guessTokenType(ctx context.Context, token string) (TokenTypeEnum, error) {
Expand All @@ -187,9 +187,7 @@ func (h *Handler) guessTokenType(ctx context.Context, token string) (TokenTypeEn
}

func (h *Handler) getTokenFromRequest(r *http.Request) (string, TokenTypeEnum, error) {
if r.Method != "POST" {
return "", 0, newIntrospectBadRequestError(fmt.Sprintf("HTTP method is \"%s\", expected \"POST\".", r.Method))
} else if err := r.ParseForm(); err != nil {
if err := r.ParseForm(); err != nil {
return "", 0, newIntrospectBadRequestError("Unable to parse HTTP body, make sure to send a properly formatted form request body.")
} else if len(r.PostForm) == 0 {
return "", 0, newIntrospectBadRequestError("The POST body can not be empty.")
Expand Down
11 changes: 3 additions & 8 deletions server/introspection/introspection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,9 @@ func TestGetTokenFromRequestSuccess(t *testing.T) {
func TestGetTokenFromRequestFailure(t *testing.T) {
h := testHandler(t)

_, _, err := h.getTokenFromRequest(httptest.NewRequest(http.MethodGet, "https://test.tech/token/introspect", nil))
require.ErrorIs(t, err, &introspectionError{
typ: oauth2.InvalidRequest,
desc: "HTTP method is \"GET\", expected \"POST\".",
code: http.StatusBadRequest,
})

_, _, err = h.getTokenFromRequest(httptest.NewRequest(http.MethodPost, "https://test.tech/token/introspect", nil))
// The method is now enforced at the router level (POST only), so
// getTokenFromRequest no longer checks it; only body validation remains.
_, _, err := h.getTokenFromRequest(httptest.NewRequest(http.MethodPost, "https://test.tech/token/introspect", nil))
require.ErrorIs(t, err, &introspectionError{
typ: oauth2.InvalidRequest,
desc: "The POST body can not be empty.",
Expand Down
18 changes: 12 additions & 6 deletions server/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,20 @@ import "net/http"

// Mux registers HTTP routes. The server provides the implementation (path
// prefixing, per-route headers, CORS); handlers only name their routes.
//
// The registration methods take an optional list of HTTP methods. When methods
// are given the route only matches those methods and the server answers any
// other method with a uniform 405, so handlers no longer guard the method
// themselves. Passing no methods leaves the route open to every method.
type Mux interface {
// Handle mounts h at pattern.
Handle(pattern string, h http.Handler)
// HandleFunc mounts h at pattern.
HandleFunc(pattern string, h http.HandlerFunc)
// Handle mounts h at pattern, optionally restricting it to methods.
Handle(pattern string, h http.Handler, methods ...string)
// HandleFunc mounts h at pattern, optionally restricting it to methods.
HandleFunc(pattern string, h http.HandlerFunc, methods ...string)
// HandleCORS mounts h at pattern with cross-origin support (discovery,
// token, keys and similar public endpoints).
HandleCORS(pattern string, h http.HandlerFunc)
// token, keys and similar public endpoints), optionally restricting it to
// methods.
HandleCORS(pattern string, h http.HandlerFunc, methods ...string)
// HandlePrefix mounts h for every path under pattern, stripping the prefix.
HandlePrefix(pattern string, h http.Handler)
}
Expand Down
48 changes: 35 additions & 13 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,16 +256,20 @@ type Server struct {
// routeMux adapts the server's route-registration closures to router.Mux so
// domain handlers can mount their own routes.
type routeMux struct {
handle func(string, http.Handler)
handleFunc func(string, http.HandlerFunc)
handleCORS func(string, http.HandlerFunc)
handle func(string, http.Handler, ...string)
handleFunc func(string, http.HandlerFunc, ...string)
handleCORS func(string, http.HandlerFunc, ...string)
handlePrefix func(string, http.Handler)
}

func (m routeMux) Handle(p string, h http.Handler) { m.handle(p, h) }
func (m routeMux) HandleFunc(p string, h http.HandlerFunc) { m.handleFunc(p, h) }
func (m routeMux) HandleCORS(p string, h http.HandlerFunc) { m.handleCORS(p, h) }
func (m routeMux) HandlePrefix(p string, h http.Handler) { m.handlePrefix(p, h) }
func (m routeMux) Handle(p string, h http.Handler, methods ...string) { m.handle(p, h, methods...) }
func (m routeMux) HandleFunc(p string, h http.HandlerFunc, methods ...string) {
m.handleFunc(p, h, methods...)
}
func (m routeMux) HandleCORS(p string, h http.HandlerFunc, methods ...string) {
m.handleCORS(p, h, methods...)
}
func (m routeMux) HandlePrefix(p string, h http.Handler) { m.handlePrefix(p, h) }

// newDiscoveryHandler builds a discovery handler from the server's settings. It
// is shared by the mounted handler and ConstructDiscovery.
Expand Down Expand Up @@ -581,28 +585,46 @@ func newServer(ctx context.Context, c Config) (*Server, error) {
}

r := mux.NewRouter().SkipClean(true).UseEncodedPath()
handle := func(p string, h http.Handler) {
r.Handle(path.Join(issuerURL.Path, p), handlerWithHeaders(p, h))
handle := func(p string, h http.Handler, methods ...string) {
route := r.Handle(path.Join(issuerURL.Path, p), handlerWithHeaders(p, h))
if len(methods) > 0 {
route.Methods(methods...)
}
}
handleFunc := func(p string, h http.HandlerFunc) {
handle(p, h)
handleFunc := func(p string, h http.HandlerFunc, methods ...string) {
handle(p, h, methods...)
}
handlePrefix := func(p string, h http.Handler) {
prefix := path.Join(issuerURL.Path, p)
r.PathPrefix(prefix).Handler(http.StripPrefix(prefix, h))
}
handleWithCORS := func(p string, h http.HandlerFunc) {
handleWithCORS := func(p string, h http.HandlerFunc, methods ...string) {
var handler http.Handler = h
routeMethods := methods
if len(c.AllowedOrigins) > 0 {
cors := handlers.CORS(
handlers.AllowedOrigins(c.AllowedOrigins),
handlers.AllowedHeaders(c.AllowedHeaders),
)
handler = cors(handler)
// CORS preflight requests use OPTIONS; allow it through to the CORS
// middleware so it can answer them instead of the 405 handler.
if len(methods) > 0 {
routeMethods = append(append([]string{}, methods...), http.MethodOptions)
}
}
route := r.Handle(path.Join(issuerURL.Path, p), handlerWithHeaders(p, handler))
if len(routeMethods) > 0 {
route.Methods(routeMethods...)
}
r.Handle(path.Join(issuerURL.Path, p), handlerWithHeaders(p, handler))
}
r.NotFoundHandler = http.NotFoundHandler()
// A route whose path matches but whose method does not yields a uniform 405,
// so handlers no longer guard the method themselves. It runs through
// handlerWithHeaders so configured response headers still apply.
r.MethodNotAllowedHandler = handlerWithHeaders("method_not_allowed", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
s.renderError(r, w, http.StatusMethodNotAllowed, ErrMsgMethodNotAllowed)
}))

// Self-contained domains mount their own routes through the router.Mux
// abstraction, so this list is the only place they are wired in.
Expand Down