From 887a7eab67ae0dcab609c1de9ad5b5b09af7d8fa Mon Sep 17 00:00:00 2001 From: Hiram Chirino Date: Thu, 22 Feb 2024 17:18:05 -0500 Subject: [PATCH 1/2] apiserver: support multiple Organization owners * added a roles []string field to user_organizations table, can be set to owner|member, allows for additional roles in the future. * remove the organization owner_id field, as we can find owners using the roles field. * added a new StringArray type that better deals with the differences between sqlite and postgresql Signed-off-by: Hiram Chirino --- .../features/invitation-api.feature | 10 +- .../features/organization-api.feature | 3 +- .../features/user-organization-api.feature | 8 ++ .../api/public/model_models_add_invitation.go | 5 +- .../api/public/model_models_invitation.go | 1 + .../api/public/model_models_organization.go | 1 - .../public/model_models_user_organization.go | 1 + internal/database/database.go | 1 + internal/database/datatype/string_array.go | 81 +++++++++++++ .../migration_20240221_0000/migration.go | 111 ++++++++++++++++++ internal/database/migrations/migrations.go | 13 ++ internal/docs/docs.go | 22 +++- internal/docs/swagger.json | 22 +++- internal/docs/swagger.yaml | 15 ++- internal/handlers/invitations.go | 33 ++++-- internal/handlers/organization.go | 37 ++++-- internal/handlers/security_group.go | 12 +- internal/handlers/user.go | 71 +++++++---- internal/handlers/vpc.go | 11 +- internal/models/base.go | 3 + internal/models/invitation.go | 2 + internal/models/organization.go | 6 +- internal/models/user_organization.go | 11 +- internal/util/string.go | 9 ++ 24 files changed, 414 insertions(+), 75 deletions(-) create mode 100644 internal/database/datatype/string_array.go create mode 100644 internal/database/migration_20240221_0000/migration.go diff --git a/integration-tests/features/invitation-api.feature b/integration-tests/features/invitation-api.feature index 4865823ad..f0d8dc898 100644 --- a/integration-tests/features/invitation-api.feature +++ b/integration-tests/features/invitation-api.feature @@ -37,6 +37,7 @@ Feature: Invitations API "expires_at": "${response.expires_at}", "id": "${invitation_id}", "organization_id": "${thompson_user_id}", + "roles": ["member"], "email": "${johnson_user_id}@redhat.com", "user_id": "${johnson_user_id}" } @@ -102,6 +103,7 @@ Feature: Invitations API "expires_at": "${response.expires_at}", "id": "${invitation_id}", "organization_id": "${thompson_organization_id}", + "roles": ["member"], "user_id": "${johnson_user_id}" } """ @@ -123,11 +125,11 @@ Feature: Invitations API "picture": "", "username": "${thompson_username}" }, + "roles": ["member"], "organization": { "description": "${thompson_username}'s organization", "id": "${thompson_organization_id}", - "name": "${thompson_username}", - "owner_id": "${thompson_user_id}" + "name": "${thompson_username}" }, "user_id": "${johnson_user_id}" } @@ -150,11 +152,11 @@ Feature: Invitations API "picture": "", "username": "${thompson_username}" }, + "roles": ["member"], "organization": { "description": "${thompson_username}'s organization", "id": "${thompson_organization_id}", - "name": "${thompson_username}", - "owner_id": "${thompson_user_id}" + "name": "${thompson_username}" }, "user_id": "${johnson_user_id}" } diff --git a/integration-tests/features/organization-api.feature b/integration-tests/features/organization-api.feature index e848621ff..1ff8a8e44 100644 --- a/integration-tests/features/organization-api.feature +++ b/integration-tests/features/organization-api.feature @@ -42,8 +42,7 @@ Feature: Organization API { "description": "${oscar_username}'s organization", "id": "${oscar_organization_id}", - "name": "${oscar_username}", - "owner_id": "${oscar_user_id}" + "name": "${oscar_username}" } ] """ diff --git a/integration-tests/features/user-organization-api.feature b/integration-tests/features/user-organization-api.feature index e3412ff54..8d5532d52 100644 --- a/integration-tests/features/user-organization-api.feature +++ b/integration-tests/features/user-organization-api.feature @@ -42,6 +42,7 @@ Feature: Invitations API [{ "organization_id": "${brent_organization.id}", "user_id": "${brent_user.id}", + "roles": ["owner"], "user": ${brent_user} }] """ @@ -106,14 +107,17 @@ Feature: Invitations API [{ "organization_id": "${brent_organization.id}", "user_id": "${anil_user.id}", + "roles": ["member"], "user": ${anil_user} }, { "organization_id": "${brent_organization.id}", "user_id": "${brent_user.id}", + "roles": ["owner"], "user": ${brent_user} }, { "organization_id": "${brent_organization.id}", "user_id": "${russel_user.id}", + "roles": ["member"], "user": ${russel_user} }] """ @@ -150,6 +154,7 @@ Feature: Invitations API { "organization_id": "${brent_organization.id}", "user_id": "${anil_user.id}", + "roles": ["member"], "user": ${anil_user} } """ @@ -162,6 +167,7 @@ Feature: Invitations API { "organization_id": "${brent_organization.id}", "user_id": "${anil_user.id}", + "roles": ["member"], "user": ${anil_user} } """ @@ -173,10 +179,12 @@ Feature: Invitations API [{ "organization_id": "${brent_organization.id}", "user_id": "${brent_user.id}", + "roles": ["owner"], "user": ${brent_user} }, { "organization_id": "${brent_organization.id}", "user_id": "${russel_user.id}", + "roles": ["member"], "user": ${russel_user} }] """ \ No newline at end of file diff --git a/internal/api/public/model_models_add_invitation.go b/internal/api/public/model_models_add_invitation.go index 0f28fa3cb..937bd5474 100644 --- a/internal/api/public/model_models_add_invitation.go +++ b/internal/api/public/model_models_add_invitation.go @@ -13,8 +13,9 @@ package public // ModelsAddInvitation struct for ModelsAddInvitation type ModelsAddInvitation struct { // The email address of the user to invite (one of email or user_id is required) - Email string `json:"email,omitempty"` - OrganizationId string `json:"organization_id,omitempty"` + Email string `json:"email,omitempty"` + OrganizationId string `json:"organization_id,omitempty"` + Roles []string `json:"roles,omitempty"` // The user id to invite (one of email or user_id is required) UserId string `json:"user_id,omitempty"` } diff --git a/internal/api/public/model_models_invitation.go b/internal/api/public/model_models_invitation.go index 1015fecaf..664cf274f 100644 --- a/internal/api/public/model_models_invitation.go +++ b/internal/api/public/model_models_invitation.go @@ -19,5 +19,6 @@ type ModelsInvitation struct { Id string `json:"id,omitempty"` Organization ModelsOrganization `json:"organization,omitempty"` OrganizationId string `json:"organization_id,omitempty"` + Roles []string `json:"roles,omitempty"` UserId string `json:"user_id,omitempty"` } diff --git a/internal/api/public/model_models_organization.go b/internal/api/public/model_models_organization.go index 4acadcbec..442714a57 100644 --- a/internal/api/public/model_models_organization.go +++ b/internal/api/public/model_models_organization.go @@ -15,5 +15,4 @@ type ModelsOrganization struct { Description string `json:"description,omitempty"` Id string `json:"id,omitempty"` Name string `json:"name,omitempty"` - OwnerId string `json:"owner_id,omitempty"` } diff --git a/internal/api/public/model_models_user_organization.go b/internal/api/public/model_models_user_organization.go index 8fa163f92..14e4fc179 100644 --- a/internal/api/public/model_models_user_organization.go +++ b/internal/api/public/model_models_user_organization.go @@ -13,6 +13,7 @@ package public // ModelsUserOrganization struct for ModelsUserOrganization type ModelsUserOrganization struct { OrganizationId string `json:"organization_id,omitempty"` + Roles []string `json:"roles,omitempty"` User ModelsUser `json:"user,omitempty"` UserId string `json:"user_id,omitempty"` } diff --git a/internal/database/database.go b/internal/database/database.go index 2140db719..dac0b45aa 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -14,6 +14,7 @@ import ( _ "github.com/nexodus-io/nexodus/internal/database/migration_20231130_0000" _ "github.com/nexodus-io/nexodus/internal/database/migration_20231206_0000" _ "github.com/nexodus-io/nexodus/internal/database/migration_20231211_0000" + _ "github.com/nexodus-io/nexodus/internal/database/migration_20240221_0000" "sort" "github.com/cenkalti/backoff/v4" diff --git a/internal/database/datatype/string_array.go b/internal/database/datatype/string_array.go new file mode 100644 index 000000000..55863939a --- /dev/null +++ b/internal/database/datatype/string_array.go @@ -0,0 +1,81 @@ +package datatype + +import ( + "context" + "database/sql/driver" + "encoding/json" + "errors" + "fmt" + "github.com/lib/pq" + "gorm.io/gorm" + "gorm.io/gorm/clause" + "gorm.io/gorm/schema" +) + +type StringArray []string + +// GormDataType gorm common data type +func (StringArray) GormDataType() string { + return "string_array" +} + +// GormDBDataType gorm db data type +func (StringArray) GormDBDataType(db *gorm.DB, field *schema.Field) string { + switch db.Dialector.Name() { + case "postgres": + return "text[]" + default: + return "JSON" + } +} + +// Value return json value, implement driver.Valuer interface +func (j StringArray) Value() (driver.Value, error) { + return json.Marshal(j) +} + +func (j StringArray) GormValue(ctx context.Context, db *gorm.DB) clause.Expr { + switch db.Dialector.Name() { + case "postgres": + return gorm.Expr("?", pq.StringArray(j)) + default: + data, err := json.Marshal(j) + if err != nil { + db.Error = err + } + return gorm.Expr("?", string(data)) + } +} + +// implements sql.Scanner interface +func (j *StringArray) Scan(value interface{}) error { + var bytes []byte + switch v := value.(type) { + case []byte: + bytes = v + case string: + bytes = []byte(v) + default: + return errors.New(fmt.Sprint("Failed to unmarshal string array value:", value)) + } + + if len(bytes) == 0 { + *j = nil + return nil + } + if bytes[0] == '[' { + err := json.Unmarshal(bytes, j) + return err + } + if bytes[0] == '{' { + // Unmarshal as Postgres text array + var a pq.StringArray + err := a.Scan(value) + if err != nil { + return err + } + *j = StringArray(a) + return nil + } + return errors.New(fmt.Sprint("Failed to unmarshal string array value:", value)) +} diff --git a/internal/database/migration_20240221_0000/migration.go b/internal/database/migration_20240221_0000/migration.go new file mode 100644 index 000000000..6141ca130 --- /dev/null +++ b/internal/database/migration_20240221_0000/migration.go @@ -0,0 +1,111 @@ +package migration_20240221_0000 + +import ( + "github.com/google/uuid" + "github.com/lib/pq" + "github.com/nexodus-io/nexodus/internal/database/datatype" + . "github.com/nexodus-io/nexodus/internal/database/migrations" + "github.com/nexodus-io/nexodus/internal/models" + "gorm.io/gorm" +) + +type UserOrganization struct { + Roles datatype.StringArray `json:"roles" swaggertype:"array,string"` +} +type Invitation struct { + Roles datatype.StringArray `json:"roles" swaggertype:"array,string"` +} + +func init() { + migrationId := "20240221-0000" + CreateMigrationFromActions(migrationId, + AddTableColumnsAction(&UserOrganization{}), + AddTableColumnsAction(&Invitation{}), + ExecActionIf( + `CREATE INDEX IF NOT EXISTS "idx_user_organizations_roles" ON "user_organizations" USING GIN ("roles")`, + `DROP INDEX IF EXISTS idx_user_organizations_roles`, + NotOnSqlLite, + ), + + // While inspecting the DB I realized a bunch of id columns are not uuids... this should fix that + ChangeColumnTypeActionIf(`devices`, `owner_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`devices`, `vpc_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`devices`, `organization_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`devices`, `security_group_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`devices`, `reg_key_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`organizations`, `owner_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`reg_keys`, `owner_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`reg_keys`, `vpc_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`reg_keys`, `organization_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`reg_keys`, `device_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`reg_keys`, `security_group_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`security_groups`, `organization_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`security_groups`, `vpc_id`, `text`, `uuid`, NotOnSqlLite), + ChangeColumnTypeActionIf(`vpcs`, `organization_id`, `text`, `uuid`, NotOnSqlLite), + + func(tx *gorm.DB, apply bool) error { + if apply && NotOnSqlLite(tx) { + + // this will fill in the role for the user_organizations table + type UserOrganization struct { + UserID uuid.UUID `json:"user_id" gorm:"type:uuid;primary_key"` + OrganizationID uuid.UUID `json:"organization_id" gorm:"type:uuid;primary_key"` + Roles pq.StringArray `json:"roles" gorm:"type:text[]" swaggertype:"array,string"` + } + + result := tx.Model(&models.UserOrganization{}). + Where("roles IS NULL"). + Update("roles", pq.StringArray{"member"}) + if result.Error != nil { + return result.Error + } + + type Organization struct { + ID uuid.UUID + OwnerID uuid.UUID + } + rows := []Organization{} + + // make all sure all orgs have a member with the owner role + sql := `SELECT DISTINCT id, owner_id FROM organizations LEFT JOIN user_organizations ON user_organizations.organization_id = organizations.id WHERE organization_id is NULL` + result = tx.Raw(sql).FindInBatches(&rows, 100, func(tx *gorm.DB, batch int) error { + for _, r := range rows { + result := tx.Create(&UserOrganization{ + UserID: r.OwnerID, + OrganizationID: r.ID, + Roles: []string{"owner"}, + }) + if result.Error != nil { + return result.Error + } + } + return nil + }) + if result.Error != nil { + return result.Error + } + + // make sure the owner's memberhip has the owner role + sql = `select * FROM organizations LEFT JOIN user_organizations ON user_organizations.organization_id = organizations.id WHERE organizations.owner_id = user_organizations.user_id AND roles <> '{owner}'` + result = tx.Raw(sql).FindInBatches(&rows, 100, func(tx *gorm.DB, batch int) error { + for _, r := range rows { + result := tx.Save(&UserOrganization{ + UserID: r.OwnerID, + OrganizationID: r.ID, + Roles: []string{"owner"}, + }) + if result.Error != nil { + return result.Error + } + } + return nil + }) + if result.Error != nil { + return result.Error + } + + } + return nil + }, + ) +} diff --git a/internal/database/migrations/migrations.go b/internal/database/migrations/migrations.go index 1e842da64..0dea83d92 100644 --- a/internal/database/migrations/migrations.go +++ b/internal/database/migrations/migrations.go @@ -154,6 +154,19 @@ func DropTableAction(table interface{}) MigrationAction { return nil } } +func ChangeColumnTypeAction(table string, columnName string, oldType string, newType string) MigrationAction { + return ExecAction( + fmt.Sprintf(`ALTER TABLE %s ALTER COLUMN %s TYPE %s USING %s::%s`, table, columnName, newType, columnName, newType), + fmt.Sprintf(`ALTER TABLE %s ALTER COLUMN %s TYPE %s USING %s::%s`, table, columnName, oldType, columnName, oldType), + ) +} +func ChangeColumnTypeActionIf(table string, columnName string, oldType string, newType string, condition func(tx *gorm.DB) bool) MigrationAction { + return ExecActionIf( + fmt.Sprintf(`ALTER TABLE %s ALTER COLUMN %s TYPE %s USING %s::%s`, table, columnName, newType, columnName, newType), + fmt.Sprintf(`ALTER TABLE %s ALTER COLUMN %s TYPE %s USING %s::%s`, table, columnName, oldType, columnName, oldType), + condition, + ) +} func AddTableColumnsAction(table interface{}) MigrationAction { caller := "" diff --git a/internal/docs/docs.go b/internal/docs/docs.go index 2df480113..00f4b67c7 100644 --- a/internal/docs/docs.go +++ b/internal/docs/docs.go @@ -3389,6 +3389,12 @@ const docTemplate = `{ "organization_id": { "type": "string" }, + "roles": { + "type": "array", + "items": { + "type": "string" + } + }, "user_id": { "description": "The user id to invite (one of email or user_id is required)", "type": "string" @@ -3726,6 +3732,12 @@ const docTemplate = `{ "organization_id": { "type": "string" }, + "roles": { + "type": "array", + "items": { + "type": "string" + } + }, "user_id": { "type": "string" } @@ -3810,10 +3822,6 @@ const docTemplate = `{ "name": { "type": "string", "example": "zone-red" - }, - "owner_id": { - "type": "string", - "example": "aa22666c-0f57-45cb-a449-16efecc04f2e" } } }, @@ -4128,6 +4136,12 @@ const docTemplate = `{ "organization_id": { "type": "string" }, + "roles": { + "type": "array", + "items": { + "type": "string" + } + }, "user": { "$ref": "#/definitions/models.User" }, diff --git a/internal/docs/swagger.json b/internal/docs/swagger.json index fc9c2fdb3..6413e62e0 100644 --- a/internal/docs/swagger.json +++ b/internal/docs/swagger.json @@ -3382,6 +3382,12 @@ "organization_id": { "type": "string" }, + "roles": { + "type": "array", + "items": { + "type": "string" + } + }, "user_id": { "description": "The user id to invite (one of email or user_id is required)", "type": "string" @@ -3719,6 +3725,12 @@ "organization_id": { "type": "string" }, + "roles": { + "type": "array", + "items": { + "type": "string" + } + }, "user_id": { "type": "string" } @@ -3803,10 +3815,6 @@ "name": { "type": "string", "example": "zone-red" - }, - "owner_id": { - "type": "string", - "example": "aa22666c-0f57-45cb-a449-16efecc04f2e" } } }, @@ -4121,6 +4129,12 @@ "organization_id": { "type": "string" }, + "roles": { + "type": "array", + "items": { + "type": "string" + } + }, "user": { "$ref": "#/definitions/models.User" }, diff --git a/internal/docs/swagger.yaml b/internal/docs/swagger.yaml index 6b53d2215..78705c1c0 100644 --- a/internal/docs/swagger.yaml +++ b/internal/docs/swagger.yaml @@ -41,6 +41,10 @@ definitions: type: string organization_id: type: string + roles: + items: + type: string + type: array user_id: description: The user id to invite (one of email or user_id is required) type: string @@ -322,6 +326,10 @@ definitions: $ref: '#/definitions/models.Organization' organization_id: type: string + roles: + items: + type: string + type: array user_id: type: string type: object @@ -394,9 +402,6 @@ definitions: name: example: zone-red type: string - owner_id: - example: aa22666c-0f57-45cb-a449-16efecc04f2e - type: string type: object models.RegKey: properties: @@ -616,6 +621,10 @@ definitions: properties: organization_id: type: string + roles: + items: + type: string + type: array user: $ref: '#/definitions/models.User' user_id: diff --git a/internal/handlers/invitations.go b/internal/handlers/invitations.go index 8a80e1164..4794ebbec 100644 --- a/internal/handlers/invitations.go +++ b/internal/handlers/invitations.go @@ -3,8 +3,10 @@ package handlers import ( "errors" "github.com/nexodus-io/nexodus/internal/database" + "github.com/nexodus-io/nexodus/internal/util" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "golang.org/x/exp/maps" "net/http" "strings" "time" @@ -15,6 +17,11 @@ import ( "gorm.io/gorm" ) +var allowedRoles = map[string]struct{}{ + "member": {}, + "owner": {}, +} + // CreateInvitation creates an invitation // @Summary Create an invitation // @Description Create an invitation to an organization @@ -50,6 +57,13 @@ func (api *API) CreateInvitation(c *gin.Context) { if request.UserID != nil && request.Email != nil { c.JSON(http.StatusBadRequest, models.NewFieldNotPresentError("both email and user_id present")) } + if len(util.FilterOutAllowed(request.Roles, allowedRoles)) > 0 { + c.JSON(http.StatusBadRequest, models.NewFieldValidationError("roles", "allowed values are: "+strings.Join(maps.Keys(allowedRoles), ", "))) + return + } + if len(request.Roles) == 0 { + request.Roles = []string{"member"} + } db := api.db.WithContext(ctx) @@ -67,6 +81,7 @@ func (api *API) CreateInvitation(c *gin.Context) { OrganizationID: request.OrganizationID, ExpiresAt: expiry, UserID: request.UserID, + Roles: request.Roles, } var user models.User @@ -195,12 +210,13 @@ func (api *API) InvitationIsForCurrentUser(c *gin.Context, db *gorm.DB) *gorm.DB func (api *API) InvitationIsForCurrentUserOrOrgOwner(c *gin.Context, db *gorm.DB) *gorm.DB { userId := api.GetCurrentUserID(c) + allowedRoles := []string{"owner"} // this could potentially be driven by rego output if api.dialect == database.DialectSqlLite { - return db.Where("user_id = ? OR organization_id in (SELECT id FROM organizations where owner_id=?)", userId, userId) + return db.Where("user_id = ? OR organization_id in (SELECT DISTINCT organization_id FROM user_organizations, json_each(roles) AS role where user_id=? AND role.value IN (?))", userId, userId, allowedRoles) } else { - return db.Where("user_id = ? OR organization_id::text in (SELECT id::text FROM organizations where owner_id=?)", userId, userId) + return db.Where("user_id = ? OR organization_id in (SELECT DISTINCT organization_id FROM user_organizations where user_id=? AND (roles && ?))", userId, userId, models.StringArray(allowedRoles)) } } @@ -283,17 +299,18 @@ func (api *API) AcceptInvitation(c *gin.Context) { First(&invitation, "id = ?", id); res.Error != nil { return errInvitationNotFound } + var user models.User - if res := tx.Preload("Organizations").First(&user, "id = ?", invitation.UserID); res.Error != nil { + if res := tx.First(&user, "id = ?", invitation.UserID); res.Error != nil { return errUserNotFound } - var org models.Organization - if res := tx.First(&org, "id = ?", invitation.OrganizationID); res.Error != nil { - return errOrgNotFound + userOrganization := models.UserOrganization{ + UserID: user.ID, + OrganizationID: invitation.OrganizationID, + Roles: invitation.Roles, } - user.Organizations = append(user.Organizations, &org) - if res := tx.Save(&user); res.Error != nil { + if res := tx.Create(&userOrganization); res.Error != nil { return res.Error } if res := tx.Delete(&invitation); res.Error != nil { diff --git a/internal/handlers/organization.go b/internal/handlers/organization.go index e8e6ef871..98531f2c8 100644 --- a/internal/handlers/organization.go +++ b/internal/handlers/organization.go @@ -68,7 +68,6 @@ func (api *API) CreateOrganization(c *gin.Context) { org = models.Organization{ Name: request.Name, - OwnerID: userId, Description: request.Description, } @@ -80,6 +79,18 @@ func (api *API) CreateOrganization(c *gin.Context) { return res.Error } + if res := tx.Create(&models.UserOrganization{ + UserID: userId, + OrganizationID: org.ID, + Roles: []string{"owner"}, + }); res.Error != nil { + if database.IsDuplicateError(res.Error) { + return errDuplicateOrganization{ID: org.ID.String()} + } + api.logger.Error("Failed to create organization owner: ", res.Error) + return res.Error + } + span.SetAttributes(attribute.String("id", org.ID.String())) api.logger.Infof("New organization request [ %s ] request", org.Name) return nil @@ -102,19 +113,22 @@ func (api *API) CreateOrganization(c *gin.Context) { func (api *API) OrganizationIsReadableByCurrentUser(c *gin.Context, db *gorm.DB) *gorm.DB { userId := api.GetCurrentUserID(c) - - // this could potentially be driven by rego output + allowedRoles := []string{"owner", "member"} if api.dialect == database.DialectSqlLite { - return db.Where("owner_id = ? OR id in (SELECT organization_id FROM user_organizations where user_id=?)", userId, userId) + return db.Where("id in (SELECT DISTINCT organization_id FROM user_organizations, json_each(roles) AS role where user_id=? AND role.value IN (?))", userId, allowedRoles) } else { - return db.Where("owner_id = ? OR id::text in (SELECT organization_id::text FROM user_organizations where user_id=?)", userId, userId) + return db.Where("id in (SELECT DISTINCT organization_id FROM user_organizations where user_id=? AND (roles && ?))", userId, models.StringArray(allowedRoles)) } } func (api *API) OrganizationIsOwnedByCurrentUser(c *gin.Context, db *gorm.DB) *gorm.DB { userId := api.GetCurrentUserID(c) - // this could potentially be driven by rego output - return db.Where("owner_id = ?", userId) + allowedRoles := []string{"owner"} + if api.dialect == database.DialectSqlLite { + return db.Where("id in (SELECT DISTINCT organization_id FROM user_organizations, json_each(roles) AS role where user_id=? AND role.value IN (?))", userId, allowedRoles) + } else { + return db.Where("id in (SELECT DISTINCT organization_id FROM user_organizations where user_id=? AND (roles && ?))", userId, models.StringArray(allowedRoles)) + } } // ListOrganizations lists all Organizations @@ -229,7 +243,7 @@ func (api *API) DeleteOrganization(c *gin.Context) { var org models.Organization err = api.transaction(ctx, func(tx *gorm.DB) error { - result := api.OrganizationIsReadableByCurrentUser(c, tx). + result := api.OrganizationIsOwnedByCurrentUser(c, tx). First(&org, "id = ?", orgID) if result.Error != nil { if errors.Is(result.Error, gorm.ErrRecordNotFound) { @@ -239,8 +253,13 @@ func (api *API) DeleteOrganization(c *gin.Context) { } } - if org.ID == org.OwnerID { + user := models.User{} + result = tx.Unscoped().First(&user, "id = ?", orgID) + if result.Error == nil { + // found a user with the same id, it's a user's default org... return NewApiResponseError(http.StatusBadRequest, models.NewNotAllowedError("default organization cannot be deleted")) + } else if !errors.Is(result.Error, gorm.ErrRecordNotFound) { + return result.Error } return deleteOrganization(tx, orgID) diff --git a/internal/handlers/security_group.go b/internal/handlers/security_group.go index 68fa9e2c6..0910badb2 100644 --- a/internal/handlers/security_group.go +++ b/internal/handlers/security_group.go @@ -54,19 +54,23 @@ func (d securityGroupList) Len() int { func (api *API) SecurityGroupIsReadableByCurrentUser(c *gin.Context, db *gorm.DB) *gorm.DB { userId := api.GetCurrentUserID(c) + allowedRoles := []string{"owner", "member"} if api.dialect == database.DialectSqlLite { - return db.Where("organization_id in (SELECT organization_id FROM user_organizations where user_id=?) OR organization_id in (SELECT id FROM organizations where owner_id=?)", userId, userId) + //return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations where user_id=?) OR organization_id in (SELECT id FROM organizations where owner_id=?)", userId, userId) + return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations, json_each(roles) AS role where user_id=? AND role.value IN (?))", userId, allowedRoles) } else { - return db.Where("organization_id::text in (SELECT organization_id::text FROM user_organizations where user_id=?) OR organization_id::text in (SELECT id::text FROM organizations where owner_id=?)", userId, userId) + return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations where user_id=? AND (roles && ?))", userId, models.StringArray(allowedRoles)) } } func (api *API) SecurityGroupIsWriteableByCurrentUser(c *gin.Context, db *gorm.DB) *gorm.DB { userId := api.GetCurrentUserID(c) + allowedRoles := []string{"owner"} if api.dialect == database.DialectSqlLite { - return db.Where("organization_id in (SELECT id FROM organizations where owner_id=?)", userId) + //return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations where user_id=?) OR organization_id in (SELECT id FROM organizations where owner_id=?)", userId, userId) + return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations, json_each(roles) AS role where user_id=? AND role.value IN (?))", userId, allowedRoles) } else { - return db.Where("organization_id::text in (SELECT id::text FROM organizations where owner_id=?)", userId) + return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations where user_id=? AND (roles && ?))", userId, models.StringArray(allowedRoles)) } } diff --git a/internal/handlers/user.go b/internal/handlers/user.go index e02ba2137..5acc6c803 100644 --- a/internal/handlers/user.go +++ b/internal/handlers/user.go @@ -16,7 +16,6 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "gorm.io/gorm" - "gorm.io/gorm/clause" ) // key for username in gin.Context @@ -179,14 +178,8 @@ func (api *API) CreateUserIfNotExists(ctx context.Context, idpId string, userNam Base: models.Base{ ID: user.ID, }, - OwnerID: user.ID, Name: userName, Description: fmt.Sprintf("%s's organization", userName), - Users: []*models.User{{ - Base: models.Base{ - ID: user.ID, - }, - }}, }); res.Error != nil { if database.IsDuplicateError(res.Error) { res.Error = gorm.ErrDuplicatedKey @@ -194,6 +187,15 @@ func (api *API) CreateUserIfNotExists(ctx context.Context, idpId string, userNam return res.Error } + // Makes the user the owner of the organization + if res := tx.Create(&models.UserOrganization{ + UserID: user.ID, + OrganizationID: user.ID, + Roles: []string{"owner"}, + }); res.Error != nil { + return res.Error + } + // Create the default vpc if res := tx.Create(&models.VPC{ Base: models.Base{ @@ -349,17 +351,48 @@ func (api *API) DeleteUser(c *gin.Context) { return result.Error } - // We need to delete all the organizations that the user owns - orgs := []models.Organization{} - if res := tx.Where("owner_id = ?", userId). - Find(&orgs); res.Error != nil && !errors.Is(res.Error, gorm.ErrRecordNotFound) { + // find the organizations the user is an owner of + ownerRole := []string{"owner"} + res := tx + if api.dialect == database.DialectSqlLite { + res = tx.Where("user_id = ? AND EXISTS (SELECT * FROM json_each(roles) AS role WHERE role.value IN ?)", userId, ownerRole) + } else { + res = tx.Where("user_id = ? AND (roles && ?)", userId, models.StringArray(ownerRole)) + } + + userOrgs := []models.UserOrganization{} + if res = res.Find(&userOrgs); res.Error != nil && !errors.Is(res.Error, gorm.ErrRecordNotFound) { return result.Error } - for _, org := range orgs { - err := deleteOrganization(tx, org.ID) - if err != nil { - return err + for _, org := range userOrgs { + + // check if the user is the only owner of the organization + res := tx.Model(&models.UserOrganization{}) + if api.dialect == database.DialectSqlLite { + where := "organization_id = ? AND EXISTS (SELECT * FROM json_each(roles) AS role WHERE role.value IN ?)" + res = res.Where(where, org.OrganizationID, ownerRole) + } else { + where := "organization_id = ? AND (roles && ?)" + res = res.Where(where, org.OrganizationID, models.StringArray(ownerRole)) + } + var count int64 + if res = res.Count(&count); res.Error != nil { + return result.Error + } + + // if the user is the only owner, delete the organization + if count <= 1 { + err := deleteOrganization(tx, org.OrganizationID) + if err != nil { + return err + } + } else { + // remove the user from the organization + if res := tx.Where("user_id = ?", userId). + Delete(&models.UserOrganization{}); res.Error != nil { + return result.Error + } } } @@ -402,11 +435,6 @@ func (api *API) DeleteUser(c *gin.Context) { c.JSON(http.StatusOK, user) } -type UserOrganization struct { - UserID string `json:"user_id"` - OrganizationID uuid.UUID `json:"organization_id"` -} - // DeleteUserFromOrganization removes a user from an organization // @Summary Remove a User from an Organization // @Description Deletes an existing organization associated to a user @@ -447,10 +475,9 @@ func (api *API) DeleteUserFromOrganization(c *gin.Context) { return errOrgNotFound } if res := api.db. - Select(clause.Associations). Where("user_id = ?", userID). Where("organization_id = ?", orgID). - Delete(&UserOrganization{}); res.Error != nil { + Delete(&models.UserOrganization{}); res.Error != nil { api.SendInternalServerError(c, fmt.Errorf("failed to remove the association from the user_organizations table: %w", res.Error)) } return nil diff --git a/internal/handlers/vpc.go b/internal/handlers/vpc.go index 6d6386189..e1b01ed10 100644 --- a/internal/handlers/vpc.go +++ b/internal/handlers/vpc.go @@ -156,19 +156,22 @@ func (api *API) CreateVPC(c *gin.Context) { func (api *API) VPCIsReadableByCurrentUser(c *gin.Context, db *gorm.DB) *gorm.DB { userId := api.GetCurrentUserID(c) + + allowedRoles := []string{"owner", "member"} if api.dialect == database.DialectSqlLite { - return db.Where("organization_id in (SELECT organization_id FROM user_organizations where user_id=?)", userId) + return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations, json_each(roles) AS role where user_id=? AND role.value IN (?))", userId, allowedRoles) } else { - return db.Where("organization_id::text in (SELECT organization_id::text FROM user_organizations where user_id=?)", userId) + return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations where user_id=? AND (roles && ?))", userId, models.StringArray(allowedRoles)) } } func (api *API) VPCIsOwnedByCurrentUser(c *gin.Context, db *gorm.DB) *gorm.DB { userId := api.GetCurrentUserID(c) + allowedRoles := []string{"owner"} if api.dialect == database.DialectSqlLite { - return db.Where("organization_id in (SELECT id FROM organizations where owner_id=?)", userId) + return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations, json_each(roles) AS role where user_id=? AND role.value IN (?))", userId, allowedRoles) } else { - return db.Where("organization_id::text in (SELECT id::text FROM organizations where owner_id=?)", userId) + return db.Where("organization_id in (SELECT DISTINCT organization_id FROM user_organizations where user_id=? AND (roles && ?))", userId, models.StringArray(allowedRoles)) } } diff --git a/internal/models/base.go b/internal/models/base.go index 4f6c91ff7..5ff26d110 100644 --- a/internal/models/base.go +++ b/internal/models/base.go @@ -1,12 +1,15 @@ package models import ( + "github.com/nexodus-io/nexodus/internal/database/datatype" "time" "github.com/google/uuid" "gorm.io/gorm" ) +type StringArray = datatype.StringArray + // Base contains common columns for all tables. type Base struct { ID uuid.UUID `gorm:"type:uuid;primary_key;" json:"id" example:"aa22666c-0f57-45cb-a449-16efecc04f2e"` diff --git a/internal/models/invitation.go b/internal/models/invitation.go index 1dfbbee21..b2ff8eaea 100644 --- a/internal/models/invitation.go +++ b/internal/models/invitation.go @@ -16,10 +16,12 @@ type Invitation struct { ExpiresAt time.Time `json:"expires_at"` FromID uuid.UUID `json:"-"` From *User `json:"from,omitempty"` + Roles StringArray `json:"roles" swaggertype:"array,string"` } type AddInvitation struct { Email *string `json:"email"` // The email address of the user to invite (one of email or user_id is required) UserID *uuid.UUID `json:"user_id"` // The user id to invite (one of email or user_id is required) OrganizationID uuid.UUID `json:"organization_id"` + Roles []string `json:"roles"` } diff --git a/internal/models/organization.go b/internal/models/organization.go index 5d12f5e5b..0c1b155e1 100644 --- a/internal/models/organization.go +++ b/internal/models/organization.go @@ -1,16 +1,14 @@ package models import ( - "github.com/google/uuid" "gorm.io/gorm" ) // Organization contains Users and VPCs type Organization struct { Base - OwnerID uuid.UUID `json:"owner_id" example:"aa22666c-0f57-45cb-a449-16efecc04f2e"` - Name string `json:"name" gorm:"uniqueIndex" sql:"index" example:"zone-red"` - Description string `json:"description" example:"Team A"` + Name string `json:"name" gorm:"uniqueIndex" sql:"index" example:"zone-red"` + Description string `json:"description" example:"Team A"` Users []*User `json:"-" gorm:"many2many:user_organizations;"` Invitations []*Invitation `json:"-"` diff --git a/internal/models/user_organization.go b/internal/models/user_organization.go index 84ce649e6..286d3896c 100644 --- a/internal/models/user_organization.go +++ b/internal/models/user_organization.go @@ -1,10 +1,13 @@ package models -import "github.com/google/uuid" +import ( + "github.com/google/uuid" +) // UserOrganization record means the user is a member of the organization type UserOrganization struct { - UserID uuid.UUID `json:"user_id" gorm:"type:uuid;primary_key"` - OrganizationID uuid.UUID `json:"organization_id" gorm:"type:uuid;primary_key"` - User *User `json:"user,omitempty"` + UserID uuid.UUID `json:"user_id" gorm:"type:uuid;primary_key"` + OrganizationID uuid.UUID `json:"organization_id" gorm:"type:uuid;primary_key"` + User *User `json:"user,omitempty"` + Roles StringArray `json:"roles" swaggertype:"array,string"` } diff --git a/internal/util/string.go b/internal/util/string.go index ff40f16ed..42109647b 100644 --- a/internal/util/string.go +++ b/internal/util/string.go @@ -1,3 +1,12 @@ package util func PtrString(v string) *string { return &v } + +func FilterOutAllowed(elems []string, allowed map[string]struct{}) (notAllowed []string) { + for _, e := range elems { + if _, allow := allowed[e]; !allow { + notAllowed = append(notAllowed, e) + } + } + return +} From 6d700543008a60d41ba34c2b744213ecab769858 Mon Sep 17 00:00:00 2001 From: Hiram Chirino Date: Fri, 23 Feb 2024 13:15:12 -0500 Subject: [PATCH 2/2] nexctl/frontend: support setting/viewing invitation/org roles Signed-off-by: Hiram Chirino --- cmd/nexctl/invitation.go | 14 ++++++++++ ui/src/components/StringListField.tsx | 25 +++++++++++++++++ ui/src/pages/Invitations.tsx | 39 +++++++++++++++++++++++++++ ui/src/pages/Organizations.tsx | 6 ----- 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 ui/src/components/StringListField.tsx diff --git a/cmd/nexctl/invitation.go b/cmd/nexctl/invitation.go index 79f8ac7c6..ff2dc1e44 100644 --- a/cmd/nexctl/invitation.go +++ b/cmd/nexctl/invitation.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/nexodus-io/nexodus/internal/api/public" "github.com/urfave/cli/v3" + "strings" ) func createInvitationCommand() *cli.Command { @@ -35,6 +36,12 @@ func createInvitationCommand() *cli.Command { Name: "organization-id", Required: false, }, + &cli.StringSliceFlag{ + Name: "role", + Required: false, + DefaultText: "member", + Value: []string{"member"}, + }, }, Action: func(ctx context.Context, command *cli.Command) error { organizationId, err := getUUID(command, "organization-id") @@ -45,10 +52,13 @@ func createInvitationCommand() *cli.Command { if err != nil { return err } + role := command.StringSlice("role") + return createInvitation(ctx, command, public.ModelsAddInvitation{ OrganizationId: organizationId, UserId: userId, Email: command.String("email"), + Roles: role, }) }, }, @@ -102,6 +112,10 @@ func invitationsTableFields() []TableField { return fmt.Sprintf("%s <%s>", inv.From.FullName, inv.From.Username) }}) fields = append(fields, TableField{Header: "EMAIL", Field: "Email"}) + fields = append(fields, TableField{Header: "ROLES", Formatter: func(item interface{}) string { + inv := item.(public.ModelsInvitation) + return strings.Join(inv.Roles, ", ") + }}) fields = append(fields, TableField{Header: "EXPIRES AT", Field: "ExpiresAt"}) return fields } diff --git a/ui/src/components/StringListField.tsx b/ui/src/components/StringListField.tsx new file mode 100644 index 000000000..c6b2cd1d4 --- /dev/null +++ b/ui/src/components/StringListField.tsx @@ -0,0 +1,25 @@ +import { + Identifier, + RaRecord, + RecordContextProvider, + useRecordContext, + UseRecordContextParams, +} from "react-admin"; + +export const StringListField = ( + props: UseRecordContextParams>, +) => { + const record = useRecordContext(props); + return ( +
+ {record[props.source].map((item: any) => { + const value = props.mappper ? props.mappper(item) : item; + return ( + + {props.children} + + ); + })} +
+ ); +}; diff --git a/ui/src/pages/Invitations.tsx b/ui/src/pages/Invitations.tsx index 639b96c57..f20ebdbb6 100644 --- a/ui/src/pages/Invitations.tsx +++ b/ui/src/pages/Invitations.tsx @@ -23,9 +23,27 @@ import { Identifier, DateTimeInput, AutocompleteInput, + CheckboxGroupInput, + ArrayField, + SingleFieldList, + ChipField, + WrapperField, } from "react-admin"; import { backend, fetchJson as apiFetchJson } from "../common/Api"; +import { StringListField } from "../components/StringListField"; + +export const roleChoices = [ + { id: "owner", name: "Owner" }, + { id: "member", name: "Member" }, +]; + +export function choiceMapper(choices: { id: string; name: string }[]) { + return function (x: any) { + const choice = choices.find((c) => c.id === x); + return choice ? choice.name : x; + }; +} const InvitationListBulkActions = () => ( @@ -97,6 +115,13 @@ export const InvitationList = () => ( + + + @@ -110,6 +135,13 @@ export const InvitationShow = () => ( + + + @@ -145,6 +177,13 @@ export const InvitationCreate = () => { source="expires_at" fullWidth /> + ); diff --git a/ui/src/pages/Organizations.tsx b/ui/src/pages/Organizations.tsx index 2ab78786d..badcb4d10 100644 --- a/ui/src/pages/Organizations.tsx +++ b/ui/src/pages/Organizations.tsx @@ -29,12 +29,6 @@ export const OrganizationList = () => ( > - );