Skip to content

feat: add enhanced support for Slack OAuth #10151

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

Closed
wants to merge 1 commit into from
Closed

feat: add enhanced support for Slack OAuth #10151

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: 6 additions & 0 deletions cli/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2259,6 +2259,12 @@ func parseExternalAuthProvidersFromEnv(prefix string, environ []string) ([]coder
provider.DisplayName = v.Value
case "DISPLAY_ICON":
provider.DisplayIcon = v.Value
case "SLACK_AUTHED_USER_TOKEN":
b, err := strconv.ParseBool(v.Value)
if err != nil {
return nil, xerrors.Errorf("parse bool: %s", v.Value)
}
provider.SlackAuthedUserToken = b
}
providers[providerNum] = provider
}
Expand Down
12 changes: 12 additions & 0 deletions coderd/coderdtest/oidctest/idp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ type FakeIDP struct {
// "Authorized Redirect URLs". This can be used to emulate that.
hookValidRedirectURL func(redirectURL string) error
hookUserInfo func(email string) (jwt.MapClaims, error)
hookExtra func(email string) map[string]interface{}
fakeCoderd func(req *http.Request) (*http.Response, error)
hookOnRefresh func(email string) error
// Custom authentication for the client. This is useful if you want
Expand Down Expand Up @@ -112,6 +113,12 @@ func WithRefresh(hook func(email string) error) func(*FakeIDP) {
}
}

func WithExtra(extra func(email string) map[string]interface{}) func(*FakeIDP) {
return func(f *FakeIDP) {
f.hookExtra = extra
}
}

func WithCustomClientAuth(hook func(t testing.TB, req *http.Request) (url.Values, error)) func(*FakeIDP) {
return func(f *FakeIDP) {
f.hookAuthenticateClient = hook
Expand Down Expand Up @@ -621,6 +628,11 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler {
"expires_in": int64((time.Minute * 5).Seconds()),
"id_token": f.encodeClaims(t, claims),
}
if f.hookExtra != nil {
for k, v := range f.hookExtra(email) {
token[k] = v
}
}
// Store the claims for the next refresh
f.refreshIDTokenClaims.Store(refreshToken, claims)

Expand Down
47 changes: 37 additions & 10 deletions coderd/externalauth/externalauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ type Config struct {
// AppInstallationsURL is an API endpoint that returns a list of
// installations for the user. This is used for GitHub Apps.
AppInstallationsURL string

// SlackAuthedUserToken is true if the user token should be returned
// instead of the bot token.
SlackAuthedUserToken bool
}

// RefreshToken automatically refreshes the token if expired and permitted.
Expand Down Expand Up @@ -101,6 +105,22 @@ func (c *Config) RefreshToken(ctx context.Context, db database.Store, externalAu
// we aren't trying to surface an error, we're just trying to obtain a valid token.
return externalAuthLink, false, nil
}

// Slack's new OAuth2 flow has the user access token in a different field.
// It's weird and unfortunate, but the only way to access the user token.
// See: https://api.slack.com/authentication/oauth-v2#exchanging
if c.Type == string(codersdk.EnhancedExternalAuthProviderSlack) && c.SlackAuthedUserToken {
rawMap, ok := token.Extra("authed_user").(map[string]interface{})
if !ok {
return externalAuthLink, false, xerrors.Errorf("slack: could not obtain user access token from payload: %+v", token.Extra("authed_user"))
}
accessToken, ok := rawMap["access_token"].(string)
if !ok {
return externalAuthLink, false, xerrors.Errorf("slack: could not obtain user access token from payload: %+v", token.Extra("authed_user"))
}
token.AccessToken = accessToken
}

r := retry.New(50*time.Millisecond, 200*time.Millisecond)
// See the comment below why the retry and cancel is required.
retryCtx, retryCtxCancel := context.WithTimeout(ctx, time.Second)
Expand Down Expand Up @@ -424,16 +444,17 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([
}

cfg := &Config{
OAuth2Config: oauthConfig,
ID: entry.ID,
Regex: regex,
Type: entry.Type,
NoRefresh: entry.NoRefresh,
ValidateURL: entry.ValidateURL,
AppInstallationsURL: entry.AppInstallationsURL,
AppInstallURL: entry.AppInstallURL,
DisplayName: entry.DisplayName,
DisplayIcon: entry.DisplayIcon,
OAuth2Config: oauthConfig,
ID: entry.ID,
Regex: regex,
Type: entry.Type,
NoRefresh: entry.NoRefresh,
ValidateURL: entry.ValidateURL,
AppInstallationsURL: entry.AppInstallationsURL,
AppInstallURL: entry.AppInstallURL,
DisplayName: entry.DisplayName,
DisplayIcon: entry.DisplayIcon,
SlackAuthedUserToken: entry.SlackAuthedUserToken,
}

if entry.DeviceFlow {
Expand Down Expand Up @@ -539,6 +560,12 @@ var defaults = map[codersdk.EnhancedExternalAuthProvider]codersdk.ExternalAuthCo
DeviceCodeURL: "https://github.com/login/device/code",
AppInstallationsURL: "https://api.github.com/user/installations",
},
codersdk.EnhancedExternalAuthProviderSlack: {
AuthURL: "https://slack.com/oauth/v2/authorize",
TokenURL: "https://slack.com/api/oauth.v2.access",
DisplayName: "Slack",
DisplayIcon: "/icon/slack.svg",
},
}

// jwtConfig is a new OAuth2 config that uses a custom
Expand Down
50 changes: 41 additions & 9 deletions coderd/externalauth/externalauth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestRefreshToken(t *testing.T) {
return nil, xerrors.New("should not be called")
}),
},
GitConfigOpt: func(cfg *externalauth.Config) {
ExternalAuthOpt: func(cfg *externalauth.Config) {
cfg.NoRefresh = true
},
})
Expand Down Expand Up @@ -74,7 +74,7 @@ func TestRefreshToken(t *testing.T) {
return jwt.MapClaims{}, nil
}),
},
GitConfigOpt: func(cfg *externalauth.Config) {
ExternalAuthOpt: func(cfg *externalauth.Config) {
cfg.NoRefresh = true
},
})
Expand Down Expand Up @@ -117,7 +117,7 @@ func TestRefreshToken(t *testing.T) {
return jwt.MapClaims{}, xerrors.New(staticError)
}),
},
GitConfigOpt: func(cfg *externalauth.Config) {
ExternalAuthOpt: func(cfg *externalauth.Config) {
},
})

