Browse code

allow configuration of the SA oauth client grant flows

deads2k authored on 2016/05/19 23:58:50
Showing 9 changed files
... ...
@@ -7,8 +7,10 @@ import (
7 7
 
8 8
 	"github.com/RangelReale/osin"
9 9
 
10
-	"github.com/openshift/origin/pkg/auth/api"
11 10
 	"k8s.io/kubernetes/pkg/auth/user"
11
+	"k8s.io/kubernetes/pkg/serviceaccount"
12
+
13
+	"github.com/openshift/origin/pkg/auth/api"
12 14
 )
13 15
 
14 16
 // GrantCheck implements osinserver.AuthorizeHandler to ensure requested scopes have been authorized
... ...
@@ -30,6 +32,7 @@ func NewGrantCheck(check GrantChecker, handler GrantHandler, errorHandler GrantE
30 30
 // If the response is written, true is returned.
31 31
 // If the response is not written, false is returned.
32 32
 func (h *GrantCheck) HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
33
+
33 34
 	// Requests must already be authorized before we will check grants
34 35
 	if !ar.Authorized {
35 36
 		return false, nil
... ...
@@ -122,3 +125,24 @@ func (g *redirectGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.Res
122 122
 	http.Redirect(w, req, redirectURL.String(), http.StatusFound)
123 123
 	return false, true, nil
124 124
 }
125
+
126
+type serviceAccountAwareGrant struct {
127
+	standardGrantHandler GrantHandler
128
+	// saClientGrantHandler allows an autogrant handler to do something else when the client is a service account.
129
+	// TODO: I think this can be removed once we can set granthandler overrides per-client, but we need something for safety now.
130
+	saClientGrantHandler GrantHandler
131
+}
132
+
133
+// NewAutoGrant returns a grant handler that automatically approves client authorizations
134
+func NewServiceAccountAwareGrant(standardGrantHandler, saClientGrantHandler GrantHandler) GrantHandler {
135
+	return &serviceAccountAwareGrant{standardGrantHandler: standardGrantHandler, saClientGrantHandler: saClientGrantHandler}
136
+}
137
+
138
+// GrantNeeded implements the GrantHandler interface
139
+func (g *serviceAccountAwareGrant) GrantNeeded(user user.Info, grant *api.Grant, w http.ResponseWriter, req *http.Request) (bool, bool, error) {
140
+	if _, _, err := serviceaccount.SplitUsername(grant.Client.GetId()); err == nil {
141
+		return g.saClientGrantHandler.GrantNeeded(user, grant, w, req)
142
+	}
143
+
144
+	return g.standardGrantHandler.GrantNeeded(user, grant, w, req)
145
+}
... ...
@@ -889,6 +889,10 @@ type OpenIDClaims struct {
889 889
 type GrantConfig struct {
890 890
 	// Method: allow, deny, prompt
891 891
 	Method GrantHandlerType
892
+
893
+	// ServiceAccountMethod is used for determining client authorization for service account oauth client.
894
+	// It must be either: deny, prompt
895
+	ServiceAccountMethod GrantHandlerType
892 896
 }
893 897
 
894 898
 type GrantHandlerType string
... ...
@@ -903,6 +907,7 @@ const (
903 903
 )
904 904
 
905 905
 var ValidGrantHandlerTypes = sets.NewString(string(GrantHandlerAuto), string(GrantHandlerPrompt), string(GrantHandlerDeny))
906
+var ValidServiceAccountGrantHandlerTypes = sets.NewString(string(GrantHandlerPrompt), string(GrantHandlerDeny))
906 907
 
907 908
 type EtcdConfig struct {
908 909
 	// ServingInfo describes how to start serving the etcd master
... ...
@@ -164,6 +164,11 @@ func addDefaultingFuncs(scheme *runtime.Scheme) {
164 164
 				obj.MappingMethod = "claim"
165 165
 			}
166 166
 		},
167
+		func(obj *GrantConfig) {
168
+			if len(obj.ServiceAccountMethod) == 0 {
169
+				obj.ServiceAccountMethod = "prompt"
170
+			}
171
+		},
167 172
 	)
168 173
 	if err != nil {
169 174
 		// If one of the conversion functions is malformed, detect it immediately.
... ...
@@ -221,8 +221,9 @@ func (GoogleIdentityProvider) SwaggerDoc() map[string]string {
221 221
 }
222 222
 
223 223
 var map_GrantConfig = map[string]string{
224
-	"":       "GrantConfig holds the necessary configuration options for grant handlers",
225
-	"method": "Method: allow, deny, prompt",
224
+	"":                     "GrantConfig holds the necessary configuration options for grant handlers",
225
+	"method":               "Method: allow, deny, prompt",
226
+	"serviceAccountMethod": "ServiceAccountMethod is used for determining client authorization for service account oauth client. It must be either: deny, prompt",
226 227
 }
227 228
 
228 229
 func (GrantConfig) SwaggerDoc() map[string]string {
... ...
@@ -887,6 +887,10 @@ type OpenIDClaims struct {
887 887
 type GrantConfig struct {
888 888
 	// Method: allow, deny, prompt
889 889
 	Method GrantHandlerType `json:"method"`
890
+
891
+	// ServiceAccountMethod is used for determining client authorization for service account oauth client.
892
+	// It must be either: deny, prompt
893
+	ServiceAccountMethod GrantHandlerType `json:"serviceAccountMethod"`
890 894
 }
891 895
 
892 896
 type GrantHandlerType string
... ...
@@ -203,6 +203,7 @@ oauthConfig:
203 203
   assetPublicURL: ""
204 204
   grantConfig:
205 205
     method: ""
206
+    serviceAccountMethod: ""
206 207
   identityProviders:
207 208
   - challenge: false
208 209
     login: false
... ...
@@ -387,7 +387,10 @@ func validateGrantConfig(config api.GrantConfig, fldPath *field.Path) field.Erro
387 387
 	allErrs := field.ErrorList{}
388 388
 
389 389
 	if !api.ValidGrantHandlerTypes.Has(string(config.Method)) {
390
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("method"), config.Method, fmt.Sprintf("must be one of: %v", api.ValidGrantHandlerTypes.List())))
390
+		allErrs = append(allErrs, field.NotSupported(fldPath.Child("method"), config.Method, api.ValidGrantHandlerTypes.List()))
391
+	}
392
+	if !api.ValidServiceAccountGrantHandlerTypes.Has(string(config.ServiceAccountMethod)) {
393
+		allErrs = append(allErrs, field.NotSupported(fldPath.Child("serviceAccountMethod"), config.ServiceAccountMethod, api.ValidServiceAccountGrantHandlerTypes.List()))
391 394
 	}
392 395
 
393 396
 	return allErrs
... ...
@@ -333,23 +333,38 @@ func (c *AuthConfig) getAuthorizeAuthenticationHandlers(mux cmdutil.Mux, errorHa
333 333
 
334 334
 // getGrantHandler returns the object that handles approving or rejecting grant requests
335 335
 func (c *AuthConfig) getGrantHandler(mux cmdutil.Mux, auth authenticator.Request, clientregistry clientregistry.Getter, authregistry clientauthregistry.Registry) handlers.GrantHandler {
336
-	switch c.Options.GrantConfig.Method {
336
+	startGrantServer := false
337
+
338
+	var saGrantHandler handlers.GrantHandler
339
+	switch c.Options.GrantConfig.ServiceAccountMethod {
337 340
 	case configapi.GrantHandlerDeny:
338
-		return handlers.NewEmptyGrant()
341
+		saGrantHandler = handlers.NewEmptyGrant()
342
+	case configapi.GrantHandlerPrompt:
343
+		startGrantServer = true
344
+		saGrantHandler = handlers.NewRedirectGrant(OpenShiftApprovePrefix)
345
+	default:
346
+		glog.Fatalf("No grant handler found that matches %v.  The oauth server cannot start!", c.Options.GrantConfig.ServiceAccountMethod)
347
+	}
339 348
 
349
+	var standardGrantHandler handlers.GrantHandler
350
+	switch c.Options.GrantConfig.Method {
351
+	case configapi.GrantHandlerDeny:
352
+		standardGrantHandler = handlers.NewEmptyGrant()
340 353
 	case configapi.GrantHandlerAuto:
341
-		return handlers.NewAutoGrant()
342
-
354
+		standardGrantHandler = handlers.NewAutoGrant()
343 355
 	case configapi.GrantHandlerPrompt:
344
-		grantServer := grant.NewGrant(c.getCSRF(), auth, grant.DefaultFormRenderer, clientregistry, authregistry)
345
-		grantServer.Install(mux, OpenShiftApprovePrefix)
346
-		return handlers.NewRedirectGrant(OpenShiftApprovePrefix)
347
-
356
+		startGrantServer = true
357
+		standardGrantHandler = handlers.NewRedirectGrant(OpenShiftApprovePrefix)
348 358
 	default:
349 359
 		glog.Fatalf("No grant handler found that matches %v.  The oauth server cannot start!", c.Options.GrantConfig.Method)
350 360
 	}
351 361
 
352
-	return nil
362
+	if startGrantServer {
363
+		grantServer := grant.NewGrant(c.getCSRF(), auth, grant.DefaultFormRenderer, clientregistry, authregistry)
364
+		grantServer.Install(mux, OpenShiftApprovePrefix)
365
+	}
366
+
367
+	return handlers.NewServiceAccountAwareGrant(standardGrantHandler, saGrantHandler)
353 368
 }
354 369
 
355 370
 // getAuthenticationFinalizer returns an authentication finalizer which is called just prior to writing a response to an authorization request
... ...
@@ -5,9 +5,13 @@ package integration
5 5
 import (
6 6
 	"crypto/tls"
7 7
 	"encoding/base64"
8
+	"io/ioutil"
8 9
 	"net/http"
9 10
 	"net/http/httptest"
10 11
 	"net/http/httputil"
12
+	"net/url"
13
+	"regexp"
14
+	"strings"
11 15
 	"testing"
12 16
 	"time"
13 17
 
... ...
@@ -19,6 +23,7 @@ import (
19 19
 	"k8s.io/kubernetes/pkg/serviceaccount"
20 20
 
21 21
 	"github.com/openshift/origin/pkg/client"
22
+	"github.com/openshift/origin/pkg/cmd/server/origin"
22 23
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
23 24
 	"github.com/openshift/origin/pkg/oauth/scope"
24 25
 	saoauth "github.com/openshift/origin/pkg/serviceaccounts/oauthclient"
... ...
@@ -109,7 +114,8 @@ func TestSAAsOAuthClient(t *testing.T) {
109 109
 		Scope:        scope.Join([]string{"user:info", "role:edit:" + projectName}),
110 110
 		SendClientSecretInParams: true,
111 111
 	}
112
-	runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, false, true)
112
+	runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true)
113
+	clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
113 114
 
114 115
 	oauthClientConfig = &osincli.ClientConfig{
115 116
 		ClientId:     serviceaccount.MakeUsername(defaultSA.Namespace, defaultSA.Name),
... ...
@@ -120,7 +126,8 @@ func TestSAAsOAuthClient(t *testing.T) {
120 120
 		Scope:        scope.Join([]string{"user:info", "role:edit:other-ns"}),
121 121
 		SendClientSecretInParams: true,
122 122
 	}
123
-	runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false)
123
+	runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, false, false)
124
+	clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
124 125
 
125 126
 	oauthClientConfig = &osincli.ClientConfig{
126 127
 		ClientId:     serviceaccount.MakeUsername(defaultSA.Namespace, defaultSA.Name),
... ...
@@ -131,11 +138,14 @@ func TestSAAsOAuthClient(t *testing.T) {
131 131
 		Scope:        scope.Join([]string{"user:info"}),
132 132
 		SendClientSecretInParams: true,
133 133
 	}
134
-	runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, false, false)
135
-
134
+	runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false)
135
+	clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
136 136
 }
137 137
 
138
-func runOAuthFlow(t *testing.T, clusterAdminClientConfig *restclient.Config, projectName string, oauthClientConfig *osincli.ClientConfig, authorizationCodes, authorizationErrors chan string, expectAuthCodeError, expectBuildSuccess bool) {
138
+var grantCSRFRegex = regexp.MustCompile(`input type="hidden" name="csrf" value="([^"]*)"`)
139
+var grantThenRegex = regexp.MustCompile(`input type="hidden" name="then" value="([^"]*)"`)
140
+
141
+func runOAuthFlow(t *testing.T, clusterAdminClientConfig *restclient.Config, projectName string, oauthClientConfig *osincli.ClientConfig, authorizationCodes, authorizationErrors chan string, expectGrantSuccess, expectBuildSuccess bool) {
139 142
 	oauthRuntimeClient, err := osincli.NewClient(oauthClientConfig)
140 143
 	if err != nil {
141 144
 		t.Fatalf("unexpected error: %v", err)
... ...
@@ -171,10 +181,99 @@ func runOAuthFlow(t *testing.T, clusterAdminClientConfig *restclient.Config, pro
171 171
 		t.Fatalf("didn't get an unauthorized:\n %v", string(response))
172 172
 	}
173 173
 
174
-	authenticatedAuthorizeHTTPRequest, err := http.NewRequest("GET", authorizeURL.String(), nil)
175
-	authenticatedAuthorizeHTTPRequest.Header.Add("X-CSRF-Token", "csrf-01")
176
-	authenticatedAuthorizeHTTPRequest.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
177
-	authentictedAuthorizeResponse, err := directHTTPClient.Do(authenticatedAuthorizeHTTPRequest)
174
+	// first we should get a redirect to a grant flow
175
+	authenticatedAuthorizeHTTPRequest1, err := http.NewRequest("GET", authorizeURL.String(), nil)
176
+	authenticatedAuthorizeHTTPRequest1.Header.Add("X-CSRF-Token", "csrf-01")
177
+	authenticatedAuthorizeHTTPRequest1.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
178
+	authentictedAuthorizeResponse1, err := directHTTPClient.Transport.RoundTrip(authenticatedAuthorizeHTTPRequest1)
179
+	if err != nil {
180
+		t.Fatalf("unexpected error: %v", err)
181
+	}
182
+	if authentictedAuthorizeResponse1.StatusCode != http.StatusFound {
183
+		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse1, true)
184
+		t.Fatalf("unexpected status :\n %v", string(response))
185
+	}
186
+
187
+	// second we get a webpage with a prompt.  Yeah, this next bit gets nasty
188
+	authenticatedAuthorizeHTTPRequest2, err := http.NewRequest("GET", clusterAdminClientConfig.Host+authentictedAuthorizeResponse1.Header.Get("Location"), nil)
189
+	authenticatedAuthorizeHTTPRequest2.Header.Add("X-CSRF-Token", "csrf-01")
190
+	authenticatedAuthorizeHTTPRequest2.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
191
+	authentictedAuthorizeResponse2, err := directHTTPClient.Transport.RoundTrip(authenticatedAuthorizeHTTPRequest2)
192
+	if err != nil {
193
+		t.Fatalf("unexpected error: %v", err)
194
+	}
195
+	if authentictedAuthorizeResponse2.StatusCode != http.StatusOK {
196
+		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse2, true)
197
+		t.Fatalf("unexpected status :\n %v", string(response))
198
+	}
199
+	// have to parse the page to get the csrf value.  Yeah, that's nasty, I can't think of another way to do it without creating a new grant handler
200
+	body, err := ioutil.ReadAll(authentictedAuthorizeResponse2.Body)
201
+	if err != nil {
202
+		t.Fatalf("unexpected error: %v", err)
203
+	}
204
+	if !expectGrantSuccess {
205
+		if !strings.Contains(string(body), "requested illegal scopes") {
206
+			t.Fatalf("missing expected message: %v", string(body))
207
+		}
208
+		return
209
+	}
210
+	csrfMatches := grantCSRFRegex.FindStringSubmatch(string(body))
211
+	if len(csrfMatches) != 2 {
212
+		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse2, false)
213
+		t.Fatalf("unexpected body :\n %v\n%v", string(response), string(body))
214
+	}
215
+	thenMatches := grantThenRegex.FindStringSubmatch(string(body))
216
+	if len(thenMatches) != 2 {
217
+		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse2, false)
218
+		t.Fatalf("unexpected body :\n %v\n%v", string(response), string(body))
219
+	}
220
+	t.Logf("CSRF is %v", csrfMatches)
221
+	t.Logf("then is %v", thenMatches)
222
+
223
+	// third we respond and approve the grant, then let the transport follow redirects and give us the code
224
+	postBody := strings.NewReader(url.Values(map[string][]string{
225
+		"then":         {thenMatches[1]},
226
+		"csrf":         {csrfMatches[1]},
227
+		"client_id":    {oauthClientConfig.ClientId},
228
+		"user_name":    {"harold"},
229
+		"scopes":       {oauthClientConfig.Scope},
230
+		"redirect_uri": {clusterAdminClientConfig.Host},
231
+		"approve":      {"true"},
232
+	}).Encode())
233
+	authenticatedAuthorizeHTTPRequest3, err := http.NewRequest("POST", clusterAdminClientConfig.Host+origin.OpenShiftApprovePrefix, postBody)
234
+	authenticatedAuthorizeHTTPRequest3.Header.Set("Content-Type", "application/x-www-form-urlencoded")
235
+	authenticatedAuthorizeHTTPRequest3.Header.Add("X-CSRF-Token", csrfMatches[1])
236
+	authenticatedAuthorizeHTTPRequest3.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
237
+	for i := range authentictedAuthorizeResponse2.Cookies() {
238
+		cookie := authentictedAuthorizeResponse2.Cookies()[i]
239
+		authenticatedAuthorizeHTTPRequest3.AddCookie(cookie)
240
+	}
241
+	authentictedAuthorizeResponse3, err := directHTTPClient.Transport.RoundTrip(authenticatedAuthorizeHTTPRequest3)
242
+	if err != nil {
243
+		t.Fatalf("unexpected error: %v", err)
244
+	}
245
+	if authentictedAuthorizeResponse3.StatusCode != http.StatusFound {
246
+		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse3, true)
247
+		t.Fatalf("unexpected status :\n %v", string(response))
248
+	}
249
+
250
+	// fourth, the grant redirects us again to have us send the code to the server
251
+	authenticatedAuthorizeHTTPRequest4, err := http.NewRequest("GET", clusterAdminClientConfig.Host+authentictedAuthorizeResponse3.Header.Get("Location"), nil)
252
+	authenticatedAuthorizeHTTPRequest4.Header.Add("X-CSRF-Token", "csrf-01")
253
+	authenticatedAuthorizeHTTPRequest4.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
254
+	authentictedAuthorizeResponse4, err := directHTTPClient.Transport.RoundTrip(authenticatedAuthorizeHTTPRequest4)
255
+	if err != nil {
256
+		t.Fatalf("unexpected error: %v", err)
257
+	}
258
+	if authentictedAuthorizeResponse4.StatusCode != http.StatusFound {
259
+		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse4, true)
260
+		t.Fatalf("unexpected status :\n %v", string(response))
261
+	}
262
+
263
+	authenticatedAuthorizeHTTPRequest5, err := http.NewRequest("GET", authentictedAuthorizeResponse4.Header.Get("Location"), nil)
264
+	authenticatedAuthorizeHTTPRequest5.Header.Add("X-CSRF-Token", "csrf-01")
265
+	authenticatedAuthorizeHTTPRequest5.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
266
+	authentictedAuthorizeResponse5, err := directHTTPClient.Do(authenticatedAuthorizeHTTPRequest5)
178 267
 	if err != nil {
179 268
 		t.Fatalf("unexpected error: %v", err)
180 269
 	}
... ...
@@ -182,18 +281,8 @@ func runOAuthFlow(t *testing.T, clusterAdminClientConfig *restclient.Config, pro
182 182
 	authorizationCode := ""
183 183
 	select {
184 184
 	case authorizationCode = <-authorizationCodes:
185
-		if expectAuthCodeError {
186
-			response, _ := httputil.DumpResponse(authentictedAuthorizeResponse, true)
187
-			t.Fatalf("got a code:\n %v", string(response))
188
-		}
189
-	case <-authorizationErrors:
190
-		if !expectAuthCodeError {
191
-			response, _ := httputil.DumpResponse(authentictedAuthorizeResponse, true)
192
-			t.Fatalf("unexpected error:\n %v", string(response))
193
-		}
194
-		return
195 185
 	case <-time.After(10 * time.Second):
196
-		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse, true)
186
+		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse5, true)
197 187
 		t.Fatalf("didn't get a code:\n %v", string(response))
198 188
 	}
199 189