mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-08-29 13:43:51 +00:00
[v12.0/forgejo] fix: store code challenge correctly in session (#8682)
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/8678 - Even though the test file contains some good extensive testing, it didn't bother to actually call `/login/oauth/access_token` to see if the received code actually resulted into a access token. - The fix itself is... well yeah self-explanatory. - Resolves forgejo/forgejo#8669 Co-authored-by: Gusted <postmaster@gusted.xyz> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8682 Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org> Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
This commit is contained in:
parent
267f314aef
commit
89a84a51e8
2 changed files with 62 additions and 1 deletions
|
@ -489,7 +489,7 @@ func AuthorizeOAuth(ctx *context.Context) {
|
||||||
}, form.RedirectURI)
|
}, form.RedirectURI)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if err := ctx.Session.Set("CodeChallengeMethod", form.CodeChallenge); err != nil {
|
if err := ctx.Session.Set("CodeChallenge", form.CodeChallenge); err != nil {
|
||||||
handleAuthorizeError(ctx, AuthorizeError{
|
handleAuthorizeError(ctx, AuthorizeError{
|
||||||
ErrorCode: ErrorCodeServerError,
|
ErrorCode: ErrorCodeServerError,
|
||||||
ErrorDescription: "cannot set code challenge",
|
ErrorDescription: "cannot set code challenge",
|
||||||
|
|
|
@ -1535,3 +1535,64 @@ func TestSignUpViaOAuth2FA(t *testing.T) {
|
||||||
// Make sure user has to go through 2FA.
|
// Make sure user has to go through 2FA.
|
||||||
assert.Equal(t, "/user/webauthn", test.RedirectURL(resp))
|
assert.Equal(t, "/user/webauthn", test.RedirectURL(resp))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestAccessTokenWithPKCE(t *testing.T) {
|
||||||
|
defer tests.PrepareTestEnv(t)()
|
||||||
|
|
||||||
|
var u *url.URL
|
||||||
|
t.Run("Grant", func(t *testing.T) {
|
||||||
|
session := loginUser(t, "user4")
|
||||||
|
req := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{
|
||||||
|
"_csrf": GetCSRF(t, session, "/login/oauth/authorize?client_id=ce5a1322-42a7-11ed-b878-0242ac120002&redirect_uri=b&response_type=code&code_challenge_method=plain&code_challenge=CODE&state=thestate"),
|
||||||
|
"client_id": "ce5a1322-42a7-11ed-b878-0242ac120002",
|
||||||
|
"redirect_uri": "b",
|
||||||
|
"state": "thestate",
|
||||||
|
"granted": "true",
|
||||||
|
})
|
||||||
|
resp := session.MakeRequest(t, req, http.StatusSeeOther)
|
||||||
|
|
||||||
|
var err error
|
||||||
|
u, err = url.Parse(test.RedirectURL(resp))
|
||||||
|
require.NoError(t, err)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Incorrect code verfifier", func(t *testing.T) {
|
||||||
|
req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
|
||||||
|
"client_id": "ce5a1322-42a7-11ed-b878-0242ac120002",
|
||||||
|
"code": u.Query().Get("code"),
|
||||||
|
"code_verifier": "just a guess",
|
||||||
|
"grant_type": "authorization_code",
|
||||||
|
"redirect_uri": "b",
|
||||||
|
})
|
||||||
|
resp := MakeRequest(t, req, http.StatusBadRequest)
|
||||||
|
|
||||||
|
var respBody map[string]any
|
||||||
|
DecodeJSON(t, resp, &respBody)
|
||||||
|
|
||||||
|
if assert.Len(t, respBody, 2) {
|
||||||
|
assert.Equal(t, "unauthorized_client", respBody["error"])
|
||||||
|
assert.Equal(t, "failed PKCE code challenge", respBody["error_description"])
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Get access token", func(t *testing.T) {
|
||||||
|
req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
|
||||||
|
"client_id": "ce5a1322-42a7-11ed-b878-0242ac120002",
|
||||||
|
"code": u.Query().Get("code"),
|
||||||
|
"code_verifier": "CODE",
|
||||||
|
"grant_type": "authorization_code",
|
||||||
|
"redirect_uri": "b",
|
||||||
|
})
|
||||||
|
resp := MakeRequest(t, req, http.StatusOK)
|
||||||
|
|
||||||
|
var respBody map[string]any
|
||||||
|
DecodeJSON(t, resp, &respBody)
|
||||||
|
|
||||||
|
if assert.Len(t, respBody, 4) {
|
||||||
|
assert.NotEmpty(t, respBody["access_token"])
|
||||||
|
assert.NotEmpty(t, respBody["token_type"])
|
||||||
|
assert.NotEmpty(t, respBody["expires_in"])
|
||||||
|
assert.NotEmpty(t, respBody["refresh_token"])
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue