From ad5c43ae5d90dc92a5ce173894c72b5f6c248bc0 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 13 Apr 2020 21:02:48 +0200 Subject: [PATCH] Reject duplicate AccessToken names (#10994) * make sure duplicate token names cannot be used * add check to api routes too * add @lunny s suggestion * fix & don't forget User.ID * AccessTokenByNameExists() return error too * unique token for each test * fix lint Signed-off-by: 6543 <6543@obermui.de> Co-authored-by: Lanre Adelowo --- integrations/integration_test.go | 6 ++++- models/token.go | 5 ++++ models/token_test.go | 36 ++++++++++++++++++++++++++++ options/locale/locale_en-US.ini | 1 + routers/api/v1/user/app.go | 12 ++++++++++ routers/user/setting/applications.go | 12 ++++++++++ 6 files changed, 71 insertions(+), 1 deletion(-) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index 138d751859..c6a4169751 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -330,14 +330,18 @@ func loginUserWithPassword(t testing.TB, userName, password string) *TestSession return session } +//token has to be unique this counter take care of +var tokenCounter int64 + func getTokenForLoggedInUser(t testing.TB, session *TestSession) string { t.Helper() + tokenCounter++ req := NewRequest(t, "GET", "/user/settings/applications") resp := session.MakeRequest(t, req, http.StatusOK) doc := NewHTMLParser(t, resp.Body) req = NewRequestWithValues(t, "POST", "/user/settings/applications", map[string]string{ "_csrf": doc.GetCSRF(), - "name": "api-testing-token", + "name": fmt.Sprintf("api-testing-token-%d", tokenCounter), }) resp = session.MakeRequest(t, req, http.StatusFound) req = NewRequest(t, "GET", "/user/settings/applications") diff --git a/models/token.go b/models/token.go index 7ad9d41676..71a9f0975f 100644 --- a/models/token.go +++ b/models/token.go @@ -77,6 +77,11 @@ func GetAccessTokenBySHA(token string) (*AccessToken, error) { return nil, ErrAccessTokenNotExist{token} } +// AccessTokenByNameExists checks if a token name has been used already by a user. +func AccessTokenByNameExists(token *AccessToken) (bool, error) { + return x.Table("access_token").Where("name = ?", token.Name).And("uid = ?", token.UID).Exist() +} + // ListAccessTokens returns a list of access tokens belongs to given user. func ListAccessTokens(uid int64, listOptions ListOptions) ([]*AccessToken, error) { sess := x. diff --git a/models/token_test.go b/models/token_test.go index 45f50a1b82..572a5de609 100644 --- a/models/token_test.go +++ b/models/token_test.go @@ -27,6 +27,42 @@ func TestNewAccessToken(t *testing.T) { assert.Error(t, NewAccessToken(invalidToken)) } +func TestAccessTokenByNameExists(t *testing.T) { + + name := "Token Gitea" + + assert.NoError(t, PrepareTestDatabase()) + token := &AccessToken{ + UID: 3, + Name: name, + } + + // Check to make sure it doesn't exists already + exist, err := AccessTokenByNameExists(token) + assert.NoError(t, err) + assert.False(t, exist) + + // Save it to the database + assert.NoError(t, NewAccessToken(token)) + AssertExistsAndLoadBean(t, token) + + // This token must be found by name in the DB now + exist, err = AccessTokenByNameExists(token) + assert.NoError(t, err) + assert.True(t, exist) + + user4Token := &AccessToken{ + UID: 4, + Name: name, + } + + // Name matches but different user ID, this shouldn't exists in the + // database + exist, err = AccessTokenByNameExists(user4Token) + assert.NoError(t, err) + assert.False(t, exist) +} + func TestGetAccessTokenBySHA(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) token, err := GetAccessTokenBySHA("d2c6c1ba3890b309189a8e618c72a162e4efbf36") diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a565a754af..9a1a458f66 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -517,6 +517,7 @@ new_token_desc = Applications using a token have full access to your account. token_name = Token Name generate_token = Generate Token generate_token_success = Your new token has been generated. Copy it now as it will not be shown again. +generate_token_name_duplicate = %s has been used as an application name already. Please use a new one. delete_token = Delete access_token_deletion = Delete Access Token access_token_deletion_desc = Deleting a token will revoke access to your account for applications using it. Continue? diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go index 9ec506bcf2..f29572ef62 100644 --- a/routers/api/v1/user/app.go +++ b/routers/api/v1/user/app.go @@ -6,6 +6,7 @@ package user import ( + "errors" "net/http" "code.gitea.io/gitea/models" @@ -89,6 +90,17 @@ func CreateAccessToken(ctx *context.APIContext, form api.CreateAccessTokenOption UID: ctx.User.ID, Name: form.Name, } + + exist, err := models.AccessTokenByNameExists(t) + if err != nil { + ctx.InternalServerError(err) + return + } + if exist { + ctx.Error(http.StatusBadRequest, "AccessTokenByNameExists", errors.New("access token name has been used already")) + return + } + if err := models.NewAccessToken(t); err != nil { ctx.Error(http.StatusInternalServerError, "NewAccessToken", err) return diff --git a/routers/user/setting/applications.go b/routers/user/setting/applications.go index e7bf612269..febb5b0c19 100644 --- a/routers/user/setting/applications.go +++ b/routers/user/setting/applications.go @@ -43,6 +43,18 @@ func ApplicationsPost(ctx *context.Context, form auth.NewAccessTokenForm) { UID: ctx.User.ID, Name: form.Name, } + + exist, err := models.AccessTokenByNameExists(t) + if err != nil { + ctx.ServerError("AccessTokenByNameExists", err) + return + } + if exist { + ctx.Flash.Error(ctx.Tr("settings.generate_token_name_duplicate", t.Name)) + ctx.Redirect(setting.AppSubURL + "/user/settings/applications") + return + } + if err := models.NewAccessToken(t); err != nil { ctx.ServerError("NewAccessToken", err) return