Skip to content

Commit e0ddef1

Browse files
authored
Merge branch 'main' into bogavril/5809
2 parents 0a6063e + 022dcde commit e0ddef1

446 files changed

Lines changed: 10106 additions & 71427 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/copilot-instructions.md

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,67 @@
1+
# Code Review Rules
2+
3+
These rules apply to Copilot code review. Read all rules before commenting.
4+
5+
## Review scope
6+
7+
- Only comment on lines added or modified in the PR diff
8+
- Do not comment on pre-existing code unless the PR directly introduces the issue
9+
- Do not comment on style, formatting, or indentation
10+
- Focus exclusively on: bugs, security issues, logic errors, API contract violations
11+
- If unsure whether something is a bug, do not comment
12+
- Prefer no comment over a speculative comment
13+
- Do not re-post a comment already made on an earlier commit in the same PR
14+
15+
## Repo-specific patterns — do NOT flag these
16+
17+
These patterns are correct in this repo. Do not suggest changes:
18+
19+
- `[RunOn]` inherits from `TestMethodAttribute`. Do not flag as missing `[TestMethod]`
20+
- `Client.AppConfig.X` resolves via parent namespace `Microsoft.Identity`. Do not flag as unresolved namespace
21+
- `Assert.IsTrue(bool?)` is a valid MSTest overload. Do not flag nullable bool as a type mismatch
22+
- `Assert.DoesNotContain(substring, value)` — MSTest v4 signature is substring first, value second
23+
- `ConfigureAwait(false)` is intentional in library code. Do not suggest removal
24+
25+
## ConcurrentDictionary.GetOrAdd — always use factory delegate
26+
27+
`GetOrAdd(key, value)` eagerly evaluates the value arg. Flag any call where the second argument is not a delegate/lambda/method group:
28+
29+
- Bad: `pool.GetOrAdd(key, new ExpensiveObject());`
30+
- Good: `pool.GetOrAdd(key, _ => new ExpensiveObject());`
31+
32+
## C# coding standards
33+
34+
- Use `is null` / `is not null` instead of `== null` / `!= null`
35+
- No reflection in product code (`/src`). Acceptable in tests
36+
- Static fields: `s_camelCase` (e.g., `s_knownHosts`)
37+
- Ordinal string comparisons for protocol values, identifiers, cache keys
38+
- Validate inputs at method boundaries (fail fast with specific exception types)
39+
- Do not include secrets/tokens/PII in exception messages or logs
40+
- Use `nameof` instead of string literals for member names
41+
42+
## Testing standards
43+
44+
- MSTest SDK v4 with NSubstitute for mocking
45+
- Use `// Arrange`, `// Act`, `// Assert` comments
46+
- Prefer deterministic tests (no timing flakiness)
47+
48+
## Public API changes
49+
50+
- Update `PublicAPI.Unshipped.txt` for any public API additions/removals
51+
- XML doc comments required on all public APIs
52+
- Maintain backward compatibility
53+
54+
## MSAL-specific rules
55+
56+
- Use certificate-based auth over client secrets when possible
57+
- Use async APIs consistently
58+
- Keep dependencies minimal and well-justified
59+
60+
---
61+
62+
<!-- Everything below this line is for Copilot Chat and Copilot Agent only. -->
63+
<!-- Copilot code review reads only the first 4,000 characters of this file. -->
64+
165
Carefully review all markdown documents in the ../.clinerules folder. Those are your custom instructions.
266

