-
Notifications
You must be signed in to change notification settings - Fork 3
chore: role base implementation #355
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 18 commits
4dd7b85
b2a1d4f
2e96770
4a216b7
22ae5dc
5bb7617
ac4e7bc
c335d42
e735d67
660d31c
0f2aa6c
6626154
c230769
bee6e59
323eec5
e54574c
9e9bf26
d1d8585
6340390
fbb5a87
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 | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,6 +25,7 @@ import { | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import { USER_MOVEMENT_STATUSES } from 'erxes-api-shared/core-modules'; | ||||||||||||||||||||||||||||||||||||||||||
| import { title } from 'process'; | ||||||||||||||||||||||||||||||||||||||||||
| import { PERMISSION_ROLES } from '~/modules/permissions/db/constants'; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const SALT_WORK_FACTOR = 10; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -234,7 +235,7 @@ export const loadUserClass = (models: IModels, subdomain: string) => { | |||||||||||||||||||||||||||||||||||||||||
| this.checkPassword(password); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return models.Users.create({ | ||||||||||||||||||||||||||||||||||||||||||
| const user = await models.Users.create({ | ||||||||||||||||||||||||||||||||||||||||||
| isOwner, | ||||||||||||||||||||||||||||||||||||||||||
| username, | ||||||||||||||||||||||||||||||||||||||||||
| email, | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -246,6 +247,13 @@ export const loadUserClass = (models: IModels, subdomain: string) => { | |||||||||||||||||||||||||||||||||||||||||
| password: notUsePassword ? '' : await this.generatePassword(password), | ||||||||||||||||||||||||||||||||||||||||||
| code: await this.generateUserCode(), | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| models.Roles.create({ | ||||||||||||||||||||||||||||||||||||||||||
| userId: user._id, | ||||||||||||||||||||||||||||||||||||||||||
| role: PERMISSION_ROLES.MEMBER, | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+251
to
+254
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. logic: Missing
Suggested change
Comment on lines
+251
to
+254
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. Assign OWNER role when
- models.Roles.create({
- userId: user._id,
- role: PERMISSION_ROLES.MEMBER,
- });
+ const role = isOwner
+ ? PERMISSION_ROLES.OWNER
+ : PERMISSION_ROLES.MEMBER;
+
+ models.Roles.create({
+ userId: user._id,
+ role,
+ });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return user; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -329,7 +337,7 @@ export const loadUserClass = (models: IModels, subdomain: string) => { | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| this.checkPassword(password); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| await models.Users.create({ | ||||||||||||||||||||||||||||||||||||||||||
| const user = await models.Users.create({ | ||||||||||||||||||||||||||||||||||||||||||
| email, | ||||||||||||||||||||||||||||||||||||||||||
| groupIds: [groupId], | ||||||||||||||||||||||||||||||||||||||||||
| isActive: true, | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -341,6 +349,11 @@ export const loadUserClass = (models: IModels, subdomain: string) => { | |||||||||||||||||||||||||||||||||||||||||
| brandIds, | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| models.Roles.create({ | ||||||||||||||||||||||||||||||||||||||||||
| userId: user._id, | ||||||||||||||||||||||||||||||||||||||||||
| role: PERMISSION_ROLES.MEMBER, | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+352
to
+355
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. logic: Missing
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return token; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -795,7 +808,7 @@ export const loadUserClass = (models: IModels, subdomain: string) => { | |||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if (user.isOwner && !user.lastSeenAt) { | ||||||||||||||||||||||||||||||||||||||||||
| if (!user.lastSeenAt) { | ||||||||||||||||||||||||||||||||||||||||||
| const pluginNames = await getPlugins(); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| for (const pluginName of pluginNames) { | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,17 +1,19 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IContext } from '~/connectionResolvers'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| IUser, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| IDetail, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ILink, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| IEmailSignature, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ILink, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| IUser, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Resolver, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } from 'erxes-api-shared/core-types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IContext } from '~/connectionResolvers'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { PERMISSION_ROLES } from '~/modules/permissions/db/constants'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface IUsersEdit extends IUser { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| channelIds?: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| _id: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| export const userMutations = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export const userMutations: Record<string, Resolver> = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| async usersCreateOwner( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| _parent: undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -48,7 +50,12 @@ export const userMutations = { | |||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| await models.Users.createUser(doc); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const user = await models.Users.createUser(doc); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| models.Roles.create({ | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. Similarly, consider awaiting the call to models.Roles.create in the usersCreateOwner mutation for consistent error handling.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| userId: user._id, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| role: PERMISSION_ROLES.OWNER, | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+55
to
+57
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. issue (bug_risk): Role creation is not awaited after user creation. Not awaiting models.Roles.create may lead to permission issues if later code assumes the role exists. Please await this call. |
||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+55
to
+58
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. logic: Missing
Suggested change
Comment on lines
+53
to
+58
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. Prevent owner role downgrade and await persistence
- models.Roles.create({
- userId: user._id,
- role: PERMISSION_ROLES.OWNER,
- });
+ await models.Roles.findOneAndUpdate(
+ { userId: user._id },
+ { $set: { role: PERMISSION_ROLES.OWNER }, $setOnInsert: { userId: user._id } },
+ { upsert: true },
+ );📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if (subscribeEmail && process.env.NODE_ENV === 'production') { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| await fetch('https://erxes.io/subscribe', { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -138,14 +145,14 @@ export const userMutations = { | |||||||||||||||||||||||||||||||||||||||||||||||||
| details, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| links, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| employeeId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| positionIds | ||||||||||||||||||||||||||||||||||||||||||||||||||
| positionIds, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }: { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| username: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| email: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| details: IDetail; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| links: ILink; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| employeeId: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| positionIds: string[] | ||||||||||||||||||||||||||||||||||||||||||||||||||
| positionIds: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| { user, models }: IContext, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -158,7 +165,7 @@ export const userMutations = { | |||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| links, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| employeeId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| positionIds | ||||||||||||||||||||||||||||||||||||||||||||||||||
| positionIds, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const updatedUser = await models.Users.editProfile(user._id, doc); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -351,3 +358,5 @@ export const userMutations = { | |||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| userMutations.usersCreateOwner.skipPermission = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| export const PERMISSION_ROLES = { | ||
| OWNER: 'owner', | ||
| ADMIN: 'admin', | ||
| MEMBER: 'member', | ||
| ALL: ['owner', 'admin', 'member'], | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { Schema } from 'mongoose'; | ||
| import { PERMISSION_ROLES } from '~/modules/permissions/db/constants'; | ||
|
|
||
| export const roleSchema = new Schema( | ||
| { | ||
| userId: { type: String, label: 'User', index: true, required: true }, | ||
| role: { | ||
| type: String, | ||
| enum: PERMISSION_ROLES.ALL, | ||
| label: 'Role', | ||
| index: true, | ||
| required: true, | ||
| }, | ||
| }, | ||
| { | ||
| timestamps: true, | ||
| }, | ||
| ); | ||
|
|
||
| roleSchema.index({ userId: 1, role: 1 }, { unique: true }); | ||
|
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. 🧩 Analysis chainCompound unique index correctly enforces one document per (userId, role) pair. The compound unique index allows a user to have multiple role documents (e.g., one for OWNER, another for ADMIN). Verify this multi-role-per-user design aligns with your authorization model. If the intent is strictly one role per user, change the index to Run this script to confirm whether the codebase expects single or multiple roles per user: 🏁 Script executed: #!/bin/bash
# Check if role assignment logic expects one role or multiple roles per user
echo "=== Searching for role assignment patterns ==="
rg -nP --type=ts 'models\.Roles\.(create|update|getRole)' -A 3 -B 1
echo ""
echo "=== Checking for role queries/filters ==="
rg -nP --type=ts 'userId.*role|role.*userId' -C 2 -g '!**/node_modules/**'Length of output: 4857 🏁 Script executed: #!/bin/bash
set -e
echo "=== getRole implementation ==="
rg -nC5 "static async getRole" backend/core-api/src/modules/permissions/db/models/Roles.ts
echo ""
echo "=== createRole implementation ==="
rg -nC5 "static async createRole" backend/core-api/src/modules/permissions/db/models/Roles.ts
echo ""
echo "=== updateRole implementation ==="
rg -nC5 "static async updateRole" backend/core-api/src/modules/permissions/db/models/Roles.tsLength of output: 984 Enforce single-role-per-user by updating unique index roleSchema.index({ userId: 1, role: 1 }, { unique: true });to roleSchema.index({ userId: 1 }, { unique: true });to align with getRole/createRole/updateRole—all of which operate on a single document per user. 🤖 Prompt for AI Agents |
||
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.
Consider awaiting the asynchronous call to models.Roles.create when assigning a role, so that any errors in role creation are caught.