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

Security issue when create agent is called from different user simultaneously #1825

Closed
Sapessii opened this issue Oct 3, 2024 · 7 comments · Fixed by #1834
Closed

Security issue when create agent is called from different user simultaneously #1825

Sapessii opened this issue Oct 3, 2024 · 7 comments · Fixed by #1834

Comments

@Sapessii
Copy link

Sapessii commented Oct 3, 2024

Describe the bug

Hi, when the create agent endpoint is called simultaneously from different users, the user_id is always stick with the first user, hence resulting in multiple users that can access to the chat of others.

I think it's related with the middleware setting the set_current_user function is called.

@cpacker
Copy link
Collaborator

cpacker commented Oct 6, 2024

Thanks for bringing this up @Sapessii ! Just opened a fix in PR #1834 - once it passes test will get it merged in ASAP.

@Sapessii
Copy link
Author

Sapessii commented Oct 6, 2024

Awesome thank you so much!

@github-project-automation github-project-automation bot moved this from To triage to Done in 🐛 MemGPT issue tracker Oct 7, 2024
@Sapessii
Copy link
Author

Sapessii commented Oct 8, 2024

Hi! @cpacker, I just tried the fix but is not getting the header correctly, it's working when using

user_id: str = Header(..., alias="user_id")

@cpacker
Copy link
Collaborator

cpacker commented Oct 8, 2024

@Sapessii my understanding of FastAPI code is that alias shouldn't be required in this case since user_id is the variable name + header key name. Similarly, if we go from None to ..., I believe that means that user_id now becomes required in the header, vs being optional.

Testing now to see if I can repro (thanks for the report!)

@Sapessii
Copy link
Author

Sapessii commented Oct 8, 2024

@cpacker sure no problem :) thanks to you!

@cpacker
Copy link
Collaborator

cpacker commented Oct 8, 2024

@Sapessii can you check my comments in #1843?

I think this seems to be working (at least in that PR). Will merge it in shortly.

@Sapessii
Copy link
Author

Sapessii commented Oct 8, 2024

@cpacker working perfectly! thank you so much

@Sapessii Sapessii closed this as completed Oct 8, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in 🐛 MemGPT issue tracker Oct 8, 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 a pull request may close this issue.

2 participants