Expand All @@ -142,7 +142,7 @@ func TestRefreshToken(t *testing.T) {
return jwt.MapClaims{}, oidctest.StatusError(http.StatusUnauthorized, xerrors.New(staticError))
}),
},
GitConfigOpt: func(cfg *externalauth.Config) {
ExternalAuthOpt: func(cfg *externalauth.Config) {
},
})

Expand Down Expand Up @@ -175,7 +175,7 @@ func TestRefreshToken(t *testing.T) {
return jwt.MapClaims{}, oidctest.StatusError(http.StatusUnauthorized, xerrors.New(staticError))
}),
},
GitConfigOpt: func(cfg *externalauth.Config) {
ExternalAuthOpt: func(cfg *externalauth.Config) {
cfg.Type = codersdk.EnhancedExternalAuthProviderGitHub.String()
},
})
Expand Down Expand Up @@ -205,7 +205,7 @@ func TestRefreshToken(t *testing.T) {
return jwt.MapClaims{}, nil
}),
},
GitConfigOpt: func(cfg *externalauth.Config) {
ExternalAuthOpt: func(cfg *externalauth.Config) {
cfg.Type = codersdk.EnhancedExternalAuthProviderGitHub.String()
},
})
Expand Down Expand Up @@ -236,7 +236,7 @@ func TestRefreshToken(t *testing.T) {
return jwt.MapClaims{}, nil
}),
},
GitConfigOpt: func(cfg *externalauth.Config) {
ExternalAuthOpt: func(cfg *externalauth.Config) {
cfg.Type = codersdk.EnhancedExternalAuthProviderGitHub.String()
},
DB: db,
Expand All @@ -260,6 +260,38 @@ func TestRefreshToken(t *testing.T) {
require.NoError(t, err)
require.Equal(t, updated.OAuthAccessToken, dbLink.OAuthAccessToken, "token is updated in the DB")
})

