Skip to content

Commit f60ee1d

Browse files
ecrupperecrupper
authored
enhance(secrets): verify casing of orgs and repos in SCM before adding secret (#700)
* enhance(secrets): verify casing and existence of orgs and repos in SCM before adding secret * fix some comments * add clarifying comments * ditch equalfold check since it will always be true at that point
1 parent f0741c9 commit f60ee1d

File tree

7 files changed

+375
-0
lines changed

7 files changed

+375
-0
lines changed

‎api/secret.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,55 @@ func CreateSecret(c *gin.Context) {
109109
}
110110
}
111111

112+
if strings.EqualFold(t, constants.SecretOrg) {
113+
// retrieve org name from SCM
114+
//
115+
// SCM can be case insensitive, causing access retrieval to work
116+
// but Org/Repo != org/repo in Vela. So this check ensures that
117+
// what a user inputs matches the casing we expect in Vela since
118+
// the SCM will have the source of truth for casing.
119+
org, err := scm.FromContext(c).GetOrgName(u, o)
120+
if err != nil {
121+
retErr := fmt.Errorf("unable to retrieve organization %s", o)
122+
123+
util.HandleError(c, http.StatusNotFound, retErr)
124+
125+
return
126+
}
127+
128+
// check if casing is accurate
129+
if org != o {
130+
retErr := fmt.Errorf("unable to retrieve organization %s. Did you mean %s?", o, org)
131+
132+
util.HandleError(c, http.StatusNotFound, retErr)
133+
134+
return
135+
}
136+
}
137+
138+
if strings.EqualFold(t, constants.SecretRepo) {
139+
// retrieve repo name from SCM
140+
//
141+
// same story as org secret. SCM has accurate casing.
142+
scmRepo, err := scm.FromContext(c).GetRepoName(u, o, n)
143+
if err != nil {
144+
retErr := fmt.Errorf("unable to retrieve repository %s/%s", o, n)
145+
146+
util.HandleError(c, http.StatusNotFound, retErr)
147+
148+
return
149+
}
150+
151+
// check if casing is accurate
152+
if scmRepo != n {
153+
retErr := fmt.Errorf("unable to retrieve repository %s. Did you mean %s?", n, scmRepo)
154+
155+
util.HandleError(c, http.StatusNotFound, retErr)
156+
157+
return
158+
}
159+
}
160+
112161
// update engine logger with API metadata
113162
//
114163
// https://pkg.go.dev/github.com/sirupsen/logrus?tab=doc#Entry.WithFields

‎scm/github/org.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright (c) 2022 Target Brands, Inc. All rights reserved.
2+
//
3+
// Use of this source code is governed by the LICENSE file in this repository.
4+
5+
package github
6+
7+
import (
8+
"net/http"
9+
10+
"github.com/sirupsen/logrus"
11+
12+
"github.com/go-vela/types/library"
13+
)
14+
15+
// GetOrgName gets org name from Github.
16+
func (c *client) GetOrgName(u *library.User, o string) (string, error) {
17+
c.Logger.WithFields(logrus.Fields{
18+
"org": o,
19+
"user": u.GetName(),
20+
}).Tracef("retrieving org information for %s", o)
21+
22+
// create GitHub OAuth client with user's token
23+
client := c.newClientToken(u.GetToken())
24+
25+
// send an API call to get the org info
26+
orgInfo, resp, err := client.Organizations.Get(ctx, o)
27+
28+
orgName := orgInfo.GetLogin()
29+
30+
// if org is not found, return the personal org
31+
if resp.StatusCode == http.StatusNotFound {
32+
user, _, err := client.Users.Get(ctx, "")
33+
if err != nil {
34+
return "", err
35+
}
36+
37+
orgName = user.GetLogin()
38+
} else if err != nil {
39+
return "", err
40+
}
41+
42+
return orgName, nil
43+
}

