Merge pull request #22767 from enj/enj/i/frame_mux

Bug 1693018: Add CSRF to token request endpoint

Author: eparis

pkg/oauthserver/mux.go

@@ -1,10 +1,15 @@
 package oauthserver
 
-import (
-	"net/http"
-)
+import "net/http"
 
 type Mux interface {
 	Handle(pattern string, handler http.Handler)
 	HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request))
 }
+
+type Endpoints interface {
+	// Install registers one or more http.Handlers into the given mux.
+	// It is expected that the provided prefix will serve all operations.
+	// prefix MUST NOT end in a slash.
+	Install(mux Mux, prefix string)
+}

pkg/oauthserver/oauthserver/auth.go

@@ -149,7 +149,7 @@ func (c *OAuthServerConfig) WithOAuth(handler http.Handler) (http.Handler, error
 		loginURL = c.ExtraOAuthConfig.Options.MasterPublicURL
 	}
 
-	tokenRequestEndpoints := tokenrequest.NewEndpoints(loginURL, openShiftLogoutPrefix, c.getOsinOAuthClient, c.ExtraOAuthConfig.OAuthAccessTokenClient)
+	tokenRequestEndpoints := tokenrequest.NewTokenRequest(loginURL, openShiftLogoutPrefix, c.getOsinOAuthClient, c.ExtraOAuthConfig.OAuthAccessTokenClient, c.getCSRF())
 	tokenRequestEndpoints.Install(mux, urls.OpenShiftOAuthAPIPrefix)
 
 	if session := c.ExtraOAuthConfig.SessionAuth; session != nil {

pkg/oauthserver/osinserver/osinserver.go

@@ -14,7 +14,7 @@ import (
 	"github.com/openshift/origin/pkg/oauthserver"
 )
 
-type Server struct {
+type osinServer struct {
 	config       *osin.ServerConfig
 	server       *osin.Server
 	authorize    AuthorizeHandler
@@ -31,15 +31,15 @@ func (l Logger) Printf(format string, v ...interface{}) {
 	}
 }
 
-func New(config *osin.ServerConfig, storage osin.Storage, authorize AuthorizeHandler, access AccessHandler, errorHandler ErrorHandler) *Server {
+func New(config *osin.ServerConfig, storage osin.Storage, authorize AuthorizeHandler, access AccessHandler, errorHandler ErrorHandler) oauthserver.Endpoints {
 	server := osin.NewServer(config, storage)
 
 	// Override tokengen to ensure we get valid length tokens
 	server.AuthorizeTokenGen = TokenGen{}
 	server.AccessTokenGen = TokenGen{}
 	server.Logger = Logger{}
 
-	return &Server{
+	return &osinServer{
 		config:       config,
 		server:       server,
 		authorize:    authorize,
@@ -48,17 +48,13 @@ func New(config *osin.ServerConfig, storage osin.Storage, authorize AuthorizeHan
 	}
 }
 
-// Install registers the Server OAuth handlers into a mux. It is expected that the
-// provided prefix will serve all operations
-func (s *Server) Install(mux oauthserver.Mux, paths ...string) {
-	for _, prefix := range paths {
-		mux.HandleFunc(path.Join(prefix, urls.AuthorizePath), s.handleAuthorize)
-		mux.HandleFunc(path.Join(prefix, urls.TokenPath), s.handleToken)
-		mux.HandleFunc(path.Join(prefix, urls.InfoPath), s.handleInfo)
-	}
+func (s *osinServer) Install(mux oauthserver.Mux, prefix string) {
+	mux.HandleFunc(path.Join(prefix, urls.AuthorizePath), s.handleAuthorize)
+	mux.HandleFunc(path.Join(prefix, urls.TokenPath), s.handleToken)
+	mux.HandleFunc(path.Join(prefix, urls.InfoPath), s.handleInfo)
 }
 
-func (s *Server) handleAuthorize(w http.ResponseWriter, r *http.Request) {
+func (s *osinServer) handleAuthorize(w http.ResponseWriter, r *http.Request) {
 	resp := s.server.NewResponse()
 	defer resp.Close()
 
@@ -97,7 +93,7 @@ func (s *Server) handleAuthorize(w http.ResponseWriter, r *http.Request) {
 	osin.OutputJSON(resp, w, r)
 }
 
-func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) {
+func (s *osinServer) handleToken(w http.ResponseWriter, r *http.Request) {
 	resp := s.server.NewResponse()
 	defer resp.Close()
 
@@ -114,7 +110,7 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) {
 	osin.OutputJSON(resp, w, r)
 }
 
-func (s *Server) handleInfo(w http.ResponseWriter, r *http.Request) {
+func (s *osinServer) handleInfo(w http.ResponseWriter, r *http.Request) {
 	resp := s.server.NewResponse()
 	defer resp.Close()
 

pkg/oauthserver/server/grant/grant.go

@@ -5,7 +5,6 @@ import (
 	"net/http"
 	"net/url"
 	"path"
-	"strings"
 
 	"k8s.io/klog"
 
@@ -98,13 +97,8 @@ func NewGrant(csrf csrf.CSRF, auth authenticator.Request, render FormRenderer, c
 	}
 }
 
-// Install registers the grant handler into a mux. It is expected that the
-// provided prefix will serve all operations. Path MUST NOT end in a slash.
-func (l *Grant) Install(mux oauthserver.Mux, paths ...string) {
-	for _, path := range paths {
-		path = strings.TrimRight(path, "/")
-		mux.HandleFunc(path, l.ServeHTTP)
-	}
+func (l *Grant) Install(mux oauthserver.Mux, prefix string) {
+	mux.Handle(prefix, l)
 }
 
 func (l *Grant) ServeHTTP(w http.ResponseWriter, req *http.Request) {

pkg/oauthserver/server/login/login.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"html/template"
 	"net/http"
-	"strings"
 
 	"k8s.io/klog"
 
@@ -89,13 +88,8 @@ func NewLogin(provider string, csrf csrf.CSRF, auth PasswordAuthenticator, rende
 	}
 }
 
-// Install registers the login handler into a mux. It is expected that the
-// provided prefix will serve all operations. Path MUST NOT end in a slash.
-func (l *Login) Install(mux oauthserver.Mux, paths ...string) {
-	for _, path := range paths {
-		path = strings.TrimRight(path, "/")
-		mux.HandleFunc(path, l.ServeHTTP)
-	}
+func (l *Login) Install(mux oauthserver.Mux, prefix string) {
+	mux.Handle(prefix, l)
 }
 
 func (l *Login) ServeHTTP(w http.ResponseWriter, req *http.Request) {

pkg/oauthserver/server/logout/logout.go

@@ -11,12 +11,11 @@ import (
 	"github.com/openshift/origin/pkg/oauthserver"
 	"github.com/openshift/origin/pkg/oauthserver/server/redirect"
 	"github.com/openshift/origin/pkg/oauthserver/server/session"
-	"github.com/openshift/origin/pkg/oauthserver/server/tokenrequest"
 )
 
 const thenParam = "then"
 
-func NewLogout(invalidator session.SessionInvalidator, redirect string) tokenrequest.Endpoints {
+func NewLogout(invalidator session.SessionInvalidator, redirect string) oauthserver.Endpoints {
 	return &logout{
 		invalidator: invalidator,
 		redirect:    redirect,
@@ -28,10 +27,8 @@ type logout struct {
 	redirect    string
 }
 
-func (l *logout) Install(mux oauthserver.Mux, paths ...string) {
-	for _, path := range paths {
-		mux.Handle(path, l)
-	}
+func (l *logout) Install(mux oauthserver.Mux, prefix string) {
+	mux.Handle(prefix, l)
 }
 
 func (l *logout) ServeHTTP(w http.ResponseWriter, req *http.Request) {