Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make file expiry for guest links configurable when creating guest links #613

Merged
merged 18 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5aef9c8
differentiate between url and file expiration for guest links, use fi…
Balk-Z Dec 6, 2024
ddc9024
integrate file expiration into frontend and add to json payload
Balk-Z Dec 6, 2024
ad20ab8
add database changes for storing file expiration information
Balk-Z Dec 6, 2024
2c3d594
remove a debug statement
Balk-Z Dec 6, 2024
cc0bcad
make guest file expiration fixed so that persisting of file doesn't d…
Balk-Z Dec 6, 2024
f383f77
ensure existing databases are migrated properly
Balk-Z Dec 6, 2024
8c8fea6
remove unused formatting function and resolve some formatting weirdness
Balk-Z Dec 6, 2024
a8cb111
Revert "make guest file expiration fixed so that persisting of file d…
Balk-Z Dec 7, 2024
6a32552
use ExpirationTime instead of string
Balk-Z Dec 7, 2024
4bfd557
use FileLifetime instead of ExpirationTime
Balk-Z Dec 7, 2024
d70047c
ensure databases lacking file_expiration_time values report correct v…
Balk-Z Dec 7, 2024
8e90dc8
rename FileExpires to FileLifetime, correct spelling of fileLifetime
Balk-Z Dec 8, 2024
89ee870
resolve the sql null error without changing the sql query
Balk-Z Dec 8, 2024
495b59b
rename fileExpirationTime to fileLifetime
Balk-Z Dec 8, 2024
b27a283
pass FileLifetime from frontend instead of using ExpirationTimes
Balk-Z Dec 8, 2024
2a8cbc1
add file_lifetime test for infinite file lifetime
Balk-Z Dec 8, 2024
a89e8ac
add tests for upload of guest files and guest link creation where Fil…
Balk-Z Dec 8, 2024
8e8b590
implement all review comments, namely refactoring and adjusting guest…
Balk-Z Dec 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions handlers/guest_links.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ func (s Server) guestLinksDelete() http.HandlerFunc {
func guestLinkFromRequest(r *http.Request) (picoshare.GuestLink, error) {
var payload struct {
Label string `json:"label"`
Expiration string `json:"expirationTime"`
UrlExpiration string `json:"urlExpirationTime"`
FileExpiration string `json:"fileLifetime"`
MaxFileBytes *uint64 `json:"maxFileBytes"`
MaxFileUploads *int `json:"maxFileUploads"`
}
Expand All @@ -82,7 +83,12 @@ func guestLinkFromRequest(r *http.Request) (picoshare.GuestLink, error) {
return picoshare.GuestLink{}, err
}

expiration, err := parse.Expiration(payload.Expiration)
urlExpiration, err := parse.Expiration(payload.UrlExpiration)
if err != nil {
return picoshare.GuestLink{}, err
}

fileExpiration, err := parse.GuestFileLifeTime(payload.FileExpiration)
if err != nil {
return picoshare.GuestLink{}, err
}
Expand All @@ -99,7 +105,8 @@ func guestLinkFromRequest(r *http.Request) (picoshare.GuestLink, error) {

return picoshare.GuestLink{
Label: label,
Expires: expiration,
UrlExpires: urlExpiration,
FileLifetime: fileExpiration,
MaxFileBytes: maxFileBytes,
MaxFileUploads: maxFileUploads,
}, nil
Expand Down
74 changes: 56 additions & 18 deletions handlers/guest_links_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ func TestGuestLinksPostAcceptsValidRequest(t *testing.T) {
description: "minimally populated request",
payload: `{
"label": null,
"expirationTime":"2030-01-02T03:04:25Z",
"urlExpirationTime":"2030-01-02T03:04:25Z",
"fileLifetime":"876000h0m0s",
"maxFileBytes": null,
"maxFileUploads": null
}`,
expected: picoshare.GuestLink{
Label: picoshare.GuestLinkLabel(""),
Expires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
UrlExpires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
FileLifetime: picoshare.FileLifetimeInfinite,
MaxFileBytes: picoshare.GuestUploadUnlimitedFileSize,
MaxFileUploads: picoshare.GuestUploadUnlimitedFileUploads,
},
Expand All @@ -42,13 +44,49 @@ func TestGuestLinksPostAcceptsValidRequest(t *testing.T) {
description: "fully populated request",
payload: `{
"label": "For my good pal, Maurice",
"expirationTime":"2030-01-02T03:04:25Z",
"urlExpirationTime":"2030-01-02T03:04:25Z",
"fileLifetime":"876000h0m0s",
"maxFileBytes": 1048576,
"maxFileUploads": 1
}`,
expected: picoshare.GuestLink{
Label: picoshare.GuestLinkLabel("For my good pal, Maurice"),
Expires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
UrlExpires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
FileLifetime: picoshare.FileLifetimeInfinite,
MaxFileBytes: makeGuestUploadMaxFileBytes(1048576),
MaxFileUploads: makeGuestUploadCountLimit(1),
},
},
{
description: "guest file expires in 1 day",
payload: `{
"label": "For my good pal, Maurice",
"urlExpirationTime":"2030-01-02T03:04:25Z",
"fileLifetime":"24h0m0s",
"maxFileBytes": 1048576,
"maxFileUploads": 1
}`,
expected: picoshare.GuestLink{
Label: picoshare.GuestLinkLabel("For my good pal, Maurice"),
UrlExpires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
FileLifetime: picoshare.NewFileLifetimeInDays(1),
MaxFileBytes: makeGuestUploadMaxFileBytes(1048576),
MaxFileUploads: makeGuestUploadCountLimit(1),
},
},
{
description: "guest file expires in 30 day",
payload: `{
"label": "For my good pal, Maurice",
"urlExpirationTime":"2030-01-02T03:04:25Z",
"fileLifetime":"720h0m0s",
"maxFileBytes": 1048576,
"maxFileUploads": 1
}`,
expected: picoshare.GuestLink{
Label: picoshare.GuestLinkLabel("For my good pal, Maurice"),
UrlExpires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
FileLifetime: picoshare.NewFileLifetimeInDays(30),
MaxFileBytes: makeGuestUploadMaxFileBytes(1048576),
MaxFileUploads: makeGuestUploadCountLimit(1),
},
Expand Down Expand Up @@ -117,7 +155,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "invalid label field (non-string)",
payload: `{
"label": 5,
"expirationTime":"2025-01-01T00:00:00Z",
"urlExpirationTime":"2025-01-01T00:00:00Z",
"maxFileBytes": null,
"maxFileUploads": null
}`,
Expand All @@ -126,13 +164,13 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "invalid label field (too long)",
payload: fmt.Sprintf(`{
"label": "%s",
"expirationTime":"2025-01-01T00:00:00Z",
"urlExpirationTime":"2025-01-01T00:00:00Z",
"maxFileBytes": null,
"maxFileUploads": null
}`, strings.Repeat("A", 201)),
},
{
description: "missing expirationTime field",
description: "missing urlExpirationTime field",
payload: `{
"label": null,
"maxFileBytes": null,
Expand All @@ -143,7 +181,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "invalid expirationTime field",
payload: `{
"label": null,
"expirationTime": 25,
"urlExpirationTime": 25,
"maxFileBytes": null,
"maxFileUploads": null
}`,
Expand All @@ -152,7 +190,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "negative maxFileBytes field",
payload: `{
"label": null,
"expirationTime":"2025-01-01T00:00:00Z",
"urlExpirationTime":"2025-01-01T00:00:00Z",
"maxFileBytes": -5,
"maxFileUploads": null
}`,
Expand All @@ -161,7 +199,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "decimal maxFileBytes field",
payload: `{
"label": null,
"expirationTime":"2025-01-01T00:00:00Z",
"urlExpirationTime":"2025-01-01T00:00:00Z",
"maxFileBytes": 1.5,
"maxFileUploads": null
}`,
Expand All @@ -170,7 +208,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "too low a maxFileBytes field",
payload: `{
"label": null,
"expirationTime":"2025-01-01T00:00:00Z",
"urlExpirationTime":"2025-01-01T00:00:00Z",
"maxFileBytes": 1,
"maxFileUploads": null
}`,
Expand All @@ -179,7 +217,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "zero maxFileBytes field",
payload: `{
"label": null,
"expirationTime":"2025-01-01T00:00:00Z",
"urlExpirationTime":"2025-01-01T00:00:00Z",
"maxFileBytes": 0,
"maxFileUploads": null
}`,
Expand All @@ -188,7 +226,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "negative maxFileUploads field",
payload: `{
"label": null,
"expirationTime":"2025-01-01T00:00:00Z",
"urlExpirationTime":"2025-01-01T00:00:00Z",
"maxFileBytes": null,
"maxFileUploads": -5
}`,
Expand All @@ -197,7 +235,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "decimal maxFileUploads field",
payload: `{
"label": null,
"expirationTime":"2025-01-01T00:00:00Z",
"urlExpirationTime":"2025-01-01T00:00:00Z",
"maxFileBytes": null,
"maxFileUploads": 1.5
}`,
Expand All @@ -206,7 +244,7 @@ func TestGuestLinksPostRejectsInvalidRequest(t *testing.T) {
description: "zero maxFileUploads field",
payload: `{
"label": null,
"expirationTime":"2025-01-01T00:00:00Z",
"urlExpirationTime":"2025-01-01T00:00:00Z",
"maxFileBytes": null,
"maxFileUploads": 0
}`,
Expand Down Expand Up @@ -245,9 +283,9 @@ func makeGuestUploadCountLimit(i int) picoshare.GuestUploadCountLimit {
func TestDeleteExistingGuestLink(t *testing.T) {
dataStore := test_sqlite.New()
dataStore.InsertGuestLink(picoshare.GuestLink{
ID: picoshare.GuestLinkID("abcdefgh23456789"),
Created: time.Now(),
Expires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
ID: picoshare.GuestLinkID("abcdefgh23456789"),
Created: time.Now(),
UrlExpires: mustParseExpirationTime("2030-01-02T03:04:25Z"),
})

s := handlers.New(mockAuthenticator{}, &dataStore, nilSpaceChecker, nilGarbageCollector)
Expand Down
22 changes: 22 additions & 0 deletions handlers/parse/guest_link.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
package parse

import (
"errors"
"fmt"
"time"

"github.com/mtlynch/picoshare/v2/picoshare"
)

// Arbitrary limit to prevent too-long labels in the UI.
const MaxGuestLinkLabelLength = 200
const validTimeUnits = "1ns, 1us (or 1µs), 1ms, 1s, 1m, 1h"
Balk-Z marked this conversation as resolved.
Show resolved Hide resolved

var ErrGuestLinkLabelTooLong = fmt.Errorf("label too long - limit %d characters", MaxGuestLinkLabelLength)
var ErrFileLifeTimeUnrecognizedFormat = fmt.Errorf("unrecognized format for file life time, must be in %s format", validTimeUnits)
Balk-Z marked this conversation as resolved.
Show resolved Hide resolved
var ErrFileLifeTimeTooShort = errors.New("file life time must be at least one hour in the future")

func GuestLinkLabel(label string) (picoshare.GuestLinkLabel, error) {
if len(label) > MaxGuestLinkLabelLength {
Expand All @@ -18,3 +23,20 @@ func GuestLinkLabel(label string) (picoshare.GuestLinkLabel, error) {

return picoshare.GuestLinkLabel(label), nil
}

func GuestFileLifeTime(fileLifetimeRaw string) (picoshare.FileLifetime, error) {
Balk-Z marked this conversation as resolved.
Show resolved Hide resolved
dur, err := time.ParseDuration(fileLifetimeRaw)
if err != nil {
return picoshare.FileLifetime{}, ErrFileLifeTimeUnrecognizedFormat
}

if picoshare.NewFileLifetimeFromDuration(dur) == picoshare.FileLifetimeInfinite {
return picoshare.FileLifetimeInfinite, nil
}

if dur < (time.Hour * 1) {
return picoshare.FileLifetime{}, ErrFileLifeTimeTooShort
}

return picoshare.NewFileLifetimeFromDuration(dur), nil
}
6 changes: 4 additions & 2 deletions handlers/static/js/controllers/guestLinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

export async function guestLinkNew(
label,
expirationTime,
urlExpirationTime,
fileLifetime,
maxFileBytes,
maxFileUploads
) {
Expand All @@ -11,7 +12,8 @@ export async function guestLinkNew(
credentials: "include",
body: JSON.stringify({
label,
expirationTime,
urlExpirationTime,
fileLifetime,
maxFileBytes,
maxFileUploads,
}),
Expand Down
27 changes: 25 additions & 2 deletions handlers/templates/pages/guest-link-create.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

const labelInput = document.getElementById("label");
const expirationSelect = document.getElementById("expiration-select");
const expirationSelectFile = document.getElementById(
Balk-Z marked this conversation as resolved.
Show resolved Hide resolved
"expiration-select-file"
);
const maxFileBytesInput = document.getElementById("max-file-size");
const fileUploadLimitInput = document.getElementById("file-upload-limit");
const createLinkForm = document.getElementById("create-guest-link-form");
Expand All @@ -22,7 +25,8 @@
function guestLinkFromInputs() {
return {
label: labelInput.value || null,
expirationTime: expirationSelect.value,
urlExpirationTime: expirationSelect.value,
fileLifetime: expirationSelectFile.value,
maxFileBytes: maxFileBytesInput.valueAsNumber
? megabytesToBytes(maxFileBytesInput.valueAsNumber)
: null,
Expand All @@ -42,7 +46,8 @@
const guestLink = guestLinkFromInputs();
guestLinkNew(
guestLink.label,
guestLink.expirationTime,
guestLink.urlExpirationTime,
guestLink.fileLifetime,
guestLink.maxFileBytes,
guestLink.maxFileUploads
)
Expand Down Expand Up @@ -102,6 +107,24 @@ <h1 class="title">Create Guest Link</h1>
</div>
</div>

<div class="field my-5">
<label class="label">Guest Files Expire</label>
<div class="control">
<div class="select">
<select id="expiration-select-file">
{{ range .FileLifetimeOptions }}
<option
value="{{ formatLifetime .FileLifetime }}"
{{ if .IsDefault }}selected{{ end }}
>
{{ .FileLifetime.FriendlyName }}
</option>
{{ end }}
</select>
</div>
</div>
</div>

<div class="field my-5">
<label class="label">Max file size <i>(optional)</i></label>
<div class="control">
Expand Down
8 changes: 6 additions & 2 deletions handlers/templates/pages/guest-link-index.html
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ <h1 class="title">Guest Links</h1>
<tr>
<th>Label</th>
<th>Created</th>
<th>Expiration</th>
<th>URL Expiration</th>
<th>File Expiration</th>
<th>Max Upload Size</th>
<th>Uploads</th>
<th class="has-text-right">Actions</th>
Expand All @@ -140,7 +141,10 @@ <h1 class="title">Guest Links</h1>
</td>
<td class="is-vcentered">{{ formatDate .Created }}</td>
<td class="is-vcentered expiration">
{{ formatExpiration .Expires }}
{{ formatExpiration .UrlExpires }}
</td>
<td class="is-vcentered expiration">
{{ .FileLifetime.FriendlyName }}
</td>
<td class="is-vcentered">
{{ formatSizeLimit .MaxFileBytes }}
Expand Down
4 changes: 3 additions & 1 deletion handlers/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ func (s Server) guestEntryPost() http.HandlerFunc {
r.Body = http.MaxBytesReader(w, r.Body, int64(*gl.MaxFileBytes))
}

id, err := s.insertFileFromRequest(r, picoshare.NeverExpire, guestLinkID)
t := picoshare.ExpirationTime(time.Now().Add(gl.FileLifetime.Duration()))
Balk-Z marked this conversation as resolved.
Show resolved Hide resolved

id, err := s.insertFileFromRequest(r, t, guestLinkID)
if err != nil {
var de *dbError
if errors.As(err, &de) {
Expand Down
Loading
Loading