From c64a244c278f7795be228d09a68dd801ebc16f40 Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Mon, 22 May 2017 23:11:29 +0200 Subject: [PATCH 1/4] add visibility database field --- model/repo.go | 1 + store/datastore/ddl/mysql/ddl_gen.go | 24 +++++++++++++++++ .../files/013_add_column_repo_visibility.sql | 11 ++++++++ store/datastore/ddl/postgres/ddl_gen.go | 26 ++++++++++++++++++- .../files/013_add_column_repo_visibility.sql | 11 ++++++++ store/datastore/ddl/sqlite/ddl_gen.go | 26 ++++++++++++++++++- .../files/013_add_column_repo_visibility.sql | 11 ++++++++ 7 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 store/datastore/ddl/mysql/files/013_add_column_repo_visibility.sql create mode 100644 store/datastore/ddl/postgres/files/013_add_column_repo_visibility.sql create mode 100644 store/datastore/ddl/sqlite/files/013_add_column_repo_visibility.sql diff --git a/model/repo.go b/model/repo.go index c650e5dd9..81a81e74e 100644 --- a/model/repo.go +++ b/model/repo.go @@ -22,6 +22,7 @@ type Repo struct { Clone string `json:"clone_url,omitempty" meddler:"repo_clone"` Branch string `json:"default_branch,omitempty" meddler:"repo_branch"` Timeout int64 `json:"timeout,omitempty" meddler:"repo_timeout"` + Visibility string `json:"visibility" meddler:"repo_visibility"` IsPrivate bool `json:"private,omitempty" meddler:"repo_private"` IsTrusted bool `json:"trusted" meddler:"repo_trusted"` IsStarred bool `json:"starred,omitempty" meddler:"-"` diff --git a/store/datastore/ddl/mysql/ddl_gen.go b/store/datastore/ddl/mysql/ddl_gen.go index cc4e9f3b4..53ed2ae11 100644 --- a/store/datastore/ddl/mysql/ddl_gen.go +++ b/store/datastore/ddl/mysql/ddl_gen.go @@ -88,6 +88,14 @@ var migrations = []struct { name: "create-index-sender-repos", stmt: createIndexSenderRepos, }, + { + name: "alter-table-add-repo-visibility", + stmt: alterTableAddRepoVisibility, + }, + { + name: "update-table-set-repo-visibility", + stmt: updateTableSetRepoVisibility, + }, } // Migrate performs the database migration. If the migration fails @@ -442,3 +450,19 @@ CREATE TABLE IF NOT EXISTS senders ( var createIndexSenderRepos = ` CREATE INDEX sender_repo_ix ON senders (sender_repo_id); ` + +// +// 013_add_column_repo_visibility.sql +// + +var alterTableAddRepoVisibility = ` +ALTER TABLE repos ADD COLUMN repo_visibility INTEGER +` + +var updateTableSetRepoVisibility = ` +UPDATE repos +SET repo_visibility = CASE + WHEN repo_private = 0 THEN 'public' + ELSE 'private' + END +` diff --git a/store/datastore/ddl/mysql/files/013_add_column_repo_visibility.sql b/store/datastore/ddl/mysql/files/013_add_column_repo_visibility.sql new file mode 100644 index 000000000..eb6e8ac5c --- /dev/null +++ b/store/datastore/ddl/mysql/files/013_add_column_repo_visibility.sql @@ -0,0 +1,11 @@ +-- name: alter-table-add-repo-visibility + +ALTER TABLE repos ADD COLUMN repo_visibility INTEGER + +-- name: update-table-set-repo-visibility + +UPDATE repos +SET repo_visibility = CASE + WHEN repo_private = 0 THEN 'public' + ELSE 'private' + END diff --git a/store/datastore/ddl/postgres/ddl_gen.go b/store/datastore/ddl/postgres/ddl_gen.go index c06a2d7c3..d2f49fdbf 100644 --- a/store/datastore/ddl/postgres/ddl_gen.go +++ b/store/datastore/ddl/postgres/ddl_gen.go @@ -88,6 +88,14 @@ var migrations = []struct { name: "create-index-sender-repos", stmt: createIndexSenderRepos, }, + { + name: "alter-table-add-repo-visibility", + stmt: alterTableAddRepoVisibility, + }, + { + name: "update-table-set-repo-visibility", + stmt: updateTableSetRepoVisibility, + }, } // Migrate performs the database migration. If the migration fails @@ -150,7 +158,7 @@ func selectCompleted(db *sql.DB) (map[string]struct{}, error) { var migrationTableCreate = ` CREATE TABLE IF NOT EXISTS migrations ( - name VARCHAR(512) + name VARCHAR(255) ,UNIQUE(name) ) ` @@ -442,3 +450,19 @@ CREATE TABLE IF NOT EXISTS senders ( var createIndexSenderRepos = ` CREATE INDEX IF NOT EXISTS sender_repo_ix ON senders (sender_repo_id); ` + +// +// 013_add_column_repo_visibility.sql +// + +var alterTableAddRepoVisibility = ` +ALTER TABLE repos ADD COLUMN repo_visibility INTEGER +` + +var updateTableSetRepoVisibility = ` +UPDATE repos +SET repo_visibility = (CASE + WHEN repo_private = 0 THEN 'public' + ELSE 'private' + END) +` diff --git a/store/datastore/ddl/postgres/files/013_add_column_repo_visibility.sql b/store/datastore/ddl/postgres/files/013_add_column_repo_visibility.sql new file mode 100644 index 000000000..109d79dfb --- /dev/null +++ b/store/datastore/ddl/postgres/files/013_add_column_repo_visibility.sql @@ -0,0 +1,11 @@ +-- name: alter-table-add-repo-visibility + +ALTER TABLE repos ADD COLUMN repo_visibility INTEGER + +-- name: update-table-set-repo-visibility + +UPDATE repos +SET repo_visibility = (CASE + WHEN repo_private = 0 THEN 'public' + ELSE 'private' + END) diff --git a/store/datastore/ddl/sqlite/ddl_gen.go b/store/datastore/ddl/sqlite/ddl_gen.go index d00be5140..ab3bd2639 100644 --- a/store/datastore/ddl/sqlite/ddl_gen.go +++ b/store/datastore/ddl/sqlite/ddl_gen.go @@ -92,6 +92,14 @@ var migrations = []struct { name: "create-index-sender-repos", stmt: createIndexSenderRepos, }, + { + name: "alter-table-add-repo-visibility", + stmt: alterTableAddRepoVisibility, + }, + { + name: "update-table-set-repo-visibility", + stmt: updateTableSetRepoVisibility, + }, } // Migrate performs the database migration. If the migration fails @@ -154,7 +162,7 @@ func selectCompleted(db *sql.DB) (map[string]struct{}, error) { var migrationTableCreate = ` CREATE TABLE IF NOT EXISTS migrations ( - name VARCHAR(512) + name VARCHAR(255) ,UNIQUE(name) ) ` @@ -443,3 +451,19 @@ CREATE TABLE IF NOT EXISTS senders ( var createIndexSenderRepos = ` CREATE INDEX IF NOT EXISTS sender_repo_ix ON senders (sender_repo_id); ` + +// +// 013_add_column_repo_visibility.sql +// + +var alterTableAddRepoVisibility = ` +ALTER TABLE repos ADD COLUMN repo_visibility INTEGER +` + +var updateTableSetRepoVisibility = ` +UPDATE repos +SET repo_visibility = CASE + WHEN repo_private = 0 THEN 'public' + ELSE 'private' + END +` diff --git a/store/datastore/ddl/sqlite/files/013_add_column_repo_visibility.sql b/store/datastore/ddl/sqlite/files/013_add_column_repo_visibility.sql new file mode 100644 index 000000000..eb6e8ac5c --- /dev/null +++ b/store/datastore/ddl/sqlite/files/013_add_column_repo_visibility.sql @@ -0,0 +1,11 @@ +-- name: alter-table-add-repo-visibility + +ALTER TABLE repos ADD COLUMN repo_visibility INTEGER + +-- name: update-table-set-repo-visibility + +UPDATE repos +SET repo_visibility = CASE + WHEN repo_private = 0 THEN 'public' + ELSE 'private' + END From 9ed9f8f1c9da142a74259d171066b9f518aae1ae Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Mon, 22 May 2017 23:16:42 +0200 Subject: [PATCH 2/4] change repo_visibility to text --- store/datastore/ddl/mysql/ddl_gen.go | 2 +- .../ddl/mysql/files/013_add_column_repo_visibility.sql | 2 +- store/datastore/ddl/postgres/ddl_gen.go | 4 ++-- .../ddl/postgres/files/013_add_column_repo_visibility.sql | 4 ++-- store/datastore/ddl/sqlite/ddl_gen.go | 2 +- .../ddl/sqlite/files/013_add_column_repo_visibility.sql | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/store/datastore/ddl/mysql/ddl_gen.go b/store/datastore/ddl/mysql/ddl_gen.go index 53ed2ae11..e70048265 100644 --- a/store/datastore/ddl/mysql/ddl_gen.go +++ b/store/datastore/ddl/mysql/ddl_gen.go @@ -456,7 +456,7 @@ CREATE INDEX sender_repo_ix ON senders (sender_repo_id); // var alterTableAddRepoVisibility = ` -ALTER TABLE repos ADD COLUMN repo_visibility INTEGER +ALTER TABLE repos ADD COLUMN repo_visibility VARCHAR(50) ` var updateTableSetRepoVisibility = ` diff --git a/store/datastore/ddl/mysql/files/013_add_column_repo_visibility.sql b/store/datastore/ddl/mysql/files/013_add_column_repo_visibility.sql index eb6e8ac5c..937294b49 100644 --- a/store/datastore/ddl/mysql/files/013_add_column_repo_visibility.sql +++ b/store/datastore/ddl/mysql/files/013_add_column_repo_visibility.sql @@ -1,6 +1,6 @@ -- name: alter-table-add-repo-visibility -ALTER TABLE repos ADD COLUMN repo_visibility INTEGER +ALTER TABLE repos ADD COLUMN repo_visibility VARCHAR(50) -- name: update-table-set-repo-visibility diff --git a/store/datastore/ddl/postgres/ddl_gen.go b/store/datastore/ddl/postgres/ddl_gen.go index d2f49fdbf..cd44d250f 100644 --- a/store/datastore/ddl/postgres/ddl_gen.go +++ b/store/datastore/ddl/postgres/ddl_gen.go @@ -456,13 +456,13 @@ CREATE INDEX IF NOT EXISTS sender_repo_ix ON senders (sender_repo_id); // var alterTableAddRepoVisibility = ` -ALTER TABLE repos ADD COLUMN repo_visibility INTEGER +ALTER TABLE repos ADD COLUMN repo_visibility VARCHAR(50) ` var updateTableSetRepoVisibility = ` UPDATE repos SET repo_visibility = (CASE - WHEN repo_private = 0 THEN 'public' + WHEN repo_private = true THEN 'public' ELSE 'private' END) ` diff --git a/store/datastore/ddl/postgres/files/013_add_column_repo_visibility.sql b/store/datastore/ddl/postgres/files/013_add_column_repo_visibility.sql index 109d79dfb..481117786 100644 --- a/store/datastore/ddl/postgres/files/013_add_column_repo_visibility.sql +++ b/store/datastore/ddl/postgres/files/013_add_column_repo_visibility.sql @@ -1,11 +1,11 @@ -- name: alter-table-add-repo-visibility -ALTER TABLE repos ADD COLUMN repo_visibility INTEGER +ALTER TABLE repos ADD COLUMN repo_visibility VARCHAR(50) -- name: update-table-set-repo-visibility UPDATE repos SET repo_visibility = (CASE - WHEN repo_private = 0 THEN 'public' + WHEN repo_private = true THEN 'public' ELSE 'private' END) diff --git a/store/datastore/ddl/sqlite/ddl_gen.go b/store/datastore/ddl/sqlite/ddl_gen.go index ab3bd2639..22cfb91d8 100644 --- a/store/datastore/ddl/sqlite/ddl_gen.go +++ b/store/datastore/ddl/sqlite/ddl_gen.go @@ -457,7 +457,7 @@ CREATE INDEX IF NOT EXISTS sender_repo_ix ON senders (sender_repo_id); // var alterTableAddRepoVisibility = ` -ALTER TABLE repos ADD COLUMN repo_visibility INTEGER +ALTER TABLE repos ADD COLUMN repo_visibility TEXT ` var updateTableSetRepoVisibility = ` diff --git a/store/datastore/ddl/sqlite/files/013_add_column_repo_visibility.sql b/store/datastore/ddl/sqlite/files/013_add_column_repo_visibility.sql index eb6e8ac5c..f8dcef8f3 100644 --- a/store/datastore/ddl/sqlite/files/013_add_column_repo_visibility.sql +++ b/store/datastore/ddl/sqlite/files/013_add_column_repo_visibility.sql @@ -1,6 +1,6 @@ -- name: alter-table-add-repo-visibility -ALTER TABLE repos ADD COLUMN repo_visibility INTEGER +ALTER TABLE repos ADD COLUMN repo_visibility TEXT -- name: update-table-set-repo-visibility From b1cbe65985b0de3328a8a09af5e3c4f0e236d42e Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Tue, 23 May 2017 00:44:58 +0200 Subject: [PATCH 3/4] use visibility to alter permissions --- model/const.go | 6 ++++ model/repo.go | 1 + router/middleware/session/repo.go | 39 ++++---------------------- router/middleware/session/repo_test.go | 37 +----------------------- server/repo.go | 17 +++++++++++ 5 files changed, 31 insertions(+), 69 deletions(-) diff --git a/model/const.go b/model/const.go index dfc2f3086..2cfd2e994 100644 --- a/model/const.go +++ b/model/const.go @@ -25,3 +25,9 @@ const ( RepoFossil = "fossil" RepoPerforce = "perforce" ) + +const ( + VisibilityPublic = "public" + VisibilityPrivate = "private" + VisibilityInternal = "internal" +) diff --git a/model/repo.go b/model/repo.go index 81a81e74e..cab0c02e1 100644 --- a/model/repo.go +++ b/model/repo.go @@ -41,6 +41,7 @@ type RepoPatch struct { IsTrusted *bool `json:"trusted,omitempty"` IsGated *bool `json:"gated,omitempty"` Timeout *int64 `json:"timeout,omitempty"` + Visibility *string `json:"visibility,omitempty"` AllowPull *bool `json:"allow_pr,omitempty"` AllowPush *bool `json:"allow_push,omitempty"` AllowDeploy *bool `json:"allow_deploy,omitempty"` diff --git a/router/middleware/session/repo.go b/router/middleware/session/repo.go index d687ccce8..24c6d5790 100644 --- a/router/middleware/session/repo.go +++ b/router/middleware/session/repo.go @@ -2,7 +2,6 @@ package session import ( "net/http" - "os" "github.com/drone/drone/cache" "github.com/drone/drone/model" @@ -79,7 +78,6 @@ func Perm(c *gin.Context) *model.Perm { } func SetPerm() gin.HandlerFunc { - PUBLIC_MODE := os.Getenv("PUBLIC_MODE") return func(c *gin.Context) { user := User(c) @@ -87,49 +85,24 @@ func SetPerm() gin.HandlerFunc { perm := &model.Perm{} switch { - // if the user is not authenticated, and the - // repository is private, the user has NO permission - // to view the repository. - case user == nil && repo.IsPrivate == true: - perm.Pull = false - perm.Push = false - perm.Admin = false - - // if the user is not authenticated, but the repository - // is public, the user has pull-rights only. - case user == nil && repo.IsPrivate == false: - perm.Pull = true - perm.Push = false - perm.Admin = false - - case user.Admin: + case user != nil && user.Admin: perm.Pull = true perm.Push = true perm.Admin = true - // otherwise if the user is authenticated we should - // check the remote system to get the users permissiosn. - default: + case user != nil: var err error perm, err = cache.GetPerms(c, user, repo.Owner, repo.Name) if err != nil { - perm.Pull = false - perm.Push = false - perm.Admin = false - - // debug log.Errorf("Error fetching permission for %s %s", user.Login, repo.FullName) } - // if we couldn't fetch permissions, but the repository - // is public, we should grant the user pull access. - if err != nil && repo.IsPrivate == false { - perm.Pull = true - } } - // all build logs are visible in public mode - if PUBLIC_MODE != "" { + switch { + case repo.Visibility == model.VisibilityPublic: + perm.Pull = true + case repo.Visibility == model.VisibilityInternal && user != nil: perm.Pull = true } diff --git a/router/middleware/session/repo_test.go b/router/middleware/session/repo_test.go index 6d524a9b7..349b4e3ac 100644 --- a/router/middleware/session/repo_test.go +++ b/router/middleware/session/repo_test.go @@ -1,44 +1,9 @@ package session import ( - "os" "testing" - - "github.com/drone/drone/model" - "github.com/franela/goblin" - "github.com/gin-gonic/gin" ) func TestSetPerm(t *testing.T) { - g := goblin.Goblin(t) - g.Describe("SetPerm", func() { - g.BeforeEach(func() { - os.Unsetenv("PUBLIC_MODE") - }) - g.It("Should set pull to false (private repo, user not logged in)", func() { - c := gin.Context{} - c.Set("repo", &model.Repo{ - IsPrivate: true, - }) - SetPerm()(&c) - v, ok := c.Get("perm") - g.Assert(ok).IsTrue("perm was not set") - p, ok := v.(*model.Perm) - g.Assert(ok).IsTrue("perm was the wrong type") - g.Assert(p.Pull).IsFalse("pull should be false") - }) - g.It("Should set pull to true (private repo, user not logged in, public mode)", func() { - os.Setenv("PUBLIC_MODE", "true") - c := gin.Context{} - c.Set("repo", &model.Repo{ - IsPrivate: true, - }) - SetPerm()(&c) - v, ok := c.Get("perm") - g.Assert(ok).IsTrue("perm was not set") - p, ok := v.(*model.Perm) - g.Assert(ok).IsTrue("perm was the wrong type") - g.Assert(p.Pull).IsTrue("pull should be true") - }) - }) + } diff --git a/server/repo.go b/server/repo.go index f4cb1a6f9..b965ed052 100644 --- a/server/repo.go +++ b/server/repo.go @@ -55,11 +55,15 @@ func PostRepo(c *gin.Context) { r.UserID = user.ID r.AllowPush = true r.AllowPull = true + r.Visibility = model.VisibilityPublic r.Config = ".drone.yml" r.Timeout = 60 // 1 hour default build time r.Hash = base32.StdEncoding.EncodeToString( securecookie.GenerateRandomKey(32), ) + if r.IsPrivate { + r.Visibility = model.VisibilityPrivate + } // crates the jwt token used to verify the repository t := token.New(token.HookToken, r.FullName) @@ -132,6 +136,19 @@ func PatchRepo(c *gin.Context) { if in.Config != nil { repo.Config = *in.Config } + if in.Visibility != nil { + switch *in.Visibility { + case model.VisibilityInternal: + repo.Visibility = model.VisibilityInternal + case model.VisibilityPrivate: + repo.Visibility = model.VisibilityPrivate + case model.VisibilityPublic: + repo.Visibility = model.VisibilityPublic + default: + c.String(400, "Invalid visibility type") + return + } + } err := store.UpdateRepo(c, repo) if err != nil { From 8a62c626bdda094f1f339d0dcd90c616f9dc1800 Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Tue, 23 May 2017 00:54:04 +0200 Subject: [PATCH 4/4] cleanup visibility verification logic --- server/repo.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/server/repo.go b/server/repo.go index b965ed052..39b8a6205 100644 --- a/server/repo.go +++ b/server/repo.go @@ -138,12 +138,8 @@ func PatchRepo(c *gin.Context) { } if in.Visibility != nil { switch *in.Visibility { - case model.VisibilityInternal: - repo.Visibility = model.VisibilityInternal - case model.VisibilityPrivate: - repo.Visibility = model.VisibilityPrivate - case model.VisibilityPublic: - repo.Visibility = model.VisibilityPublic + case model.VisibilityInternal, model.VisibilityPrivate, model.VisibilityPublic: + repo.Visibility = *in.Visibility default: c.String(400, "Invalid visibility type") return