Skip to content

DISABLE_ACCESS_TOKENS parameter for disabling access tokens added #18488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

DISABLE_ACCESS_TOKENS parameter for disabling access tokens added #18488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2bde83b
DISABLE_ACCESS_TOKENS parameter for disabling access tokens added
pboguslawski Jan 31, 2022
7a5e483
Missing negation added
pboguslawski Jan 31, 2022
3de67bf
Disabling existing access tokens added
pboguslawski Feb 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions custom/conf/app.example.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,9 @@ INTERNAL_TOKEN=
;; Set to true to disable webhooks feature.
;DISABLE_WEBHOOKS = false
;;
;; Set to false to disable access tokens feature.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a comment here that it should be changed only if using some kind of SSO and that it might break existing integrations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a comment here that it should be changed only if using some kind of SSO and that it might break existing integrations.

If someone is touching such settings, should test it before in nonprod env. Describing all possible scenarios here (like SSO/noSSO, only basic auth without tokens, etc.) does not make sense IHMO.

;DISABLE_ACCESS_TOKENS = false
;;
;; Set to false to allow pushes to gitea repositories despite having an incomplete environment - NOT RECOMMENDED
;ONLY_ALLOW_PUSH_IF_GITEA_ENVIRONMENT_SET = true
;;
Expand Down
1 change: 1 addition & 0 deletions docs/content/doc/advanced/config-cheat-sheet.en-us.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ Certain queues have defaults that override the defaults set in `[queue]` (this o
Gitea instance and perform arbitrary actions in the name of the Gitea OS user.
This maybe harmful to you website or your operating system.
- `DISABLE_WEBHOOKS`: **false**: Set to `true` to disable webhooks feature.
- `DISABLE_ACCESS_TOKENS`: **false**: Set to `true` to disable access tokens feature.
- `ONLY_ALLOW_PUSH_IF_GITEA_ENVIRONMENT_SET`: **true**: Set to `false` to allow local users to push to gitea-repositories without setting up the Gitea environment. This is not recommended and if you want local users to push to Gitea repositories you should set the environment appropriately.
- `IMPORT_LOCAL_PATHS`: **false**: Set to `false` to prevent all users (including admin) from importing local path on server.
- `INTERNAL_TOKEN`: **\<random at every install if no uri set\>**: Secret used to validate communication within Gitea binary.
Expand Down
6 changes: 6 additions & 0 deletions models/token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ func GetAccessTokenBySHA(token string) (*AccessToken, error) {
if token == "" {
return nil, ErrAccessTokenEmpty{}
}

// Existing tokens are invalid if access tokens feature is disabled.
if setting.DisableAccessTokens {
return nil, ErrAccessTokenNotExist{token}
}

// A token is defined as being SHA1 sum these are 40 hexadecimal bytes long
if len(token) != 40 {
return nil, ErrAccessTokenNotExist{token}
Expand Down
2 changes: 2 additions & 0 deletions modules/setting/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ var (
ImportLocalPaths bool
DisableGitHooks bool
DisableWebhooks bool
DisableAccessTokens bool
OnlyAllowPushIfGiteaEnvironmentSet bool
PasswordComplexity []string
PasswordHashAlgo string
Expand Down Expand Up @@ -868,6 +869,7 @@ func loadFromConf(allowEmpty bool, extraConfig string) {
ImportLocalPaths = sec.Key("IMPORT_LOCAL_PATHS").MustBool(false)
DisableGitHooks = sec.Key("DISABLE_GIT_HOOKS").MustBool(true)
DisableWebhooks = sec.Key("DISABLE_WEBHOOKS").MustBool(false)
DisableAccessTokens = sec.Key("DISABLE_ACCESS_TOKENS").MustBool(false)
OnlyAllowPushIfGiteaEnvironmentSet = sec.Key("ONLY_ALLOW_PUSH_IF_GITEA_ENVIRONMENT_SET").MustBool(true)
PasswordHashAlgo = sec.Key("PASSWORD_HASH_ALGO").MustString("pbkdf2")
CSRFCookieHTTPOnly = sec.Key("CSRF_COOKIE_HTTP_ONLY").MustBool(true)
Expand Down
6 changes: 6 additions & 0 deletions modules/templates/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,12 @@ func NewFuncMap() []template.FuncMap {
"DisableWebhooks": func() bool {
return setting.DisableWebhooks
},
"DisableAccessTokens": func() bool {
return setting.DisableAccessTokens
},
"DisableOAuth2": func() bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't mention anything about OAuth2 in this PR, what gives?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is your potential reasoning and use-case for wanting to have these disabled?

If someone want to disable some auth options (like tokens) - should be allowed to do it (i.e. for security reasons). Other stuff like OAuth2 has switches.

Realistically, almost no one will want to have it disabled as it breaks integration of almost any external service (including all CI/CD)

Think of systems that are isolated from all other apps and allows only authenticated user access (i.e. for security reasons).

All optional stuff in gitea should have switch to disable it. Don't try to be smarter than app owner.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find your security argument not convincing, considering that token authentication is far more secure than username/password combination for Basic Auth. Can you elaborate exactly how disabling this brings improved security over users being forced to specify (and remember) their username/password combinations in Git clients?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate exactly how disabling this brings improved security over users being forced to specify (and remember) their username/password combinations in Git clients?

Think of authentication with HTTP header using reverse proxy. In this scenario one may want to authenticate users ONLY with HTTP header from proxy and disable all other auth stuff (like ssh keys, passwords, basic auths and other shiny toys). Now its proxy job to make auth secure (SSO for example using secure auth methods).

Copy link
Contributor Author

@pboguslawski pboguslawski Feb 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Git clients are already smart enough to use more advanced auth methods than username+password. Tokens should be left available as option of course if one needs it (i.e. external app that understands tokens only).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think of authentication with HTTP header using reverse proxy. In this scenario one may want to authenticate users ONLY with HTTP header from proxy and disable all other auth stuff (like ssh keys, passwords, basic auths and other shiny toys). Now its proxy job to make auth secure (SSO for example using secure auth methods).

Assuming that Gitea can authenticate Git requests this way it would be a valid scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can.

return !setting.OAuth2.Enable
},
"DisableImportLocal": func() bool {
return !setting.ImportLocalPaths
},
Expand Down
23 changes: 18 additions & 5 deletions routers/web/user/setting/applications.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package setting

import (
"fmt"
"net/http"

"code.gitea.io/gitea/models"
Expand Down Expand Up @@ -44,6 +45,11 @@ func ApplicationsPost(ctx *context.Context) {
return
}

if setting.DisableAccessTokens {
ctx.ServerError("AccessToken", fmt.Errorf("cannot modify access token; access tokens disabled"))
return
}

t := &models.AccessToken{
UID: ctx.User.ID,
Name: form.Name,
Expand Down Expand Up @@ -73,6 +79,10 @@ func ApplicationsPost(ctx *context.Context) {

// DeleteApplication response for delete user access token
func DeleteApplication(ctx *context.Context) {
if setting.DisableAccessTokens {
ctx.ServerError("DeleteAccessToken", fmt.Errorf("cannot delete access token; access tokens disabled"))
return
}
if err := models.DeleteAccessTokenByID(ctx.FormInt64("id"), ctx.User.ID); err != nil {
ctx.Flash.Error("DeleteAccessTokenByID: " + err.Error())
} else {
Expand All @@ -85,14 +95,17 @@ func DeleteApplication(ctx *context.Context) {
}

func loadApplicationsData(ctx *context.Context) {
tokens, err := models.ListAccessTokens(models.ListAccessTokensOptions{UserID: ctx.User.ID})
if err != nil {
ctx.ServerError("ListAccessTokens", err)
return
if !setting.DisableAccessTokens {
tokens, err := models.ListAccessTokens(models.ListAccessTokensOptions{UserID: ctx.User.ID})
if err != nil {
ctx.ServerError("ListAccessTokens", err)
return
}
ctx.Data["Tokens"] = tokens
}
ctx.Data["Tokens"] = tokens
ctx.Data["EnableOAuth2"] = setting.OAuth2.Enable
if setting.OAuth2.Enable {
var err error
ctx.Data["Applications"], err = auth.GetOAuth2ApplicationsByUserID(ctx.User.ID)
if err != nil {
ctx.ServerError("GetOAuth2ApplicationsByUserID", err)
Expand Down
2 changes: 2 additions & 0 deletions templates/user/settings/applications.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
{{template "user/settings/navbar" .}}
<div class="ui container">
{{template "base/alert" .}}
{{if not DisableAccessTokens}}
<h4 class="ui top attached header">
{{.i18n.Tr "settings.manage_access_token"}}
</h4>
Expand Down Expand Up @@ -46,6 +47,7 @@
</button>
</form>
</div>
{{end}}

{{if .EnableOAuth2}}
{{template "user/settings/grants_oauth2" .}}
Expand Down
2 changes: 2 additions & 0 deletions templates/user/settings/navbar.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
<a class="{{if .PageIsSettingsSecurity}}active{{end}} item" href="{{AppSubUrl}}/user/settings/security">
{{.i18n.Tr "settings.security"}}
</a>
{{if or (not DisableAccessTokens) (not DisableOAuth2)}}
<a class="{{if .PageIsSettingsApplications}}active{{end}} item" href="{{AppSubUrl}}/user/settings/applications">
{{.i18n.Tr "settings.applications"}}
</a>
{{end}}
<a class="{{if .PageIsSettingsKeys}}active{{end}} item" href="{{AppSubUrl}}/user/settings/keys">
{{.i18n.Tr "settings.ssh_gpg_keys"}}
</a>
Expand Down