refactor(backend): Move credentials storage to prisma user #8283
Conversation
Co-authored-by: Reinier van der Leer <[email protected]>
Co-authored-by: Reinier van der Leer <[email protected]>
required to work on mac edge
… away in the future
…als-input.tsx Co-authored-by: Reinier van der Leer <[email protected]>
… is best way to ensure the stringifyication will work
|
I've added changes of my reviews: I've tested the migration and it works just fine. But I didn't test all the credentials. Feel free to let me know, modify, or revert as needed. |
autogpt_platform/backend/migrations/20241007175112_add_oauth_creds_user_trigger/migration.sql
Outdated
Show resolved
Hide resolved
|
@majdyz I can see that the custom migrations have been removed - can you confirm you have tested this against the dev db & running migrations with prisma works? I have tried this earlier and it does not work for me without custom ones - but if you're saying it does, can you show me the steps and then we can move ahead with it as it is |
|
@aarushik93 I have tested this on my local db. How do I test this on dev db ? Would you be able to assist me on this?
What was the issue?
The normal step |
|
Moved the conversation to discord: https://discord.com/channels/1126875755960336515/1298223231081254923 while this doesn't work for me locally, running everything in docker. @majdyz has tested against dev and the trigger is successfully created. So let's merge this PR on that front |
github-actions
bot
added
the
conflicts
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
aarushik93
dismissed
their stale review
The merge-base changed after approval.
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
github-actions
bot
removed
the
conflicts
| @@ -131,7 +132,7 @@ def delete(self, user_id: str, credentials_id: str) -> None: | |||
|
|
|||
| def _acquire_lock(self, user_id: str, credentials_id: str, *args: str) -> RedisLock: | |||
| key = ( | |||
| self.store.supabase.supabase_url, | |||
| self.store.db_manager, | |||
There was a problem hiding this comment.
Unless self.store.db_manager magically stringifies to a string representing the DB connection, this doesn't work. Can be fixed by either of:
- Replace
self.store.db_managerwith a variable uniquely identifying the DB that the credentials are stored in - Omit
self.store.db_managerif we can absolutely assume thatIntegrationCredentialsStorewill never be used with multiple different DBs within the same system
|
|
||
| def locked_user_metadata(self, user_id: str): | ||
| key = (self.supabase.supabase_url, f"user:{user_id}", "metadata") | ||
| key = (self.db_manager, f"user:{user_id}", "metadata") |
There was a problem hiding this comment.
Background
We are currently storing the OAuth credentials on the supabase user raw metadata table, which gets inserted into the token that is sent with every request. That causes problems because for security, python rejects all requests with a header over 4096 length
Changes 🏗️
Testing 🔍
Note
Only for the new autogpt platform, currently in autogpt_platform/