From 017945ca7b895b48f0c13bfa94f6da479d2357cd Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 28 Aug 2026 20:32:39 -0700 Subject: [PATCH] fix: honor explicit apihost throughout deploy Fixes #129 --- internal/auth/auth.go | 6 ++- internal/auth/auth_test.go | 36 ++++++++++++++++ internal/pkg/platform/deploy.go | 22 +++++++--- internal/pkg/platform/deploy_test.go | 64 ++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 8 deletions(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 624b6247..847a399c 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -238,7 +238,11 @@ func (c *Client) auths(ctx context.Context) (map[string]types.SlackAuth, error) var updatedAuthsByTeamID types.AuthByTeamID var hasUpdateRotation, hasUpdateNaming bool - updatedAuthsByName, hasUpdateRotation = c.rotateTokenAll(ctx, auths) + if c.config.APIHostFlag != "" { + updatedAuthsByName = auths + } else { + updatedAuthsByName, hasUpdateRotation = c.rotateTokenAll(ctx, auths) + } // As of v2.4.0 we migrate users credentials json to storing auths by team_id updatedAuthsByTeamID, hasUpdateNaming = c.migrateToAuthByTeamID(ctx, updatedAuthsByName) diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index e0a4c70c..2c3cab7a 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -374,6 +374,31 @@ func Test_AuthsRotation(t *testing.T) { require.NoError(t, err, "Should not return an error when the contents of credentials are valid") }) + t.Run("token rotation is skipped with an explicit API host", func(t *testing.T) { + ctx, authClient := setup(t) + fiveMinutesAgo := int(time.Now().Unix()) - 60*5 + savedAPIHost := "https://saved.slack.test" + authClient.config.APIHostFlag = "api.your.test.endpoint" + + workspaceAuth := types.SlackAuth{ + APIHost: &savedAPIHost, + Token: "expiredToken", + RefreshToken: "valid-refresh-token", + ExpiresAt: fiveMinutesAgo, + TeamDomain: "workspace-a", + TeamID: "T123456789A", + } + _, err := authClient.setAuths(ctx, types.AuthByTeamDomain{ + workspaceAuth.TeamID: workspaceAuth, + }) + require.NoError(t, err) + + updatedAuths, err := authClient.auths(ctx) + + require.NoError(t, err) + require.Equal(t, workspaceAuth, updatedAuths[workspaceAuth.TeamID]) + }) + t.Run("token rotation returns an error", func(t *testing.T) { // Setup ctx, authClient := setup(t) @@ -566,6 +591,7 @@ func Test_SetSelectedAuth(t *testing.T) { mockAPIHost := "dev.slack.com" tests := map[string]struct { auth types.SlackAuth + apiHostFlag string expectedAPIHost string expectedAPIURL string }{ @@ -577,10 +603,20 @@ func Test_SetSelectedAuth(t *testing.T) { expectedAPIHost: fmt.Sprintf("https://%s", mockAPIHost), expectedAPIURL: fmt.Sprintf("https://%s/api/", mockAPIHost), }, + "explicit API host overrides authentication host": { + auth: types.SlackAuth{ + TeamID: "T002", + APIHost: &mockAPIHost, + }, + apiHostFlag: "api.your.test.endpoint", + expectedAPIHost: "https://api.your.test.endpoint", + expectedAPIURL: "https://api.your.test.endpoint/api/", + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { ctx, authClient, osMock := setup(t) + authClient.config.APIHostFlag = tc.apiHostFlag authClient.SetSelectedAuth(ctx, tc.auth, authClient.config, osMock) assert.Equal(t, authClient.config.TeamFlag, tc.auth.TeamID) assert.Equal(t, authClient.config.APIHostResolved, tc.expectedAPIHost) diff --git a/internal/pkg/platform/deploy.go b/internal/pkg/platform/deploy.go index 3dca5556..6cca052c 100644 --- a/internal/pkg/platform/deploy.go +++ b/internal/pkg/platform/deploy.go @@ -185,13 +185,8 @@ func deployApp(ctx context.Context, clients *shared.ClientFactory, app types.App var elapsedDeploy = time.Since(startDeploy) var deployTime = fmt.Sprintf("%.1fs", elapsedDeploy.Seconds()) - // Set the SLACK_API_URL environment variable for development workspaces - // - // Note: This errors silently to continue deployment without any problem - var apiHost = clients.Config.APIHostResolved - if clients.Auth().IsAPIHostSlackDev(apiHost) { - apiHostURL := fmt.Sprintf("%s/api/", apiHost) - _ = clients.API().AddVariable(ctx, token, app.AppID, "SLACK_API_URL", apiHostURL) + if err := setAPIHostVariable(ctx, clients, app.AppID); err != nil { + return err } successfulDeployText := deploySuccessText(clients, app, manifest, authSession, deployTime) @@ -200,6 +195,19 @@ func deployApp(ctx context.Context, clients *shared.ClientFactory, app types.App return nil } +// setAPIHostVariable configures deployed apps to use the resolved custom API +// host when the invocation explicitly requests one or targets Slack development. +func setAPIHostVariable(ctx context.Context, clients *shared.ClientFactory, appID string) error { + apiHost := clients.Config.APIHostResolved + if clients.Config.APIHostFlag == "" && !clients.Auth().IsAPIHostSlackDev(apiHost) { + return nil + } + + token := config.GetContextToken(ctx) + apiHostURL := fmt.Sprintf("%s/api/", apiHost) + return clients.API().AddVariable(ctx, token, appID, "SLACK_API_URL", apiHostURL) +} + // deploySuccessText formats the success message and app information for a deployed app func deploySuccessText(clients *shared.ClientFactory, app types.App, manifest types.SlackYaml, authSession api.AuthSession, deployTime string) string { parsedAppInfo := map[string]string{} diff --git a/internal/pkg/platform/deploy_test.go b/internal/pkg/platform/deploy_test.go index 1af95194..a64ac8bc 100644 --- a/internal/pkg/platform/deploy_test.go +++ b/internal/pkg/platform/deploy_test.go @@ -15,14 +15,78 @@ package platform import ( + "context" + "errors" "testing" "github.com/slackapi/slack-cli/internal/api" + "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestSetAPIHostVariable(t *testing.T) { + tests := map[string]struct { + apiHostFlag string + apiHostResolved string + isSlackDev bool + addVariableErr error + expectUpdate bool + }{ + "explicit custom host is added": { + apiHostFlag: "api.your.test.endpoint", + apiHostResolved: "https://api.your.test.endpoint", + expectUpdate: true, + }, + "explicit custom host update returns an error": { + apiHostFlag: "api.your.test.endpoint", + apiHostResolved: "https://api.your.test.endpoint", + addVariableErr: errors.New("variable update failed"), + expectUpdate: true, + }, + "development host without an explicit flag is added": { + apiHostResolved: "https://dev.slack.com", + isSlackDev: true, + expectUpdate: true, + }, + "production host without an explicit flag is not added": { + apiHostResolved: "https://slack.com", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := config.SetContextToken(context.Background(), "token") + clientsMock := shared.NewClientsMock() + clientsMock.Config.APIHostFlag = tc.apiHostFlag + clientsMock.Config.APIHostResolved = tc.apiHostResolved + if tc.apiHostFlag == "" { + clientsMock.Auth.On("IsAPIHostSlackDev", tc.apiHostResolved).Return(tc.isSlackDev).Once() + } + if tc.expectUpdate { + clientsMock.API.On( + "AddVariable", + ctx, + "token", + "A123", + "SLACK_API_URL", + tc.apiHostResolved+"/api/", + ).Return(tc.addVariableErr).Once() + } + clients := shared.NewClientFactory(clientsMock.MockClientFactory()) + + err := setAPIHostVariable(ctx, clients, "A123") + + require.ErrorIs(t, err, tc.addVariableErr) + clientsMock.API.AssertNumberOfCalls(t, "AddVariable", map[bool]int{true: 1, false: 0}[tc.expectUpdate]) + clientsMock.Auth.AssertExpectations(t) + clientsMock.API.AssertExpectations(t) + }) + } +} + func TestDeploySuccessText(t *testing.T) { tests := map[string]struct { app types.App