Skip to content

Writable deploy keys (closes #671) #3225

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.

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

Merged
merged 16 commits into from
Jan 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
53 changes: 53 additions & 0 deletions integrations/api_keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
package integrations

import (
"fmt"
"net/http"
"testing"

"code.gitea.io/gitea/models"
api "code.gitea.io/sdk/gitea"
)

Expand Down Expand Up @@ -37,3 +39,54 @@ func TestDeleteDeployKeyNoLogin(t *testing.T) {
req := NewRequest(t, "DELETE", "/api/v1/repos/user2/repo1/keys/1")
MakeRequest(t, req, http.StatusUnauthorized)
}

func TestCreateReadOnlyDeployKey(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Thank you very much for adding tests!

However, our suite of integration tests is becoming quite unwieldy. Would you consider instead writing equivalent unit tests for the DeployKeysPosthandler? See routers/repo/issue_label_test.go for some examples.

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 wrote the unit tests, but I don't know if it's ok to dump the integration tests.
Those are testing the API and the unit tests the interface.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, let's leave the integration tests. Eventually, it might make sense to convert our API integration tests to unit tests, but that's for another day. Thanks for adding unit tests!

prepareTestEnv(t)
repo := models.AssertExistsAndLoadBean(t, &models.Repository{Name: "repo1"}).(*models.Repository)
repoOwner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User)

session := loginUser(t, repoOwner.Name)

keysURL := fmt.Sprintf("/api/v1/repos/%s/%s/keys", repoOwner.Name, repo.Name)
rawKeyBody := api.CreateKeyOption{
Title: "read-only",
Key: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n",
ReadOnly: true,
}
req := NewRequestWithJSON(t, "POST", keysURL, rawKeyBody)
resp := session.MakeRequest(t, req, http.StatusCreated)

var newDeployKey api.DeployKey
DecodeJSON(t, resp, &newDeployKey)
models.AssertExistsAndLoadBean(t, &models.DeployKey{
ID: newDeployKey.ID,
Name: rawKeyBody.Title,
Content: rawKeyBody.Key,
Mode: models.AccessModeRead,
})
}

func TestCreateReadWriteDeployKey(t *testing.T) {
prepareTestEnv(t)
repo := models.AssertExistsAndLoadBean(t, &models.Repository{Name: "repo1"}).(*models.Repository)
repoOwner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User)

session := loginUser(t, repoOwner.Name)

keysURL := fmt.Sprintf("/api/v1/repos/%s/%s/keys", repoOwner.Name, repo.Name)
rawKeyBody := api.CreateKeyOption{
Title: "read-write",
Key: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDsufOCrDDlT8DLkodnnJtbq7uGflcPae7euTfM+Laq4So+v4WeSV362Rg0O/+Sje1UthrhN6lQkfRkdWIlCRQEXg+LMqr6RhvDfZquE2Xwqv/itlz7LjbdAUdYoO1iH7rMSmYvQh4WEnC/DAacKGbhdGIM/ZBz0z6tHm7bPgbI9ykEKekTmPwQFP1Qebvf5NYOFMWqQ2sCEAI9dBMVLoojsIpV+KADf+BotiIi8yNfTG2rzmzpxBpW9fYjd1Sy1yd4NSUpoPbEJJYJ1TrjiSWlYOVq9Ar8xW1O87i6gBjL/3zN7ANeoYhaAXupdOS6YL22YOK/yC0tJtXwwdh/eSrh",
}
req := NewRequestWithJSON(t, "POST", keysURL, rawKeyBody)
resp := session.MakeRequest(t, req, http.StatusCreated)

var newDeployKey api.DeployKey
DecodeJSON(t, resp, &newDeployKey)
models.AssertExistsAndLoadBean(t, &models.DeployKey{
ID: newDeployKey.ID,
Name: rawKeyBody.Title,
Content: rawKeyBody.Key,
Mode: models.AccessModeWrite,
})
}
1 change: 1 addition & 0 deletions models/fixtures/deploy_key.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[] # empty
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ var migrations = []Migration{
NewMigration("add reactions", addReactions),
// v54 -> v55
NewMigration("add pull request options", addPullRequestOptions),
// v55 -> v56
NewMigration("add writable deploy keys", addModeToDeploKeys),
}

// Migrate database to current version
Expand Down
23 changes: 23 additions & 0 deletions models/migrations/v55.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2018 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package migrations

import (
"fmt"

"code.gitea.io/gitea/models"
"github.com/go-xorm/xorm"
)

