Skip to content

Commit e84558b

Browse files
hickfordwxiaoguang
hickford
and
wxiaoguang
authored
Improve OAuth integration tests (#21390)
In particular, test explicit error responses. No change to behaviour. Co-authored-by: wxiaoguang <[email protected]>
1 parent 9862936 commit e84558b

File tree

2 files changed

+133
-33
lines changed

2 files changed

+133
-33
lines changed

‎models/fixtures/oauth2_application.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44
name: "Test"
55
client_id: "da7da3ba-9a13-4167-856f-3899de0b0138"
66
client_secret: "$2a$10$UYRgUSgekzBp6hYe8pAdc.cgB4Gn06QRKsORUnIYTYQADs.YR/uvi" # bcrypt of "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=
7-
redirect_uris: '["a"]'
7+
redirect_uris: '["a", "https://example.com/xyzzy"]'
88
created_unix: 1546869730
99
updated_unix: 1546869730

‎tests/integration/oauth_test.go‎

Lines changed: 132 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,29 +12,59 @@ import (
1212

1313
"code.gitea.io/gitea/modules/json"
1414
"code.gitea.io/gitea/modules/setting"
15+
"code.gitea.io/gitea/routers/web/auth"
1516
"code.gitea.io/gitea/tests"
1617

1718
"github.com/stretchr/testify/assert"
1819
)
1920

20-
const defaultAuthorize = "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=a&response_type=code&state=thestate"
21-
22-
func TestNoClientID(t *testing.T) {
21+
func TestAuthorizeNoClientID(t *testing.T) {
2322
defer tests.PrepareTestEnv(t)()
2423
req := NewRequest(t, "GET", "/login/oauth/authorize")
2524
ctx := loginUser(t, "user2")
26-
ctx.MakeRequest(t, req, http.StatusBadRequest)
25+
resp := ctx.MakeRequest(t, req, http.StatusBadRequest)
26+
assert.Contains(t, resp.Body.String(), "Client ID not registered")
27+
}
28+
29+
func TestAuthorizeUnregisteredRedirect(t *testing.T) {
30+
defer tests.PrepareTestEnv(t)()
31+
req := NewRequest(t, "GET", "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=UNREGISTERED&response_type=code&state=thestate")
32+
ctx := loginUser(t, "user1")
33+
resp := ctx.MakeRequest(t, req, http.StatusBadRequest)
34+
assert.Contains(t, resp.Body.String(), "Unregistered Redirect URI")
35+
}
36+
37+
func TestAuthorizeUnsupportedResponseType(t *testing.T) {
38+
defer tests.PrepareTestEnv(t)()
39+
req := NewRequest(t, "GET", "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=a&response_type=UNEXPECTED&state=thestate")
40+
ctx := loginUser(t, "user1")
41+
resp := ctx.MakeRequest(t, req, http.StatusSeeOther)
42+
u, err := resp.Result().Location()
43+
assert.NoError(t, err)
44+
assert.Equal(t, "unsupported_response_type", u.Query().Get("error"))
45+
assert.Equal(t, "Only code response type is supported.", u.Query().Get("error_description"))
46+
}
47+
48+
func TestAuthorizeUnsupportedCodeChallengeMethod(t *testing.T) {
49+
defer tests.PrepareTestEnv(t)()
50+
req := NewRequest(t, "GET", "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=a&response_type=code&state=thestate&code_challenge_method=UNEXPECTED")
51+
ctx := loginUser(t, "user1")
52+
resp := ctx.MakeRequest(t, req, http.StatusSeeOther)
53+
u, err := resp.Result().Location()
54+
assert.NoError(t, err)
55+
assert.Equal(t, "invalid_request", u.Query().Get("error"))
56+
assert.Equal(t, "unsupported code challenge method", u.Query().Get("error_description"))
2757
}
2858

29-
func TestLoginRedirect(t *testing.T) {
59+
func TestAuthorizeLoginRedirect(t *testing.T) {
3060
defer tests.PrepareTestEnv(t)()
3161
req := NewRequest(t, "GET", "/login/oauth/authorize")
3262
assert.Contains(t, MakeRequest(t, req, http.StatusSeeOther).Body.String(), "/user/login")
3363
}
3464

35-
func TestShowAuthorize(t *testing.T) {
65+
func TestAuthorizeShow(t *testing.T) {
3666
defer tests.PrepareTestEnv(t)()
37-
req := NewRequest(t, "GET", defaultAuthorize)
67+
req := NewRequest(t, "GET", "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=a&response_type=code&state=thestate")
3868
ctx := loginUser(t, "user4")
3969
resp := ctx.MakeRequest(t, req, http.StatusOK)
4070

@@ -43,15 +73,17 @@ func TestShowAuthorize(t *testing.T) {
4373
htmlDoc.GetCSRF()
4474
}
4575

46-
func TestRedirectWithExistingGrant(t *testing.T) {
76+
func TestAuthorizeRedirectWithExistingGrant(t *testing.T) {
4777
defer tests.PrepareTestEnv(t)()
48-
req := NewRequest(t, "GET", defaultAuthorize)
78+
req := NewRequest(t, "GET", "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=https%3A%2F%2Fexample.com%2Fxyzzy&response_type=code&state=thestate")
4979
ctx := loginUser(t, "user1")
5080
resp := ctx.MakeRequest(t, req, http.StatusSeeOther)
5181
u, err := resp.Result().Location()
5282
assert.NoError(t, err)
5383
assert.Equal(t, "thestate", u.Query().Get("state"))
5484
assert.Truef(t, len(u.Query().Get("code")) > 30, "authorization code '%s' should be longer then 30", u.Query().Get("code"))
85+
u.RawQuery = ""
86+
assert.Equal(t, "https://example.com/xyzzy", u.String())
5587
}
5688

5789
func TestAccessTokenExchange(t *testing.T) {
@@ -62,7 +94,7 @@ func TestAccessTokenExchange(t *testing.T) {
6294
"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
6395
"redirect_uri": "a",
6496
"code": "authcode",
65-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
97+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
6698
})
6799
resp := MakeRequest(t, req, http.StatusOK)
68100
type response struct {
@@ -78,15 +110,15 @@ func TestAccessTokenExchange(t *testing.T) {
78110
assert.True(t, len(parsed.RefreshToken) > 10)
79111
}
80112

81-
func TestAccessTokenExchangeWithoutPKCE(t *testing.T) {
113+
func TestAccessTokenExchangeJSON(t *testing.T) {
82114
defer tests.PrepareTestEnv(t)()
83115
req := NewRequestWithJSON(t, "POST", "/login/oauth/access_token", map[string]string{
84116
"grant_type": "authorization_code",
85117
"client_id": "da7da3ba-9a13-4167-856f-3899de0b0138",
86118
"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
87119
"redirect_uri": "a",
88120
"code": "authcode",
89-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
121+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
90122
})
91123
resp := MakeRequest(t, req, http.StatusOK)
92124
type response struct {
@@ -102,16 +134,20 @@ func TestAccessTokenExchangeWithoutPKCE(t *testing.T) {
102134
assert.True(t, len(parsed.RefreshToken) > 10)
103135
}
104136

105-
func TestAccessTokenExchangeJSON(t *testing.T) {
137+
func TestAccessTokenExchangeWithoutPKCE(t *testing.T) {
106138
defer tests.PrepareTestEnv(t)()
107-
req := NewRequestWithJSON(t, "POST", "/login/oauth/access_token", map[string]string{
139+
req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
108140
"grant_type": "authorization_code",
109141
"client_id": "da7da3ba-9a13-4167-856f-3899de0b0138",
110142
"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
111143
"redirect_uri": "a",
112144
"code": "authcode",
113145
})
114-
MakeRequest(t, req, http.StatusBadRequest)
146+
resp := MakeRequest(t, req, http.StatusBadRequest)
147+
parsedError := new(auth.AccessTokenError)
148+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
149+
assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
150+
assert.Equal(t, "failed PKCE code challenge", parsedError.ErrorDescription)
115151
}
116152

117153
func TestAccessTokenExchangeWithInvalidCredentials(t *testing.T) {
@@ -123,49 +159,73 @@ func TestAccessTokenExchangeWithInvalidCredentials(t *testing.T) {
123159
"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
124160
"redirect_uri": "a",
125161
"code": "authcode",
126-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
162+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
127163
})
128-
MakeRequest(t, req, http.StatusBadRequest)
164+
resp := MakeRequest(t, req, http.StatusBadRequest)
165+
parsedError := new(auth.AccessTokenError)
166+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
167+
assert.Equal(t, "invalid_client", string(parsedError.ErrorCode))
168+
assert.Equal(t, "cannot load client with client id: '???'", parsedError.ErrorDescription)
169+
129170
// invalid client secret
130171
req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
131172
"grant_type": "authorization_code",
132173
"client_id": "da7da3ba-9a13-4167-856f-3899de0b0138",
133174
"client_secret": "???",
134175
"redirect_uri": "a",
135176
"code": "authcode",
136-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
177+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
137178
})
138-
MakeRequest(t, req, http.StatusBadRequest)
179+
resp = MakeRequest(t, req, http.StatusBadRequest)
180+
parsedError = new(auth.AccessTokenError)
181+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
182+
assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
183+
assert.Equal(t, "invalid client secret", parsedError.ErrorDescription)
184+
139185
// invalid redirect uri
140186
req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
141187
"grant_type": "authorization_code",
142188
"client_id": "da7da3ba-9a13-4167-856f-3899de0b0138",
143189
"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
144190
"redirect_uri": "???",
145191
"code": "authcode",
146-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
192+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
147193
})
148-
MakeRequest(t, req, http.StatusBadRequest)
194+
resp = MakeRequest(t, req, http.StatusBadRequest)
195+
parsedError = new(auth.AccessTokenError)
196+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
197+
assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
198+
assert.Equal(t, "unexpected redirect URI", parsedError.ErrorDescription)
199+
149200
// invalid authorization code
150201
req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
151202
"grant_type": "authorization_code",
152203
"client_id": "da7da3ba-9a13-4167-856f-3899de0b0138",
153204
"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
154205
"redirect_uri": "a",
155206
"code": "???",
156-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
207+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
157208
})
158-
MakeRequest(t, req, http.StatusBadRequest)
209+
resp = MakeRequest(t, req, http.StatusBadRequest)
210+
parsedError = new(auth.AccessTokenError)
211+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
212+
assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
213+
assert.Equal(t, "client is not authorized", parsedError.ErrorDescription)
214+
159215
// invalid grant_type
160216
req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
161217
"grant_type": "???",
162218
"client_id": "da7da3ba-9a13-4167-856f-3899de0b0138",
163219
"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
164220
"redirect_uri": "a",
165221
"code": "authcode",
166-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
222+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
167223
})
168-
MakeRequest(t, req, http.StatusBadRequest)
224+
resp = MakeRequest(t, req, http.StatusBadRequest)
225+
parsedError = new(auth.AccessTokenError)
226+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
227+
assert.Equal(t, "unsupported_grant_type", string(parsedError.ErrorCode))
228+
assert.Equal(t, "Only refresh_token or authorization_code grant type is supported", parsedError.ErrorDescription)
169229
}
170230

171231
func TestAccessTokenExchangeWithBasicAuth(t *testing.T) {
@@ -174,7 +234,7 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) {
174234
"grant_type": "authorization_code",
175235
"redirect_uri": "a",
176236
"code": "authcode",
177-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
237+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
178238
})
179239
req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpKaUE9")
180240
resp := MakeRequest(t, req, http.StatusOK)
@@ -195,19 +255,54 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) {
195255
"grant_type": "authorization_code",
196256
"redirect_uri": "a",
197257
"code": "authcode",
198-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
258+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
199259
})
200260
req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OmJsYWJsYQ==")
201-
MakeRequest(t, req, http.StatusBadRequest)
261+
resp = MakeRequest(t, req, http.StatusBadRequest)
262+
parsedError := new(auth.AccessTokenError)
263+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
264+
assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
265+
assert.Equal(t, "invalid client secret", parsedError.ErrorDescription)
202266

