Skip to content

Commit 05decca

Browse files
jackHay22delvh
authored andcommitted
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](OAI/OpenAPI-Specification#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 <[email protected]>
1 parent ff3d047 commit 05decca

File tree

5 files changed

+39
-7
lines changed

5 files changed

+39
-7
lines changed

custom/conf/app.example.ini

+5
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,11 @@ INTERNAL_TOKEN=
492492
;; 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.
493493
;; This cache will store the successfully hashed tokens in a LRU cache as a balance between performance and security.
494494
;SUCCESSFUL_TOKENS_CACHE_SIZE = 20
495+
;;
496+
;; Reject API tokens sent in URL query string (Accept Header-based API tokens only). This avoids security vulnerabilities
497+
;; stemming from cached/logged plain-text API tokens.
498+
;; In future releases, this will become the default behavior
499+
;DISABLE_QUERY_AUTH_TOKEN = false
495500

496501
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
497502
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

modules/setting/security.go

+8
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ var (
3434
PasswordHashAlgo string
3535
PasswordCheckPwn bool
3636
SuccessfulTokensCacheSize int
37+
DisableQueryAuthToken bool
3738
CSRFCookieName = "_csrf"
3839
CSRFCookieHTTPOnly = true
3940
)
@@ -157,4 +158,11 @@ func loadSecurityFrom(rootCfg ConfigProvider) {
157158
PasswordComplexity = append(PasswordComplexity, name)
158159
}
159160
}
161+
162+
// TODO: default value should be true in future releases
163+
DisableQueryAuthToken = sec.Key("DISABLE_QUERY_AUTH_TOKEN").MustBool(false)
164+
165+
if !DisableQueryAuthToken {
166+
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.")
167+
}
160168
}

routers/api/v1/api.go

+11
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,12 @@
3535
// type: apiKey
3636
// name: token
3737
// in: query
38+
// description: This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead.
3839
// AccessToken:
3940
// type: apiKey
4041
// name: access_token
4142
// in: query
43+
// description: This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead.
4244
// AuthorizationHeaderToken:
4345
// type: apiKey
4446
// name: Authorization
@@ -788,6 +790,13 @@ func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.APIC
788790
}
789791
}
790792

793+
// check for and warn against deprecated authentication options
794+
func checkDeprecatedAuthMethods(ctx *context.APIContext) {
795+
if ctx.FormString("token") != "" || ctx.FormString("access_token") != "" {
796+
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.")
797+
}
798+
}
799+
791800
// Routes registers all v1 APIs routes to web application.
792801
func Routes() *web.Route {
793802
m := web.NewRoute()
@@ -806,6 +815,8 @@ func Routes() *web.Route {
806815
}
807816
m.Use(context.APIContexter())
808817

818+
m.Use(checkDeprecatedAuthMethods)
819+
809820
// Get user from session if logged in.
810821
m.Use(apiAuth(buildAuthGroup()))
811822

services/auth/oauth2.go

+13-7
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
auth_model "code.gitea.io/gitea/models/auth"
1515
user_model "code.gitea.io/gitea/models/user"
1616
"code.gitea.io/gitea/modules/log"
17+
"code.gitea.io/gitea/modules/setting"
1718
"code.gitea.io/gitea/modules/timeutil"
1819
"code.gitea.io/gitea/modules/web/middleware"
1920
"code.gitea.io/gitea/services/auth/source/oauth2"
@@ -62,14 +63,19 @@ func (o *OAuth2) Name() string {
6263
// representing whether the token exists or not
6364
func parseToken(req *http.Request) (string, bool) {
6465
_ = req.ParseForm()
65-
// Check token.
66-
if token := req.Form.Get("token"); token != "" {
67-
return token, true
68-
}
69-
// Check access token.
70-
if token := req.Form.Get("access_token"); token != "" {
71-
return token, true
66+
if !setting.DisableQueryAuthToken {
67+
// Check token.
68+
if token := req.Form.Get("token"); token != "" {
69+
return token, true
70+
}
71+
// Check access token.
72+
if token := req.Form.Get("access_token"); token != "" {
73+
return token, true
74+
}
75+
} else if req.Form.Get("token") != "" || req.Form.Get("access_token") != "" {
76+
log.Warn("API token sent in query string but DISABLE_QUERY_AUTH_TOKEN=true")
7277
}
78+
7379
// check header token
7480
if auHead := req.Header.Get("Authorization"); auHead != "" {
7581
auths := strings.Fields(auHead)

templates/swagger/v1_json.tmpl

+2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)