Azure-Samples / cosmosdb-chatgpt

Sample application that combines Azure Cosmos DB with Azure OpenAI ChatGPT service
MIT License
262 stars 315 forks source link

Assistance with adding data separation (in a multi-user env, users cannot see each others chats) #52

Open SOE-YoungS opened 1 year ago

SOE-YoungS commented 1 year ago

This issue is for a: (mark with an x)

- [ ] bug report -> please search issues before submitting
- [ x ] feature request
- [ ] documentation issue or request
- [ ] regression (a behavior that used to work and stopped in a new release)

Minimal steps to reproduce

  1. Start the application & have 2 users open it in a browser.
  2. User 1 can then see User 2's chats and vice versa.

Expected/desired behavior

User 1 should not be able to see User 2's chats & vice versa.

Mention any other details that might be useful

I've deployed this for a large company (enterprise) as a quick method of demoing Azure OpenAI across business and allowing users to get hands on with it. However, almost immediately, complaints started to come in and haven't stopped (regarding seeing each others conversations).

I'm not familiar with Blazor, but to counter this issue, I've added "easy-auth" and code to retrieve the users AAD Id, which is then stored in the session (and against messages) item in cosmos DB & is retrieved when the page loads via "NavMenu.razor", then gets passed to "ChatPane.razor" & "ChatService.cs" etc... as needed.

When sessions or ChatMessages are loaded, the UserId has also been added into the queries to CosmosDb, to ensure that only the users session data is returned.

However, seemingly randomly, the users view switches to see another persons chats (instead of their own).

I can repro this now, by starting a debug session in VS, then opening two browser sessions (logged in as different AAD users). Post initial page load, both users see their own views, but randomly, Tab1 will have the same conversation displayed as Tab 2, refreshing the page (Tab1), then shows the correct data for Tab1 again (Tab2, remains unchanged).

Rince and repeat...

Code Example: Enterprise-Dev

Can anyone give me a nudge in the right direction please?

markjbrown commented 1 year ago

I think what's missing is Implementing RBAC on your Cosmos DB account. you can get started with that here, https://learn.microsoft.com/en-us/azure/cosmos-db/how-to-setup-rbac.

You'll likely also need another mechanism for server-side cache. What you are doing might work as it needs to be per user.

Hope that helps.

SOE-YoungS commented 1 year ago

@markjbrown thanks for the pointer, I'll take a look into changing the cache first. I'm not sure that RBAC on the CosmosDB will have much impact here as it seems that Blazor is running as a single instance & isn't managing individual sessions as expected.

The changes to add easy auth & including a UserId in CosmosDb items already separate conversations (IE: refreshing the page restores the users chat), the issue seems to happen as the page is rendered and sent to the client, as there's seemingly no tracking of connected clients going on & therefore all connected clients receive the updated view at the same time.

superpoussin22 commented 1 year ago

@SOE-YoungS did you take a look on : https://github.com/microsoft/azurechat

abeussink commented 1 year ago

There are some static variables and static callbacks that will need to be not static to work for splitting up chats by user. That's on top of tracking the user in the database and updating queries.