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>

diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini
index 665a1d96e20ce9d0736ea52a0bd3312effcd8ea5..7075ecff35aa01f6c27a9df581fa464c4fff08bf 100644 (file)
--- 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 90f614d4cd302a889ae112b1101867d1b420a935..efe0d7eea691dddf9e441721d9eafdff10b13b5c 100644 (file)
--- 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 0c8672f5aa7f1afd1ac44c970002704eea57629e..68d1f3ae3bf12091892805da8dddb2cfbe68769c 100644 (file)
--- 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 08a2a05539b5abca9662c6f0d687e28b55d28b94..f2f7858a850c1523e028b5cd77eb08e082f64619 100644 (file)
--- 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 2a726a77c629fc4c055d29e5ecc2bb2b758b1860..73f86bf8a61ddbe1331d721b114a0ca8987b7e72 100644 (file)
--- 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"