Skip to content

Commit 6416f06

Browse files
authored
Fix ssh deploy and user key constraints (#1357) (#5939) (#5966)
Backport of #5939 1. A key can either be an ssh user key or a deploy key. It cannot be both. 2. If a key is a user key - it can only be associated with one user. 3. If a key is a deploy key - it can be used in multiple repositories and the permissions it has on those repositories can be different. 4. If a repository is deleted, its deploy keys must be deleted too. We currently don't enforce any of this and multiple repositories access with different permissions doesn't work at all. This PR enforces the following constraints: - [x] You should not be able to add the same user key as another user - [x] You should not be able to add a ssh user key which is being used as a deploy key - [x] You should not be able to add a ssh deploy key which is being used as a user key - [x] If you add an ssh deploy key to another repository you should be able to use it in different modes without losing the ability to use it in the other mode. - [x] If you delete a repository you must delete all its deploy keys. Fix #1357
1 parent 1a8ab63 commit 6416f06

File tree

14 files changed

+694
-342
lines changed

14 files changed

+694
-342
lines changed

cmd/serv.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -234,19 +234,20 @@ func runServ(c *cli.Context) error {
234234

235235
// Check deploy key or user key.
236236
if key.Type == models.KeyTypeDeploy {
237-
if key.Mode < requestedMode {
238-
fail("Key permission denied", "Cannot push with deployment key: %d", key.ID)
239-
}
240-
241-
// Check if this deploy key belongs to current repository.
242-
has, err := private.HasDeployKey(key.ID, repo.ID)
237+
// Now we have to get the deploy key for this repo
238+
deployKey, err := private.GetDeployKey(key.ID, repo.ID)
243239
if err != nil {
244240
fail("Key access denied", "Failed to access internal api: [key_id: %d, repo_id: %d]", key.ID, repo.ID)
245241
}
246-
if !has {
242+
243+
if deployKey == nil {
247244
fail("Key access denied", "Deploy key access denied: [key_id: %d, repo_id: %d]", key.ID, repo.ID)
248245
}
249246

247+
if deployKey.Mode < requestedMode {
248+
fail("Key permission denied", "Cannot push with read-only deployment key: %d to repo_id: %d", key.ID, repo.ID)
249+
}
250+
250251
// Update deploy key activity.
251252
if err = private.UpdateDeployKeyUpdated(key.ID, repo.ID); err != nil {
252253
fail("Internal error", "UpdateDeployKey: %v", err)
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
// Copyright 2019 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package integrations
6+
7+
import (
8+
"fmt"
9+
"io/ioutil"
10+
"net/http"
11+
"testing"
12+
13+
api "code.gitea.io/sdk/gitea"
14+
"github.com/stretchr/testify/assert"
15+
)
16+
17+
type APITestContext struct {
18+
Reponame string
19+
Session *TestSession
20+
Token string
21+
Username string
22+
ExpectedCode int
23+
}
24+
25+
func NewAPITestContext(t *testing.T, username, reponame string) APITestContext {
26+
session := loginUser(t, username)
27+
token := getTokenForLoggedInUser(t, session)
28+
return APITestContext{
29+
Session: session,
30+
Token: token,
31+
Username: username,
32+
Reponame: reponame,
33+
}
34+
}
35+
36+
func (ctx APITestContext) GitPath() string {
37+
return fmt.Sprintf("%s/%s.git", ctx.Username, ctx.Reponame)
38+
}
39+
40+
func doAPICreateRepository(ctx APITestContext, empty bool, callback ...func(*testing.T, api.Repository)) func(*testing.T) {
41+
return func(t *testing.T) {
42+
createRepoOption := &api.CreateRepoOption{
43+
AutoInit: !empty,
44+
Description: "Temporary repo",
45+
Name: ctx.Reponame,
46+
Private: true,
47+
Gitignores: "",
48+
License: "WTFPL",
49+
Readme: "Default",
50+
}
51+
req := NewRequestWithJSON(t, "POST", "/api/v1/user/repos?token="+ctx.Token, createRepoOption)
52+
if ctx.ExpectedCode != 0 {
53+
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
54+
return
55+
}
56+
resp := ctx.Session.MakeRequest(t, req, http.StatusCreated)
57+
58+
var repository api.Repository
59+
DecodeJSON(t, resp, &repository)
60+
if len(callback) > 0 {
61+
callback[0](t, repository)
62+
}
63+
}
64+
}
65+
66+
func doAPIGetRepository(ctx APITestContext, callback ...func(*testing.T, api.Repository)) func(*testing.T) {
67+
return func(t *testing.T) {
68+
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s?token=%s", ctx.Username, ctx.Reponame, ctx.Token)
69+
70+
req := NewRequest(t, "GET", urlStr)
71+
if ctx.ExpectedCode != 0 {
72+
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
73+
return
74+
}
75+
resp := ctx.Session.MakeRequest(t, req, http.StatusOK)
76+
77+
var repository api.Repository
78+
DecodeJSON(t, resp, &repository)
79+
if len(callback) > 0 {
80+
callback[0](t, repository)
81+
}
82+
}
83+
}
84+
85+
func doAPIDeleteRepository(ctx APITestContext) func(*testing.T) {
86+
return func(t *testing.T) {
87+
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s?token=%s", ctx.Username, ctx.Reponame, ctx.Token)
88+
89+
req := NewRequest(t, "DELETE", urlStr)
90+
if ctx.ExpectedCode != 0 {
91+
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
92+
return
93+
}
94+
ctx.Session.MakeRequest(t, req, http.StatusNoContent)
95+
}
96+
}
97+
98+
func doAPICreateUserKey(ctx APITestContext, keyname, keyFile string, callback ...func(*testing.T, api.PublicKey)) func(*testing.T) {
99+
return func(t *testing.T) {
100+
urlStr := fmt.Sprintf("/api/v1/user/keys?token=%s", ctx.Token)
101+
102+
dataPubKey, err := ioutil.ReadFile(keyFile + ".pub")
103+
assert.NoError(t, err)
104+
req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateKeyOption{
105+
Title: keyname,
106+
Key: string(dataPubKey),
107+
})
108+
if ctx.ExpectedCode != 0 {
109+
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
110+
return
111+
}
112+
resp := ctx.Session.MakeRequest(t, req, http.StatusCreated)
113+
var publicKey api.PublicKey
114+
DecodeJSON(t, resp, &publicKey)
115+
if len(callback) > 0 {
116+
callback[0](t, publicKey)
117+
}
118+
}
119+
}
120+
121+
func doAPIDeleteUserKey(ctx APITestContext, keyID int64) func(*testing.T) {
122+
return func(t *testing.T) {
123+
urlStr := fmt.Sprintf("/api/v1/user/keys/%d?token=%s", keyID, ctx.Token)
124+
125+
req := NewRequest(t, "DELETE", urlStr)
126+
if ctx.ExpectedCode != 0 {
127+
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
128+
return
129+
}
130+
ctx.Session.MakeRequest(t, req, http.StatusNoContent)
131+
}
132+
}
133+
134+
func doAPICreateDeployKey(ctx APITestContext, keyname, keyFile string, readOnly bool) func(*testing.T) {
135+
return func(t *testing.T) {
136+
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/keys?token=%s", ctx.Username, ctx.Reponame, ctx.Token)
137+
138+
dataPubKey, err := ioutil.ReadFile(keyFile + ".pub")
139+
assert.NoError(t, err)
140+
req := NewRequestWithJSON(t, "POST", urlStr, api.CreateKeyOption{
141+
Title: keyname,
142+
Key: string(dataPubKey),
143+
ReadOnly: readOnly,
144+
})
145+
146+
if ctx.ExpectedCode != 0 {
147+
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
148+
return
149+
}
150+
ctx.Session.MakeRequest(t, req, http.StatusCreated)
151+
}
152+
}

integrations/deploy_key_push_test.go

Lines changed: 0 additions & 160 deletions
This file was deleted.

0 commit comments

Comments
 (0)