-
Notifications
You must be signed in to change notification settings - Fork 63
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
Update azure-openai to check OIDC TokenCredential #634
base: main
Are you sure you want to change the base?
Update azure-openai to check OIDC TokenCredential #634
Conversation
4234763
to
4832f63
Compare
Thanks! I'll have a look next week, but I want @csotiriou to also have a look at this |
Thanks @geoand, @csotiriou. Agreed, #646 is great. My hope with this PR is that it just works for Quarkus OIDC or SmallRye JWT users, they only configure Azure OIDC provider, and Azure OpenAI provider and is done, though asking users register dynamic filters to have OIDC tokens propagated can be quite reasonable too, though it will be a bit of an extra effort as they get it for free with the general OIDC token propagation which is really what this PR emulates. But in any case, for this PR, I'm yet to confirm I can login to Azure and use my authorization code flow token to access Azure OpenAI, I did a few tries yesterday, but to no avail. @csotiriou If you know what scope I should set here to make it work then let me know please :-) |
Thank you! I'm thinking that OIDC support out of the box may be able to be provided via an additional small module which uses the mechanism of #646 as a base to set up everything automatically with OIDC, thus providing the best of both worlds. This is just a thought, and I don't know if that aligns with your vision, however.
Unfortunately, I'm not sure, since I'm using SSO to login and retrieve a token, and the token is vastly different. Is there any error you are getting by using this PR, or something similar? Also, may I ask about your setup? Can I set up a similar environment using a personal account or something similar for testing purposes? |
Hi @csotiriou Sure, we can finalize the setup for OIDC flows once your PR goes in. (Note the same though would have to work for any provider like vertex-gemini and other similar providers passing tokens, I have a PR opened a bit earlier which works - which is why, should we depend for OIDC on your new interface, it should really be a global one...) I'm getting infamous quarkus-langchain4j setup is in this PR, if you look at the I submitted a form to request adding Azure AI service to my Azure tenant, and when it was approved I created an Azure OpenAI account, created a new deployment and there I created a The demo flow is that I login to the Quarkus endpoint with Azure and call a poem resource from the HTML page and the token now flows to Azure, but instead of a poem, the above error is returned. If it can be of interest then indeed, it would be great if you could try this PR with your tenant if you have Azure AI service enabled there. Please take your time in any case, I'm off to my PTO from early morning tomorrow and be back on 11th June, so I won't be able to reply until then. In any case, thanks for your thoughts, it is appreciated, thanks |
Also CC Eric, @edeandrea |
I think this makes sense, but I also think that it should be built on top of #646 |
if (tokenCredential.isResolvable() && tokenCredential.get() != null) { | ||
context.getHeaders().add("Authorization", "Bearer " + tokenCredential.get().getToken()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this take useSecurityIdentityToken
into account?
@@ -0,0 +1,115 @@ | |||
# Secure Vertex AI Gemini Poem Demo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong title :)
This PR is similar to #626.
If #626 is approved then, as far as this PR is concerned, once the azure-openai and openai-common changes are reviewed, I'd trim it by avoiding adding yet another demo identical to the one added in #626 and instead, try to merge both demos into a single one, showing multiple quarkus-langchain4j models in action and complementing it with annotation based OIDC tenant selection (either Google or Microsoft Entra ID)