From f144521aea0d7a08b9bd5f17e49bae4021bd7a45 Mon Sep 17 00:00:00 2001
From: Giteabot <teabot@gitea.io>
Date: Tue, 12 Dec 2023 13:45:00 +0800
Subject: [PATCH] Deprecate query string auth tokens (#28390) (#28430)

Backport #28390 by @jackHay22

## Changes
- Add deprecation warning to `Token` and `AccessToken` authentication
methods in swagger.
- Add deprecation warning header to API response. Example:
  ```
  HTTP/1.1 200 OK
  ...
  Warning: token and access_token API authentication is deprecated
  ...
  ```
- Add setting `DISABLE_QUERY_AUTH_TOKEN` to reject query string auth
tokens entirely. Default is `false`

## Next steps
- `DISABLE_QUERY_AUTH_TOKEN` should be true in a subsequent release and
the methods should be removed in swagger
- `DISABLE_QUERY_AUTH_TOKEN` should be removed and the implementation of
the auth methods in question should be removed

## Open questions
- Should there be further changes to the swagger documentation?
Deprecation is not yet supported for security definitions (coming in
[OpenAPI Spec version
3.2.0](https://github.com/OAI/OpenAPI-Specification/issues/2506))
- Should the API router logger sanitize urls that use `token` or
`access_token`? (This is obviously an insufficient solution on its own)

Co-authored-by: Jack Hay <jack@allspice.io>
Co-authored-by: delvh <dev.lh@web.de>
---
 custom/conf/app.example.ini    |  5 +++++
 modules/setting/security.go    |  8 ++++++++
 routers/api/v1/api.go          | 11 +++++++++++
 services/auth/oauth2.go        | 20 +++++++++++++-------
 templates/swagger/v1_json.tmpl |  2 ++
 5 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini
index 665a1d96e2..7075ecff35 100644
--- a/custom/conf/app.example.ini
+++ b/custom/conf/app.example.ini
@@ -491,6 +491,11 @@ INTERNAL_TOKEN=
 ;; Cache successful token hashes. API tokens are stored in the DB as pbkdf2 hashes however, this means that there is a potentially significant hashing load when there are multiple API operations.
 ;; This cache will store the successfully hashed tokens in a LRU cache as a balance between performance and security.
 ;SUCCESSFUL_TOKENS_CACHE_SIZE = 20
+;;
+;; Reject API tokens sent in URL query string (Accept Header-based API tokens only). This avoids security vulnerabilities
+;; stemming from cached/logged plain-text API tokens.
+;; In future releases, this will become the default behavior
+;DISABLE_QUERY_AUTH_TOKEN = false
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
diff --git a/modules/setting/security.go b/modules/setting/security.go
index 90f614d4cd..efe0d7eea6 100644
--- a/modules/setting/security.go
+++ b/modules/setting/security.go
@@ -35,6 +35,7 @@ var (
 	PasswordHashAlgo                   string
 	PasswordCheckPwn                   bool
 	SuccessfulTokensCacheSize          int
+	DisableQueryAuthToken              bool
 	CSRFCookieName                     = "_csrf"
 	CSRFCookieHTTPOnly                 = true
 )
@@ -159,4 +160,11 @@ func loadSecurityFrom(rootCfg ConfigProvider) {
 			PasswordComplexity = append(PasswordComplexity, name)
 		}
 	}
+
+	// TODO: default value should be true in future releases
+	DisableQueryAuthToken = sec.Key("DISABLE_QUERY_AUTH_TOKEN").MustBool(false)
+
+	if !DisableQueryAuthToken {
+		log.Warn("Enabling Query API Auth tokens is not recommended. DISABLE_QUERY_AUTH_TOKEN will default to true in gitea 1.23 and will be removed in gitea 1.24.")
+	}
 }
diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
index 0c8672f5aa..68d1f3ae3b 100644
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@@ -35,10 +35,12 @@
 //	     type: apiKey
 //	     name: token
 //	     in: query
+//	     description: This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead.
 //	AccessToken:
 //	     type: apiKey
 //	     name: access_token
 //	     in: query
+//	     description: This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead.
 //	AuthorizationHeaderToken:
 //	     type: apiKey
 //	     name: Authorization
@@ -787,6 +789,13 @@ func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.APIC
 	}
 }
 
+// check for and warn against deprecated authentication options
+func checkDeprecatedAuthMethods(ctx *context.APIContext) {
+	if ctx.FormString("token") != "" || ctx.FormString("access_token") != "" {
+		ctx.Resp.Header().Set("Warning", "token and access_token API authentication is deprecated and will be removed in gitea 1.23. Please use AuthorizationHeaderToken instead. Existing queries will continue to work but without authorization.")
+	}
+}
+
 // Routes registers all v1 APIs routes to web application.
 func Routes() *web.Route {
 	m := web.NewRoute()
@@ -805,6 +814,8 @@ func Routes() *web.Route {
 	}
 	m.Use(context.APIContexter())
 
+	m.Use(checkDeprecatedAuthMethods)
+
 	// Get user from session if logged in.
 	m.Use(apiAuth(buildAuthGroup()))
 
diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go
index 08a2a05539..f2f7858a85 100644
--- a/services/auth/oauth2.go
+++ b/services/auth/oauth2.go
@@ -14,6 +14,7 @@ import (
 	auth_model "code.gitea.io/gitea/models/auth"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/web/middleware"
 	"code.gitea.io/gitea/services/auth/source/oauth2"
@@ -62,14 +63,19 @@ func (o *OAuth2) Name() string {
 // representing whether the token exists or not
 func parseToken(req *http.Request) (string, bool) {
 	_ = req.ParseForm()
-	// Check token.
-	if token := req.Form.Get("token"); token != "" {
-		return token, true
-	}
-	// Check access token.
-	if token := req.Form.Get("access_token"); token != "" {
-		return token, true
+	if !setting.DisableQueryAuthToken {
+		// Check token.
+		if token := req.Form.Get("token"); token != "" {
+			return token, true
+		}
+		// Check access token.
+		if token := req.Form.Get("access_token"); token != "" {
+			return token, true
+		}
+	} else if req.Form.Get("token") != "" || req.Form.Get("access_token") != "" {
+		log.Warn("API token sent in query string but DISABLE_QUERY_AUTH_TOKEN=true")
 	}
+
 	// check header token
 	if auHead := req.Header.Get("Authorization"); auHead != "" {
 		auths := strings.Fields(auHead)
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index 2a726a77c6..73f86bf8a6 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -23955,6 +23955,7 @@
   },
   "securityDefinitions": {
     "AccessToken": {
+      "description": "This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead.",
       "type": "apiKey",
       "name": "access_token",
       "in": "query"
@@ -23987,6 +23988,7 @@
       "in": "header"
     },
     "Token": {
+      "description": "This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead.",
       "type": "apiKey",
       "name": "token",
       "in": "query"