semantic-kernel icon indicating copy to clipboard operation
semantic-kernel copied to clipboard

Add AAD and API Key auth

Open glahaye opened this issue 2 years ago • 3 comments

Motivation and Context

It is desired to control access to the chat backend / SK service.

Description

  • Added AAD auth by using Microsoft libraries
  • Added API key auth by creating a custom authenticator
  • Added a passthrough "authenticator" to disable auth

Contribution Checklist

  • [ ] The code builds clean without any errors or warnings
  • [ ] The PR follows SK Contribution Guidelines (https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
  • [ ] The code follows the .NET coding conventions (https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions) verified with dotnet format
  • [ ] All unit tests pass, and I have added new tests where possible
  • [ ] I didn't break anyone :smile:

glahaye avatar Apr 19 '23 04:04 glahaye

How do we stop users from sending credentials over HTTP (not HTTPS) given that HTTPS is disabled in the sample?

I'm concerned about the bad guidance we're giving, considering that many devs will copy and run the app assuming we did the right things. We shouldn't expect that everyone will read READMEs or comments and hope for the best.

This is an excellent question that we are currently evaluating outside the of the scope of this PR.

adrianwyatt avatar Apr 19 '23 17:04 adrianwyatt

How do we stop users from sending credentials over HTTP (not HTTPS) given that HTTPS is disabled in the sample? I'm concerned about the bad guidance we're giving, considering that many devs will copy and run the app assuming we did the right things. We shouldn't expect that everyone will read READMEs or comments and hope for the best.

This is an excellent question that we are currently evaluating outside the of the scope of this PR.

Well, now the code needs to be explicitly edited and compiled to allow that to happen!

glahaye avatar Apr 19 '23 19:04 glahaye

How do we stop users from sending credentials over HTTP (not HTTPS) given that HTTPS is disabled in the sample?

I'm concerned about the bad guidance we're giving, considering that many devs will copy and run the app assuming we did the right things. We shouldn't expect that everyone will read READMEs or comments and hope for the best.

Also, a change is coming in in parallel to this one to make HTTPS the default again.

glahaye avatar Apr 19 '23 22:04 glahaye