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

IDownstreamApi handles Continuous Access Evaluation (CAE) #2616

Merged
merged 19 commits into from
Feb 26, 2024

Conversation

pmaytak
Copy link
Contributor

@pmaytak pmaytak commented Dec 13, 2023

Completes (partially) #2550

Changes:

  • Add a retry in IDownstreamApi for user and app requests. If the resource returns a 401 error, use claims to get a new token and call the resource again.

As per offline discussion, users will have to add client capabilities, which can be done like so:

services.Configure<ConfidentialClientApplicationOptions>(
                                 o => o.ClientCapabilities = new[] { "cp1" });

Tests:

  • Unit tests.
  • Integration tests will be part of Add CAE integration tests for downstream API #2653 when CAE environment is setup in the lab.
  • Manual tests.
    • User flow were tested by revoking the test user's session; app flow - by disabling the app's service principal.
    • Tested by calling Graph: /me endpoint for user flow, /users endpoint for app flow using the client and service dev apps.

Test scenarios:

  • Web app, user flow - when the user's session is revoked, Graph does send a 401 error with claims; when claims are used to get a new token, STS returns an error that the user needs to re-login; Identity Web handles the MsalUiRequiredException; after the user signs-in again, the app is redirected to call the downstream API again, which succeeds with the new token.
  • Web app, client credentials flow - when the service principal is disabled, Graph does send a 401 error with claims; but when acquiring a new token with claims, STS returns an error that the SP is disabled.
  • Web API, OBO flow - CAE doesn't work, not supported. The resource doesn't enforce the policies.

Manual test steps:

  • Set up dev up WebAppCallsWebApiCallsGraph with an appropriate tenant config.
  • In the appsettings add scopes for Graph for user and app:
    "GraphUser": {
        "Scopes": [ "user.read" ],
        "BaseUrl": "https://graph.microsoft.com/v1.0"
    },
    "GraphApp": {
        "Scopes": [ "https://graph.microsoft.com/.default" ],
        "BaseUrl": "https://graph.microsoft.com/v1.0"
    },

Testing user flow:

  1. In the controller Index action, add calling Graph for user: await _downstreamApi.GetForUserAsync<TestPoco>("GraphUser", options => options.RelativePath = "me");
  2. Run the client app, sign in with a test user.
  3. The token cache is populated with an access and refresh token.
  4. Navigate to the above controller action to call _downstreamApi.GetForUser.
    5 Internally Id Web uses the user token from the cache and calls Graph with it.
  5. Graph should return an HTTP 200 successful response.
  6. Revoke the test user's session.
  • Go to Azure Portal > Microsoft Entra ID > Users blade > select the test user > click Revoke sessions.
  • image
  1. Wait about 5 minutes for the changes to propagate to Graph.
  2. Navigate to the above controller action again to call _downstreamApi.GetForUser.
    10 Internally Id Web will use the cached token and call Graph with it.
  3. Graph will now return an HTTP 401 with claims.
  4. The retry logic will try to get a new user token, AAD will return an error, Id Web will catch the MsalUiRequiredEx, and ask the user to sign in again.
  5. Sign in again with the test user.
  6. Id Web will redirect to the controller action again which will call _downstreamApi.GetForUser.
  7. Id Web will retrieve the new cached token, and successfully call Graph.

Testing client credentials:

  1. In the controller Index action, add calling Graph for app: await _downstreamApi.GetForAppAsync<TestPoco>("GraphApp", options => options.RelativePath = "users");
  2. Run the client app, sign in with a test user.
  3. Navigate to the above controller action to call _downstreamApi.GetForApp.
  4. Internally Id Web gets an app token from AAD and calls Graph with it.
  5. Graph should return an HTTP 200 successful response.
  6. Disable the app's service principal.
  • Go to Azure Portal > Microsoft Entra ID > Enterprise applications > select this application > Properties blade > change "Enabled for users to sign-in?" to No.
  • This will prevent user sign-in and also accessing with app tokens.
  • image
  1. Wait about 5 minutes for the changes to propagate to Graph.
  2. Navigate to the above controller action again to call _downstreamApi.GetForApp.
  3. Internally Id Web will use the cached token and call Graph with it.
  4. Graph will now return an HTTP 401 with claims.
  5. The retry logic will try to get a new user token with claims, AAD will return an error that the service principal is disabled.

@pmaytak pmaytak changed the title IDownstreamApi handles CAE IDownstreamApi handles Continuous Access Evaluation (CAE) Dec 21, 2023
@pmaytak pmaytak marked this pull request as ready for review January 27, 2024 22:41
Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

doesn't seem to be the right approach and would prefer to have E2E test working before we merge.

@pmaytak
Copy link
Contributor Author

pmaytak commented Feb 23, 2024

I've reduced the scope of this PR. It doesn't set CAE flag by default; it only retries downstream API requests if the first response was 401 and contains claims.

Copy link
Collaborator

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

There is one question that needs to be solved (do not extract the claims if we don't have a 401)?

Copy link
Collaborator

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
thanks @pmaytak

Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@pmaytak pmaytak merged commit 407db82 into master Feb 26, 2024
4 checks passed
@pmaytak pmaytak deleted the pmaytak/cae-downstream branch February 26, 2024 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants