Files
coder/coderd/mcp_internal_test.go
Kyle Carberry d889ba1842 feat: add user_oidc auth type for MCP servers (#24793)
Adds a 5th MCP server authentication mode, `user_oidc` ("User OIDC
Identity"), that forwards the calling user's OIDC access token from
`user_links.oauth_access_token` to the upstream MCP server as
`Authorization: Bearer <token>`.

The token is read from `user_links` and refreshed transparently via
`oauth2.TokenSource` before each MCP request. No new per-MCP-server
secret storage and no per-user connect/disconnect step.

**Limitation**: only users who logged in via OIDC have a forwardable
token. Users authenticated via password or GitHub will see requests sent
without an `Authorization` header, and the upstream MCP server is
expected to respond with 401. A pluggable token source (e.g. CLI-minted
E2E tokens) is left as future work.

<details>
<summary>Implementation notes</summary>

- Schema: new
`coderd/database/migrations/000481_mcp_user_oidc_auth.{up,down}.sql`
relaxes the `mcp_server_configs.auth_type` CHECK constraint to include
`user_oidc`. Down migration deletes affected rows before restoring the
old constraint.
- SDK validation: `codersdk/mcp.go` extends `oneof` for
`CreateMCPServerConfigRequest` and `UpdateMCPServerConfigRequest`.
- Handler: `coderd/mcp.go` adds `case "user_oidc":` to the
field-clearing switch on update. The existing list and detail handlers
already report `auth_connected = true` for any non-`oauth2` auth type.
- Header construction: `coderd/x/chatd/mcpclient/mcpclient.go`
introduces a `UserOIDCTokenSource` interface and adds the `user_oidc`
case to `buildAuthHeaders`. `ConnectAll` / `connectOne` /
`buildAuthHeaders` gain `userID uuid.UUID, oidcSrc UserOIDCTokenSource`
parameters.
- Wiring: `coderd/x/chatd/chatd.go` adds `OIDCTokenSource` to `Config` /
`Server` and passes `chat.OwnerID` plus the source through `ConnectAll`.
`coderd/coderd.go` constructs the source next to the `chatd.New` call
when `options.OIDCConfig` is non-nil.
- Token source: `oidcMCPTokenSource` lives in `coderd/mcp.go`. It reads
the user's OIDC link, refreshes via `oauth2.TokenSource`, and writes the
refreshed token back to `user_links`. Logic is duplicated from
`provisionerdserver.ObtainOIDCAccessToken` to avoid an MCP ->
provisionerdserver dependency. The two copies must be kept in sync; a
comment on `oidcMCPTokenSource` records this.
- Frontend: `MCPServerAdminPanel.tsx` adds the new dropdown option, an
explanatory helper block (no admin-configurable fields), and a Storybook
story (`CreateServerUserOIDC`).
- Tests:
- `mcpclient_test.go`: `TestConnectAll_UserOIDCAuth`,
`TestConnectAll_UserOIDCAuth_NoLink`,
`TestConnectAll_UserOIDCAuth_NilSource`. All existing tests updated for
the new signature.
- `mcp_test.go`: extends `TestMCPServerConfigsAuthConnected` to assert
`auth_connected=true` for `user_oidc`; adds
`TestMCPServerConfigsUserOIDCClearsFields` and
`TestMCPServerConfigsUserOIDCDirect`.
- Docs: `docs/ai-coder/agents/platform-controls/mcp-servers.md`
describes the new mode and its OIDC-only limitation.

</details>

This PR was created by Coder Agents.

---------

Co-authored-by: Coder Agents <agents@coder.com>
2026-05-03 11:31:48 -04:00

217 lines
6.4 KiB
Go

package coderd
import (
"context"
"sync/atomic"
"testing"
"time"
"github.com/google/uuid"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/require"
"golang.org/x/oauth2"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbgen"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/testutil"
)
// dbauthzTestStore wraps the test database with the same dbauthz layer
// used in production (coderd.go:370). Without it the test would not
// catch RBAC failures from the chatd subject; with it the test fails
// loudly if the elevation in OIDCAccessToken is removed or weakened.
func dbauthzTestStore(t *testing.T, db database.Store) database.Store {
t.Helper()
authz := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
acs := &atomic.Pointer[dbauthz.AccessControlStore]{}
var tacs dbauthz.AccessControlStore = fakeAccessControlStore{}
acs.Store(&tacs)
return dbauthz.New(db, authz, testutil.Logger(t), acs)
}
// fakeAccessControlStore mirrors coderdtest.FakeAccessControlStore but is
// inlined here to avoid an import cycle (coderdtest imports coderd).
type fakeAccessControlStore struct{}
func (fakeAccessControlStore) GetTemplateAccessControl(t database.Template) dbauthz.TemplateAccessControl {
return dbauthz.TemplateAccessControl{
RequireActiveVersion: t.RequireActiveVersion,
}
}
func (fakeAccessControlStore) SetTemplateAccessControl(context.Context, database.Store, uuid.UUID, dbauthz.TemplateAccessControl) error {
panic("not implemented")
}
func TestShouldRefreshOIDCToken(t *testing.T) {
t.Parallel()
now := dbtime.Now()
cases := []struct {
name string
link database.UserLink
want bool
}{
{
name: "NoRefreshToken",
link: database.UserLink{OAuthExpiry: now.Add(-time.Hour)},
},
{
name: "ZeroExpiry",
link: database.UserLink{OAuthRefreshToken: "refresh"},
},
{
name: "Expired",
link: database.UserLink{
OAuthRefreshToken: "refresh",
OAuthExpiry: now.Add(-time.Hour),
},
want: true,
},
{
name: "Fresh",
link: database.UserLink{
OAuthRefreshToken: "refresh",
OAuthExpiry: now.Add(time.Hour),
},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
got, _ := shouldRefreshOIDCToken(tc.link)
require.Equal(t, tc.want, got)
})
}
}
func TestOIDCMCPTokenSource(t *testing.T) {
t.Parallel()
logger := testutil.Logger(t)
t.Run("NilConfig", func(t *testing.T) {
t.Parallel()
db, _ := dbtestutil.NewDB(t)
require.Nil(t, newOIDCMCPTokenSource(db, nil, logger))
})
t.Run("NoLink", func(t *testing.T) {
// When the user has no OIDC link the source returns ("", nil)
// rather than an error so the caller can fall through to
// "no Authorization header".
t.Parallel()
db, _ := dbtestutil.NewDB(t)
store := dbauthzTestStore(t, db)
user := dbgen.User(t, db, database.User{LoginType: database.LoginTypeOIDC})
src := newOIDCMCPTokenSource(store, &testutil.OAuth2Config{}, logger)
ctx := dbauthz.AsChatd(context.Background())
tok, err := src.OIDCAccessToken(ctx, user.ID)
require.NoError(t, err)
require.Empty(t, tok)
})
t.Run("FreshToken", func(t *testing.T) {
// A non-expired token is returned as-is; no refresh is performed.
t.Parallel()
db, _ := dbtestutil.NewDB(t)
store := dbauthzTestStore(t, db)
user := dbgen.User(t, db, database.User{})
dbgen.UserLink(t, db, database.UserLink{
UserID: user.ID,
LoginType: database.LoginTypeOIDC,
OAuthAccessToken: "fresh",
OAuthRefreshToken: "refresh",
OAuthExpiry: dbtime.Now().Add(time.Hour),
})
src := newOIDCMCPTokenSource(store, &testutil.OAuth2Config{
Token: &oauth2.Token{AccessToken: "should-not-be-used"},
}, logger)
ctx := dbauthz.AsChatd(context.Background())
tok, err := src.OIDCAccessToken(ctx, user.ID)
require.NoError(t, err)
require.Equal(t, "fresh", tok)
})
t.Run("RefreshExpired", func(t *testing.T) {
// An expired token triggers a refresh; the new token is
// persisted via UpdateUserLink. This exercises the dbauthz
// elevation: chatd lacks ResourceSystem.Read and
// ResourceUser.UpdatePersonal so a non-elevated context
// would fail both reads and writes.
t.Parallel()
db, _ := dbtestutil.NewDB(t)
store := dbauthzTestStore(t, db)
user := dbgen.User(t, db, database.User{})
dbgen.UserLink(t, db, database.UserLink{
UserID: user.ID,
LoginType: database.LoginTypeOIDC,
OAuthAccessToken: "stale",
OAuthRefreshToken: "refresh",
OAuthExpiry: dbtime.Now().Add(-time.Hour),
})
src := newOIDCMCPTokenSource(store, &testutil.OAuth2Config{
Token: &oauth2.Token{
AccessToken: "fresh",
RefreshToken: "new-refresh",
Expiry: dbtime.Now().Add(time.Hour),
},
}, logger)
ctx := dbauthz.AsChatd(context.Background())
tok, err := src.OIDCAccessToken(ctx, user.ID)
require.NoError(t, err)
require.Equal(t, "fresh", tok)
// Verify the refresh was persisted via UpdateUserLink.
got, err := db.GetUserLinkByUserIDLoginType(
dbauthz.AsSystemRestricted(context.Background()),
database.GetUserLinkByUserIDLoginTypeParams{
UserID: user.ID,
LoginType: database.LoginTypeOIDC,
},
)
require.NoError(t, err)
require.Equal(t, "fresh", got.OAuthAccessToken)
require.Equal(t, "new-refresh", got.OAuthRefreshToken)
})
t.Run("RefreshFailureReturnsEmpty", func(t *testing.T) {
// A refresh attempt that fails (e.g. invalid client config)
// must not surface an error to the caller; per the
// UserOIDCTokenSource contract this is treated as "no
// Authorization header".
t.Parallel()
db, _ := dbtestutil.NewDB(t)
store := dbauthzTestStore(t, db)
user := dbgen.User(t, db, database.User{})
dbgen.UserLink(t, db, database.UserLink{
UserID: user.ID,
LoginType: database.LoginTypeOIDC,
OAuthAccessToken: "stale",
OAuthRefreshToken: "refresh",
OAuthExpiry: dbtime.Now().Add(-time.Hour),
})
// An empty oauth2.Config triggers a refresh failure
// because it has no token endpoint to call.
src := newOIDCMCPTokenSource(store, &oauth2.Config{}, logger)
ctx := dbauthz.AsChatd(context.Background())
tok, err := src.OIDCAccessToken(ctx, user.ID)
require.NoError(t, err)
require.Empty(t, tok)
})
}