Add organization avatar uploads
- SHA
575ebbc0102d790d1491b2d6c493193e9f9ae8d7- Parents
-
acfe4d1 - Tree
4134a72
575ebbc
575ebbc0102d790d1491b2d6c493193e9f9ae8d7acfe4d1
4134a72docs/internal/storage.mdmodified@@ -36,6 +36,8 @@ lfs/<owner>/<repo>/<sha256> # LFS objects (post-MVP, key shape reserve | ||
| 36 | 36 | attachments/<scope>/<id>/<filename> # issue/PR/comment attachments |
| 37 | 37 | avatars/<user_id>/<hash>.png # largest rendered avatar variant |
| 38 | 38 | avatars/<user_id>/<hash>-<size>.png # smaller rendered avatar variants |
| 39 | +avatars/orgs/<org_id>/<hash>.png # largest rendered org avatar variant | |
| 40 | +avatars/orgs/<org_id>/<hash>-<size>.png | |
| 39 | 41 | backups/... # S37 |
| 40 | 42 | ``` |
| 41 | 43 | |
internal/web/handlers/orgs/avatar.goadded@@ -0,0 +1,149 @@ | ||
| 1 | +// SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | |
| 3 | +package orgs | |
| 4 | + | |
| 5 | +import ( | |
| 6 | + "bytes" | |
| 7 | + "errors" | |
| 8 | + "fmt" | |
| 9 | + "net/http" | |
| 10 | + | |
| 11 | + "github.com/jackc/pgx/v5/pgtype" | |
| 12 | + | |
| 13 | + "github.com/tenseleyFlow/shithub/internal/avatars" | |
| 14 | + "github.com/tenseleyFlow/shithub/internal/infra/storage" | |
| 15 | + orgsdb "github.com/tenseleyFlow/shithub/internal/orgs/sqlc" | |
| 16 | + "github.com/tenseleyFlow/shithub/internal/web/middleware" | |
| 17 | +) | |
| 18 | + | |
| 19 | +const avatarUploadCap = avatars.MaxUploadBytes + 64*1024 | |
| 20 | + | |
| 21 | +func (h *Handlers) settingsProfile(w http.ResponseWriter, r *http.Request) { | |
| 22 | + org, ok := h.orgFromSlug(w, r) | |
| 23 | + if !ok { | |
| 24 | + return | |
| 25 | + } | |
| 26 | + viewer := middleware.CurrentUserFromContext(r.Context()) | |
| 27 | + if !h.requireOrgOwner(w, r, org.ID, viewer) { | |
| 28 | + return | |
| 29 | + } | |
| 30 | + h.renderSettingsProfile(w, r, org, "", "") | |
| 31 | +} | |
| 32 | + | |
| 33 | +func (h *Handlers) settingsAvatarUpload(w http.ResponseWriter, r *http.Request) { | |
| 34 | + org, ok := h.orgFromSlug(w, r) | |
| 35 | + if !ok { | |
| 36 | + return | |
| 37 | + } | |
| 38 | + viewer := middleware.CurrentUserFromContext(r.Context()) | |
| 39 | + if !h.requireOrgOwner(w, r, org.ID, viewer) { | |
| 40 | + return | |
| 41 | + } | |
| 42 | + if h.d.ObjectStore == nil { | |
| 43 | + h.d.Render.HTTPError(w, r, http.StatusServiceUnavailable, "avatar storage is not configured") | |
| 44 | + return | |
| 45 | + } | |
| 46 | + r.Body = http.MaxBytesReader(w, r.Body, avatarUploadCap) | |
| 47 | + //nolint:gosec // G120 false positive: avatarUploadCap is constant-bounded, MaxBytesReader enforces the cap. | |
| 48 | + if err := r.ParseMultipartForm(avatarUploadCap); err != nil { | |
| 49 | + h.renderSettingsProfile(w, r, org, friendlyAvatarErr(err), "") | |
| 50 | + return | |
| 51 | + } | |
| 52 | + file, _, err := r.FormFile("avatar") | |
| 53 | + if err != nil { | |
| 54 | + h.renderSettingsProfile(w, r, org, "Choose an image file to upload.", "") | |
| 55 | + return | |
| 56 | + } | |
| 57 | + defer func() { _ = file.Close() }() | |
| 58 | + | |
| 59 | + variants, hash, err := avatars.Process(file) | |
| 60 | + if err != nil { | |
| 61 | + h.renderSettingsProfile(w, r, org, friendlyAvatarErr(err), "") | |
| 62 | + return | |
| 63 | + } | |
| 64 | + prefix := fmt.Sprintf("avatars/orgs/%d/%s", org.ID, hash) | |
| 65 | + largest := "" | |
| 66 | + for _, v := range variants { | |
| 67 | + key := fmt.Sprintf("%s-%d.png", prefix, v.Size) | |
| 68 | + if v.Size == variants[0].Size { | |
| 69 | + key = prefix + ".png" | |
| 70 | + largest = key | |
| 71 | + } | |
| 72 | + if _, err := h.d.ObjectStore.Put( | |
| 73 | + r.Context(), key, | |
| 74 | + bytes.NewReader(v.Data), | |
| 75 | + storage.PutOpts{ContentType: "image/png", ContentLength: int64(len(v.Data))}, | |
| 76 | + ); err != nil { | |
| 77 | + h.d.Logger.ErrorContext(r.Context(), "org avatar: put", "error", err, "key", key) | |
| 78 | + h.renderSettingsProfile(w, r, org, "Could not store that avatar. Try again.", "") | |
| 79 | + return | |
| 80 | + } | |
| 81 | + } | |
| 82 | + if err := orgsdb.New().SetOrgAvatarKey(r.Context(), h.d.Pool, orgsdb.SetOrgAvatarKeyParams{ | |
| 83 | + ID: org.ID, | |
| 84 | + AvatarObjectKey: pgtype.Text{String: largest, Valid: true}, | |
| 85 | + }); err != nil { | |
| 86 | + h.d.Logger.ErrorContext(r.Context(), "org avatar: db update", "error", err) | |
| 87 | + h.renderSettingsProfile(w, r, org, "Could not save that avatar. Try again.", "") | |
| 88 | + return | |
| 89 | + } | |
| 90 | + http.Redirect(w, r, orgSettingsProfilePath(org), http.StatusSeeOther) | |
| 91 | +} | |
| 92 | + | |
| 93 | +func (h *Handlers) settingsAvatarRemove(w http.ResponseWriter, r *http.Request) { | |
| 94 | + org, ok := h.orgFromSlug(w, r) | |
| 95 | + if !ok { | |
| 96 | + return | |
| 97 | + } | |
| 98 | + viewer := middleware.CurrentUserFromContext(r.Context()) | |
| 99 | + if !h.requireOrgOwner(w, r, org.ID, viewer) { | |
| 100 | + return | |
| 101 | + } | |
| 102 | + if err := orgsdb.New().SetOrgAvatarKey(r.Context(), h.d.Pool, orgsdb.SetOrgAvatarKeyParams{ | |
| 103 | + ID: org.ID, | |
| 104 | + AvatarObjectKey: pgtype.Text{}, | |
| 105 | + }); err != nil { | |
| 106 | + h.d.Logger.ErrorContext(r.Context(), "org avatar: db clear", "error", err) | |
| 107 | + h.renderSettingsProfile(w, r, org, "Could not remove that avatar. Try again.", "") | |
| 108 | + return | |
| 109 | + } | |
| 110 | + http.Redirect(w, r, orgSettingsProfilePath(org), http.StatusSeeOther) | |
| 111 | +} | |
| 112 | + | |
| 113 | +func (h *Handlers) renderSettingsProfile( | |
| 114 | + w http.ResponseWriter, | |
| 115 | + r *http.Request, | |
| 116 | + org orgsdb.Org, | |
| 117 | + errMsg string, | |
| 118 | + success string, | |
| 119 | +) { | |
| 120 | + _ = h.d.Render.RenderPage(w, r, "orgs/settings_profile", map[string]any{ | |
| 121 | + "Title": org.Slug + " · profile settings", | |
| 122 | + "CSRFToken": middleware.CSRFTokenForRequest(r), | |
| 123 | + "Org": org, | |
| 124 | + "AvatarURL": "/avatars/" + org.Slug, | |
| 125 | + "AvatarUploadEnabled": h.d.ObjectStore != nil, | |
| 126 | + "HasAvatar": org.AvatarObjectKey.Valid && org.AvatarObjectKey.String != "", | |
| 127 | + "Error": errMsg, | |
| 128 | + "Success": success, | |
| 129 | + }) | |
| 130 | +} | |
| 131 | + | |
| 132 | +func orgSettingsProfilePath(org orgsdb.Org) string { | |
| 133 | + return "/organizations/" + org.Slug + "/settings/profile" | |
| 134 | +} | |
| 135 | + | |
| 136 | +func friendlyAvatarErr(err error) string { | |
| 137 | + switch { | |
| 138 | + case errors.Is(err, avatars.ErrTooLarge): | |
| 139 | + return "Avatar must be 5 MB or smaller." | |
| 140 | + case errors.Is(err, avatars.ErrUnsupported): | |
| 141 | + return "Avatar must be a PNG, JPEG, or GIF image." | |
| 142 | + case errors.Is(err, avatars.ErrDecompression): | |
| 143 | + return "Avatar dimensions are too large." | |
| 144 | + case errors.Is(err, avatars.ErrDecode): | |
| 145 | + return "Could not decode that image." | |
| 146 | + default: | |
| 147 | + return "Could not upload that avatar." | |
| 148 | + } | |
| 149 | +} | |
internal/web/handlers/orgs/avatar_test.goadded@@ -0,0 +1,192 @@ | ||
| 1 | +// SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | |
| 3 | +package orgs_test | |
| 4 | + | |
| 5 | +import ( | |
| 6 | + "bytes" | |
| 7 | + "context" | |
| 8 | + "image" | |
| 9 | + "image/color" | |
| 10 | + "image/png" | |
| 11 | + "io" | |
| 12 | + "log/slog" | |
| 13 | + "mime/multipart" | |
| 14 | + "net/http" | |
| 15 | + "net/http/httptest" | |
| 16 | + "net/url" | |
| 17 | + "strings" | |
| 18 | + "testing" | |
| 19 | + "testing/fstest" | |
| 20 | + | |
| 21 | + "github.com/go-chi/chi/v5" | |
| 22 | + | |
| 23 | + "github.com/tenseleyFlow/shithub/internal/infra/storage" | |
| 24 | + orgsdb "github.com/tenseleyFlow/shithub/internal/orgs/sqlc" | |
| 25 | + "github.com/tenseleyFlow/shithub/internal/testing/dbtest" | |
| 26 | + orgsh "github.com/tenseleyFlow/shithub/internal/web/handlers/orgs" | |
| 27 | + "github.com/tenseleyFlow/shithub/internal/web/middleware" | |
| 28 | + "github.com/tenseleyFlow/shithub/internal/web/render" | |
| 29 | +) | |
| 30 | + | |
| 31 | +func TestOrgAvatarUploadRoundTrip(t *testing.T) { | |
| 32 | + t.Parallel() | |
| 33 | + ctx := context.Background() | |
| 34 | + pool := dbtest.NewTestDB(t) | |
| 35 | + q := orgsdb.New() | |
| 36 | + viewerID := insertOrgAvatarUser(t, pool, "mfwolffe") | |
| 37 | + orgID := insertOrgAvatarOrg(t, pool, viewerID, "tenseleyFlow") | |
| 38 | + | |
| 39 | + tmplFS := fstest.MapFS{ | |
| 40 | + "_layout.html": {Data: []byte(`{{ define "layout" }}<html><body>{{ template "page" . }}</body></html>{{ end }}`)}, | |
| 41 | + "orgs/settings_profile.html": {Data: []byte(`{{ define "page" }}{{ with .Error }}ERROR={{ . }}{{ end }}<form action="/organizations/{{ .Org.Slug }}/settings/profile/avatar"><input name=csrf_token value="{{.CSRFToken}}"></form>{{ if .HasAvatar }}REMOVE=/organizations/{{ .Org.Slug }}/settings/profile/avatar/remove{{ end }}{{ end }}`)}, | |
| 42 | + "errors/403.html": {Data: []byte(`{{ define "page" }}403{{ end }}`)}, | |
| 43 | + "errors/404.html": {Data: []byte(`{{ define "page" }}404{{ end }}`)}, | |
| 44 | + "errors/500.html": {Data: []byte(`{{ define "page" }}500{{ end }}`)}, | |
| 45 | + } | |
| 46 | + rr, err := render.New(tmplFS, render.Options{}) | |
| 47 | + if err != nil { | |
| 48 | + t.Fatalf("render.New: %v", err) | |
| 49 | + } | |
| 50 | + h, err := orgsh.New(orgsh.Deps{ | |
| 51 | + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), | |
| 52 | + Render: rr, | |
| 53 | + Pool: pool, | |
| 54 | + ObjectStore: storage.NewMemoryStore(), | |
| 55 | + }) | |
| 56 | + if err != nil { | |
| 57 | + t.Fatalf("orgsh.New: %v", err) | |
| 58 | + } | |
| 59 | + r := chi.NewRouter() | |
| 60 | + r.Use(func(next http.Handler) http.Handler { | |
| 61 | + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 62 | + viewer := middleware.CurrentUser{ID: viewerID, Username: "mfwolffe"} | |
| 63 | + next.ServeHTTP(w, r.WithContext(middleware.WithCurrentUserForTest(r.Context(), viewer))) | |
| 64 | + }) | |
| 65 | + }) | |
| 66 | + h.MountCreate(r) | |
| 67 | + srv := httptest.NewServer(r) | |
| 68 | + t.Cleanup(srv.Close) | |
| 69 | + | |
| 70 | + cli := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { | |
| 71 | + return http.ErrUseLastResponse | |
| 72 | + }} | |
| 73 | + | |
| 74 | + resp, err := cli.Get(srv.URL + "/organizations/tenseleyFlow/settings/profile") | |
| 75 | + if err != nil { | |
| 76 | + t.Fatalf("GET settings: %v", err) | |
| 77 | + } | |
| 78 | + body, _ := io.ReadAll(resp.Body) | |
| 79 | + _ = resp.Body.Close() | |
| 80 | + if resp.StatusCode != http.StatusOK { | |
| 81 | + t.Fatalf("GET status=%d body=%s", resp.StatusCode, body) | |
| 82 | + } | |
| 83 | + if !strings.Contains(string(body), "/organizations/tenseleyFlow/settings/profile/avatar") { | |
| 84 | + t.Fatalf("expected upload form, got %s", body) | |
| 85 | + } | |
| 86 | + | |
| 87 | + resp = postOrgAvatar(t, cli, srv.URL+"/organizations/tenseleyFlow/settings/profile/avatar", makeOrgTestPNG(t)) | |
| 88 | + _ = resp.Body.Close() | |
| 89 | + if resp.StatusCode != http.StatusSeeOther { | |
| 90 | + t.Fatalf("upload status=%d", resp.StatusCode) | |
| 91 | + } | |
| 92 | + if got := resp.Header.Get("Location"); got != "/organizations/tenseleyFlow/settings/profile" { | |
| 93 | + t.Fatalf("upload Location=%q", got) | |
| 94 | + } | |
| 95 | + org, err := q.GetOrgByID(ctx, pool, orgID) | |
| 96 | + if err != nil { | |
| 97 | + t.Fatalf("GetOrgByID: %v", err) | |
| 98 | + } | |
| 99 | + if !org.AvatarObjectKey.Valid || !strings.HasPrefix(org.AvatarObjectKey.String, "avatars/orgs/") { | |
| 100 | + t.Fatalf("avatar key=%q valid=%v", org.AvatarObjectKey.String, org.AvatarObjectKey.Valid) | |
| 101 | + } | |
| 102 | + | |
| 103 | + resp, err = cli.PostForm(srv.URL+"/organizations/tenseleyFlow/settings/profile/avatar/remove", url.Values{}) | |
| 104 | + if err != nil { | |
| 105 | + t.Fatalf("POST remove: %v", err) | |
| 106 | + } | |
| 107 | + _ = resp.Body.Close() | |
| 108 | + if resp.StatusCode != http.StatusSeeOther { | |
| 109 | + t.Fatalf("remove status=%d", resp.StatusCode) | |
| 110 | + } | |
| 111 | + org, err = q.GetOrgByID(ctx, pool, orgID) | |
| 112 | + if err != nil { | |
| 113 | + t.Fatalf("GetOrgByID after remove: %v", err) | |
| 114 | + } | |
| 115 | + if org.AvatarObjectKey.Valid { | |
| 116 | + t.Fatalf("expected cleared avatar key, got %q", org.AvatarObjectKey.String) | |
| 117 | + } | |
| 118 | +} | |
| 119 | + | |
| 120 | +func postOrgAvatar(t *testing.T, cli *http.Client, endpoint string, png []byte) *http.Response { | |
| 121 | + t.Helper() | |
| 122 | + body := &bytes.Buffer{} | |
| 123 | + mw := multipart.NewWriter(body) | |
| 124 | + part, err := mw.CreateFormFile("avatar", "avatar.png") | |
| 125 | + if err != nil { | |
| 126 | + t.Fatalf("CreateFormFile: %v", err) | |
| 127 | + } | |
| 128 | + if _, err := part.Write(png); err != nil { | |
| 129 | + t.Fatalf("write png: %v", err) | |
| 130 | + } | |
| 131 | + if err := mw.Close(); err != nil { | |
| 132 | + t.Fatalf("close multipart: %v", err) | |
| 133 | + } | |
| 134 | + req, err := http.NewRequest(http.MethodPost, endpoint, body) | |
| 135 | + if err != nil { | |
| 136 | + t.Fatalf("NewRequest: %v", err) | |
| 137 | + } | |
| 138 | + req.Header.Set("Content-Type", mw.FormDataContentType()) | |
| 139 | + resp, err := cli.Do(req) | |
| 140 | + if err != nil { | |
| 141 | + t.Fatalf("POST avatar: %v", err) | |
| 142 | + } | |
| 143 | + return resp | |
| 144 | +} | |
| 145 | + | |
| 146 | +func makeOrgTestPNG(t *testing.T) []byte { | |
| 147 | + t.Helper() | |
| 148 | + img := image.NewRGBA(image.Rect(0, 0, 64, 64)) | |
| 149 | + for y := 0; y < 64; y++ { | |
| 150 | + for x := 0; x < 64; x++ { | |
| 151 | + img.Set(x, y, color.RGBA{R: 80, G: 30, B: 110, A: 255}) | |
| 152 | + } | |
| 153 | + } | |
| 154 | + buf := &bytes.Buffer{} | |
| 155 | + if err := png.Encode(buf, img); err != nil { | |
| 156 | + t.Fatalf("encode png: %v", err) | |
| 157 | + } | |
| 158 | + return buf.Bytes() | |
| 159 | +} | |
| 160 | + | |
| 161 | +func insertOrgAvatarUser(t *testing.T, db orgsdb.DBTX, username string) int64 { | |
| 162 | + t.Helper() | |
| 163 | + var id int64 | |
| 164 | + if err := db.QueryRow(context.Background(), | |
| 165 | + `INSERT INTO users (username, password_hash) VALUES ($1, $2) RETURNING id`, | |
| 166 | + username, | |
| 167 | + "$argon2id$v=19$m=16384,t=1,p=1$AAAAAAAAAAAAAAAA$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", | |
| 168 | + ).Scan(&id); err != nil { | |
| 169 | + t.Fatalf("insert user: %v", err) | |
| 170 | + } | |
| 171 | + return id | |
| 172 | +} | |
| 173 | + | |
| 174 | +func insertOrgAvatarOrg(t *testing.T, db orgsdb.DBTX, userID int64, slug string) int64 { | |
| 175 | + t.Helper() | |
| 176 | + var orgID int64 | |
| 177 | + if err := db.QueryRow(context.Background(), | |
| 178 | + `INSERT INTO orgs (slug, display_name, created_by_user_id) | |
| 179 | + VALUES ($1, $1, $2) | |
| 180 | + RETURNING id`, | |
| 181 | + slug, userID, | |
| 182 | + ).Scan(&orgID); err != nil { | |
| 183 | + t.Fatalf("insert org: %v", err) | |
| 184 | + } | |
| 185 | + if _, err := db.Exec(context.Background(), | |
| 186 | + `INSERT INTO org_members (org_id, user_id, role) VALUES ($1, $2, 'owner')`, | |
| 187 | + orgID, userID, | |
| 188 | + ); err != nil { | |
| 189 | + t.Fatalf("insert org member: %v", err) | |
| 190 | + } | |
| 191 | + return orgID | |
| 192 | +} | |
internal/web/handlers/orgs/orgs.gomodified@@ -29,6 +29,7 @@ import ( | ||
| 29 | 29 | "github.com/jackc/pgx/v5/pgxpool" |
| 30 | 30 | |
| 31 | 31 | authemail "github.com/tenseleyFlow/shithub/internal/auth/email" |
| 32 | + "github.com/tenseleyFlow/shithub/internal/infra/storage" | |
| 32 | 33 | "github.com/tenseleyFlow/shithub/internal/orgs" |
| 33 | 34 | orgsdb "github.com/tenseleyFlow/shithub/internal/orgs/sqlc" |
| 34 | 35 | "github.com/tenseleyFlow/shithub/internal/web/middleware" |
@@ -44,6 +45,7 @@ type Deps struct { | ||
| 44 | 45 | EmailFrom string |
| 45 | 46 | SiteName string |
| 46 | 47 | BaseURL string |
| 48 | + ObjectStore storage.ObjectStore | |
| 47 | 49 | } |
| 48 | 50 | |
| 49 | 51 | // Handlers groups the org surface handlers. |
@@ -69,6 +71,9 @@ func New(d Deps) (*Handlers, error) { | ||
| 69 | 71 | func (h *Handlers) MountCreate(r chi.Router) { |
| 70 | 72 | r.Get("/organizations/new", h.newForm) |
| 71 | 73 | r.Post("/organizations", h.createSubmit) |
| 74 | + r.Get("/organizations/{org}/settings/profile", h.settingsProfile) | |
| 75 | + r.Post("/organizations/{org}/settings/profile/avatar", h.settingsAvatarUpload) | |
| 76 | + r.Post("/organizations/{org}/settings/profile/avatar/remove", h.settingsAvatarRemove) | |
| 72 | 77 | } |
| 73 | 78 | |
| 74 | 79 | // MountOrgRoutes registers the per-org surface under /{org}/people |
internal/web/handlers/profile/profile.gomodified@@ -29,6 +29,7 @@ import ( | ||
| 29 | 29 | "github.com/tenseleyFlow/shithub/internal/avatars" |
| 30 | 30 | "github.com/tenseleyFlow/shithub/internal/infra/storage" |
| 31 | 31 | "github.com/tenseleyFlow/shithub/internal/orgs" |
| 32 | + orgsdb "github.com/tenseleyFlow/shithub/internal/orgs/sqlc" | |
| 32 | 33 | usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc" |
| 33 | 34 | "github.com/tenseleyFlow/shithub/internal/web/middleware" |
| 34 | 35 | "github.com/tenseleyFlow/shithub/internal/web/render" |
@@ -211,38 +212,32 @@ func (h *Handlers) renderUnavailable(w http.ResponseWriter, r *http.Request, use | ||
| 211 | 212 | |
| 212 | 213 | // ------------------------------ avatar ---------------------------------- |
| 213 | 214 | |
| 214 | -// serveAvatar resolves the username, then either streams the uploaded | |
| 215 | -// avatar from object storage or returns the deterministic SVG identicon. | |
| 215 | +// serveAvatar resolves the slug, then either streams the uploaded | |
| 216 | +// user/org avatar from object storage or returns the deterministic SVG | |
| 217 | +// identicon. | |
| 216 | 218 | // |
| 217 | 219 | // Implementation notes: |
| 218 | 220 | // - Lookup-by-username happens on every request. At our scale this is |
| 219 | 221 | // fine; if the avatar route becomes hot we can add an LRU. |
| 220 | -// - Suspended/deleted users get the identicon (NOT a 404) so the | |
| 221 | -// suspended-page UX still has *something* to render in the header. | |
| 222 | +// - Missing, suspended, or deleted principals get the identicon (NOT a | |
| 223 | +// 404) so avatar URLs leak less existence state. | |
| 222 | 224 | // - Cache-Control: long max-age + immutable. Avatar contents are |
| 223 | -// content-addressed at upload time (S10 stores under | |
| 224 | -// avatars/<owner>/<sha256>.<ext>) so the URL changes when the image | |
| 225 | -// does, making "immutable" safe. | |
| 225 | +// content-addressed at upload time so the URL changes when the image | |
| 226 | +// changes, making "immutable" safe. | |
| 226 | 227 | func (h *Handlers) serveAvatar(w http.ResponseWriter, r *http.Request) { |
| 227 | - username := chi.URLParam(r, "username") | |
| 228 | - user, err := h.q.GetUserByUsername(r.Context(), h.d.Pool, username) | |
| 229 | - if err != nil { | |
| 230 | - // Don't 404 on missing user — silently fall through to the | |
| 231 | - // identicon. Avatar URLs leak less existence info that way. | |
| 232 | - writeIdenticon(w, r, username) | |
| 233 | - return | |
| 234 | - } | |
| 235 | - if !user.AvatarObjectKey.Valid || user.AvatarObjectKey.String == "" { | |
| 236 | - writeIdenticon(w, r, user.Username) | |
| 228 | + slug := chi.URLParam(r, "username") | |
| 229 | + key, seed := h.avatarKeyForSlug(r, slug) | |
| 230 | + if key == "" { | |
| 231 | + writeIdenticon(w, r, seed) | |
| 237 | 232 | return |
| 238 | 233 | } |
| 239 | 234 | if h.d.ObjectStore == nil { |
| 240 | - writeIdenticon(w, r, user.Username) | |
| 235 | + writeIdenticon(w, r, seed) | |
| 241 | 236 | return |
| 242 | 237 | } |
| 243 | - rc, meta, err := h.d.ObjectStore.Get(r.Context(), user.AvatarObjectKey.String) | |
| 238 | + rc, meta, err := h.d.ObjectStore.Get(r.Context(), key) | |
| 244 | 239 | if err != nil { |
| 245 | - writeIdenticon(w, r, user.Username) | |
| 240 | + writeIdenticon(w, r, seed) | |
| 246 | 241 | return |
| 247 | 242 | } |
| 248 | 243 | defer func() { _ = rc.Close() }() |
@@ -257,6 +252,24 @@ func (h *Handlers) serveAvatar(w http.ResponseWriter, r *http.Request) { | ||
| 257 | 252 | _, _ = io.Copy(w, rc) |
| 258 | 253 | } |
| 259 | 254 | |
| 255 | +func (h *Handlers) avatarKeyForSlug(r *http.Request, slug string) (string, string) { | |
| 256 | + user, err := h.q.GetUserByUsername(r.Context(), h.d.Pool, slug) | |
| 257 | + if err == nil { | |
| 258 | + if user.AvatarObjectKey.Valid { | |
| 259 | + return user.AvatarObjectKey.String, user.Username | |
| 260 | + } | |
| 261 | + return "", user.Username | |
| 262 | + } | |
| 263 | + org, err := orgsdb.New().GetOrgBySlug(r.Context(), h.d.Pool, slug) | |
| 264 | + if err == nil { | |
| 265 | + if org.AvatarObjectKey.Valid { | |
| 266 | + return org.AvatarObjectKey.String, org.Slug | |
| 267 | + } | |
| 268 | + return "", org.Slug | |
| 269 | + } | |
| 270 | + return "", slug | |
| 271 | +} | |
| 272 | + | |
| 260 | 273 | func writeIdenticon(w http.ResponseWriter, _ *http.Request, username string) { |
| 261 | 274 | w.Header().Set("Content-Type", "image/svg+xml") |
| 262 | 275 | // Identicons depend ONLY on the username; cache forever. |
internal/web/handlers/profile/profile_test.gomodified@@ -19,6 +19,7 @@ import ( | ||
| 19 | 19 | "github.com/jackc/pgx/v5/pgxpool" |
| 20 | 20 | |
| 21 | 21 | authpkg "github.com/tenseleyFlow/shithub/internal/auth" |
| 22 | + "github.com/tenseleyFlow/shithub/internal/infra/storage" | |
| 22 | 23 | "github.com/tenseleyFlow/shithub/internal/testing/dbtest" |
| 23 | 24 | usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc" |
| 24 | 25 | profileh "github.com/tenseleyFlow/shithub/internal/web/handlers/profile" |
@@ -33,6 +34,10 @@ type profileEnv struct { | ||
| 33 | 34 | } |
| 34 | 35 | |
| 35 | 36 | func setupProfileEnv(t *testing.T) *profileEnv { |
| 37 | + return setupProfileEnvWithStore(t, nil) | |
| 38 | +} | |
| 39 | + | |
| 40 | +func setupProfileEnvWithStore(t *testing.T, objectStore storage.ObjectStore) *profileEnv { | |
| 36 | 41 | t.Helper() |
| 37 | 42 | pool := dbtest.NewTestDB(t) |
| 38 | 43 | |
@@ -53,6 +58,7 @@ func setupProfileEnv(t *testing.T) *profileEnv { | ||
| 53 | 58 | h, err := profileh.New(profileh.Deps{ |
| 54 | 59 | Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), |
| 55 | 60 | Render: rr, Pool: pool, |
| 61 | + ObjectStore: objectStore, | |
| 56 | 62 | }) |
| 57 | 63 | if err != nil { |
| 58 | 64 | t.Fatalf("profileh.New: %v", err) |
@@ -501,6 +507,40 @@ func TestProfile_AvatarReturnsIdenticonForNoKey(t *testing.T) { | ||
| 501 | 507 | } |
| 502 | 508 | } |
| 503 | 509 | |
| 510 | +func TestProfile_AvatarStreamsOrgAvatar(t *testing.T) { | |
| 511 | + t.Parallel() | |
| 512 | + store := storage.NewMemoryStore() | |
| 513 | + env := setupProfileEnvWithStore(t, store) | |
| 514 | + owner := env.insertUser(t, "alice", "Alice", "") | |
| 515 | + orgID := env.insertOrg(t, "acme", "Acme", "", owner) | |
| 516 | + key := "avatars/orgs/acme/test.png" | |
| 517 | + if _, err := store.Put(context.Background(), key, strings.NewReader("org-avatar"), storage.PutOpts{ | |
| 518 | + ContentType: "image/png", | |
| 519 | + ContentLength: int64(len("org-avatar")), | |
| 520 | + }); err != nil { | |
| 521 | + t.Fatalf("store.Put: %v", err) | |
| 522 | + } | |
| 523 | + if _, err := env.pool.Exec(context.Background(), | |
| 524 | + `UPDATE orgs SET avatar_object_key = $1 WHERE id = $2`, | |
| 525 | + key, orgID, | |
| 526 | + ); err != nil { | |
| 527 | + t.Fatalf("set org avatar: %v", err) | |
| 528 | + } | |
| 529 | + | |
| 530 | + resp, _ := http.Get(env.srv.URL + "/avatars/acme") | |
| 531 | + defer func() { _ = resp.Body.Close() }() | |
| 532 | + if resp.StatusCode != http.StatusOK { | |
| 533 | + t.Fatalf("status %d", resp.StatusCode) | |
| 534 | + } | |
| 535 | + if resp.Header.Get("Content-Type") != "image/png" { | |
| 536 | + t.Fatalf("content-type %q", resp.Header.Get("Content-Type")) | |
| 537 | + } | |
| 538 | + body, _ := io.ReadAll(resp.Body) | |
| 539 | + if string(body) != "org-avatar" { | |
| 540 | + t.Fatalf("body=%q", body) | |
| 541 | + } | |
| 542 | +} | |
| 543 | + | |
| 504 | 544 | // TestReservedNameList_HasReasonableContents is the route-audit test: it |
| 505 | 545 | // asserts every top-level path segment shithub registers as of S09 is on |
| 506 | 546 | // the reserved list. When a future sprint adds a new top-level route, |
internal/web/orgs_wiring.gomodified@@ -14,6 +14,7 @@ import ( | ||
| 14 | 14 | |
| 15 | 15 | "github.com/tenseleyFlow/shithub/internal/auth/email" |
| 16 | 16 | "github.com/tenseleyFlow/shithub/internal/infra/config" |
| 17 | + "github.com/tenseleyFlow/shithub/internal/infra/storage" | |
| 17 | 18 | orgshandlers "github.com/tenseleyFlow/shithub/internal/web/handlers/orgs" |
| 18 | 19 | "github.com/tenseleyFlow/shithub/internal/web/render" |
| 19 | 20 | ) |
@@ -23,6 +24,7 @@ import ( | ||
| 23 | 24 | func buildOrgHandlers( |
| 24 | 25 | cfg config.Config, |
| 25 | 26 | pool *pgxpool.Pool, |
| 27 | + objectStore storage.ObjectStore, | |
| 26 | 28 | tmplFS fs.FS, |
| 27 | 29 | logger *slog.Logger, |
| 28 | 30 | ) (*orgshandlers.Handlers, error) { |
@@ -39,6 +41,7 @@ func buildOrgHandlers( | ||
| 39 | 41 | EmailFrom: cfg.Auth.EmailFrom, |
| 40 | 42 | SiteName: cfg.Auth.SiteName, |
| 41 | 43 | BaseURL: cfg.Auth.BaseURL, |
| 44 | + ObjectStore: objectStore, | |
| 42 | 45 | }) |
| 43 | 46 | } |
| 44 | 47 | |
internal/web/server.gomodified@@ -238,7 +238,7 @@ func Run(ctx context.Context, opts Options) error { | ||
| 238 | 238 | deps.NotifPublicMounter = notifH.MountPublic |
| 239 | 239 | |
| 240 | 240 | // S30 — orgs. |
| 241 | - orgH, err := buildOrgHandlers(cfg, pool, deps.TemplatesFS, logger) | |
| 241 | + orgH, err := buildOrgHandlers(cfg, pool, objectStore, deps.TemplatesFS, logger) | |
| 242 | 242 | if err != nil { |
| 243 | 243 | return fmt.Errorf("org handlers: %w", err) |
| 244 | 244 | } |
internal/web/templates/orgs/profile.htmlmodified@@ -13,7 +13,7 @@ | ||
| 13 | 13 | </ul> |
| 14 | 14 | </div> |
| 15 | 15 | <div class="shithub-org-hero-actions"> |
| 16 | - {{ if .IsOwner }}<button type="button" class="shithub-button" disabled>Settings</button>{{ else }}<button type="button" class="shithub-button" disabled>Follow</button>{{ end }} | |
| 16 | + {{ if .IsOwner }}<a href="/organizations/{{ .Org.Slug }}/settings/profile" class="shithub-button">Settings</a>{{ else }}<button type="button" class="shithub-button" disabled>Follow</button>{{ end }} | |
| 17 | 17 | </div> |
| 18 | 18 | </div> |
| 19 | 19 | </header> |
@@ -27,7 +27,7 @@ | ||
| 27 | 27 | <a href="/{{ .Org.Slug }}/people" class="shithub-org-nav-item">{{ octicon "person" }} People <span class="shithub-tab-count">{{ .MemberCount }}</span></a> |
| 28 | 28 | <span class="shithub-org-nav-item is-disabled" aria-disabled="true">{{ octicon "shield-check" }} Security and quality</span> |
| 29 | 29 | <span class="shithub-org-nav-item is-disabled" aria-disabled="true">{{ octicon "pulse" }} Insights</span> |
| 30 | - {{ if .IsOwner }}<span class="shithub-org-nav-item is-disabled" aria-disabled="true">{{ octicon "gear" }} Settings</span>{{ end }} | |
| 30 | + {{ if .IsOwner }}<a href="/organizations/{{ .Org.Slug }}/settings/profile" class="shithub-org-nav-item">{{ octicon "gear" }} Settings</a>{{ end }} | |
| 31 | 31 | </nav> |
| 32 | 32 | |
| 33 | 33 | {{ if .Org.SuspendedAt.Valid }} |
internal/web/templates/orgs/settings_profile.htmladded@@ -0,0 +1,36 @@ | ||
| 1 | +{{ define "page" -}} | |
| 2 | +<section class="shithub-org-settings"> | |
| 3 | + <header class="shithub-org-profile-head"> | |
| 4 | + <h1>{{ .Org.DisplayName }} · Profile settings</h1> | |
| 5 | + <p class="shithub-meta">@{{ .Org.Slug }}</p> | |
| 6 | + </header> | |
| 7 | + | |
| 8 | + {{ with .Error }}<p class="shithub-flash shithub-flash-error" role="alert">{{ . }}</p>{{ end }} | |
| 9 | + {{ with .Success }}<p class="shithub-flash shithub-flash-success" role="status">{{ . }}</p>{{ end }} | |
| 10 | + | |
| 11 | + <section class="shithub-settings-section" aria-labelledby="org-avatar-heading"> | |
| 12 | + <h2 id="org-avatar-heading">Profile picture</h2> | |
| 13 | + <div class="shithub-profile-picture-settings"> | |
| 14 | + <img src="{{ .AvatarURL }}" alt="" class="shithub-profile-edit-avatar"> | |
| 15 | + {{ if .AvatarUploadEnabled }} | |
| 16 | + <form method="POST" action="/organizations/{{ .Org.Slug }}/settings/profile/avatar" enctype="multipart/form-data" novalidate> | |
| 17 | + <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}"> | |
| 18 | + <label> | |
| 19 | + <span>Upload new picture</span> | |
| 20 | + <input type="file" name="avatar" accept="image/png,image/jpeg,image/gif" required> | |
| 21 | + </label> | |
| 22 | + <button type="submit" class="shithub-button shithub-button-primary">Upload</button> | |
| 23 | + </form> | |
| 24 | + {{ if .HasAvatar }} | |
| 25 | + <form method="POST" action="/organizations/{{ .Org.Slug }}/settings/profile/avatar/remove" novalidate> | |
| 26 | + <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}"> | |
| 27 | + <button type="submit" class="shithub-button shithub-button-danger">Remove picture</button> | |
| 28 | + </form> | |
| 29 | + {{ end }} | |
| 30 | + {{ else }} | |
| 31 | + <p class="shithub-meta">Avatar uploads are disabled until object storage is configured.</p> | |
| 32 | + {{ end }} | |
| 33 | + </div> | |
| 34 | + </section> | |
| 35 | +</section> | |
| 36 | +{{- end }} | |
internal/web/templates/settings/organizations.htmlmodified@@ -32,7 +32,7 @@ | ||
| 32 | 32 | <div class="shithub-settings-org-actions"> |
| 33 | 33 | {{ if .CanManage }} |
| 34 | 34 | <button type="button" class="shithub-button shithub-button-small" disabled>Compare plans</button> |
| 35 | - <a href="/{{ .Slug }}/people" class="shithub-button shithub-button-small">Settings</a> | |
| 35 | + <a href="/organizations/{{ .Slug }}/settings/profile" class="shithub-button shithub-button-small">Settings</a> | |
| 36 | 36 | {{ end }} |
| 37 | 37 | <button type="button" class="shithub-button shithub-button-small shithub-button-danger" disabled>Leave</button> |
| 38 | 38 | </div> |