-
Notifications
You must be signed in to change notification settings - Fork 207
Add --remote-auth-scope-param-name for non-standard OAuth scope parameters #4712
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -509,6 +509,7 @@ type OAuthFlowConfig struct { | |
| SkipBrowser bool | ||
| Resource string // RFC 8707 resource indicator (optional) | ||
| OAuthParams map[string]string | ||
| ScopeParamName string // Override scope query parameter name (e.g., "user_scope" for Slack) | ||
| } | ||
|
|
||
| // OAuthFlowResult contains the result of an OAuth flow | ||
|
|
@@ -645,6 +646,7 @@ func createOAuthConfig(ctx context.Context, issuer string, config *OAuthFlowConf | |
| config.CallbackPort, | ||
| config.Resource, | ||
| config.OAuthParams, | ||
| config.ScopeParamName, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ScopeParamName silently ignored on OIDC discovery fallback path
Could set it on the returned config after the OIDC call: cfg, err := oauth.CreateOAuthConfigFromOIDC(ctx, issuer, ...)
if err != nil { return nil, err }
cfg.ScopeParamName = config.ScopeParamName
return cfg, nil |
||
| ) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import ( | |
| "net/http" | ||
| "os" | ||
| "os/signal" | ||
| "strings" | ||
| "syscall" | ||
| "time" | ||
|
|
||
|
|
@@ -60,6 +61,13 @@ type Config struct { | |
|
|
||
| // OAuthParams are additional parameters to pass to the authorization URL | ||
| OAuthParams map[string]string | ||
|
|
||
| // ScopeParamName overrides the query parameter name used to send scopes in the | ||
| // authorization URL. When empty (default), the standard "scope" parameter is used. | ||
| // Some providers use non-standard parameter names (e.g., Slack uses "user_scope" | ||
| // for user-token scopes). When set, scopes are sent under this parameter name | ||
| // instead of "scope", and the standard "scope" parameter is cleared. | ||
| ScopeParamName string | ||
| } | ||
|
|
||
| // Flow handles the OAuth authentication flow | ||
|
|
@@ -267,6 +275,18 @@ func (f *Flow) buildAuthURL() string { | |
| } | ||
| } | ||
|
|
||
| // When a custom scope parameter name is configured, move scopes from the | ||
| // standard "scope" parameter to the custom one. This supports OAuth providers | ||
| // that use non-standard parameter names (e.g., Slack's "user_scope"). | ||
| // The standard "scope" is cleared by setting it to empty; oauth2Config.Scopes | ||
| // is preserved so token refresh requests still include scopes correctly. | ||
| if f.config.ScopeParamName != "" && len(f.oauth2Config.Scopes) > 0 { | ||
| opts = append(opts, | ||
| oauth2.SetAuthURLParam("scope", ""), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The test at One approach: temporarily nil out if f.config.ScopeParamName != "" && len(f.oauth2Config.Scopes) > 0 {
scopeValue := strings.Join(f.oauth2Config.Scopes, " ")
savedScopes := f.oauth2Config.Scopes
f.oauth2Config.Scopes = nil
defer func() { f.oauth2Config.Scopes = savedScopes }()
opts = append(opts,
oauth2.SetAuthURLParam(f.config.ScopeParamName, scopeValue),
)
}And update the test assertion to verify truly absent: _, has := query["scope"]
assert.False(t, has, "scope parameter should be absent, not empty") |
||
| oauth2.SetAuthURLParam(f.config.ScopeParamName, strings.Join(f.oauth2Config.Scopes, " ")), | ||
| ) | ||
| } | ||
|
|
||
| // Add PKCE parameters if enabled | ||
| if f.config.UsePKCE { | ||
| opts = append(opts, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: duplicate OAuthFlowConfig construction
The two
OAuthFlowConfigstruct literals (here and at line 393) are identical — the only difference is the first argument toPerformOAuthFlow. Pre-existing, but this PR extends it by one more field. Consider extracting a helper to avoid future drift.