From 72b8701eefcf7d2d85cba0cfcc905c52d0cc66c9 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 5 Aug 2025 02:12:20 +0000 Subject: [PATCH 1/2] chore: make workspace sdk dialer fail fast for authnz errors --- codersdk/workspacesdk/dialer.go | 36 +++++++++++++++++++------ codersdk/workspacesdk/dialer_test.go | 40 ++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/codersdk/workspacesdk/dialer.go b/codersdk/workspacesdk/dialer.go index 71cac0c5f04b1..3ed03a31a31c3 100644 --- a/codersdk/workspacesdk/dialer.go +++ b/codersdk/workspacesdk/dialer.go @@ -24,6 +24,8 @@ var permanentErrorStatuses = []int{ http.StatusBadRequest, // returned if API mismatch http.StatusNotFound, // returned if user doesn't have permission or agent doesn't exist http.StatusInternalServerError, // returned if database is not reachable, + http.StatusUnauthorized, // returned if user is not authenticated + http.StatusForbidden, // returned if user is not authorized } type WebsocketDialer struct { @@ -39,6 +41,24 @@ type WebsocketDialer struct { isFirst bool } +// checkResumeTokenFailure checks if the parsed error indicates a resume token failure +// and updates the resumeTokenFailed flag accordingly. Returns true if a resume token +// failure was detected. +func (w *WebsocketDialer) checkResumeTokenFailure(ctx context.Context, sdkErr *codersdk.Error) bool { + if sdkErr == nil { + return false + } + + for _, v := range sdkErr.Validations { + if v.Field == "resume_token" { + w.logger.Warn(ctx, "failed to dial tailnet v2+ API: server replied invalid resume token; unsetting for next connection attempt") + w.resumeTokenFailed = true + return true + } + } + return false +} + type WebsocketDialerOption func(*WebsocketDialer) func WithWorkspaceUpdates(req *proto.WorkspaceUpdatesRequest) WebsocketDialerOption { @@ -82,9 +102,14 @@ func (w *WebsocketDialer) Dial(ctx context.Context, r tailnet.ResumeTokenControl if w.isFirst { if res != nil && slices.Contains(permanentErrorStatuses, res.StatusCode) { err = codersdk.ReadBodyAsError(res) - // A bit more human-readable help in the case the API version was rejected var sdkErr *codersdk.Error if xerrors.As(err, &sdkErr) { + // Check for resume token failure first + if w.checkResumeTokenFailure(ctx, sdkErr) { + return tailnet.ControlProtocolClients{}, err + } + + // A bit more human-readable help in the case the API version was rejected if sdkErr.Message == AgentAPIMismatchMessage && sdkErr.StatusCode() == http.StatusBadRequest { sdkErr.Helper = fmt.Sprintf( @@ -107,13 +132,8 @@ func (w *WebsocketDialer) Dial(ctx context.Context, r tailnet.ResumeTokenControl bodyErr := codersdk.ReadBodyAsError(res) var sdkErr *codersdk.Error if xerrors.As(bodyErr, &sdkErr) { - for _, v := range sdkErr.Validations { - if v.Field == "resume_token" { - // Unset the resume token for the next attempt - w.logger.Warn(ctx, "failed to dial tailnet v2+ API: server replied invalid resume token; unsetting for next connection attempt") - w.resumeTokenFailed = true - return tailnet.ControlProtocolClients{}, err - } + if w.checkResumeTokenFailure(ctx, sdkErr) { + return tailnet.ControlProtocolClients{}, err } } if !errors.Is(err, context.Canceled) { diff --git a/codersdk/workspacesdk/dialer_test.go b/codersdk/workspacesdk/dialer_test.go index dbe351e4e492c..227299d43afda 100644 --- a/codersdk/workspacesdk/dialer_test.go +++ b/codersdk/workspacesdk/dialer_test.go @@ -270,6 +270,46 @@ func TestWebsocketDialer_ResumeTokenFailure(t *testing.T) { require.Error(t, err) } +func TestWebsocketDialer_UnauthenticatedFailFast(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + logger := slogtest.Make(t, &slogtest.Options{ + IgnoreErrors: true, + }).Leveled(slog.LevelDebug) + + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(ctx, w, http.StatusUnauthorized, codersdk.Response{}) + })) + defer svr.Close() + svrURL, err := url.Parse(svr.URL) + require.NoError(t, err) + + uut := workspacesdk.NewWebsocketDialer(logger, svrURL, &websocket.DialOptions{}) + + _, err = uut.Dial(ctx, nil) + require.Error(t, err) +} + +func TestWebsocketDialer_UnauthorizedFailFast(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + logger := slogtest.Make(t, &slogtest.Options{ + IgnoreErrors: true, + }).Leveled(slog.LevelDebug) + + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(ctx, w, http.StatusUnauthorized, codersdk.Response{}) + })) + defer svr.Close() + svrURL, err := url.Parse(svr.URL) + require.NoError(t, err) + + uut := workspacesdk.NewWebsocketDialer(logger, svrURL, &websocket.DialOptions{}) + + _, err = uut.Dial(ctx, nil) + require.Error(t, err) +} + func TestWebsocketDialer_UplevelVersion(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitShort) From b5a1cd6a670d5d59c45f36b2d40a8c8472ea9b9f Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 5 Aug 2025 05:03:39 +0000 Subject: [PATCH 2/2] review --- codersdk/workspacesdk/dialer.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codersdk/workspacesdk/dialer.go b/codersdk/workspacesdk/dialer.go index 3ed03a31a31c3..39d02931e6ae1 100644 --- a/codersdk/workspacesdk/dialer.go +++ b/codersdk/workspacesdk/dialer.go @@ -24,8 +24,10 @@ var permanentErrorStatuses = []int{ http.StatusBadRequest, // returned if API mismatch http.StatusNotFound, // returned if user doesn't have permission or agent doesn't exist http.StatusInternalServerError, // returned if database is not reachable, - http.StatusUnauthorized, // returned if user is not authenticated http.StatusForbidden, // returned if user is not authorized + // StatusUnauthorized is only a permanent error if the error is not due to + // an invalid resume token. See `checkResumeTokenFailure`. + http.StatusUnauthorized, } type WebsocketDialer struct {