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

Changes from 1 commit
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
4 changes: 4 additions & 0 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 @@ -174,6 +174,10 @@ async function handler(
if (keyRes.isErr()) {
return apiError(req, res, keyRes.error);
}

// 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 Down
Loading