Skip to content

Commit c50beac

Browse files
BigTailWolfBigTailWolf
committed
address comments
1 parent 399b52f commit c50beac

File tree

7 files changed

+70
-93
lines changed

7 files changed

+70
-93
lines changed

‎google/internal/externalaccount/basecredentials.go

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,12 @@ import (
88
"context"
99
"fmt"
1010
"net/http"
11-
"net/url"
1211
"regexp"
1312
"strconv"
14-
"strings"
1513
"time"
1614

1715
"golang.org/x/oauth2"
18-
"golang.org/x/oauth2/google/internal/sts_exchange"
16+
"golang.org/x/oauth2/google/internal/stsexchange"
1917
)
2018

2119
// now aliases time.Now for testing
@@ -64,31 +62,10 @@ type Config struct {
6462
WorkforcePoolUserProject string
6563
}
6664

67-
// Each element consists of a list of patterns. validateURLs checks for matches
68-
// that include all elements in a given list, in that order.
69-
7065
var (
7166
validWorkforceAudiencePattern *regexp.Regexp = regexp.MustCompile(`//iam\.googleapis\.com/locations/[^/]+/workforcePools/`)
7267
)
7368

74-
func validateURL(input string, patterns []*regexp.Regexp, scheme string) bool {
75-
parsed, err := url.Parse(input)
76-
if err != nil {
77-
return false
78-
}
79-
if !strings.EqualFold(parsed.Scheme, scheme) {
80-
return false
81-
}
82-
toTest := parsed.Host
83-
84-
for _, pattern := range patterns {
85-
if pattern.MatchString(toTest) {
86-
return true
87-
}
88-
}
89-
return false
90-
}
91-
9269
func validateWorkforceAudience(input string) bool {
9370
return validWorkforceAudiencePattern.MatchString(input)
9471
}
@@ -231,7 +208,7 @@ func (ts tokenSource) Token() (*oauth2.Token, error) {
231208
if err != nil {
232209
return nil, err
233210
}
234-
stsRequest := sts_exchange.StsTokenExchangeRequest{
211+
stsRequest := stsexchange.StsTokenExchangeRequest{
235212
GrantType: "urn:ietf:params:oauth:grant-type:token-exchange",
236213
Audience: conf.Audience,
237214
Scope: conf.Scopes,
@@ -242,7 +219,7 @@ func (ts tokenSource) Token() (*oauth2.Token, error) {
242219
header := make(http.Header)
243220
header.Add("Content-Type", "application/x-www-form-urlencoded")
244221
header.Add("x-goog-api-client", getMetricsHeaderValue(conf, credSource))
245-
clientAuth := sts_exchange.ClientAuthentication{
222+
clientAuth := stsexchange.ClientAuthentication{
246223
AuthStyle: oauth2.AuthStyleInHeader,
247224
ClientID: conf.ClientID,
248225
ClientSecret: conf.ClientSecret,
@@ -255,7 +232,7 @@ func (ts tokenSource) Token() (*oauth2.Token, error) {
255232
"userProject": conf.WorkforcePoolUserProject,
256233
}
257234
}
258-
stsResp, err := sts_exchange.ExchangeToken(ts.ctx, conf.TokenURL, &stsRequest, clientAuth, header, options)
235+
stsResp, err := stsexchange.ExchangeToken(ts.ctx, conf.TokenURL, &stsRequest, clientAuth, header, options)
259236
if err != nil {
260237
return nil, err
261238
}

‎google/internal/externalaccountauthorizeduser/externalaccountauthorizeduser.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"time"
1111

1212
"golang.org/x/oauth2"
13-
"golang.org/x/oauth2/google/internal/sts_exchange"
13+
"golang.org/x/oauth2/google/internal/stsexchange"
1414
)
1515

1616
// now aliases time.Now for testing.
@@ -87,13 +87,13 @@ func (ts tokenSource) Token() (*oauth2.Token, error) {
8787
return nil, errors.New("oauth2/google: The credentials do not contain the necessary fields need to refresh the access token. You must specify refresh_token, token_url, client_id, and client_secret.")
8888
}
8989

90-
clientAuth := sts_exchange.ClientAuthentication{
90+
clientAuth := stsexchange.ClientAuthentication{
9191
AuthStyle: oauth2.AuthStyleInHeader,
9292
ClientID: conf.ClientID,
9393
ClientSecret: conf.ClientSecret,
9494
}
9595

96-
stsResponse, err := sts_exchange.RefreshAccessToken(ts.ctx, conf.TokenURL, conf.RefreshToken, clientAuth, nil)
96+
stsResponse, err := stsexchange.RefreshAccessToken(ts.ctx, conf.TokenURL, conf.RefreshToken, clientAuth, nil)
9797
if err != nil {
9898
return nil, err
9999
}

‎google/internal/externalaccountauthorizeduser/externalaccountauthorizeduser_test.go

Lines changed: 59 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
"time"
1616

1717
"golang.org/x/oauth2"
18-
"golang.org/x/oauth2/google/internal/sts_exchange"
18+
"golang.org/x/oauth2/google/internal/stsexchange"
1919
)
2020

2121
const expiryDelta = 10 * time.Second
@@ -33,59 +33,11 @@ type testRefreshTokenServer struct {
3333
Authorization string
3434
ContentType string
3535
Body string
36-
ResponsePayload *sts_exchange.Response
36+
ResponsePayload *stsexchange.Response
3737
Response string
3838
server *httptest.Server
3939
}
4040

41-
func (trts *testRefreshTokenServer) Run(t *testing.T) (string, error) {
42-
if trts.server != nil {
43-
return "", errors.New("Server is already running")
44-
}
45-
trts.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
46-
if got, want := r.URL.String(), trts.URL; got != want {
47-
t.Errorf("URL.String(): got %v but want %v", got, want)
48-
}
49-
headerAuth := r.Header.Get("Authorization")
50-
if got, want := headerAuth, trts.Authorization; got != want {
51-
t.Errorf("got %v but want %v", got, want)
52-
}
53-
headerContentType := r.Header.Get("Content-Type")
54-
if got, want := headerContentType, trts.ContentType; got != want {
55-
t.Errorf("got %v but want %v", got, want)
56-
}
57-
body, err := ioutil.ReadAll(r.Body)
58-
if err != nil {
59-
t.Fatalf("Failed reading request body: %s.", err)
60-
}
61-
if got, want := string(body), trts.Body; got != want {
62-
t.Errorf("Unexpected exchange payload: got %v but want %v", got, want)
63-
}
64-
w.Header().Set("Content-Type", "application/json")
65-
if trts.ResponsePayload != nil {
66-
content, err := json.Marshal(trts.ResponsePayload)
67-
if err != nil {
68-
t.Fatalf("unable to marshall response JSON")
69-
}
70-
w.Write(content)
71-
} else {
72-
w.Write([]byte(trts.Response))
73-
}
74-
}))
75-
return trts.server.URL, nil
76-
}
77-
78-
func (trts *testRefreshTokenServer) Close() error {
79-
if trts.server == nil {
80-
return errors.New("No server is running")
81-
}
82-
trts.server.Close()
83-
trts.server = nil
84-
return nil
85-
}
86-
87-
// Tests
88-
8941
func TestExernalAccountAuthorizedUser_JustToken(t *testing.T) {
9042
config := &Config{
9143
Token: "AAAAAAA",
@@ -111,18 +63,18 @@ func TestExernalAccountAuthorizedUser_TokenRefreshWithRefreshTokenInRespondse(t
11163
Authorization: "Basic Q0xJRU5UX0lEOkNMSUVOVF9TRUNSRVQ=",
11264
ContentType: "application/x-www-form-urlencoded",
11365
Body: "grant_type=refresh_token&refresh_token=BBBBBBBBB",
114-
ResponsePayload: &sts_exchange.Response{
66+
ResponsePayload: &stsexchange.Response{
11567
ExpiresIn: 3600,
11668
AccessToken: "AAAAAAA",
11769
RefreshToken: "CCCCCCC",
11870
},
11971
}
12072

121-
url, err := server.Run(t)
73+
url, err := server.run(t)
12274
if err != nil {
12375
t.Fatalf("Error starting server")
12476
}
125-
defer server.Close()
77+
defer server.close(t)
12678

12779
config := &Config{
12880
RefreshToken: "BBBBBBBBB",
@@ -153,17 +105,17 @@ func TestExernalAccountAuthorizedUser_MinimumFieldsRequiredForRefresh(t *testing
153105
Authorization: "Basic Q0xJRU5UX0lEOkNMSUVOVF9TRUNSRVQ=",
154106
ContentType: "application/x-www-form-urlencoded",
155107
Body: "grant_type=refresh_token&refresh_token=BBBBBBBBB",
156-
ResponsePayload: &sts_exchange.Response{
108+
ResponsePayload: &stsexchange.Response{
157109
ExpiresIn: 3600,
158110
AccessToken: "AAAAAAA",
159111
},
160112
}
161113

162-
url, err := server.Run(t)
114+
url, err := server.run(t)
163115
if err != nil {
164116
t.Fatalf("Error starting server")
165117
}
166-
defer server.Close()
118+
defer server.close(t)
167119

168120
config := &Config{
169121
RefreshToken: "BBBBBBBBB",
@@ -191,17 +143,17 @@ func TestExternalAccountAuthorizedUser_MissingRefreshFields(t *testing.T) {
191143
Authorization: "Basic Q0xJRU5UX0lEOkNMSUVOVF9TRUNSRVQ=",
192144
ContentType: "application/x-www-form-urlencoded",
193145
Body: "grant_type=refresh_token&refresh_token=BBBBBBBBB",
194-
ResponsePayload: &sts_exchange.Response{
146+
ResponsePayload: &stsexchange.Response{
195147
ExpiresIn: 3600,
196148
AccessToken: "AAAAAAA",
197149
},
198150
}
199151

200-
url, err := server.Run(t)
152+
url, err := server.run(t)
201153
if err != nil {
202154
t.Fatalf("Error starting server")
203155
}
204-
defer server.Close()
156+
defer server.close(t)
205157
testCases := []struct {
206158
name string
207159
config Config
@@ -257,3 +209,51 @@ func TestExternalAccountAuthorizedUser_MissingRefreshFields(t *testing.T) {
257209
})
258210
}
259211
}
212+
213+
func (trts *testRefreshTokenServer) run(t *testing.T) (string, error) {
214+
t.Helper()
215+
if trts.server != nil {
216+
return "", errors.New("Server is already running")
217+
}
218+
trts.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
219+
if got, want := r.URL.String(), trts.URL; got != want {
220+
t.Errorf("URL.String(): got %v but want %v", got, want)
221+
}
222+
headerAuth := r.Header.Get("Authorization")
223+
if got, want := headerAuth, trts.Authorization; got != want {
224+
t.Errorf("got %v but want %v", got, want)
225+
}
226+
headerContentType := r.Header.Get("Content-Type")
227+
if got, want := headerContentType, trts.ContentType; got != want {
228+
t.Errorf("got %v but want %v", got, want)
229+
}
230+
body, err := ioutil.ReadAll(r.Body)
231+
if err != nil {
232+
t.Fatalf("Failed reading request body: %s.", err)
233+
}
234+
if got, want := string(body), trts.Body; got != want {
235+
t.Errorf("Unexpected exchange payload: got %v but want %v", got, want)
236+
}
237+
w.Header().Set("Content-Type", "application/json")
238+
if trts.ResponsePayload != nil {
239+
content, err := json.Marshal(trts.ResponsePayload)
240+
if err != nil {
241+
t.Fatalf("unable to marshall response JSON")
242+
}
243+
w.Write(content)
244+
} else {
245+
w.Write([]byte(trts.Response))
246+
}
247+
}))
248+
return trts.server.URL, nil
249+
}
250+
251+
func (trts *testRefreshTokenServer) close(t *testing.T) error {
252+
t.Helper()
253+
if trts.server == nil {
254+
return errors.New("No server is running")
255+
}
256+
trts.server.Close()
257+
trts.server = nil
258+
return nil
259+
}

‎google/internal/sts_exchange/clientauth.go renamed to ‎google/internal/stsexchange/clientauth.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
package sts_exchange
5+
package stsexchange
66

77
import (
88
"encoding/base64"

‎google/internal/sts_exchange/clientauth_test.go renamed to ‎google/internal/stsexchange/clientauth_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
package sts_exchange
5+
package stsexchange
66

77
import (
88
"net/http"

‎google/internal/sts_exchange/sts_exchange.go renamed to ‎google/internal/stsexchange/sts_exchange.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
package sts_exchange
5+
package stsexchange
66

77
import (
88
"context"

‎google/internal/sts_exchange/sts_exchange_test.go renamed to ‎google/internal/stsexchange/sts_exchange_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
package sts_exchange
5+
package stsexchange
66

77
import (
88
"context"

0 commit comments

Comments
 (0)