203267
// missing header
204268
req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
205269
"grant_type": "authorization_code",
206270
"redirect_uri": "a",
207271
"code": "authcode",
208-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
272+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
273+
})
274+
resp = MakeRequest(t, req, http.StatusBadRequest)
275+
parsedError = new(auth.AccessTokenError)
276+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
277+
assert.Equal(t, "invalid_client", string(parsedError.ErrorCode))
278+
assert.Equal(t, "cannot load client with client id: ''", parsedError.ErrorDescription)
279+
280+
// client_id inconsistent with Authorization header
281+
req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
282+
"grant_type": "authorization_code",
283+
"redirect_uri": "a",
284+
"code": "authcode",
285+
"client_id": "inconsistent",
286+
})
287+
req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpKaUE9")
288+
resp = MakeRequest(t, req, http.StatusBadRequest)
289+
parsedError = new(auth.AccessTokenError)
290+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
291+
assert.Equal(t, "invalid_request", string(parsedError.ErrorCode))
292+
assert.Equal(t, "client_id in request body inconsistent with Authorization header", parsedError.ErrorDescription)
293+
294+
// client_secret inconsistent with Authorization header
295+
req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
296+
"grant_type": "authorization_code",
297+
"redirect_uri": "a",
298+
"code": "authcode",
299+
"client_secret": "inconsistent",
209300
})
210-
MakeRequest(t, req, http.StatusBadRequest)
301+
req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpKaUE9")
302+
parsedError = new(auth.AccessTokenError)
303+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
304+
assert.Equal(t, "invalid_request", string(parsedError.ErrorCode))
305+
assert.Equal(t, "client_id in request body inconsistent with Authorization header", parsedError.ErrorDescription)
211306
}
212307

213308
func TestRefreshTokenInvalidation(t *testing.T) {
@@ -218,7 +313,7 @@ func TestRefreshTokenInvalidation(t *testing.T) {
218313
"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
219314
"redirect_uri": "a",
220315
"code": "authcode",
221-
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
316+
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
222317
})
223318
resp := MakeRequest(t, req, http.StatusOK)
224319
type response struct {
@@ -256,6 +351,11 @@ func TestRefreshTokenInvalidation(t *testing.T) {
256351
refreshReq.Body = io.NopCloser(bytes.NewReader(bs))
257352
MakeRequest(t, refreshReq, http.StatusOK)
258353

354+
// repeat request should fail
259355
refreshReq.Body = io.NopCloser(bytes.NewReader(bs))
260-
MakeRequest(t, refreshReq, http.StatusBadRequest)
356+
resp = MakeRequest(t, refreshReq, http.StatusBadRequest)
357+
parsedError := new(auth.AccessTokenError)
358+
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
359+
assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
360+
assert.Equal(t, "token was already used", parsedError.ErrorDescription)
261361
}

0 commit comments

Comments
 (0)