-
Notifications
You must be signed in to change notification settings - Fork 903
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
fix: fixes authorization checks for update product server action #2231
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
Thank you for following the naming conventions for pull request titles! 🙏 |
apps/web/app/(app)/environments/[environmentId]/settings/product/actions.tsInstead of making two separate calls to get the team and then the membership, you could create a new function that gets the membership by the environmentId and userId directly. This would reduce the number of database calls and improve the performance of the code. const membership = await getMembershipByEnvironmentIdUserId(environmentId, session.user.id);
if (!membership) {
throw new AuthorizationError("Not authorized");
}
if (membership.role === "viewer" || membership.role === "developer") {
throw new AuthorizationError("You are not allowed to update products.");
}
Instead of using multiple if statements to check the role of the user, you could use a switch statement. This would make the code more readable and easier to maintain. switch (membership.role) {
case "viewer":
case "developer":
throw new AuthorizationError("You are not allowed to update products.");
default:
const updatedProduct = await updateProduct(productId, data);
return updatedProduct;
}
|
const team = await getTeamByEnvironmentId(environmentId); | ||
const membership = team ? await getMembershipByUserIdTeamId(session.user.id, team.id) : null; | ||
|
||
if (!membership) { | ||
throw new AuthorizationError("Not authorized"); | ||
} | ||
|
||
if (membership.role === "viewer" || membership.role === "developer") { | ||
throw new AuthorizationError("You are not allowed to update products."); | ||
} |
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.
Reducing the number of database calls by creating a new function that gets the membership by the environmentId and userId directly.
const team = await getTeamByEnvironmentId(environmentId); | |
const membership = team ? await getMembershipByUserIdTeamId(session.user.id, team.id) : null; | |
if (!membership) { | |
throw new AuthorizationError("Not authorized"); | |
} | |
if (membership.role === "viewer" || membership.role === "developer") { | |
throw new AuthorizationError("You are not allowed to update products."); | |
} | |
const membership = await getMembershipByEnvironmentIdUserId(environmentId, session.user.id); | |
if (!membership) { | |
throw new AuthorizationError("Not authorized"); | |
} | |
if (membership.role === "viewer" || membership.role === "developer") { | |
throw new AuthorizationError("You are not allowed to update products."); | |
} |
if (membership.role === "viewer" || membership.role === "developer") { | ||
throw new AuthorizationError("You are not allowed to update products."); | ||
} | ||
|
||
const updatedProduct = await updateProduct(productId, data); | ||
return updatedProduct; |
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.
Replaced the multiple if statements with a switch statement for better readability and maintainability.
if (membership.role === "viewer" || membership.role === "developer") { | |
throw new AuthorizationError("You are not allowed to update products."); | |
} | |
const updatedProduct = await updateProduct(productId, data); | |
return updatedProduct; | |
switch (membership.role) { | |
case "viewer": | |
case "developer": | |
throw new AuthorizationError("You are not allowed to update products."); | |
default: | |
const updatedProduct = await updateProduct(productId, data); | |
return updatedProduct; | |
} |
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.
@pandeymangg thanks for fixing this and the quick turnaround! 💪
What does this PR do?
Adds authorization checks in the update product server action allowing only
owner
,admin
andeditor
to update a productFixes # (issue)
How should this be tested?
Checklist
Required
pnpm build
console.logs
git pull origin main
Appreciated