Skip to content

Commit

Permalink
Add projectID to quarterdeck register (#152)
Browse files Browse the repository at this point in the history
Co-authored-by: Benjamin Bengfort <benjamin@rotational.io>
  • Loading branch information
pdeziel and bbengfort committed Feb 3, 2023
1 parent 6dfc6dc commit feca9e1
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 15 deletions.
1 change: 1 addition & 0 deletions pkg/quarterdeck/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type PageQuery struct {
//===========================================================================

type RegisterRequest struct {
ProjectID string `json:"project_id"`
Name string `json:"name"`
Email string `json:"email"`
Password string `json:"password"`
Expand Down
40 changes: 37 additions & 3 deletions pkg/quarterdeck/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/rotationalio/ensign/pkg/quarterdeck/permissions"
"github.com/rotationalio/ensign/pkg/quarterdeck/tokens"
"github.com/rotationalio/ensign/pkg/utils/gravatar"
ulids "github.com/rotationalio/ensign/pkg/utils/ulid"
"github.com/rs/zerolog/log"
)

Expand All @@ -25,8 +26,10 @@ import (
// raw passwords.
//
// An organization is created for the user registering based on the organization data
// in the register request and the user is assigned the Owner role. This endpoint does
// not handle adding users to existing organizations through collaborator invites.
// in the register request and the user is assigned the Owner role. A project ID can be
// provided in the request to allow the client to safely create a default project for
// the user, alhough the field is optional. This endpoint does not handle adding users
// to existing organizations through collaborator invites.
// TODO: add rate limiting to ensure that we don't get spammed with registrations
func (s *Server) Register(c *gin.Context) {
var (
Expand All @@ -35,6 +38,8 @@ func (s *Server) Register(c *gin.Context) {
out *api.RegisterReply
)

ctx := c.Request.Context()

if err = c.BindJSON(&in); err != nil {
log.Warn().Err(err).Msg("could not parse register request")
c.JSON(http.StatusBadRequest, api.ErrorResponse("could not parse request"))
Expand All @@ -46,6 +51,16 @@ func (s *Server) Register(c *gin.Context) {
return
}

// ProjecID is an optional field
var projectID ulid.ULID
if in.ProjectID != "" {
if projectID, err = ulid.Parse(in.ProjectID); err != nil {
log.Error().Err(err).Str("project_id", in.ProjectID).Msg("could not parse project ID")
c.JSON(http.StatusBadRequest, api.ErrorResponse("could not parse project ID in request"))
return
}
}

// Create a user model to insert into the database.
user := &models.User{
Name: in.Name,
Expand All @@ -68,7 +83,7 @@ func (s *Server) Register(c *gin.Context) {
Domain: in.Domain,
}

if err = user.Create(c.Request.Context(), org, permissions.RoleOwner); err != nil {
if err = user.Create(ctx, org, permissions.RoleOwner); err != nil {
// Handle constraint errors
var dberr *models.ConstraintError
if errors.As(err, &dberr) {
Expand All @@ -81,6 +96,25 @@ func (s *Server) Register(c *gin.Context) {
return
}

// If a project ID is provided then link the user's organization to the project by
// creating a database record. This allows a path for the client to create a
// default project for new users without having to go through a separate,
// authenticated request. See ProjectCreate for more details.
if !ulids.IsZero(projectID) {
// TODO: Failure to save this record will create an inconsistent situation
// where the user and organization were created but the project was not linked
// to the organization.
op := &models.OrganizationProject{
OrgID: org.ID,
ProjectID: projectID,
}
if err = op.Save(ctx); err != nil {
log.Error().Err(err).Msg("could not link organization to project")
c.JSON(http.StatusInternalServerError, api.ErrorResponse("could not process registration"))
return
}
}

// Prepare response to return to the registering user.
out = &api.RegisterReply{
ID: user.ID,
Expand Down
31 changes: 30 additions & 1 deletion pkg/quarterdeck/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ func (s *quarterdeckTestSuite) TestRegister() {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

project := "01GKHJRF01YXHZ51YMMKV3RCMK"
projectID := ulid.MustParse(project)
req := &api.RegisterRequest{
Name: "Rachel Johnson",
Email: "rachel@example.com",
Expand All @@ -44,6 +46,27 @@ func (s *quarterdeckTestSuite) TestRegister() {
require.NoError(err, "could not get user from database")
require.Equal(rep.Email, user.Email, "user creation check failed")

// Test with a project ID provided
req.Email = "jane@example.com"
req.Domain = "it-services"
req.ProjectID = project
rep, err = s.client.Register(ctx, req)
require.NoError(err, "unable to create user from valid request")

// Test that the user made it into the database
user, err = models.GetUser(context.Background(), rep.ID, rep.OrgID)
require.NoError(err, "could not get user from database")
require.Equal(rep.Email, user.Email, "user creation check failed")

// Test that the organization project link was created in the database
op := &models.OrganizationProject{
OrgID: rep.OrgID,
ProjectID: projectID,
}
ok, err := op.Exists(context.Background())
require.NoError(err, "could not check if organization project link exists")
require.True(ok, "organization project link was not created")

// Test error paths
// Test password mismatch
req.PwCheck = "notthe same"
Expand All @@ -62,8 +85,14 @@ func (s *quarterdeckTestSuite) TestRegister() {
_, err = s.client.Register(ctx, req)
s.CheckError(err, http.StatusBadRequest, "missing required field: email")

// Test cannot create existing user
// Test invalid project ID
req.Email = "jannel@example.com"
req.ProjectID = "invalid"
_, err = s.client.Register(ctx, req)
s.CheckError(err, http.StatusBadRequest, "could not parse project ID in request")

// Test cannot create existing user
req.ProjectID = ""
_, err = s.client.Register(ctx, req)
s.CheckError(err, http.StatusConflict, "user or organization already exists")

Expand Down
13 changes: 8 additions & 5 deletions pkg/tenant/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
middleware "github.com/rotationalio/ensign/pkg/quarterdeck/middleware"
"github.com/rotationalio/ensign/pkg/tenant/api/v1"
"github.com/rotationalio/ensign/pkg/tenant/db"
"github.com/rotationalio/ensign/pkg/utils/ulid"
"github.com/rs/zerolog/log"
)

Expand Down Expand Up @@ -41,11 +42,13 @@ func (s *Server) Register(c *gin.Context) {
}

// Make the register request to Quarterdeck
projectID := ulid.New()
req := &qd.RegisterRequest{
Name: params.Name,
Email: params.Email,
Password: params.Password,
PwCheck: params.PwCheck,
ProjectID: projectID.String(),
Name: params.Name,
Email: params.Email,
Password: params.Password,
PwCheck: params.PwCheck,
}

var reply *qd.RegisterReply
Expand All @@ -67,7 +70,7 @@ func (s *Server) Register(c *gin.Context) {

// Create a default tenant and project for the new user
// Note: This method returns an error if the member model is invalid
if err = db.CreateUserResources(ctx, member); err != nil {
if err = db.CreateUserResources(ctx, projectID, member); err != nil {
log.Error().Str("user_id", reply.ID.String()).Err(err).Msg("could not create default tenant and project for new user")
// TODO: Does this leave the user in a bad state? Can they still use the app?
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/tenant/db/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ package db
import (
"context"
"fmt"

"github.com/oklog/ulid/v2"
)

// CreateUserResources creates all the necessary database objects for a new user given
// a partially constructed member model. This method should be called after a new user
// has been successfully registered with Quarterdeck in order to allow the user to
// access default resources such as the tenant and project when they login.
func CreateUserResources(ctx context.Context, member *Member) (err error) {
func CreateUserResources(ctx context.Context, projectID ulid.ULID, member *Member) (err error) {
// Ensure the user data is valid before creating anything
if err = member.Validate(false); err != nil {
return err
Expand All @@ -33,6 +35,7 @@ func CreateUserResources(ctx context.Context, member *Member) (err error) {

// New user should have a default project
project := &Project{
ID: projectID,
OrgID: member.OrgID,
TenantID: tenant.ID,
Name: fmt.Sprintf("%s's Project", member.Name),
Expand Down
13 changes: 8 additions & 5 deletions pkg/tenant/db/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/oklog/ulid/v2"
"github.com/rotationalio/ensign/pkg/tenant/db"
ulids "github.com/rotationalio/ensign/pkg/utils/ulid"
pb "github.com/trisacrypto/directory/pkg/trtl/pb/v1"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand All @@ -14,6 +15,8 @@ func (s *dbTestSuite) TestCreateUserResources() {
require := s.Require()
ctx := context.Background()

projectID := ulids.New()

// Configure trtl to return success for all requests
s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) {
if len(in.Key) == 0 || len(in.Value) == 0 {
Expand All @@ -30,21 +33,21 @@ func (s *dbTestSuite) TestCreateUserResources() {
Name: "Leopold Wentzel",
Role: "Member",
}
require.ErrorIs(db.CreateUserResources(ctx, member), db.ErrMissingOrgID, "expected error when orgID is missing")
require.ErrorIs(db.CreateUserResources(ctx, projectID, member), db.ErrMissingOrgID, "expected error when orgID is missing")

// Should return an error if user name is missing
member.Name = ""
member.OrgID = ulid.MustParse("02ABCYAWC4PA72YC53RVXAEC67")
require.ErrorIs(db.CreateUserResources(ctx, member), db.ErrMissingMemberName, "expected error when member name is missing")
require.ErrorIs(db.CreateUserResources(ctx, projectID, member), db.ErrMissingMemberName, "expected error when member name is missing")

// Should return an error if user role is missing
member.Name = "Leopold Wentzel"
member.Role = ""
require.ErrorIs(db.CreateUserResources(ctx, member), db.ErrMissingMemberRole, "expected error when member role is missing")
require.ErrorIs(db.CreateUserResources(ctx, projectID, member), db.ErrMissingMemberRole, "expected error when member role is missing")

// Succesfully creating all the required resources
member.Role = "Member"
require.NoError(db.CreateUserResources(ctx, member), "expected no error when creating user resources")
require.NoError(db.CreateUserResources(ctx, projectID, member), "expected no error when creating user resources")
require.NotEmpty(member.ID, "expected member ID to be set")
require.NotEmpty(member.TenantID, "expected tenant ID to be set")
require.NotEmpty(member.Created, "expected created time to be set")
Expand All @@ -54,5 +57,5 @@ func (s *dbTestSuite) TestCreateUserResources() {
s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) {
return nil, status.Error(codes.Internal, "trtl error")
}
require.Error(db.CreateUserResources(ctx, member), "expected error when trtl returns an error")
require.Error(db.CreateUserResources(ctx, projectID, member), "expected error when trtl returns an error")
}

0 comments on commit feca9e1

Please sign in to comment.