‎scm/github/org_test.go‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// Copyright (c) 2022 Target Brands, Inc. All rights reserved.
2+
//
3+
// Use of this source code is governed by the LICENSE file in this repository.
4+
5+
package github
6+
7+
import (
8+
"net/http"
9+
"net/http/httptest"
10+
"reflect"
11+
"testing"
12+
13+
"github.com/gin-gonic/gin"
14+
15+
"github.com/go-vela/types/library"
16+
)
17+
18+
func TestGithub_GetOrgName(t *testing.T) {
19+
// setup context
20+
gin.SetMode(gin.TestMode)
21+
22+
resp := httptest.NewRecorder()
23+
_, engine := gin.CreateTestContext(resp)
24+
25+
// setup mock server
26+
engine.GET("/api/v3/orgs/:org", func(c *gin.Context) {
27+
c.Header("Content-Type", "application/json")
28+
c.Status(http.StatusOK)
29+
c.File("testdata/get_org.json")
30+
})
31+
32+
s := httptest.NewServer(engine)
33+
defer s.Close()
34+
35+
// setup types
36+
u := new(library.User)
37+
u.SetName("foo")
38+
u.SetToken("bar")
39+
40+
want := "github"
41+
42+
client, _ := NewTest(s.URL)
43+
44+
// run test
45+
got, err := client.GetOrgName(u, "github")
46+
47+
if resp.Code != http.StatusOK {
48+
t.Errorf("GetOrgName returned %v, want %v", resp.Code, http.StatusOK)
49+
}
50+
51+
if err != nil {
52+
t.Errorf("GetOrgName returned err: %v", err)
53+
}
54+
55+
if !reflect.DeepEqual(got, want) {
56+
t.Errorf("GetOrgName is %v, want %v", got, want)
57+
}
58+
}
59+
60+
func TestGithub_GetOrgName_Personal(t *testing.T) {
61+
// setup context
62+
gin.SetMode(gin.TestMode)
63+
64+
resp := httptest.NewRecorder()
65+
_, engine := gin.CreateTestContext(resp)
66+
67+
// setup mock server
68+
engine.GET("/api/v3/user", func(c *gin.Context) {
69+
c.Header("Content-Type", "application/json")
70+
c.Status(http.StatusOK)
71+
c.File("testdata/user.json")
72+
})
73+
74+
s := httptest.NewServer(engine)
75+
defer s.Close()
76+
77+
// setup types
78+
u := new(library.User)
79+
u.SetName("foo")
80+
u.SetToken("bar")
81+
82+
want := "octocat"
83+
84+
client, _ := NewTest(s.URL)
85+
86+
// run test
87+
got, err := client.GetOrgName(u, "octocat")
88+
89+
if resp.Code != http.StatusOK {
90+
t.Errorf("GetOrgName returned %v, want %v", resp.Code, http.StatusOK)
91+
}
92+
93+
if err != nil {
94+
t.Errorf("GetOrgName returned err: %v", err)
95+
}
96+
97+
if !reflect.DeepEqual(got, want) {
98+
t.Errorf("GetOrgName is %v, want %v", got, want)
99+
}
100+
}
101+
102+
func TestGithub_GetOrgName_Fail(t *testing.T) {
103+
// setup context
104+
gin.SetMode(gin.TestMode)
105+
106+
resp := httptest.NewRecorder()
107+
_, engine := gin.CreateTestContext(resp)
108+
109+
// setup mock server
110+
engine.GET("/api/v3/orgs/:org", func(c *gin.Context) {
111+
c.Header("Content-Type", "application/json")
112+
c.Status(http.StatusNotFound)
113+
})
114+
115+
s := httptest.NewServer(engine)
116+
defer s.Close()
117+
118+
// setup types
119+
u := new(library.User)
120+
u.SetName("foo")
121+
u.SetToken("bar")
122+
123+
client, _ := NewTest(s.URL)
124+
125+
// run test
126+
_, err := client.GetOrgName(u, "octocat")
127+
128+
if err == nil {
129+
t.Error("GetOrgName should return error")
130+
}
131+
}

‎scm/github/repo.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,26 @@ func (c *client) GetRepo(u *library.User, r *library.Repo) (*library.Repo, error
309309
return toLibraryRepo(*repo), nil
310310
}
311311