t.Run("SlackUserToken", func(t *testing.T) {
t.Parallel()

db := dbfake.New()
fake, config, link := setupOauth2Test(t, testConfig{
FakeIDPOpts: []oidctest.FakeIDPOpt{
oidctest.WithExtra(func(email string) map[string]interface{} {
return map[string]interface{}{
"authed_user": map[string]interface{}{
"access_token": "slack-user-token",
},
}
}),
Comment on lines +270 to +276
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add the existing fields to the params of WithExtra? Maybe just pass the token in and let the function mutate the existing map.

I ask because it would be ideal to pass the actual access token:

oidctest.WithExtra(func(email string, token map[string]interface{}) {
	token["authed_user"] = map[string]interface{}{
		"access_token": token["access_token"],
	}
	// Completely nuke the original access token?
	token["access_token"] = "bogus_unused_token"
}),

This would make it valid to the fake OIDC IDP.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was primarily trying to mimic the usage on the output side, which has the Extra property. So while we could merge them together, I think it's better to make it super consumable to the other end.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is the refresh works because the refresh token is set correctly.

But the access-token is invalidly set to slack-user-token. We only check access-token on /user-info, so maybe it does not matter. But I could see it being a benefit to set the extra value to the correctly generated access_token.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The access_token isn't being set. authed_user is which has a property of access_token.

},
ExternalAuthOpt: func(cfg *externalauth.Config) {
cfg.Type = codersdk.EnhancedExternalAuthProviderSlack.String()
cfg.SlackAuthedUserToken = true
cfg.ValidateURL = ""
},
DB: db,
})

ctx := oidc.ClientContext(context.Background(), fake.HTTPClient(nil))
// Force a refresh
link.OAuthExpiry = expired

updated, ok, err := config.RefreshToken(ctx, db, link)
require.NoError(t, err)
require.True(t, ok)
require.Equal(t, "slack-user-token", updated.OAuthAccessToken)
})
}

func TestConvertYAML(t *testing.T) {
Expand Down Expand Up @@ -344,7 +376,7 @@ func TestConvertYAML(t *testing.T) {
type testConfig struct {
FakeIDPOpts []oidctest.FakeIDPOpt
CoderOIDCConfigOpts []func(cfg *coderd.OIDCConfig)
GitConfigOpt func(cfg *externalauth.Config)
ExternalAuthOpt func(cfg *externalauth.Config)
// If DB is passed in, the link will be inserted into the DB.
DB database.Store
}
Expand All @@ -367,7 +399,7 @@ func setupOauth2Test(t *testing.T, settings testConfig) (*oidctest.FakeIDP, *ext
ID: providerID,
ValidateURL: fake.WellknownConfig().UserInfoURL,
}
settings.GitConfigOpt(config)
settings.ExternalAuthOpt(config)

oauthToken, err := fake.GenerateAuthenticatedToken(jwt.MapClaims{
"email": "test@coder.com",
Expand Down
6 changes: 6 additions & 0 deletions codersdk/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,12 @@ type ExternalAuthConfig struct {
DisplayName string `json:"display_name"`
// DisplayIcon is a URL to an icon to display in the UI.
DisplayIcon string `json:"display_icon"`

// SlackAuthedUserToken is a Slack-specific field that controls
// whether the Bot or User token is returned from the OAuth exchange.
// Slack returns multiple OAuth tokens as part of it's flow.
// See: https://api.slack.com/authentication/oauth-v2#exchanging
SlackAuthedUserToken bool `json:"slack_authed_user_token"`
}

type ProvisionerConfig struct {
Expand Down
1 change: 1 addition & 0 deletions codersdk/externalauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (
EnhancedExternalAuthProviderGitHub EnhancedExternalAuthProvider = "github"
EnhancedExternalAuthProviderGitLab EnhancedExternalAuthProvider = "gitlab"
EnhancedExternalAuthProviderBitBucket EnhancedExternalAuthProvider = "bitbucket"
EnhancedExternalAuthProviderSlack EnhancedExternalAuthProvider = "slack"
)

type ExternalAuth struct {
Expand Down
6 changes: 6 additions & 0 deletions site/static/icon/slack.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.