Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DevApps WebAppCallsMicrosoftGraph improvement suggestions #3284

Open
rayluo opened this issue Mar 6, 2025 · 0 comments · May be fixed by #3278
Open

DevApps WebAppCallsMicrosoftGraph improvement suggestions #3284

rayluo opened this issue Mar 6, 2025 · 0 comments · May be fixed by #3278

Comments

@rayluo
Copy link

rayluo commented Mar 6, 2025

Microsoft.Identity.Web Library

Microsoft.Identity.Web

Microsoft.Identity.Web version

master branch

Web app

Sign-in users and call web APIs

Web API

Not Applicable

Token cache serialization

In-memory caches

Description

The sample works with the out-of-the-box settings, but tends to run into exceptions when a new developer experiments different settings.

Creating this issue per @jmprieur 's suggestion.

Please refer to the reproduction steps for details.

Reproduction steps

  1. Checkout the master branch and run the Dev App WebAppCallsMicrosoftGraph as-is, sign in (I used the lab4's test account). The web app technically works, but what it shows on screen is something like this:

    Welcome
    , Learn about building Web apps with ASP.NET Core.
    
    result = 
    Property	Value
    photo		NO PHOTO
    

    It supposes to show a username and an api call result before "Learn" and after "result =". But it didn't.
    Investigation shows that the test account does not have a photo, which is fine, but the current try...catch in Index.cshtml.cs is overly broad, so it unexpectedly bypassed the username and api call snippets.

    So, the first suggestion here is to move the ViewData["name"] = user.DisplayName; and var graphData = await _downstreamApi.CallApiForUserAsync("GraphBeta"); ViewData["json"] = await graphData.Content.ReadAsStringAsync(); out of the current try...catch.

    After making the change, this Dev App can show username and api call result. But that api call was made to the https://graph.microsoft.com/beta which is a metadata endpoint, usually not what a developer really wants.

  2. As an experiment partially inspired by this line options.BaseUrl = "https://graph.microsoft.com/v1.0/me";, a developer may attempt to make similar change, but declaratively, inside appsettings.json, from "BaseUrl": "https://graph.microsoft.com/beta" to "BaseUrl": "https://graph.microsoft.com/v1.0/me".

    After that appsettings.json change, rerun the app, it runs into an exception:

    An unhandled exception occurred while processing the request.
    ODataError: Resource 'me' does not exist or one of its queried reference-property objects are not present.
    
    WebAppCallsMicrosoftGraph.Pages.IndexModel.OnGet() in Index.cshtml.cs
    +
             var user = await _graphServiceClient.Me.GetAsync(r => 
    

    Perhaps the app developer is not supposed to configure the baseurl like that. Nonetheless, it doesn't harm for us to also wrap the var user = await _graphServiceClient.Me.GetAsync(...); part into its dedicated try...catch clause. Once we do that, this Dev App will work robustly, displaying the api result, while gracefully displaying the username as empty (or whatever placeholder value we choose).

Error message

No response

Id Web logs

No response

Relevant code snippets

https://github.com/AzureAD/microsoft-identity-web/blob/master/tests/DevApps/WebAppCallsMicrosoftGraph/Pages/Index.cshtml.cs#L30-L54

Regression

No response

Expected behavior

Please refer to the repro steps.

@rayluo rayluo linked a pull request Mar 6, 2025 that will close this issue
4 tasks
@rayluo rayluo linked a pull request Mar 6, 2025 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant