X Tutup
Skip to content

Commit c80292c

Browse files
mislavmislav
committed
Extend Config object with GITHUB_TOKEN support
Adding GITHUB_TOKEN & GITHUB_ENTERPRISE_TOKEN support orthogonal to Config was getting out of hand, especially in `auth` commands that adjust their messaging and error status based on the presence of these environment variables. The new approach builds in support for tokens from environment straight into Config object by composition. Thus, commands need not ever be concerned with any specific environment variables.
1 parent feadf68 commit c80292c

File tree

14 files changed

+325
-256
lines changed

14 files changed

+325
-256
lines changed

‎internal/config/config_type.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ const defaultGitProtocol = "https"
1515
// This interface describes interacting with some persistent configuration for gh.
1616
type Config interface {
1717
Get(string, string) (string, error)
18+
GetWithSource(string, string) (string, string, error)
1819
Set(string, string, string) error
1920
UnsetHost(string)
2021
Hosts() ([]string, error)
2122
Aliases() (*AliasConfig, error)
23+
CheckWriteable(string, string) error
2224
Write() error
2325
}
2426

@@ -200,42 +202,51 @@ func (c *fileConfig) Root() *yaml.Node {
200202
}
201203

202204
func (c *fileConfig) Get(hostname, key string) (string, error) {
205+
val, _, err := c.GetWithSource(hostname, key)
206+
return val, err
207+
}
208+
209+
func (c *fileConfig) GetWithSource(hostname, key string) (string, string, error) {
203210
if hostname != "" {
204211
var notFound *NotFoundError
205212

206213
hostCfg, err := c.configForHost(hostname)
207214
if err != nil && !errors.As(err, &notFound) {
208-
return "", err
215+
return "", "", err
209216
}
210217

211218
var hostValue string
212219
if hostCfg != nil {
213220
hostValue, err = hostCfg.GetStringValue(key)
214221
if err != nil && !errors.As(err, &notFound) {
215-
return "", err
222+
return "", "", err
216223
}
217224
}
218225

219226
if hostValue != "" {
220-
return hostValue, nil
227+
// TODO: avoid hardcoding this
228+
return hostValue, "~/.config/gh/hosts.yml", nil
221229
}
222230
}
223231

232+
// TODO: avoid hardcoding this
233+
defaultSource := "~/.config/gh/config.yml"
234+
224235
value, err := c.GetStringValue(key)
225236

226237
var notFound *NotFoundError
227238

228239
if err != nil && errors.As(err, &notFound) {
229-
return defaultFor(key), nil
240+
return defaultFor(key), defaultSource, nil
230241
} else if err != nil {
231-
return "", err
242+
return "", defaultSource, err
232243
}
233244

234245
if value == "" {
235-
return defaultFor(key), nil
246+
return defaultFor(key), defaultSource, nil
236247
}
237248

238-
return value, nil
249+
return value, defaultSource, nil
239250
}
240251

