From 40bf161ff0e76ea737d7c2ec55f62f2bfe5d6222 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 5 Jun 2024 12:33:10 +0200 Subject: [PATCH 1/2] test(oauth): coverage for the redirection of a denied grant See 886a675f62233dcde3ac0d7b2181484f29344f26 Return `access_denied` error when an OAuth2 request is denied (cherry picked from commit 32c882af91cf0c0f68a486876680b38095fa53a0) --- release-notes/8.0.0/fix/4026.md | 1 + tests/integration/oauth_test.go | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 release-notes/8.0.0/fix/4026.md diff --git a/release-notes/8.0.0/fix/4026.md b/release-notes/8.0.0/fix/4026.md new file mode 100644 index 0000000000..747c3a789e --- /dev/null +++ b/release-notes/8.0.0/fix/4026.md @@ -0,0 +1 @@ +- when an OAuth grant request submitted to a Forgejo user is denied, the server from which the request originates is not notified that it has been denied diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 1da1c6f9c0..cffb8b5814 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -554,3 +554,24 @@ func TestSignUpViaOAuthWithMissingFields(t *testing.T) { resp := MakeRequest(t, req, http.StatusSeeOther) assert.Equal(t, test.RedirectURL(resp), "/user/link_account") } + +func TestOAuth_GrantApplicationOAuth(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + req := NewRequest(t, "GET", "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=a&response_type=code&state=thestate") + ctx := loginUser(t, "user4") + 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": "da7da3ba-9a13-4167-856f-3899de0b0138", + "redirect_uri": "a", + "state": "thestate", + "granted": "false", + }) + resp = ctx.MakeRequest(t, req, http.StatusSeeOther) + assert.Contains(t, test.RedirectURL(resp), "error=access_denied&error_description=the+request+is+denied") +} From d841e95191ddcc04a06dbf022a6b06884bf2bee6 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Mon, 20 May 2024 15:17:00 +0800 Subject: [PATCH 2/2] Return `access_denied` error when an OAuth2 request is denied (#30974) According to [RFC 6749](https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1), when the resource owner or authorization server denied an request, an `access_denied` error should be returned. But currently in this case Gitea does not return any error. For example, if the user clicks "Cancel" here, an `access_denied` error should be returned. (cherry picked from commit f1d9f18d96050d89a4085c961f572f07b1e653d1) (cherry picked from commit 886a675f62233dcde3ac0d7b2181484f29344f26) --- routers/web/auth/oauth.go | 10 ++++++++++ services/forms/user_form.go | 1 + templates/user/auth/grant.tmpl | 4 ++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index f5ca0bda5e..73fcd5524d 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -539,6 +539,16 @@ func GrantApplicationOAuth(ctx *context.Context) { ctx.Error(http.StatusBadRequest) return } + + if !form.Granted { + handleAuthorizeError(ctx, AuthorizeError{ + State: form.State, + ErrorDescription: "the request is denied", + ErrorCode: ErrorCodeAccessDenied, + }, form.RedirectURI) + return + } + app, err := auth.GetOAuth2ApplicationByClientID(ctx, form.ClientID) if err != nil { ctx.ServerError("GetOAuth2ApplicationByClientID", err) diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 4e603a3115..0b7bea4638 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -162,6 +162,7 @@ func (f *AuthorizationForm) Validate(req *http.Request, errs binding.Errors) bin // GrantApplicationForm form for authorizing oauth2 clients type GrantApplicationForm struct { ClientID string `binding:"Required"` + Granted bool RedirectURI string State string Scope string diff --git a/templates/user/auth/grant.tmpl b/templates/user/auth/grant.tmpl index cb9bba8749..a18a3bd27a 100644 --- a/templates/user/auth/grant.tmpl +++ b/templates/user/auth/grant.tmpl @@ -23,8 +23,8 @@ - - Cancel + +