Repository navigation
Fix Graph URL tokenization for parenthesized segments - #2043
Open
waldekmastykarz wants to merge 2 commits into
Open
waldekmastykarz wants to merge 2 commits into
waldekmastykarz wants to merge 2 commits into
Conversation
…2041 - Fix out-of-range Substring when extracting text within parentheses, which made every key/function segment (e.g. users('id')) throw - Match function names containing digits, underscores and dots (e.g. getOffice365ActiveUserDetail, microsoft.graph.delta) - Keep empty parentheses as-is (delta()) - Split parameters outside OData quoted literals - Sanitize path segments positionally instead of using a global string replace, which corrupted later segments with the same value Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Parenthesized user-key URLs bypass delegated-scope correction in both minimal-permissions plugins, requiring an expanded endpoint filter and a permission-calculation regression test.
1 open issue
What changed in this PR
Fixes parenthesized Microsoft Graph URL tokenization so permissions plugins can process key and function segments without exceptions.
Changes:
- Corrects parameter extraction and supports digits, underscores, and namespaces in function names.
- Preserves empty parentheses and handles separators inside quoted OData values.
- Sanitizes path segments independently and adds regression tests.
| File | Description |
|---|---|
| DevProxy.Integration.Tests/GraphUtilsTests.cs | Adds coverage for parenthesized keys, functions, quoted parameters, and repeated IDs. |
| DevProxy.Abstractions/Utils/ProxyUtils.cs | Fixes parenthesized-segment parsing and prevents cross-segment replacements. |
Review effort: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Closes #2041
GraphUtils.GetTokenizedUrl(viaProxyUtils.SanitizeUrl) threwArgumentOutOfRangeExceptionfor any Graph URL with a parenthesized segment, so the Graph permissions plugins skipped those requests.Changes
Fix the substring bug: the text inside the parentheses was extracted with the wrong length, so every key or function segment threw (
users('id'),applications(appId='…'),delta()).Widen the function regex: it now accepts digits,
_and.in the name (getOffice365ActiveUserDetail,microsoft.graph.…). Previously these segments fell through to{reports-id}.Empty parentheses are kept as-is (
delta()).Quote-aware parameter parsing:
,and=inside OData quoted literals no longer split parameters.Positional segment sanitization: each path segment is now sanitized separately and the segments are rejoined. Previously a global
string.Replacelet earlier IDs corrupt later segments. For example,…/items/1/getActivitiesByInterval(startDateTime='2017-01-01')became'20{items-id}7-…', and/sites/root/lists/1/items/1became/lists/{lists-id}/items/{lists-id}.Delegated user-scope correction:
GraphUtils.UpdateUserScopesAsyncnow also treats/users(<value>)endpoints as other-user endpoints, so they get the same scope correction as/users/{users-id}.Tokenized format
The output keeps the format the existing code was designed for:
users(<value>),fn(key=<value>),delta(). I checked it againsthttps://graph-devx-api.microsoft.com/permissions(POST). It resolves/users(<value>),/applications(appId=<value>),/devices(deviceId=<value>)and/me/drive/recent(). Thereports/get*functions return "not found" in every format I tried (<value>,{period},'D7', and others). That's a gap in the DevX data, not a tokenization problem.Tests
Added
GraphUtilsTestscases for key-in-parentheses, functions with parameters (including quoted commas/equals and escaped quotes), namespaced functions and repeated segment values. Added anUpdateUserScopesAsynctest that uses a fakeHttpClientand covers both user-key shapes.