312+
// GetRepoName returns the name of the repository in the SCM.
313+
func (c *client) GetRepoName(u *library.User, o string, r string) (string, error) {
314+
c.Logger.WithFields(logrus.Fields{
315+
"org": o,
316+
"repo": r,
317+
"user": u.GetName(),
318+
}).Tracef("retrieving repository information for %s/%s", o, r)
319+
320+
// create GitHub OAuth client with user's token
321+
client := c.newClientToken(u.GetToken())
322+
323+
// send an API call to get the repo info
324+
repo, _, err := client.Repositories.Get(ctx, o, r)
325+
if err != nil {
326+
return "", err
327+
}
328+
329+
return repo.GetName(), nil
330+
}
331+
312332
// ListUserRepos returns a list of all repos the user has access to.
313333
func (c *client) ListUserRepos(u *library.User) ([]*library.Repo, error) {
314334
c.Logger.WithFields(logrus.Fields{

‎scm/github/repo_test.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,79 @@ func TestGithub_GetRepo_Fail(t *testing.T) {
10131013
}
10141014
}
10151015

1016+
func TestGithub_GetRepoName(t *testing.T) {
1017+
// setup context
1018+
gin.SetMode(gin.TestMode)
1019+
1020+
resp := httptest.NewRecorder()
1021+
_, engine := gin.CreateTestContext(resp)
1022+
1023+
// setup mock server
1024+
engine.GET("/api/v3/repos/:owner/:repo", func(c *gin.Context) {
1025+
c.Header("Content-Type", "application/json")
1026+
c.Status(http.StatusOK)
1027+
c.File("testdata/get_repo.json")
1028+
})
1029+
1030+
s := httptest.NewServer(engine)
1031+
defer s.Close()
1032+
1033+
// setup types
1034+
u := new(library.User)
1035+
u.SetName("foo")
1036+
u.SetToken("bar")
1037+
1038+
want := "Hello-World"
1039+
1040+
client, _ := NewTest(s.URL)
1041+
1042+
// run test
1043+
got, err := client.GetRepoName(u, "octocat", "Hello-World")
1044+
1045+
if resp.Code != http.StatusOK {
1046+
t.Errorf("GetRepoName returned %v, want %v", resp.Code, http.StatusOK)
1047+
}
1048+
1049+
if err != nil {
1050+
t.Errorf("GetRepoName returned err: %v", err)
1051+
}
1052+
1053+
if !reflect.DeepEqual(got, want) {
1054+
t.Errorf("GetRepoName is %v, want %v", got, want)
1055+
}
1056+
}
1057+
1058+
func TestGithub_GetRepoName_Fail(t *testing.T) {
1059+
// setup context
1060+
gin.SetMode(gin.TestMode)
1061+
1062+
resp := httptest.NewRecorder()
1063+
_, engine := gin.CreateTestContext(resp)
1064+
1065+
// setup mock server
1066+
engine.GET("/api/v3/repos/:owner/:repo", func(c *gin.Context) {
1067+
c.Header("Content-Type", "application/json")
1068+
c.Status(http.StatusNotFound)
1069+
})
1070+
1071+
s := httptest.NewServer(engine)
1072+
defer s.Close()
1073+
1074+
// setup types
1075+
u := new(library.User)
1076+
u.SetName("foo")
1077+
u.SetToken("bar")
1078+
1079+
client, _ := NewTest(s.URL)
1080+
1081+
// run test
1082+
_, err := client.GetRepoName(u, "octocat", "Hello-World")
1083+
1084+
if err == nil {
1085+
t.Error("GetRepoName should return error")
1086+
}
1087+
}
1088+
10161089
func TestGithub_ListUserRepos(t *testing.T) {
10171090
// setup context
10181091
gin.SetMode(gin.TestMode)

‎scm/github/testdata/get_org.json‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
{
2+
"login": "github",
3+
"id": 1,
4+
"node_id": "MDEyOk9yZ2FuaXphdGlvbjE=",
5+
"url": "https://api.github.com/orgs/github",
6+
"repos_url": "https://api.github.com/orgs/github/repos",
7+
"events_url": "https://api.github.com/orgs/github/events",
8+
"hooks_url": "https://api.github.com/orgs/github/hooks",
9+
"issues_url": "https://api.github.com/orgs/github/issues",
10+
"members_url": "https://api.github.com/orgs/github/members{/member}",
11+
"public_members_url": "https://api.github.com/orgs/github/public_members{/member}",
12+
"avatar_url": "https://github.com/images/error/octocat_happy.gif",
13+
"description": "A great organization",
14+
"name": "github",
15+
"company": "GitHub",
16+
"blog": "https://github.com/blog",
17+
"location": "San Francisco",
18+
"email": "[email protected]",
19+
"twitter_username": "github",
20+
"is_verified": true,
21+
"has_organization_projects": true,
22+
"has_repository_projects": true,
23+
"public_repos": 2,
24+
"public_gists": 1,
25+
"followers": 20,
26+
"following": 0,
27+
"html_url": "https://github.com/octocat",
28+
"created_at": "2008-01-14T04:33:35Z",
29+
"updated_at": "2014-03-03T18:58:10Z",
30+
"type": "Organization",
31+
"total_private_repos": 100,
32+
"owned_private_repos": 100,
33+
"private_gists": 81,
34+
"disk_usage": 10000,
35+
"collaborators": 8,
36+
"billing_email": "[email protected]",
37+
"plan": {
38+
"name": "Medium",
39+
"space": 400,
40+
"private_repos": 20,
41+
"filled_seats": 4,
42+
"seats": 5
43+
},
44+
"default_repository_permission": "read",
45+
"members_can_create_repositories": true,
46+
"two_factor_requirement_enabled": true,
47+
"members_allowed_repository_creation_type": "all",
48+
"members_can_create_public_repositories": false,
49+
"members_can_create_private_repositories": false,
50+
"members_can_create_internal_repositories": false,
51+
"members_can_create_pages": true,
52+
"members_can_fork_private_repositories": false
53+
}

0 commit comments

Comments
 (0)