Skip to content

Commit 80416a3

Browse files
committed
fix(cli): address Copilot review feedback
- Align error message wording to say "retry HTTP client" in loadConfig - Add token_type_hint assertions to revocation tests for RFC 7009 compliance
1 parent aeee301 commit 80416a3

2 files changed

Lines changed: 36 additions & 17 deletions

File tree

config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func loadConfig() *AppConfig {
236236
var err error
237237
cfg.RetryClient, err = retry.NewBackgroundClient(retry.WithHTTPClient(baseHTTPClient))
238238
if err != nil {
239-
fmt.Fprintf(os.Stderr, "Error: failed to create HTTP client: %v\n", err)
239+
fmt.Fprintf(os.Stderr, "Error: failed to create retry HTTP client: %v\n", err)
240240
os.Exit(1)
241241
}
242242

token_cmd_test.go

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,11 @@ func TestRunTokenDelete(t *testing.T) {
106106

107107
func TestRunTokenDelete_ServerRevocation(t *testing.T) {
108108
t.Run("successful revocation and local delete", func(t *testing.T) {
109-
var revokedTokens []string
109+
type revokeCall struct {
110+
token string
111+
tokenTypeHint string
112+
}
113+
var revokeCalls []revokeCall
110114
var mu sync.Mutex
111115
srv := httptest.NewServer(
112116
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -115,7 +119,10 @@ func TestRunTokenDelete_ServerRevocation(t *testing.T) {
115119
return
116120
}
117121
mu.Lock()
118-
revokedTokens = append(revokedTokens, r.FormValue("token"))
122+
revokeCalls = append(revokeCalls, revokeCall{
123+
token: r.FormValue("token"),
124+
tokenTypeHint: r.FormValue("token_type_hint"),
125+
})
119126
mu.Unlock()
120127
w.WriteHeader(http.StatusOK)
121128
}),
@@ -163,16 +170,24 @@ func TestRunTokenDelete_ServerRevocation(t *testing.T) {
163170

164171
mu.Lock()
165172
defer mu.Unlock()
166-
if len(revokedTokens) != 2 {
167-
t.Fatalf("expected 2 revoke calls, got %d", len(revokedTokens))
173+
if len(revokeCalls) != 2 {
174+
t.Fatalf("expected 2 revoke calls, got %d", len(revokeCalls))
168175
}
169176
// Revocations run concurrently, so order is non-deterministic.
170-
got := map[string]bool{revokedTokens[0]: true, revokedTokens[1]: true}
171-
if !got["refresh-456"] {
172-
t.Errorf("expected refresh token to be revoked, got %v", revokedTokens)
177+
// Build a map from token to its type hint for assertion.
178+
hintByToken := make(map[string]string, len(revokeCalls))
179+
for _, c := range revokeCalls {
180+
hintByToken[c.token] = c.tokenTypeHint
181+
}
182+
if hint, ok := hintByToken["refresh-456"]; !ok {
183+
t.Errorf("expected refresh token to be revoked, got %v", revokeCalls)
184+
} else if hint != "refresh_token" {
185+
t.Errorf("refresh token_type_hint: got %q, want %q", hint, "refresh_token")
173186
}
174-
if !got["access-123"] {
175-
t.Errorf("expected access token to be revoked, got %v", revokedTokens)
187+
if hint, ok := hintByToken["access-123"]; !ok {
188+
t.Errorf("expected access token to be revoked, got %v", revokeCalls)
189+
} else if hint != "access_token" {
190+
t.Errorf("access token_type_hint: got %q, want %q", hint, "access_token")
176191
}
177192
})
178193

@@ -269,16 +284,20 @@ func TestRunTokenDelete_ServerRevocation(t *testing.T) {
269284
})
270285

271286
t.Run("only access token no refresh token", func(t *testing.T) {
272-
var revokedTokens []string
273-
var mu sync.Mutex
287+
var (
288+
gotToken string
289+
gotTokenTypeHint string
290+
mu sync.Mutex
291+
)
274292
srv := httptest.NewServer(
275293
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
276294
if err := r.ParseForm(); err != nil {
277295
http.Error(w, "bad form", http.StatusBadRequest)
278296
return
279297
}
280298
mu.Lock()
281-
revokedTokens = append(revokedTokens, r.FormValue("token"))
299+
gotToken = r.FormValue("token")
300+
gotTokenTypeHint = r.FormValue("token_type_hint")
282301
mu.Unlock()
283302
w.WriteHeader(http.StatusOK)
284303
}),
@@ -319,11 +338,11 @@ func TestRunTokenDelete_ServerRevocation(t *testing.T) {
319338

320339
mu.Lock()
321340
defer mu.Unlock()
322-
if len(revokedTokens) != 1 {
323-
t.Fatalf("expected 1 revoke call (access only), got %d", len(revokedTokens))
341+
if gotToken != "access-only" {
342+
t.Errorf("token: got %q, want %q", gotToken, "access-only")
324343
}
325-
if revokedTokens[0] != "access-only" {
326-
t.Errorf("expected access token, got %q", revokedTokens[0])
344+
if gotTokenTypeHint != "access_token" {
345+
t.Errorf("token_type_hint: got %q, want %q", gotTokenTypeHint, "access_token")
327346
}
328347
})
329348
}

0 commit comments

Comments
 (0)