Browse code

Add scope representing full user permissions, default token requests with unspecified scope

Jordan Liggitt authored on 2016/07/28 03:31:58
Showing 20 changed files
... ...
@@ -33,6 +33,7 @@ import (
33 33
 	image "github.com/openshift/origin/pkg/image/api"
34 34
 	"github.com/openshift/origin/pkg/image/api/docker10"
35 35
 	"github.com/openshift/origin/pkg/image/api/dockerpre012"
36
+	oauthapi "github.com/openshift/origin/pkg/oauth/api"
36 37
 	quotaapi "github.com/openshift/origin/pkg/quota/api"
37 38
 	quotaapiv1 "github.com/openshift/origin/pkg/quota/api/v1"
38 39
 	route "github.com/openshift/origin/pkg/route/api"
... ...
@@ -417,6 +418,12 @@ func fuzzInternalObject(t *testing.T, forVersion unversioned.GroupVersion, item
417 417
 			j.Spec.Template.Spec.InitContainers = nil
418 418
 			j.Status.Template.Spec.InitContainers = nil
419 419
 		},
420
+		func(j *oauthapi.OAuthClientAuthorization, c fuzz.Continue) {
421
+			c.FuzzNoCustom(j)
422
+			if len(j.Scopes) == 0 {
423
+				j.Scopes = append(j.Scopes, "user:full")
424
+			}
425
+		},
420 426
 
421 427
 		func(j *runtime.Object, c fuzz.Continue) {
422 428
 			// runtime.EmbeddedObject causes a panic inside of fuzz because runtime.Object isn't handled.
... ...
@@ -26,7 +26,7 @@ func NewAuthorizeAuthenticator(request authenticator.Request, handler Authentica
26 26
 // HandleAuthorize implements osinserver.AuthorizeHandler to ensure the AuthorizeRequest is authenticated.
27 27
 // If the request is authenticated, UserData and Authorized are set and false is returned.
28 28
 // If the request is not authenticated, the auth handler is called and the request is not authorized
29
-func (h *AuthorizeAuthenticator) HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
29
+func (h *AuthorizeAuthenticator) HandleAuthorize(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error) {
30 30
 	info, ok, err := h.request.AuthenticateRequest(ar.HttpRequest)
31 31
 	if err != nil {
32 32
 		glog.V(4).Infof("OAuth authentication error: %v", err)
... ...
@@ -5,13 +5,20 @@ import (
5 5
 	"fmt"
6 6
 	"net/http"
7 7
 	"net/url"
8
+	"strings"
8 9
 
9 10
 	"github.com/RangelReale/osin"
10 11
 
11 12
 	"k8s.io/kubernetes/pkg/auth/user"
13
+	utilruntime "k8s.io/kubernetes/pkg/util/runtime"
14
+	"k8s.io/kubernetes/pkg/util/sets"
12 15
 
13 16
 	"github.com/openshift/origin/pkg/auth/api"
17
+	scopeauthorizer "github.com/openshift/origin/pkg/authorization/authorizer/scope"
14 18
 	oauthapi "github.com/openshift/origin/pkg/oauth/api"
19
+	"github.com/openshift/origin/pkg/oauth/api/validation"
20
+	"github.com/openshift/origin/pkg/oauth/scope"
21
+	"github.com/openshift/origin/pkg/oauth/server/osinserver"
15 22
 )
16 23
 
17 24
 // GrantCheck implements osinserver.AuthorizeHandler to ensure requested scopes have been authorized
... ...
@@ -22,7 +29,7 @@ type GrantCheck struct {
22 22
 }
23 23
 
24 24
 // NewGrantCheck returns a new GrantCheck
25
-func NewGrantCheck(check GrantChecker, handler GrantHandler, errorHandler GrantErrorHandler) *GrantCheck {
25
+func NewGrantCheck(check GrantChecker, handler GrantHandler, errorHandler GrantErrorHandler) osinserver.AuthorizeHandler {
26 26
 	return &GrantCheck{check, handler, errorHandler}
27 27
 }
28 28
 
... ...
@@ -32,7 +39,7 @@ func NewGrantCheck(check GrantChecker, handler GrantHandler, errorHandler GrantE
32 32
 // If the requested scopes are not authorized, or an error occurs, AuthorizeRequest.Authorized is set to false.
33 33
 // If the response is written, true is returned.
34 34
 // If the response is not written, false is returned.
35
-func (h *GrantCheck) HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
35
+func (h *GrantCheck) HandleAuthorize(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error) {
36 36
 
37 37
 	// Requests must already be authorized before we will check grants
38 38
 	if !ar.Authorized {
... ...
@@ -44,7 +51,40 @@ func (h *GrantCheck) HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseW
44 44
 
45 45
 	user, ok := ar.UserData.(user.Info)
46 46
 	if !ok || user == nil {
47
-		return h.errorHandler.GrantError(errors.New("the provided user data is not user.Info"), w, ar.HttpRequest)
47
+		utilruntime.HandleError(fmt.Errorf("the provided user data is not a user.Info object: %#v", user))
48
+		resp.SetError("server_error", "")
49
+		return false, nil
50
+	}
51
+
52
+	client, ok := ar.Client.GetUserData().(*oauthapi.OAuthClient)
53
+	if !ok || client == nil {
54
+		utilruntime.HandleError(fmt.Errorf("the provided client is not an *api.OAuthClient object: %#v", client))
55
+		resp.SetError("server_error", "")
56
+		return false, nil
57
+	}
58
+
59
+	// Normalize the scope request, and ensure all tokens contain a scope
60
+	scopes := scope.Split(ar.Scope)
61
+	if len(scopes) == 0 {
62
+		scopes = append(scopes, scopeauthorizer.UserFull)
63
+	}
64
+	ar.Scope = scope.Join(scopes)
65
+
66
+	// Validate the requested scopes
67
+	if scopeErrors := validation.ValidateScopes(scopes, nil); len(scopeErrors) > 0 {
68
+		resp.SetError("invalid_scope", scopeErrors.ToAggregate().Error())
69
+		return false, nil
70
+	}
71
+
72
+	invalidScopes := sets.NewString()
73
+	for _, scope := range scopes {
74
+		if err := scopeauthorizer.ValidateScopeRestrictions(client, scope); err != nil {
75
+			invalidScopes.Insert(scope)
76
+		}
77
+	}
78
+	if len(invalidScopes) > 0 {
79
+		resp.SetError("access_denied", fmt.Sprintf("scope denied: %s", strings.Join(invalidScopes.List(), " ")))
80
+		return false, nil
48 81
 	}
49 82
 
50 83
 	grant := &api.Grant{
... ...
@@ -57,7 +97,9 @@ func (h *GrantCheck) HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseW
57 57
 	// Check if the user has already authorized this grant
58 58
 	authorized, err := h.check.HasAuthorizedClient(user, grant)
59 59
 	if err != nil {
60
-		return h.errorHandler.GrantError(err, w, ar.HttpRequest)
60
+		utilruntime.HandleError(err)
61
+		resp.SetError("server_error", "")
62
+		return false, nil
61 63
 	}
62 64
 	if authorized {
63 65
 		ar.Authorized = true
... ...
@@ -7,6 +7,7 @@ import (
7 7
 	"testing"
8 8
 	"time"
9 9
 
10
+	"github.com/RangelReale/osin"
10 11
 	"github.com/RangelReale/osincli"
11 12
 	kapi "k8s.io/kubernetes/pkg/api"
12 13
 	apierrs "k8s.io/kubernetes/pkg/api/errors"
... ...
@@ -25,6 +26,8 @@ import (
25 25
 )
26 26
 
27 27
 type testHandlers struct {
28
+	AuthorizeHandler osinserver.AuthorizeHandler
29
+
28 30
 	User         user.Info
29 31
 	Authenticate bool
30 32
 	Err          error
... ...
@@ -33,6 +36,11 @@ type testHandlers struct {
33 33
 	GrantNeed    bool
34 34
 	GrantErr     error
35 35
 
36
+	HandleAuthorizeReq     *osin.AuthorizeRequest
37
+	HandleAuthorizeResp    *osin.Response
38
+	HandleAuthorizeHandled bool
39
+	HandleAuthorizeErr     error
40
+
36 41
 	AuthNeedHandled bool
37 42
 	AuthNeedErr     error
38 43
 
... ...
@@ -43,6 +51,13 @@ type testHandlers struct {
43 43
 	HandledErr error
44 44
 }
45 45
 
46
+func (h *testHandlers) HandleAuthorize(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (handled bool, err error) {
47
+	h.HandleAuthorizeReq = ar
48
+	h.HandleAuthorizeResp = resp
49
+	h.HandleAuthorizeHandled, h.HandleAuthorizeErr = h.AuthorizeHandler.HandleAuthorize(ar, resp, w)
50
+	return h.HandleAuthorizeHandled, h.HandleAuthorizeErr
51
+}
52
+
46 53
 func (h *testHandlers) AuthenticationNeeded(client api.Client, w http.ResponseWriter, req *http.Request) (bool, error) {
47 54
 	h.AuthNeed = true
48 55
 	return h.AuthNeedHandled, h.AuthNeedErr
... ...
@@ -82,9 +97,14 @@ func TestRegistryAndServer(t *testing.T) {
82 82
 		Secret:       "secret",
83 83
 		RedirectURIs: []string{assertServer.URL + "/assert"},
84 84
 	}
85
-	validClientAuth := &oapi.OAuthClientAuthorization{
86
-		UserName:   "user",
87
-		ClientName: "test",
85
+
86
+	restrictedClient := &oapi.OAuthClient{
87
+		ObjectMeta:   kapi.ObjectMeta{Name: "test"},
88
+		Secret:       "secret",
89
+		RedirectURIs: []string{assertServer.URL + "/assert"},
90
+		ScopeRestrictions: []oapi.ScopeRestriction{
91
+			{ExactValues: []string{"user:info"}},
92
+		},
88 93
 	}
89 94
 
90 95
 	testCases := map[string]struct {
... ...
@@ -98,7 +118,7 @@ func TestRegistryAndServer(t *testing.T) {
98 98
 		"needs auth": {
99 99
 			Client: validClient,
100 100
 			Check: func(h *testHandlers, _ *http.Request) {
101
-				if !h.AuthNeed || h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil {
101
+				if !h.AuthNeed || h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil || h.HandleAuthorizeReq.Authorized {
102 102
 					t.Errorf("expected request to need authentication: %#v", h)
103 103
 				}
104 104
 			},
... ...
@@ -110,11 +130,37 @@ func TestRegistryAndServer(t *testing.T) {
110 110
 				Name: "user",
111 111
 			},
112 112
 			Check: func(h *testHandlers, _ *http.Request) {
113
-				if h.AuthNeed || !h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil {
113
+				if h.AuthNeed || !h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil || h.HandleAuthorizeReq.Authorized {
114 114
 					t.Errorf("expected request to need to grant access: %#v", h)
115 115
 				}
116 116
 			},
117 117
 		},
118
+		"invalid scope": {
119
+			Client:      validClient,
120
+			AuthSuccess: true,
121
+			AuthUser: &user.DefaultInfo{
122
+				Name: "user",
123
+			},
124
+			Scope: "some-scope",
125
+			Check: func(h *testHandlers, _ *http.Request) {
126
+				if h.AuthNeed || h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil || h.HandleAuthorizeReq.Authorized || h.HandleAuthorizeResp.ErrorId != "invalid_scope" {
127
+					t.Errorf("expected invalid_scope error: %#v, %#v, %#v", h, h.HandleAuthorizeReq, h.HandleAuthorizeResp)
128
+				}
129
+			},
130
+		},
131
+		"disallowed scope": {
132
+			Client:      restrictedClient,
133
+			AuthSuccess: true,
134
+			AuthUser: &user.DefaultInfo{
135
+				Name: "user",
136
+			},
137
+			Scope: "user:full",
138
+			Check: func(h *testHandlers, _ *http.Request) {
139
+				if h.AuthNeed || h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil || h.HandleAuthorizeReq.Authorized || h.HandleAuthorizeResp.ErrorId != "access_denied" {
140
+					t.Errorf("expected access_denied error: %#v, %#v, %#v", h, h.HandleAuthorizeReq, h.HandleAuthorizeResp)
141
+				}
142
+			},
143
+		},
118 144
 		"has non covered grant": {
119 145
 			Client:      validClient,
120 146
 			AuthSuccess: true,
... ...
@@ -124,11 +170,11 @@ func TestRegistryAndServer(t *testing.T) {
124 124
 			ClientAuth: &oapi.OAuthClientAuthorization{
125 125
 				UserName:   "user",
126 126
 				ClientName: "test",
127
-				Scopes:     []string{"test"},
127
+				Scopes:     []string{"user:info"},
128 128
 			},
129
-			Scope: "test other",
129
+			Scope: "user:info user:check-access",
130 130
 			Check: func(h *testHandlers, req *http.Request) {
131
-				if h.AuthNeed || !h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil {
131
+				if h.AuthNeed || !h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil || h.HandleAuthorizeReq.Authorized {
132 132
 					t.Errorf("expected request to need to grant access because of uncovered scopes: %#v", h)
133 133
 				}
134 134
 			},
... ...
@@ -142,12 +188,12 @@ func TestRegistryAndServer(t *testing.T) {
142 142
 			ClientAuth: &oapi.OAuthClientAuthorization{
143 143
 				UserName:   "user",
144 144
 				ClientName: "test",
145
-				Scopes:     []string{"test", "other"},
145
+				Scopes:     []string{"user:info", "user:check-access"},
146 146
 			},
147
-			Scope: "test other",
147
+			Scope: "user:info user:check-access",
148 148
 			Check: func(h *testHandlers, req *http.Request) {
149
-				if h.AuthNeed || h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil {
150
-					t.Errorf("unexpected flow: %#v", h)
149
+				if h.AuthNeed || h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil || !h.HandleAuthorizeReq.Authorized || h.HandleAuthorizeResp.ErrorId != "" {
150
+					t.Errorf("unexpected flow: %#v, %#v, %#v", h, h.HandleAuthorizeReq, h.HandleAuthorizeResp)
151 151
 				}
152 152
 			},
153 153
 		},
... ...
@@ -157,10 +203,14 @@ func TestRegistryAndServer(t *testing.T) {
157 157
 			AuthUser: &user.DefaultInfo{
158 158
 				Name: "user",
159 159
 			},
160
-			ClientAuth: validClientAuth,
160
+			ClientAuth: &oapi.OAuthClientAuthorization{
161
+				UserName:   "user",
162
+				ClientName: "test",
163
+				Scopes:     []string{"user:full"},
164
+			},
161 165
 			Check: func(h *testHandlers, req *http.Request) {
162
-				if h.AuthNeed || h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil {
163
-					t.Errorf("unexpected flow: %#v", h)
166
+				if h.AuthNeed || h.GrantNeed || h.AuthErr != nil || h.GrantErr != nil || !h.HandleAuthorizeReq.Authorized || h.HandleAuthorizeResp.ErrorId != "" {
167
+					t.Errorf("unexpected flow: %#v, %#v, %#v", h, h.HandleAuthorizeReq, h.HandleAuthorizeResp)
164 168
 					return
165 169
 				}
166 170
 				if req == nil {
... ...
@@ -193,21 +243,24 @@ func TestRegistryAndServer(t *testing.T) {
193 193
 		}
194 194
 		storage := registrystorage.New(access, authorize, client, NewUserConversion())
195 195
 		config := osinserver.NewDefaultServerConfig()
196
+
197
+		h.AuthorizeHandler = osinserver.AuthorizeHandlers{
198
+			handlers.NewAuthorizeAuthenticator(
199
+				h,
200
+				h,
201
+				h,
202
+			),
203
+			handlers.NewGrantCheck(
204
+				NewClientAuthorizationGrantChecker(grant),
205
+				h,
206
+				h,
207
+			),
208
+		}
209
+
196 210
 		server := osinserver.New(
197 211
 			config,
198 212
 			storage,
199
-			osinserver.AuthorizeHandlers{
200
-				handlers.NewAuthorizeAuthenticator(
201
-					h,
202
-					h,
203
-					h,
204
-				),
205
-				handlers.NewGrantCheck(
206
-					NewClientAuthorizationGrantChecker(grant),
207
-					h,
208
-					h,
209
-				),
210
-			},
213
+			h,
211 214
 			osinserver.AccessHandlers{
212 215
 				handlers.NewDenyAccessAuthenticator(),
213 216
 			},
... ...
@@ -72,6 +72,18 @@ func TestAuthorize(t *testing.T) {
72 72
 			attributes:     defaultauthorizer.DefaultAuthorizationAttributes{Verb: "get", NonResourceURL: true, URL: "/api"},
73 73
 			expectedCalled: true,
74 74
 		},
75
+		{
76
+			name:           "user:full covers any resource",
77
+			user:           &user.DefaultInfo{Extra: map[string][]string{authorizationapi.ScopesKey: {"user:full"}}},
78
+			attributes:     defaultauthorizer.DefaultAuthorizationAttributes{Verb: "update", Resource: "users", ResourceName: "harold"},
79
+			expectedCalled: true,
80
+		},
81
+		{
82
+			name:           "user:full covers any non-resource",
83
+			user:           &user.DefaultInfo{Extra: map[string][]string{authorizationapi.ScopesKey: {"user:full"}}},
84
+			attributes:     defaultauthorizer.DefaultAuthorizationAttributes{Verb: "post", NonResourceURL: true, URL: "/foo/bar/baz"},
85
+			expectedCalled: true,
86
+		},
75 87
 	}
76 88
 
77 89
 	for _, tc := range testCases {
... ...
@@ -79,12 +79,15 @@ var ScopeEvaluators = []ScopeEvaluator{
79 79
 // namespace:<namespace name>:<comma-delimited verbs>:<comma-delimited resources>
80 80
 
81 81
 const (
82
-	UserInfo        = "info"
83
-	UserAccessCheck = "check-access"
82
+	UserInfo        = UserIndicator + "info"
83
+	UserAccessCheck = UserIndicator + "check-access"
84 84
 
85 85
 	// UserListProject gives explicit permission to see the projects a user can see.  This is often used to prime secondary ACL systems
86 86
 	// unrelated to openshift and to display projects for selection in a secondary UI.
87
-	UserListProject = "list-projects"
87
+	UserListProject = UserIndicator + "list-projects"
88
+
89
+	// UserFull includes all permissions of the user
90
+	UserFull = UserIndicator + "full"
88 91
 )
89 92
 
90 93
 // user:<scope name>
... ...
@@ -96,9 +99,7 @@ func (userEvaluator) Handles(scope string) bool {
96 96
 
97 97
 func (userEvaluator) Validate(scope string) error {
98 98
 	switch scope {
99
-	case UserIndicator + UserInfo,
100
-		UserIndicator + UserAccessCheck,
101
-		UserIndicator + UserListProject:
99
+	case UserFull, UserInfo, UserAccessCheck, UserListProject:
102 100
 		return nil
103 101
 	}
104 102
 
... ...
@@ -107,12 +108,14 @@ func (userEvaluator) Validate(scope string) error {
107 107
 
108 108
 func (userEvaluator) Describe(scope string) string {
109 109
 	switch scope {
110
-	case UserIndicator + UserInfo:
111
-		return "Information about you, including: username, identity names, and group membership."
112
-	case UserIndicator + UserAccessCheck:
110
+	case UserInfo:
111
+		return "Information about you, including username, identity names, and group membership."
112
+	case UserAccessCheck:
113 113
 		return `Information about user privileges, e.g. "Can I create builds?"`
114
-	case UserIndicator + UserListProject:
114
+	case UserListProject:
115 115
 		return `See projects you're aware of and the metadata (display name, description, etc) about those projects.`
116
+	case UserFull:
117
+		return `Full access to the server with all of your permissions.`
116 118
 	default:
117 119
 		return fmt.Sprintf("unrecognized scope: %v", scope)
118 120
 	}
... ...
@@ -120,19 +123,24 @@ func (userEvaluator) Describe(scope string) string {
120 120
 
121 121
 func (userEvaluator) ResolveRules(scope, namespace string, clusterPolicyGetter client.ClusterPolicyLister) ([]authorizationapi.PolicyRule, error) {
122 122
 	switch scope {
123
-	case UserIndicator + UserInfo:
123
+	case UserInfo:
124 124
 		return []authorizationapi.PolicyRule{
125 125
 			{Verbs: sets.NewString("get"), APIGroups: []string{userapi.GroupName}, Resources: sets.NewString("users"), ResourceNames: sets.NewString("~")},
126 126
 		}, nil
127
-	case UserIndicator + UserAccessCheck:
127
+	case UserAccessCheck:
128 128
 		return []authorizationapi.PolicyRule{
129 129
 			{Verbs: sets.NewString("create"), APIGroups: []string{authorizationapi.GroupName}, Resources: sets.NewString("subjectaccessreviews", "localsubjectaccessreviews"), AttributeRestrictions: &authorizationapi.IsPersonalSubjectAccessReview{}},
130 130
 			authorizationapi.NewRule("create").Groups(authorizationapi.GroupName).Resources("selfsubjectrulesreviews").RuleOrDie(),
131 131
 		}, nil
132
-	case UserIndicator + UserListProject:
132
+	case UserListProject:
133 133
 		return []authorizationapi.PolicyRule{
134 134
 			{Verbs: sets.NewString("list"), APIGroups: []string{projectapi.GroupName}, Resources: sets.NewString("projects")},
135 135
 		}, nil
136
+	case UserFull:
137
+		return []authorizationapi.PolicyRule{
138
+			{Verbs: sets.NewString("*"), APIGroups: []string{"*"}, Resources: sets.NewString("*")},
139
+			{Verbs: sets.NewString("*"), NonResourceURLs: sets.NewString("*")},
140
+		}, nil
136 141
 	default:
137 142
 		return nil, fmt.Errorf("unrecognized scope: %v", scope)
138 143
 	}
... ...
@@ -33,28 +33,28 @@ func TestUserEvaluator(t *testing.T) {
33 33
 		},
34 34
 		{
35 35
 			name:     "info",
36
-			scopes:   []string{UserIndicator + UserInfo},
36
+			scopes:   []string{UserInfo},
37 37
 			numRules: 2,
38 38
 		},
39 39
 		{
40 40
 			name:     "one-error",
41
-			scopes:   []string{UserIndicator, UserIndicator + UserInfo},
41
+			scopes:   []string{UserIndicator, UserInfo},
42 42
 			err:      "unrecognized scope",
43 43
 			numRules: 2,
44 44
 		},
45 45
 		{
46 46
 			name:     "access",
47
-			scopes:   []string{UserIndicator + UserAccessCheck},
47
+			scopes:   []string{UserAccessCheck},
48 48
 			numRules: 3,
49 49
 		},
50 50
 		{
51 51
 			name:     "both",
52
-			scopes:   []string{UserIndicator + UserInfo, UserIndicator + UserAccessCheck},
52
+			scopes:   []string{UserInfo, UserAccessCheck},
53 53
 			numRules: 4,
54 54
 		},
55 55
 		{
56 56
 			name:     "list-projects",
57
-			scopes:   []string{UserIndicator + UserListProject},
57
+			scopes:   []string{UserListProject},
58 58
 			numRules: 2,
59 59
 		},
60 60
 	}
... ...
@@ -15,6 +15,7 @@ type OAuthClientAuthorizationInterface interface {
15 15
 	Create(obj *oauthapi.OAuthClientAuthorization) (*oauthapi.OAuthClientAuthorization, error)
16 16
 	List(opts kapi.ListOptions) (*oauthapi.OAuthClientAuthorizationList, error)
17 17
 	Get(name string) (*oauthapi.OAuthClientAuthorization, error)
18
+	Update(obj *oauthapi.OAuthClientAuthorization) (*oauthapi.OAuthClientAuthorization, error)
18 19
 	Delete(name string) error
19 20
 	Watch(opts kapi.ListOptions) (watch.Interface, error)
20 21
 }
... ...
@@ -35,6 +36,12 @@ func (c *oauthClientAuthorizations) Create(obj *oauthapi.OAuthClientAuthorizatio
35 35
 	return
36 36
 }
37 37
 
38
+func (c *oauthClientAuthorizations) Update(obj *oauthapi.OAuthClientAuthorization) (result *oauthapi.OAuthClientAuthorization, err error) {
39
+	result = &oauthapi.OAuthClientAuthorization{}
40
+	err = c.r.Put().Resource("oAuthClientAuthorizations").Name(obj.Name).Body(obj).Do().Into(result)
41
+	return
42
+}
43
+
38 44
 func (c *oauthClientAuthorizations) List(opts kapi.ListOptions) (result *oauthapi.OAuthClientAuthorizationList, err error) {
39 45
 	result = &oauthapi.OAuthClientAuthorizationList{}
40 46
 	err = c.r.Get().Resource("oAuthClientAuthorizations").VersionedParams(&opts, kapi.ParameterCodec).Do().Into(result)
... ...
@@ -39,6 +39,15 @@ func (c *FakeOAuthClientAuthorization) Create(inObj *oauthapi.OAuthClientAuthori
39 39
 	return obj.(*oauthapi.OAuthClientAuthorization), err
40 40
 }
41 41
 
42
+func (c *FakeOAuthClientAuthorization) Update(inObj *oauthapi.OAuthClientAuthorization) (*oauthapi.OAuthClientAuthorization, error) {
43
+	obj, err := c.Fake.Invokes(ktestclient.NewRootUpdateAction("oauthClientAuthorizations", inObj), inObj)
44
+	if obj == nil {
45
+		return nil, err
46
+	}
47
+
48
+	return obj.(*oauthapi.OAuthClientAuthorization), err
49
+}
50
+
42 51
 func (c *FakeOAuthClientAuthorization) Delete(name string) error {
43 52
 	_, err := c.Fake.Invokes(ktestclient.NewRootDeleteAction("oauthClientAuthorizations", name), &oauthapi.OAuthClientAuthorization{})
44 53
 	return err
... ...
@@ -370,14 +370,14 @@ func (c *AuthConfig) getGrantHandler(mux cmdutil.Mux, auth authenticator.Request
370 370
 func (c *AuthConfig) getAuthenticationFinalizer() osinserver.AuthorizeHandler {
371 371
 	if c.SessionAuth != nil {
372 372
 		// The session needs to know the authorize flow is done so it can invalidate the session
373
-		return osinserver.AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
373
+		return osinserver.AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error) {
374 374
 			_ = c.SessionAuth.InvalidateAuthentication(w, ar.HttpRequest)
375 375
 			return false, nil
376 376
 		})
377 377
 	}
378 378
 
379 379
 	// Otherwise return a no-op finalizer
380
-	return osinserver.AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
380
+	return osinserver.AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error) {
381 381
 		return false, nil
382 382
 	})
383 383
 }
... ...
@@ -14,22 +14,25 @@ type Mux interface {
14 14
 
15 15
 // AuthorizeHandler populates an AuthorizeRequest or handles the request itself
16 16
 type AuthorizeHandler interface {
17
-	// HandleAuthorize populates an AuthorizeRequest (typically the Authorized and UserData fields)
18
-	// and returns false, or writes the response itself and returns true.
19
-	HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseWriter) (handled bool, err error)
17
+	// HandleAuthorize does one of the following:
18
+	// 1. populates an AuthorizeRequest (typically the Authorized and UserData fields) and returns false
19
+	// 2. populates the error fields of a Response object and returns false
20
+	// 3. returns an internal server error and returns false
21
+	// 4. writes the response itself and returns true
22
+	HandleAuthorize(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (handled bool, err error)
20 23
 }
21 24
 
22
-type AuthorizeHandlerFunc func(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error)
25
+type AuthorizeHandlerFunc func(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error)
23 26
 
24
-func (f AuthorizeHandlerFunc) HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
25
-	return f(ar, w)
27
+func (f AuthorizeHandlerFunc) HandleAuthorize(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error) {
28
+	return f(ar, resp, w)
26 29
 }
27 30
 
28 31
 type AuthorizeHandlers []AuthorizeHandler
29 32
 
30
-func (all AuthorizeHandlers) HandleAuthorize(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
33
+func (all AuthorizeHandlers) HandleAuthorize(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error) {
31 34
 	for _, h := range all {
32
-		if handled, err := h.HandleAuthorize(ar, w); handled || err != nil {
35
+		if handled, err := h.HandleAuthorize(ar, resp, w); handled || err != nil {
33 36
 			return handled, err
34 37
 		}
35 38
 	}
... ...
@@ -82,7 +82,7 @@ func (s *Server) handleAuthorize(w http.ResponseWriter, r *http.Request) {
82 82
 
83 83
 		} else {
84 84
 
85
-			handled, err := s.authorize.HandleAuthorize(ar, w)
85
+			handled, err := s.authorize.HandleAuthorize(ar, resp, w)
86 86
 			if err != nil {
87 87
 				s.errorHandler.HandleError(err, w, r)
88 88
 				return
... ...
@@ -22,7 +22,7 @@ func TestClientCredentialFlow(t *testing.T) {
22 22
 	oauthServer := New(
23 23
 		NewDefaultServerConfig(),
24 24
 		storage,
25
-		AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
25
+		AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error) {
26 26
 			ar.Authorized = true
27 27
 			return false, nil
28 28
 		}),
... ...
@@ -65,7 +65,7 @@ func TestAuthorizeStartFlow(t *testing.T) {
65 65
 	oauthServer := New(
66 66
 		NewDefaultServerConfig(),
67 67
 		storage,
68
-		AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
68
+		AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error) {
69 69
 			ar.Authorized = true
70 70
 			return false, nil
71 71
 		}),
... ...
@@ -83,7 +83,7 @@ func (a *saOAuthClientAdapter) GetClient(ctx kapi.Context, name string) (*oautha
83 83
 
84 84
 func getScopeRestrictionsFor(namespace, name string) []oauthapi.ScopeRestriction {
85 85
 	return []oauthapi.ScopeRestriction{
86
-		{ExactValues: []string{scopeauthorizer.UserIndicator + scopeauthorizer.UserInfo, scopeauthorizer.UserIndicator + scopeauthorizer.UserAccessCheck}},
86
+		{ExactValues: []string{scopeauthorizer.UserInfo, scopeauthorizer.UserAccessCheck}},
87 87
 		{ClusterRole: &oauthapi.ClusterRoleScopeRestriction{RoleNames: []string{"*"}, Namespaces: []string{namespace}, AllowEscalation: true}},
88 88
 	}
89 89
 }
... ...
@@ -82,7 +82,7 @@ func TestNodeAuth(t *testing.T) {
82 82
 		ObjectMeta: kapi.ObjectMeta{Name: "whoami-token-plus-some-padding-here-to-make-the-limit"},
83 83
 		ClientName: origin.OpenShiftCLIClientID,
84 84
 		ExpiresIn:  200,
85
-		Scopes:     []string{scope.UserIndicator + scope.UserInfo},
85
+		Scopes:     []string{scope.UserInfo},
86 86
 		UserName:   bobUser.Name,
87 87
 		UserUID:    string(bobUser.UID),
88 88
 	}
... ...
@@ -86,7 +86,7 @@ func TestOAuthStorage(t *testing.T) {
86 86
 	oauthServer := osinserver.New(
87 87
 		osinserver.NewDefaultServerConfig(),
88 88
 		storage,
89
-		osinserver.AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, w http.ResponseWriter) (bool, error) {
89
+		osinserver.AuthorizeHandlerFunc(func(ar *osin.AuthorizeRequest, resp *osin.Response, w http.ResponseWriter) (bool, error) {
90 90
 			ar.UserData = "test"
91 91
 			ar.Authorized = true
92 92
 			return false, nil
... ...
@@ -2,18 +2,18 @@ package integration
2 2
 
3 3
 import (
4 4
 	"crypto/tls"
5
-	"encoding/base64"
6
-	"io/ioutil"
7 5
 	"net/http"
6
+	"net/http/cookiejar"
8 7
 	"net/http/httptest"
9 8
 	"net/http/httputil"
10
-	"net/url"
11
-	"regexp"
12
-	"strings"
9
+	"reflect"
13 10
 	"testing"
14 11
 	"time"
15 12
 
13
+	"golang.org/x/net/html"
14
+
16 15
 	"github.com/RangelReale/osincli"
16
+	"github.com/golang/glog"
17 17
 
18 18
 	kapi "k8s.io/kubernetes/pkg/api"
19 19
 	kapierrors "k8s.io/kubernetes/pkg/api/errors"
... ...
@@ -23,11 +23,12 @@ import (
23 23
 	"k8s.io/kubernetes/pkg/util/wait"
24 24
 
25 25
 	"github.com/openshift/origin/pkg/client"
26
-	"github.com/openshift/origin/pkg/cmd/server/origin"
27 26
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
27
+	oauthapi "github.com/openshift/origin/pkg/oauth/api"
28 28
 	"github.com/openshift/origin/pkg/oauth/scope"
29 29
 	saoauth "github.com/openshift/origin/pkg/serviceaccounts/oauthclient"
30 30
 	testutil "github.com/openshift/origin/test/util"
31
+	htmlutil "github.com/openshift/origin/test/util/html"
31 32
 	testserver "github.com/openshift/origin/test/util/server"
32 33
 )
33 34
 
... ...
@@ -52,6 +53,7 @@ func TestSAAsOAuthClient(t *testing.T) {
52 52
 		}
53 53
 	}))
54 54
 	defer oauthServer.Close()
55
+	redirectURL := oauthServer.URL + "/oauthcallback"
55 56
 
56 57
 	clusterAdminClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig)
57 58
 	if err != nil {
... ...
@@ -74,6 +76,17 @@ func TestSAAsOAuthClient(t *testing.T) {
74 74
 		t.Fatalf("unexpected error: %v", err)
75 75
 	}
76 76
 
77
+	promptingClient, err := clusterAdminClient.OAuthClients().Create(&oauthapi.OAuthClient{
78
+		ObjectMeta:            kapi.ObjectMeta{Name: "prompting-client"},
79
+		Secret:                "prompting-client-secret",
80
+		RedirectURIs:          []string{redirectURL},
81
+		GrantMethod:           oauthapi.GrantHandlerPrompt,
82
+		RespondWithChallenges: true,
83
+	})
84
+	if err != nil {
85
+		t.Fatalf("unexpected error: %v", err)
86
+	}
87
+
77 88
 	// get the SA ready with redirect URIs and secret annotations
78 89
 	var defaultSA *kapi.ServiceAccount
79 90
 
... ...
@@ -86,7 +99,7 @@ func TestSAAsOAuthClient(t *testing.T) {
86 86
 		if defaultSA.Annotations == nil {
87 87
 			defaultSA.Annotations = map[string]string{}
88 88
 		}
89
-		defaultSA.Annotations[saoauth.OAuthRedirectURISecretAnnotationPrefix+"one"] = oauthServer.URL
89
+		defaultSA.Annotations[saoauth.OAuthRedirectURISecretAnnotationPrefix+"one"] = redirectURL
90 90
 		defaultSA.Annotations[saoauth.OAuthWantChallengesAnnotationPrefix] = "true"
91 91
 		defaultSA, err = clusterAdminKubeClient.ServiceAccounts(projectName).Update(defaultSA)
92 92
 		return err
... ...
@@ -116,209 +129,315 @@ func TestSAAsOAuthClient(t *testing.T) {
116 116
 		t.Fatalf("unexpected error: %v", err)
117 117
 	}
118 118
 
119
-	oauthClientConfig := &osincli.ClientConfig{
120
-		ClientId:     serviceaccount.MakeUsername(defaultSA.Namespace, defaultSA.Name),
121
-		ClientSecret: string(oauthSecret.Data[kapi.ServiceAccountTokenKey]),
122
-		AuthorizeUrl: clusterAdminClientConfig.Host + "/oauth/authorize",
123
-		TokenUrl:     clusterAdminClientConfig.Host + "/oauth/token",
124
-		RedirectUrl:  oauthServer.URL,
125
-		Scope:        scope.Join([]string{"user:info", "role:edit:" + projectName}),
126
-		SendClientSecretInParams: true,
119
+	promptApprovalSuccess := []string{
120
+		"GET /oauth/authorize",
121
+		"received challenge",
122
+		"GET /oauth/authorize",
123
+		"redirect to /oauth/approve",
124
+		"form",
125
+		"POST /oauth/approve",
126
+		"redirect to /oauth/authorize",
127
+		"redirect to /oauthcallback",
128
+		"code",
129
+	}
130
+	noPromptSuccess := []string{
131
+		"GET /oauth/authorize",
132
+		"received challenge",
133
+		"GET /oauth/authorize",
134
+		"redirect to /oauthcallback",
135
+		"code",
127 136
 	}
128
-	runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true)
129
-	clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
130
-
131
-	oauthClientConfig = &osincli.ClientConfig{
132
-		ClientId:     serviceaccount.MakeUsername(defaultSA.Namespace, defaultSA.Name),
133
-		ClientSecret: string(oauthSecret.Data[kapi.ServiceAccountTokenKey]),
134
-		AuthorizeUrl: clusterAdminClientConfig.Host + "/oauth/authorize",
135
-		TokenUrl:     clusterAdminClientConfig.Host + "/oauth/token",
136
-		RedirectUrl:  oauthServer.URL,
137
-		Scope:        scope.Join([]string{"user:info", "role:edit:other-ns"}),
138
-		SendClientSecretInParams: true,
139
-	}
140
-	runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, false, false)
141
-	clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
142
-
143
-	oauthClientConfig = &osincli.ClientConfig{
144
-		ClientId:     serviceaccount.MakeUsername(defaultSA.Namespace, defaultSA.Name),
145
-		ClientSecret: string(oauthSecret.Data[kapi.ServiceAccountTokenKey]),
146
-		AuthorizeUrl: clusterAdminClientConfig.Host + "/oauth/authorize",
147
-		TokenUrl:     clusterAdminClientConfig.Host + "/oauth/token",
148
-		RedirectUrl:  oauthServer.URL,
149
-		Scope:        scope.Join([]string{"user:info"}),
150
-		SendClientSecretInParams: true,
151
-	}
152
-	runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false)
153
-	clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
154
-}
155 137
 
156
-var grantCSRFRegex = regexp.MustCompile(`input type="hidden" name="csrf" value="([^"]*)"`)
157
-var grantThenRegex = regexp.MustCompile(`input type="hidden" name="then" value="([^"]*)"`)
138
+	// Test with a normal OAuth client
139
+	{
140
+		oauthClientConfig := &osincli.ClientConfig{
141
+			ClientId:                 promptingClient.Name,
142
+			ClientSecret:             promptingClient.Secret,
143
+			AuthorizeUrl:             clusterAdminClientConfig.Host + "/oauth/authorize",
144
+			TokenUrl:                 clusterAdminClientConfig.Host + "/oauth/token",
145
+			RedirectUrl:              redirectURL,
146
+			SendClientSecretInParams: true,
147
+		}
148
+		t.Log("Testing unrestricted scope")
149
+		oauthClientConfig.Scope = ""
150
+		// approval steps are needed for unscoped access
151
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, promptApprovalSuccess)
152
+		// verify the persisted client authorization looks like we expect
153
+		if clientAuth, err := clusterAdminClient.OAuthClientAuthorizations().Get("harold:" + oauthClientConfig.ClientId); err != nil {
154
+			t.Fatalf("Unexpected error: %v", err)
155
+		} else if !reflect.DeepEqual(clientAuth.Scopes, []string{"user:full"}) {
156
+			t.Fatalf("Unexpected scopes: %v", clientAuth.Scopes)
157
+		} else {
158
+			// update the authorization to not contain any approved scopes
159
+			clientAuth.Scopes = nil
160
+			if _, err := clusterAdminClient.OAuthClientAuthorizations().Update(clientAuth); err != nil {
161
+				t.Fatalf("Unexpected error: %v", err)
162
+			}
163
+		}
164
+		// approval steps are needed again for unscoped access
165
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, promptApprovalSuccess)
166
+		// with the authorization stored, approval steps are skipped
167
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, noPromptSuccess)
158 168
 
159
-func runOAuthFlow(t *testing.T, clusterAdminClientConfig *restclient.Config, projectName string, oauthClientConfig *osincli.ClientConfig, authorizationCodes, authorizationErrors chan string, expectGrantSuccess, expectBuildSuccess bool) {
160
-	oauthRuntimeClient, err := osincli.NewClient(oauthClientConfig)
161
-	if err != nil {
162
-		t.Fatalf("unexpected error: %v", err)
163
-	}
164
-	oauthRuntimeClient.Transport = &http.Transport{
165
-		TLSClientConfig: &tls.Config{
166
-			InsecureSkipVerify: true,
167
-		},
168
-	}
169
+		// Approval step is needed again. Second time, the approval steps is not needed
170
+		t.Log("Testing restricted scope")
171
+		oauthClientConfig.Scope = "user:info"
172
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false, promptApprovalSuccess)
173
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false, noPromptSuccess)
169 174
 
170
-	directHTTPClient := &http.Client{
171
-		Transport: &http.Transport{
172
-			TLSClientConfig: &tls.Config{
173
-				InsecureSkipVerify: true,
174
-			},
175
-		},
176
-	}
175
+		// Now request an unscoped token again, and no approval should be needed
176
+		t.Log("Testing unrestricted scope")
177
+		oauthClientConfig.Scope = ""
178
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, noPromptSuccess)
177 179
 
178
-	// make sure we're prompted for a password
179
-	authorizeRequest := oauthRuntimeClient.NewAuthorizeRequest(osincli.CODE)
180
-	authorizeURL := authorizeRequest.GetAuthorizeUrlWithParams("opaque-scope")
181
-	authorizeHTTPRequest, err := http.NewRequest("GET", authorizeURL.String(), nil)
182
-	if err != nil {
183
-		t.Fatalf("unexpected error: %v", err)
184
-	}
185
-	authorizeHTTPRequest.Header.Add("X-CSRF-Token", "csrf-01")
186
-	authorizeResponse, err := directHTTPClient.Do(authorizeHTTPRequest)
187
-	if err != nil {
188
-		t.Fatalf("unexpected error: %v", err)
189
-	}
190
-	if authorizeResponse.StatusCode != http.StatusUnauthorized {
191
-		response, _ := httputil.DumpResponse(authorizeResponse, true)
192
-		t.Fatalf("didn't get an unauthorized:\n %v", string(response))
180
+		clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
193 181
 	}
194 182
 
195
-	// first we should get a redirect to a grant flow
196
-	authenticatedAuthorizeHTTPRequest1, err := http.NewRequest("GET", authorizeURL.String(), nil)
197
-	authenticatedAuthorizeHTTPRequest1.Header.Add("X-CSRF-Token", "csrf-01")
198
-	authenticatedAuthorizeHTTPRequest1.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
199
-	authentictedAuthorizeResponse1, err := directHTTPClient.Transport.RoundTrip(authenticatedAuthorizeHTTPRequest1)
200
-	if err != nil {
201
-		t.Fatalf("unexpected error: %v", err)
202
-	}
203
-	if authentictedAuthorizeResponse1.StatusCode != http.StatusFound {
204
-		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse1, true)
205
-		t.Fatalf("unexpected status :\n %v", string(response))
183
+	{
184
+		oauthClientConfig := &osincli.ClientConfig{
185
+			ClientId:     serviceaccount.MakeUsername(defaultSA.Namespace, defaultSA.Name),
186
+			ClientSecret: string(oauthSecret.Data[kapi.ServiceAccountTokenKey]),
187
+			AuthorizeUrl: clusterAdminClientConfig.Host + "/oauth/authorize",
188
+			TokenUrl:     clusterAdminClientConfig.Host + "/oauth/token",
189
+			RedirectUrl:  redirectURL,
190
+			Scope:        scope.Join([]string{"user:info", "role:edit:" + projectName}),
191
+			SendClientSecretInParams: true,
192
+		}
193
+		t.Log("Testing allowed scopes")
194
+		// First time, the approval steps are needed
195
+		// Second time, the approval steps are skipped
196
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, promptApprovalSuccess)
197
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, true, noPromptSuccess)
198
+		clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
206 199
 	}
207 200
 
208
-	// second we get a webpage with a prompt.  Yeah, this next bit gets nasty
209
-	authenticatedAuthorizeHTTPRequest2, err := http.NewRequest("GET", clusterAdminClientConfig.Host+authentictedAuthorizeResponse1.Header.Get("Location"), nil)
210
-	authenticatedAuthorizeHTTPRequest2.Header.Add("X-CSRF-Token", "csrf-01")
211
-	authenticatedAuthorizeHTTPRequest2.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
212
-	authentictedAuthorizeResponse2, err := directHTTPClient.Transport.RoundTrip(authenticatedAuthorizeHTTPRequest2)
213
-	if err != nil {
214
-		t.Fatalf("unexpected error: %v", err)
215
-	}
216
-	if authentictedAuthorizeResponse2.StatusCode != http.StatusOK {
217
-		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse2, true)
218
-		t.Fatalf("unexpected status :\n %v", string(response))
219
-	}
220
-	// 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
221
-	body, err := ioutil.ReadAll(authentictedAuthorizeResponse2.Body)
222
-	if err != nil {
223
-		t.Fatalf("unexpected error: %v", err)
201
+	{
202
+		oauthClientConfig := &osincli.ClientConfig{
203
+			ClientId:     serviceaccount.MakeUsername(defaultSA.Namespace, defaultSA.Name),
204
+			ClientSecret: string(oauthSecret.Data[kapi.ServiceAccountTokenKey]),
205
+			AuthorizeUrl: clusterAdminClientConfig.Host + "/oauth/authorize",
206
+			TokenUrl:     clusterAdminClientConfig.Host + "/oauth/token",
207
+			RedirectUrl:  redirectURL,
208
+			Scope:        scope.Join([]string{"user:info", "role:edit:other-ns"}),
209
+			SendClientSecretInParams: true,
210
+		}
211
+		t.Log("Testing disallowed scopes")
212
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, false, false, []string{
213
+			"GET /oauth/authorize",
214
+			"received challenge",
215
+			"GET /oauth/authorize",
216
+			"redirect to /oauthcallback",
217
+			"error:access_denied",
218
+		})
219
+		clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
224 220
 	}
225
-	if !expectGrantSuccess {
226
-		if !strings.Contains(string(body), "requested illegal scopes") {
227
-			t.Fatalf("missing expected message: %v", string(body))
221
+
222
+	{
223
+		t.Log("Testing invalid scopes")
224
+		oauthClientConfig := &osincli.ClientConfig{
225
+			ClientId:     serviceaccount.MakeUsername(defaultSA.Namespace, defaultSA.Name),
226
+			ClientSecret: string(oauthSecret.Data[kapi.ServiceAccountTokenKey]),
227
+			AuthorizeUrl: clusterAdminClientConfig.Host + "/oauth/authorize",
228
+			TokenUrl:     clusterAdminClientConfig.Host + "/oauth/token",
229
+			RedirectUrl:  redirectURL,
230
+			Scope:        scope.Join([]string{"unknown-scope"}),
231
+			SendClientSecretInParams: true,
228 232
 		}
229
-		return
233
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, false, false, []string{
234
+			"GET /oauth/authorize",
235
+			"received challenge",
236
+			"GET /oauth/authorize",
237
+			"redirect to /oauthcallback",
238
+			"error:invalid_scope",
239
+		})
240
+		clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
230 241
 	}
231
-	csrfMatches := grantCSRFRegex.FindStringSubmatch(string(body))
232
-	if len(csrfMatches) != 2 {
233
-		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse2, false)
234
-		t.Fatalf("unexpected body :\n %v\n%v", string(response), string(body))
242
+
243
+	{
244
+		t.Log("Testing allowed scopes with failed API call")
245
+		oauthClientConfig := &osincli.ClientConfig{
246
+			ClientId:     serviceaccount.MakeUsername(defaultSA.Namespace, defaultSA.Name),
247
+			ClientSecret: string(oauthSecret.Data[kapi.ServiceAccountTokenKey]),
248
+			AuthorizeUrl: clusterAdminClientConfig.Host + "/oauth/authorize",
249
+			TokenUrl:     clusterAdminClientConfig.Host + "/oauth/token",
250
+			RedirectUrl:  redirectURL,
251
+			Scope:        scope.Join([]string{"user:info"}),
252
+			SendClientSecretInParams: true,
253
+		}
254
+		// First time, the approval is needed
255
+		// Second time, the approval is skipped
256
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false, promptApprovalSuccess)
257
+		runOAuthFlow(t, clusterAdminClientConfig, projectName, oauthClientConfig, authorizationCodes, authorizationErrors, true, false, noPromptSuccess)
258
+		clusterAdminClient.OAuthClientAuthorizations().Delete("harold:" + oauthClientConfig.ClientId)
235 259
 	}
236
-	thenMatches := grantThenRegex.FindStringSubmatch(string(body))
237
-	if len(thenMatches) != 2 {
238
-		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse2, false)
239
-		t.Fatalf("unexpected body :\n %v\n%v", string(response), string(body))
260
+}
261
+
262
+func drain(ch chan string) {
263
+	for {
264
+		select {
265
+		case <-ch:
266
+		default:
267
+			return
268
+		}
240 269
 	}
241
-	t.Logf("CSRF is %v", csrfMatches)
242
-	t.Logf("then is %v", thenMatches)
243
-
244
-	// third we respond and approve the grant, then let the transport follow redirects and give us the code
245
-	postBody := strings.NewReader(url.Values(map[string][]string{
246
-		"then":         {thenMatches[1]},
247
-		"csrf":         {csrfMatches[1]},
248
-		"client_id":    {oauthClientConfig.ClientId},
249
-		"user_name":    {"harold"},
250
-		"scopes":       {oauthClientConfig.Scope},
251
-		"redirect_uri": {clusterAdminClientConfig.Host},
252
-		"approve":      {"true"},
253
-	}).Encode())
254
-	authenticatedAuthorizeHTTPRequest3, err := http.NewRequest("POST", clusterAdminClientConfig.Host+origin.OpenShiftApprovePrefix, postBody)
255
-	authenticatedAuthorizeHTTPRequest3.Header.Set("Content-Type", "application/x-www-form-urlencoded")
256
-	authenticatedAuthorizeHTTPRequest3.Header.Add("X-CSRF-Token", csrfMatches[1])
257
-	authenticatedAuthorizeHTTPRequest3.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
258
-	for i := range authentictedAuthorizeResponse2.Cookies() {
259
-		cookie := authentictedAuthorizeResponse2.Cookies()[i]
260
-		authenticatedAuthorizeHTTPRequest3.AddCookie(cookie)
270
+}
271
+
272
+type basicAuthTransport struct {
273
+	rt       http.RoundTripper
274
+	username string
275
+	password string
276
+}
277
+
278
+func (b *basicAuthTransport) RoundTrip(req *http.Request) (*http.Response, error) {
279
+	if len(b.username) > 0 || len(b.password) > 0 {
280
+		req.SetBasicAuth(b.username, b.password)
261 281
 	}
262
-	authentictedAuthorizeResponse3, err := directHTTPClient.Transport.RoundTrip(authenticatedAuthorizeHTTPRequest3)
282
+	return b.rt.RoundTrip(req)
283
+}
284
+
285
+func runOAuthFlow(
286
+	t *testing.T,
287
+	clusterAdminClientConfig *restclient.Config,
288
+	projectName string,
289
+	oauthClientConfig *osincli.ClientConfig,
290
+	authorizationCodes chan string,
291
+	authorizationErrors chan string,
292
+	expectGrantSuccess bool,
293
+	expectBuildSuccess bool,
294
+	expectOperations []string,
295
+) {
296
+	drain(authorizationCodes)
297
+	drain(authorizationErrors)
298
+
299
+	oauthRuntimeClient, err := osincli.NewClient(oauthClientConfig)
263 300
 	if err != nil {
264 301
 		t.Fatalf("unexpected error: %v", err)
265 302
 	}
266
-	if authentictedAuthorizeResponse3.StatusCode != http.StatusFound {
267
-		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse3, true)
268
-		t.Fatalf("unexpected status :\n %v", string(response))
269
-	}
303
+	testTransport := &basicAuthTransport{rt: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}}
304
+	oauthRuntimeClient.Transport = testTransport
270 305
 
271
-	// fourth, the grant redirects us again to have us send the code to the server
272
-	authenticatedAuthorizeHTTPRequest4, err := http.NewRequest("GET", clusterAdminClientConfig.Host+authentictedAuthorizeResponse3.Header.Get("Location"), nil)
273
-	authenticatedAuthorizeHTTPRequest4.Header.Add("X-CSRF-Token", "csrf-01")
274
-	authenticatedAuthorizeHTTPRequest4.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
275
-	authentictedAuthorizeResponse4, err := directHTTPClient.Transport.RoundTrip(authenticatedAuthorizeHTTPRequest4)
306
+	authorizeRequest := oauthRuntimeClient.NewAuthorizeRequest(osincli.CODE)
307
+	req, err := http.NewRequest("GET", authorizeRequest.GetAuthorizeUrlWithParams("opaque-state").String(), nil)
276 308
 	if err != nil {
277 309
 		t.Fatalf("unexpected error: %v", err)
278 310
 	}
279
-	if authentictedAuthorizeResponse4.StatusCode != http.StatusFound {
280
-		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse4, true)
281
-		t.Fatalf("unexpected status :\n %v", string(response))
311
+
312
+	operations := []string{}
313
+	jar, _ := cookiejar.New(nil)
314
+	directHTTPClient := &http.Client{
315
+		Transport: testTransport,
316
+		CheckRedirect: func(redirectReq *http.Request, via []*http.Request) error {
317
+			glog.Infof("302 Location: %s", redirectReq.URL.String())
318
+			req = redirectReq
319
+			operations = append(operations, "redirect to "+redirectReq.URL.Path)
320
+			return nil
321
+		},
322
+		Jar: jar,
282 323
 	}
283 324
 
284
-	authenticatedAuthorizeHTTPRequest5, err := http.NewRequest("GET", authentictedAuthorizeResponse4.Header.Get("Location"), nil)
285
-	authenticatedAuthorizeHTTPRequest5.Header.Add("X-CSRF-Token", "csrf-01")
286
-	authenticatedAuthorizeHTTPRequest5.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("harold:any-pass")))
287
-	authentictedAuthorizeResponse5, err := directHTTPClient.Do(authenticatedAuthorizeHTTPRequest5)
288
-	if err != nil {
289
-		t.Fatalf("unexpected error: %v", err)
325
+	for {
326
+		glog.Infof("%s %s", req.Method, req.URL.String())
327
+		operations = append(operations, req.Method+" "+req.URL.Path)
328
+
329
+		// Always set the csrf header
330
+		req.Header.Set("X-CSRF-Token", "1")
331
+		resp, err := directHTTPClient.Do(req)
332
+		if err != nil {
333
+			glog.Infof("%#v", operations)
334
+			glog.Infof("%#v", jar)
335
+			glog.Errorf("Error %v\n%#v\n%#v", err, err, resp)
336
+			t.Errorf("Error %v\n%#v\n%#v", err, err, resp)
337
+			return
338
+		}
339
+		defer resp.Body.Close()
340
+
341
+		// Save the current URL for reference
342
+		currentURL := req.URL
343
+
344
+		if resp.StatusCode == 401 {
345
+			// Set up a username and password once we're challenged
346
+			testTransport.username = "harold"
347
+			testTransport.password = "any-pass"
348
+			operations = append(operations, "received challenge")
349
+			continue
350
+		}
351
+
352
+		if resp.StatusCode != 200 {
353
+			responseDump, _ := httputil.DumpResponse(resp, true)
354
+			t.Errorf("Unexpected response %s", string(responseDump))
355
+			return
356
+		}
357
+
358
+		doc, err := html.Parse(resp.Body)
359
+		if err != nil {
360
+			t.Error(err)
361
+			return
362
+		}
363
+		forms := htmlutil.GetElementsByTagName(doc, "form")
364
+		// if there's a single form, submit it
365
+		if len(forms) > 1 {
366
+			t.Errorf("More than one form encountered: %d", len(forms))
367
+			return
368
+		}
369
+		if len(forms) == 0 {
370
+			break
371
+		}
372
+		req, err = htmlutil.NewRequestFromForm(forms[0], currentURL)
373
+		if err != nil {
374
+			t.Error(err)
375
+			return
376
+		}
377
+		operations = append(operations, "form")
290 378
 	}
291 379
 
292 380
 	authorizationCode := ""
293 381
 	select {
294 382
 	case authorizationCode = <-authorizationCodes:
295
-	case <-time.After(10 * time.Second):
296
-		response, _ := httputil.DumpResponse(authentictedAuthorizeResponse5, true)
297
-		t.Fatalf("didn't get a code:\n %v", string(response))
383
+		operations = append(operations, "code")
384
+	case authorizationError := <-authorizationErrors:
385
+		operations = append(operations, "error:"+authorizationError)
386
+	case <-time.After(5 * time.Second):
387
+		t.Error("didn't get a code or an error")
298 388
 	}
299 389
 
300
-	accessRequest := oauthRuntimeClient.NewAccessRequest(osincli.AUTHORIZATION_CODE, &osincli.AuthorizeData{Code: authorizationCode})
301
-	accessData, err := accessRequest.GetToken()
302
-	if err != nil {
303
-		t.Fatalf("unexpected error: %v", err)
390
+	if !reflect.DeepEqual(operations, expectOperations) {
391
+		t.Errorf("Expected:\n%v\nGot\n%v", expectOperations, operations)
304 392
 	}
305 393
 
306
-	whoamiConfig := clientcmd.AnonymousClientConfig(clusterAdminClientConfig)
307
-	whoamiConfig.BearerToken = accessData.AccessToken
308
-	whoamiClient, err := client.New(&whoamiConfig)
309
-	if err != nil {
310
-		t.Fatalf("unexpected error: %v", err)
311
-	}
394
+	if len(authorizationCode) > 0 {
395
+		accessRequest := oauthRuntimeClient.NewAccessRequest(osincli.AUTHORIZATION_CODE, &osincli.AuthorizeData{Code: authorizationCode})
396
+		accessData, err := accessRequest.GetToken()
397
+		if err != nil {
398
+			t.Errorf("unexpected error: %v", err)
399
+			return
400
+		}
312 401
 
313
-	if _, err := whoamiClient.Builds(projectName).List(kapi.ListOptions{}); !kapierrors.IsForbidden(err) && !expectBuildSuccess {
314
-		t.Fatalf("unexpected error: %v", err)
315
-	}
402
+		whoamiConfig := clientcmd.AnonymousClientConfig(clusterAdminClientConfig)
403
+		whoamiConfig.BearerToken = accessData.AccessToken
404
+		whoamiClient, err := client.New(&whoamiConfig)
405
+		if err != nil {
406
+			t.Errorf("unexpected error: %v", err)
407
+			return
408
+		}
316 409
 
317
-	user, err := whoamiClient.Users().Get("~")
318
-	if err != nil {
319
-		t.Fatalf("unexpected error: %v", err)
320
-	}
321
-	if user.Name != "harold" {
322
-		t.Fatalf("expected %v, got %v", "harold", user.Name)
410
+		_, err = whoamiClient.Builds(projectName).List(kapi.ListOptions{})
411
+		if expectBuildSuccess && err != nil {
412
+			t.Errorf("unexpected error: %v", err)
413
+			return
414
+		}
415
+		if !expectBuildSuccess && !kapierrors.IsForbidden(err) {
416
+			t.Errorf("expected forbidden error, got %v", err)
417
+			return
418
+		}
419
+
420
+		user, err := whoamiClient.Users().Get("~")
421
+		if err != nil {
422
+			t.Errorf("unexpected error: %v", err)
423
+			return
424
+		}
425
+		if user.Name != "harold" {
426
+			t.Errorf("expected %v, got %v", "harold", user.Name)
427
+			return
428
+		}
323 429
 	}
324 430
 }
... ...
@@ -58,7 +58,7 @@ func TestScopedTokens(t *testing.T) {
58 58
 		ObjectMeta: kapi.ObjectMeta{Name: "whoami-token-plus-some-padding-here-to-make-the-limit"},
59 59
 		ClientName: origin.OpenShiftCLIClientID,
60 60
 		ExpiresIn:  200,
61
-		Scopes:     []string{scope.UserIndicator + scope.UserInfo},
61
+		Scopes:     []string{scope.UserInfo},
62 62
 		UserName:   userName,
63 63
 		UserUID:    string(haroldUser.UID),
64 64
 	}
65 65
new file mode 100644
... ...
@@ -0,0 +1,119 @@
0
+package html
1
+
2
+import (
3
+	"fmt"
4
+	"io"
5
+	"net/http"
6
+	"net/url"
7
+	"strings"
8
+
9
+	"golang.org/x/net/html"
10
+)
11
+
12
+func visit(n *html.Node, visitor func(*html.Node)) {
13
+	visitor(n)
14
+	for c := n.FirstChild; c != nil; c = c.NextSibling {
15
+		visit(c, visitor)
16
+	}
17
+}
18
+
19
+func GetElementsByTagName(root *html.Node, tagName string) []*html.Node {
20
+	elements := []*html.Node{}
21
+	visit(root, func(n *html.Node) {
22
+		if n.Type == html.ElementNode && n.Data == tagName {
23
+			elements = append(elements, n)
24
+		}
25
+	})
26
+	return elements
27
+}
28
+
29
+func GetAttr(element *html.Node, attrName string) (string, bool) {
30
+	for _, attr := range element.Attr {
31
+		if attr.Key == attrName {
32
+			return attr.Val, true
33
+		}
34
+	}
35
+	return "", false
36
+}
37
+
38
+// NewRequestFromForm builds a request that simulates submitting the given form. It honors:
39
+// Form method (defaults to GET)
40
+// Form action (defaults to currentURL if missing, or currentURL's scheme/host if server-relative)
41
+// <input name="..." value="..."> values (only the first type="submit" input's value is included)
42
+func NewRequestFromForm(form *html.Node, currentURL *url.URL) (*http.Request, error) {
43
+	var (
44
+		reqMethod string
45
+		reqURL    *url.URL
46
+		reqBody   io.Reader
47
+		reqHeader http.Header = http.Header{}
48
+		err       error
49
+	)
50
+
51
+	// Method defaults to GET if empty
52
+	if method, _ := GetAttr(form, "method"); len(method) > 0 {
53
+		reqMethod = strings.ToUpper(method)
54
+	} else {
55
+		reqMethod = "GET"
56
+	}
57
+
58
+	// URL defaults to current URL if empty
59
+	if action, _ := GetAttr(form, "action"); len(action) > 0 {
60
+		reqURL, err = url.Parse(action)
61
+		if err != nil {
62
+			return nil, err
63
+		}
64
+		if reqURL.Scheme == "" {
65
+			reqURL.Scheme = currentURL.Scheme
66
+		}
67
+		if reqURL.Host == "" {
68
+			reqURL.Host = currentURL.Host
69
+		}
70
+	} else {
71
+		reqURL, err = url.Parse(currentURL.String())
72
+		if err != nil {
73
+			return nil, err
74
+		}
75
+	}
76
+
77
+	formData := url.Values{}
78
+	if reqMethod == "GET" {
79
+		// Start with any existing query params when we're submitting via GET
80
+		formData = reqURL.Query()
81
+	}
82
+	addedSubmit := false
83
+	for _, input := range GetElementsByTagName(form, "input") {
84
+		if name, ok := GetAttr(input, "name"); ok {
85
+			if value, ok := GetAttr(input, "value"); ok {
86
+				// Check if this is a submit input
87
+				if inputType, _ := GetAttr(input, "type"); inputType == "submit" {
88
+					// Only add the value of the first one.
89
+					// We're simulating submitting the form.
90
+					if addedSubmit {
91
+						continue
92
+					}
93
+					// Remember we've added a submit input
94
+					addedSubmit = true
95
+				}
96
+				formData.Add(name, value)
97
+			}
98
+		}
99
+	}
100
+
101
+	switch reqMethod {
102
+	case "GET":
103
+		reqURL.RawQuery = formData.Encode()
104
+	case "POST":
105
+		reqHeader.Set("Content-Type", "application/x-www-form-urlencoded")
106
+		reqBody = strings.NewReader(formData.Encode())
107
+	default:
108
+		return nil, fmt.Errorf("unknown method: %s", reqMethod)
109
+	}
110
+
111
+	req, err := http.NewRequest(reqMethod, reqURL.String(), reqBody)
112
+	if err != nil {
113
+		return nil, err
114
+	}
115
+	req.Header = reqHeader
116
+	return req, nil
117
+
118
+}
0 119
new file mode 100644
... ...
@@ -0,0 +1,131 @@
0
+package html
1
+
2
+import (
3
+	"net/http"
4
+	"net/url"
5
+	"reflect"
6
+	"strings"
7
+	"testing"
8
+
9
+	"golang.org/x/net/html"
10
+)
11
+
12
+const (
13
+	sampleGetForm = `
14
+<html>
15
+<body>
16
+    <form id="1">
17
+        <input id="i1-1" name="a" value="av">
18
+        <input id="i1-2" type="submit" name="sa" value="sav">
19
+        <input id="i1-3" type="submit" name="sb" value="sbv">
20
+    </form>
21
+    <form id="2">
22
+        <input id="i2-1" name="c" value="cv">
23
+        <input id="i2-2" type="submit" name="sc" value="scv">
24
+        <input id="i2-3" type="submit" name="sd" value="sdv">
25
+    </form>
26
+</body>
27
+</html>
28
+`
29
+
30
+	samplePostForm = `
31
+<html>
32
+<body>
33
+    <form id="1" action="/test" method="post">
34
+        <input id="i1-1" name="a" value="av">
35
+        <input id="i1-2" type="submit" name="sa" value="sav">
36
+        <input id="i1-3" type="submit" name="sb" value="sbv">
37
+    </form>
38
+    <form id="2">
39
+        <input id="i2-1" name="c" value="cv">
40
+        <input id="i2-2" type="submit" name="sc" value="scv">
41
+        <input id="i2-3" type="submit" name="sd" value="sdv">
42
+    </form>
43
+</body>
44
+</html>
45
+`
46
+)
47
+
48
+func TestGetElementsByTagName(t *testing.T) {
49
+	tests := []struct {
50
+		Data        string
51
+		TagName     string
52
+		ExpectedIds []string
53
+	}{
54
+		{
55
+			Data:        sampleGetForm,
56
+			TagName:     `form`,
57
+			ExpectedIds: []string{"1", "2"},
58
+		},
59
+		{
60
+			Data:        sampleGetForm,
61
+			TagName:     `input`,
62
+			ExpectedIds: []string{"i1-1", "i1-2", "i1-3", "i2-1", "i2-2", "i2-3"},
63
+		},
64
+	}
65
+
66
+	for i, tc := range tests {
67
+		root, err := html.Parse(strings.NewReader(tc.Data))
68
+		if err != nil {
69
+			t.Errorf("%d: %v", i, err)
70
+			continue
71
+		}
72
+		elements := GetElementsByTagName(root, tc.TagName)
73
+		ids := []string{}
74
+		for _, e := range elements {
75
+			id, _ := GetAttr(e, "id")
76
+			ids = append(ids, id)
77
+		}
78
+		if !reflect.DeepEqual(tc.ExpectedIds, ids) {
79
+			t.Errorf("%d: expected %#v, got %#v", i, tc.ExpectedIds, ids)
80
+			continue
81
+		}
82
+	}
83
+}
84
+
85
+func TestNewRequestFromForm(t *testing.T) {
86
+
87
+	currentURL, _ := url.Parse("https://localhost:1234")
88
+
89
+	relativeGetReq, _ := http.NewRequest("GET", "https://localhost:1234?a=av&sa=sav", nil)
90
+
91
+	relativePostReq, _ := http.NewRequest("POST", "https://localhost:1234/test", strings.NewReader(url.Values{
92
+		"a":  []string{"av"},
93
+		"sa": []string{"sav"},
94
+	}.Encode()))
95
+	relativePostReq.Header.Set("Content-Type", "application/x-www-form-urlencoded")
96
+
97
+	tests := []struct {
98
+		Data            string
99
+		CurrentURL      *url.URL
100
+		ExpectedRequest *http.Request
101
+	}{
102
+		{
103
+			Data:            sampleGetForm,
104
+			CurrentURL:      currentURL,
105
+			ExpectedRequest: relativeGetReq,
106
+		},
107
+		{
108
+			Data:            samplePostForm,
109
+			CurrentURL:      currentURL,
110
+			ExpectedRequest: relativePostReq,
111
+		},
112
+	}
113
+
114
+	for i, tc := range tests {
115
+		root, err := html.Parse(strings.NewReader(tc.Data))
116
+		if err != nil {
117
+			t.Fatal(err)
118
+		}
119
+		forms := GetElementsByTagName(root, "form")
120
+		req, err := NewRequestFromForm(forms[0], tc.CurrentURL)
121
+		if err != nil {
122
+			t.Errorf("%d: %v", i, err)
123
+			continue
124
+		}
125
+		if !reflect.DeepEqual(tc.ExpectedRequest, req) {
126
+			t.Errorf("%d: expected\n%#v\ngot\n%#v", i, tc.ExpectedRequest, req)
127
+			continue
128
+		}
129
+	}
130
+}