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

FEAT Add User AuthN Support to AOAI Chat Targets #182

Merged

Conversation

nina-msft
Copy link
Contributor

@nina-msft nina-msft commented Apr 30, 2024

Description

Adds use_aad_auth parameter to all AOAIChatTarget implementations.

FYI: @rdheekonda @rlundeen2

Tests and Documentation

  • Unit Test added for DefaultAzureCredential happy path
  • Docs: Added example of use_aad_auth flag to prompt_targets.py and ran Jupytext on that notebook

@nina-msft nina-msft changed the title FEAT Add User AuthN Support to AOAI Chat Targets [DRAFT] FEAT Add User AuthN Support to AOAI Chat Targets Apr 30, 2024
Copy link
Contributor

@rlundeen2 rlundeen2 left a comment

Choose a reason for hiding this comment

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

This looks great! Before merging please

  • Test in an actual endpoint. We don't necessarily need this permanently added to jupyter notebooks although that's not a bad idea.
  • Unit tests to check the paths are set correctly

Feel free to hit me up if you want me to take another look :)

@nina-msft nina-msft changed the title [DRAFT] FEAT Add User AuthN Support to AOAI Chat Targets FEAT Add User AuthN Support to AOAI Chat Targets Apr 30, 2024
@nina-msft nina-msft marked this pull request as ready for review April 30, 2024 19:07
@nina-msft
Copy link
Contributor Author

This looks great! Before merging please

  • Test in an actual endpoint. We don't necessarily need this permanently added to jupyter notebooks although that's not a bad idea.
  • Unit tests to check the paths are set correctly

Feel free to hit me up if you want me to take another look :)

@rlundeen I tested with AOAIChatTarget using the endpoint that I have user auth permissions with and verified it works :-) Updated the documentation to highlight this flag as an option, but set it to False by default so that API KEY is used from .env

I added a unit test for the Azure Auth functionality, I'm not sure what you mean by checking paths. Let me know if there's other testing needed!

pyrit/auth/azure_auth.py Outdated Show resolved Hide resolved
pyrit/auth/azure_auth.py Outdated Show resolved Hide resolved
pyrit/auth/azure_auth.py Outdated Show resolved Hide resolved
@nina-msft nina-msft merged commit b103c4a into Azure:main Apr 30, 2024
5 checks passed
@nina-msft nina-msft deleted the users/nina-msft/add-user-auth-gptv-target branch April 30, 2024 21:33
QDAP-Fred pushed a commit to QDAP-Fred/QUART that referenced this pull request May 13, 2024
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.

None yet

4 participants