Skip to content
This repository has been archived by the owner on Jun 30, 2023. It is now read-only.

CreateFromResourceUrlAsync gone missing in ADAL 5.x preview 1 #1599

Closed
6 tasks
MattB-msft opened this issue May 9, 2019 · 8 comments
Closed
6 tasks

CreateFromResourceUrlAsync gone missing in ADAL 5.x preview 1 #1599

MattB-msft opened this issue May 9, 2019 · 8 comments

Comments

@MattB-msft
Copy link

MattB-msft commented May 9, 2019

Which Version of ADAL are you using ?
ADAL5.0.1-preview => ADAL 5.0.5

Which platform has the issue?
,net462

What authentication flow has the issue?

  • Desktop / Mobile
    • [x ] Interactive
    • Integrated Windows Auth
    • Username Password
    • Device code flow (browserless)
  • Web App
    • Authorization code
    • OBO
  • Web API
    • OBO

Other? - please describe;

Repro

 AuthenticationParameters authParameters = AuthenticationParameters.CreateFromResourceUrlAsync(requestUri).Result;

Expected behavior
CreateFromResourceUrlAsync is no longer present as static.. though it appears in GitHub codebase still.

Actual behavior
A clear and concise description of what happens, e.g. exception is thrown, UI freezes

Possible Solution
Looks like it got lost with this PR:
486c46c#diff-ac95f94d0b5f9e01efcf64255d0e85b1

Looks like this may have been a mistake?
You cannot create an instance of AuthenticationParameters to get at it.

@henrik-me
Copy link
Contributor

Change was intentional, however for certain scenarios makes sense that this is available as a static as well. @bgavrilMS ?

@henrik-me
Copy link
Contributor

@MattB-msft : Can you pls. describe the scenario where you need this?

@MattB-msft
Copy link
Author

MattB-msft commented May 14, 2019 via email

@bgavrilMS
Copy link
Member

Yes, I see the problem. We refactored ADAL to use an abstraction over the http layer and we made these methods non-static to be able to inject an HttpManager. Sadly, the constructor is internal, so there is no way to use the public method.

Here's what I propose:

  • remove the non-static method. It won't break anyone because it can't be called, as the ctor for the class is internal
  • bring back the static

@bgavrilMS
Copy link
Member

@MattB-msft - you can use the following workaround until a new release is ready:

  • send a non-authenticated GET request to the resource. It should fail with a 401.
  • extract the "WWW-Authenticate" header from the response
  • Call CreateFromResponseAuthenticateHeader with the value

@bgavrilMS
Copy link
Member

@MattB-msft - I was trying to write an Integration test for this helper class, but I couldn't actually find a resource that returns a 401. Typical MS resource like the graph just return an HTML message, and other resource, like Sharepoint return 403. What resource do you use this against?

@MattB-msft
Copy link
Author

You can use this: https://disco.crm.dynamics.com/api/discovery/
that will return something to the effect of :
www-authenticate →Bearer error=access_denied, error_description=Authentication fail. Ticket id c4f10488-251b-4996-97ce-6407f994f022, authorization_uri=https://login.microsoftonline.com/common/oauth2/authorize, resource_id=https://disco.crm.dynamics.com/

note, you will still need to parse off the oauth2/authorize for 4.x+ ADAL clients.

@bgavrilMS bgavrilMS added the Fixed label Jun 6, 2019
@bgavrilMS bgavrilMS added this to the 5.0.6 milestone Jun 6, 2019
@jennyf19
Copy link
Contributor

jennyf19 commented Jun 13, 2019

@MattB-msft fixed in adal 5.1.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants