Sc 15474/invite registration #389
Conversation
pkg/tenant/auth.go
Outdated
| @@ -139,13 +142,53 @@ func (s *Server) Login(c *gin.Context) { | |||
| Password: params.Password, | |||
| } | |||
|
|
|||
| ok := hasInviteToken(params.InviteToken) | |||
There was a problem hiding this comment.
Instead of the bool can we just use params.InviteToken != nil? To me that's actually a lot more clear.
pkg/tenant/auth.go
Outdated
| if ok && reply.AccessToken != "" { | ||
| // Parse access token to get the orgID. | ||
| var claims *jwt.RegisteredClaims | ||
| if claims, err = tokens.ParseUnverified(reply.AccessToken); err != nil { |
There was a problem hiding this comment.
Glad this method worked out!
There was a problem hiding this comment.
So looks like we should create another method in the tokens package which passes in the token claims instead of the jwt claims:
func ParseUnverifiedTokenClaims(tks string) (claims *tokens.Claims, err error) {
claims := &tokens.Claims{}
if _, _, err = tsparser.ParseUnverified(tks, claims); err != nil {
return nil, err
}
return claims, nil
}
pkg/tenant/auth.go
Outdated
| var reply *qd.LoginReply | ||
| if reply, err = s.quarterdeck.Login(c.Request.Context(), req); err != nil { | ||
| sentry.Debug(c).Err(err).Msg("tracing quarterdeck error in tenant") | ||
| api.ReplyQuarterdeckError(c, err) | ||
| return | ||
| } | ||
|
|
||
| // Update the status of a member with an invite token to Confirmed. | ||
| if ok && reply.AccessToken != "" { |
There was a problem hiding this comment.
Probably only need to check params.InviteToken != "" here since (hopefully) quarterdeck always returns an error if it does not return an access token.
| sentry.Error(c).Err(err).Msg("could not update member in the database") | ||
| c.JSON(http.StatusInternalServerError, api.ErrorResponse("could not update member")) | ||
| return | ||
| } |
There was a problem hiding this comment.
I think we also need to include the same delete/create logic you had in register since the same situation applies: if the user creates a new account after the invite was already issued then the member ID needs to be updated, which in trtl means creating a new record.
pkg/tenant/db/errors.go
Outdated
| @@ -34,7 +34,8 @@ var ( | |||
| ErrInvalidTopicName = errors.New("invalid topic name") | |||
|
|
|||
| // Database state errors | |||
| ErrMemberExists = errors.New("member already exists") | |||
| ErrMemberExists = errors.New("member already exists") | |||
| ErrInvalidMemberEmail = errors.New("member does not exist") | |||
There was a problem hiding this comment.
Should this just be ErrMemberEmailNotFound to prevent confusing it with email validation?
|
|
||
| rep, err := db.GetMemberByEmail(ctx, orgID, email) | ||
| require.NoError(err, "could not get member by email") | ||
| require.Equal(rep.Email, email, "expected member email to match") |
Co-authored-by: Patrick Deziel <42919891+pdeziel@users.noreply.github.com>
| @@ -333,3 +334,13 @@ func (s *TokenTestSuite) TestParseExpiredToken() { | |||
| func TestTokenTestSuite(t *testing.T) { | |||
| suite.Run(t, new(TokenTestSuite)) | |||
| } | |||
|
|
|||
| func TestParseUnverifiedTokenClaims(t *testing.T) { | |||
There was a problem hiding this comment.
Thanks for adding the test!
Scope of changes
Adds an invite token to Tenant
Loginrequests. Also adds a method to the Tenant db package that retrieves a member by a given email address.Fixes SC-15474
Type of change
Acceptance criteria
Describe how reviewers can test this change to be sure that it works correctly. Add a checklist if possible.
Author checklist
Reviewer(s) checklist