From 415ce8db4390c8b53a0b68917f59c4e2f4d392ce Mon Sep 17 00:00:00 2001 From: Ben Visness Date: Sat, 11 Dec 2021 13:08:10 -0600 Subject: [PATCH] Rework project visibility --- src/admintools/adminproject.go | 2 +- src/hmndata/project_helper.go | 148 ++++++++++++++------------------- src/hmndata/tag_helper.go | 2 +- src/models/project.go | 11 ++- src/website/feed.go | 7 +- src/website/projects.go | 11 ++- src/website/routes.go | 11 ++- src/website/user.go | 13 ++- 8 files changed, 100 insertions(+), 105 deletions(-) diff --git a/src/admintools/adminproject.go b/src/admintools/adminproject.go index d878cb7..a215058 100644 --- a/src/admintools/adminproject.go +++ b/src/admintools/adminproject.go @@ -47,8 +47,8 @@ func addCreateProjectCommand(projectCommand *cobra.Command) { defer tx.Rollback(ctx) p, err := hmndata.FetchProject(ctx, tx, nil, models.HMNProjectID, hmndata.ProjectsQuery{ - IncludeHidden: true, Lifecycles: models.AllProjectLifecycles, + IncludeHidden: true, }) if err != nil { panic(err) diff --git a/src/hmndata/project_helper.go b/src/hmndata/project_helper.go index 1be1cf4..8f2b867 100644 --- a/src/hmndata/project_helper.go +++ b/src/hmndata/project_helper.go @@ -20,8 +20,9 @@ const ( ) type ProjectsQuery struct { - // Available on all project queries - Lifecycles []models.ProjectLifecycle // if empty, defaults to models.VisibleProjectLifecycles + // Available on all project queries. By default, you will get projects that + // are generally visible to all users. + Lifecycles []models.ProjectLifecycle // If empty, defaults to visible lifecycles. Do not conflate this with permissions; those are checked separately. Types ProjectTypeQuery // bitfield IncludeHidden bool @@ -65,11 +66,6 @@ func FetchProjects( perf.StartBlock("SQL", "Fetch projects") defer perf.EndBlock() - var currentUserID *int - if currentUser != nil { - currentUserID = ¤tUser.ID - } - tx, err := dbConn.Begin(ctx) if err != nil { return nil, oops.New(err, "failed to start transaction") @@ -80,11 +76,9 @@ func FetchProjects( Project models.Project `db:"project"` LogoLightAsset *models.Asset `db:"logolight_asset"` LogoDarkAsset *models.Asset `db:"logodark_asset"` + Tag *models.Tag `db:"tags"` } - // If true, join against the project owners table and check visibility permissions - checkOwnerVisibility := q.IncludeHidden && currentUser != nil - // Fetch all valid projects (not yet subject to user permission checks) var qb db.QueryBuilder if len(q.OrderBy) > 0 { @@ -96,31 +90,31 @@ func FetchProjects( handmade_project AS project LEFT JOIN handmade_asset AS logolight_asset ON logolight_asset.id = project.logolight_asset_id LEFT JOIN handmade_asset AS logodark_asset ON logodark_asset.id = project.logodark_asset_id + LEFT JOIN tags ON project.tag = tags.id `) if len(q.OwnerIDs) > 0 { - qb.Add(` - INNER JOIN handmade_user_projects AS owner_filter ON owner_filter.project_id = project.id - `) - } - if checkOwnerVisibility { - qb.Add(` - LEFT JOIN handmade_user_projects AS owner_visibility ON owner_visibility.project_id = project.id - `) + qb.Add( + ` + JOIN ( + SELECT project_id, array_agg(user_id) AS owner_ids + FROM handmade_user_projects + WHERE user_id = ANY ($?) + GROUP BY project_id + ) AS owner_filter ON project.id = owner_filter.project_id + `, + q.OwnerIDs, + ) } + + // Filters (permissions are checked after the query, in Go) qb.Add(` WHERE TRUE `) - - // Filters - if len(q.ProjectIDs) > 0 { - qb.Add(`AND project.id = ANY ($?)`, q.ProjectIDs) - } - if len(q.Slugs) > 0 { - qb.Add(`AND (project.slug != '' AND project.slug = ANY ($?))`, q.Slugs) - } - if len(q.OwnerIDs) > 0 { - qb.Add(`AND (owner_filter.user_id = ANY ($?))`, q.OwnerIDs) + if len(q.Lifecycles) > 0 { + qb.Add(`AND project.lifecycle = ANY ($?)`, q.Lifecycles) + } else { + qb.Add(`AND project.lifecycle = ANY ($?)`, models.VisibleProjectLifecycles) } if q.Types != 0 { qb.Add(`AND (FALSE`) @@ -132,21 +126,14 @@ func FetchProjects( } qb.Add(`)`) } - - // Visibility - if checkOwnerVisibility { - qb.Add(`AND ($? OR owner_visibility.user_id = $? OR (TRUE`, currentUser.IsStaff, currentUser.ID) - } if !q.IncludeHidden { - qb.Add(`AND NOT hidden`) + qb.Add(`AND NOT project.hidden`) } - if len(q.Lifecycles) > 0 { - qb.Add(`AND project.lifecycle = ANY($?)`, q.Lifecycles) - } else { - qb.Add(`AND project.lifecycle = ANY($?)`, models.VisibleProjectLifecycles) + if len(q.ProjectIDs) > 0 { + qb.Add(`AND project.id = ANY ($?)`, q.ProjectIDs) } - if checkOwnerVisibility { - qb.Add(`))`) + if len(q.Slugs) > 0 { + qb.Add(`AND (project.slug != '' AND project.slug = ANY ($?))`, q.Slugs) } // Output @@ -164,21 +151,6 @@ func FetchProjects( } iprojects := itProjects.ToSlice() - // Fetch project tags - var tagIDs []int - for _, iproject := range iprojects { - tagID := iproject.(*projectRow).Project.TagID - if tagID != nil { - tagIDs = append(tagIDs, *tagID) - } - } - tags, err := FetchTags(ctx, tx, TagQuery{ - IDs: tagIDs, - }) - if err != nil { - return nil, err - } - // Fetch project owners to do permission checks projectIds := make([]int, len(iprojects)) for i, iproject := range iprojects { @@ -195,49 +167,55 @@ func FetchProjects( owners := projectOwners[i].Owners /* - Per our spec, a user can see a project if: - - The project is official - - The project is personal and all of the project's owners are approved - - The project is personal and the current user is a collaborator (regardless of user status) + Here's the rundown on project permissions: + + - In general, users can only see projects that are Generally Visible. + - As an exception, users can always see projects that they own. + - As an exception, staff can always see every project. + + A project is Generally Visible if all the following conditions are true: + - The project has a "visible" lifecycle (per models.VisibleProjectLifecycles) + - The project is not hidden + - One of the following is true: + - The project is official + - The project is personal and all of the project's owners are approved + + As an exception, the HMN project is always generally visible. See https://www.notion.so/handmade-network/Technical-Plan-a11aaa9ea2f14d9a95f7d7780edd789c */ - var projectVisible bool - if row.Project.Personal { - allOwnersApproved := true - for _, owner := range owners { - if owner.Status != models.UserStatusApproved { - allOwnersApproved = false - } - if currentUserID != nil && *currentUserID == owner.ID { - projectVisible = true - } + currentUserIsOwner := false + allOwnersApproved := true + for _, owner := range owners { + if owner.Status != models.UserStatusApproved { + allOwnersApproved = false } - if allOwnersApproved { - projectVisible = true + if currentUser != nil && owner.ID == currentUser.ID { + currentUserIsOwner = true } - } else { - projectVisible = true } - if projectVisible { - var projectTag *models.Tag - if row.Project.TagID != nil { - for _, tag := range tags { - if tag.ID == *row.Project.TagID { - projectTag = tag - break - } - } - } + projectGenerallyVisible := true && + row.Project.Lifecycle.In(models.VisibleProjectLifecycles) && + !row.Project.Hidden && + (!row.Project.Personal || allOwnersApproved || row.Project.IsHMN()) + if row.Project.IsHMN() { + projectGenerallyVisible = true // hard override + } + projectVisible := false || + projectGenerallyVisible || + currentUserIsOwner || + (currentUser != nil && currentUser.IsStaff) + + if projectVisible { res = append(res, ProjectAndStuff{ Project: row.Project, LogoLightAsset: row.LogoLightAsset, LogoDarkAsset: row.LogoDarkAsset, Owners: owners, - Tag: projectTag, + Tag: row.Tag, }) } } @@ -485,8 +463,8 @@ func SetProjectTag( defer tx.Rollback(ctx) p, err := FetchProject(ctx, tx, nil, projectID, ProjectsQuery{ - IncludeHidden: true, Lifecycles: models.AllProjectLifecycles, + IncludeHidden: true, }) if err != nil { return nil, err diff --git a/src/hmndata/tag_helper.go b/src/hmndata/tag_helper.go index 1f10ff1..ca201b4 100644 --- a/src/hmndata/tag_helper.go +++ b/src/hmndata/tag_helper.go @@ -18,7 +18,7 @@ type TagQuery struct { func FetchTags(ctx context.Context, dbConn db.ConnOrTx, q TagQuery) ([]*models.Tag, error) { perf := perf.ExtractPerf(ctx) - perf.StartBlock("SQL", "Fetch snippets") + perf.StartBlock("SQL", "Fetch tags") defer perf.EndBlock() var qb db.QueryBuilder diff --git a/src/models/project.go b/src/models/project.go index c5ee726..e089b70 100644 --- a/src/models/project.go +++ b/src/models/project.go @@ -36,7 +36,7 @@ var AllProjectLifecycles = []ProjectLifecycle{ ProjectLifecycleLTS, } -// NOTE(asaf): Just checking the lifecycle is not sufficient. Visible projects also must have flags = 0. +// NOTE(asaf): Just checking the lifecycle is not sufficient. Visible projects also must not be hidden. var VisibleProjectLifecycles = []ProjectLifecycle{ ProjectLifecycleActive, ProjectLifecycleHiatus, @@ -44,6 +44,15 @@ var VisibleProjectLifecycles = []ProjectLifecycle{ ProjectLifecycleLTS, } +func (lc ProjectLifecycle) In(lcs []ProjectLifecycle) bool { + for _, v := range lcs { + if lc == v { + return true + } + } + return false +} + const RecentProjectUpdateTimespanSec = 60 * 60 * 24 * 28 // NOTE(asaf): Four weeks type Project struct { diff --git a/src/website/feed.go b/src/website/feed.go index ad5ca29..99a6bdd 100644 --- a/src/website/feed.go +++ b/src/website/feed.go @@ -157,10 +157,9 @@ func AtomFeed(c *RequestContext) ResponseData { itemsPerFeed = 100000 } projectsAndStuff, err := hmndata.FetchProjects(c.Context(), c.Conn, nil, hmndata.ProjectsQuery{ - Lifecycles: models.VisibleProjectLifecycles, - Limit: itemsPerFeed, - Types: hmndata.OfficialProjects, - OrderBy: "date_approved DESC", + Limit: itemsPerFeed, + Types: hmndata.OfficialProjects, + OrderBy: "date_approved DESC", }) if err != nil { return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "failed to fetch feed projects")) diff --git a/src/website/projects.go b/src/website/projects.go index 6101295..e4a9cb6 100644 --- a/src/website/projects.go +++ b/src/website/projects.go @@ -265,7 +265,10 @@ func ProjectHomepage(c *RequestContext) ResponseData { Value: c.CurrentProject.Blurb, }) - p, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, c.CurrentProject.ID, hmndata.ProjectsQuery{}) + p, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, c.CurrentProject.ID, hmndata.ProjectsQuery{ + Lifecycles: models.AllProjectLifecycles, + IncludeHidden: true, + }) if err != nil { return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "failed to fetch project details")) } @@ -491,16 +494,16 @@ func ProjectEdit(c *RequestContext) ResponseData { c.Context(), c.Conn, c.CurrentUser, c.CurrentProject.ID, hmndata.ProjectsQuery{ + Lifecycles: models.AllProjectLifecycles, IncludeHidden: true, }, ) - owners, err := hmndata.FetchProjectOwners(c.Context(), c.Conn, p.Project.ID) if err != nil { - return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "Failed to fetch project owners")) + return c.ErrorResponse(http.StatusInternalServerError, err) } projectSettings := templates.ProjectToProjectSettings( &p.Project, - owners, + p.Owners, p.TagText(), p.LogoURL("light"), p.LogoURL("dark"), c.Theme, diff --git a/src/website/routes.go b/src/website/routes.go index fd5ba7f..e4b3a13 100644 --- a/src/website/routes.go +++ b/src/website/routes.go @@ -300,7 +300,10 @@ func NewWebsiteRoutes(longRequestContext context.Context, conn *pgxpool.Pool) ht if err != nil { panic(oops.New(err, "project id was not numeric (bad regex in routing)")) } - p, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, id, hmndata.ProjectsQuery{}) + p, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, id, hmndata.ProjectsQuery{ + Lifecycles: models.AllProjectLifecycles, + IncludeHidden: true, + }) if err != nil { if errors.Is(err, db.NotFound) { return FourOhFour(c) @@ -465,7 +468,10 @@ func LoadCommonWebsiteData(c *RequestContext) (bool, ResponseData) { var owners []*models.User if len(slug) > 0 { - dbProject, err := hmndata.FetchProjectBySlug(c.Context(), c.Conn, c.CurrentUser, slug, hmndata.ProjectsQuery{}) + dbProject, err := hmndata.FetchProjectBySlug(c.Context(), c.Conn, c.CurrentUser, slug, hmndata.ProjectsQuery{ + Lifecycles: models.AllProjectLifecycles, + IncludeHidden: true, + }) if err == nil { c.CurrentProject = &dbProject.Project owners = dbProject.Owners @@ -480,6 +486,7 @@ func LoadCommonWebsiteData(c *RequestContext) (bool, ResponseData) { if c.CurrentProject == nil { dbProject, err := hmndata.FetchProject(c.Context(), c.Conn, c.CurrentUser, models.HMNProjectID, hmndata.ProjectsQuery{ + Lifecycles: models.AllProjectLifecycles, IncludeHidden: true, }) if err != nil { diff --git a/src/website/user.go b/src/website/user.go index de68fc6..9d48c2f 100644 --- a/src/website/user.go +++ b/src/website/user.go @@ -102,13 +102,12 @@ func UserProfile(c *RequestContext) ResponseData { } c.Perf.EndBlock() - projectsQuery := hmndata.ProjectsQuery{ - OwnerIDs: []int{profileUser.ID}, - Lifecycles: models.VisibleProjectLifecycles, - OrderBy: "all_last_updated DESC", - } - - projectsAndStuff, err := hmndata.FetchProjects(c.Context(), c.Conn, c.CurrentUser, projectsQuery) + projectsAndStuff, err := hmndata.FetchProjects(c.Context(), c.Conn, c.CurrentUser, hmndata.ProjectsQuery{ + OwnerIDs: []int{profileUser.ID}, + Lifecycles: models.AllProjectLifecycles, + IncludeHidden: true, + OrderBy: "all_last_updated DESC", + }) templateProjects := make([]templates.Project, 0, len(projectsAndStuff)) numPersonalProjects := 0 for _, p := range projectsAndStuff {