From 4e879fed90665331d2a57e5abee9e0f02372c470 Mon Sep 17 00:00:00 2001 From: Jack Hay Date: Mon, 11 Dec 2023 22:48:53 -0500 Subject: [PATCH] Deprecate query string auth tokens (#28390) ## 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: delvh --- 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 18d6fe37a8..e10c4f7582 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -492,6 +492,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 92caa05fad..4adfe20635 100644 --- a/modules/setting/security.go +++ b/modules/setting/security.go @@ -34,6 +34,7 @@ var ( PasswordHashAlgo string PasswordCheckPwn bool SuccessfulTokensCacheSize int + DisableQueryAuthToken bool CSRFCookieName = "_csrf" CSRFCookieHTTPOnly = true ) @@ -157,4 +158,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 623c798fee..13c6762b54 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 @@ -788,6 +790,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() @@ -806,6 +815,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 2541726a64..6cf2beafec 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -24046,6 +24046,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" @@ -24078,6 +24079,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"