Skip to content

fix: honor explicit apihost throughout deploy #661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mvanhorn wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: honor explicit apihost throughout deploy #661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion internal/auth/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 36 additions & 0 deletions internal/auth/auth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}{
Expand All @@ -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)
Expand Down
22 changes: 15 additions & 7 deletions internal/pkg/platform/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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{}
Expand Down
64 changes: 64 additions & 0 deletions internal/pkg/platform/deploy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down