func addModeToDeploKeys(x *xorm.Engine) error {
type DeployKey struct {
Mode models.AccessMode `xorm:"NOT NULL DEFAULT 1"`
}

if err := x.Sync2(new(DeployKey)); err != nil {
return fmt.Errorf("Sync2: %v", err)
}
return nil
}
21 changes: 17 additions & 4 deletions models/ssh_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,8 @@ type DeployKey struct {
Fingerprint string
Content string `xorm:"-"`

Mode AccessMode `xorm:"NOT NULL DEFAULT 1"`

CreatedUnix util.TimeStamp `xorm:"created"`
UpdatedUnix util.TimeStamp `xorm:"updated"`
HasRecentActivity bool `xorm:"-"`
Expand All @@ -622,6 +624,11 @@ func (key *DeployKey) GetContent() error {
return nil
}

// IsReadOnly checks if the key can only be used for read operations
func (key *DeployKey) IsReadOnly() bool {
return key.Mode == AccessModeRead
}

func checkDeployKey(e Engine, keyID, repoID int64, name string) error {
// Note: We want error detail, not just true or false here.
has, err := e.
Expand All @@ -646,7 +653,7 @@ func checkDeployKey(e Engine, keyID, repoID int64, name string) error {
}

// addDeployKey adds new key-repo relation.
func addDeployKey(e *xorm.Session, keyID, repoID int64, name, fingerprint string) (*DeployKey, error) {
func addDeployKey(e *xorm.Session, keyID, repoID int64, name, fingerprint string, mode AccessMode) (*DeployKey, error) {
if err := checkDeployKey(e, keyID, repoID, name); err != nil {
return nil, err
}
Expand All @@ -656,6 +663,7 @@ func addDeployKey(e *xorm.Session, keyID, repoID int64, name, fingerprint string
RepoID: repoID,
Name: name,
Fingerprint: fingerprint,
Mode: mode,
}
_, err := e.Insert(key)
return key, err
Expand All @@ -670,15 +678,20 @@ func HasDeployKey(keyID, repoID int64) bool {
}

// AddDeployKey add new deploy key to database and authorized_keys file.
func AddDeployKey(repoID int64, name, content string) (*DeployKey, error) {
func AddDeployKey(repoID int64, name, content string, readOnly bool) (*DeployKey, error) {
fingerprint, err := calcFingerprint(content)
if err != nil {
return nil, err
}

accessMode := AccessModeRead
if !readOnly {
accessMode = AccessModeWrite
}

pkey := &PublicKey{
Fingerprint: fingerprint,
Mode: AccessModeRead,
Mode: accessMode,
Type: KeyTypeDeploy,
}
has, err := x.Get(pkey)
Expand All @@ -701,7 +714,7 @@ func AddDeployKey(repoID int64, name, content string) (*DeployKey, error) {
}
}

key, err := addDeployKey(sess, pkey.ID, repoID, name, pkey.Fingerprint)
key, err := addDeployKey(sess, pkey.ID, repoID, name, pkey.Fingerprint, accessMode)
if err != nil {
return nil, fmt.Errorf("addDeployKey: %v", err)
}
Expand Down
7 changes: 4 additions & 3 deletions modules/auth/user_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,10 @@ func (f *AddOpenIDForm) Validate(ctx *macaron.Context, errs binding.Errors) bind

// AddKeyForm form for adding SSH/GPG key
type AddKeyForm struct {
Type string `binding:"OmitEmpty"`
Title string `binding:"Required;MaxSize(50)"`
Content string `binding:"Required"`
Type string `binding:"OmitEmpty"`
Title string `binding:"Required;MaxSize(50)"`
Content string `binding:"Required"`
IsWritable bool
}

// Validate validates the fields
Expand Down
4 changes: 4 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,8 @@ valid_until = Valid until
valid_forever = Valid forever
last_used = Last used on
no_activity = No recent activity
can_read_info = Read
can_write_info = Write
key_state_desc = This key has been used in the last 7 days
token_state_desc = This token has been used in the last 7 days
show_openid = Show on profile
Expand Down Expand Up @@ -995,6 +997,8 @@ settings.add_dingtalk_hook_desc = Add <a href="%s">Dingtalk</a> integration to y
settings.deploy_keys = Deploy Keys
settings.add_deploy_key = Add Deploy Key
settings.deploy_key_desc = Deploy keys have read-only access. They are not the same as personal account SSH keys.
settings.is_writable = Allow write access
settings.is_writable_info = Can this key be used to <strong>push</strong> to this repository? Deploy keys always have pull access.
settings.no_deploy_keys = You haven't added any deploy keys.
settings.title = Title
settings.deploy_key_content = Content
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func CreateDeployKey(ctx *context.APIContext, form api.CreateKeyOption) {
return
}

key, err := models.AddDeployKey(ctx.Repo.Repository.ID, form.Title, content)
key, err := models.AddDeployKey(ctx.Repo.Repository.ID, form.Title, content, form.ReadOnly)
if err != nil {
HandleAddKeyError(ctx, err)
return
Expand Down
2 changes: 1 addition & 1 deletion routers/repo/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ func DeployKeysPost(ctx *context.Context, form auth.AddKeyForm) {
return
}

key, err := models.AddDeployKey(ctx.Repo.Repository.ID, form.Title, content)
key, err := models.AddDeployKey(ctx.Repo.Repository.ID, form.Title, content, !form.IsWritable)
if err != nil {
ctx.Data["HasError"] = true
switch {
Expand Down
61 changes: 61 additions & 0 deletions routers/repo/settings_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright 2017 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package repo

import (
"net/http"
"testing"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/auth"
"code.gitea.io/gitea/modules/test"

"github.com/stretchr/testify/assert"
)

func TestAddReadOnlyDeployKey(t *testing.T) {
models.PrepareTestEnv(t)

ctx := test.MockContext(t, "user2/repo1/settings/keys")

test.LoadUser(t, ctx, 2)
test.LoadRepo(t, ctx, 2)

addKeyForm := auth.AddKeyForm{
Title: "read-only",
Content: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n",
}
DeployKeysPost(ctx, addKeyForm)
assert.EqualValues(t, http.StatusFound, ctx.Resp.Status())

models.AssertExistsAndLoadBean(t, &models.DeployKey{
Name: addKeyForm.Title,
Content: addKeyForm.Content,
Mode: models.AccessModeRead,
})
}

func TestAddReadWriteOnlyDeployKey(t *testing.T) {
models.PrepareTestEnv(t)

ctx := test.MockContext(t, "user2/repo1/settings/keys")

test.LoadUser(t, ctx, 2)
test.LoadRepo(t, ctx, 2)

addKeyForm := auth.AddKeyForm{
Title: "read-write",
Content: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n",
IsWritable: true,
}
DeployKeysPost(ctx, addKeyForm)
assert.EqualValues(t, http.StatusFound, ctx.Resp.Status())

models.AssertExistsAndLoadBean(t, &models.DeployKey{
Name: addKeyForm.Title,
Content: addKeyForm.Content,
Mode: models.AccessModeWrite,
})
}
11 changes: 10 additions & 1 deletion templates/repo/settings/deploy_keys.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
{{.Fingerprint}}
</div>
<div class="activity meta">
<i>{{$.i18n.Tr "settings.add_on"}} <span>{{.CreatedUnix.FormatShort}}</span> — <i class="octicon octicon-info"></i> {{if .HasUsed}}{{$.i18n.Tr "settings.last_used"}} <span {{if .HasRecentActivity}}class="green"{{end}}>{{.UpdatedUnix.FormatShort}}</span>{{else}}{{$.i18n.Tr "settings.no_activity"}}{{end}}</i>
<i>{{$.i18n.Tr "settings.add_on"}} <span>{{.CreatedUnix.FormatShort}}</span> — <i class="octicon octicon-info"></i> {{if .HasUsed}}{{$.i18n.Tr "settings.last_used"}} <span {{if .HasRecentActivity}}class="green"{{end}}>{{.UpdatedUnix.FormatShort}}</span>{{else}}{{$.i18n.Tr "settings.no_activity"}}{{end}} - <span>{{$.i18n.Tr "settings.can_read_info"}}{{if not .IsReadOnly}} / {{$.i18n.Tr "settings.can_write_info"}} {{end}}</i>
</div>
</div>
</div>
Expand Down Expand Up @@ -60,6 +60,15 @@
<label for="content">{{.i18n.Tr "repo.settings.deploy_key_content"}}</label>
<textarea id="ssh-key-content" name="content" required>{{.content}}</textarea>
</div>
<div class="field">
<div class="ui checkbox {{if .Err_IsWritable}}error{{end}}">
<input id="ssh-key-is-writable" name="is_writable" class="hidden" type="checkbox" value="1">
<label for="is_writable">
{{.i18n.Tr "repo.settings.is_writable"}}
</label>
<small style="padding-left: 26px;">{{$.i18n.Tr "repo.settings.is_writable_info" | Str2html}}</small>
</div>
</div>
<button class="ui green button">
{{.i18n.Tr "repo.settings.add_deploy_key"}}
</button>
Expand Down
4 changes: 4 additions & 0 deletions vendor/code.gitea.io/sdk/gitea/repo_key.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions vendor/vendor.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
"revisionTime": "2017-12-22T02:43:26Z"
},
{
"checksumSHA1": "QQ7g7B9+EIzGjO14KCGEs9TNEzM=",
"checksumSHA1": "Qtq0kW+BnpYMOriaoCjMa86WGG8=",
"path": "code.gitea.io/sdk/gitea",
"revision": "ec7d3af43b598c1a3f2cb12f633b9625649d8e54",
"revisionTime": "2017-11-28T12:30:39Z"
"revision": "79eee8f12c7fc1cc5b802c5cdc5b494ef3733866",
"revisionTime": "2017-12-20T06:57:50Z"
},
{
"checksumSHA1": "bOODD4Gbw3GfcuQPU2dI40crxxk=",
Expand Down