Skip to content

Commit 12c70bb

Browse files
author
aeneasr
committed
all: sanitized tests and apis
1 parent ba84987 commit 12c70bb

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+837
-971
lines changed

access_request.go

-13
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,6 @@ func NewAccessRequest(session interface{}) *AccessRequest {
2020
}
2121
}
2222

23-
func (a *AccessRequest) DidHandleGrantTypes() bool {
24-
for _, grantType := range a.GrantTypes {
25-
if !StringInSlice(grantType, a.HandledGrantType) {
26-
return false
27-
}
28-
}
29-
return true
30-
}
31-
32-
func (a *AccessRequest) SetGrantTypeHandled(name string) {
33-
a.HandledGrantType = append(a.HandledGrantType, name)
34-
}
35-
3623
func (a *AccessRequest) GetGrantTypes() Arguments {
3724
return a.GrantTypes
3825
}

access_request_handler.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,11 @@ func (f *Fosite) NewAccessRequest(ctx context.Context, r *http.Request, session
7777
accessRequest.Client = client
7878

7979
for _, loader := range f.TokenEndpointHandlers {
80-
if err := loader.ValidateTokenEndpointRequest(ctx, r, accessRequest); err != nil {
80+
if err := loader.HandleTokenEndpointRequest(ctx, r, accessRequest); err != nil {
8181
return accessRequest, err
8282
}
8383
}
8484

85-
if !accessRequest.DidHandleGrantTypes() {
86-
return accessRequest, errors.New(ErrUnsupportedGrantType)
87-
}
88-
8985
if !accessRequest.GetScopes().Has(f.RequiredScope) {
9086
return accessRequest, errors.New(ErrInvalidScope)
9187
}

access_request_handler_test.go

+5-44
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func TestNewAccessRequest(t *testing.T) {
124124
store.EXPECT().GetClient(gomock.Eq("foo")).Return(client, nil)
125125
client.EXPECT().GetHashedSecret().Return([]byte("foo"))
126126
hasher.EXPECT().Compare(gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
127-
handler.EXPECT().ValidateTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any()).Return(ErrServerError)
127+
handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any()).Return(ErrServerError)
128128
},
129129
handlers: TokenEndpointHandlers{handler},
130130
},
@@ -136,48 +136,11 @@ func TestNewAccessRequest(t *testing.T) {
136136
form: url.Values{
137137
"grant_type": {"foo"},
138138
},
139-
expectErr: ErrUnsupportedGrantType,
140139
mock: func() {
141140
store.EXPECT().GetClient(gomock.Eq("foo")).Return(client, nil)
142141
client.EXPECT().GetHashedSecret().Return([]byte("foo"))
143142
hasher.EXPECT().Compare(gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
144-
handler.EXPECT().ValidateTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
145-
},
146-
handlers: TokenEndpointHandlers{handler},
147-
},
148-
{
149-
header: http.Header{
150-
"Authorization": {basicAuth("foo", "bar")},
151-
},
152-
method: "POST",
153-
form: url.Values{
154-
"grant_type": {"foo"},
155-
},
156-
expectErr: ErrUnsupportedGrantType,
157-
mock: func() {
158-
store.EXPECT().GetClient(gomock.Eq("foo")).Return(client, nil)
159-
client.EXPECT().GetHashedSecret().Return([]byte("foo"))
160-
hasher.EXPECT().Compare(gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
161-
handler.EXPECT().ValidateTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, a AccessRequester) {
162-
a.SetGrantTypeHandled("bar")
163-
}).Return(nil)
164-
},
165-
handlers: TokenEndpointHandlers{handler},
166-
},
167-
{
168-
header: http.Header{
169-
"Authorization": {basicAuth("foo", "bar")},
170-
},
171-
method: "POST",
172-
form: url.Values{
173-
"grant_type": {"foo"},
174-
},
175-
mock: func() {
176-
store.EXPECT().GetClient(gomock.Eq("foo")).Return(client, nil)
177-
client.EXPECT().GetHashedSecret().Return([]byte("foo"))
178-
hasher.EXPECT().Compare(gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
179-
handler.EXPECT().ValidateTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, a AccessRequester) {
180-
a.SetGrantTypeHandled("foo")
143+
handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, a AccessRequester) {
181144
a.SetScopes([]string{"asdfasdf"})
182145
}).Return(nil)
183146
},
@@ -196,15 +159,13 @@ func TestNewAccessRequest(t *testing.T) {
196159
store.EXPECT().GetClient(gomock.Eq("foo")).Return(client, nil)
197160
client.EXPECT().GetHashedSecret().Return([]byte("foo"))
198161
hasher.EXPECT().Compare(gomock.Eq([]byte("foo")), gomock.Eq([]byte("bar"))).Return(nil)
199-
handler.EXPECT().ValidateTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, a AccessRequester) {
200-
a.SetGrantTypeHandled("foo")
162+
handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, a AccessRequester) {
201163
a.SetScopes([]string{DefaultRequiredScopeName})
202164
}).Return(nil)
203165
},
204166
handlers: TokenEndpointHandlers{handler},
205167
expect: &AccessRequest{
206-
GrantTypes: Arguments{"foo"},
207-
HandledGrantType: []string{"foo"},
168+
GrantTypes: Arguments{"foo"},
208169
Request: Request{
209170
Client: client,
210171
},
@@ -227,7 +188,7 @@ func TestNewAccessRequest(t *testing.T) {
227188
t.Logf("Error occured: %s", err.(*errors.Error).ErrorStack())
228189
}
229190
if err == nil {
230-
pkg.AssertObjectKeysEqual(t, c.expect, ar, "GrantType", "Client")
191+
pkg.AssertObjectKeysEqual(t, c.expect, ar, "GrantTypes", "Client")
231192
assert.NotNil(t, ar.GetRequestedAt())
232193
}
233194
t.Logf("Passed test case %d", k)

access_response_handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ func (f *Fosite) NewAccessResponse(ctx context.Context, req *http.Request, reque
1313

1414
response := NewAccessResponse()
1515
for _, tk = range f.TokenEndpointHandlers {
16-
if err = tk.HandleTokenEndpointRequest(ctx, req, requester, response); err != nil {
16+
if err = tk.PopulateTokenEndpointResponse(ctx, req, requester, response); err != nil {
1717
return nil, errors.Wrap(err, 1)
1818
}
1919
}

access_response_handler_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,21 @@ func TestNewAccessResponse(t *testing.T) {
3131
},
3232
{
3333
mock: func() {
34-
handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(ErrServerError)
34+
handler.EXPECT().PopulateTokenEndpointResponse(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(ErrServerError)
3535
},
3636
handlers: TokenEndpointHandlers{handler},
3737
expectErr: ErrServerError,
3838
},
3939
{
4040
mock: func() {
41-
handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
41+
handler.EXPECT().PopulateTokenEndpointResponse(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
4242
},
4343
handlers: TokenEndpointHandlers{handler},
4444
expectErr: ErrUnsupportedGrantType,
4545
},
4646
{
4747
mock: func() {
48-
handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, _ AccessRequester, resp AccessResponder) {
48+
handler.EXPECT().PopulateTokenEndpointResponse(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, _ AccessRequester, resp AccessResponder) {
4949
resp.SetAccessToken("foo")
5050
}).Return(nil)
5151
},
@@ -54,7 +54,7 @@ func TestNewAccessResponse(t *testing.T) {
5454
},
5555
{
5656
mock: func() {
57-
handler.EXPECT().HandleTokenEndpointRequest(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, _ AccessRequester, resp AccessResponder) {
57+
handler.EXPECT().PopulateTokenEndpointResponse(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, _ *http.Request, _ AccessRequester, resp AccessResponder) {
5858
resp.SetAccessToken("foo")
5959
resp.SetTokenType("bar")
6060
}).Return(nil)

arguments.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import "strings"
44

55
type Arguments []string
66

7-
func (r Arguments) Is(items ...string) bool {
7+
func (r Arguments) Matches(items ...string) bool {
88
found := make(map[string]bool)
99
for _, item := range items {
1010
if !StringInSlice(item, r) {
@@ -13,7 +13,7 @@ func (r Arguments) Is(items ...string) bool {
1313
found[item] = true
1414
}
1515

16-
return len(found) != len(r) && len(r) != len(items)
16+
return len(found) == len(r) && len(r) == len(items)
1717
}
1818

1919
func (r Arguments) Has(items ...string) bool {

arguments_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func TestArgumentsHas(t *testing.T) {
9595
}
9696
}
9797

98-
func TestArgumentsIs(t *testing.T) {
98+
func TestArgumentsMatches(t *testing.T) {
9999
for k, c := range []struct {
100100
args Arguments
101101
is []string
@@ -124,12 +124,12 @@ func TestArgumentsIs(t *testing.T) {
124124
{
125125
args: Arguments{"foo", "bar"},
126126
is: []string{"foo"},
127-
expect: true,
127+
expect: false,
128128
},
129129
{
130130
args: Arguments{"foo", "bar"},
131131
is: []string{"bar"},
132-
expect: true,
132+
expect: false,
133133
},
134134
{
135135
args: Arguments{"foo", "bar"},
@@ -142,7 +142,7 @@ func TestArgumentsIs(t *testing.T) {
142142
expect: false,
143143
},
144144
} {
145-
assert.Equal(t, c.expect, c.args.Is(c.is...), "%d", k)
145+
assert.Equal(t, c.expect, c.args.Matches(c.is...), "%d", k)
146146
t.Logf("Passed test case %d", k)
147147
}
148148
}

authorize_error.go

+10-5
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,22 @@ package fosite
22

33
import (
44
"net/http"
5-
6-
"github.com/ory-am/common/pkg"
5+
"encoding/json"
76
)
87

9-
const minStateLength = 8
10-
118
func (c *Fosite) WriteAuthorizeError(rw http.ResponseWriter, ar AuthorizeRequester, err error) {
129
rfcerr := ErrorToRFC6749Error(err)
1310

1411
if !ar.IsRedirectURIValid() {
15-
pkg.WriteIndentJSON(rw, rfcerr)
12+
js, err := json.MarshalIndent(rfcerr, "", "\t")
13+
if err != nil {
14+
http.Error(rw, err.Error(), http.StatusInternalServerError)
15+
return
16+
}
17+
18+
rw.Header().Set("Content-Type", "application/json")
19+
rw.WriteHeader(rfcerr.StatusCode)
20+
rw.Write(js)
1621
return
1722
}
1823

authorize_request.go

+9
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,15 @@ type AuthorizeRequest struct {
1414
Request
1515
}
1616

17+
func NewAuthorizeRequest() *AuthorizeRequest {
18+
return &AuthorizeRequest{
19+
ResponseTypes: Arguments{},
20+
RedirectURI: &url.URL{},
21+
HandledResponseTypes: Arguments{},
22+
Request: *NewRequest(),
23+
}
24+
}
25+
1726
func (d *AuthorizeRequest) IsRedirectURIValid() bool {
1827
if d.GetRedirectURI() == nil {
1928
return false

authorize_request_handler.go

+2-9
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,7 @@ func (c *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (Auth
5555
// values, where the order of values does not matter (e.g., response
5656
// type "a b" is the same as "b a"). The meaning of such composite
5757
// response types is defined by their respective specifications.
58-
responseTypes := removeEmpty(strings.Split(r.Form.Get("response_type"), " "))
59-
60-
// Enable support of multiple response types. This is for example required by OpenID Connect.
61-
if !c.EnableHybridAuthorizationFlow && len(responseTypes) > 1 {
62-
return request, errors.New(ErrInvalidRequest)
63-
}
64-
65-
request.ResponseTypes = responseTypes
58+
request.ResponseTypes = removeEmpty(strings.Split(r.Form.Get("response_type"), " "))
6659

6760
// rfc6819 4.4.1.8. Threat: CSRF Attack against redirect-uri
6861
// The "state" parameter should be used to link the authorization
@@ -74,7 +67,7 @@ func (c *Fosite) NewAuthorizeRequest(ctx context.Context, r *http.Request) (Auth
7467
if state == "" {
7568
return request, errors.New(ErrInvalidState)
7669
} else if len(state) < minStateLength {
77-
// We're assuming that using less then 6 characters for the state can not be considered "unguessable"
70+
// We're assuming that using less then 8 characters for the state can not be considered "unguessable"
7871
return request, errors.New(ErrInvalidState)
7972
}
8073
request.State = state

authorize_request_handler_test.go

+1-39
Original file line numberDiff line numberDiff line change
@@ -149,47 +149,9 @@ func TestNewAuthorizeRequest(t *testing.T) {
149149
},
150150
expectedError: ErrInvalidScope,
151151
},
152-
{
153-
desc: "should not pass because hybrid flow is not active",
154-
conf: &Fosite{Store: store},
155-
query: url.Values{
156-
"redirect_uri": {"https://foo.bar/cb"},
157-
"client_id": {"1234"},
158-
"response_type": {"code token"},
159-
"state": {"strong-state"},
160-
"scope": {DefaultRequiredScopeName + " foo bar"},
161-
},
162-
mock: func() {
163-
store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"https://foo.bar/cb"}}, nil)
164-
},
165-
expectedError: ErrInvalidRequest,
166-
},
167-
{
168-
desc: "should not pass because hybrid flow is not active",
169-
conf: &Fosite{Store: store},
170-
query: url.Values{
171-
"redirect_uri": {"https://foo.bar/cb"},
172-
"client_id": {"1234"},
173-
"response_type": {"code foo"},
174-
"state": {"strong-state"},
175-
"scope": {DefaultRequiredScopeName + " foo bar"},
176-
},
177-
mock: func() {
178-
store.EXPECT().GetClient("1234").Return(&SecureClient{RedirectURIs: []string{"https://foo.bar/cb"}}, nil)
179-
},
180-
expect: &AuthorizeRequest{
181-
RedirectURI: redir,
182-
ResponseTypes: []string{"code"},
183-
State: "strong-state",
184-
Request: Request{
185-
Scopes: []string{DefaultRequiredScopeName, "foo", "bar"},
186-
Client: &SecureClient{RedirectURIs: []string{"https://foo.bar/cb"}},
187-
},
188-
},
189-
},
190152
{
191153
desc: "should pass",
192-
conf: &Fosite{Store: store, EnableHybridAuthorizationFlow: true},
154+
conf: &Fosite{Store: store},
193155
query: url.Values{
194156
"redirect_uri": {"https://foo.bar/cb"},
195157
"client_id": {"1234"},

authorize_response.go

+8
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ type AuthorizeResponse struct {
1313
Fragment url.Values
1414
}
1515

16+
func NewAuthorizeResponse() *AuthorizeResponse {
17+
return &AuthorizeResponse{
18+
Header: http.Header{},
19+
Query: url.Values{},
20+
Fragment: url.Values{},
21+
}
22+
}
23+
1624
func (a *AuthorizeResponse) SetID(id string) {
1725
a.ID = id
1826
}

0 commit comments

Comments
 (0)