Support for Custom URI Schemes in OAuth2 Redirect URIs

[wxiaoguang]

Fix #34349

https://gitea.com/gitea/docs/pulls/387

Diff with ignoring spaces: https://github.com/go-gitea/gitea/pull/37356/changes?w=1

By the way, remove (ctx *APIContext) HasAPIError() and (ctx *APIContext) GetErrMsg() because they do nothing, the error handling has been done in API's middeware

The existing OAuth2 tests were not quite right, refactored them together

[Copilot]

Pull request overview

This PR adds support for OAuth2 application redirect URIs that use custom URI schemes (for native/mobile/desktop app integrations), controlled by a new CUSTOM_SCHEMES setting, and wires this validation into both the web UI and API endpoints.

Changes:

• Introduces OAuth2.CustomSchemes configuration and documents it in app.example.ini.
• Replaces the previous redirect URI list validation with a new scheme-based validator shared by web UI and API.
• Updates Swagger responses and adjusts integration tests for the new behavior.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
services/forms/user_form.go	Adds redirect-URI validation helper and custom-scheme support for the settings UI form.
routers/api/v1/user/app.go	Adds redirect-URI validation to create/update OAuth2 application API endpoints.
modules/setting/oauth2.go	Adds CustomSchemes to OAuth2 settings struct.
custom/conf/app.example.ini	Documents new CUSTOM_SCHEMES configuration option.
modules/web/middleware/binding.go	Adds ReportValidationError helper for form validation reporting.
templates/swagger/v1_json.tmpl	Adds 400 response to an OAuth2 application endpoint in Swagger JSON.
tests/integration/user_settings_test.go	Updates assertions and adds coverage for custom scheme acceptance in UI.
modules/validation/binding.go / modules/validation/binding_test.go / modules/validation/validurllist_test.go	Removes the ValidUrlList binding rule and its tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/integration/oauth_test.go:512

• testRefreshTokenInvalidation mutates the global setting.OAuth2.InvalidateRefreshTokens but does not restore it. Since this file now runs many OAuth2 subtests under a single PrepareTestEnv, this can leak state into later subtests and make the suite order-dependent/flaky. Save the original value and defer restoring it (or use test.MockVariableValue) around these assignments.

	// test without invalidation
	setting.OAuth2.InvalidateRefreshTokens = false

	req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
		"grant_type": "refresh_token",
		"client_id":  "da7da3ba-9a13-4167-856f-3899de0b0138",
		// omit secret
		"redirect_uri":  "https://example.com",
		"refresh_token": parsed.RefreshToken,
	})
	resp = MakeRequest(t, req, http.StatusBadRequest)
	parsedError := new(oauth2_provider.AccessTokenError)
	assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
	assert.Equal(t, "invalid_client", string(parsedError.ErrorCode))
	assert.Equal(t, "invalid empty client secret", parsedError.ErrorDescription)

	req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
		"grant_type":    "refresh_token",
		"client_id":     "da7da3ba-9a13-4167-856f-3899de0b0138",
		"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
		"redirect_uri":  "https://example.com",
		"refresh_token": "UNEXPECTED",
	})
	resp = MakeRequest(t, req, http.StatusBadRequest)
	parsedError = new(oauth2_provider.AccessTokenError)
	assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError))
	assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
	assert.Equal(t, "unable to parse refresh token", parsedError.ErrorDescription)

	req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
		"grant_type":    "refresh_token",
		"client_id":     "da7da3ba-9a13-4167-856f-3899de0b0138",
		"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
		"redirect_uri":  "https://example.com",
		"refresh_token": parsed.RefreshToken,
	})

	bs, err := io.ReadAll(req.Body)
	assert.NoError(t, err)

	req.Body = io.NopCloser(bytes.NewReader(bs))
	MakeRequest(t, req, http.StatusOK)

	req.Body = io.NopCloser(bytes.NewReader(bs))
	MakeRequest(t, req, http.StatusOK)

	// test with invalidation
	setting.OAuth2.InvalidateRefreshTokens = true
	req.Body = io.NopCloser(bytes.NewReader(bs))

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[silverwind]

1. DetectInvalidOAuth2ApplicationRedirectURI uses strings.Cut(u, ":"), which accepts pathological inputs like "my-scheme:" or "x:" (empty remainder). Prefer url.Parse and require a non-empty opaque/host.
2. No test asserts that a scheme not in CustomSchemes (e.g. javascript:alert(1)) is rejected — worth adding since this is the security-critical path.
3. middleware.Validate now runs AssignForm even when errs.Len() == 0, whereas it previously returned early. This affects every form in the app — either restore the early return or call out the intentional change.
4. Dropping ValidUrlList means malformed http(s) URIs (bad ports, invalid IPv6, etc.) now pass validation since only the scheme token before : is checked. If intentional, worth documenting; otherwise run IsValidURL for http/https entries.

---

This comment was written with the help of Claude Opus 4.7

[wxiaoguang]

1. DetectInvalidOAuth2ApplicationRedirectURI uses strings.Cut(u, ":"), which accepts pathological inputs like "my-scheme:" or "x:" (empty remainder). Prefer url.Parse and require a non-empty opaque/host.

No need. No bad case

2. No test asserts that a scheme not in CustomSchemes (e.g. javascript:alert(1)) is rejected — worth adding since this is the security-critical path.

Site admin added it, they must like it. Just like they can run rm -rf /

3. middleware.Validate now runs AssignForm even when errs.Len() == 0, whereas it previously returned early. This affects every form in the app — either restore the early return or call out the intentional change.

What's wrong?

4. Dropping ValidUrlList means malformed http(s) URIs (bad ports, invalid IPv6, etc.) now pass validation since only the scheme token before : is checked. If intentional, worth documenting; otherwise run IsValidURL for http/https entries.

Totally useless.

[wxiaoguang]

2. No test asserts that a scheme not in CustomSchemes (e.g. javascript:alert(1)) is rejected — worth adding since this is the security-critical path.

For the "tests", maybe your AI is still blind.

[wxiaoguang]

4. Dropping ValidUrlList means malformed http(s) URIs (bad ports, invalid IPv6, etc.) now pass validation since only the scheme token before : is checked. If intentional, worth documenting; otherwise run IsValidURL for http/https entries.

Totally useless.

Although it is useless, in case some newbie would complain that "Oh, the URL I inputted is invalid", added the IsValidURL check in 787d61e