367
---
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Source code review rules
2+
3+
Applies to: src/**/*.cs
4+
5+
These rules apply when reviewing production source code in this repository.
6+
7+
## Namespace resolution — do NOT flag
8+
9+
- `Client.AppConfig.X`, `Client.Internal.X`, and similar short namespace references resolve via parent namespace `Microsoft.Identity`. Do not flag as unresolved.
10+
11+
## ConcurrentDictionary.GetOrAdd
12+
13+
`GetOrAdd(key, value)` eagerly evaluates the value argument. Flag any call where the second argument is not a delegate/lambda/method group:
14+
15+
- Bad: `pool.GetOrAdd(key, new ExpensiveObject());`
16+
- Good: `pool.GetOrAdd(key, _ => new ExpensiveObject());`
17+
18+
## `ConfigureAwait(false)`
19+
20+
`ConfigureAwait(false)` is intentional in library code. Do not suggest removal.
21+
22+
## Public API changes
23+
24+
- Any public API additions or removals must be reflected in `PublicAPI.Unshipped.txt`
25+
- XML doc comments are required on all public APIs
26+
- Maintain backward compatibility — breaking changes require explicit justification
27+
28+
## MSAL-specific
29+
30+
- Keep dependencies minimal and well-justified
31+
32+
## Scope
33+
34+
- Only comment on source code added or modified in the PR diff
35+
- Do not comment on pre-existing code unless the PR directly introduces the issue
36+
- Do not comment on style or formatting — coding standards are enforced via .editorconfig
37+
- Focus on: bugs, security issues, logic errors, API contract violations
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Test file review rules
2+
3+
Applies to: tests/**/*.cs
4+
5+
These rules apply when reviewing test files in this repository.
6+
7+
## Test framework patterns — do NOT flag
8+
9+
- `[RunOn]` inherits from `TestMethodAttribute` (see `tests/Microsoft.Identity.Test.Integration.netcore/Infrastructure/TargetFramework.cs` line 15). Tests decorated with `[RunOn]` WILL be discovered by MSTest. Do not flag as missing `[TestMethod]`.
10+
- `Assert.IsTrue(bool?)` is a valid MSTest overload. Do not flag nullable bool arguments as type mismatches.
11+
- `Assert.DoesNotContain(substring, value)` — in MSTest v4, the first argument is the substring and the second is the value to search. Do not suggest swapping arguments.
12+
- `Assert.HasCount(expected, collection)` — valid MSTest v4 assertion. Do not suggest `Assert.AreEqual` for count checks.
13+
14+
## Test conventions
15+
16+
- Use MSTest SDK v4 with NSubstitute for mocking
17+
- Use `// Arrange`, `// Act`, `// Assert` comments
18+
- Prefer deterministic tests: avoid `Thread.Sleep`, timing dependencies, or environment-specific behavior
19+
- Copy existing style in nearby files for test method names
20+
21+
## Scope
22+
23+
- Only comment on test code that is added or modified in the PR diff
24+
- Do not comment on pre-existing test patterns or style
25+
- Do not re-post comments already made on earlier commits
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
name: Auto-Answer Issues
2+
3+
on:
4+
issues:
5+
types: [opened, labeled]
6+
7+
permissions:
8+
issues: write
9+
contents: read
10+
11+
jobs:
12+
answer-issue:
13+
runs-on: ubuntu-latest
14+
# Only run for issues created by org members or owners (i.e., Microsoft Open Source enterprise members).
15+
# github.event.issue.author_association is set by GitHub based on the issue author's relationship
16+
# to this repository. MEMBER = org member, OWNER = repo/org owner. This prevents untrusted
17+
# external contributors from triggering the Azure OpenAI-backed responder and consuming secrets/tokens.
18+
if: |
19+
github.event.issue.author_association == 'MEMBER' ||
20+
github.event.issue.author_association == 'OWNER' ||
21+
github.event.issue.author_association == 'COLLABORATOR'
22+
steps:
23+
- name: Checkout repository
24+
uses: actions/checkout@v4
25+
26+
- name: Setup Node.js
27+
uses: actions/setup-node@v4
28+
with:
29+
node-version: '20'
30+
31+
- name: Install dependencies
32+
run: npm install @octokit/rest openai
33+
34+
- name: Generate and post response
35+
env:
36+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
37+
AZURE_OPENAI_API_KEY: ${{ secrets.AZURE_OPENAI_API_KEY }}
38+
AZURE_OPENAI_ENDPOINT: ${{ secrets.AZURE_OPENAI_ENDPOINT }}
39+
AZURE_OPENAI_DEPLOYMENT: ${{ secrets.AZURE_OPENAI_DEPLOYMENT }}
40+
AZURE_OPENAI_API_VERSION: "2024-12-01-preview"
41+
ISSUE_NUMBER: ${{ github.event.issue.number }}
42+
ISSUE_TITLE: ${{ github.event.issue.title }}
43+
ISSUE_BODY: ${{ github.event.issue.body }}
44+
REPO_OWNER: ${{ github.repository_owner }}
45+
REPO_NAME: ${{ github.event.repository.name }}
46+
run: |
47+
node - << 'EOF'
48+
async function main() {
49+
// Use dynamic import() for ESM-only packages (openai v4+ and @octokit/rest v19+
50+
// are ESM-only; dynamic import() works from CommonJS in Node 20).
51+
const { Octokit } = await import("@octokit/rest");
52+
const { AzureOpenAI } = await import("openai");
53+
const {
54+
GITHUB_TOKEN,
55+
AZURE_OPENAI_API_KEY,
56+
AZURE_OPENAI_ENDPOINT,
57+
AZURE_OPENAI_DEPLOYMENT,
58+
AZURE_OPENAI_API_VERSION,
59+
ISSUE_NUMBER,
60+
ISSUE_TITLE,
61+
ISSUE_BODY,
62+
REPO_OWNER,
63+
REPO_NAME
64+
} = process.env;
65+
66+
if (!GITHUB_TOKEN) {
67+
throw new Error("GITHUB_TOKEN is not set.");
68+
}
69+
if (!AZURE_OPENAI_API_KEY) {
70+
throw new Error("AZURE_OPENAI_API_KEY is not set.");
71+
}
72+
if (!AZURE_OPENAI_ENDPOINT) {
73+
throw new Error("AZURE_OPENAI_ENDPOINT is not set.");
74+
}
75+
if (!AZURE_OPENAI_DEPLOYMENT) {
76+
throw new Error("AZURE_OPENAI_DEPLOYMENT is not set.");
77+
}
78+
if (!ISSUE_NUMBER || !REPO_OWNER || !REPO_NAME) {
79+
throw new Error("Required issue/repository environment variables are missing.");
80+
}
81+
82+
const issueNumber = parseInt(ISSUE_NUMBER, 10);
83+
const title = ISSUE_TITLE || "";
84+
const body = ISSUE_BODY || "";
85+
86+
const octokit = new Octokit({ auth: GITHUB_TOKEN });
87+
88+
// Check if the bot has already commented on this issue to avoid duplicate responses.
89+
const comments = await octokit.paginate(octokit.issues.listComments, {
90+
owner: REPO_OWNER,
91+
repo: REPO_NAME,
92+
issue_number: issueNumber,
93+
per_page: 100
94+
});
95+
const botAlreadyCommented = comments.some(
96+
(comment) => comment.user?.login === "github-actions[bot]"
97+
);
98+
if (botAlreadyCommented) {
99+
console.log("Bot has already commented on this issue. Skipping.");
100+
return;
101+
}
102+
103+
const openai = new AzureOpenAI({
104+
apiKey: AZURE_OPENAI_API_KEY,
105+
endpoint: AZURE_OPENAI_ENDPOINT,
106+
deployment: AZURE_OPENAI_DEPLOYMENT,
107+
apiVersion: AZURE_OPENAI_API_VERSION
108+
});
109+
110+
const prompt = `
111+
You are a helpful GitHub assistant. An issue has been opened in the repository \`${REPO_OWNER}/${REPO_NAME}\`.
112+
113+
Title:
114+
${title}
115+
116+
Body:
117+
${body}
118+
119+
Please write a concise, friendly reply that:
120+
- Acknowledges the question or issue.
121+
- Asks for any missing information if needed.
122+
- Suggests next steps or possible causes based only on the information provided.
123+
- Uses markdown formatting suitable for a GitHub issue comment.
124+
`;
125+
126+
const completion = await openai.chat.completions.create({
127+
model: AZURE_OPENAI_DEPLOYMENT,
128+
messages: [
129+
{ role: "system", content: "You are a helpful assistant for triaging GitHub issues." },
130+
{ role: "user", content: prompt }
131+
]
132+
});
133+
134+
const reply = (completion.choices[0]?.message?.content || "").trim();
135+
if (!reply) {
136+
throw new Error("Azure OpenAI returned an empty response.");
137+
}
138+
139+
await octokit.issues.createComment({
140+
owner: REPO_OWNER,
141+
repo: REPO_NAME,
142+
issue_number: issueNumber,
143+
body: reply
144+
});
145+
}
146+
147+
main().catch((error) => {
148+
console.error("Failed to generate or post response:", error);
149+
process.exit(1);
150+
});
151+
EOF

