-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: handle invalid invitation token gracefully in signup page #7126
base: main
Are you sure you want to change the base?
Conversation
Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗 |
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.
❌ Changes requested. Reviewed everything up to fbcd6c9 in 2 minutes and 26 seconds
More details
- Looked at
47
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. frontend/src/pages/SignUp/SignUp.tsx:346
- Draft comment:
Avoid using inline styles with hardcoded colors. Use design tokens or CSS classes instead of '#D89614'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. frontend/src/pages/SignUp/SignUp.tsx:357
- Draft comment:
Avoid inline styling; extract color '#D89614' into a design token or CSS variable. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. frontend/src/types/common/index.ts:30
- Draft comment:
Added 'Unavailable' to ErrorType enum. Ensure its usage is consistent throughout the app. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. pkg/query-service/auth/auth.go:455
- Draft comment:
Replacing BadRequest with ForbiddenError for invalid invitation tokens correctly signals a 403 error. This improves clarity in error handling. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions. It simply states that a change is good, which violates the rule against making purely informative comments.
Workflow ID: wflow_XaKTa1636hdMNbop
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@@ -452,7 +452,7 @@ func RegisterInvitedUser(ctx context.Context, req *RegisterRequest, nopassword b | |||
invite, err := ValidateInvite(ctx, req) | |||
if err != nil { | |||
zap.L().Error("failed to validate invite token", zap.Error(err)) | |||
return nil, model.BadRequest(model.ErrSignupFailed{}) | |||
return nil, model.ForbiddenError(fmt.Errorf("invalid/used/expired invitation token, please contact your admin")) |
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.
Redirecting to Login page only on Forbidden
errors. I'm assuming we don't want to redirect on any and all errors thrown by sign-up API, that would be a bad experience. So throwing a Forbidden
instead of BadRequest
made sense here.
If not then I'm open to suggestions! :)
Summary
If the sign up fails, we redirect the users to the Login Page where we should display the error message: "invalid/used/expired invitation token. Please contact your admin."
Related Issues / PR's
#7011
Screenshots
Screen.Recording.2025-02-16.at.7.17.15.p.m.mov
Affected Areas and Manually Tested Areas
frontend/src/pages/SignUp/SignUp.tsx
pkg/query-service/auth/auth.go
Important
Handle invalid invitation tokens by redirecting users to the login page with an error message in
SignUp.tsx
and updating error handling inauth.go
.SignUp.tsx
.RegisterInvitedUser
inauth.go
to return aForbiddenError
with a specific message for invalid tokens.Unavailable
toErrorType
incommon/index.ts
.auth.go
.This description was created by
for fbcd6c9. It will automatically update as commits are pushed.