-
Notifications
You must be signed in to change notification settings - Fork 129
Add Authentication RFD #330
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
Conversation
benbrandt
left a comment
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.
Another option might be:
- provide two new capabilities on the client
- Request text
- Request to open link
- The agent returns auth methods as they do now
- User picks one of the options
- Within the authenticate method, the agent asks the client for a key and optionally a link to open to get said key or device code
The reason I bring this up is usually for the oauth flows there are specific links and maybe the need to spin up a local http server (at least for codex this is the case) to handle the redirect post-auth. So I don't know that the agent can upfront provide the urls without entering in to a specific auth flow.
Maybe what i am proposing is too generic and opens pandora's box though....
|
@benbrandt why I think approach with agent requesting a text input won't work as good as we want it to : |
|
Ack ACP and MCP are different specs (editor-agent vs. tool integration), but MCP's elicitation (URL mode) provides prior art for out-of-band credential handling that avoids LLM/agent exposure. It pauses for browser input without touching the protocol flow. Is this relevant to the design discussion for how to handle sensitive info exchange in ACP? |
|
Yeah I think MCP elicitation is not a bad way to approach it for two reasons:
I have some quibbles with the api design... but I don't know that it is worth changing for reason 1 above. The interesting thing will be that this elicitation would come outside the context of a session. Which I believe you brought up yesterday @anna239 in our call: we may want a very explicit stage for this, because if we allow for elicitation, we'd need to know if it should show up within a session feed or somewhere else. |
|
I agree that URL-elicitation mechanism would work well for the oauth scenario, let me update rfd with this approach |
|
Added part about MCP-like elicitation mechanism for oauth, @codefromthecrypt thank you so much for pointing this out to me. |
|
I began studying this today (including which editors handle things similarly and might be impacted). I didnt finish that research but dont block on me. I will comment before or after the fact once I parse things well enough to be relevant! |
codefromthecrypt
left a comment
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.
looks like real progress
I think these folks who are building auth handling can also validate, and maybe not mentioned?
- Zed: @rtfeldman (_meta.terminal-auth), @cole-miller (SSH/Windows login)
- codecompanion.nvim: @olimorris
- AionUi: @kuishou68
Meanwhile I think I read @anna239 suggested to split this
- Auth Types RFD - Just the 4 types + requiresRestart + authParams
- Elicitation RFD - General-purpose mechanism, not auth-specific
Happy to see things moving
|
Hey! What do you think about |
|
Hey! I published a very early version of the ACP registry: https://github.com/agentclientprotocol/registry For now, I have listed only agents that have some authentication methods. At the moment, this is mostly opening a browser and starting an HTTP server from the agent process, or launching a terminal via I think that supporting a proper authentication flow is crucial for smooth distribution in editors and IDEs. |
|
@codefromthecrypt @benbrandt hi folks, what do you think about merging this and starting the implementation? Or are there any more concerns we have to resolve before that? |
benbrandt
left a comment
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.
Hmm reading the current state and thinking about this some more, I think we need to think through how we handle the workflows that cannot handle reauthenticating while running (needing a restart)
I think we should move this to a synchronous call to talk through, maybe I am just missing something here, but I feel like we might be introducing too many variations here, or at least they could potentially be providing in a simpler way
nikomatsakis
left a comment
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.
I don't really have a strong opinion on this one, just found a typo. I think that having something convenient for common cases makes sense, along with some "catch-all" option like Terminal Auth.
Co-authored-by: Niko Matsakis <[email protected]>
1b8c326 to
71a61b5
Compare
|
Hmm I suspect a mintlify race condition... Hopefully solved on the next merge |
Define a way for an agent to declare different ways to authenticate, this will allow clients to present better UX to users.