Pass groupids to core #6628
Pass groupids to core #6628
Conversation
| secrets, | ||
| }); | ||
| const runRes = await coreAPI.createRunStream( | ||
| auth.getNonNullableWorkspace(), |
There was a problem hiding this comment.
Don't we already have owner from above ?
There was a problem hiding this comment.
Ah yeah good catch. Let me fix.
| secrets, | ||
| }); | ||
| const dustRun = await coreAPI.createRun( | ||
| auth.getNonNullableWorkspace(), |
There was a problem hiding this comment.
same here, we already have owner ?
There was a problem hiding this comment.
BTW here at the top we shouldn't use getNonNullableWorkspace because it throws while we want to 404 if we don't have a ws
front/lib/auth.ts
Outdated
| ): Promise<{ auth: Authenticator; keyWorkspaceId: string }> { | ||
| ): Promise<{ | ||
| auth: Authenticator; | ||
| keyWorkspaceId: string; |
There was a problem hiding this comment.
Do we still need keyWorkspaceId? Can't it be inferred from keyWorkspace
| // Confusingly, the auth workspace here is the the one from the URL, not the one from the key. | ||
| // Where as auth.groups are the groups associated with the the key. |
There was a problem hiding this comment.
💯 to change the behaviour and keep in auth the workspace of the user key - keeping a different workspace here is very confusing. The workspace from the url is only there to know in which workspace is the app , but it has nothing to with authentication.
There was a problem hiding this comment.
We will always have two workspaces here as we can run an app from workspace A with a key from workspace B. This is tricky indeed and need thorough consideration in the context of groups 👍
There was a problem hiding this comment.
We do need an auth on the app workspace as this is the one we use to retrieve the app. Let's call this the appAuth.
That being said it would be nice to have an auth on the key independent of the app workspace. This one could be called keyAuth.
That way we would not need to fetch the groups and could pass the keyAuth directly to create run?
| @@ -285,9 +290,12 @@ async function handler( | |||
| "App run creation" | |||
| ); | |||
|
|
|||
| const runRes = await coreAPI.createRunStream({ | |||
| const groups = await GroupResource.listWorkspaceGroupsFromKey( | |||
There was a problem hiding this comment.
As discussed on slack this should be the keyAuth groups which, once we implement X-Dust-User-Id, will correctly represent the calling user's groups
* Pass group ids to core when creating runs * Add BaseAuthenticator type * 🙈 * ✂️ * ✨ * Address comments from review * Use key workspace and groups * 👕 * Address comments from review * ✨ * 📖
Description
This PR passes the group ids to core when creating a run, so we can validate permission when querying a data source.
Risk
Deploy Plan