Browse code

Refactor external oauth providers, add GitHub, use osincli.Client

Refactor auth mux wiring

Code review

Remove custom String() functions, dump config in verbose logging only

Move token auth failure logging into failure handler

Validate state, redirect inside success handler, add success handler chain

Code review 2: godoc, require error handlers and use consistently, fix closure bug

Jordan Liggitt authored on 2014/10/22 21:54:06
Showing 25 changed files
... ...
@@ -1,5 +1,6 @@
1 1
 package api
2 2
 
3
+// TODO: Add display name to common meta?
3 4
 type UserInfo interface {
4 5
 	GetName() string
5 6
 	GetUID() string
... ...
@@ -1,7 +1,6 @@
1 1
 package bearertoken
2 2
 
3 3
 import (
4
-	"fmt"
5 4
 	"net/http"
6 5
 	"strings"
7 6
 
... ...
@@ -30,7 +29,3 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (api.UserInfo, bo
30 30
 	token := parts[1]
31 31
 	return a.auth.AuthenticateToken(token)
32 32
 }
33
-
34
-func (a *Authenticator) String() string {
35
-	return fmt.Sprintf("TokenBearerAuthenticator{tokenValidator:%v}", a.auth)
36
-}
... ...
@@ -2,7 +2,6 @@ package file
2 2
 
3 3
 import (
4 4
 	"encoding/csv"
5
-	"fmt"
6 5
 	"io"
7 6
 	"os"
8 7
 
... ...
@@ -57,7 +56,3 @@ func (a *TokenAuthenticator) AuthenticateToken(value string) (api.UserInfo, bool
57 57
 	}
58 58
 	return user, true, nil
59 59
 }
60
-
61
-func (a *TokenAuthenticator) String() string {
62
-	return fmt.Sprintf("CsvTokenValidator{csvLocation:%v}", a.path)
63
-}
... ...
@@ -1,7 +1,6 @@
1 1
 package requestheader
2 2
 
3 3
 import (
4
-	"fmt"
5 4
 	"net/http"
6 5
 
7 6
 	"github.com/openshift/origin/pkg/auth/api"
... ...
@@ -35,7 +34,3 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (api.UserInfo, bo
35 35
 	}
36 36
 	return user, true, nil
37 37
 }
38
-
39
-func (h *Authenticator) String() string {
40
-	return fmt.Sprintf("RequestHeaderAuthenticator{userHeaderName:%v}", h.config.UserNameHeader)
41
-}
... ...
@@ -16,11 +16,8 @@ func NewRequestAuthenticator(context RequestContext, auth authenticator.Request,
16 16
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
17 17
 		user, ok, err := auth.AuthenticateRequest(req)
18 18
 		if err != nil || !ok {
19
-			// TODO make this actually fail.  For now, just let us know and continue on your merry way
20
-			glog.V(2).Infof("Token authentication failed when accessing: %v", req.URL)
21
-
22
-			// failed.ServeHTTP(w, req)
23
-			// return
19
+			failed.ServeHTTP(w, req)
20
+			return
24 21
 		} else {
25 22
 			glog.V(1).Infof("Found user, %v, when accessing %v", user, req.URL)
26 23
 
27 24
deleted file mode 100644
... ...
@@ -1,42 +0,0 @@
1
-package googlecallback
2
-
3
-import (
4
-	"fmt"
5
-	"net/http"
6
-	"net/url"
7
-
8
-	"github.com/golang/glog"
9
-)
10
-
11
-type GoogleAuthenticationHandler struct {
12
-	RedirectUri string
13
-	ClientId    string
14
-}
15
-
16
-func (g *GoogleAuthenticationHandler) AuthenticationNeeded(w http.ResponseWriter, req *http.Request) {
17
-	glog.V(1).Infof("Authentication needed for %v", g)
18
-
19
-	// TODO: tidy this
20
-	// build our url:
21
-	oauthUrl, _ := url.Parse("https://accounts.google.com/o/oauth2/auth")
22
-	query := url.Values{}
23
-	query.Set("response_type", "code")
24
-	query.Set("client_id", g.ClientId)
25
-	query.Set("redirect_uri", g.RedirectUri)
26
-	query.Set("scope", "profile email")
27
-	query.Set("state", "go-to-somewhere")
28
-	query.Set("include_granted_scopes", "true")
29
-	query.Set("access_type", "offline")
30
-	oauthUrl.RawQuery = query.Encode()
31
-
32
-	glog.V(1).Infof("redirect to  %v", oauthUrl)
33
-
34
-	http.Redirect(w, req, oauthUrl.String(), http.StatusFound)
35
-}
36
-func (g *GoogleAuthenticationHandler) AuthenticationError(err error, w http.ResponseWriter, req *http.Request) {
37
-	fmt.Fprintf(w, "<body>AuthenticationError - %s</body>", err)
38
-}
39
-
40
-func (g *GoogleAuthenticationHandler) String() string {
41
-	return fmt.Sprintf("GoogleAuthenticationHandler{ClientId: %v}", g.ClientId)
42
-}
43 1
deleted file mode 100644
... ...
@@ -1,86 +0,0 @@
1
-package googlecallback
2
-
3
-import (
4
-	// "bytes"
5
-	"encoding/base64"
6
-	"encoding/json"
7
-	"io/ioutil"
8
-	"net/http"
9
-	"net/url"
10
-	"strings"
11
-
12
-	"github.com/golang/glog"
13
-	"github.com/openshift/origin/pkg/oauth/registry/accesstoken"
14
-)
15
-
16
-type OauthCallbackHandler struct {
17
-	TokenRegistry accesstoken.Registry
18
-	ClientId      string
19
-	ClientSecret  string
20
-}
21
-
22
-func (o *OauthCallbackHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
23
-	code := req.URL.Query().Get("code")
24
-	if len(code) == 0 {
25
-		w.WriteHeader(http.StatusBadRequest)
26
-		return
27
-	}
28
-
29
-	// the code we get back seems to have multiple parts, we need everything before the period
30
-	code = code[0:strings.Index(code, ".")]
31
-	glog.Infof("Got a callback from google for: %v", code)
32
-
33
-	//
34
-	// with this code, I thee post.  We need the token
35
-	resp, err := http.PostForm("https://accounts.google.com/o/oauth2/token", url.Values{
36
-		"grant_type":    {"authorization_code"},
37
-		"code":          {code},
38
-		"client_id":     {o.ClientId},
39
-		"client_secret": {o.ClientSecret},
40
-		"redirect_uri":  {"http://localhost:8080/oauth2callback/google"},
41
-	})
42
-	if err != nil {
43
-		glog.Errorf("Error making post: %v", err)
44
-	}
45
-	body, err := ioutil.ReadAll(resp.Body)
46
-	if err != nil {
47
-		glog.Errorf("Error making post: %v", err)
48
-	}
49
-	var jsonResponse interface{}
50
-	err = json.Unmarshal(body, &jsonResponse)
51
-	if err != nil {
52
-		glog.Errorf("Error making post: %v", err)
53
-	}
54
-	tokenResponseMap := jsonResponse.(map[string]interface{})
55
-	glog.V(1).Infof("Got a response: %v", tokenResponseMap)
56
-
57
-	googleAccessToken := tokenResponseMap["access_token"].(string)
58
-	glog.Infof("Got a google access token of: %v", googleAccessToken)
59
-	googleIdToken := tokenResponseMap["id_token"].(string)
60
-	glog.Infof("Got a google id token of: %v", googleIdToken)
61
-
62
-	// pull the ID from the id token.  We don't need to validate since we got the info directly from
63
-	// our https request to google
64
-	encodedIdTokenParts := strings.Split(googleIdToken, ".")
65
-	if len(encodedIdTokenParts) >= 2 {
66
-		encodedId := encodedIdTokenParts[1]
67
-		decodedIdBytes := make([]byte, base64.StdEncoding.DecodedLen(len(encodedId)))
68
-		base64.StdEncoding.Decode(decodedIdBytes, []byte(encodedId))
69
-
70
-		// TODO complete hack. figure out why the decode fails
71
-		decodedIdBytes = append(decodedIdBytes, byte('}'))
72
-
73
-		glog.V(1).Infof("decodedIdBytes %v", string(decodedIdBytes))
74
-
75
-		var decodedIdJson interface{}
76
-		err = json.Unmarshal(decodedIdBytes, &decodedIdJson)
77
-		if err != nil {
78
-			glog.Errorf("Error interpretting token: %v", err)
79
-		}
80
-
81
-		idMap := decodedIdJson.(map[string]interface{})
82
-		emailAddress := idMap["email"]
83
-		glog.Infof("User email: %v", emailAddress)
84
-
85
-	}
86
-}
87 1
new file mode 100644
... ...
@@ -0,0 +1,86 @@
0
+package github
1
+
2
+import (
3
+	"encoding/json"
4
+	"fmt"
5
+	"io/ioutil"
6
+	"net/http"
7
+
8
+	"github.com/RangelReale/osincli"
9
+	"github.com/openshift/origin/pkg/auth/api"
10
+	"github.com/openshift/origin/pkg/auth/oauth/external"
11
+)
12
+
13
+const (
14
+	githubAuthorizeUrl = "https://github.com/login/oauth/authorize"
15
+	githubTokenUrl     = "https://github.com/login/oauth/access_token"
16
+	githubUserApiUrl   = "https://api.github.com/user"
17
+	githubOauthScope   = "user:email"
18
+)
19
+
20
+type provider struct {
21
+	client_id, client_secret string
22
+}
23
+
24
+type githubUser struct {
25
+	ID    uint64
26
+	Login string
27
+	Email string
28
+	Name  string
29
+}
30
+
31
+func NewProvider(client_id, client_secret string) external.Provider {
32
+	return provider{client_id, client_secret}
33
+}
34
+
35
+func (p provider) NewConfig() (*osincli.ClientConfig, error) {
36
+	config := &osincli.ClientConfig{
37
+		ClientId:                 p.client_id,
38
+		ClientSecret:             p.client_secret,
39
+		ErrorsInStatusCode:       true,
40
+		SendClientSecretInParams: true,
41
+		AuthorizeUrl:             githubAuthorizeUrl,
42
+		TokenUrl:                 githubTokenUrl,
43
+		Scope:                    githubOauthScope,
44
+	}
45
+	return config, nil
46
+}
47
+
48
+func (p provider) AddCustomParameters(req *osincli.AuthorizeRequest) {
49
+}
50
+
51
+func (p provider) GetUserInfo(data *osincli.AccessData) (api.UserInfo, bool, error) {
52
+
53
+	req, _ := http.NewRequest("GET", githubUserApiUrl, nil)
54
+	req.Header.Set("Authorization", fmt.Sprintf("bearer %s", data.AccessToken))
55
+
56
+	res, err := http.DefaultClient.Do(req)
57
+	if err != nil {
58
+		return nil, false, err
59
+	}
60
+
61
+	body, err := ioutil.ReadAll(res.Body)
62
+	if err != nil {
63
+		return nil, false, err
64
+	}
65
+
66
+	userdata := githubUser{}
67
+	err = json.Unmarshal(body, &userdata)
68
+	if err != nil {
69
+		return nil, false, err
70
+	}
71
+
72
+	if userdata.ID == 0 {
73
+		return nil, false, fmt.Errorf("Could not retrieve GitHub id")
74
+	}
75
+
76
+	user := &api.DefaultUserInfo{
77
+		Name: fmt.Sprintf("%d", userdata.ID),
78
+		Extra: map[string]string{
79
+			"name":  userdata.Name,
80
+			"login": userdata.Login,
81
+			"email": userdata.Email,
82
+		},
83
+	}
84
+	return user, true, nil
85
+}
0 86
new file mode 100644
... ...
@@ -0,0 +1,99 @@
0
+package google
1
+
2
+import (
3
+	"encoding/base64"
4
+	"encoding/json"
5
+	"fmt"
6
+	"strings"
7
+
8
+	"github.com/RangelReale/osincli"
9
+	"github.com/golang/glog"
10
+	"github.com/openshift/origin/pkg/auth/api"
11
+	"github.com/openshift/origin/pkg/auth/oauth/external"
12
+)
13
+
14
+const (
15
+	googleAuthorizeUrl = "https://accounts.google.com/o/oauth2/auth"
16
+	googleTokenUrl     = "https://accounts.google.com/o/oauth2/token"
17
+	googleOauthScope   = "profile email"
18
+)
19
+
20
+type provider struct {
21
+	client_id, client_secret string
22
+}
23
+
24
+func NewProvider(client_id, client_secret string) external.Provider {
25
+	return provider{client_id, client_secret}
26
+}
27
+
28
+func (p provider) NewConfig() (*osincli.ClientConfig, error) {
29
+	config := &osincli.ClientConfig{
30
+		ClientId:                 p.client_id,
31
+		ClientSecret:             p.client_secret,
32
+		ErrorsInStatusCode:       true,
33
+		SendClientSecretInParams: true,
34
+		AuthorizeUrl:             googleAuthorizeUrl,
35
+		TokenUrl:                 googleTokenUrl,
36
+		Scope:                    googleOauthScope,
37
+	}
38
+	return config, nil
39
+}
40
+
41
+func (p provider) AddCustomParameters(req *osincli.AuthorizeRequest) {
42
+	req.CustomParameters["include_granted_scopes"] = "true"
43
+	req.CustomParameters["access_type"] = "offline"
44
+}
45
+
46
+func (p provider) GetUserInfo(data *osincli.AccessData) (api.UserInfo, bool, error) {
47
+	idToken, ok := data.ResponseData["id_token"].(string)
48
+	if !ok {
49
+		return nil, false, fmt.Errorf("No id_token returned in %v", data.ResponseData)
50
+	}
51
+
52
+	userdata, err := decodeJWT(idToken)
53
+	if err != nil {
54
+		return nil, false, err
55
+	}
56
+
57
+	id, _ := userdata["id"].(string)
58
+	email, _ := userdata["email"].(string)
59
+	if id == "" || email == "" {
60
+		return nil, false, fmt.Errorf("Could not retrieve Google id")
61
+	}
62
+
63
+	user := &api.DefaultUserInfo{
64
+		Name: id,
65
+		Extra: map[string]string{
66
+			"name":  email,
67
+			"email": email,
68
+		},
69
+	}
70
+	return user, true, nil
71
+}
72
+
73
+// Decode JWT
74
+// http://openid.net/specs/draft-jones-json-web-token-07.html
75
+func decodeJWT(jwt string) (map[string]interface{}, error) {
76
+	jwtParts := strings.Split(jwt, ".")
77
+	if len(jwtParts) != 3 {
78
+		return nil, fmt.Errorf("Invalid JSON Web Token: expected 3 parts, got %d", len(jwtParts))
79
+	}
80
+
81
+	encodedPayload := jwtParts[1]
82
+	glog.Infof("encodedPayload: %s\n", encodedPayload)
83
+
84
+	decodedPayload, err := base64.StdEncoding.DecodeString(encodedPayload)
85
+	if err != nil {
86
+		return nil, fmt.Errorf("Error decoding payload: %v\n", err)
87
+	}
88
+	glog.Infof("decodedPayload: %s\n", decodedPayload)
89
+
90
+	var data map[string]interface{}
91
+	err = json.Unmarshal([]byte(decodedPayload), &data)
92
+	if err != nil {
93
+		return nil, fmt.Errorf("Error parsing token: %v\n", err)
94
+	}
95
+	glog.Infof("data: %#v\n", data)
96
+
97
+	return data, nil
98
+}
0 99
new file mode 100644
... ...
@@ -0,0 +1,173 @@
0
+package external
1
+
2
+import (
3
+	"fmt"
4
+	"net/http"
5
+	"net/url"
6
+
7
+	"github.com/RangelReale/osincli"
8
+	"github.com/golang/glog"
9
+	"github.com/openshift/origin/pkg/auth/api"
10
+	"github.com/openshift/origin/pkg/auth/oauth/handlers"
11
+)
12
+
13
+type Handler struct {
14
+	provider     Provider
15
+	state        State
16
+	clientConfig *osincli.ClientConfig
17
+	client       *osincli.Client
18
+	success      handlers.AuthenticationSuccessHandler
19
+	error        handlers.AuthenticationErrorHandler
20
+}
21
+
22
+func NewHandler(provider Provider, state State, redirectUrl string, success handlers.AuthenticationSuccessHandler, error handlers.AuthenticationErrorHandler) (*Handler, error) {
23
+	clientConfig, err := provider.NewConfig()
24
+	if err != nil {
25
+		return nil, err
26
+	}
27
+
28
+	clientConfig.RedirectUrl = redirectUrl
29
+
30
+	client, err := osincli.NewClient(clientConfig)
31
+	if err != nil {
32
+		return nil, err
33
+	}
34
+
35
+	return &Handler{
36
+		provider:     provider,
37
+		state:        state,
38
+		clientConfig: clientConfig,
39
+		client:       client,
40
+		success:      success,
41
+		error:        error,
42
+	}, nil
43
+}
44
+
45
+// Implements oauth.handlers.AuthenticationHandler
46
+func (h *Handler) AuthenticationNeeded(w http.ResponseWriter, req *http.Request) {
47
+	glog.V(4).Infof("Authentication needed for %v", h)
48
+
49
+	authReq := h.client.NewAuthorizeRequest(osincli.CODE)
50
+	h.provider.AddCustomParameters(authReq)
51
+
52
+	state, err := h.state.Generate(w, req)
53
+	if err != nil {
54
+		glog.V(4).Infof("Error generating state: %v", err)
55
+		h.AuthenticationError(err, w, req)
56
+		return
57
+	}
58
+
59
+	oauthUrl := authReq.GetAuthorizeUrlWithParams(state)
60
+	glog.V(4).Infof("redirect to %v", oauthUrl)
61
+
62
+	http.Redirect(w, req, oauthUrl.String(), http.StatusFound)
63
+}
64
+
65
+// Implements oauth.handlers.AuthenticationHandler
66
+func (h *Handler) AuthenticationError(err error, w http.ResponseWriter, req *http.Request) {
67
+	h.error.AuthenticationError(err, w, req)
68
+}
69
+
70
+// Handles the callback request in response to an external oauth flow
71
+func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
72
+
73
+	// Extract auth code
74
+	authReq := h.client.NewAuthorizeRequest(osincli.CODE)
75
+	authData, err := authReq.HandleRequest(req)
76
+	if err != nil {
77
+		glog.V(4).Infof("Error handling request: %v", err)
78
+		h.AuthenticationError(err, w, req)
79
+		return
80
+	}
81
+
82
+	glog.V(4).Infof("Got auth data")
83
+
84
+	// Exchange code for a token
85
+	accessReq := h.client.NewAccessRequest(osincli.AUTHORIZATION_CODE, authData)
86
+	accessData, err := accessReq.GetToken()
87
+	if err != nil {
88
+		glog.V(4).Infof("Error getting access token:", err)
89
+		h.AuthenticationError(err, w, req)
90
+		return
91
+	}
92
+
93
+	glog.V(4).Infof("Got access data")
94
+
95
+	user, ok, err := h.provider.GetUserInfo(accessData)
96
+	if err != nil {
97
+		glog.V(4).Infof("Error getting user info: %v", err)
98
+		h.AuthenticationError(err, w, req)
99
+		return
100
+	}
101
+	if !ok || user == nil {
102
+		glog.V(4).Infof("Could not get user info from access token")
103
+		h.AuthenticationError(fmt.Errorf("Could not get user info from access token"), w, req)
104
+		return
105
+	}
106
+
107
+	glog.V(4).Infof("Got user data: %#v", user)
108
+
109
+	ok, err = h.state.Check(authData.State, w, req)
110
+	if !ok {
111
+		glog.V(4).Infof("State is invalid")
112
+		h.AuthenticationError(fmt.Errorf("State is invalid"), w, req)
113
+		return
114
+	}
115
+	if err != nil {
116
+		glog.V(4).Infof("Error verifying state: %v", err)
117
+		h.AuthenticationError(err, w, req)
118
+		return
119
+	}
120
+
121
+	err = h.success.AuthenticationSucceeded(user, authData.State, w, req)
122
+	if err != nil {
123
+		glog.V(4).Infof("Error calling success handler: %v", err)
124
+		h.AuthenticationError(err, w, req)
125
+		return
126
+	}
127
+}
128
+
129
+// Provides default state-building, validation, and parsing to contain CSRF and "then" redirection
130
+type defaultState struct{}
131
+
132
+func DefaultState() State {
133
+	return defaultState{}
134
+}
135
+func (defaultState) Generate(w http.ResponseWriter, req *http.Request) (string, error) {
136
+	state := url.Values{
137
+		"csrf": {"..."}, // TODO: get csrf
138
+		"then": {req.URL.String()},
139
+	}
140
+	return state.Encode(), nil
141
+}
142
+func (defaultState) Check(state string, w http.ResponseWriter, req *http.Request) (bool, error) {
143
+	values, err := url.ParseQuery(state)
144
+	if err != nil {
145
+		return false, err
146
+	}
147
+	csrf := values.Get("csrf")
148
+	if csrf != "..." {
149
+		return false, fmt.Errorf("State did not contain valid CSRF token (expected %s, got %s)", "...", csrf)
150
+	}
151
+
152
+	then := values.Get("then")
153
+	if then == "" {
154
+		return false, fmt.Errorf("State did not contain a redirect")
155
+	}
156
+
157
+	return true, nil
158
+}
159
+func (defaultState) AuthenticationSucceeded(user api.UserInfo, state string, w http.ResponseWriter, req *http.Request) error {
160
+	values, err := url.ParseQuery(state)
161
+	if err != nil {
162
+		return err
163
+	}
164
+
165
+	then := values.Get("then")
166
+	if len(then) == 0 {
167
+		return fmt.Errorf("No redirect given")
168
+	}
169
+
170
+	http.Redirect(w, req, then, http.StatusFound)
171
+	return nil
172
+}
0 173
new file mode 100644
... ...
@@ -0,0 +1,27 @@
0
+/*
1
+Package external implements an OAuth flow with an external identity provider
2
+*/
3
+
4
+package external
5
+
6
+import (
7
+	"net/http"
8
+
9
+	"github.com/RangelReale/osincli"
10
+	"github.com/openshift/origin/pkg/auth/api"
11
+)
12
+
13
+// Provider encapsulates the URLs, configuration, any custom authorize request parameters, and
14
+// the method for transforming an access token into an identity, for an external OAuth provider.
15
+type Provider interface {
16
+	NewConfig() (*osincli.ClientConfig, error)
17
+	AddCustomParameters(*osincli.AuthorizeRequest)
18
+	GetUserInfo(*osincli.AccessData) (api.UserInfo, bool, error)
19
+}
20
+
21
+// State handles generating and verifying the state parameter round-tripped to an external OAuth flow.
22
+// Examples: CSRF protection, post authentication redirection
23
+type State interface {
24
+	Generate(w http.ResponseWriter, req *http.Request) (string, error)
25
+	Check(state string, w http.ResponseWriter, req *http.Request) (bool, error)
26
+}
... ...
@@ -1,7 +1,6 @@
1 1
 package handlers
2 2
 
3 3
 import (
4
-	"fmt"
5 4
 	"net/http"
6 5
 
7 6
 	"github.com/RangelReale/osin"
... ...
@@ -107,7 +106,3 @@ func (denyAuthenticator) AuthenticateAssertion(assertionType, data string) (api.
107 107
 func (denyAuthenticator) AuthenticateClient(client api.Client) (api.UserInfo, bool, error) {
108 108
 	return nil, false, nil
109 109
 }
110
-
111
-func (h *AuthorizeAuthenticator) String() string {
112
-	return fmt.Sprintf("AuthorizationAuthenticator{AuthenticationHandler:%v, RequestAuthenticator:%v}", h.handler, h.request)
113
-}
... ...
@@ -42,7 +42,7 @@ func (h *GrantCheck) HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseW
42 42
 		return true
43 43
 	}
44 44
 	if !ok {
45
-		h.handler.GrantNeeded(grant, w, req)
45
+		h.handler.GrantNeeded(ar.Client, user, grant, w, req)
46 46
 		return true
47 47
 	}
48 48
 
... ...
@@ -11,16 +11,12 @@ type AuthenticationHandler interface {
11 11
 	AuthenticationError(err error, w http.ResponseWriter, req *http.Request)
12 12
 }
13 13
 
14
-// AuthenticationSucceeded is called when a user was successfully authenticated
15
-// The user object may not be nil
16
-type AuthenticationSucceeded interface {
17
-	AuthenticationSucceeded(user api.UserInfo, w http.ResponseWriter, req *http.Request) error
14
+type AuthenticationSuccessHandler interface {
15
+	AuthenticationSucceeded(user api.UserInfo, state string, w http.ResponseWriter, req *http.Request) error
18 16
 }
19 17
 
20
-// InvalidateAuthentication is called when an authentication is being invalidated (e.g. session timeout or log out)
21
-// The user parameter may be nil if unknown
22
-type AuthenticationInvalidator interface {
23
-	InvalidateAuthentication(user api.UserInfo, w http.ResponseWriter, req *http.Request) error
18
+type AuthenticationErrorHandler interface {
19
+	AuthenticationError(err error, w http.ResponseWriter, req *http.Request)
24 20
 }
25 21
 
26 22
 type GrantChecker interface {
... ...
@@ -28,6 +24,19 @@ type GrantChecker interface {
28 28
 }
29 29
 
30 30
 type GrantHandler interface {
31
-	GrantNeeded(grant *api.Grant, w http.ResponseWriter, req *http.Request)
31
+	GrantNeeded(client api.Client, user api.UserInfo, grant *api.Grant, w http.ResponseWriter, req *http.Request)
32 32
 	GrantError(err error, w http.ResponseWriter, req *http.Request)
33 33
 }
34
+
35
+// AuthenticationSuccessHandlers combines multiple AuthenticationSuccessHandler objects into a chain.
36
+// On success, each handler is called. If any handler returns an error, the chain is aborted.
37
+type AuthenticationSuccessHandlers []AuthenticationSuccessHandler
38
+
39
+func (all AuthenticationSuccessHandlers) AuthenticationSucceeded(user api.UserInfo, state string, w http.ResponseWriter, req *http.Request) error {
40
+	for _, h := range all {
41
+		if err := h.AuthenticationSucceeded(user, state, w, req); err != nil {
42
+			return err
43
+		}
44
+	}
45
+	return nil
46
+}
... ...
@@ -38,7 +38,7 @@ func (h *testHandlers) AuthenticateRequest(req *http.Request) (api.UserInfo, boo
38 38
 	return h.User, h.Authenticate, h.Err
39 39
 }
40 40
 
41
-func (h *testHandlers) GrantNeeded(grant *api.Grant, w http.ResponseWriter, req *http.Request) {
41
+func (h *testHandlers) GrantNeeded(client api.Client, user api.UserInfo, grant *api.Grant, w http.ResponseWriter, req *http.Request) {
42 42
 	h.GrantNeed = true
43 43
 }
44 44
 
... ...
@@ -37,7 +37,3 @@ func (a *TokenAuthenticator) AuthenticateToken(value string) (api.UserInfo, bool
37 37
 		Scope: scope.Join(token.AuthorizeToken.Scopes),
38 38
 	}, true, nil
39 39
 }
40
-
41
-func (a *TokenAuthenticator) String() string {
42
-	return "EtcdTokenValidator"
43
-}
... ...
@@ -8,11 +8,12 @@ import (
8 8
 
9 9
 	"github.com/openshift/origin/pkg/auth/api"
10 10
 	"github.com/openshift/origin/pkg/auth/authenticator"
11
+	"github.com/openshift/origin/pkg/auth/oauth/handlers"
11 12
 )
12 13
 
13 14
 type RequestAuthenticator interface {
14 15
 	authenticator.Request
15
-	AuthenticationSucceeded(user api.UserInfo, then string, w http.ResponseWriter, req *http.Request)
16
+	handlers.AuthenticationSuccessHandler
16 17
 }
17 18
 
18 19
 type ConfirmFormRenderer interface {
... ...
@@ -26,10 +26,11 @@ func (t *testImplicit) AuthenticateRequest(req *http.Request) (api.UserInfo, boo
26 26
 	return t.User, t.Success, t.Err
27 27
 }
28 28
 
29
-func (t *testImplicit) AuthenticationSucceeded(user api.UserInfo, then string, w http.ResponseWriter, req *http.Request) {
29
+func (t *testImplicit) AuthenticationSucceeded(user api.UserInfo, then string, w http.ResponseWriter, req *http.Request) error {
30 30
 	t.Called = true
31 31
 	t.User = user
32 32
 	t.Then = then
33
+	return nil
33 34
 }
34 35
 
35 36
 func TestImplicit(t *testing.T) {
... ...
@@ -7,13 +7,13 @@ import (
7 7
 
8 8
 	"github.com/golang/glog"
9 9
 
10
-	"github.com/openshift/origin/pkg/auth/api"
11 10
 	"github.com/openshift/origin/pkg/auth/authenticator"
11
+	"github.com/openshift/origin/pkg/auth/oauth/handlers"
12 12
 )
13 13
 
14 14
 type PasswordAuthenticator interface {
15 15
 	authenticator.Password
16
-	AuthenticationSucceeded(context api.UserInfo, then string, w http.ResponseWriter, req *http.Request)
16
+	handlers.AuthenticationSuccessHandler
17 17
 }
18 18
 
19 19
 type LoginFormRenderer interface {
... ...
@@ -41,10 +41,11 @@ func (t *testAuth) AuthenticatePassword(user, password string) (api.UserInfo, bo
41 41
 	return t.User, t.Success, t.Err
42 42
 }
43 43
 
44
-func (t *testAuth) AuthenticationSucceeded(user api.UserInfo, then string, w http.ResponseWriter, req *http.Request) {
44
+func (t *testAuth) AuthenticationSucceeded(user api.UserInfo, then string, w http.ResponseWriter, req *http.Request) error {
45 45
 	t.Called = true
46 46
 	t.User = user
47 47
 	t.Then = then
48
+	return nil
48 49
 }
49 50
 
50 51
 func TestLogin(t *testing.T) {
... ...
@@ -2,7 +2,6 @@ package session
2 2
 
3 3
 import (
4 4
 	"errors"
5
-	"fmt"
6 5
 	"net/http"
7 6
 
8 7
 	"github.com/openshift/origin/pkg/auth/api"
... ...
@@ -44,7 +43,7 @@ func (a *SessionAuthenticator) AuthenticateRequest(req *http.Request) (api.UserI
44 44
 	}, true, nil
45 45
 }
46 46
 
47
-func (a *SessionAuthenticator) AuthenticationSucceeded(user api.UserInfo, w http.ResponseWriter, req *http.Request) error {
47
+func (a *SessionAuthenticator) AuthenticationSucceeded(user api.UserInfo, state string, w http.ResponseWriter, req *http.Request) error {
48 48
 	session, err := a.store.Get(req, a.name)
49 49
 	if err != nil {
50 50
 		return err
... ...
@@ -62,7 +61,3 @@ func (a *SessionAuthenticator) InvalidateAuthentication(context api.UserInfo, w
62 62
 	session.Values()[UserNameKey] = ""
63 63
 	return a.store.Save(w, req)
64 64
 }
65
-
66
-func (a *SessionAuthenticator) String() string {
67
-	return fmt.Sprintf("SessionAuthenticator{name=%v}", a.name)
68
-}
... ...
@@ -14,13 +14,15 @@ import (
14 14
 	"github.com/openshift/origin/pkg/auth/authenticator/bearertoken"
15 15
 	authfile "github.com/openshift/origin/pkg/auth/authenticator/file"
16 16
 	"github.com/openshift/origin/pkg/auth/authenticator/requestheader"
17
+	"github.com/openshift/origin/pkg/auth/oauth/external"
18
+	"github.com/openshift/origin/pkg/auth/oauth/external/github"
19
+	"github.com/openshift/origin/pkg/auth/oauth/external/google"
17 20
 	"github.com/openshift/origin/pkg/auth/oauth/handlers"
18 21
 	"github.com/openshift/origin/pkg/auth/oauth/registry"
19 22
 	authnregistry "github.com/openshift/origin/pkg/auth/oauth/registry"
20 23
 	"github.com/openshift/origin/pkg/auth/server/login"
21 24
 	"github.com/openshift/origin/pkg/auth/server/session"
22 25
 
23
-	"github.com/openshift/origin/pkg/auth/oauth/callbackhandlers/googlecallback"
24 26
 	cmdutil "github.com/openshift/origin/pkg/cmd/util"
25 27
 	oauthetcd "github.com/openshift/origin/pkg/oauth/registry/etcd"
26 28
 	"github.com/openshift/origin/pkg/oauth/server/osinserver"
... ...
@@ -34,18 +36,11 @@ const (
34 34
 )
35 35
 
36 36
 type AuthConfig struct {
37
+	MasterAddr     string
37 38
 	SessionSecrets []string
38 39
 	EtcdHelper     tools.EtcdHelper
39 40
 }
40 41
 
41
-func getGoogleClientId() string {
42
-	return env("ORIGIN_GOOGLE_CLIENT_ID", "")
43
-}
44
-
45
-func getGoogleClientSecret() string {
46
-	return env("ORIGIN_GOOGLE_CLIENT_SECRET", "")
47
-}
48
-
49 42
 // InstallAPI starts an OAuth2 server and registers the supported REST APIs
50 43
 // into the provided mux, then returns an array of strings indicating what
51 44
 // endpoints were started (these are format strings that will expect to be sent
... ...
@@ -54,11 +49,20 @@ func (c *AuthConfig) InstallAPI(mux cmdutil.Mux) []string {
54 54
 	oauthEtcd := oauthetcd.New(c.EtcdHelper)
55 55
 
56 56
 	authRequestHandler := c.getAuthenticationRequestHandler()
57
-	authHandler := c.getAuthenticationHandler()
57
+
58
+	// Check if the authentication handler wants to be told when we authenticated
59
+	success, ok := authRequestHandler.(handlers.AuthenticationSuccessHandler)
60
+	if !ok {
61
+		success = emptySuccess{}
62
+	}
63
+	authHandler := c.getAuthenticationHandler(mux, success, emptyError{})
58 64
 
59 65
 	storage := registrystorage.New(oauthEtcd, oauthEtcd, oauthEtcd, registry.NewUserConversion())
60 66
 	config := osinserver.NewDefaultServerConfig()
61 67
 
68
+	grantChecker := registry.NewClientAuthorizationGrantChecker(oauthEtcd)
69
+	grantHandler := emptyGrant{}
70
+
62 71
 	server := osinserver.New(
63 72
 		config,
64 73
 		storage,
... ...
@@ -68,8 +72,8 @@ func (c *AuthConfig) InstallAPI(mux cmdutil.Mux) []string {
68 68
 				authRequestHandler,
69 69
 			),
70 70
 			handlers.NewGrantCheck(
71
-				registry.NewClientAuthorizationGrantChecker(oauthEtcd),
72
-				emptyGrant{},
71
+				grantChecker,
72
+				grantHandler,
73 73
 			),
74 74
 		},
75 75
 		osinserver.AccessHandlers{
... ...
@@ -77,14 +81,11 @@ func (c *AuthConfig) InstallAPI(mux cmdutil.Mux) []string {
77 77
 		},
78 78
 	)
79 79
 	server.Install(mux, OpenShiftOAuthAPIPrefix)
80
-	glog.Infof("oauth server configured as: %v", server)
81
-
82
-	mux.Handle(OpenShiftOAuthCallbackPrefix+"/google", &googlecallback.OauthCallbackHandler{oauthetcd.New(c.EtcdHelper), getGoogleClientId(), getGoogleClientSecret()})
83
-
84
-	// Check if the authentication handler wants to be told when we authenticated
85
-	successHandler, _ := authRequestHandler.(handlers.AuthenticationSucceeded)
86
-	login := login.NewLogin(emptyCsrf{}, &callbackPasswordAuthenticator{emptyPasswordAuth{}, successHandler}, login.DefaultLoginFormRenderer)
87
-	login.Install(mux, OpenShiftLoginPrefix)
80
+	// glog.Infof("oauth server configured as: %#v", server)
81
+	// glog.Infof("auth handler: %#v", authHandler)
82
+	// glog.Infof("auth request handler: %#v", authRequestHandler)
83
+	// glog.Infof("grant checker: %#v", grantChecker)
84
+	// glog.Infof("grant handler: %#v", grantHandler)
88 85
 
89 86
 	return []string{
90 87
 		fmt.Sprintf("Started OAuth2 API at %%s%s", OpenShiftOAuthAPIPrefix),
... ...
@@ -92,16 +93,38 @@ func (c *AuthConfig) InstallAPI(mux cmdutil.Mux) []string {
92 92
 	}
93 93
 }
94 94
 
95
-func (c *AuthConfig) getAuthenticationHandler() handlers.AuthenticationHandler {
95
+func (c *AuthConfig) getAuthenticationHandler(mux cmdutil.Mux, success handlers.AuthenticationSuccessHandler, error handlers.AuthenticationErrorHandler) handlers.AuthenticationHandler {
96 96
 	// TODO presumeably we'll want either a list of what we've got or a way to describe a registry of these
97 97
 	// hard-coded strings as a stand-in until it gets sorted out
98 98
 	var authHandler handlers.AuthenticationHandler
99 99
 	authHandlerType := env("ORIGIN_AUTH_HANDLER", "empty")
100 100
 	switch authHandlerType {
101
-	case "google":
102
-		authHandler = &googlecallback.GoogleAuthenticationHandler{"http://localhost:8080" + OpenShiftOAuthCallbackPrefix + "/google", getGoogleClientId()}
101
+	case "google", "github":
102
+		callbackPath := OpenShiftOAuthCallbackPrefix + "/" + authHandlerType
103
+
104
+		var oauthProvider external.Provider
105
+		if authHandlerType == "google" {
106
+			oauthProvider = google.NewProvider(env("ORIGIN_GOOGLE_CLIENT_ID", ""), env("ORIGIN_GOOGLE_CLIENT_SECRET", ""))
107
+		} else if authHandlerType == "github" {
108
+			oauthProvider = github.NewProvider(env("ORIGIN_GITHUB_CLIENT_ID", ""), env("ORIGIN_GITHUB_CLIENT_SECRET", ""))
109
+		}
110
+
111
+		state := external.DefaultState()
112
+		success := handlers.AuthenticationSuccessHandlers{success, state.(handlers.AuthenticationSuccessHandler)}
113
+		oauthHandler, err := external.NewHandler(oauthProvider, state, c.MasterAddr+callbackPath, success, error)
114
+		if err != nil {
115
+			glog.Fatalf("unexpected error: %v", err)
116
+		}
117
+
118
+		mux.Handle(callbackPath, oauthHandler)
119
+		authHandler = oauthHandler
103 120
 	case "password":
104 121
 		authHandler = &redirectAuthHandler{RedirectURL: OpenShiftLoginPrefix, ThenParam: "then"}
122
+
123
+		success := handlers.AuthenticationSuccessHandlers{success, redirectSuccessHandler{}}
124
+
125
+		login := login.NewLogin(emptyCsrf{}, &callbackPasswordAuthenticator{emptyPasswordAuth{}, success}, login.DefaultLoginFormRenderer)
126
+		login.Install(mux, OpenShiftLoginPrefix)
105 127
 	case "empty":
106 128
 		authHandler = emptyAuth{}
107 129
 	default:
... ...
@@ -120,7 +143,7 @@ func (c *AuthConfig) getAuthenticationRequestHandler() authenticator.Request {
120 120
 	case "bearer":
121 121
 		tokenAuthenticator, err := GetTokenAuthenticator(c.EtcdHelper)
122 122
 		if err != nil {
123
-			glog.Fatalf("Error creating TokenAutenticator: %v.  The oauth server cannot start!", err)
123
+			glog.Fatalf("Error creating TokenAuthenticator: %v.  The oauth server cannot start!", err)
124 124
 		}
125 125
 		authRequestHandler = bearertoken.New(tokenAuthenticator)
126 126
 	case "requestheader":
... ...
@@ -144,7 +167,7 @@ func GetTokenAuthenticator(etcdHelper tools.EtcdHelper) (authenticator.Token, er
144 144
 	case "file":
145 145
 		return authfile.NewTokenAuthenticator(env("ORIGIN_AUTH_FILE_TOKEN_AUTHENTICATOR_PATH", "authorizedTokens.csv"))
146 146
 	default:
147
-		return nil, errors.New(fmt.Sprintf("No TokenAutenticator found that matches %v.  The oauth server cannot start!", tokenAuthenticatorType))
147
+		return nil, errors.New(fmt.Sprintf("No TokenAuthenticator found that matches %v.  The oauth server cannot start!", tokenAuthenticatorType))
148 148
 	}
149 149
 }
150 150
 
... ...
@@ -192,8 +215,8 @@ func (auth *redirectAuthHandler) String() string {
192 192
 
193 193
 type emptyGrant struct{}
194 194
 
195
-func (emptyGrant) GrantNeeded(grant *api.Grant, w http.ResponseWriter, req *http.Request) {
196
-	fmt.Fprintf(w, "<body>GrantNeeded - not implemented<pre>%#v</pre></body>", grant)
195
+func (emptyGrant) GrantNeeded(client api.Client, user api.UserInfo, grant *api.Grant, w http.ResponseWriter, req *http.Request) {
196
+	fmt.Fprintf(w, "<body>GrantNeeded - not implemented<pre>%#v\n%#v\n%#v</pre></body>", client, user, grant)
197 197
 }
198 198
 
199 199
 func (emptyGrant) GrantError(err error, w http.ResponseWriter, req *http.Request) {
... ...
@@ -228,28 +251,31 @@ func (emptyPasswordAuth) AuthenticatePassword(user, password string) (api.UserIn
228 228
 // Combines password auth, successful login callback, and "then" param redirection
229 229
 //
230 230
 type callbackPasswordAuthenticator struct {
231
-	password authenticator.Password
232
-	success  handlers.AuthenticationSucceeded
231
+	authenticator.Password
232
+	handlers.AuthenticationSuccessHandler
233 233
 }
234 234
 
235
-// for login.PasswordAuthenticator
236
-func (auth *callbackPasswordAuthenticator) AuthenticatePassword(user, password string) (api.UserInfo, bool, error) {
237
-	return auth.password.AuthenticatePassword(user, password)
238
-}
235
+// Redirects to the then param on successful authentication
236
+type redirectSuccessHandler struct{}
239 237
 
240
-// for login.PasswordAuthenticator
241
-func (auth *callbackPasswordAuthenticator) AuthenticationSucceeded(user api.UserInfo, then string, w http.ResponseWriter, req *http.Request) {
242
-	if auth.success != nil {
243
-		err := auth.success.AuthenticationSucceeded(user, w, req)
244
-		if err != nil {
245
-			fmt.Fprintf(w, "<body>Could not save session, err=%#v</body>", err)
246
-			return
247
-		}
238
+func (redirectSuccessHandler) AuthenticationSucceeded(user api.UserInfo, then string, w http.ResponseWriter, req *http.Request) error {
239
+	if len(then) == 0 {
240
+		return fmt.Errorf("Auth succeeded, but no redirect existed - user=%#v", user)
248 241
 	}
249 242
 
250
-	if len(then) != 0 {
251
-		http.Redirect(w, req, then, http.StatusFound)
252
-	} else {
253
-		fmt.Fprintf(w, "<body>PasswordAuthenticationSucceeded - user=%#v</body>", user)
254
-	}
243
+	http.Redirect(w, req, then, http.StatusFound)
244
+	return nil
245
+}
246
+
247
+type emptySuccess struct{}
248
+
249
+func (emptySuccess) AuthenticationSucceeded(user api.UserInfo, state string, w http.ResponseWriter, req *http.Request) error {
250
+	glog.V(4).Infof("AuthenticationSucceeded: %v (state=%s)", user, state)
251
+	return nil
252
+}
253
+
254
+type emptyError struct{}
255
+
256
+func (emptyError) AuthenticationError(err error, w http.ResponseWriter, req *http.Request) {
257
+	glog.V(4).Infof("AuthenticationError: %v", err)
255 258
 }
... ...
@@ -216,17 +216,21 @@ func (c *MasterConfig) wrapHandlerWithAuthentication(handler http.Handler) http.
216 216
 	requestsToUsers := authcontext.NewRequestContextMap() // this tracks requests back to users for authorization
217 217
 	tokenAuthenticator, err := GetTokenAuthenticator(c.EtcdHelper)
218 218
 	if err != nil {
219
-		glog.Fatalf("Error creating TokenAutenticator: %v.  The oauth server cannot start!", err)
219
+		glog.Fatalf("Error creating TokenAuthenticator: %v.  The oauth server cannot start!", err)
220 220
 	}
221
-	handler = authfilter.NewRequestAuthenticator(
221
+	return authfilter.NewRequestAuthenticator(
222 222
 		requestsToUsers,
223 223
 		bearertoken.New(tokenAuthenticator),
224
-		http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { // failure handler
225
-			w.WriteHeader(http.StatusUnauthorized) // just say you're unauthorized
224
+		http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
225
+			// TODO: make this failure handler actually fail once internal components can get auth tokens to do their job
226
+			// w.WriteHeader(http.StatusUnauthorized)
227
+			// return
228
+
229
+			// For now, just let us know and continue on your merry way
230
+			glog.V(2).Infof("Token authentication failed when accessing: %v", req.URL)
231
+			handler.ServeHTTP(w, req)
226 232
 		}),
227
-		handler) // success handler
228
-
229
-	return handler
233
+		handler)
230 234
 }
231 235
 
232 236
 // RunAssetServer starts the asset server for the OpenShift UI.
... ...
@@ -190,6 +190,7 @@ func NewCommandStartServer(name string) *cobra.Command {
190 190
 				osmaster.EnsureCORSAllowedOrigins(cfg.CORSAllowedOrigins)
191 191
 
192 192
 				auth := &origin.AuthConfig{
193
+					MasterAddr:     cfg.MasterAddr.URL.String(),
193 194
 					SessionSecrets: []string{"secret"},
194 195
 					EtcdHelper:     etcdHelper,
195 196
 				}
... ...
@@ -1,7 +1,6 @@
1 1
 package osinserver
2 2
 
3 3
 import (
4
-	"fmt"
5 4
 	"net/http"
6 5
 	"strings"
7 6
 
... ...
@@ -90,6 +89,6 @@ func (s *Server) handleInfo(w http.ResponseWriter, r *http.Request) {
90 90
 	osin.OutputJSON(resp, w, r)
91 91
 }
92 92
 
93
-func (s *Server) String() string {
94
-	return fmt.Sprintf("OsinServer{AuthorizeHandler:%v, AccessHandler:%v}", s.authorize, s.access)
95
-}
93
+// func (s *Server) String() string {
94
+// 	return fmt.Sprintf("osinserver.Server{config:%#v, authorize:%v, access:%v}", s.config, s.authorize, s.access)
95
+// }