241252
func (c *fileConfig) Set(hostname, key, value string) error {
@@ -281,6 +292,11 @@ func (c *fileConfig) configForHost(hostname string) (*HostConfig, error) {
281292
return nil, &NotFoundError{fmt.Errorf("could not find config entry for %q", hostname)}
282293
}
283294

295+
func (c *fileConfig) CheckWriteable(hostname, key string) error {
296+
// TODO: check filesystem permissions
297+
return nil
298+
}
299+
284300
func (c *fileConfig) Write() error {
285301
mainData := yaml.Node{Kind: yaml.MappingNode}
286302
hostsData := yaml.Node{Kind: yaml.MappingNode}

‎internal/config/from_env.go‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package config
2+
3+
import (
4+
"fmt"
5+
"os"
6+
7+
"github.com/cli/cli/internal/ghinstance"
8+
)
9+
10+
const (
11+
GITHUB_TOKEN = "GITHUB_TOKEN"
12+
GITHUB_ENTERPRISE_TOKEN = "GITHUB_ENTERPRISE_TOKEN"
13+
)
14+
15+
func InheritEnv(c Config) Config {
16+
return &envConfig{Config: c}
17+
}
18+
19+
type envConfig struct {
20+
Config
21+
}
22+
23+
func (c *envConfig) Hosts() ([]string, error) {
24+
hasDefault := false
25+
hosts, err := c.Config.Hosts()
26+
for _, h := range hosts {
27+
if h == ghinstance.Default() {
28+
hasDefault = true
29+
}
30+
}
31+
if (err != nil || !hasDefault) && os.Getenv(GITHUB_TOKEN) != "" {
32+
hosts = append([]string{ghinstance.Default()}, hosts...)
33+
return hosts, nil
34+
}
35+
return hosts, err
36+
}
37+
38+
func (c *envConfig) Get(hostname, key string) (string, error) {
39+
val, _, err := c.GetWithSource(hostname, key)
40+
return val, err
41+
}
42+
43+
func (c *envConfig) GetWithSource(hostname, key string) (string, string, error) {
44+
if hostname != "" && key == "oauth_token" {
45+
envName := GITHUB_TOKEN
46+
if ghinstance.IsEnterprise(hostname) {
47+
envName = GITHUB_ENTERPRISE_TOKEN
48+
}
49+
50+
if value := os.Getenv(envName); value != "" {
51+
return value, envName, nil
52+
}
53+
}
54+
55+
return c.Config.GetWithSource(hostname, key)
56+
}
57+
58+
func (c *envConfig) CheckWriteable(hostname, key string) error {
59+
if hostname != "" && key == "oauth_token" {
60+
envName := GITHUB_TOKEN
61+
if ghinstance.IsEnterprise(hostname) {
62+
envName = GITHUB_ENTERPRISE_TOKEN
63+
}
64+
65+
if os.Getenv(envName) != "" {
66+
return fmt.Errorf("read-only token in %s cannot be modified", envName)
67+
}
68+
}
69+
70+
return c.Config.CheckWriteable(hostname, key)
71+
}

‎internal/config/from_env_test.go‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
package config
2+
3+
import (
4+
"os"
5+
"testing"
6+
7+
"github.com/MakeNowJust/heredoc"
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestInheritEnv(t *testing.T) {
12+
orig_GITHUB_TOKEN := os.Getenv("GITHUB_TOKEN")
13+
orig_GITHUB_ENTERPRISE_TOKEN := os.Getenv("GITHUB_ENTERPRISE_TOKEN")
14+
t.Cleanup(func() {
15+
os.Setenv("GITHUB_TOKEN", orig_GITHUB_TOKEN)
16+
os.Setenv("GITHUB_ENTERPRISE_TOKEN", orig_GITHUB_ENTERPRISE_TOKEN)
17+
})
18+
19+
type wants struct {
20+
hosts []string
21+
token string
22+
source string
23+
writeable bool
24+
}
25+
26+
tests := []struct {
27+
name string
28+
baseConfig string
29+
GITHUB_TOKEN string
30+
GITHUB_ENTERPRISE_TOKEN string
31+
hostname string
32+
wants wants
33+
}{
34+
{
35+
name: "blank",
36+
baseConfig: ``,
37+
GITHUB_TOKEN: "",
38+
GITHUB_ENTERPRISE_TOKEN: "",
39+
hostname: "github.com",
40+
wants: wants{
41+
hosts: []string(nil),
42+
token: "",
43+
source: "~/.config/gh/config.yml",
44+
writeable: true,
45+
},
46+
},
47+
{
48+
name: "GITHUB_TOKEN over blank config",
49+
baseConfig: ``,
50+
GITHUB_TOKEN: "OTOKEN",
51+
GITHUB_ENTERPRISE_TOKEN: "",
52+
hostname: "github.com",
53+
wants: wants{
54+
hosts: []string{"github.com"},
55+
token: "OTOKEN",
56+
source: "GITHUB_TOKEN",
57+
writeable: false,
58+
},
59+
},
60+
{
61+
name: "GITHUB_TOKEN not applicable to GHE",
62+
baseConfig: ``,
63+
GITHUB_TOKEN: "OTOKEN",
64+
GITHUB_ENTERPRISE_TOKEN: "",
65+
hostname: "example.org",
66+
wants: wants{
67+
hosts: []string{"github.com"},
68+
token: "",
69+
source: "~/.config/gh/config.yml",
70+
writeable: true,
71+
},
72+
},
73+
{
74+
name: "GITHUB_ENTERPRISE_TOKEN over blank config",
75+
baseConfig: ``,
76+
GITHUB_TOKEN: "",
77+
GITHUB_ENTERPRISE_TOKEN: "ENTOKEN",
78+
hostname: "example.org",
79+
wants: wants{
80+
hosts: []string(nil),
81+
token: "ENTOKEN",
82+
source: "GITHUB_ENTERPRISE_TOKEN",
83+
writeable: false,
84+
},
85+
},
86+
{
87+
name: "token from file",
88+
baseConfig: heredoc.Doc(`
89+
hosts:
90+
github.com:
91+
oauth_token: OTOKEN
92+
`),
93+
GITHUB_TOKEN: "",
94+
GITHUB_ENTERPRISE_TOKEN: "",
95+
hostname: "github.com",
96+
wants: wants{
97+
hosts: []string{"github.com"},
98+
token: "OTOKEN",
99+
source: "~/.config/gh/hosts.yml",
100+
writeable: true,
101+
},
102+
},
103+
{
104+
name: "GITHUB_TOKEN shadows token from file",
105+
baseConfig: heredoc.Doc(`
106+
hosts:
107+
github.com:
108+
oauth_token: OTOKEN
109+
`),
110+
GITHUB_TOKEN: "ENVTOKEN",
111+
GITHUB_ENTERPRISE_TOKEN: "",
112+
hostname: "github.com",
113+
wants: wants{
114+
hosts: []string{"github.com"},
115+
token: "ENVTOKEN",
116+
source: "GITHUB_TOKEN",
117+
writeable: false,
118+
},
119+
},
120+
{
121+
name: "GITHUB_TOKEN adds host entry",
122+
baseConfig: heredoc.Doc(`
123+
hosts:
124+
example.org:
125+
oauth_token: OTOKEN
126+
`),
127+
GITHUB_TOKEN: "ENVTOKEN",
128+
GITHUB_ENTERPRISE_TOKEN: "",
129+
hostname: "github.com",
130+
wants: wants{
131+
hosts: []string{"github.com", "example.org"},
132+
token: "ENVTOKEN",
133+
source: "GITHUB_TOKEN",
134+
writeable: false,
135+
},
136+
},
137+
}
138+
for _, tt := range tests {
139+
t.Run(tt.name, func(t *testing.T) {
140+
os.Setenv("GITHUB_TOKEN", tt.GITHUB_TOKEN)
141+
os.Setenv("GITHUB_ENTERPRISE_TOKEN", tt.GITHUB_ENTERPRISE_TOKEN)
142+
143+
baseCfg := NewFromString(tt.baseConfig)
144+
cfg := InheritEnv(baseCfg)
145+
146+
hosts, _ := cfg.Hosts()
147+
assert.Equal(t, tt.wants.hosts, hosts)
148+
149+
val, source, _ := cfg.GetWithSource(tt.hostname, "oauth_token")
150+
assert.Equal(t, tt.wants.token, val)
151+
assert.Equal(t, tt.wants.source, source)
152+
153+
val, _ = cfg.Get(tt.hostname, "oauth_token")
154+
assert.Equal(t, tt.wants.token, val)
155+
156+
err := cfg.CheckWriteable(tt.hostname, "oauth_token")
157+
assert.Equal(t, tt.wants.writeable, err == nil)
158+
})
159+
}
160+
}

0 commit comments

Comments
 (0)
X Tutup