tom/forkjo
tom/forkjo
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions 1

Merge pull request '[v7.0/forgejo] RFC 6749 Section 10.2 conformance' (#4046) from bp-v7.0/forgejo-5924694 into v7.0/forgejo

Browse source 
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4046
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
This commit is contained in:
Earl Warren 2024-06-06 11:55:55 +00:00
commit 874dde0d4c
3 changed files with 81 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
models/fixtures/oauth2_application.yml
View file

@ -14,7 +14,7 @@
name: "Test native app"
client_id: "ce5a1322-42a7-11ed-b878-0242ac120002"
client_secret: "$2a$10$UYRgUSgekzBp6hYe8pAdc.cgB4Gn06QRKsORUnIYTYQADs.YR/uvi" # bcrypt of "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=
redirect_uris: '["http://127.0.0.1"]'
redirect_uris: '["b", "http://127.0.0.1"]'
created_unix: 1546869730
updated_unix: 1546869730
confidential_client: false

22
routers/web/auth/oauth.go
View file

@ -470,8 +470,9 @@ func AuthorizeOAuth(ctx *context.Context) {
return
}
// Redirect if user already granted access
if grant != nil {
// Redirect if user already granted access and the application is confidential.
// I.e. always require authorization for public clients as recommended by RFC 6749 Section 10.2
if app.ConfidentialClient && grant != nil {
code, err := grant.GenerateNewAuthorizationCode(ctx, form.RedirectURI, form.CodeChallenge, form.CodeChallengeMethod)
if err != nil {
handleServerError(ctx, form.State, form.RedirectURI)
@ -555,7 +556,13 @@ func GrantApplicationOAuth(ctx *context.Context) {
ctx.ServerError("GetOAuth2ApplicationByClientID", err)
return
}
grant, err := app.CreateGrant(ctx, ctx.Doer.ID, form.Scope)
grant, err := app.GetGrantByUserID(ctx, ctx.Doer.ID)
if err != nil {
handleServerError(ctx, form.State, form.RedirectURI)
return
}
if grant == nil {
grant, err = app.CreateGrant(ctx, ctx.Doer.ID, form.Scope)
if err != nil {
handleAuthorizeError(ctx, AuthorizeError{
State: form.State,
@ -564,6 +571,15 @@ func GrantApplicationOAuth(ctx *context.Context) {
}, form.RedirectURI)
return
}
} else if grant.Scope != form.Scope {
handleAuthorizeError(ctx, AuthorizeError{
State: form.State,
ErrorDescription: "a grant exists with different scope",
ErrorCode: ErrorCodeServerError,
}, form.RedirectURI)
return
}
if len(form.Nonce) > 0 {
err := grant.SetNonce(ctx, form.Nonce)
if err != nil {

61
tests/integration/oauth_test.go
View file

@ -79,6 +79,65 @@ func TestAuthorizeShow(t *testing.T) {
htmlDoc.GetCSRF()
}
func TestOAuth_AuthorizeConfidentialTwice(t *testing.T) {
defer tests.PrepareTestEnv(t)()
// da7da3ba-9a13-4167-856f-3899de0b0138 a confidential client in models/fixtures/oauth2_application.yml
// request authorization for the first time shows the grant page ...
authorizeURL := "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=a&response_type=code&state=thestate"
req := NewRequest(t, "GET", authorizeURL)
ctx := loginUser(t, "user4")
resp := ctx.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
htmlDoc.AssertElement(t, "#authorize-app", true)
// ... and the user grants the authorization
req = NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{
"_csrf": htmlDoc.GetCSRF(),
"client_id": "da7da3ba-9a13-4167-856f-3899de0b0138",
"redirect_uri": "a",
"state": "thestate",
"granted": "true",
})
resp = ctx.MakeRequest(t, req, http.StatusSeeOther)
assert.Contains(t, test.RedirectURL(resp), "code=")
// request authorization the second time and the grant page is not shown again, redirection happens immediately
req = NewRequest(t, "GET", authorizeURL)
resp = ctx.MakeRequest(t, req, http.StatusSeeOther)
assert.Contains(t, test.RedirectURL(resp), "code=")
}
func TestOAuth_AuthorizePublicTwice(t *testing.T) {
defer tests.PrepareTestEnv(t)()
// ce5a1322-42a7-11ed-b878-0242ac120002 is a public client in models/fixtures/oauth2_application.yml
authorizeURL := "/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"
ctx := loginUser(t, "user4")
// a public client must be authorized every time
for _, name := range []string{"First", "Second"} {
t.Run(name, func(t *testing.T) {
req := NewRequest(t, "GET", authorizeURL)
resp := ctx.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
htmlDoc.AssertElement(t, "#authorize-app", true)
req = NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{
"_csrf": htmlDoc.GetCSRF(),
"client_id": "ce5a1322-42a7-11ed-b878-0242ac120002",
"redirect_uri": "b",
"state": "thestate",
"granted": "true",
})
resp = ctx.MakeRequest(t, req, http.StatusSeeOther)
assert.Contains(t, test.RedirectURL(resp), "code=")
})
}
}
func TestAuthorizeRedirectWithExistingGrant(t *testing.T) {
defer tests.PrepareTestEnv(t)()
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")
@ -480,7 +539,7 @@ func TestSignInOAuthCallbackRedirectToEscaping(t *testing.T) {
gitlab := addAuthSource(t, authSourcePayloadGitLabCustom(gitlabName))
//
// Create a user as if it had been previously been created by the GitLab
// Create a user as if it had been previously created by the GitLab
// authentication source.
//
userGitLabUserID := "5678"