microsoftgraph / msgraph-sdk-dotnet

Microsoft Graph Client Library for .NET!
https://graph.microsoft.com
Other
690 stars 246 forks source link

GraphClient fail to fetch User if UPN start with $(dollar sign) #2635

Open yejingyu opened 3 weeks ago

yejingyu commented 3 weeks ago

Describe the bug

Based on the userPrincipalName section of this documentation, An UPN starts with $ (dollar sign) is a valid UPN. However, if we pass in a UPN starting with $, we would get an error saying:

Microsoft.Graph.Models.ODataErrors.ODataError: The request URI is not valid. Since the segment 'users' refers to a collection, this must be the last segment in the request URI or it must be followed by an function or action that can be bound to it otherwise all intermediate segments must refer to a single resource.

Expected behavior

If the UPN is valid, we should be able to get the user without any error.

How to reproduce

Using the code below:

var upn = "$abc@def.com";

User user = await graphClient
  .Users[upn]
  .GetAsync(requestConfiguration =>
  {
      requestConfiguration.QueryParameters.Select = new string[] { "id", "UserPrincipalName" };
  })
  .ConfigureAwait(false);

we can also trigger the same error (link) in the Graph Explorer playground.

SDK Version

5.46.0

Latest version known to work for scenario above?

No response

Known Workarounds

N/A Previously we called Graph with HTTP Client and HttpUtility.UrlEncode(upn) works. However, because the Graph SDK 5.0 is doing the encoding, double encoding would only make the problem worse.

Debug output

No response

Configuration

No response

Other information

Based on this thread, it is a known feature when calling the Graph API. we have different API syntax for UPN starting with $.

shemogumbe commented 3 weeks ago

Hello @yejingyu thanks for using the SDK and for raising this.

Looking at the docs https://learn.microsoft.com/en-us/microsoft-365/enterprise/prepare-for-directory-synchronization?view=o365-worldwide#2-directory-object-and-attribute-preparation I don't see where it explicitly says $ sign is allowed in UPNs, neither does it discourage it's use as it is not inluded in the list of unsupported characters.

Note that the $ character in interpreted as the beging of an Odata system query option e.g $filter, $expand, $select etc, using this directly in UPN could lead to that being misinterpreted as such and lead to a bad URL as you see in graph explorer.

  1. Is there a specific reason why the UPN must start with $
  2. If 1 above is true, have you considered URL-encouding the UPN before executing the request, example
    
    using System.Net;

var upn = WebUtility.UrlEncode("$abc@def.com");


This way you end up having $_abc encoded to %24abc.
MartinM85 commented 3 weeks ago

The Graph API doc recommends to enclose the userPrincipalName with $ in parentheses and single quotes, but it's not supported right now by SDK.

yejingyu commented 3 weeks ago

Hi @shemogumbe

  1. The document didn't include $ as an unsupported character. In addition, we can create a UPN starting with $ . Thus, I don't have any justification to convince my teammates, my customers, and myself that $-UPN is invalid UPN. This is also the reason I believe Graph Client should support the UPN starting with $.
  2. As this comment suggests, Graph Client is doing URL encoding too. Unless we have the option to disable this feature, double encoding would simply fail all requests.