Skip to content

Commit a3bdb9e

Browse files
TheSpiritXIIITheSpiritXIII
authored
Support empty OAuth2 inline secrets (#547)
Signed-off-by: Daniel Hrabovcak <[email protected]>
1 parent bd0376d commit a3bdb9e

File tree

4 files changed

+180
-66
lines changed

4 files changed

+180
-66
lines changed

‎config/generate.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ func SerialNumber() *big.Int {
7171
serialNumber.Add(&serial, big.NewInt(1))
7272

7373
return &serial
74-
7574
}
7675

7776
func GenerateCertificateAuthority(commonName string, parentCert *x509.Certificate, parentKey *rsa.PrivateKey) (*x509.Certificate, *rsa.PrivateKey, error) {
@@ -170,7 +169,7 @@ func writeCertificateAndKey(path string, cert *x509.Certificate, key *rsa.Privat
170169
return err
171170
}
172171

173-
if err := os.WriteFile(fmt.Sprintf("%s.crt", path), b.Bytes(), 0644); err != nil {
172+
if err := os.WriteFile(fmt.Sprintf("%s.crt", path), b.Bytes(), 0o644); err != nil {
174173
return err
175174
}
176175

@@ -179,7 +178,7 @@ func writeCertificateAndKey(path string, cert *x509.Certificate, key *rsa.Privat
179178
return err
180179
}
181180

182-
if err := os.WriteFile(fmt.Sprintf("%s.key", path), b.Bytes(), 0644); err != nil {
181+
if err := os.WriteFile(fmt.Sprintf("%s.key", path), b.Bytes(), 0o644); err != nil {
183182
return err
184183
}
185184

@@ -239,7 +238,7 @@ func main() {
239238
log.Fatal(err)
240239
}
241240

242-
if err := os.WriteFile("testdata/tls-ca-chain.pem", b.Bytes(), 0644); err != nil {
241+
if err := os.WriteFile("testdata/tls-ca-chain.pem", b.Bytes(), 0o644); err != nil {
243242
log.Fatal(err)
244243
}
245244
}

‎config/http_config.go

+3-7
Original file line numberDiff line numberDiff line change
@@ -378,9 +378,6 @@ func (c *HTTPClientConfig) Validate() error {
378378
if len(c.OAuth2.ClientID) == 0 {
379379
return fmt.Errorf("oauth2 client_id must be configured")
380380
}
381-
if len(c.OAuth2.ClientSecret) == 0 && len(c.OAuth2.ClientSecretFile) == 0 {
382-
return fmt.Errorf("either oauth2 client_secret or client_secret_file must be configured")
383-
}
384381
if len(c.OAuth2.TokenURL) == 0 {
385382
return fmt.Errorf("oauth2 token_url must be configured")
386383
}
@@ -729,13 +726,12 @@ func (rt *oauth2RoundTripper) RoundTrip(req *http.Request) (*http.Response, erro
729726
rt.mtx.RLock()
730727
changed = secret != rt.secret
731728
rt.mtx.RUnlock()
729+
} else {
730+
// Either an inline secret or nothing (use an empty string) was provided.
731+
secret = string(rt.config.ClientSecret)
732732
}
733733

734734
if changed || rt.rt == nil {
735-
if rt.config.ClientSecret != "" {
736-
secret = string(rt.config.ClientSecret)
737-
}
738-
739735
config := &clientcredentials.Config{
740736
ClientID: rt.config.ClientID,
741737
ClientSecret: secret,

‎config/http_config_test.go

+174-52
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,6 @@ var invalidHTTPClientConfigs = []struct {
115115
httpClientConfigFile: "testdata/http.conf.oauth2-no-client-id.bad.yaml",
116116
errMsg: "oauth2 client_id must be configured",
117117
},
118-
{
119-
httpClientConfigFile: "testdata/http.conf.oauth2-no-client-secret.bad.yaml",
120-
errMsg: "either oauth2 client_secret or client_secret_file must be configured",
121-
},
122118
{
123119
httpClientConfigFile: "testdata/http.conf.oauth2-no-token-url.bad.yaml",
124120
errMsg: "oauth2 token_url must be configured",
@@ -423,6 +419,46 @@ func TestNewClientFromConfig(t *testing.T) {
423419
}
424420
},
425421
},
422+
{
423+
clientConfig: HTTPClientConfig{
424+
OAuth2: &OAuth2{
425+
ClientID: "ExpectedUsername",
426+
TLSConfig: TLSConfig{
427+
CAFile: TLSCAChainPath,
428+
CertFile: ClientCertificatePath,
429+
KeyFile: ClientKeyNoPassPath,
430+
ServerName: "",
431+
InsecureSkipVerify: false,
432+
},
433+
},
434+
TLSConfig: TLSConfig{
435+
CAFile: TLSCAChainPath,
436+
CertFile: ClientCertificatePath,
437+
KeyFile: ClientKeyNoPassPath,
438+
ServerName: "",
439+
InsecureSkipVerify: false,
440+
},
441+
},
442+
handler: func(w http.ResponseWriter, r *http.Request) {
443+
switch r.URL.Path {
444+
case "/token":
445+
res, _ := json.Marshal(oauth2TestServerResponse{
446+
AccessToken: ExpectedAccessToken,
447+
TokenType: "Bearer",
448+
})
449+
w.Header().Add("Content-Type", "application/json")
450+
_, _ = w.Write(res)
451+
452+
default:
453+
authorization := r.Header.Get("Authorization")
454+
if authorization != "Bearer "+ExpectedAccessToken {
455+
fmt.Fprintf(w, "Expected Authorization header %q, got %q", "Bearer "+ExpectedAccessToken, authorization)
456+
} else {
457+
fmt.Fprint(w, ExpectedMessage)
458+
}
459+
}
460+
},
461+
},
426462
{
427463
clientConfig: HTTPClientConfig{
428464
OAuth2: &OAuth2{
@@ -1448,38 +1484,81 @@ type oauth2TestServerResponse struct {
14481484
TokenType string `json:"token_type"`
14491485
}
14501486

1451-
func TestOAuth2(t *testing.T) {
1452-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1487+
type testOAuthServer struct {
1488+
tokenTS *httptest.Server
1489+
ts *httptest.Server
1490+
}
1491+
1492+
// newTestOAuthServer returns a new test server with the expected base64 encoded client ID and secret.
1493+
func newTestOAuthServer(t testing.TB, expectedAuth *string) testOAuthServer {
1494+
var previousAuth string
1495+
tokenTS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1496+
auth := r.Header.Get("Authorization")
1497+
if auth != *expectedAuth {
1498+
t.Fatalf("bad auth, expected %s, got %s", *expectedAuth, auth)
1499+
}
1500+
if auth == previousAuth {
1501+
t.Fatal("token endpoint called twice")
1502+
}
1503+
previousAuth = auth
14531504
res, _ := json.Marshal(oauth2TestServerResponse{
14541505
AccessToken: "12345",
14551506
TokenType: "Bearer",
14561507
})
14571508
w.Header().Add("Content-Type", "application/json")
14581509
_, _ = w.Write(res)
14591510
}))
1460-
defer ts.Close()
1511+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1512+
auth := r.Header.Get("Authorization")
1513+
if auth != "Bearer 12345" {
1514+
t.Fatalf("bad auth, expected %s, got %s", "Bearer 12345", auth)
1515+
}
1516+
fmt.Fprintln(w, "Hello, client")
1517+
}))
1518+
return testOAuthServer{
1519+
tokenTS: tokenTS,
1520+
ts: ts,
1521+
}
1522+
}
1523+
1524+
func (s *testOAuthServer) url() string {
1525+
return s.ts.URL
1526+
}
1527+
1528+
func (s *testOAuthServer) tokenURL() string {
1529+
return s.tokenTS.URL
1530+
}
1531+
1532+
func (s *testOAuthServer) close() {
1533+
s.tokenTS.Close()
1534+
s.ts.Close()
1535+
}
1536+
1537+
func TestOAuth2(t *testing.T) {
1538+
var expectedAuth string
1539+
ts := newTestOAuthServer(t, &expectedAuth)
1540+
defer ts.close()
14611541

14621542
yamlConfig := fmt.Sprintf(`
14631543
client_id: 1
14641544
client_secret: 2
14651545
scopes:
14661546
- A
14671547
- B
1468-
token_url: %s/token
1548+
token_url: %s
14691549
endpoint_params:
14701550
hi: hello
1471-
`, ts.URL)
1551+
`, ts.tokenURL())
14721552
expectedConfig := OAuth2{
14731553
ClientID: "1",
14741554
ClientSecret: "2",
14751555
Scopes: []string{"A", "B"},
14761556
EndpointParams: map[string]string{"hi": "hello"},
1477-
TokenURL: fmt.Sprintf("%s/token", ts.URL),
1557+
TokenURL: ts.tokenURL(),
14781558
}
14791559

14801560
var unmarshalledConfig OAuth2
1481-
err := yaml.Unmarshal([]byte(yamlConfig), &unmarshalledConfig)
1482-
if err != nil {
1561+
if err := yaml.Unmarshal([]byte(yamlConfig), &unmarshalledConfig); err != nil {
14831562
t.Fatalf("Expected no error unmarshalling yaml, got %v", err)
14841563
}
14851564
if !reflect.DeepEqual(unmarshalledConfig, expectedConfig) {
@@ -1491,9 +1570,59 @@ endpoint_params:
14911570
client := http.Client{
14921571
Transport: rt,
14931572
}
1494-
resp, _ := client.Get(ts.URL)
1573+
1574+
// Default secret.
1575+
expectedAuth = "Basic MToy"
1576+
resp, err := client.Get(ts.url())
1577+
if err != nil {
1578+
t.Fatal(err)
1579+
}
14951580

14961581
authorization := resp.Request.Header.Get("Authorization")
1582+
if authorization != "Bearer 12345" {
1583+
t.Fatalf("Expected authorization header to be 'Bearer', got '%s'", authorization)
1584+
}
1585+
1586+
// Making a second request with the same secret should not re-call the token API.
1587+
_, err = client.Get(ts.url())
1588+
if err != nil {
1589+
t.Fatal(err)
1590+
}
1591+
1592+
// Empty secret.
1593+
expectedAuth = "Basic MTo="
1594+
expectedConfig.ClientSecret = ""
1595+
resp, err = client.Get(ts.url())
1596+
if err != nil {
1597+
t.Fatal(err)
1598+
}
1599+
1600+
authorization = resp.Request.Header.Get("Authorization")
1601+
if authorization != "Bearer 12345" {
1602+
t.Fatalf("Expected authorization header to be 'Bearer 12345', got '%s'", authorization)
1603+
}
1604+
1605+
// Making a second request with the same secret should not re-call the token API.
1606+
resp, err = client.Get(ts.url())
1607+
if err != nil {
1608+
t.Fatal(err)
1609+
}
1610+
1611+
// Update secret.
1612+
expectedAuth = "Basic MToxMjM0NTY3"
1613+
expectedConfig.ClientSecret = "1234567"
1614+
_, err = client.Get(ts.url())
1615+
if err != nil {
1616+
t.Fatal(err)
1617+
}
1618+
1619+
// Making a second request with the same secret should not re-call the token API.
1620+
_, err = client.Get(ts.url())
1621+
if err != nil {
1622+
t.Fatal(err)
1623+
}
1624+
1625+
authorization = resp.Request.Header.Get("Authorization")
14971626
if authorization != "Bearer 12345" {
14981627
t.Fatalf("Expected authorization header to be 'Bearer 12345', got '%s'", authorization)
14991628
}
@@ -1543,33 +1672,9 @@ func TestOAuth2UserAgent(t *testing.T) {
15431672
}
15441673

15451674
func TestOAuth2WithFile(t *testing.T) {
1546-
var expectedAuth *string
1547-
var previousAuth string
1548-
tokenTS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1549-
auth := r.Header.Get("Authorization")
1550-
if auth != *expectedAuth {
1551-
t.Fatalf("bad auth, expected %s, got %s", *expectedAuth, auth)
1552-
}
1553-
if auth == previousAuth {
1554-
t.Fatal("token endpoint called twice")
1555-
}
1556-
previousAuth = auth
1557-
res, _ := json.Marshal(oauth2TestServerResponse{
1558-
AccessToken: "12345",
1559-
TokenType: "Bearer",
1560-
})
1561-
w.Header().Add("Content-Type", "application/json")
1562-
_, _ = w.Write(res)
1563-
}))
1564-
defer tokenTS.Close()
1565-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1566-
auth := r.Header.Get("Authorization")
1567-
if auth != "Bearer 12345" {
1568-
t.Fatalf("bad auth, expected %s, got %s", "Bearer 12345", auth)
1569-
}
1570-
fmt.Fprintln(w, "Hello, client")
1571-
}))
1572-
defer ts.Close()
1675+
var expectedAuth string
1676+
ts := newTestOAuthServer(t, &expectedAuth)
1677+
defer ts.close()
15731678

15741679
secretFile, err := os.CreateTemp("", "oauth2_secret")
15751680
if err != nil {
@@ -1586,13 +1691,13 @@ scopes:
15861691
token_url: %s
15871692
endpoint_params:
15881693
hi: hello
1589-
`, secretFile.Name(), tokenTS.URL)
1694+
`, secretFile.Name(), ts.tokenURL())
15901695
expectedConfig := OAuth2{
15911696
ClientID: "1",
15921697
ClientSecretFile: secretFile.Name(),
15931698
Scopes: []string{"A", "B"},
15941699
EndpointParams: map[string]string{"hi": "hello"},
1595-
TokenURL: tokenTS.URL,
1700+
TokenURL: ts.tokenURL(),
15961701
}
15971702

15981703
var unmarshalledConfig OAuth2
@@ -1610,40 +1715,57 @@ endpoint_params:
16101715
Transport: rt,
16111716
}
16121717

1613-
tk := "Basic MToxMjM0NTY="
1614-
expectedAuth = &tk
1718+
// Empty secret file.
1719+
expectedAuth = "Basic MTo="
1720+
resp, err := client.Get(ts.url())
1721+
if err != nil {
1722+
t.Fatal(err)
1723+
}
1724+
1725+
authorization := resp.Request.Header.Get("Authorization")
1726+
if authorization != "Bearer 12345" {
1727+
t.Fatalf("Expected authorization header to be 'Bearer', got '%s'", authorization)
1728+
}
1729+
1730+
// Making a second request with the same file content should not re-call the token API.
1731+
_, err = client.Get(ts.url())
1732+
if err != nil {
1733+
t.Fatal(err)
1734+
}
1735+
1736+
// File populated.
1737+
expectedAuth = "Basic MToxMjM0NTY="
16151738
if _, err := secretFile.Write([]byte("123456")); err != nil {
16161739
t.Fatal(err)
16171740
}
1618-
resp, err := client.Get(ts.URL)
1741+
resp, err = client.Get(ts.url())
16191742
if err != nil {
16201743
t.Fatal(err)
16211744
}
16221745

1623-
authorization := resp.Request.Header.Get("Authorization")
1746+
authorization = resp.Request.Header.Get("Authorization")
16241747
if authorization != "Bearer 12345" {
16251748
t.Fatalf("Expected authorization header to be 'Bearer 12345', got '%s'", authorization)
16261749
}
16271750

16281751
// Making a second request with the same file content should not re-call the token API.
1629-
resp, err = client.Get(ts.URL)
1752+
resp, err = client.Get(ts.url())
16301753
if err != nil {
16311754
t.Fatal(err)
16321755
}
16331756

1634-
tk = "Basic MToxMjM0NTY3"
1635-
expectedAuth = &tk
1757+
// Update file.
1758+
expectedAuth = "Basic MToxMjM0NTY3"
16361759
if _, err := secretFile.Write([]byte("7")); err != nil {
16371760
t.Fatal(err)
16381761
}
1639-
1640-
_, err = client.Get(ts.URL)
1762+
_, err = client.Get(ts.url())
16411763
if err != nil {
16421764
t.Fatal(err)
16431765
}
16441766

16451767
// Making a second request with the same file content should not re-call the token API.
1646-
_, err = client.Get(ts.URL)
1768+
_, err = client.Get(ts.url())
16471769
if err != nil {
16481770
t.Fatal(err)
16491771
}

‎config/testdata/http.conf.oauth2-no-client-secret.bad.yaml

-3
This file was deleted.

0 commit comments

Comments
 (0)