feat(msteams): add federated credential support (certificate + managed identity)#53615
feat(msteams): add federated credential support (certificate + managed identity)#53615HDYA wants to merge 24 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
bot
added
channel: msteams
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3c8dfa94a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Greptile SummaryThis PR adds certificate and Azure Managed Identity authentication paths to the MS Teams integration by refactoring Key findings:
Confidence Score: 3/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/msteams/src/sdk.test.ts
Line: 141
Comment:
**`createMSTeamsAdapter` + `continueConversation` test coverage dropped**
The previous `sdk.test.ts` contained a test titled `"provides deleteActivity in proactive continueConversation contexts"` that verified an HTTP `DELETE` was issued via `ctx.deleteActivity` inside the `logic` callback. That test — and the whole import of `createMSTeamsAdapter` — was removed in this PR and not replaced.
`createSendContext` (used by `continueConversation`) still wires up `deleteActivity` via `deleteActivityViaRest`, so the production code path still exists. A coverage gap now exists for the `continueConversation` proactive messaging flow. Consider adding at minimum one test that exercises `createMSTeamsAdapter.continueConversation` and verifies `sendActivity` / `deleteActivity` behavior against a `fetch` mock.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/msteams/src/sdk.ts
Line: 101-115
Comment:
**Credential instance recreated on every token call — token cache never reused**
`tokenProvider` creates a brand-new `ManagedIdentityCredential` / `DefaultAzureCredential` instance on every invocation. Each `@azure/identity` credential object maintains its own in-memory token cache; constructing a fresh instance discards that cache, so every call to `tokenProvider` issues a new outbound token request to Azure IMDS or AAD — even when the previous token is still valid.
Under realistic load (one Teams message → one `tokenProvider` call), this results in a new IMDS/AAD round-trip per message. IMDS is rate-limited, and `DefaultAzureCredential` probes multiple credential sources in sequence on each construction, making the overhead significant.
The credential should be created once (lazily, to keep the dynamic import on-demand) and captured in the closure — a module-level promise variable initialised on first call is the standard pattern for this in `@azure/identity` users. Something like:
```typescript
let credentialPromise: Promise<ManagedIdentityCredential | DefaultAzureCredential> | null = null;
const getCredential = async () => {
if (!credentialPromise) {
credentialPromise = import("@azure/identity").then((az) =>
creds.managedIdentityClientId
? new az.ManagedIdentityCredential(creds.managedIdentityClientId)
: new az.DefaultAzureCredential(),
);
}
return credentialPromise;
};
const tokenProvider = async (): Promise<string> => {
const credential = await getCredential();
const token = await credential.getToken(BOT_FRAMEWORK_SCOPE);
if (!token?.token) throw new Error("Failed to acquire token via managed identity.");
return token.token;
};
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/msteams/src/sdk.ts
Line: 85
Comment:
**Unguarded `readFileSync` surfaces raw filesystem errors**
If `creds.certificatePath` points to a missing or unreadable file, Node.js throws a raw `ENOENT`/`EACCES` error with no context about certificate-based auth. Wrapping the call in a try/catch and re-throwing with a descriptive message (e.g. `"Failed to read certificate file at '${creds.certificatePath}': ${err.message}"`) would produce a much clearer diagnostic for operators configuring this for the first time.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Merge origin/main into hdya/teams-authen..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a763a2493
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7faecd5659
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
7faecd5 to
3f09bf8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f09bf808b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e16c60c86
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
7e16c60 to
dbb0fb2
Compare
|
it appears that the MAC OS check error is from main branch |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ccbc9d5e9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
3ccbc9d to
f9cd5ea
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9cd5eafdd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
f9cd5ea to
718220a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 718220abb3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
75e9857 to
1ee45b1
Compare
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
1ee45b1 to
06716e6
Compare
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
06716e6 to
802ea3e
Compare
openclaw-barnacle
bot
removed
the
docs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 802ea3e9ee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
802ea3e to
25279e4
Compare
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
25279e4 to
698f758
Compare
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
192937e to
145def1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 145def140c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9db007103
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
b5c2006 to
4e8541c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e8541c6eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
pnpm-lock.yaml
Outdated
| '@typescript/native-preview': | ||
| specifier: 7.0.0-dev.20260401.1 | ||
| version: 7.0.0-dev.20260401.1 | ||
| specifier: 7.0.0-dev.20260331.1 | ||
| version: 7.0.0-dev.20260331.1 |
There was a problem hiding this comment.
The root lockfile now pins @typescript/native-preview to 7.0.0-dev.20260331.1, but package.json in this same commit still requires 7.0.0-dev.20260401.1; with CI running pnpm install --frozen-lockfile in .github/workflows/ci.yml, this mismatch will fail dependency installation before tests/build. Fresh evidence in this commit is the changed root importer entry at pnpm-lock.yaml now using the older dev build number.
Useful? React with 👍 / 👎.
extensions/msteams/src/sdk.test.ts
Outdated
| expect(appInstances[0]).toEqual({ | ||
| clientId: "my-app-id", | ||
| clientSecret: "my-secret", | ||
| tenantId: "my-tenant", | ||
| }); |
There was a problem hiding this comment.
createMSTeamsApp now always injects a no-op HTTP plugin, so the captured constructor options include a plugins property; this test uses strict toEqual with only clientId/clientSecret/tenantId, which makes the new case fail even when behavior is correct. Running this test file will produce a deterministic false negative until the assertion is relaxed (for example toMatchObject) or includes plugins.
Useful? React with 👍 / 👎.
…d identity) Add alternative authentication methods for Microsoft Teams beyond client secrets. Users can now authenticate using: - Certificate-based auth (PEM file + thumbprint) - Azure Managed Identity (system- or user-assigned) Changes: - MSTeamsCredentials is now a discriminated union with type: 'secret' | 'federated' - createMSTeamsApp dispatches by credential type: - secret: clientId + clientSecret (existing behavior, fully backward compatible) - certificate: reads PEM, passes clientCertificate to SDK App constructor - managed identity: dynamically imports @azure/identity for tokenProvider - MSTeamsConfig type gains 5 new optional fields: authType, certificatePath, certificateThumbprint, useManagedIdentity, managedIdentityClientId - Zod schema updated with new fields + cross-field validation in superRefine - 6 new env vars: MSTEAMS_AUTH_TYPE, MSTEAMS_CERTIFICATE_PATH, MSTEAMS_CERTIFICATE_THUMBPRINT, MSTEAMS_USE_MANAGED_IDENTITY, MSTEAMS_MANAGED_IDENTITY_CLIENT_ID - Tests updated for all auth paths (21 total: 14 token + 7 sdk) - No new npm dependencies (@azure/identity is dynamically imported)
…azure/identity dep The Teams SDK App constructor expects 'token' for custom token factories, not 'tokenProvider'. This caused the SDK to ignore the custom credential resolution (DefaultAzureCredential/WorkloadIdentityCredential) and fall back to its internal ManagedIdentityApplication via IMDS, which fails on AKS workload identity. Also adds @azure/identity as a production dependency so the dynamic import resolves at runtime.
Bump @microsoft/teams.apps from 2.0.5 to 2.0.6. v2.0.6 removes the problematic /api* wildcard route pattern incompatible with Express 5. This eliminates the need for the sed workaround in the AKSclaw Dockerfile (commit 705b855). Fix sdk.test.ts: rename .tokenProvider to .token to match sdk.ts property name, use vi.hoisted for @azure/identity mock constructors. All 350 msteams tests pass.
Add comprehensive docs for certificate-based and managed identity authentication: - New section: Federated Authentication (Certificate + Managed Identity) - Option A: Certificate-based auth with PEM file - Option B: Azure Managed Identity (system/user-assigned) - AKS Workload Identity setup guide with step-by-step instructions - Auth type comparison table - New config keys and env vars documented - Quick setup updated to reference federated auth
Auto-formatted by oxfmt to pass CI check.
Addresses review feedback and fixes implementation bugs: - P1: Hoist credential instances outside tokenProvider into a lazily- initialized cached promise, so @azure/identity token cache is reused across messages instead of creating fresh credential per call. - P1: System-assigned MI now uses ManagedIdentityCredential() instead of DefaultAzureCredential to avoid unintended auth source probing. - P1: Certificate auth now uses ClientCertificateCredential via token function instead of non-existent clientCertificate SDK option that was silently ignored at runtime. - P2: Wrap readFileSync in try/catch with descriptive error message when certificate file is missing or unreadable. - Fix: Token function signature now accepts (scope, tenantId?) to match SDK's TokenCredentials['token'] contract. - Fix: All TypeScript type errors resolved (add Client to test fake SDK, work around tsgo AppOptions narrowing).
The superRefine block rejected authType=federated when appId, tenantId, or certificatePath/useManagedIdentity were absent from the config object, but these fields can legitimately come from MSTEAMS_* environment variables. This blocked env-var-only deployments (a common secret-management pattern). Runtime validation in resolveMSTeamsCredentials() already handles the field resolution and returns undefined when config is incomplete.
Use nullish coalescing (??) instead of logical OR (||) so that an explicit config value of false is respected over the MSTEAMS_USE_ MANAGED_IDENTITY environment variable. Previously, a federated config that set useManagedIdentity: false with certificatePath would still resolve to managed identity if the env var was true.
…quired Certificate auth now uses @azure/identity ClientCertificateCredential which extracts the thumbprint from the PEM content automatically. Remove the mandatory thumbprint step from setup instructions and mark the certificateThumbprint config field as optional.
Address greptile P2 review comment: the previous deleteActivity + continueConversation test was removed during the SDK refactor. Add tests for: - continueConversation sendActivity (REST API client) - continueConversation deleteActivity (REST DELETE) - Missing serviceUrl / conversation.id validation - process() 200 response for message activities - process() immediate 200 for invoke activities
Vitest hoisting can transform function declarations inside vi.mock factories into arrow functions, which cannot be used with `new`. Switch to class syntax to ensure constructors work after hoisting. Also regenerate config docs baseline after rebase with main.
Snapshot MSTEAMS_* env vars before clearing and restore original values in teardown, preventing state leaks across suites in shared-process mode.
Federated certificate and managed-identity branches now receive the noOpHttp plugin, preventing Express 5 path-to-regexp incompatibility from the default Teams HTTP plugin.
HDYA
force-pushed
the
hdya/teams-authentications
branch
from
4e8541c to
30f4a03
Compare
1 participant
Add alternative authentication methods for Microsoft Teams beyond client secrets. Users can now authenticate using:
ClientCertificateCredential)ManagedIdentityCredential)Changes:
ClientCertificateCredentialfrom@azure/identitywith a custom token callbackManagedIdentityCredentialfrom@azure/identitywith a custom token callbackSummary
MSTeamsCredentialsis now a discriminated union (secret|federated). NewcreateFederatedApp,createCertificateApp, andcreateManagedIdentityAppfactory functions dispatch based on credential type. Credential instances are lazily created and cached for token reuse. Zod schema extended with optional fields.appId + appPassword + tenantIdsecret-based auth path is untouched. Default behavior (noauthTypeconfigured) resolves to"secret"— fully backward compatible.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause / Regression History (if applicable)
N/A — this is a new feature, not a bug fix.
Regression Test Plan (if applicable)
extensions/msteams/src/sdk.test.ts,extensions/msteams/src/token.test.tsClientCertificateCredentialfor token acquisitionManagedIdentityCredentialwith lazy cachinguseManagedIdentity: falseoverrides envMSTEAMS_USE_MANAGED_IDENTITY=trueUser-visible / Behavior Changes
authType,certificatePath,certificateThumbprint,useManagedIdentity,managedIdentityClientIdMSTEAMS_AUTH_TYPE,MSTEAMS_CERTIFICATE_PATH,MSTEAMS_CERTIFICATE_THUMBPRINT,MSTEAMS_USE_MANAGED_IDENTITY,MSTEAMS_MANAGED_IDENTITY_CLIENT_IDappId + appPasswordconfigs continue working without modificationSecurity Impact (required)
ManagedIdentityCredentialfor MI,ClientCertificateCredentialfor certs)Repro + Verification
Environment
authType: "federated",useManagedIdentity: truewith AKS workload identitySteps
authType: "federated"anduseManagedIdentity: trueExpected
Actual
Evidence
Human Verification (required)
useManagedIdentity: falseoverriding env varReview Conversations
Compatibility / Migration
resolveAuthType()defaults to"secret"when not configuredFailure Recovery (if this breaks)
authTypefrom config (defaults back to secret auth)Risks and Mitigations