Address review feedback: fix empty scope= and OIDC fallback path

- Replace SetAuthURLParam("scope", "") with temporarily nil-ing
  oauth2Config.Scopes before AuthCodeURL, then restoring via defer.
  This omits the scope parameter entirely instead of producing an
  invalid empty scope= (RFC 6749 §3.3).
- Propagate ScopeParamName on the OIDC discovery fallback path in
  createOAuthConfig, so --remote-auth-scope-param-name works with
  --remote-auth-issuer as well.
- Strengthen test assertion to verify scope parameter is truly absent,
  not just empty-valued.

Signed-off-by: Gustavo Gomez <gmogmz@indeed.com>

Author: gmogmzGithub

pkg/auth/discovery/discovery.go

@@ -652,7 +652,7 @@ func createOAuthConfig(ctx context.Context, issuer string, config *OAuthFlowConf
 
 	// Fall back to OIDC discovery
 	slog.Debug("Using OIDC discovery")
-	return oauth.CreateOAuthConfigFromOIDC(
+	cfg, err := oauth.CreateOAuthConfigFromOIDC(
 		ctx,
 		issuer,
 		config.ClientID,
@@ -662,6 +662,11 @@ func createOAuthConfig(ctx context.Context, issuer string, config *OAuthFlowConf
 		config.CallbackPort,
 		config.Resource,
 	)
+	if err != nil {
+		return nil, err
+	}
+	cfg.ScopeParamName = config.ScopeParamName
+	return cfg, nil
 }
 
 func newOAuthFlow(ctx context.Context, oauthConfig *oauth.Config, config *OAuthFlowConfig) (*OAuthFlowResult, error) {

pkg/auth/oauth/flow.go

@@ -278,12 +278,16 @@ 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.
+	// We temporarily nil out oauth2Config.Scopes so the library omits the standard
+	// "scope" parameter entirely (an empty scope= would violate RFC 6749 §3.3).
+	// Scopes are restored via defer so token refresh requests still work correctly.
 	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("scope", ""),
-			oauth2.SetAuthURLParam(f.config.ScopeParamName, strings.Join(f.oauth2Config.Scopes, " ")),
+			oauth2.SetAuthURLParam(f.config.ScopeParamName, scopeValue),
 		)
 	}
 

pkg/auth/oauth/flow_test.go

@@ -270,8 +270,9 @@ func TestBuildAuthURL(t *testing.T) {
 				require.NoError(t, err)
 
 				query := parsedURL.Query()
-				// Standard "scope" should be cleared
-				assert.Empty(t, query.Get("scope"))
+				// Standard "scope" parameter should be absent, not empty
+				_, hasScope := query["scope"]
+				assert.False(t, hasScope, "scope parameter should be absent, not empty")
 				// Scopes should appear under the custom parameter name
 				assert.Contains(t, query.Get("user_scope"), "search:read")
 				assert.Contains(t, query.Get("user_scope"), "chat:write")