CHANGELOG.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,29 @@
1+
4.84.0
2+
======
3+
4+
### New Features
5+
- Added `CacheOptions.DisableInternalCacheOptions` static property and `CacheOptions.IsInternalCacheDisabled` to allow disabling MSAL's internal token cache. Added `CacheRefreshReason.CacheDisabled` and `MsalError.InternalCacheDisabled` to support this scenario. [#5947](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5947)
6+
- Added `AuthenticationResultExtensions.GetRefreshToken()` extension method for accessing refresh tokens from `AuthenticationResult`. [#5947](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5947)
7+
- Added `WithAttributeTokens` and `WithExtraBodyParameters` extension methods on `AbstractConfidentialClientAcquireTokenParameterBuilder` for enhanced extensibility. [#5888](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5888)
8+
- Added `CertificateOptions.SendCertificateOverMtls` for mTLS Proof-of-Possession certificate support. [#5849](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5849)
9+
- Added `AssertionRequestOptions.CorrelationId` property for correlation ID support in FIC assertion requests. [#5937](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5937)
10+
11+
### Changes
12+
- Removed experimental feature gate from `WithClientAssertion(ClientSignedAssertion)` overload. [#5945](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5945)
13+
- Removed embedded Newtonsoft.Json dependency, migrated to `System.Text.Json` exclusively. [#5959](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5959)
14+
- Removed mTLS PoP region as a hard requirement. [#5902](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5902)
15+
- Refactored client credential material resolution. [#5835](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5835)
16+
- Added in-process MAA token caching to PopKeyAttestor. [#5887](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5887)
17+
- Added raw STS error code to MsalFailure metric. [#5961](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5961)
18+
- Support forwarding MSAL client metadata headers through IMDS to ESTS. [#5912](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5912)
19+
20+
### Bug Fixes
21+
- Fixed eager evaluation in `ConcurrentDictionary.GetOrAdd` calls. [#5950](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5950)
22+
- Fixed `System.ValueTuple` conditional dependency to `net462` only. [#5894](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5894)
23+
- Validated `clientSignedAssertionProvider` delegate is non-null in `WithClientAssertion`. [#5956](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5956)
24+
- Improved `MtlsPopTokenNotSupportedinImdsV1` error message clarity. [#5908](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5908)
25+
- Added additional checks for issuer validation. [#5931](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5931)
26+
127
4.83.3
228
======
329

Directory.Packages.props

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@
6969
<PackageVersion Include="NuGet.Versioning" Version="6.8.0" />
7070
<PackageVersion Include="NUnit" Version="3.13.3" />
7171
<PackageVersion Include="NUnit3TestAdapter" Version="4.5.0" />
72-
<PackageVersion Include="OpenTelemetry.Exporter.Console" Version="1.6.0" />
73-
<PackageVersion Include="OpenTelemetry.Exporter.InMemory" Version="1.6.0" />
72+
<PackageVersion Include="OpenTelemetry.Exporter.Console" Version="1.15.3" />
73+
<PackageVersion Include="OpenTelemetry.Exporter.InMemory" Version="1.15.3" />
7474
<PackageVersion Include="Polly" Version="7.2.3" />
7575
<PackageVersion Include="Selenium.Support" Version="4.19.0" />
7676
<PackageVersion Include="Selenium.WebDriver" Version="4.19.0" />

PerformanceTests.sln

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Identity.Client.T
99
EndProject
1010
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Identity.Client", "src\client\Microsoft.Identity.Client\Microsoft.Identity.Client.csproj", "{E94D56E5-AAF5-4BF1-B956-BB600F6E1C0C}"
1111
EndProject
12+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Identity.Lab.Api", "src\client\Microsoft.Identity.Lab.Api\Microsoft.Identity.Lab.Api.csproj", "{4A4136D1-8A7B-4C95-9328-DF7CA4F437C6}"
13+
EndProject
14+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Identity.Test.Common", "tests\Microsoft.Identity.Test.Common\Microsoft.Identity.Test.Common.csproj", "{A1839AD4-12CE-4C9F-9E46-55F76A0A68DD}"
15+
EndProject
1216
Global
1317
GlobalSection(SolutionConfigurationPlatforms) = preSolution
1418
Debug|Any CPU = Debug|Any CPU
@@ -23,6 +27,14 @@ Global
2327
{E94D56E5-AAF5-4BF1-B956-BB600F6E1C0C}.Debug|Any CPU.Build.0 = Debug|Any CPU
2428
{E94D56E5-AAF5-4BF1-B956-BB600F6E1C0C}.Release|Any CPU.ActiveCfg = Release|Any CPU
2529
{E94D56E5-AAF5-4BF1-B956-BB600F6E1C0C}.Release|Any CPU.Build.0 = Release|Any CPU
30+
{4A4136D1-8A7B-4C95-9328-DF7CA4F437C6}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
31+
{4A4136D1-8A7B-4C95-9328-DF7CA4F437C6}.Debug|Any CPU.Build.0 = Debug|Any CPU
32+
{4A4136D1-8A7B-4C95-9328-DF7CA4F437C6}.Release|Any CPU.ActiveCfg = Release|Any CPU
33+
{4A4136D1-8A7B-4C95-9328-DF7CA4F437C6}.Release|Any CPU.Build.0 = Release|Any CPU
34+
{A1839AD4-12CE-4C9F-9E46-55F76A0A68DD}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
35+
{A1839AD4-12CE-4C9F-9E46-55F76A0A68DD}.Debug|Any CPU.Build.0 = Debug|Any CPU
36+
{A1839AD4-12CE-4C9F-9E46-55F76A0A68DD}.Release|Any CPU.ActiveCfg = Release|Any CPU
37+
{A1839AD4-12CE-4C9F-9E46-55F76A0A68DD}.Release|Any CPU.Build.0 = Release|Any CPU
2638
EndGlobalSection
2739
GlobalSection(SolutionProperties) = preSolution
2840
HideSolutionNode = FALSE

build/pipeline-perf-tests-automation.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ jobs:
3737
inputs:
3838
restoreSolution: PerformanceTests.sln
3939

40+
- task: DotNetCoreCLI@2
41+
displayName: 'dotnet restore'
42+
inputs:
43+
command: 'restore'
44+
projects: 'PerformanceTests.sln'
45+
4046
- task: MSBuild@1
4147
displayName: Build PerformanceTests.sln
4248
inputs:

build/platform_and_feature_flags.props

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
<Project>
2+
<!-- System.Text.Json is used on all target frameworks -->
3+
<PropertyGroup>
4+
<DefineConstants>$(DefineConstants);SUPPORTS_SYSTEM_TEXT_JSON</DefineConstants>
5+
</PropertyGroup>
26
<PropertyGroup Condition="'$(TargetFramework)' == '$(TargetFrameworkNetCore)' or '$(TargetFramework)' == '$(TargetFrameworkNet)'">
3-
<DefineConstants>$(DefineConstants);SUPPORTS_SYSTEM_TEXT_JSON;NET_CORE;SUPPORTS_CONFIDENTIAL_CLIENT;SUPPORTS_CUSTOM_CACHE;SUPPORTS_BROKER;SUPPORTS_WIN32;</DefineConstants>
7+
<DefineConstants>$(DefineConstants);NET_CORE;SUPPORTS_CONFIDENTIAL_CLIENT;SUPPORTS_CUSTOM_CACHE;SUPPORTS_BROKER;SUPPORTS_WIN32;</DefineConstants>
48
</PropertyGroup>
59
<PropertyGroup Condition="'$(TargetFramework)' == '$(TargetFrameworkNet)' or '$(TargetFramework)' == '$(TargetFrameworkNetDesktop462)' or '$(TargetFramework)' == '$(TargetFrameworkNetDesktop472)' or '$(TargetFramework)' == '$(TargetFrameworkNetStandard)'">
610
<DefineConstants>$(DefineConstants);SUPPORTS_OTEL;</DefineConstants>

0 commit comments

Comments
 (0)