Skip to content

Conversation

kfahad5607
Copy link
Contributor

Fixes #141 by checking for entities before deleting/modifying them to avoid prisma query error.

Route handler changes

  • Checking if team membership exists in Remove a user from a team
  • Checking if team and user exist in Grant a team permission to a user
  • Checking if team-permission for user exists in Revoke a team permission from a user
  • Checking if team exists in Delete a team
  • Checking if user exists in Update user
  • Checking if team membership exists when selected_team_id is passed in request body in Update user
  • Checking if user exists in Delete user

Other changes

  • Updated selectedTeamIdSchema to validate string as uuid
  • Added new helper function ensureUserTeamPermissionExist in request-checks to use in Revoke a team permission from a user
  • Added new error UserTeamPermissionNotFound to use in ensureUserTeamPermissionExist

Copy link

vercel bot commented Aug 4, 2024

Someone is attempting to deploy a commit to the Stack Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant
Copy link

CLAassistant commented Aug 4, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ fomalhautb
❌ Fahad Khan


Fahad Khan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@fomalhautb fomalhautb left a comment

Choose a reason for hiding this comment

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

Looks pretty clean! Also can you sign the CLA?

@@ -105,6 +105,67 @@ export async function ensureUserHasTeamPermission(
}
}

export async function ensureUserTeamPermissionExist(
Copy link
Contributor

@fomalhautb fomalhautb Aug 4, 2024

Choose a reason for hiding this comment

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

I think this function is a little bit duplicated to ensureUserHasTeamPermission. Maybe you can combine them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am combining these functions, but logically, they signify different types of errors.

  1. ensureUserHasTeamPermission

    • This function throws a TeamPermissionRequired error, which indicates an authorization error. Users might misconstrue this error as a permission issue in the case of the Revoke a team permission from a user API request.
  2. ensureUserTeamPermissionExist

    • This function throws a UserTeamPermissionNotFound error, indicating that the permission the user is trying to delete does not exist. This is more appropriate for the Revoke a team permission from a user API request.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I added errorType parameter to decide which type of error to throw in the ensureUserHasTeamPermission function.

@@ -158,6 +167,11 @@ export const usersCrudHandlers = createLazyProxy(() => createCrudHandlers(usersC
}
}

// If the selected_team_id is present and we reach here that means user does exist. Hence, we have this check.
if (!data.selected_team_id) {
await ensureUserExist(tx, { projectId: auth.project.id, userId: params.user_id });
Copy link
Contributor

Choose a reason for hiding this comment

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

We can move this check to the beginning and remove the condition so it is easier to follow

@kfahad5607
Copy link
Contributor Author

I have made the requested changes.

  • Deleted the ensureUserTeamPermissionExist function and UserTeamPermissionNotFound error.
  • Updated the ensureUserHasTeamPermission function to accept both System Permission and Custom Permission via options.permissionId.
  • Checking for an existing membership between the team and user, instead of performing individual checks for the user and team in Grant a team permission to a user.

PS: I have also signed the CLA.

@N2D4
Copy link
Contributor

N2D4 commented Aug 7, 2024

CLA assistant check

FYI, looks like the bot hasn't picked up the CLA, but I can confirm that you signed it, so all good!

Copy link

vercel bot commented Aug 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stack-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2024 11:51pm

@fomalhautb fomalhautb merged commit 2fef40b into stack-auth:dev Aug 9, 2024
6 of 12 checks passed
@fomalhautb
Copy link
Contributor

Thanks for the contribution!

@kfahad5607 kfahad5607 deleted the fix/141_error-handling-in-api-handlers branch August 10, 2024 10:18
N2D4 pushed a commit that referenced this pull request Aug 10, 2024
* Added entity checks to provide better errors in API for 'server' access type

* Removed 'ensureUserTeamPermissionExist', changed permissionId type to string in 'ensureUserHasTeamPermission'

* added different error types for user team permission

---------

Co-authored-by: Fahad Khan <fahad.khan@net-mon.net>
Co-authored-by: Zai Shi <zaishi00@outlook.com>
fomalhautb added a commit that referenced this pull request Aug 12, 2024
* Team invitation (#171)

* team invitation wip

* implemented handler

* team invitation callback wip

* added team invitation frontend

* fixed listCurrentUserTeamPermissions

* added team invitation email template

* fixed bugs

* fixed verification code handler

* added more checks to team invitation verification

* fixed team invitation page

* restructured verification code handler

* fixed frontend

* fixed team invitation tests

* added more team invitation test

* fixed bug

* added migration file

* removed unused code

* Allow Next.js version `latest` in package.json

* Fix typo

* Update error message

* Remove unnecessary console.warn

* Updated "edit this page" button

* Hide unsupported properties from docs

* OAuth token tests

* Fix typo

* added create user button

* added create user button (#173)

* added basic team settings

* Create SECURITY.md

* added editable text

* added more team settings

* Export button in tables

* Export all pages of tables

* Update security policy

* Fix docs typo

* More docs typos

* Improved user creation handlers

* added list users on client

* updated team-settings

* hide team setting component for now

* Fix: Improve error handling for Server API (#170)

* Added entity checks to provide better errors in API for 'server' access type

* Removed 'ensureUserTeamPermissionExist', changed permissionId type to string in 'ensureUserHasTeamPermission'

* added different error types for user team permission

---------

Co-authored-by: Fahad Khan <fahad.khan@net-mon.net>
Co-authored-by: Zai Shi <zaishi00@outlook.com>

* added ensureClientUserAuthenticated

* improved error handling

* removed unused imports

* fixed bug

* added member list

* Sign up restriction button on dashboard

Fix #66, #74

* moved data table to stack-ui

* added remove user modal

* fixed chokidar

* updated ui

* fixed merge

* fixed merge

* fixed merge

* updated settings component

* improved mobile styles

* added user invitation ui

* added team creation page

* added team creation to team component

* added setting icon to team switcher

* added settings sections

* added client_team_creation_enabled

* added frontend team creation enabled checks

* updated demo page

* added member profile update

* fixed profile editing

* added leave team button

* added create/delete team redirect

* fixed column header, updated team setting

* fixed account setting padding

* updated tests

---------

Co-authored-by: Stan Wohlwend <n2d4xc@gmail.com>
Co-authored-by: Fahad Khan <62707456+kfahad5607@users.noreply.github.com>
Co-authored-by: Fahad Khan <fahad.khan@net-mon.net>
fomalhautb added a commit that referenced this pull request Aug 28, 2024
* Team invitation (#171)

* team invitation wip

* implemented handler

* team invitation callback wip

* added team invitation frontend

* fixed listCurrentUserTeamPermissions

* added team invitation email template

* fixed bugs

* fixed verification code handler

* added more checks to team invitation verification

* fixed team invitation page

* restructured verification code handler

* fixed frontend

* fixed team invitation tests

* added more team invitation test

* fixed bug

* added migration file

* removed unused code

* Allow Next.js version `latest` in package.json

* Fix typo

* Update error message

* Remove unnecessary console.warn

* Updated "edit this page" button

* Hide unsupported properties from docs

* OAuth token tests

* Fix typo

* added create user button (#173)

* Create SECURITY.md

* Export button in tables

* Export all pages of tables

* Update security policy

* Fix docs typo

* More docs typos

* Improved user creation handlers

* Fix: Improve error handling for Server API (#170)

* Added entity checks to provide better errors in API for 'server' access type

* Removed 'ensureUserTeamPermissionExist', changed permissionId type to string in 'ensureUserHasTeamPermission'

* added different error types for user team permission

---------

Co-authored-by: Fahad Khan <fahad.khan@net-mon.net>
Co-authored-by: Zai Shi <zaishi00@outlook.com>

* Sign up restriction button on dashboard

Fix #66, #74

* Fix type error

* TOTP 2FA endpoints

* TOTP MFA components

* Improved description for disabling sign ups

* Added 'allowedErrorTypes' for error propagation

---------

Co-authored-by: Zai Shi <zaishi00@outlook.com>
Co-authored-by: Stan Wohlwend <n2d4xc@gmail.com>
Co-authored-by: Fahad Khan <fahad.khan@net-mon.net>
fomalhautb added a commit that referenced this pull request Sep 14, 2024
)

* Team invitation (#171)

* team invitation wip

* implemented handler

* team invitation callback wip

* added team invitation frontend

* fixed listCurrentUserTeamPermissions

* added team invitation email template

* fixed bugs

* fixed verification code handler

* added more checks to team invitation verification

* fixed team invitation page

* restructured verification code handler

* fixed frontend

* fixed team invitation tests

* added more team invitation test

* fixed bug

* added migration file

* removed unused code

* Allow Next.js version `latest` in package.json

* Fix typo

* Update error message

* Remove unnecessary console.warn

* Updated "edit this page" button

* Hide unsupported properties from docs

* OAuth token tests

* Fix typo

* added create user button (#173)

* Create SECURITY.md

* Export button in tables

* Export all pages of tables

* Update security policy

* Fix docs typo

* More docs typos

* Improved user creation handlers

* Fix: Improve error handling for Server API (#170)

* Added entity checks to provide better errors in API for 'server' access type

* Removed 'ensureUserTeamPermissionExist', changed permissionId type to string in 'ensureUserHasTeamPermission'

* added different error types for user team permission

---------

Co-authored-by: Fahad Khan <fahad.khan@net-mon.net>
Co-authored-by: Zai Shi <zaishi00@outlook.com>

* Sign up restriction button on dashboard

Fix #66, #74

* Fix type error

* TOTP 2FA endpoints

* TOTP MFA components

* Improved description for disabling sign ups

* Removed 'selected_team_id' from create user schema

---------

Co-authored-by: Zai Shi <zaishi00@outlook.com>
Co-authored-by: Stan Wohlwend <n2d4xc@gmail.com>
Co-authored-by: Fahad Khan <fahad.khan@net-mon.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Error Handling in API Handlers
5 participants