feat(microsoft): add microsoft people sdk - #471
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
4 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/microsoft-people/src/client.ts">
<violation number="1" location="packages/microsoft-people/src/client.ts:5">
P3: This new client duplicates the existing Microsoft Graph transport implementation, creating a second maintenance path for auth, pagination URLs, and error handling. A shared internal Graph client/helper would keep those behaviors aligned.</violation>
<violation number="2" location="packages/microsoft-people/src/client.ts:51">
P2: The `Authorization` header is set first and then user-supplied headers are spread after it (`{ Authorization: ..., ...headers }`). This allows callers to the public `get`/`number` methods to pass an `Authorization` header that overrides the client's access token. If a caller unintentionally (or maliciously) includes an `Authorization` header in their `RequestHeaders`, it would silently replace the bearer token that was configured at construction time, leading to unexpected authentication behavior. Consider spreading user headers first then setting `Authorization` last so it cannot be overridden.</violation>
</file>
<file name="packages/microsoft-people/src/error.ts">
<violation number="1" location="packages/microsoft-people/src/error.ts:31">
P3: Microsoft Graph error handling now has two near-identical implementations that have already drifted in malformed-body validation. Extract the shared OData/API error implementation into a common module so fixes apply consistently to both SDKs.</violation>
<violation number="2" location="packages/microsoft-people/src/error.ts:177">
P2: Malformed JSON error bodies can be returned as typed `ODataError`s and produce messages such as `400 [object Object]` when `message` is non-string. Validate the parsed value's object and string fields before returning it, matching the Microsoft Calendar client.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| function parseErrorBody(text: string): ODataError | undefined { | ||
| try { | ||
| const error: ODataError = JSON.parse(text); |
There was a problem hiding this comment.
P2: Malformed JSON error bodies can be returned as typed ODataErrors and produce messages such as 400 [object Object] when message is non-string. Validate the parsed value's object and string fields before returning it, matching the Microsoft Calendar client.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/microsoft-people/src/error.ts, line 177:
<comment>Malformed JSON error bodies can be returned as typed `ODataError`s and produce messages such as `400 [object Object]` when `message` is non-string. Validate the parsed value's object and string fields before returning it, matching the Microsoft Calendar client.</comment>
<file context>
@@ -0,0 +1,244 @@
+
+function parseErrorBody(text: string): ODataError | undefined {
+ try {
+ const error: ODataError = JSON.parse(text);
+
+ if (!error.error.code) {
</file context>
|
|
||
| private buildHeaders(headers?: RequestHeaders) { | ||
| return { | ||
| Authorization: `Bearer ${this.accessToken}`, |
There was a problem hiding this comment.
P2: The Authorization header is set first and then user-supplied headers are spread after it ({ Authorization: ..., ...headers }). This allows callers to the public get/number methods to pass an Authorization header that overrides the client's access token. If a caller unintentionally (or maliciously) includes an Authorization header in their RequestHeaders, it would silently replace the bearer token that was configured at construction time, leading to unexpected authentication behavior. Consider spreading user headers first then setting Authorization last so it cannot be overridden.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/microsoft-people/src/client.ts, line 51:
<comment>The `Authorization` header is set first and then user-supplied headers are spread after it (`{ Authorization: ..., ...headers }`). This allows callers to the public `get`/`number` methods to pass an `Authorization` header that overrides the client's access token. If a caller unintentionally (or maliciously) includes an `Authorization` header in their `RequestHeaders`, it would silently replace the bearer token that was configured at construction time, leading to unexpected authentication behavior. Consider spreading user headers first then setting `Authorization` last so it cannot be overridden.</comment>
<file context>
@@ -0,0 +1,163 @@
+
+ private buildHeaders(headers?: RequestHeaders) {
+ return {
+ Authorization: `Bearer ${this.accessToken}`,
+ ...headers,
+ };
</file context>
| @@ -0,0 +1,163 @@ | |||
| import { APIError, ConnectionError, TimeoutError } from "./error"; | |||
There was a problem hiding this comment.
P3: This new client duplicates the existing Microsoft Graph transport implementation, creating a second maintenance path for auth, pagination URLs, and error handling. A shared internal Graph client/helper would keep those behaviors aligned.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/microsoft-people/src/client.ts, line 5:
<comment>This new client duplicates the existing Microsoft Graph transport implementation, creating a second maintenance path for auth, pagination URLs, and error handling. A shared internal Graph client/helper would keep those behaviors aligned.</comment>
<file context>
@@ -0,0 +1,163 @@
+import type { QueryParams, RequestHeaders } from "./interfaces";
+import { Users } from "./users";
+
+export class MicrosoftPeople {
+ private static readonly BASE_URL = "https://graph.microsoft.com/v1.0";
+ private static readonly ORIGIN_URL = "https://graph.microsoft.com";
</file context>
| [key: string]: unknown; | ||
| } | ||
|
|
||
| export class APIError< |
There was a problem hiding this comment.
P3: Microsoft Graph error handling now has two near-identical implementations that have already drifted in malformed-body validation. Extract the shared OData/API error implementation into a common module so fixes apply consistently to both SDKs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/microsoft-people/src/error.ts, line 31:
<comment>Microsoft Graph error handling now has two near-identical implementations that have already drifted in malformed-body validation. Extract the shared OData/API error implementation into a common module so fixes apply consistently to both SDKs.</comment>
<file context>
@@ -0,0 +1,244 @@
+ [key: string]: unknown;
+}
+
+export class APIError<
+ TStatus extends number | undefined = number | undefined,
+ THeaders extends Headers | undefined = Headers | undefined,
</file context>
Description
Briefly describe what you did and why.
Screenshots / Recordings
Add screenshots or recordings here to help reviewers understand your changes.
Type of Change
Related Areas
Testing
Checklist
Notes
(Optional) Add anything else you'd like to share.
By submitting, I confirm I understand and stand behind this code. If AI was used, I’ve reviewed and verified everything myself.
Summary by cubic
Adds
@analog/microsoft-people, a typed client for Microsoft Graph People to list, page, count, and fetch person details. Strengthens reliability with robust transport/API error handling and clear types.MicrosoftPeopleclient withusers.people:list,listMore(nextLink),$count,get.$top,$skip,$search,$filter,$count,$orderby,$select,$expand.APIError,AuthenticationError,RateLimitError,TimeoutError,ConnectionError, etc.).AbortSignaland custom headers; handlesuserId: "me"alias and validates absolute@odata.nextLinkorigins.Written for commit 1a12509. Summary will update on new commits.