Skip to content

Conversation

@somalaya
Copy link
Contributor

@somalaya somalaya commented Aug 8, 2025

Issue : In my testing, I noticed that OneAuth team started sending a config for a confidential client and that there is a corner case for device enrollment url's scheme on eSTS side. Our contract with eSTS has been to send "browser://" scheme for device enrollment url redirect and we enable webcp flow once we notice the url is of browser scheme and has device enrollment parameters.

But eSTS's corner case is that when the calling app is a web app, they pass https:// scheme instead. My logic to handle webcp flow is not triggered in this case and breaks the user's sign in.
Although this is a corner case scenario, we may start seeing issues if consumers of OneAuth or MSAL register themselves as webapp/confidential clients unknowingly.

Link to ests code where they replace browser scheme with https in specific cases : https://msazure.visualstudio.com/One/_git/ESTS-Main?path=/src/Product/Microsoft.AzureAD.ESTS/Sts/ConditionalAccess/DevicePolicyError.cs&version=GBmaster&_a=contents&line=497&lineStyle=plain&lineEnd=503&lineStartColumn=1&lineEndColumn=10

My fix : Is a small change to start supporting webcp flow with https:// scheme as well.

Fixes AB#3344894

Copilot AI review requested due to automatic review settings August 8, 2025 18:46
@somalaya somalaya requested a review from a team as a code owner August 8, 2025 18:46
@github-actions
Copy link

github-actions bot commented Aug 8, 2025

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a corner case in device enrollment URL handling where eSTS sends https:// scheme instead of the expected browser:// scheme for web applications. The fix extends the webview client to handle device CA requests with https:// scheme in addition to the existing browser:// scheme handling.

  • Extended device CA request handling to support https:// scheme URLs
  • Added test coverage for the new https scheme handling
  • Refactored device CA processing logic into a separate method for better code organization

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
AzureActiveDirectoryWebViewClient.java Added https scheme support for device CA requests and refactored processing logic
AzureActiveDirectoryWebViewClientTest.java Added test case to verify https device CA URL handling

@somalaya somalaya changed the title Handling https scheme for device enrollment link Handling https scheme for device enrollment link, Fixes AB#3344894 Aug 8, 2025
@github-actions
Copy link

github-actions bot commented Aug 8, 2025

✅ Work item link check complete. Description contains link AB#3344894 to an Azure Boards work item.

@somalaya somalaya added the No-Changelog This Pull-Request has no associated changelog entry. label Aug 8, 2025
@somalaya somalaya merged commit 760b740 into dev Aug 8, 2025
46 of 54 checks passed
somalaya added a commit that referenced this pull request Aug 11, 2025
…94 (#2733)

In this PR
#2732,
I added a new check to detect device CA flow and follow our current
pattern of handling the device CA requests. But I did not secure it with
a flight.

There are 2 ways of adding a flight for this
1. Add a new flight that is independent of webcp feature 
2. Depend on webcp feature only. 
I went with 2nd option, because I only really need these changes when
webcp flight is ON so that users are not blocked by new flow. So, I am
securing it with webcp flight.


Fixes
[AB#3344894](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3344894)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No-Changelog This Pull-Request has no associated changelog entry.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants