diff --git a/controllers/api/checklists.go b/controllers/api/checklists.go index b108f1c..3e4d615 100644 --- a/controllers/api/checklists.go +++ b/controllers/api/checklists.go @@ -211,9 +211,12 @@ func loadChecklistItems(db *gorm.DB, userID uint, checklistIDs []uint) (map[uint } materials := map[uint]*linkedMaterial{} if len(materialIDs) > 0 { - // Only published materials, as in the material list. + // Only published materials the caller may see, so a private + // material's title never shows. var ms []models.Material - if err := db.Select("id", "title").Where("id IN ? AND published_at IS NOT NULL", materialIDs).Find(&ms).Error; err != nil { + err := db.Select("id", "title").Where("id IN ? AND published_at IS NOT NULL", materialIDs). + Where(visibleMaterialSQL("jz_bm_materials"), userID, userID).Find(&ms).Error + if err != nil { return nil, err } for _, m := range ms { diff --git a/controllers/api/courses.go b/controllers/api/courses.go index c2b6b54..f170d4e 100644 --- a/controllers/api/courses.go +++ b/controllers/api/courses.go @@ -23,7 +23,7 @@ func (c courseRow) counts() courseCounts { } // coursesWithCounts selects courses with their session, reading and -// published material counts and the caller's read readings. +// visible published material counts and the caller's read readings. func coursesWithCounts(db *gorm.DB, userID uint, now time.Time) *gorm.DB { return db.Table("jz_bm_courses AS c").Select(`c.*, (SELECT COUNT(*) FROM jz_bm_class_sessions s WHERE s.course_id = c.id) AS sessions_count, @@ -31,7 +31,7 @@ func coursesWithCounts(db *gorm.DB, userID uint, now time.Time) *gorm.DB { (SELECT COUNT(*) FROM jz_bm_reading_progress p JOIN jz_bm_readings r ON r.id = p.reading_id WHERE r.course_id = c.id AND p.user_id = ?) AS readings_read, (SELECT COUNT(*) FROM jz_bm_materials m WHERE m.course_id = c.id - AND m.published_at IS NOT NULL AND m.published_at <= ?) AS materials_count`, userID, now.UTC()) + AND m.published_at IS NOT NULL AND m.published_at <= ? AND `+visibleMaterialSQL("m")+`) AS materials_count`, userID, now.UTC(), userID, userID) } // Courses answers GET /courses?semester=N: every course in programme order @@ -103,7 +103,7 @@ func (h *Handlers) Course(w http.ResponseWriter, r *http.Request) { writeOpaque500(w) return } - materials, err := publishedMaterialsOfCourse(db, now, row.ID) + materials, err := publishedMaterialsOfCourse(db, now, userID, row.ID) if err != nil { writeOpaque500(w) return diff --git a/controllers/api/handlers_test.go b/controllers/api/handlers_test.go index 8f754b6..fa1f442 100644 --- a/controllers/api/handlers_test.go +++ b/controllers/api/handlers_test.go @@ -38,25 +38,26 @@ func serve(t *testing.T, h http.HandlerFunc, method, target string, user *bounce func handlers(h *api.Handlers) map[string]http.HandlerFunc { return map[string]http.HandlerFunc{ - "dashboard": h.Dashboard, - "courses": h.Courses, - "course": h.Course, - "schedule": h.Schedule, - "meetings": h.Meetings, - "deadlines": h.Deadlines, - "readings": h.Readings, - "mark": h.MarkRead, - "unmark": h.UnmarkRead, - "own": h.MarkOwned, - "unown": h.UnmarkOwned, - "materials": h.Materials, - "material": h.Material, - "feed": h.CalendarFeed, - "feed-put": h.UpdateCalendarFeed, - "rotate": h.RotateCalendarFeed, - "feed-del": h.DeleteCalendarFeed, - "chat": h.Chat, - "chat-mem": h.ChatMembers, + "dashboard": h.Dashboard, + "courses": h.Courses, + "course": h.Course, + "schedule": h.Schedule, + "meetings": h.Meetings, + "deadlines": h.Deadlines, + "readings": h.Readings, + "mark": h.MarkRead, + "unmark": h.UnmarkRead, + "own": h.MarkOwned, + "unown": h.UnmarkOwned, + "materials": h.Materials, + "material": h.Material, + "mat-access": h.MaterialAccess, + "feed": h.CalendarFeed, + "feed-put": h.UpdateCalendarFeed, + "rotate": h.RotateCalendarFeed, + "feed-del": h.DeleteCalendarFeed, + "chat": h.Chat, + "chat-mem": h.ChatMembers, } } @@ -71,8 +72,9 @@ type bodyRequest struct { func bodyHandlers(h *api.Handlers) map[string]bodyRequest { return map[string]bodyRequest{ - "chat-embeds": {h.ChatEmbeds, http.MethodGet, "/?type=deadline&q=esej", ""}, - "chat-resolve": {h.ResolveChatEmbeds, http.MethodPost, "/", `{"refs":[{"type":"reading","id":1}]}`}, + "chat-embeds": {h.ChatEmbeds, http.MethodGet, "/?type=deadline&q=esej", ""}, + "chat-resolve": {h.ResolveChatEmbeds, http.MethodPost, "/", `{"refs":[{"type":"reading","id":1}]}`}, + "mat-access-put": {h.UpdateMaterialAccess, http.MethodPut, "/", `{"is_private":true,"user_ids":[]}`}, } } diff --git a/controllers/api/json.go b/controllers/api/json.go index c0e6136..4131bb4 100644 --- a/controllers/api/json.go +++ b/controllers/api/json.go @@ -199,7 +199,7 @@ func newTagsJSON(tags []models.Tag) []tagJSON { return out } -// materialListJSON is a published material in a list. +// materialListJSON is a published material the caller may see, in a list. type materialListJSON struct { ID uint `json:"id"` CourseID *uint `json:"course_id"` @@ -212,6 +212,7 @@ type materialListJSON struct { PublishedAt string `json:"published_at"` Tags []tagJSON `json:"tags"` FilesCount int64 `json:"files_count"` + IsPrivate bool `json:"is_private"` } func newMaterialListJSON(m models.Material, files int64) materialListJSON { @@ -226,6 +227,7 @@ func newMaterialListJSON(m models.Material, files int64) materialListJSON { URL: m.URL, Tags: newTagsJSON(m.Tags), FilesCount: files, + IsPrivate: m.IsPrivate, } if m.PublishedAt != nil { out.PublishedAt = models.FormatDateTime(*m.PublishedAt) @@ -233,11 +235,13 @@ func newMaterialListJSON(m models.Material, files int64) materialListJSON { return out } -// materialDetailJSON is a published material with its body and files. +// materialDetailJSON is a published material the caller may see, with its +// body and files. CanManage is true for the caller who may change its access. type materialDetailJSON struct { materialListJSON - Body *string `json:"body"` - Files []fileJSON `json:"files"` + Body *string `json:"body"` + Files []fileJSON `json:"files"` + CanManage bool `json:"can_manage"` } // fileJSON is one attached file. diff --git a/controllers/api/json_test.go b/controllers/api/json_test.go index e2d3a82..32d47a4 100644 --- a/controllers/api/json_test.go +++ b/controllers/api/json_test.go @@ -49,3 +49,44 @@ func TestMaterialJSONCarriesMeetingID(t *testing.T) { } } } + +// TestMaterialJSONCarriesPrivacy checks that is_private is always present in +// a material and that the detail carries can_manage, false by default. +func TestMaterialJSONCarriesPrivacy(t *testing.T) { + for _, tc := range []struct { + private bool + want string + }{ + {true, `"is_private":true`}, + {false, `"is_private":false`}, + } { + b, err := json.Marshal(newMaterialListJSON(models.Material{ID: 1, Title: "Materiał", Type: "pdf", IsPrivate: tc.private}, 0)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), tc.want) { + t.Errorf("material JSON %s does not contain %s", b, tc.want) + } + } + b, err := json.Marshal(materialDetailJSON{materialListJSON: newMaterialListJSON(models.Material{ID: 1, Title: "Materiał", Type: "pdf"}, 0)}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `"can_manage":false`) { + t.Errorf("material detail JSON %s does not contain can_manage false", b) + } +} + +// TestVisibleMaterialSQL checks the visibility fragment: public, the author, +// or a share of the caller, with the caller id bound twice. +func TestVisibleMaterialSQL(t *testing.T) { + got := visibleMaterialSQL("m") + for _, want := range []string{"m.is_private = FALSE", "m.author_user_id = ?", "jz_bm_material_shares", ".material_id = m.id"} { + if !strings.Contains(got, want) { + t.Errorf("visibleMaterialSQL(m) = %q, missing %q", got, want) + } + } + if n := strings.Count(got, "?"); n != 2 { + t.Errorf("visibleMaterialSQL(m) has %d placeholders, want 2", n) + } +} diff --git a/controllers/api/material_access.go b/controllers/api/material_access.go new file mode 100644 index 0000000..e87bf46 --- /dev/null +++ b/controllers/api/material_access.go @@ -0,0 +1,343 @@ +package api + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "slices" + "strconv" + "strings" + "time" + + userclasses "git.golem15.com/golem15/sm-user-plugin/classes" + usermodels "git.golem15.com/golem15/sm-user-plugin/models" + "git.golem15.com/golem15/summercms/modules/cabana" + "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/golem15/summercms/modules/surf" + "git.golem15.com/jakub/sm-bm-plugin/controllers" + "git.golem15.com/jakub/sm-bm-plugin/models" + "gorm.io/gorm" +) + +// Private materials: a material with is_private is visible only to its +// author and to the users it is shared with (jz_bm_material_shares). Every +// student read goes through visibleMaterials or, in raw SQL, through +// visibleMaterialSQL, so a private material never shows up elsewhere. + +// visibleMaterialSQL is the visibility condition of the materials table +// under alias: public, written by the caller, or shared with the caller. It +// takes the args (userID, userID). +func visibleMaterialSQL(alias string) string { + return "(" + alias + ".is_private = FALSE OR " + alias + ".author_user_id = ? OR EXISTS (SELECT 1 FROM jz_bm_material_shares ms WHERE ms.material_id = " + alias + ".id AND ms.user_id = ?))" +} + +// visibleMaterials scopes a material query to what the caller may see: a +// publication time that has passed and the visibility rule. It is the only +// scope student material reads may use. +func visibleMaterials(q *gorm.DB, now time.Time, userID uint) *gorm.DB { + return published(q, now).Where(visibleMaterialSQL("jz_bm_materials"), userID, userID) +} + +// userRefJSON is a user as the material access lists show it. +type userRefJSON struct { + ID uint `json:"id"` + Name string `json:"name"` + Email string `json:"email"` +} + +// newUserRef labels a user by "name surname", or by the email when both are +// empty. +func newUserRef(u usermodels.User) userRefJSON { + name := strings.TrimSpace(deref(u.Name) + " " + deref(u.Surname)) + if name == "" { + name = u.Email + } + return userRefJSON{ID: u.ID, Name: name, Email: u.Email} +} + +func selectUserRefs(db *gorm.DB) *gorm.DB { + return db.Model(&usermodels.User{}).Select("id", "name", "surname", "email") +} + +// listUserRefs lists every user but excludeID (none when 0), by name. +func listUserRefs(db *gorm.DB, excludeID uint) ([]userRefJSON, error) { + q := selectUserRefs(db) + if excludeID != 0 { + q = q.Where("id <> ?", excludeID) + } + var users []usermodels.User + if err := q.Order("name, surname, email, id").Find(&users).Error; err != nil { + return nil, err + } + out := make([]userRefJSON, 0, len(users)) + for _, u := range users { + out = append(out, newUserRef(u)) + } + return out, nil +} + +// userRefsByID returns the existing users among ids, keyed by id. +func userRefsByID(db *gorm.DB, ids []uint) (map[uint]userRefJSON, error) { + out := map[uint]userRefJSON{} + if len(ids) == 0 { + return out, nil + } + var users []usermodels.User + if err := selectUserRefs(db).Where("id IN ?", ids).Find(&users).Error; err != nil { + return nil, err + } + for _, u := range users { + out[u.ID] = newUserRef(u) + } + return out, nil +} + +// materialShareRefs returns the users each material is shared with, by user +// id. Deleted users drop out. +func materialShareRefs(db *gorm.DB, materialIDs []uint) (map[uint][]userRefJSON, error) { + out := map[uint][]userRefJSON{} + if len(materialIDs) == 0 { + return out, nil + } + var rows []models.MaterialShare + if err := db.Where("material_id IN ?", materialIDs).Order("material_id, user_id").Find(&rows).Error; err != nil { + return nil, err + } + userIDs := make([]uint, 0, len(rows)) + for _, row := range rows { + userIDs = append(userIDs, row.UserID) + } + users, err := userRefsByID(db, userIDs) + if err != nil { + return nil, err + } + for _, row := range rows { + if u, ok := users[row.UserID]; ok { + out[row.MaterialID] = append(out[row.MaterialID], u) + } + } + return out, nil +} + +// maxShareUsers caps the users one material may be shared with. +const maxShareUsers = 200 + +// cleanUserIDs dedupes a share list and checks it: no zero ids, at most +// maxShareUsers, and every id an existing user. msg is the validation +// message when the list is refused. +func cleanUserIDs(db *gorm.DB, ids []uint) ([]uint, string, error) { + clean := make([]uint, 0, len(ids)) + for _, id := range ids { + if id == 0 { + return nil, "Each user id must be a positive number.", nil + } + if !slices.Contains(clean, id) { + clean = append(clean, id) + } + } + if len(clean) > maxShareUsers { + return nil, "A material may be shared with at most " + strconv.Itoa(maxShareUsers) + " users.", nil + } + slices.Sort(clean) + if len(clean) == 0 { + return clean, "", nil + } + var n int64 + if err := db.Model(&usermodels.User{}).Where("id IN ?", clean).Count(&n).Error; err != nil { + return nil, "", err + } + if int(n) != len(clean) { + return nil, "The selected user_ids are invalid.", nil + } + return clean, "", nil +} + +// replaceMaterialShares makes userIDs the users the material is shared with. +func replaceMaterialShares(tx *gorm.DB, materialID uint, userIDs []uint) error { + if err := tx.Where("material_id = ?", materialID).Delete(&models.MaterialShare{}).Error; err != nil { + return err + } + for _, id := range userIDs { + if err := tx.Create(&models.MaterialShare{MaterialID: materialID, UserID: id}).Error; err != nil { + return err + } + } + return nil +} + +// managesMaterials tells whether the user holds jz.bm.access_materials. +func (h *Handlers) managesMaterials(ctx context.Context, db *gorm.DB, userID uint) (bool, error) { + principal, err := userclasses.PrincipalByID(ctx, db, userID) + if err != nil { + return false, err + } + return principal != nil && cabana.Allows(principal, []string{controllers.PermMaterials}), nil +} + +// canManageMaterial tells whether a material manager may change the +// material's access: it is theirs, or it has no author yet (the first change +// makes them its author). +func canManageMaterial(m models.Material, userID uint, manager bool) bool { + return manager && (m.AuthorUserID == nil || *m.AuthorUserID == userID) +} + +// materialAccessJSON is GET /materials/{id}/access: the material's +// visibility, the users it is shared with and the users to pick from. +type materialAccessJSON struct { + MaterialID uint `json:"material_id"` + IsPrivate bool `json:"is_private"` + AuthorUserID *uint `json:"author_user_id"` + Shares []userRefJSON `json:"shares"` + Users []userRefJSON `json:"users"` +} + +// materialAccessInput is the body of PUT /materials/{id}/access. +type materialAccessInput struct { + IsPrivate *bool `json:"is_private"` + UserIDs *[]uint `json:"user_ids"` +} + +// managedMaterialForCaller loads a material the caller sees and may manage. +// It answers 404 or 500 itself and returns false then: a material the caller +// may not manage is a 404, so nothing hints that a private one exists. +func (h *Handlers) managedMaterialForCaller(w http.ResponseWriter, r *http.Request, db *gorm.DB, userID uint) (models.Material, bool) { + var m models.Material + id, ok := surf.IntParam(r, "id") + if !ok { + writeNotFound(w) + return m, false + } + res := visibleMaterials(db, h.now(), userID).Where("jz_bm_materials.id = ?", id).Limit(1).Find(&m) + if res.Error != nil { + writeOpaque500(w) + return m, false + } + if res.RowsAffected == 0 { + writeNotFound(w) + return m, false + } + manager, err := h.managesMaterials(r.Context(), db, userID) + if err != nil { + writeOpaque500(w) + return m, false + } + if !canManageMaterial(m, userID, manager) { + writeNotFound(w) + return m, false + } + return m, true +} + +// writeMaterialAccess answers the access of m as the caller sees it. +func writeMaterialAccess(w http.ResponseWriter, db *gorm.DB, m models.Material, userID uint) { + shares, err := materialShareRefs(db, []uint{m.ID}) + if err != nil { + writeOpaque500(w) + return + } + users, err := listUserRefs(db, userID) + if err != nil { + writeOpaque500(w) + return + } + out := materialAccessJSON{MaterialID: m.ID, IsPrivate: m.IsPrivate, AuthorUserID: m.AuthorUserID, Shares: shares[m.ID], Users: users} + if out.Shares == nil { + out.Shares = []userRefJSON{} + } + writeData(w, out) +} + +// MaterialAccess answers GET /materials/{id}/access for the material's +// author (or any material manager while it has no author): its visibility, +// its shares and the users it can be shared with. Anyone else gets a 404. +func (h *Handlers) MaterialAccess(w http.ResponseWriter, r *http.Request) { + userID, ok := h.caller(w, r) + if !ok { + return + } + db, ok := h.db(w, r) + if !ok { + return + } + m, ok := h.managedMaterialForCaller(w, r, db, userID) + if !ok { + return + } + writeMaterialAccess(w, db, m, userID) +} + +// UpdateMaterialAccess answers PUT /materials/{id}/access, body +// {"is_private": bool?, "user_ids": [id]?}: absent keys keep the stored +// value, and user_ids (null meaning none) replaces the shares. The first +// change of a material with no author makes the caller its author. Access is +// as for MaterialAccess. +func (h *Handlers) UpdateMaterialAccess(w http.ResponseWriter, r *http.Request) { + userID, ok := h.caller(w, r) + if !ok { + return + } + db, ok := h.db(w, r) + if !ok { + return + } + m, ok := h.managedMaterialForCaller(w, r, db, userID) + if !ok { + return + } + raw, ok := body(w, r) + if !ok { + return + } + var in materialAccessInput + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.DisallowUnknownFields() + if err := dec.Decode(&in); err != nil { + writeInvalid(w, "invalid JSON: "+err.Error()) + return + } + var shares []uint + if in.UserIDs != nil { + clean, msg, err := cleanUserIDs(db, *in.UserIDs) + if err != nil { + writeOpaque500(w) + return + } + if msg != "" { + writeValidation(w, map[string][]string{"user_ids": {msg}}) + return + } + shares = clean + } + err := lagoon.Transaction(r.Context(), db, func(_ context.Context, tx *gorm.DB) error { + return applyMaterialAccess(tx, &m, userID, in.IsPrivate, in.UserIDs != nil, shares) + }) + if err != nil { + writeOpaque500(w) + return + } + writeMaterialAccess(w, db, m, userID) +} + +// applyMaterialAccess writes a visibility and share change onto m inside +// tx. A material with no author gets userID as author (the claim rule). +// Only is_private and author_user_id are updated; the shares are replaced +// when setShares. +func applyMaterialAccess(tx *gorm.DB, m *models.Material, userID uint, isPrivate *bool, setShares bool, shares []uint) error { + if m.AuthorUserID == nil { + id := userID + m.AuthorUserID = &id + } + if isPrivate != nil { + m.IsPrivate = *isPrivate + } + err := tx.Model(&models.Material{}).Where("id = ?", m.ID). + Updates(map[string]any{"is_private": m.IsPrivate, "author_user_id": m.AuthorUserID}).Error + if err != nil { + return err + } + if !setShares { + return nil + } + return replaceMaterialShares(tx, m.ID, shares) +} diff --git a/controllers/api/materials.go b/controllers/api/materials.go index f678753..81c62bd 100644 --- a/controllers/api/materials.go +++ b/controllers/api/materials.go @@ -48,15 +48,16 @@ func preloadMaterial(q *gorm.DB) *gorm.DB { return q.Preload("Course").Preload("Tags", func(db *gorm.DB) *gorm.DB { return db.Order("jz_bm_tags.name") }) } -// listMaterials returns one page of published materials, newest first. -func listMaterials(db *gorm.DB, now time.Time, f materialFilter, page, perPage int) ([]materialListJSON, int64, error) { - q := f.apply(published(db.Model(&models.Material{}), now)) +// listMaterials returns one page of the published materials userID may +// see, newest first. +func listMaterials(db *gorm.DB, now time.Time, userID uint, f materialFilter, page, perPage int) ([]materialListJSON, int64, error) { + q := f.apply(visibleMaterials(db.Model(&models.Material{}), now, userID)) var total int64 if err := q.Count(&total).Error; err != nil { return nil, 0, err } var rows []models.Material - err := preloadMaterial(f.apply(published(db, now))). + err := preloadMaterial(f.apply(visibleMaterials(db, now, userID))). Order("jz_bm_materials.published_at DESC, jz_bm_materials.id DESC"). Offset((page - 1) * perPage).Limit(perPage). Find(&rows).Error @@ -99,9 +100,11 @@ func materialsJSON(db *gorm.DB, rows []models.Material) ([]materialListJSON, err } // Materials answers GET /materials?course=&meeting=&tag=&type=&page=&per_page=: -// a page of published materials in the {data, meta} shape of lagoon.Paginate. +// a page of the published materials the caller may see, in the {data, meta} +// shape of lagoon.Paginate. func (h *Handlers) Materials(w http.ResponseWriter, r *http.Request) { - if _, ok := h.caller(w, r); !ok { + userID, ok := h.caller(w, r) + if !ok { return } in := query(r, "course", "meeting", "tag", "type", "page", "per_page") @@ -115,7 +118,7 @@ func (h *Handlers) Materials(w http.ResponseWriter, r *http.Request) { filter := materialFilter{CourseID: uintParam(in, "course"), MeetingID: uintParam(in, "meeting"), Tag: strings.TrimSpace(str(in, "tag")), Type: str(in, "type")} page := min(intParam(in, "page", 1), 100000) perPage := intParam(in, "per_page", h.perPage()) - rows, total, err := listMaterials(db, h.now(), filter, page, perPage) + rows, total, err := listMaterials(db, h.now(), userID, filter, page, perPage) if err != nil { writeOpaque500(w) return @@ -123,11 +126,14 @@ func (h *Handlers) Materials(w http.ResponseWriter, r *http.Request) { writeJSONPage(w, lagoon.Paginate(rows, page, perPage, total)) } -// Material answers GET /materials/{id}: one published material with its -// body, tags and files. A draft or a future publication is a 404. An MCP -// call gets the body only with include_body (see materialView). +// Material answers GET /materials/{id}: one published material the caller +// may see, with its body, tags and files. A draft, a future publication or +// another user's private material is a 404. can_manage tells the author (a +// material manager) that the access panel applies. An MCP call gets the body +// only with include_body (see materialView). func (h *Handlers) Material(w http.ResponseWriter, r *http.Request) { - if _, ok := h.caller(w, r); !ok { + userID, ok := h.caller(w, r) + if !ok { return } db, ok := h.db(w, r) @@ -140,7 +146,7 @@ func (h *Handlers) Material(w http.ResponseWriter, r *http.Request) { return } var m models.Material - res := preloadMaterial(published(db, h.now())).Where("jz_bm_materials.id = ?", id).Limit(1).Find(&m) + res := preloadMaterial(visibleMaterials(db, h.now(), userID)).Where("jz_bm_materials.id = ?", id).Limit(1).Find(&m) if res.Error != nil { writeOpaque500(w) return @@ -170,14 +176,22 @@ func (h *Handlers) Material(w http.ResponseWriter, r *http.Request) { out.Files = append(out.Files, fileJSON{ID: f.ID, URL: f.URL(), Name: f.FileName, Size: f.FileSize, Mime: f.ContentType}) } out.FilesCount = int64(len(out.Files)) + if m.AuthorUserID == nil || *m.AuthorUserID == userID { + manager, err := h.managesMaterials(r.Context(), db, userID) + if err != nil { + writeOpaque500(w) + return + } + out.CanManage = canManageMaterial(m, userID, manager) + } writeData(w, materialView(out, viaMCP(r), includeBody(r))) } -// publishedMaterialsOfCourse lists a course's published materials, newest -// first, for /courses/{id}. -func publishedMaterialsOfCourse(db *gorm.DB, now time.Time, courseID uint) ([]materialListJSON, error) { +// publishedMaterialsOfCourse lists a course's published materials that +// userID may see, newest first, for /courses/{id}. +func publishedMaterialsOfCourse(db *gorm.DB, now time.Time, userID uint, courseID uint) ([]materialListJSON, error) { var rows []models.Material - err := preloadMaterial(published(db, now)). + err := preloadMaterial(visibleMaterials(db, now, userID)). Where("jz_bm_materials.course_id = ?", courseID). Order("jz_bm_materials.published_at DESC, jz_bm_materials.id DESC"). Find(&rows).Error diff --git a/material_access_postgres_test.go b/material_access_postgres_test.go new file mode 100644 index 0000000..cc5b4e9 --- /dev/null +++ b/material_access_postgres_test.go @@ -0,0 +1,156 @@ +package bm_test + +import ( + "net/http" + "slices" + "testing" + + "git.golem15.com/golem15/summercms/modules/surf" + "git.golem15.com/jakub/sm-bm-plugin/models" +) + +// TestMaterialAccessAgainstDatabase checks the visibility rule of private +// materials in the student reads (list, detail, course detail and counters) +// for the author, a user a material is shared with and another user, and +// the author's access endpoints with the claim rule. +func TestMaterialAccessAgainstDatabase(t *testing.T) { + app, plugins, gdb := migrated(t) + seedFixture(t, plugins, "testdata/seed.example.json") + h, err := surf.Assemble(app, plugins) + if err != nil { + t.Fatalf("Assemble: %v", err) + } + authorID := createUser(t, gdb, "author@example.test") + bobID := createUser(t, gdb, "bob@example.test") + carolID := createUser(t, gdb, "carol@example.test") + var groupID uint + if err := gdb.Raw(` +INSERT INTO user_groups (name, code, permissions, created_at, updated_at) +VALUES ('BM materials', 'bm-materials', '{"jz.bm.access_materials":1}', NOW(), NOW()) +RETURNING id`).Scan(&groupID).Error; err != nil { + t.Fatal(err) + } + if err := gdb.Exec(`INSERT INTO users_groups (user_id, user_group_id) VALUES (?, ?)`, authorID, groupID).Error; err != nil { + t.Fatal(err) + } + author := apiClient{t: t, h: h, token: tokenFor(t, authorID)} + bob := apiClient{t: t, h: h, token: tokenFor(t, bobID)} + carol := apiClient{t: t, h: h, token: tokenFor(t, carolID)} + const base = "/_bm/api/v1" + + var course models.Course + if err := gdb.Order("id").Take(&course).Error; err != nil { + t.Fatal(err) + } + insert := func(title string, private bool, authorUserID *uint) uint { + t.Helper() + var id uint + err := gdb.Raw(`INSERT INTO jz_bm_materials (course_id, title, type, published_at, is_private, author_user_id) +VALUES (?, ?, 'notatka', NOW() - INTERVAL '1 day', ?, ?) RETURNING id`, course.ID, title, private, authorUserID).Scan(&id).Error + if err != nil { + t.Fatal(err) + } + return id + } + pub := insert("Publiczny", false, nil) + priv := insert("Prywatny autora", true, &authorID) + shared := insert("Udostępniony Bobowi", true, &authorID) + if err := gdb.Create(&models.MaterialShare{MaterialID: shared, UserID: bobID}).Error; err != nil { + t.Fatal(err) + } + + ids := func(rows []map[string]any) []string { + out := make([]string, 0, len(rows)) + for _, row := range rows { + out = append(out, id(row["id"])) + } + slices.Sort(out) + return out + } + sorted := func(in ...uint) []string { + out := make([]string, 0, len(in)) + for _, v := range in { + out = append(out, jsonID(v)) + } + slices.Sort(out) + return out + } + courseMaterials := func(c apiClient) ([]string, float64) { + t.Helper() + detail := obj(c.ok("GET", base+"/courses/"+jsonID(course.ID))["data"]) + raw, _ := detail["materials"].([]any) + rows := make([]map[string]any, 0, len(raw)) + for _, r := range raw { + rows = append(rows, r.(map[string]any)) + } + n, _ := obj(detail["counts"])["materials"].(float64) + return ids(rows), n + } + for _, tc := range []struct { + name string + client apiClient + want []string + }{ + {"carol", carol, sorted(pub)}, + {"bob", bob, sorted(pub, shared)}, + {"author", author, sorted(pub, priv, shared)}, + } { + if got := ids(list(tc.client.ok("GET", base+"/materials"))); !slices.Equal(got, tc.want) { + t.Errorf("%s /materials = %v, want %v", tc.name, got, tc.want) + } + got, n := courseMaterials(tc.client) + if !slices.Equal(got, tc.want) || int(n) != len(tc.want) { + t.Errorf("%s course materials = %v (count %v), want %v", tc.name, got, n, tc.want) + } + } + carol.want(http.StatusNotFound, "GET", base+"/materials/"+jsonID(priv), "") + bob.want(http.StatusNotFound, "GET", base+"/materials/"+jsonID(priv), "") + + detail := obj(author.ok("GET", base+"/materials/"+jsonID(priv))["data"]) + if detail["can_manage"] != true || detail["is_private"] != true { + t.Errorf("author detail = %v, want can_manage and is_private", detail) + } + if d := obj(bob.ok("GET", base+"/materials/"+jsonID(shared))["data"]); d["can_manage"] != false { + t.Errorf("bob detail of the shared material = %v, want can_manage false", d) + } + + // The access endpoints are the author's (a manager) only. + access := base + "/materials/" + jsonID(priv) + "/access" + carol.want(http.StatusNotFound, "GET", access, "") + bob.want(http.StatusNotFound, "GET", access, "") + carol.want(http.StatusNotFound, "PUT", access, `{"is_private":false}`) + bob.want(http.StatusNotFound, "PUT", base+"/materials/"+jsonID(shared)+"/access", `{"is_private":false}`) + got := obj(author.ok("GET", access)["data"]) + if got["is_private"] != true { + t.Errorf("access = %v", got) + } + if shares, ok := got["shares"].([]any); !ok || len(shares) != 0 { + t.Errorf("shares = %v, want []", got["shares"]) + } + var users []string + for _, u := range got["users"].([]any) { + users = append(users, id(u.(map[string]any)["id"])) + } + if !slices.Contains(users, jsonID(bobID)) || !slices.Contains(users, jsonID(carolID)) || slices.Contains(users, jsonID(authorID)) { + t.Errorf("users = %v, want bob and carol without the author", users) + } + author.want(http.StatusUnprocessableEntity, "PUT", access, `{"user_ids":[999999]}`) + author.want(http.StatusUnprocessableEntity, "PUT", access, `{"author_user_id":1}`) + put := obj(author.want(http.StatusOK, "PUT", access, `{"user_ids":[`+jsonID(carolID)+`]}`)["data"]) + if shares, _ := put["shares"].([]any); len(shares) != 1 || id(obj(shares[0])["id"]) != jsonID(carolID) { + t.Errorf("shares after PUT = %v", put["shares"]) + } + carol.ok("GET", base+"/materials/"+jsonID(priv)) + + // The claim rule: changing an author-less material makes the caller its + // author. + pubAccess := base + "/materials/" + jsonID(pub) + "/access" + claimed := obj(author.want(http.StatusOK, "PUT", pubAccess, `{"is_private":true}`)["data"]) + if claimed["is_private"] != true || id(claimed["author_user_id"]) != jsonID(authorID) { + t.Errorf("claimed = %v", claimed) + } + carol.want(http.StatusNotFound, "GET", base+"/materials/"+jsonID(pub), "") + if got := ids(list(carol.ok("GET", base+"/materials"))); !slices.Equal(got, sorted(priv)) { + t.Errorf("carol /materials after the claim = %v, want only the shared one", got) + } +} diff --git a/models/material.go b/models/material.go index 162eb04..2683c65 100644 --- a/models/material.go +++ b/models/material.go @@ -18,21 +18,24 @@ const MaterialFilesField = "files" // Material is a study material: a note, a link or uploaded files, optionally // tied to a course. A material without published_at is a draft and is not -// served by the API. +// served by the API. A private material is visible only to its author and to +// the users it is shared with (MaterialShare); a public one to every student. type Material struct { - ID uint `gorm:"column:id;primaryKey"` - CourseID *uint `gorm:"column:course_id"` - Course *Course `gorm:"foreignKey:CourseID"` - MeetingID *uint `gorm:"column:meeting_id"` - Meeting *Meeting `gorm:"foreignKey:MeetingID"` - Title string `gorm:"column:title"` - Type string `gorm:"column:type"` - Body *string `gorm:"column:body"` - Source *string `gorm:"column:source"` - URL *string `gorm:"column:url"` - PublishedAt *time.Time `gorm:"column:published_at"` - CreatedAt time.Time `gorm:"column:created_at"` - UpdatedAt time.Time `gorm:"column:updated_at"` + ID uint `gorm:"column:id;primaryKey"` + CourseID *uint `gorm:"column:course_id"` + Course *Course `gorm:"foreignKey:CourseID"` + MeetingID *uint `gorm:"column:meeting_id"` + Meeting *Meeting `gorm:"foreignKey:MeetingID"` + Title string `gorm:"column:title"` + Type string `gorm:"column:type"` + Body *string `gorm:"column:body"` + Source *string `gorm:"column:source"` + URL *string `gorm:"column:url"` + PublishedAt *time.Time `gorm:"column:published_at"` + IsPrivate bool `gorm:"column:is_private"` + AuthorUserID *uint `gorm:"column:author_user_id"` + CreatedAt time.Time `gorm:"column:created_at"` + UpdatedAt time.Time `gorm:"column:updated_at"` Tags []Tag `gorm:"many2many:jz_bm_material_tag;joinForeignKey:material_id;joinReferences:tag_id"` } @@ -45,7 +48,7 @@ func (Material) MorphName() string { return `Jz\Bm\Models\Material` } // Fillable lists the columns mass assignment may set. func (Material) Fillable() []string { - return []string{"course_id", "meeting_id", "title", "type", "body", "source", "url", "published_at"} + return []string{"course_id", "meeting_id", "title", "type", "body", "source", "url", "published_at", "is_private", "author_user_id"} } // Rules are checked by the admin API on every save. diff --git a/models/material_share.go b/models/material_share.go new file mode 100644 index 0000000..2ae1d7e --- /dev/null +++ b/models/material_share.go @@ -0,0 +1,11 @@ +package models + +// MaterialShare is the pivot row between a private material and a user it +// is shared with. +type MaterialShare struct { + MaterialID uint `gorm:"column:material_id;primaryKey"` + UserID uint `gorm:"column:user_id;primaryKey"` +} + +// TableName is the pivot table. +func (MaterialShare) TableName() string { return "jz_bm_material_shares" } diff --git a/plugin_test.go b/plugin_test.go index 3d5098b..5c15961 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -76,11 +76,11 @@ func TestPluginActivates(t *testing.T) { if p.ID() != "jz.bm" || !slices.Equal(p.Requires(), []string{"golem15.user"}) { t.Errorf("ID %q Requires %v", p.ID(), p.Requires()) } - if n := len(p.Models()); n != 13 { - t.Errorf("Models = %d, want 13", n) + if n := len(p.Models()); n != 14 { + t.Errorf("Models = %d, want 14", n) } - if n := len(p.Migrations()); n != 18 { - t.Errorf("Migrations = %d, want 18", n) + if n := len(p.Migrations()); n != 19 { + t.Errorf("Migrations = %d, want 19", n) } if !slices.ContainsFunc(p.Commands(), func(c bonfire.Command) bool { return c.Name == "bm:seed" }) { t.Error("bm:seed is not registered") @@ -131,6 +131,8 @@ func TestRoutes(t *testing.T) { "PUT /_bm/api/v1/calendar-feed", "POST /_bm/api/v1/calendar-feed/rotate", "DELETE /_bm/api/v1/calendar-feed", + "GET /_bm/api/v1/materials/{id}/access", + "PUT /_bm/api/v1/materials/{id}/access", "GET /_bm/api/v1/token/dashboard", "GET /_bm/api/v1/token/courses", "GET /_bm/api/v1/token/courses/{id}", diff --git a/registry.gen.go b/registry.gen.go index e2836d0..259e627 100644 --- a/registry.gen.go +++ b/registry.gen.go @@ -21,6 +21,7 @@ func generatedModels() []any { &models.Deadline{}, &models.Material{}, &models.MaterialTag{}, + &models.MaterialShare{}, &models.Meeting{}, &models.Reading{}, &models.ReadingProgress{}, @@ -49,6 +50,7 @@ func generatedMigrations() []*gormigrate.Migration { updates.AddMeetingToReadingsAndMaterials(), updates.CreateChecklists(), updates.CreateChecklistItems(), + updates.AddPrivacyToMaterials(), } } diff --git a/routes.go b/routes.go index 7058eba..7cf5750 100644 --- a/routes.go +++ b/routes.go @@ -161,6 +161,12 @@ func (p *Plugin) Routes(r pact.Router) error { g.Put("/calendar-feed", h.UpdateCalendarFeed) g.Post("/calendar-feed/rotate", h.RotateCalendarFeed, "throttle:10,1") g.Delete("/calendar-feed", h.DeleteCalendarFeed) + // The author's material access panel. It has no token twin: tokens + // and MCP change access through the management API. + g.Get("/materials/{id}/access", h.MaterialAccess) + g.Where("id", "[0-9]+") + g.Put("/materials/{id}/access", h.UpdateMaterialAccess, "body.limit:16384") + g.Where("id", "[0-9]+") if p.app != nil && p.app.HasPlugin("golem15.chat") { g.Get("/chat", h.Chat, bucketChat) g.Get("/chat/members", h.ChatMembers, bucketChat) diff --git a/updates/20261108120000_add_privacy_to_jz_bm_materials.go b/updates/20261108120000_add_privacy_to_jz_bm_materials.go new file mode 100644 index 0000000..4fa6757 --- /dev/null +++ b/updates/20261108120000_add_privacy_to_jz_bm_materials.go @@ -0,0 +1,36 @@ +package updates + +import ( + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// AddPrivacyToMaterials returns the +// 20261108120000_add_privacy_to_jz_bm_materials gormigrate entry: a material +// may be private, and then only its author and the users it is shared with +// see it. Existing materials stay public and have no author. Deleting the +// author keeps the material and clears the author; deleting a material or a +// user drops their shares. +func AddPrivacyToMaterials() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "20261108120000_add_privacy_to_jz_bm_materials", + Migrate: func(tx *gorm.DB) error { + return execAll(tx, ` +ALTER TABLE jz_bm_materials ADD COLUMN is_private BOOLEAN NOT NULL DEFAULT FALSE; +ALTER TABLE jz_bm_materials ADD COLUMN author_user_id INTEGER NULL REFERENCES users (id) ON DELETE SET NULL; +CREATE INDEX jz_bm_materials_author_user_id_index ON jz_bm_materials (author_user_id); +CREATE TABLE jz_bm_material_shares ( + material_id BIGINT NOT NULL REFERENCES jz_bm_materials (id) ON DELETE CASCADE, + user_id INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + PRIMARY KEY (material_id, user_id) +); +CREATE INDEX jz_bm_material_shares_user_id_index ON jz_bm_material_shares (user_id)`) + }, + Rollback: func(tx *gorm.DB) error { + return execAll(tx, ` +DROP TABLE IF EXISTS jz_bm_material_shares; +ALTER TABLE jz_bm_materials DROP COLUMN IF EXISTS author_user_id; +ALTER TABLE jz_bm_materials DROP COLUMN IF EXISTS is_private`) + }, + } +}