-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feature: machine identity groups [ENG-4237] #4995
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
347aa57
22cea00
6a8316d
5d2a34c
f9b7791
5fb0b59
eb1e890
1c25327
5e0f1d8
76d7f26
2b91afd
eb6c976
37546a7
24efd8b
d41ece8
771c981
d1b3b69
7f1636f
0aff45d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { Knex } from "knex"; | ||
|
|
||
| import { TableName } from "../schemas"; | ||
| import { createOnUpdateTrigger, dropOnUpdateTrigger } from "../utils"; | ||
|
|
||
| export async function up(knex: Knex): Promise<void> { | ||
| if (!(await knex.schema.hasTable(TableName.IdentityGroupMembership))) { | ||
| await knex.schema.createTable(TableName.IdentityGroupMembership, (t) => { | ||
| t.uuid("id", { primaryKey: true }).defaultTo(knex.fn.uuid()); | ||
| t.uuid("identityId").notNullable(); | ||
| t.foreign("identityId").references("id").inTable(TableName.Identity).onDelete("CASCADE"); | ||
| t.uuid("groupId").notNullable(); | ||
| t.foreign("groupId").references("id").inTable(TableName.Groups).onDelete("CASCADE"); | ||
| t.timestamps(true, true, true); | ||
|
|
||
| t.unique(["identityId", "groupId"]); | ||
| }); | ||
| } | ||
|
|
||
| await createOnUpdateTrigger(knex, TableName.IdentityGroupMembership); | ||
| } | ||
|
|
||
| export async function down(knex: Knex): Promise<void> { | ||
| if (await knex.schema.hasTable(TableName.IdentityGroupMembership)) { | ||
| await knex.schema.dropTable(TableName.IdentityGroupMembership); | ||
| await dropOnUpdateTrigger(knex, TableName.IdentityGroupMembership); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| // Code generated by automation script, DO NOT EDIT. | ||
| // Automated by pulling database and generating zod schema | ||
| // To update. Just run npm run generate:schema | ||
| // Written by akhilmhdh. | ||
|
|
||
| import { z } from "zod"; | ||
|
|
||
| import { TImmutableDBKeys } from "./models"; | ||
|
|
||
| export const IdentityGroupMembershipSchema = z.object({ | ||
| id: z.string().uuid(), | ||
| identityId: z.string().uuid(), | ||
| groupId: z.string().uuid(), | ||
| createdAt: z.date(), | ||
| updatedAt: z.date() | ||
| }); | ||
|
|
||
| export type TIdentityGroupMembership = z.infer<typeof IdentityGroupMembershipSchema>; | ||
| export type TIdentityGroupMembershipInsert = Omit<z.input<typeof IdentityGroupMembershipSchema>, TImmutableDBKeys>; | ||
| export type TIdentityGroupMembershipUpdate = Partial< | ||
| Omit<z.input<typeof IdentityGroupMembershipSchema>, TImmutableDBKeys> | ||
| >; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,13 @@ | ||
| import { z } from "zod"; | ||
|
|
||
| import { GroupsSchema, OrgMembershipRole, ProjectsSchema, UsersSchema } from "@app/db/schemas"; | ||
| import { GroupsSchema, IdentitiesSchema, OrgMembershipRole, ProjectsSchema, UsersSchema } from "@app/db/schemas"; | ||
| import { | ||
| EFilterReturnedProjects, | ||
| EFilterReturnedUsers, | ||
| EGroupProjectsOrderBy | ||
| FilterMemberType, | ||
| FilterReturnedIdentities, | ||
| FilterReturnedProjects, | ||
| FilterReturnedUsers, | ||
| GroupMembersOrderBy, | ||
| GroupProjectsOrderBy | ||
| } from "@app/ee/services/group/group-types"; | ||
| import { ApiDocsTags, GROUPS } from "@app/lib/api-docs"; | ||
| import { OrderByDirection } from "@app/lib/types"; | ||
|
|
@@ -13,6 +16,11 @@ import { slugSchema } from "@app/server/lib/schemas"; | |
| import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; | ||
| import { AuthMode } from "@app/services/auth/auth-type"; | ||
|
|
||
| const GroupIdentityResponseSchema = IdentitiesSchema.pick({ | ||
| id: true, | ||
| name: true | ||
| }); | ||
|
|
||
| export const registerGroupRouter = async (server: FastifyZodProvider) => { | ||
| server.route({ | ||
| url: "/", | ||
|
|
@@ -191,7 +199,7 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { | |
| limit: z.coerce.number().min(1).max(100).default(10).describe(GROUPS.LIST_USERS.limit), | ||
| username: z.string().trim().optional().describe(GROUPS.LIST_USERS.username), | ||
| search: z.string().trim().optional().describe(GROUPS.LIST_USERS.search), | ||
| filter: z.nativeEnum(EFilterReturnedUsers).optional().describe(GROUPS.LIST_USERS.filterUsers) | ||
| filter: z.nativeEnum(FilterReturnedUsers).optional().describe(GROUPS.LIST_USERS.filterUsers) | ||
| }), | ||
| response: { | ||
| 200: z.object({ | ||
|
|
@@ -202,12 +210,10 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { | |
| lastName: true, | ||
| id: true | ||
| }) | ||
| .merge( | ||
| z.object({ | ||
| isPartOfGroup: z.boolean(), | ||
| joinedGroupAt: z.date().nullable() | ||
| }) | ||
| ) | ||
| .extend({ | ||
| isPartOfGroup: z.boolean(), | ||
| joinedGroupAt: z.date().nullable() | ||
| }) | ||
| .array(), | ||
| totalCount: z.number() | ||
| }) | ||
|
|
@@ -227,6 +233,119 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { | |
| } | ||
| }); | ||
|
|
||
| server.route({ | ||
| method: "GET", | ||
| url: "/:id/identities", | ||
| config: { | ||
| rateLimit: readLimit | ||
| }, | ||
| onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), | ||
| schema: { | ||
| hide: false, | ||
| tags: [ApiDocsTags.Groups], | ||
| params: z.object({ | ||
| id: z.string().trim().describe(GROUPS.LIST_IDENTITIES.id) | ||
| }), | ||
| querystring: z.object({ | ||
| offset: z.coerce.number().min(0).default(0).describe(GROUPS.LIST_IDENTITIES.offset), | ||
| limit: z.coerce.number().min(1).max(100).default(10).describe(GROUPS.LIST_IDENTITIES.limit), | ||
| search: z.string().trim().optional().describe(GROUPS.LIST_IDENTITIES.search), | ||
| filter: z.nativeEnum(FilterReturnedIdentities).optional().describe(GROUPS.LIST_IDENTITIES.filterIdentities) | ||
| }), | ||
| response: { | ||
| 200: z.object({ | ||
| identities: GroupIdentityResponseSchema.extend({ | ||
| isPartOfGroup: z.boolean(), | ||
| joinedGroupAt: z.date().nullable() | ||
| }).array(), | ||
| totalCount: z.number() | ||
| }) | ||
| } | ||
| }, | ||
| handler: async (req) => { | ||
| const { identities, totalCount } = await server.services.group.listGroupIdentities({ | ||
| id: req.params.id, | ||
| actor: req.permission.type, | ||
| actorId: req.permission.id, | ||
| actorAuthMethod: req.permission.authMethod, | ||
| actorOrgId: req.permission.orgId, | ||
| ...req.query | ||
| }); | ||
|
|
||
| return { identities, totalCount }; | ||
| } | ||
| }); | ||
|
|
||
| server.route({ | ||
| method: "GET", | ||
| url: "/:id/members", | ||
| config: { | ||
| rateLimit: readLimit | ||
| }, | ||
| onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), | ||
| schema: { | ||
| hide: false, | ||
| tags: [ApiDocsTags.Groups], | ||
| params: z.object({ | ||
| id: z.string().trim().describe(GROUPS.LIST_MEMBERS.id) | ||
| }), | ||
| querystring: z.object({ | ||
| offset: z.coerce.number().min(0).default(0).describe(GROUPS.LIST_MEMBERS.offset), | ||
| limit: z.coerce.number().min(1).max(100).default(10).describe(GROUPS.LIST_MEMBERS.limit), | ||
| search: z.string().trim().optional().describe(GROUPS.LIST_MEMBERS.search), | ||
| orderBy: z | ||
| .nativeEnum(GroupMembersOrderBy) | ||
| .default(GroupMembersOrderBy.Name) | ||
| .optional() | ||
| .describe(GROUPS.LIST_MEMBERS.orderBy), | ||
| orderDirection: z.nativeEnum(OrderByDirection).optional().describe(GROUPS.LIST_MEMBERS.orderDirection), | ||
| memberTypeFilter: z | ||
| .union([z.nativeEnum(FilterMemberType), z.array(z.nativeEnum(FilterMemberType))]) | ||
| .optional() | ||
| .describe(GROUPS.LIST_MEMBERS.memberTypeFilter) | ||
| .transform((val) => { | ||
| if (!val) return undefined; | ||
| return Array.isArray(val) ? val : [val]; | ||
| }) | ||
| }), | ||
| response: { | ||
| 200: z.object({ | ||
| members: z | ||
| .union([ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should only returns users - returning multiple format of api response is not allowed, due to parsing issue. We should keep it seperate endpoint for both list operation |
||
| UsersSchema.pick({ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An API should not return multiple responses. It should be always one. I think we can split this into two endpoints |
||
| email: true, | ||
| username: true, | ||
| firstName: true, | ||
| lastName: true, | ||
| id: true | ||
| }).extend({ | ||
| joinedGroupAt: z.date().nullable(), | ||
| memberType: z.literal("user") | ||
| }), | ||
| GroupIdentityResponseSchema.extend({ | ||
| joinedGroupAt: z.date().nullable(), | ||
| memberType: z.literal("identity") | ||
| }) | ||
| ]) | ||
| .array(), | ||
| totalCount: z.number() | ||
| }) | ||
| } | ||
| }, | ||
| handler: async (req) => { | ||
| const { members, totalCount } = await server.services.group.listGroupMembers({ | ||
| id: req.params.id, | ||
| actor: req.permission.type, | ||
| actorId: req.permission.id, | ||
| actorAuthMethod: req.permission.authMethod, | ||
| actorOrgId: req.permission.orgId, | ||
| ...req.query | ||
| }); | ||
|
|
||
| return { members, totalCount }; | ||
| } | ||
| }); | ||
|
|
||
| server.route({ | ||
| method: "GET", | ||
| url: "/:id/projects", | ||
|
|
@@ -244,10 +363,10 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { | |
| offset: z.coerce.number().min(0).default(0).describe(GROUPS.LIST_PROJECTS.offset), | ||
| limit: z.coerce.number().min(1).max(100).default(10).describe(GROUPS.LIST_PROJECTS.limit), | ||
| search: z.string().trim().optional().describe(GROUPS.LIST_PROJECTS.search), | ||
| filter: z.nativeEnum(EFilterReturnedProjects).optional().describe(GROUPS.LIST_PROJECTS.filterProjects), | ||
| filter: z.nativeEnum(FilterReturnedProjects).optional().describe(GROUPS.LIST_PROJECTS.filterProjects), | ||
| orderBy: z | ||
| .nativeEnum(EGroupProjectsOrderBy) | ||
| .default(EGroupProjectsOrderBy.Name) | ||
| .nativeEnum(GroupProjectsOrderBy) | ||
| .default(GroupProjectsOrderBy.Name) | ||
| .describe(GROUPS.LIST_PROJECTS.orderBy), | ||
| orderDirection: z | ||
| .nativeEnum(OrderByDirection) | ||
|
|
@@ -263,11 +382,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { | |
| description: true, | ||
| type: true | ||
| }) | ||
| .merge( | ||
| z.object({ | ||
| joinedGroupAt: z.date().nullable() | ||
| }) | ||
| ) | ||
| .extend({ | ||
| joinedGroupAt: z.date().nullable() | ||
| }) | ||
| .array(), | ||
| totalCount: z.number() | ||
| }) | ||
|
|
@@ -325,6 +442,38 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { | |
| } | ||
| }); | ||
|
|
||
| server.route({ | ||
| method: "POST", | ||
| url: "/:id/identities/:identityId", | ||
| config: { | ||
| rateLimit: writeLimit | ||
| }, | ||
| onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), | ||
| schema: { | ||
| hide: false, | ||
| tags: [ApiDocsTags.Groups], | ||
| params: z.object({ | ||
| id: z.string().trim().describe(GROUPS.ADD_IDENTITY.id), | ||
| identityId: z.string().trim().describe(GROUPS.ADD_IDENTITY.identityId) | ||
| }), | ||
| response: { | ||
| 200: GroupIdentityResponseSchema | ||
| } | ||
| }, | ||
| handler: async (req) => { | ||
| const identity = await server.services.group.addIdentityToGroup({ | ||
| id: req.params.id, | ||
| identityId: req.params.identityId, | ||
| actor: req.permission.type, | ||
| actorId: req.permission.id, | ||
| actorAuthMethod: req.permission.authMethod, | ||
| actorOrgId: req.permission.orgId | ||
| }); | ||
|
|
||
| return identity; | ||
| } | ||
| }); | ||
|
|
||
| server.route({ | ||
| method: "DELETE", | ||
| url: "/:id/users/:username", | ||
|
|
@@ -362,4 +511,36 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { | |
| return user; | ||
| } | ||
| }); | ||
|
|
||
| server.route({ | ||
| method: "DELETE", | ||
| url: "/:id/identities/:identityId", | ||
| config: { | ||
| rateLimit: writeLimit | ||
| }, | ||
| onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), | ||
| schema: { | ||
| hide: false, | ||
| tags: [ApiDocsTags.Groups], | ||
| params: z.object({ | ||
| id: z.string().trim().describe(GROUPS.DELETE_IDENTITY.id), | ||
| identityId: z.string().trim().describe(GROUPS.DELETE_IDENTITY.identityId) | ||
| }), | ||
| response: { | ||
| 200: GroupIdentityResponseSchema | ||
| } | ||
| }, | ||
| handler: async (req) => { | ||
| const identity = await server.services.group.removeIdentityFromGroup({ | ||
| id: req.params.id, | ||
| identityId: req.params.identityId, | ||
| actor: req.permission.type, | ||
| actorId: req.permission.id, | ||
| actorAuthMethod: req.permission.authMethod, | ||
| actorOrgId: req.permission.orgId | ||
| }); | ||
|
|
||
| return identity; | ||
| } | ||
| }); | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we discussed to roll out a generic membership and then migrate user to it later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially proposed a separate membership. Then we’ll merge them later so the generic membership doesn’t look incomplete until we merge. I think merging it in another PR while deprecating the existing ones makes more sense. What do you suggest? Should I change to the generic membership from this PR itself?