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

Verify no ADAL dependencies #6651

Closed
tracyboehrer opened this issue Jun 22, 2023 · 7 comments · Fixed by #6690
Closed

Verify no ADAL dependencies #6651

tracyboehrer opened this issue Jun 22, 2023 · 7 comments · Fixed by #6690
Assignees

Comments

@tracyboehrer
Copy link
Member

Verify if ADAL is in use via some out-of-date dependency outside SDK. That is, we recently switched to ADAL in SDK auth code. But it's possible that some other SDK dependency is bringing it in and needs to be updated.

@ceciliaavila
Copy link
Collaborator

Hi @tracyboehrer,
The only reference left to ADAL is in Microsoft.Bot.Connector. It's being used by Microsoft.Azure.Services.AppAuthentication package.
We marked all the methods and classes as deprecated in PR#6605 but didn't remove them to avoid breaking changes.
Unfortunately, Microsoft.Azure.Services.AppAuthentication package's latest version is still using this dependency, so updating it won't solve the problem.
Should we assume the breaking changes and completely remove ADAL?

image

@ceciliaavila
Copy link
Collaborator

Adding more information to keep it on the radar.
The Bot.Connector classes using references to ADAL are the following:

  • AdalAuthenticator (Used in the other classes obsolete methods)
  • CertificateAppCredentials (Obsolete methods)
  • MicrosoftAppCredentials (Obsolete methods)
  • JwtTokenProviderFactory (Obsolete class)
  • ConstantHttpClientFactory (Internal class)

We can't remove them because it will represent a breaking change.

@wshxj123
Copy link

wshxj123 commented Sep 5, 2023

Hi @tracyboehrer ,
Do you have any suggestions about this?
I got a Component Governance Alert for the dependencies for Microsoft.IdentityModel.Clients.ActiveDirectory.
I need to fix it ASAP.

Thanks a lot.

@mit2nil
Copy link
Member

mit2nil commented Sep 7, 2023

Hi @tracyboehrer and @ceciliaavila ,

Are there any updates on this? We are also seeing component governance bugs for ADAL dependency which is going to start breaking out production builds soon if not fixed and we can not fix this ourselves as its transient dependency coming just from Microsoft.Bot.Builder 4.20.1 which is the latest version of bot builder package.

Nilay

@tracyboehrer
Copy link
Member Author

@wshxj123 & @mit2nil What is the root dependency of the alerts?

@wshxj123
Copy link

wshxj123 commented Sep 7, 2023

@wshxj123 & @mit2nil What is the root dependency of the alerts?

I pinged you on Teams just now.
The root dependency is Microsoft.IdentityModel.Clients.ActiveDirectory.
I will try to upgrade the SDKs when the new release is ready.

@Danieladu
Copy link
Contributor

Danieladu commented Sep 8, 2023

Code merged with #6687

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 a pull request may close this issue.

5 participants