From 6fd4e4e66115bc7ad57dc7c54d47c54e15ccc5b8 Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 26 Aug 2025 19:42:33 +0300 Subject: [PATCH 01/13] feat(auth): introduce authenticated client in Go using the Legacy CLI 1. Use the Legacy CLI for getting a token 2. If the token is not valid (ie expired), force re-authentication 3. If the client receives a 401 response, force re-authentication --- go.mod | 1 + go.sum | 2 + internal/auth/client.go | 33 +++++++++++++ internal/auth/jwt.go | 52 +++++++++++++++++++++ internal/auth/legacy.go | 96 ++++++++++++++++++++++++++++++++++++++ internal/auth/transport.go | 95 +++++++++++++++++++++++++++++++++++++ internal/config/schema.go | 4 +- internal/legacy/legacy.go | 4 ++ 8 files changed, 286 insertions(+), 1 deletion(-) create mode 100644 internal/auth/client.go create mode 100644 internal/auth/jwt.go create mode 100644 internal/auth/legacy.go create mode 100644 internal/auth/transport.go diff --git a/go.mod b/go.mod index f1ca1550..6fcf22e5 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( github.com/symfony-cli/terminal v1.0.7 github.com/wk8/go-ordered-map/v2 v2.1.8 golang.org/x/crypto v0.37.0 + golang.org/x/oauth2 v0.30.0 golang.org/x/sync v0.13.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index 3e2c3f24..c871ecac 100644 --- a/go.sum +++ b/go.sum @@ -148,6 +148,8 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY= golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E= +golang.org/x/oauth2 v0.30.0 h1:dnDm7JmhM45NNpd8FDDeLhK6FwqbOf4MLCM9zb1BOHI= +golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKlU= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610= diff --git a/internal/auth/client.go b/internal/auth/client.go new file mode 100644 index 00000000..80965f92 --- /dev/null +++ b/internal/auth/client.go @@ -0,0 +1,33 @@ +package auth + +import ( + "context" + "fmt" + "net/http" + + "github.com/platformsh/cli/internal/legacy" + "golang.org/x/oauth2" +) + +func NewLegacyCLIClient(ctx context.Context, wrapper *legacy.CLIWrapper) (*http.Client, error) { + ts, err := NewLegacyCLITokenSource(ctx, wrapper) + if err != nil { + return nil, fmt.Errorf("oauth2: create token source: %w", err) + } + + refresher := ts.(refresher) + baseRT := http.DefaultTransport + if rt, ok := TransportFromContext(ctx); ok && rt != nil { + baseRT = rt + } + return &http.Client{ + Transport: &Transport{ + refresher: refresher, + base: &oauth2.Transport{ + Source: ts, + Base: baseRT, + }, + wrapper: wrapper, + }, + }, nil +} diff --git a/internal/auth/jwt.go b/internal/auth/jwt.go new file mode 100644 index 00000000..c1df1489 --- /dev/null +++ b/internal/auth/jwt.go @@ -0,0 +1,52 @@ +package auth + +import ( + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "strings" +) + +// jwtClaims represents the expected claims contained in the JWT token. +type jwtClaims struct { + AccessID string `json:"access_id"` + Actor map[string]any `json:"act,omitempty"` + AuthMethods []string `json:"amr,omitempty"` + AuthenticationTime int64 `json:"auth_time,omitempty"` + ClientID string `json:"cid,omitempty"` + ExpiresAt int64 `json:"exp,omitempty"` + GrantType string `json:"grant,omitempty"` + IssuedAt int64 `json:"iat,omitempty"` + Issuer string `json:"iss,omitempty"` + JWTID string `json:"jti,omitempty"` + NotBefore int64 `json:"nbf,omitempty"` + Namespace string `json:"ns,omitempty"` + Scopes []string `json:"scp,omitempty"` + Subject string `json:"sub,omitempty"` +} + +// unsafeParseJWT parses a JWT without verifying its signature and returns its claims. +// WARNING: This is intentionally unsafe and must not be used for trust decisions. +func unsafeParseJWT(token string) (*jwtClaims, error) { + if token == "" { + return nil, errors.New("jwt: empty token") + } + parts := strings.Split(token, ".") + if len(parts) < 2 { + return nil, fmt.Errorf("jwt: malformed token, expected 3 parts, got %d", len(parts)) + } + payloadSeg := parts[1] + + // Base64 URL decode without padding as per RFC 7515. + payloadBytes, err := base64.RawURLEncoding.DecodeString(payloadSeg) + if err != nil { + return nil, fmt.Errorf("jwt: decode payload: %w", err) + } + + var claims jwtClaims + if err := json.Unmarshal(payloadBytes, &claims); err != nil { + return nil, fmt.Errorf("jwt: unmarshal claims: %w", err) + } + return &claims, nil +} diff --git a/internal/auth/legacy.go b/internal/auth/legacy.go new file mode 100644 index 00000000..807d1539 --- /dev/null +++ b/internal/auth/legacy.go @@ -0,0 +1,96 @@ +package auth + +import ( + "bytes" + "context" + "fmt" + "io" + "sync" + "time" + + "github.com/platformsh/cli/internal/legacy" + "golang.org/x/oauth2" +) + +type legacyCLITokenSource struct { + ctx context.Context + cached *oauth2.Token + wrapper *legacy.CLIWrapper + mu sync.Mutex +} + +func (ts *legacyCLITokenSource) getLegacyCLIToken() (*oauth2.Token, error) { + bt := bytes.NewBuffer(nil) + ts.wrapper.Stdout = bt + if err := ts.wrapper.Exec(ts.ctx, "auth:token", "-W"); err != nil { + return nil, fmt.Errorf("cannot retrieve token: %w", err) + } + + at, err := unsafeParseJWT(bt.String()) + + if err != nil { + return nil, fmt.Errorf("cannot parse token: %w", err) + } + + return &oauth2.Token{ + AccessToken: bt.String(), + TokenType: "Bearer", + Expiry: time.Unix(at.ExpiresAt, 0), + }, nil +} + +func (ts *legacyCLITokenSource) refreshToken() error { + ts.cached = nil + ts.wrapper.Stdout = io.Discard + if err := ts.wrapper.Exec(ts.ctx, "auth:info", "--refresh"); err != nil { + return fmt.Errorf("cannot refresh token: %w", err) + } + + return nil +} + +func (ts *legacyCLITokenSource) invalidateToken() error { + if ts.cached != nil { + ts.cached.AccessToken = "" + } + + return nil +} + +func (ts *legacyCLITokenSource) Token() (*oauth2.Token, error) { + ts.mu.Lock() + defer ts.mu.Unlock() + + if ts.cached == nil { + tok, err := ts.getLegacyCLIToken() + if err != nil { + return nil, err + } + ts.cached = tok + } + + if ts.cached != nil && ts.cached.Valid() { + return ts.cached, nil + } + + if err := ts.refreshToken(); err != nil { + return nil, err + } + + tok, err := ts.getLegacyCLIToken() + if err != nil { + return nil, err + } + + ts.cached = tok + return ts.cached, nil +} + +func NewLegacyCLITokenSource(ctx context.Context, wrapper *legacy.CLIWrapper) (oauth2.TokenSource, error) { + wrapper.ForceColor = true + wrapper.DisableInteraction = true + return &legacyCLITokenSource{ + ctx: ctx, + wrapper: wrapper, + }, nil +} diff --git a/internal/auth/transport.go b/internal/auth/transport.go new file mode 100644 index 00000000..0f29007c --- /dev/null +++ b/internal/auth/transport.go @@ -0,0 +1,95 @@ +package auth + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "net/http" + "os" + + "github.com/platformsh/cli/internal/legacy" +) + +type refresher interface { + refreshToken() error + invalidateToken() error +} + +// Transport is an HTTP RoundTripper similar to golang.org/x/oauth2.Transport. +// It injects Authorization headers using a savingSource and, on a 401 response, +// clears the cached token and retries the request once. +type Transport struct { + // base is the underlying oauth2.Transport that adds the Authorization header. + base http.RoundTripper + + // refresher is the savingSource used as the TokenSource for base; kept private + // so we can clear its cached token on 401. + refresher refresher + + wrapper *legacy.CLIWrapper +} + +// RoundTrip adds Authorization via the underlying oauth2.Transport. If the +// response is 401 Unauthorized, it clears the cached token and retries once. +func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { + req.Body = wrapReader(req.Body) + + resp, err := t.base.RoundTrip(req) + + // Retry on 401 + if resp != nil && resp.StatusCode == http.StatusUnauthorized { + fmt.Fprintln(os.Stderr, "401: refreshing token...") + t.refresher.invalidateToken() + flushReader(resp.Body) + resp, err = t.base.RoundTrip(req) + } + + if errors.Is(err, ErrNoValidCredentials) { + fmt.Fprintln(os.Stderr, "invalid credentials: re-authenticating...") + t.refresher.refreshToken() + resp, err = t.base.RoundTrip(req) + } + + return resp, err +} + +// context key for storing a custom RoundTripper. +type transportCtxKey struct{} + +// WithTransport returns a new context that carries the provided RoundTripper. +func WithTransport(ctx context.Context, rt http.RoundTripper) context.Context { + return context.WithValue(ctx, transportCtxKey{}, rt) +} + +// TransportFromContext retrieves a RoundTripper previously stored with +// WithTransport. It returns (nil, false) if none is set. +func TransportFromContext(ctx context.Context) (http.RoundTripper, bool) { + v := ctx.Value(transportCtxKey{}) + if v == nil { + return nil, false + } + rt, ok := v.(http.RoundTripper) + if !ok || rt == nil { + return nil, false + } + return rt, true +} + +func wrapReader(r io.ReadCloser) io.ReadCloser { + if r == nil { + return nil + } + bodyBytes, _ := io.ReadAll(r) + _ = r.Close() + return io.NopCloser(bytes.NewBuffer(bodyBytes)) +} + +func flushReader(r io.ReadCloser) { + if r == nil { + return + } + _, _ = io.Copy(io.Discard, r) + _ = r.Close() +} diff --git a/internal/config/schema.go b/internal/config/schema.go index 16f61c46..ac91613b 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -45,8 +45,10 @@ type Config struct { BaseURL string `validate:"required,url" yaml:"base_url"` // e.g. "https://api.platform.sh" AuthURL string `validate:"omitempty,url" yaml:"auth_url,omitempty"` // e.g. "https://auth.api.platform.sh" - UserAgent string `validate:"omitempty" yaml:"user_agent,omitempty"` // a template - see UserAgent method + UserAgent string `validate:"omitempty" yaml:"user_agent,omitempty"` // a template - see UserAgent method + SessionID string `validate:"omitempty,ascii" yaml:"session_id,omitempty"` // the ID for the authentication session - defaults to "default" + OAuth2ClientID string `validate:"required_without=AuthURL,omitempty" yaml:"oauth2_client_id,omitempty"` // e.g. "upsun-cli" OAuth2AuthorizeURL string `validate:"required_without=AuthURL,omitempty,url" yaml:"oauth2_auth_url,omitempty"` // e.g. "https://auth.api.platform.sh/oauth2/authorize" OAuth2RevokeURL string `validate:"required_without=AuthURL,omitempty,url" yaml:"oauth2_revoke_url,omitempty"` // e.g. "https://auth.api.platform.sh/oauth2/revoke" OAuth2TokenURL string `validate:"required_without=AuthURL,omitempty,url" yaml:"oauth2_token_url,omitempty"` // e.g. "https://auth.api.platform.sh/oauth2/token" diff --git a/internal/legacy/legacy.go b/internal/legacy/legacy.go index a04edfe3..0ca66192 100644 --- a/internal/legacy/legacy.go +++ b/internal/legacy/legacy.go @@ -39,6 +39,7 @@ type CLIWrapper struct { Version string Debug bool DisableInteraction bool + ForceColor bool DebugLogFunc func(string, ...any) initOnce sync.Once @@ -159,6 +160,9 @@ func (c *CLIWrapper) Exec(ctx context.Context, args ...string) error { if c.DisableInteraction { cmd.Env = append(cmd.Env, envPrefix+"NO_INTERACTION=1") } + if c.ForceColor { + cmd.Env = append(cmd.Env, "CLICOLOR_FORCE=1") + } cmd.Env = append(cmd.Env, fmt.Sprintf( "%sUSER_AGENT={APP_NAME_DASH}/%s ({UNAME_S}; {UNAME_R}; PHP %s; WRAPPER %s)", envPrefix, From 7e77910b13a79f70f46ac6e1d8e224c2beaa15f9 Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 2 Sep 2025 14:50:54 +0300 Subject: [PATCH 02/13] feat: improve logging in the transport --- internal/auth/client.go | 11 +++++++++-- internal/auth/transport.go | 23 ++++++++++++----------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/internal/auth/client.go b/internal/auth/client.go index 80965f92..bc4538a9 100644 --- a/internal/auth/client.go +++ b/internal/auth/client.go @@ -3,10 +3,13 @@ package auth import ( "context" "fmt" + "log" "net/http" + "os" - "github.com/platformsh/cli/internal/legacy" "golang.org/x/oauth2" + + "github.com/platformsh/cli/internal/legacy" ) func NewLegacyCLIClient(ctx context.Context, wrapper *legacy.CLIWrapper) (*http.Client, error) { @@ -15,7 +18,10 @@ func NewLegacyCLIClient(ctx context.Context, wrapper *legacy.CLIWrapper) (*http. return nil, fmt.Errorf("oauth2: create token source: %w", err) } - refresher := ts.(refresher) + refresher, ok := ts.(refresher) + if !ok { + return nil, fmt.Errorf("token source does not implement refresher") + } baseRT := http.DefaultTransport if rt, ok := TransportFromContext(ctx); ok && rt != nil { baseRT = rt @@ -28,6 +34,7 @@ func NewLegacyCLIClient(ctx context.Context, wrapper *legacy.CLIWrapper) (*http. Base: baseRT, }, wrapper: wrapper, + logger: log.New(os.Stderr, "", 0), }, }, nil } diff --git a/internal/auth/transport.go b/internal/auth/transport.go index 0f29007c..108498db 100644 --- a/internal/auth/transport.go +++ b/internal/auth/transport.go @@ -3,11 +3,9 @@ package auth import ( "bytes" "context" - "errors" - "fmt" "io" + "log" "net/http" - "os" "github.com/platformsh/cli/internal/legacy" ) @@ -29,6 +27,7 @@ type Transport struct { refresher refresher wrapper *legacy.CLIWrapper + logger *log.Logger } // RoundTrip adds Authorization via the underlying oauth2.Transport. If the @@ -40,21 +39,23 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { // Retry on 401 if resp != nil && resp.StatusCode == http.StatusUnauthorized { - fmt.Fprintln(os.Stderr, "401: refreshing token...") - t.refresher.invalidateToken() + _ = t.log("The access token has been refreshed. Retrying request.") + _ = t.refresher.invalidateToken() flushReader(resp.Body) resp, err = t.base.RoundTrip(req) } - if errors.Is(err, ErrNoValidCredentials) { - fmt.Fprintln(os.Stderr, "invalid credentials: re-authenticating...") - t.refresher.refreshToken() - resp, err = t.base.RoundTrip(req) - } - return resp, err } +func (t *Transport) log(msg string, args ...any) error { + if t.logger == nil { + return nil + } + t.logger.Printf(msg, args...) + return nil +} + // context key for storing a custom RoundTripper. type transportCtxKey struct{} From 85d89f2f0e7f0bf6f37a05ab499f44fa1230d0fc Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 2 Sep 2025 14:51:13 +0300 Subject: [PATCH 03/13] feat: fix locking --- internal/auth/legacy.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/internal/auth/legacy.go b/internal/auth/legacy.go index 807d1539..fe1e3340 100644 --- a/internal/auth/legacy.go +++ b/internal/auth/legacy.go @@ -8,8 +8,9 @@ import ( "sync" "time" - "github.com/platformsh/cli/internal/legacy" "golang.org/x/oauth2" + + "github.com/platformsh/cli/internal/legacy" ) type legacyCLITokenSource struct { @@ -19,7 +20,7 @@ type legacyCLITokenSource struct { mu sync.Mutex } -func (ts *legacyCLITokenSource) getLegacyCLIToken() (*oauth2.Token, error) { +func (ts *legacyCLITokenSource) unsafeGetLegacyCLIToken() (*oauth2.Token, error) { bt := bytes.NewBuffer(nil) ts.wrapper.Stdout = bt if err := ts.wrapper.Exec(ts.ctx, "auth:token", "-W"); err != nil { @@ -40,6 +41,13 @@ func (ts *legacyCLITokenSource) getLegacyCLIToken() (*oauth2.Token, error) { } func (ts *legacyCLITokenSource) refreshToken() error { + ts.mu.Lock() + defer ts.mu.Unlock() + + return ts.unsafeRefreshToken() +} + +func (ts *legacyCLITokenSource) unsafeRefreshToken() error { ts.cached = nil ts.wrapper.Stdout = io.Discard if err := ts.wrapper.Exec(ts.ctx, "auth:info", "--refresh"); err != nil { @@ -50,6 +58,13 @@ func (ts *legacyCLITokenSource) refreshToken() error { } func (ts *legacyCLITokenSource) invalidateToken() error { + ts.mu.Lock() + defer ts.mu.Unlock() + + return ts.unsafeInvalidateToken() +} + +func (ts *legacyCLITokenSource) unsafeInvalidateToken() error { if ts.cached != nil { ts.cached.AccessToken = "" } @@ -62,7 +77,7 @@ func (ts *legacyCLITokenSource) Token() (*oauth2.Token, error) { defer ts.mu.Unlock() if ts.cached == nil { - tok, err := ts.getLegacyCLIToken() + tok, err := ts.unsafeGetLegacyCLIToken() if err != nil { return nil, err } @@ -73,11 +88,11 @@ func (ts *legacyCLITokenSource) Token() (*oauth2.Token, error) { return ts.cached, nil } - if err := ts.refreshToken(); err != nil { + if err := ts.unsafeRefreshToken(); err != nil { return nil, err } - tok, err := ts.getLegacyCLIToken() + tok, err := ts.unsafeGetLegacyCLIToken() if err != nil { return nil, err } From 37649cc2194eb77d29e32d29d891efb5723c1d0b Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 2 Sep 2025 15:40:07 +0300 Subject: [PATCH 04/13] feat: simplify parsing of the JWT token --- go.mod | 11 ++++++----- go.sum | 12 ++++++++++++ internal/auth/jwt.go | 17 +++-------------- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index 6fcf22e5..64f9b21c 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/Masterminds/semver/v3 v3.3.1 github.com/fatih/color v1.18.0 github.com/go-chi/chi/v5 v5.2.1 + github.com/go-jose/go-jose/v4 v4.1.2 github.com/go-playground/validator/v10 v10.26.0 github.com/gofrs/flock v0.12.1 github.com/oklog/ulid/v2 v2.1.0 @@ -17,9 +18,9 @@ require ( github.com/stretchr/testify v1.10.0 github.com/symfony-cli/terminal v1.0.7 github.com/wk8/go-ordered-map/v2 v2.1.8 - golang.org/x/crypto v0.37.0 + golang.org/x/crypto v0.39.0 golang.org/x/oauth2 v0.30.0 - golang.org/x/sync v0.13.0 + golang.org/x/sync v0.15.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -63,7 +64,7 @@ require ( github.com/xeipuuv/gojsonschema v1.2.0 // indirect go.uber.org/multierr v1.11.0 // indirect golang.org/x/net v0.39.0 // indirect - golang.org/x/sys v0.32.0 // indirect - golang.org/x/term v0.31.0 // indirect - golang.org/x/text v0.24.0 // indirect + golang.org/x/sys v0.33.0 // indirect + golang.org/x/term v0.32.0 // indirect + golang.org/x/text v0.26.0 // indirect ) diff --git a/go.sum b/go.sum index c871ecac..7ae7a952 100644 --- a/go.sum +++ b/go.sum @@ -32,6 +32,8 @@ github.com/gabriel-vasile/mimetype v1.4.8 h1:FfZ3gj38NjllZIeJAmMhr+qKL8Wu+nOoI3G github.com/gabriel-vasile/mimetype v1.4.8/go.mod h1:ByKUIKGjh1ODkGM1asKUbQZOLGrPjydw3hYPU2YU9t8= github.com/go-chi/chi/v5 v5.2.1 h1:KOIHODQj58PmL80G2Eak4WdvUzjSJSm0vG72crDCqb8= github.com/go-chi/chi/v5 v5.2.1/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops= +github.com/go-jose/go-jose/v4 v4.1.2 h1:TK/7NqRQZfgAh+Td8AlsrvtPoUyiHh0LqVvokh+1vHI= +github.com/go-jose/go-jose/v4 v4.1.2/go.mod h1:22cg9HWM1pOlnRiY+9cQYJ9XHmya1bYW8OeDM6Ku6Oo= github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s= github.com/go-playground/assert/v2 v2.2.0/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= @@ -142,6 +144,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE= golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc= +golang.org/x/crypto v0.39.0 h1:SHs+kF4LP+f+p14esP5jAoDpHU8Gu/v9lFRK6IT5imM= +golang.org/x/crypto v0.39.0/go.mod h1:L+Xg3Wf6HoL4Bn4238Z6ft6KfEpN0tJGo53AAPC632U= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= @@ -154,6 +158,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610= golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8= +golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -165,16 +171,22 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20= golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= +golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.31.0 h1:erwDkOK1Msy6offm1mOgvspSkslFnIGsFnxOKoufg3o= golang.org/x/term v0.31.0/go.mod h1:R4BeIy7D95HzImkxGkTW1UQTtP54tio2RyHz7PwK0aw= +golang.org/x/term v0.32.0 h1:DR4lr0TjUs3epypdhTOkMmuF5CDFJ/8pOnbzMZPQ7bg= +golang.org/x/term v0.32.0/go.mod h1:uZG1FhGx848Sqfsq4/DlJr3xGGsYMu/L5GW4abiaEPQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.24.0 h1:dd5Bzh4yt5KYA8f9CJHCP4FB4D51c2c6JvN37xJJkJ0= golang.org/x/text v0.24.0/go.mod h1:L8rBsPeo2pSS+xqN0d5u2ikmjtmoJbDBT1b7nHvFCdU= +golang.org/x/text v0.26.0 h1:P42AVeLghgTYr4+xUnTRKDMqpar+PtX7KWuNQL21L8M= +golang.org/x/text v0.26.0/go.mod h1:QK15LZJUUQVJxhz7wXgxSy/CJaTFjd0G+YLonydOVQA= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/internal/auth/jwt.go b/internal/auth/jwt.go index c1df1489..29d1b506 100644 --- a/internal/auth/jwt.go +++ b/internal/auth/jwt.go @@ -6,24 +6,13 @@ import ( "errors" "fmt" "strings" + + "github.com/go-jose/go-jose/v4/jwt" ) // jwtClaims represents the expected claims contained in the JWT token. type jwtClaims struct { - AccessID string `json:"access_id"` - Actor map[string]any `json:"act,omitempty"` - AuthMethods []string `json:"amr,omitempty"` - AuthenticationTime int64 `json:"auth_time,omitempty"` - ClientID string `json:"cid,omitempty"` - ExpiresAt int64 `json:"exp,omitempty"` - GrantType string `json:"grant,omitempty"` - IssuedAt int64 `json:"iat,omitempty"` - Issuer string `json:"iss,omitempty"` - JWTID string `json:"jti,omitempty"` - NotBefore int64 `json:"nbf,omitempty"` - Namespace string `json:"ns,omitempty"` - Scopes []string `json:"scp,omitempty"` - Subject string `json:"sub,omitempty"` + ExpiresAt *jwt.NumericDate `json:"exp,omitempty"` } // unsafeParseJWT parses a JWT without verifying its signature and returns its claims. From c87c6500e5be0ea6fb46f3ed554567c12a154f44 Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 2 Sep 2025 15:40:16 +0300 Subject: [PATCH 05/13] fixup! feat: simplify parsing of the JWT token --- internal/auth/legacy.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/auth/legacy.go b/internal/auth/legacy.go index fe1e3340..d8c7e4ea 100644 --- a/internal/auth/legacy.go +++ b/internal/auth/legacy.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "sync" - "time" "golang.org/x/oauth2" @@ -36,7 +35,7 @@ func (ts *legacyCLITokenSource) unsafeGetLegacyCLIToken() (*oauth2.Token, error) return &oauth2.Token{ AccessToken: bt.String(), TokenType: "Bearer", - Expiry: time.Unix(at.ExpiresAt, 0), + Expiry: at.ExpiresAt.Time(), }, nil } From de1283d4387f8f88fea4385e62b55f12af078708 Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 2 Sep 2025 15:40:30 +0300 Subject: [PATCH 06/13] fix: handle error for token invalidation --- internal/auth/transport.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/auth/transport.go b/internal/auth/transport.go index 108498db..6fe034e0 100644 --- a/internal/auth/transport.go +++ b/internal/auth/transport.go @@ -3,6 +3,7 @@ package auth import ( "bytes" "context" + "fmt" "io" "log" "net/http" @@ -40,7 +41,9 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { // Retry on 401 if resp != nil && resp.StatusCode == http.StatusUnauthorized { _ = t.log("The access token has been refreshed. Retrying request.") - _ = t.refresher.invalidateToken() + if err := t.refresher.invalidateToken(); err != nil { + return nil, fmt.Errorf("failed to invalidate token: %w", err) + } flushReader(resp.Body) resp, err = t.base.RoundTrip(req) } From a82e1c2ade4d0c696f3ccb8e36b147f9414e1f29 Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 2 Sep 2025 16:41:49 +0300 Subject: [PATCH 07/13] fix: skip importing `go-jose` just for parsing a date --- go.mod | 1 - go.sum | 12 ------------ internal/auth/jwt.go | 31 ++++++++++++++++--------------- internal/auth/legacy.go | 4 ++-- 4 files changed, 18 insertions(+), 30 deletions(-) diff --git a/go.mod b/go.mod index 64f9b21c..fff52ec7 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/Masterminds/semver/v3 v3.3.1 github.com/fatih/color v1.18.0 github.com/go-chi/chi/v5 v5.2.1 - github.com/go-jose/go-jose/v4 v4.1.2 github.com/go-playground/validator/v10 v10.26.0 github.com/gofrs/flock v0.12.1 github.com/oklog/ulid/v2 v2.1.0 diff --git a/go.sum b/go.sum index 7ae7a952..357ab6a0 100644 --- a/go.sum +++ b/go.sum @@ -32,8 +32,6 @@ github.com/gabriel-vasile/mimetype v1.4.8 h1:FfZ3gj38NjllZIeJAmMhr+qKL8Wu+nOoI3G github.com/gabriel-vasile/mimetype v1.4.8/go.mod h1:ByKUIKGjh1ODkGM1asKUbQZOLGrPjydw3hYPU2YU9t8= github.com/go-chi/chi/v5 v5.2.1 h1:KOIHODQj58PmL80G2Eak4WdvUzjSJSm0vG72crDCqb8= github.com/go-chi/chi/v5 v5.2.1/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops= -github.com/go-jose/go-jose/v4 v4.1.2 h1:TK/7NqRQZfgAh+Td8AlsrvtPoUyiHh0LqVvokh+1vHI= -github.com/go-jose/go-jose/v4 v4.1.2/go.mod h1:22cg9HWM1pOlnRiY+9cQYJ9XHmya1bYW8OeDM6Ku6Oo= github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s= github.com/go-playground/assert/v2 v2.2.0/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= @@ -142,8 +140,6 @@ go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE= -golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc= golang.org/x/crypto v0.39.0 h1:SHs+kF4LP+f+p14esP5jAoDpHU8Gu/v9lFRK6IT5imM= golang.org/x/crypto v0.39.0/go.mod h1:L+Xg3Wf6HoL4Bn4238Z6ft6KfEpN0tJGo53AAPC632U= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= @@ -156,8 +152,6 @@ golang.org/x/oauth2 v0.30.0 h1:dnDm7JmhM45NNpd8FDDeLhK6FwqbOf4MLCM9zb1BOHI= golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKlU= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610= -golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8= golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -169,22 +163,16 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20= -golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.31.0 h1:erwDkOK1Msy6offm1mOgvspSkslFnIGsFnxOKoufg3o= -golang.org/x/term v0.31.0/go.mod h1:R4BeIy7D95HzImkxGkTW1UQTtP54tio2RyHz7PwK0aw= golang.org/x/term v0.32.0 h1:DR4lr0TjUs3epypdhTOkMmuF5CDFJ/8pOnbzMZPQ7bg= golang.org/x/term v0.32.0/go.mod h1:uZG1FhGx848Sqfsq4/DlJr3xGGsYMu/L5GW4abiaEPQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= -golang.org/x/text v0.24.0 h1:dd5Bzh4yt5KYA8f9CJHCP4FB4D51c2c6JvN37xJJkJ0= -golang.org/x/text v0.24.0/go.mod h1:L8rBsPeo2pSS+xqN0d5u2ikmjtmoJbDBT1b7nHvFCdU= golang.org/x/text v0.26.0 h1:P42AVeLghgTYr4+xUnTRKDMqpar+PtX7KWuNQL21L8M= golang.org/x/text v0.26.0/go.mod h1:QK15LZJUUQVJxhz7wXgxSy/CJaTFjd0G+YLonydOVQA= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= diff --git a/internal/auth/jwt.go b/internal/auth/jwt.go index 29d1b506..3760a9f2 100644 --- a/internal/auth/jwt.go +++ b/internal/auth/jwt.go @@ -6,36 +6,37 @@ import ( "errors" "fmt" "strings" - - "github.com/go-jose/go-jose/v4/jwt" + "time" ) -// jwtClaims represents the expected claims contained in the JWT token. -type jwtClaims struct { - ExpiresAt *jwt.NumericDate `json:"exp,omitempty"` -} - -// unsafeParseJWT parses a JWT without verifying its signature and returns its claims. +// unsafeGetJWTExpiry parses a JWT without verifying its signature and returns its expiry time. // WARNING: This is intentionally unsafe and must not be used for trust decisions. -func unsafeParseJWT(token string) (*jwtClaims, error) { +func unsafeGetJWTExpiry(token string) (time.Time, error) { if token == "" { - return nil, errors.New("jwt: empty token") + return time.Time{}, errors.New("jwt: empty token") } parts := strings.Split(token, ".") if len(parts) < 2 { - return nil, fmt.Errorf("jwt: malformed token, expected 3 parts, got %d", len(parts)) + return time.Time{}, fmt.Errorf("jwt: malformed token, expected 3 parts, got %d", len(parts)) } payloadSeg := parts[1] // Base64 URL decode without padding as per RFC 7515. payloadBytes, err := base64.RawURLEncoding.DecodeString(payloadSeg) if err != nil { - return nil, fmt.Errorf("jwt: decode payload: %w", err) + return time.Time{}, fmt.Errorf("jwt: decode payload: %w", err) } - var claims jwtClaims + var claims struct { + ExpiresAt *int64 `json:"exp,omitempty"` + } if err := json.Unmarshal(payloadBytes, &claims); err != nil { - return nil, fmt.Errorf("jwt: unmarshal claims: %w", err) + return time.Time{}, fmt.Errorf("jwt: unmarshal claims: %w", err) } - return &claims, nil + + if claims.ExpiresAt == nil { + return time.Time{}, errors.New("jwt: no expiry time found") + } + + return time.Unix(*claims.ExpiresAt, 0), nil } diff --git a/internal/auth/legacy.go b/internal/auth/legacy.go index d8c7e4ea..a2bf3e82 100644 --- a/internal/auth/legacy.go +++ b/internal/auth/legacy.go @@ -26,7 +26,7 @@ func (ts *legacyCLITokenSource) unsafeGetLegacyCLIToken() (*oauth2.Token, error) return nil, fmt.Errorf("cannot retrieve token: %w", err) } - at, err := unsafeParseJWT(bt.String()) + expiry, err := unsafeGetJWTExpiry(bt.String()) if err != nil { return nil, fmt.Errorf("cannot parse token: %w", err) @@ -35,7 +35,7 @@ func (ts *legacyCLITokenSource) unsafeGetLegacyCLIToken() (*oauth2.Token, error) return &oauth2.Token{ AccessToken: bt.String(), TokenType: "Bearer", - Expiry: at.ExpiresAt.Time(), + Expiry: expiry, }, nil } From bb5b0f20034d01c7670bfb7dc108d252ecbec2e8 Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 2 Sep 2025 17:44:33 +0300 Subject: [PATCH 08/13] feat(tests): add tests for the retry logic in the transport --- internal/auth/client.go | 3 +- internal/auth/transport.go | 5 +- internal/auth/transport_test.go | 118 ++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 internal/auth/transport_test.go diff --git a/internal/auth/client.go b/internal/auth/client.go index bc4538a9..67660428 100644 --- a/internal/auth/client.go +++ b/internal/auth/client.go @@ -33,8 +33,7 @@ func NewLegacyCLIClient(ctx context.Context, wrapper *legacy.CLIWrapper) (*http. Source: ts, Base: baseRT, }, - wrapper: wrapper, - logger: log.New(os.Stderr, "", 0), + logger: log.New(os.Stderr, "", 0), }, }, nil } diff --git a/internal/auth/transport.go b/internal/auth/transport.go index 6fe034e0..b9f84bc1 100644 --- a/internal/auth/transport.go +++ b/internal/auth/transport.go @@ -7,8 +7,6 @@ import ( "io" "log" "net/http" - - "github.com/platformsh/cli/internal/legacy" ) type refresher interface { @@ -27,8 +25,7 @@ type Transport struct { // so we can clear its cached token on 401. refresher refresher - wrapper *legacy.CLIWrapper - logger *log.Logger + logger *log.Logger } // RoundTrip adds Authorization via the underlying oauth2.Transport. If the diff --git a/internal/auth/transport_test.go b/internal/auth/transport_test.go new file mode 100644 index 00000000..859c03cf --- /dev/null +++ b/internal/auth/transport_test.go @@ -0,0 +1,118 @@ +package auth + +import ( + "bytes" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" +) + +// mockRefresher implements both the refresher and oauth2.TokenSource interfaces for testing +type mockRefresher struct { + token *oauth2.Token +} + +func (m *mockRefresher) refreshToken() error { + m.token = &oauth2.Token{ + AccessToken: "valid", + TokenType: "Bearer", + Expiry: time.Now().Add(time.Hour), + } + return nil +} + +func (m *mockRefresher) invalidateToken() error { + m.token = &oauth2.Token{ + AccessToken: "", + TokenType: "Bearer", + Expiry: time.Now().Add(-time.Hour), + } + + return nil +} + +func (m *mockRefresher) Token() (*oauth2.Token, error) { + if m.token == nil || !m.token.Valid() { + if err := m.refreshToken(); err != nil { + return nil, err + } + } + return m.token, nil +} + +func TestTransport_RoundTrip_RetryOn401(t *testing.T) { + // Create a mock server that initially returns 401, then 200 + responseCodes := []int{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Read and validate the request body + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + + // Check that we have the expected POST body + assert.Equal(t, "test-body-content", string(body)) + + if r.Header.Get("Authorization") != "Bearer valid" { + w.WriteHeader(http.StatusUnauthorized) + if _, err := w.Write([]byte(`{"error": "unauthorized"}`)); err != nil { + require.NoError(t, err) + } + responseCodes = append(responseCodes, http.StatusUnauthorized) + return + } + + w.WriteHeader(http.StatusOK) + if _, err := w.Write([]byte(`{"success": true}`)); err != nil { + require.NoError(t, err) + } + responseCodes = append(responseCodes, http.StatusOK) + })) + defer server.Close() + + // Create mock refresher with token sequence: first invalid, then valid + mockRef := &mockRefresher{ + token: &oauth2.Token{ + AccessToken: "invalid", + TokenType: "Bearer", + Expiry: time.Now().Add(time.Hour), + }, + } + + // Create our Transport with the mock refresher + transport := &Transport{ + base: &oauth2.Transport{ + Source: mockRef, + Base: http.DefaultTransport, + }, + refresher: mockRef, + } + + // Create HTTP client with our transport + client := &http.Client{Transport: transport} + + // Make a POST request with body content + requestBody := "test-body-content" + req, err := http.NewRequest("POST", server.URL, bytes.NewBufferString(requestBody)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + + // Execute the request + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + // Verify we got a successful response after retry + assert.Equal(t, http.StatusOK, resp.StatusCode) + + responseBody, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Equal(t, `{"success": true}`, string(responseBody)) + + // Assert the response codes (401 first and then a 200) + assert.Equal(t, []int{http.StatusUnauthorized, http.StatusOK}, responseCodes) +} From cbb3c499d1120b110c8a6d16a8948a564b40f078 Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 2 Sep 2025 17:55:39 +0300 Subject: [PATCH 09/13] fix(tests): make them work nicely on macos macos is using a symlink for temporary directories and the tests were failing. --- internal/config/alt/path_test.go | 2 ++ pkg/mockssh/server_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/internal/config/alt/path_test.go b/internal/config/alt/path_test.go index bde84617..b6400e9f 100644 --- a/internal/config/alt/path_test.go +++ b/internal/config/alt/path_test.go @@ -2,6 +2,7 @@ package alt_test import ( "os" + "path/filepath" "runtime" "testing" @@ -13,6 +14,7 @@ import ( func TestInPath(t *testing.T) { tempDir := t.TempDir() + tempDir, _ = filepath.EvalSymlinks(tempDir) homeDir := "/custom/home/directory" require.NoError(t, os.Setenv("HOME", homeDir)) diff --git a/pkg/mockssh/server_test.go b/pkg/mockssh/server_test.go index 2b6d068b..a7d330c1 100644 --- a/pkg/mockssh/server_test.go +++ b/pkg/mockssh/server_test.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/http" + "path/filepath" "strings" "testing" @@ -32,6 +33,7 @@ func TestServer(t *testing.T) { }) tempDir := t.TempDir() + tempDir, _ = filepath.EvalSymlinks(tempDir) sshServer.CommandHandler = mockssh.ExecHandler(tempDir, []string{}) cert := getTestSSHAuth(t, authServer.URL) From 7cae9878b04b2ea44ff5b46bfce56245295d445f Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Wed, 3 Sep 2025 16:02:33 +0300 Subject: [PATCH 10/13] chore: simplify logging mechanism in the transport --- internal/auth/client.go | 3 --- internal/auth/transport.go | 7 +++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/internal/auth/client.go b/internal/auth/client.go index 67660428..a485e197 100644 --- a/internal/auth/client.go +++ b/internal/auth/client.go @@ -3,9 +3,7 @@ package auth import ( "context" "fmt" - "log" "net/http" - "os" "golang.org/x/oauth2" @@ -33,7 +31,6 @@ func NewLegacyCLIClient(ctx context.Context, wrapper *legacy.CLIWrapper) (*http. Source: ts, Base: baseRT, }, - logger: log.New(os.Stderr, "", 0), }, }, nil } diff --git a/internal/auth/transport.go b/internal/auth/transport.go index b9f84bc1..501bd3bd 100644 --- a/internal/auth/transport.go +++ b/internal/auth/transport.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "io" - "log" "net/http" ) @@ -25,7 +24,7 @@ type Transport struct { // so we can clear its cached token on 401. refresher refresher - logger *log.Logger + LogFunc func(msg string, args ...any) } // RoundTrip adds Authorization via the underlying oauth2.Transport. If the @@ -49,10 +48,10 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { } func (t *Transport) log(msg string, args ...any) error { - if t.logger == nil { + if t.LogFunc == nil { return nil } - t.logger.Printf(msg, args...) + t.LogFunc(msg, args...) return nil } From 362dde7bddf2fb2851ffa7ae76c31b40d3576ac5 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 3 Sep 2025 16:28:45 +0100 Subject: [PATCH 11/13] Language tweak in log message --- internal/auth/transport.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/auth/transport.go b/internal/auth/transport.go index 501bd3bd..3c2a169e 100644 --- a/internal/auth/transport.go +++ b/internal/auth/transport.go @@ -36,7 +36,7 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { // Retry on 401 if resp != nil && resp.StatusCode == http.StatusUnauthorized { - _ = t.log("The access token has been refreshed. Retrying request.") + _ = t.log("The access token needs to be refreshed. Retrying request.") if err := t.refresher.invalidateToken(); err != nil { return nil, fmt.Errorf("failed to invalidate token: %w", err) } From 2f203f8d559ab08e7640ad3100fb1b0638ab8741 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 3 Sep 2025 16:26:46 +0100 Subject: [PATCH 12/13] Fix validate on OAuth2ClientID --- internal/config/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/schema.go b/internal/config/schema.go index ac91613b..002a1055 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -48,7 +48,7 @@ type Config struct { UserAgent string `validate:"omitempty" yaml:"user_agent,omitempty"` // a template - see UserAgent method SessionID string `validate:"omitempty,ascii" yaml:"session_id,omitempty"` // the ID for the authentication session - defaults to "default" - OAuth2ClientID string `validate:"required_without=AuthURL,omitempty" yaml:"oauth2_client_id,omitempty"` // e.g. "upsun-cli" + OAuth2ClientID string `validate:"omitempty" yaml:"oauth2_client_id,omitempty"` // e.g. "upsun-cli" OAuth2AuthorizeURL string `validate:"required_without=AuthURL,omitempty,url" yaml:"oauth2_auth_url,omitempty"` // e.g. "https://auth.api.platform.sh/oauth2/authorize" OAuth2RevokeURL string `validate:"required_without=AuthURL,omitempty,url" yaml:"oauth2_revoke_url,omitempty"` // e.g. "https://auth.api.platform.sh/oauth2/revoke" OAuth2TokenURL string `validate:"required_without=AuthURL,omitempty,url" yaml:"oauth2_token_url,omitempty"` // e.g. "https://auth.api.platform.sh/oauth2/token" From 63b2a914994dbe844e2e31e8eb3bb378fff09e35 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 4 Sep 2025 17:56:21 +0100 Subject: [PATCH 13/13] Update internal/auth/client.go --- internal/auth/client.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/auth/client.go b/internal/auth/client.go index a485e197..56be3950 100644 --- a/internal/auth/client.go +++ b/internal/auth/client.go @@ -10,6 +10,8 @@ import ( "github.com/platformsh/cli/internal/legacy" ) +// NewLegacyCLIClient creates an HTTP client authenticated through the legacy CLI. +// The wrapper argument must be a dedicated wrapper, not used by other callers. func NewLegacyCLIClient(ctx context.Context, wrapper *legacy.CLIWrapper) (*http.Client, error) { ts, err := NewLegacyCLITokenSource(ctx, wrapper) if err != nil {