Skip to content

Commit 0bc62a3

Browse files
enjenj
committed
Fix OAuth endpoints interface
Signed-off-by: Monis Khan <mkhan@redhat.com>
1 parent c610712 commit 0bc62a3

7 files changed

Lines changed: 52 additions & 75 deletions

File tree

‎pkg/oauthserver/mux.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
package oauthserver
22

3-
import (
4-
"net/http"
5-
)
3+
import "net/http"
64

75
type Mux interface {
86
Handle(pattern string, handler http.Handler)
97
HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request))
108
}
9+
10+
type Endpoints interface {
11+
// Install registers one or more http.Handlers into the given mux.
12+
// It is expected that the provided prefix will serve all operations.
13+
// prefix MUST NOT end in a slash.
14+
Install(mux Mux, prefix string)
15+
}

‎pkg/oauthserver/oauthserver/auth.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (c *OAuthServerConfig) WithOAuth(handler http.Handler) (http.Handler, error
149149
loginURL = c.ExtraOAuthConfig.Options.MasterPublicURL
150150
}
151151

152-
tokenRequestEndpoints := tokenrequest.NewEndpoints(loginURL, openShiftLogoutPrefix, c.getOsinOAuthClient, c.ExtraOAuthConfig.OAuthAccessTokenClient)
152+
tokenRequestEndpoints := tokenrequest.NewTokenRequest(loginURL, openShiftLogoutPrefix, c.getOsinOAuthClient, c.ExtraOAuthConfig.OAuthAccessTokenClient)
153153
tokenRequestEndpoints.Install(mux, urls.OpenShiftOAuthAPIPrefix)
154154

155155
if session := c.ExtraOAuthConfig.SessionAuth; session != nil {

‎pkg/oauthserver/osinserver/osinserver.go‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"github.com/openshift/origin/pkg/oauthserver"
1515
)
1616

