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.

Sign up for GitHub

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

Pass groupids to core #6628

Merged
merged 11 commits into from
Aug 2, 2024
Merged

Pass groupids to core #6628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
09c38a6
Pass group ids to core when creating runs
flvndvd Aug 1, 2024
2e23618
Add BaseAuthenticator type
flvndvd Aug 1, 2024
5b42e29
:see_no_evil:
flvndvd Aug 1, 2024
f705369
:scissors:
flvndvd Aug 1, 2024
f674fff
:sparkles:
flvndvd Aug 1, 2024
cad9040
Address comments from review
flvndvd Aug 1, 2024
cf1147f
Use key workspace and groups
flvndvd Aug 1, 2024
38d60df
:shirt:
flvndvd Aug 2, 2024
c0f10e7
Address comments from review
flvndvd Aug 2, 2024
3c8d833
:sparkles:
flvndvd Aug 2, 2024
4195df3
:book:
flvndvd Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions front/lib/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,10 @@ export class Authenticator {
static async fromKey(
key: KeyResource,
wId: string
): Promise<{ auth: Authenticator; keyWorkspaceId: string }> {
): Promise<{
auth: Authenticator;
keyWorkspace: LightWorkspaceType;
}> {
const [workspace, keyWorkspace] = await Promise.all([
(async () => {
return Workspace.findOne({
Expand Down Expand Up @@ -346,7 +349,7 @@ export class Authenticator {
flags,
key: key.toAuthJSON(),
}),
keyWorkspaceId: keyWorkspace.sId,
keyWorkspace: renderLightWorkspaceType({ workspace: keyWorkspace }),
};
}

Expand Down
18 changes: 13 additions & 5 deletions front/pages/api/v1/w/[wId]/apps/[aId]/runs/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import apiConfig from "@app/lib/api/config";
import { getDustAppSecrets } from "@app/lib/api/dust_app_secrets";
import { Authenticator, getAPIKey } from "@app/lib/auth";
import { Provider } from "@app/lib/models/apps";
import { GroupResource } from "@app/lib/resources/group_resource";
import type { RunUsageType } from "@app/lib/resources/run_resource";
import { RunResource } from "@app/lib/resources/run_resource";
import logger from "@app/logger/logger";
Expand Down Expand Up @@ -173,7 +174,11 @@ async function handler(
if (keyRes.isErr()) {
return apiError(req, res, keyRes.error);
}
const { auth, keyWorkspaceId } = await Authenticator.fromKey(

// TODO(2024-08-02 flav) Refactor auth.fromKey logic.
// 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.
Comment on lines +179 to +180
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

const { auth, keyWorkspace } = await Authenticator.fromKey(
keyRes.value,
req.query.wId as string
);
Expand All @@ -183,8 +188,8 @@ async function handler(
return apiError(req, res, {
status_code: 404,
api_error: {
type: "app_not_found",
message: "The app you're trying to run was not found",
type: "workspace_not_found",
message: "The workspace was not found.",
},
});
}
Expand Down Expand Up @@ -285,9 +290,12 @@ async function handler(
"App run creation"
);

const runRes = await coreAPI.createRunStream({
const groups = await GroupResource.listWorkspaceGroupsFromKey(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

keyRes.value
);

const runRes = await coreAPI.createRunStream(keyWorkspace, groups, {
projectId: app.dustAPIProjectId,
runAsWorkspaceId: keyWorkspaceId,
runType: "deploy",
specificationHash: specificationHash,
config: { blocks: config },
Expand Down
4 changes: 2 additions & 2 deletions front/pages/api/v1/w/[wId]/assistant/agent_configurations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ async function handler(
return apiError(req, res, keyRes.error);
}

const { auth, keyWorkspaceId } = await Authenticator.fromKey(
const { auth, keyWorkspace } = await Authenticator.fromKey(
keyRes.value,
req.query.wId as string
);

if (!auth.isBuilder() || keyWorkspaceId !== req.query.wId) {
if (!auth.isBuilder() || keyWorkspace.sId !== req.query.wId) {
return apiError(req, res, {
status_code: 400,
api_error: {
Expand Down
4 changes: 2 additions & 2 deletions front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/content_fragments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ async function handler(
return apiError(req, res, keyRes.error);
}

const { auth, keyWorkspaceId } = await Authenticator.fromKey(
const { auth, keyWorkspace } = await Authenticator.fromKey(
keyRes.value,
req.query.wId as string
);

if (!auth.isBuilder() || keyWorkspaceId !== req.query.wId) {
if (!auth.isBuilder() || keyWorkspace.sId !== req.query.wId) {
return apiError(req, res, {
status_code: 400,
api_error: {
Expand Down
4 changes: 2 additions & 2 deletions front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ async function handler(
return apiError(req, res, keyRes.error);
}

const { auth, keyWorkspaceId } = await Authenticator.fromKey(
const { auth, keyWorkspace } = await Authenticator.fromKey(
keyRes.value,
req.query.wId as string
);

if (!auth.isBuilder() || keyWorkspaceId !== req.query.wId) {
if (!auth.isBuilder() || keyWorkspace.sId !== req.query.wId) {
return apiError(req, res, {
status_code: 400,
api_error: {
Expand Down
4 changes: 2 additions & 2 deletions front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ async function handler(
return apiError(req, res, keyRes.error);
}

const { auth, keyWorkspaceId } = await Authenticator.fromKey(
const { auth, keyWorkspace } = await Authenticator.fromKey(
keyRes.value,
req.query.wId as string
);

if (!auth.isBuilder() || keyWorkspaceId !== req.query.wId) {
if (!auth.isBuilder() || keyWorkspace.sId !== req.query.wId) {
return apiError(req, res, {
status_code: 400,
api_error: {
Expand Down
4 changes: 2 additions & 2 deletions front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/messages/[mId]/events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@ async function handler(
return apiError(req, res, keyRes.error);
}

const { auth, keyWorkspaceId } = await Authenticator.fromKey(
const { auth, keyWorkspace } = await Authenticator.fromKey(
keyRes.value,
req.query.wId as string
);

if (!auth.isBuilder() || keyWorkspaceId !== req.query.wId) {
if (!auth.isBuilder() || keyWorkspace.sId !== req.query.wId) {
return apiError(req, res, {
status_code: 400,
api_error: {
Expand Down
4 changes: 2 additions & 2 deletions front/pages/api/v1/w/[wId]/assistant/conversations/[cId]/messages/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ async function handler(
req.query.wId as string
);
let { auth } = authenticator;
const { keyWorkspaceId } = authenticator;
const { keyWorkspace } = authenticator;

if (!auth.isBuilder() || keyWorkspaceId !== req.query.wId) {
if (!auth.isBuilder() || keyWorkspace.sId !== req.query.wId) {
return apiError(req, res, {
status_code: 400,
api_error: {
Expand Down
4 changes: 2 additions & 2 deletions front/pages/api/v1/w/[wId]/assistant/conversations/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ async function handler(
req.query.wId as string
);
let { auth } = authenticator;
const { keyWorkspaceId } = authenticator;
const { keyWorkspace } = authenticator;

if (!auth.isBuilder() || keyWorkspaceId !== req.query.wId) {
if (!auth.isBuilder() || keyWorkspace.sId !== req.query.wId) {
return apiError(req, res, {
status_code: 400,
api_error: {
Expand Down
14 changes: 11 additions & 3 deletions front/pages/api/w/[wId]/apps/[aId]/runs/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,16 @@ async function handler(
auth: Authenticator,
session: SessionWithUser
) {
let owner = auth.getNonNullableWorkspace();
let owner = auth.workspace();
if (!owner) {
return apiError(req, res, {
status_code: 404,
api_error: {
type: "workspace_not_found",
message: "The workspace was not found.",
},
});
}

const app = await getApp(auth, req.query.aId as string);

Expand Down Expand Up @@ -111,9 +120,8 @@ async function handler(
);
const inputDataset = inputConfigEntry ? inputConfigEntry.dataset : null;

const dustRun = await coreAPI.createRun({
const dustRun = await coreAPI.createRun(owner, auth.groups(), {
projectId: app.dustAPIProjectId,
runAsWorkspaceId: owner.sId,
runType: "local",
specification: dumpSpecification(
JSON.parse(req.body.specification),
Expand Down
69 changes: 36 additions & 33 deletions types/src/front/lib/core_api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
} from "../../front/run";
import { LoggerInterface } from "../../shared/logger";
import { Err, Ok, Result } from "../../shared/result";
import { GroupType } from "../groups";
import { LightWorkspaceType } from "../user";

export const MAX_CHUNK_SIZE = 512;

Expand Down Expand Up @@ -89,7 +91,6 @@ export type CoreAPITokenType = [number, string];

type CoreAPICreateRunParams = {
projectId: string;
runAsWorkspaceId: string;
runType: RunRunType;
specification?: string | null;
specificationHash?: string | null;
Expand Down Expand Up @@ -280,28 +281,29 @@ export class CoreAPI {
return this._resultFromResponse(response);
}

async createRun({
projectId,
runAsWorkspaceId,
runType,
specification,
specificationHash,
datasetId,
inputs,
config,
credentials,
secrets,
}: CoreAPICreateRunParams): Promise<CoreAPIResponse<{ run: CoreAPIRun }>> {
// TODO(GROUPS_INFRA): use the auth as argument of that method instead of `runAsWorkspaceId`
// and pass both X-Dust-Workspace-Id and X-Dust-Group-Ids.

async createRun(
workspace: LightWorkspaceType,
groups: GroupType[],
{
projectId,
runType,
specification,
specificationHash,
datasetId,
inputs,
config,
credentials,
secrets,
}: CoreAPICreateRunParams
): Promise<CoreAPIResponse<{ run: CoreAPIRun }>> {
const response = await this._fetchWithError(
`${this._url}/projects/${encodeURIComponent(projectId)}/runs`,
{
method: "POST",
headers: {
"Content-Type": "application/json",
"X-Dust-Workspace-Id": runAsWorkspaceId,
"X-Dust-Workspace-Id": workspace.sId,
"X-Dust-Group-Ids": groups.map((g) => g.sId).join(","),
},
body: JSON.stringify({
run_type: runType,
Expand All @@ -319,33 +321,34 @@ export class CoreAPI {
return this._resultFromResponse(response);
}

async createRunStream({
projectId,
runAsWorkspaceId,
runType,
specification,
specificationHash,
datasetId,
inputs,
config,
credentials,
secrets,
}: CoreAPICreateRunParams): Promise<
async createRunStream(
workspace: LightWorkspaceType,
groups: GroupType[],
{
projectId,
runType,
specification,
specificationHash,
datasetId,
inputs,
config,
credentials,
secrets,
}: CoreAPICreateRunParams
): Promise<
CoreAPIResponse<{
chunkStream: AsyncGenerator<Uint8Array, void, unknown>;
dustRunId: Promise<string>;
}>
> {
// TODO(GROUPS_INFRA): use the auth as argument of that method instead of `runAsWorkspaceId`
// and pass both X-Dust-Workspace-Id and X-Dust-Group-Ids.

const res = await this._fetchWithError(
`${this._url}/projects/${projectId}/runs/stream`,
{
method: "POST",
headers: {
"Content-Type": "application/json",
"X-Dust-Workspace-Id": runAsWorkspaceId,
"X-Dust-Workspace-Id": workspace.sId,
"X-Dust-Group-Ids": groups.map((g) => g.sId).join(","),
},
body: JSON.stringify({
run_type: runType,
Expand Down
Loading