-
Notifications
You must be signed in to change notification settings - Fork 115
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
Open
anna239
wants to merge
4
commits into
main
Choose a base branch
from
anna.zhdan/rfd-auth
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| --- | ||
| title: "AUTHENTICATION" | ||
| --- | ||
|
|
||
| Author(s): [anna239](https://github.com/anna239) | ||
|
|
||
| ## Elevator pitch | ||
|
|
||
| > What are you proposing to change? | ||
|
|
||
| I suggest adding more information about auth methods that agent supports, which will allow clients to draw more appropriate UI. | ||
|
|
||
| ## Status quo | ||
|
|
||
| > How do things work today and what problems does this cause? Why would we change things? | ||
|
|
||
| Agents have different ways of authenticating users: env vars with api keys, running a command like `<agent_name> login`, some just open a browser and use oauth. | ||
| [AuthMethod](https://agentclientprotocol.com/protocol/schema#authmethod) does not really tell the client what should be done to authenticate. This means we can't show user a control for entering key if an aggent support auth through env var. | ||
|
|
||
| ## What we propose to do about it | ||
|
|
||
| > What are you proposing to improve the situation? | ||
|
|
||
| We can add different types of AuthMethods, so that clients can show some UI for them. For example, this auth method | ||
|
|
||
| ```json | ||
| { | ||
| "id": "123", | ||
| "name": "OpenAI api key", | ||
| "description": "Provide your key", | ||
| "type": "envVar", | ||
| "varName": "OPEN_AI_KEY" | ||
| } | ||
| ``` | ||
|
|
||
| would allow client to prompt user to enter a key, then client can provide this key to an agent via `OPEN_AI_KEY` env variable. | ||
|
|
||
| ## Shiny future | ||
|
|
||
| > How will things will play out once this feature exists? | ||
|
|
||
| It will be easier for end-users to start using an agent from inside the IDE as auth process will be more straightforward | ||
|
|
||
| ## Implementation details and plan | ||
|
|
||
| > Tell me more about your implementation. What is your detailed implementation plan? | ||
|
|
||
| I suggest adding following auth types: | ||
|
|
||
| 1. Env variable | ||
|
|
||
| A user can enter a key and a client will pass it to the agent as an env variable | ||
|
|
||
| ```json | ||
| { | ||
| "id": "123", | ||
| "name": "OpenAI api key", | ||
| "description": "Provide your key", | ||
| "type": "envVar", | ||
| "varName": "OPEN_AI_KEY", | ||
| "link": "OPTIONAL link to a page where user can get their key" | ||
| } | ||
| ``` | ||
|
|
||
| 2. Start argument | ||
|
|
||
| A user can enter a key and a client will pass it to the agent as a start parameter | ||
|
|
||
| ```json | ||
| { | ||
| "id": "123", | ||
| "name": "OpenAI api key", | ||
| "description": "Provide your key", | ||
| "type": "startParam", | ||
| "paramName": "OPEN_AI_KEY", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this also be handled if we supported the full elicitation options?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's more like env var, so this one also requires restart |
||
| "link": "OPTIONAL link to a page where user can get their key" | ||
| } | ||
| ``` | ||
|
|
||
| 3. Agent auth | ||
|
|
||
| Same as what there is now -- agent handles the auth itself, this should be default type if no type is provided for backward compatibility | ||
|
|
||
| ```json | ||
| { | ||
| "id": "123", | ||
| "name": "Agent", | ||
| "description": "Authenticate through agent", | ||
| "type": "agent" | ||
| } | ||
| ``` | ||
|
|
||
| 4. Provided key | ||
|
|
||
| User can enter the key and client should pass it to the agent via [AuthenticateRequest](https://agentclientprotocol.com/protocol/schema#authenticaterequest) | ||
|
|
||
| ```json | ||
| { | ||
| "id": "123", | ||
| "name": "OpenAI api key", | ||
| "description": "Provide your key", | ||
| "type": "enterKey", | ||
| "link": "OPTIONAL link to a page where user can get their key" | ||
| } | ||
| ``` | ||
|
|
||
| for this case [AuthenticateRequest](https://agentclientprotocol.com/protocol/schema#authenticaterequest) should also be updated. I suggest adding optional authParams property | ||
|
|
||
| ```json | ||
| { | ||
| "methodId": "123", | ||
| "authParams": { | ||
| "key": "...." | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### AuthErrors | ||
|
|
||
| It might be useful to include a list of AuthMethod ids to the AUTH_REQUIRED JsonRpc error. Why do we need this if they're already shared during `initialize`: | ||
| All supported auth methods are shared during `initialize`. When user starts a session, they've already selected a model, which can narrow down a list of options. | ||
|
|
||
| ## Frequently asked questions | ||
|
|
||
| > What questions have arisen over the course of authoring this document or during subsequent discussions? | ||
|
|
||
| ### What alternative approaches did you consider, and why did you settle on this one? | ||
|
|
||
| Alternative approach would be to include this information to an agent's declaration making it more static, see [Registry RFD](https://github.com/agentclientprotocol/agent-client-protocol/pull/289) | ||
|
|
||
| ## Revision history | ||
|
|
||
| <!-- If there have been major updates to this RFD, you can include the git revisions and a summary of the changes. --> | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Question: is it too late to add the env var since the process has already started?
Could we maybe just model this as a request for a text field that the user pastes the token into? and then the agent would store it somewhere like it usually does?
Or maybe you had an idea here that I am missing?
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.
In this case we'll have to restart the process indeed. If the agent is ok with just accepting a key in JsonRpc call, then it should declare 3. option —
Provided keyThere 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.
Do you think this method should rather be included in #289 ? i.e. static information known before startup?
But I guess we want to allow the user to choose? So by choosing this we'd restart the agent for them but at least they could choose if that is what they want from the options?
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.
yes, that was my idea