17-
type Server struct {
17+
type osinServer struct {
1818
config *osin.ServerConfig
1919
server *osin.Server
2020
authorize AuthorizeHandler
@@ -31,15 +31,15 @@ func (l Logger) Printf(format string, v ...interface{}) {
3131
}
3232
}
3333

34-
func New(config *osin.ServerConfig, storage osin.Storage, authorize AuthorizeHandler, access AccessHandler, errorHandler ErrorHandler) *Server {
34+
func New(config *osin.ServerConfig, storage osin.Storage, authorize AuthorizeHandler, access AccessHandler, errorHandler ErrorHandler) oauthserver.Endpoints {
3535
server := osin.NewServer(config, storage)
3636

3737
// Override tokengen to ensure we get valid length tokens
3838
server.AuthorizeTokenGen = TokenGen{}
3939
server.AccessTokenGen = TokenGen{}
4040
server.Logger = Logger{}
4141

42-
return &Server{
42+
return &osinServer{
4343
config: config,
4444
server: server,
4545
authorize: authorize,
@@ -48,17 +48,13 @@ func New(config *osin.ServerConfig, storage osin.Storage, authorize AuthorizeHan
4848
}
4949
}
5050

51-
// Install registers the Server OAuth handlers into a mux. It is expected that the
52-
// provided prefix will serve all operations
53-
func (s *Server) Install(mux oauthserver.Mux, paths ...string) {
54-
for _, prefix := range paths {
55-
mux.HandleFunc(path.Join(prefix, urls.AuthorizePath), s.handleAuthorize)
56-
mux.HandleFunc(path.Join(prefix, urls.TokenPath), s.handleToken)
57-
mux.HandleFunc(path.Join(prefix, urls.InfoPath), s.handleInfo)
58-
}
51+
func (s *osinServer) Install(mux oauthserver.Mux, prefix string) {
52+
mux.HandleFunc(path.Join(prefix, urls.AuthorizePath), s.handleAuthorize)
53+
mux.HandleFunc(path.Join(prefix, urls.TokenPath), s.handleToken)
54+
mux.HandleFunc(path.Join(prefix, urls.InfoPath), s.handleInfo)
5955
}
6056

61-
func (s *Server) handleAuthorize(w http.ResponseWriter, r *http.Request) {
57+
func (s *osinServer) handleAuthorize(w http.ResponseWriter, r *http.Request) {
6258
resp := s.server.NewResponse()
6359
defer resp.Close()
6460

@@ -97,7 +93,7 @@ func (s *Server) handleAuthorize(w http.ResponseWriter, r *http.Request) {
9793
osin.OutputJSON(resp, w, r)
9894
}
9995

100-
func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) {
96+
func (s *osinServer) handleToken(w http.ResponseWriter, r *http.Request) {
10197
resp := s.server.NewResponse()
10298
defer resp.Close()
10399

@@ -114,7 +110,7 @@ func (s *Server) handleToken(w http.ResponseWriter, r *http.Request) {
114110
osin.OutputJSON(resp, w, r)
115111
}
116112

117-
func (s *Server) handleInfo(w http.ResponseWriter, r *http.Request) {
113+
func (s *osinServer) handleInfo(w http.ResponseWriter, r *http.Request) {
118114
resp := s.server.NewResponse()
119115
defer resp.Close()
120116

‎pkg/oauthserver/server/grant/grant.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"net/http"
66
"net/url"
77
"path"
8-
"strings"
98

109
"k8s.io/klog"
1110

@@ -98,13 +97,8 @@ func NewGrant(csrf csrf.CSRF, auth authenticator.Request, render FormRenderer, c
9897
}
9998
}
10099

101-
// Install registers the grant handler into a mux. It is expected that the
102-
// provided prefix will serve all operations. Path MUST NOT end in a slash.
103-
func (l *Grant) Install(mux oauthserver.Mux, paths ...string) {
104-
for _, path := range paths {
105-
path = strings.TrimRight(path, "/")
106-
mux.HandleFunc(path, l.ServeHTTP)
107-
}
100+
func (l *Grant) Install(mux oauthserver.Mux, prefix string) {
101+
mux.Handle(prefix, l)
108102
}
109103

110104
func (l *Grant) ServeHTTP(w http.ResponseWriter, req *http.Request) {

‎pkg/oauthserver/server/login/login.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"html/template"
99
"net/http"
10-
"strings"
1110

1211
"k8s.io/klog"
1312

@@ -89,13 +88,8 @@ func NewLogin(provider string, csrf csrf.CSRF, auth PasswordAuthenticator, rende
8988
}
9089
}
9190

92-
// Install registers the login handler into a mux. It is expected that the
93-
// provided prefix will serve all operations. Path MUST NOT end in a slash.
94-
func (l *Login) Install(mux oauthserver.Mux, paths ...string) {
95-
for _, path := range paths {
96-
path = strings.TrimRight(path, "/")
97-
mux.HandleFunc(path, l.ServeHTTP)
98-
}
91+
func (l *Login) Install(mux oauthserver.Mux, prefix string) {
92+
mux.Handle(prefix, l)
9993
}
10094

10195
func (l *Login) ServeHTTP(w http.ResponseWriter, req *http.Request) {

‎pkg/oauthserver/server/logout/logout.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,11 @@ import (
1111
"github.com/openshift/origin/pkg/oauthserver"
1212
"github.com/openshift/origin/pkg/oauthserver/server/redirect"
1313
"github.com/openshift/origin/pkg/oauthserver/server/session"
14-
"github.com/openshift/origin/pkg/oauthserver/server/tokenrequest"
1514
)
1615

1716
const thenParam = "then"
1817

19-
func NewLogout(invalidator session.SessionInvalidator, redirect string) tokenrequest.Endpoints {
18+
func NewLogout(invalidator session.SessionInvalidator, redirect string) oauthserver.Endpoints {
2019
return &logout{
2120
invalidator: invalidator,
2221
redirect: redirect,
@@ -28,10 +27,8 @@ type logout struct {
2827
redirect string
2928
}
3029

31-
func (l *logout) Install(mux oauthserver.Mux, paths ...string) {
32-
for _, path := range paths {
33-
mux.Handle(path, l)
34-
}
30+
func (l *logout) Install(mux oauthserver.Mux, prefix string) {
31+
mux.Handle(prefix, l)
3532
}
3633

3734
func (l *logout) ServeHTTP(w http.ResponseWriter, req *http.Request) {

‎pkg/oauthserver/server/tokenrequest/endpoints.go‎

Lines changed: 26 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
"github.com/openshift/origin/pkg/oauthserver/authenticator/password/bootstrap"
2020
)
2121

22-
type endpointDetails struct {
22+
type tokenRequest struct {
2323
publicMasterURL string
2424
// osinOAuthClient is the private OAuth client used by this endpoint.
2525
// It starts out nil and is lazily initialized when this endpoint is called.
@@ -38,13 +38,8 @@ type endpointDetails struct {
3838
openShiftLogoutPrefix string
3939
}
4040

41-
// TODO this interface needs to be moved
42-
type Endpoints interface {
43-
Install(mux oauthserver.Mux, paths ...string)
44-
}
45-
46-
func NewEndpoints(publicMasterURL, openShiftLogoutPrefix string, osinOAuthClientGetter func() (*osincli.Client, error), tokens v1.OAuthAccessTokenInterface) Endpoints {
47-
return &endpointDetails{
41+
func NewTokenRequest(publicMasterURL, openShiftLogoutPrefix string, osinOAuthClientGetter func() (*osincli.Client, error), tokens v1.OAuthAccessTokenInterface) oauthserver.Endpoints {
42+
return &tokenRequest{
4843
publicMasterURL: publicMasterURL,
4944
osinOAuthClientGetter: osinOAuthClientGetter,
5045
ready: make(chan struct{}),
@@ -53,23 +48,19 @@ func NewEndpoints(publicMasterURL, openShiftLogoutPrefix string, osinOAuthClient
5348
}
5449
}
5550

56-
// Install registers the request token endpoints into a mux. It is expected that the
57-
// provided prefix will serve all operations
58-
func (e *endpointDetails) Install(mux oauthserver.Mux, paths ...string) {
59-
for _, prefix := range paths {
60-
mux.HandleFunc(path.Join(prefix, urls.RequestTokenEndpoint), e.readyHandler(e.requestToken))
61-
mux.HandleFunc(path.Join(prefix, urls.DisplayTokenEndpoint), e.readyHandler(e.displayToken))
62-
mux.HandleFunc(path.Join(prefix, urls.ImplicitTokenEndpoint), e.implicitToken)
63-
}
51+
func (t *tokenRequest) Install(mux oauthserver.Mux, prefix string) {
52+
mux.HandleFunc(path.Join(prefix, urls.RequestTokenEndpoint), t.readyHandler(t.requestToken))
53+
mux.HandleFunc(path.Join(prefix, urls.DisplayTokenEndpoint), t.readyHandler(t.displayToken))
54+
mux.HandleFunc(path.Join(prefix, urls.ImplicitTokenEndpoint), t.implicitToken)
6455
}
6556

6657
// TODO we may want to start doing live lookups for this endpoint
67-
func (e *endpointDetails) readyHandler(delegate func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) {
58+
func (t *tokenRequest) readyHandler(delegate func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) {
6859
return func(w http.ResponseWriter, h *http.Request) {
6960
select {
70-
case <-e.ready:
61+
case <-t.ready:
7162
default:
72-
if err := e.safeInitOsinOAuthClientOnce(); err != nil {
63+
if err := t.safeInitOsinOAuthClientOnce(); err != nil {
7364
utilruntime.HandleError(fmt.Errorf("failed to get Osin OAuth client for token endpoint: %v", err))
7465
http.Error(w, "OAuth token endpoint is not ready", http.StatusInternalServerError)
7566
return
@@ -81,36 +72,36 @@ func (e *endpointDetails) readyHandler(delegate func(http.ResponseWriter, *http.
8172

8273
// safeInitOsinOAuthClientOnce initializes osinOAuthClient exactly once using osinOAuthClientGetter.
8374
// It is goroutine safe, reentrant and can be safely called multiple times.
84-
func (e *endpointDetails) safeInitOsinOAuthClientOnce() error {
75+
func (t *tokenRequest) safeInitOsinOAuthClientOnce() error {
8576
// Use a lock and nil check to make sure we never close endpoints.ready more than once
8677
// and that we only try to fetch osinOAuthClient until the first time we are successful
87-
e.initLock.Lock()
88-
defer e.initLock.Unlock()
89-
if e.osinOAuthClient == nil {
90-
osinOAuthClient, err := e.osinOAuthClientGetter()
78+
t.initLock.Lock()
79+
defer t.initLock.Unlock()
80+
if t.osinOAuthClient == nil {
81+
osinOAuthClient, err := t.osinOAuthClientGetter()
9182
if err != nil {
9283
return err
9384
}
94-
e.osinOAuthClient = osinOAuthClient
95-
close(e.ready)
85+
t.osinOAuthClient = osinOAuthClient
86+
close(t.ready)
9687
}
9788
return nil
9889
}
9990

10091
// requestToken works for getting a token in your browser and seeing what your token is
101-
func (e *endpointDetails) requestToken(w http.ResponseWriter, req *http.Request) {
102-
authReq := e.osinOAuthClient.NewAuthorizeRequest(osincli.CODE)
92+
func (t *tokenRequest) requestToken(w http.ResponseWriter, req *http.Request) {
93+
authReq := t.osinOAuthClient.NewAuthorizeRequest(osincli.CODE)
10394
oauthURL := authReq.GetAuthorizeUrl()
10495

10596
http.Redirect(w, req, oauthURL.String(), http.StatusFound)
10697
}
10798

108-
func (e *endpointDetails) displayToken(w http.ResponseWriter, req *http.Request) {
99+
func (t *tokenRequest) displayToken(w http.ResponseWriter, req *http.Request) {
109100
w.Header().Set("Content-Type", "text/html; charset=UTF-8")
110101
requestURL := urls.OpenShiftOAuthTokenRequestURL("") // relative url to token request endpoint
111-
data := tokenData{RequestURL: requestURL, PublicMasterURL: e.publicMasterURL}
102+
data := tokenData{RequestURL: requestURL, PublicMasterURL: t.publicMasterURL}
112103

113-
authorizeReq := e.osinOAuthClient.NewAuthorizeRequest(osincli.CODE)
104+
authorizeReq := t.osinOAuthClient.NewAuthorizeRequest(osincli.CODE)
114105
authorizeData, err := authorizeReq.HandleRequest(req)
115106
if err != nil {
116107
data.Error = fmt.Sprintf("Error handling auth request: %v", err)
@@ -119,7 +110,7 @@ func (e *endpointDetails) displayToken(w http.ResponseWriter, req *http.Request)
119110
return
120111
}
121112

122-
accessReq := e.osinOAuthClient.NewAccessRequest(osincli.AUTHORIZATION_CODE, authorizeData)
113+
accessReq := t.osinOAuthClient.NewAccessRequest(osincli.AUTHORIZATION_CODE, authorizeData)
123114
accessData, err := accessReq.GetToken()
124115
if err != nil {
125116
data.Error = fmt.Sprintf("Error getting token: %v", err)
@@ -128,7 +119,7 @@ func (e *endpointDetails) displayToken(w http.ResponseWriter, req *http.Request)
128119
return
129120
}
130121

131-
token, err := e.tokens.Get(accessData.AccessToken, metav1.GetOptions{})
122+
token, err := t.tokens.Get(accessData.AccessToken, metav1.GetOptions{})
132123
if err != nil {
133124
data.Error = "Error checking token" // do not leak error to user, do not log error
134125
w.WriteHeader(http.StatusInternalServerError)
@@ -138,7 +129,7 @@ func (e *endpointDetails) displayToken(w http.ResponseWriter, req *http.Request)
138129

139130
if token.UserName == bootstrap.BootstrapUser {
140131
// only the bootstrap user has a session we maintain for one more than OAuth flow
141-
data.LogoutURL = e.openShiftLogoutPrefix
132+
data.LogoutURL = t.openShiftLogoutPrefix
142133
}
143134

144135
data.AccessToken = accessData.AccessToken
@@ -204,7 +195,7 @@ var tokenTemplate = template.Must(template.New("tokenTemplate").Parse(`
204195
{{ end }}
205196
`))
206197

207-
func (e *endpointDetails) implicitToken(w http.ResponseWriter, req *http.Request) {
198+
func (t *tokenRequest) implicitToken(w http.ResponseWriter, req *http.Request) {
208199
w.Header().Set("Content-Type", "text/plain")
209200
_, _ = w.Write([]byte(`
210201
You have reached this page by following a redirect Location header from an OAuth authorize request.

0 commit comments

Comments
 (0)