From fa17dce04638dc05b2283afe6bd4db2a840f9e16 Mon Sep 17 00:00:00 2001 From: Aleksandr Baryshnikov Date: Mon, 29 Jan 2024 21:12:29 +0800 Subject: [PATCH] feat: pre-signed URL for S3 storage (#2855) Adds automatically background refresh of all external links if they are belongs to the current blob (S3) storage. The feature is disabled by default in order to keep backward compatibility. The background go-routine spawns once during startup and periodically signs and updates external links if that links belongs to current S3 storage. The original idea was to sign external links on-demand, however, with current architecture it will require duplicated code in plenty of places. If do it, the changes will be quite invasive and in the end pointless: I believe, the architecture will be eventually updated to give more scalable way for pluggable storage. For example - Upload/Download interface without hard dependency on external link. There are stubs already, but I don't feel confident enough to change significant part of the application architecture. --- api/v1/resource.go | 1 + api/v1/storage.go | 1 + api/v1/swagger.yaml | 4 + bin/memos/main.go | 5 + docs/api/v1.md | 23 +-- internal/jobs/presign_link.go | 140 ++++++++++++++++++ plugin/storage/s3/s3.go | 44 ++++++ store/db/mysql/resource.go | 3 + store/db/postgres/resource.go | 3 + store/db/sqlite/resource.go | 3 + store/resource.go | 1 + .../components/CreateStorageServiceDialog.tsx | 11 +- web/src/locales/en.json | 3 +- web/src/locales/ru.json | 1 + web/src/types/modules/storage.d.ts | 1 + 15 files changed, 231 insertions(+), 13 deletions(-) create mode 100644 internal/jobs/presign_link.go diff --git a/api/v1/resource.go b/api/v1/resource.go index c4702cae..b2434f8d 100644 --- a/api/v1/resource.go +++ b/api/v1/resource.go @@ -480,6 +480,7 @@ func SaveResourceBlob(ctx context.Context, s *store.Store, create *store.Resourc Bucket: s3Config.Bucket, URLPrefix: s3Config.URLPrefix, URLSuffix: s3Config.URLSuffix, + PreSign: s3Config.PreSign, }) if err != nil { return errors.Wrap(err, "Failed to create s3 client") diff --git a/api/v1/storage.go b/api/v1/storage.go index 4479a1d0..1fc13e04 100644 --- a/api/v1/storage.go +++ b/api/v1/storage.go @@ -43,6 +43,7 @@ type StorageS3Config struct { Bucket string `json:"bucket"` URLPrefix string `json:"urlPrefix"` URLSuffix string `json:"urlSuffix"` + PreSign bool `json:"presign"` } type Storage struct { diff --git a/api/v1/swagger.yaml b/api/v1/swagger.yaml index e3576e23..ad1cf0ec 100644 --- a/api/v1/swagger.yaml +++ b/api/v1/swagger.yaml @@ -238,6 +238,8 @@ definitions: type: string urlSuffix: type: string + presign: + type: boolean type: object api_v1.StorageType: enum: @@ -668,6 +670,8 @@ definitions: type: string urlSuffix: type: string + presign: + type: boolean type: object github_com_usememos_memos_api_v1.StorageType: enum: diff --git a/bin/memos/main.go b/bin/memos/main.go index 310ad11f..98821c85 100644 --- a/bin/memos/main.go +++ b/bin/memos/main.go @@ -12,6 +12,8 @@ import ( "github.com/spf13/viper" "go.uber.org/zap" + "github.com/usememos/memos/internal/jobs" + "github.com/usememos/memos/internal/log" "github.com/usememos/memos/server" _profile "github.com/usememos/memos/server/profile" @@ -91,6 +93,9 @@ var ( printGreetings() + // update (pre-sign) object storage links if applicable + go jobs.RunPreSignLinks(ctx, storeInstance) + if err := s.Start(ctx); err != nil { if err != http.ErrServerClosed { log.Error("failed to start server", zap.Error(err)) diff --git a/docs/api/v1.md b/docs/api/v1.md index 48b7704f..a8426ee7 100644 --- a/docs/api/v1.md +++ b/docs/api/v1.md @@ -1255,6 +1255,7 @@ Get GetImage from URL | secretKey | string | | No | | urlPrefix | string | | No | | urlSuffix | string | | No | +| presign | boolean | | No | #### api_v1.StorageType @@ -1540,16 +1541,18 @@ Get GetImage from URL #### github_com_usememos_memos_api_v1.StorageS3Config -| Name | Type | Description | Required | -| --------- | ------ | ----------- | -------- | -| accessKey | string | | No | -| bucket | string | | No | -| endPoint | string | | No | -| path | string | | No | -| region | string | | No | -| secretKey | string | | No | -| urlPrefix | string | | No | -| urlSuffix | string | | No | +| Name | Type | Description | Required | +|-----------|---------| ----------- | -------- | +| accessKey | string | | No | +| bucket | string | | No | +| endPoint | string | | No | +| path | string | | No | +| region | string | | No | +| secretKey | string | | No | +| urlPrefix | string | | No | +| urlSuffix | string | | No | +| presign | boolean | | No | + #### github_com_usememos_memos_api_v1.StorageType diff --git a/internal/jobs/presign_link.go b/internal/jobs/presign_link.go new file mode 100644 index 00000000..36c49bc8 --- /dev/null +++ b/internal/jobs/presign_link.go @@ -0,0 +1,140 @@ +package jobs + +import ( + "context" + "encoding/json" + "strings" + "time" + + "github.com/pkg/errors" + "go.uber.org/zap" + + apiv1 "github.com/usememos/memos/api/v1" + "github.com/usememos/memos/internal/log" + "github.com/usememos/memos/plugin/storage/s3" + "github.com/usememos/memos/store" +) + +// RunPreSignLinks is a background job that pre-signs external links stored in the database. +// It uses S3 client to generate presigned URLs and updates the corresponding resources in the store. +func RunPreSignLinks(ctx context.Context, dataStore *store.Store) { + for { + started := time.Now() + if err := signExternalLinks(ctx, dataStore); err != nil { + log.Warn("failed sign external links", zap.Error(err)) + } else { + log.Info("links pre-signed", zap.Duration("duration", time.Since(started))) + } + select { + case <-time.After(s3.LinkLifetime / 2): + case <-ctx.Done(): + return + } + } +} + +func signExternalLinks(ctx context.Context, dataStore *store.Store) error { + const pageSize = 32 + + objectStore, err := findObjectStorage(ctx, dataStore) + if err != nil { + return errors.Wrapf(err, "find object storage") + } + if objectStore == nil || !objectStore.Config.PreSign { + // object storage not set or not supported + return nil + } + + var offset int + var limit = pageSize + for { + resources, err := dataStore.ListResources(ctx, &store.FindResource{ + GetBlob: false, + Limit: &limit, + Offset: &offset, + }) + if err != nil { + return errors.Wrapf(err, "list resources, offset %d", offset) + } + + for _, res := range resources { + if res.ExternalLink == "" { + // not for object store + continue + } + if strings.Contains(res.ExternalLink, "?") && time.Since(time.Unix(res.UpdatedTs, 0)) < s3.LinkLifetime/2 { + // resource not signed (hack for migration) + // resource was recently updated - skipping + continue + } + newLink, err := objectStore.PreSignLink(ctx, res.ExternalLink) + if err != nil { + log.Warn("failed pre-sign link", zap.Int32("resource", res.ID), zap.String("link", res.ExternalLink), zap.Error(err)) + continue // do not fail - we may want update left over links too + } + now := time.Now().Unix() + // we may want to use here transaction and batch update in the future + _, err = dataStore.UpdateResource(ctx, &store.UpdateResource{ + ID: res.ID, + UpdatedTs: &now, + ExternalLink: &newLink, + }) + if err != nil { + // something with DB - better to stop here + return errors.Wrapf(err, "update resource %d link to %q", res.ID, newLink) + } + } + + offset += limit + if len(resources) < limit { + break + } + } + return nil +} + +// findObjectStorage returns current default storage if it's S3-compatible or nil otherwise. +// Returns error only in case of internal problems (ie: database or configuration issues). +// May return nil client and nil error. +func findObjectStorage(ctx context.Context, dataStore *store.Store) (*s3.Client, error) { + systemSettingStorageServiceID, err := dataStore.GetSystemSetting(ctx, &store.FindSystemSetting{Name: apiv1.SystemSettingStorageServiceIDName.String()}) + if err != nil { + return nil, errors.Wrap(err, "Failed to find SystemSettingStorageServiceIDName") + } + + storageServiceID := apiv1.DefaultStorage + if systemSettingStorageServiceID != nil { + err = json.Unmarshal([]byte(systemSettingStorageServiceID.Value), &storageServiceID) + if err != nil { + return nil, errors.Wrap(err, "Failed to unmarshal storage service id") + } + } + storage, err := dataStore.GetStorage(ctx, &store.FindStorage{ID: &storageServiceID}) + if err != nil { + return nil, errors.Wrap(err, "Failed to find StorageServiceID") + } + + if storage == nil { + return nil, nil // storage not configured - not an error, just return empty ref + } + storageMessage, err := apiv1.ConvertStorageFromStore(storage) + + if err != nil { + return nil, errors.Wrap(err, "Failed to ConvertStorageFromStore") + } + if storageMessage.Type != apiv1.StorageS3 { + return nil, nil + } + + s3Config := storageMessage.Config.S3Config + return s3.NewClient(ctx, &s3.Config{ + AccessKey: s3Config.AccessKey, + SecretKey: s3Config.SecretKey, + EndPoint: s3Config.EndPoint, + Region: s3Config.Region, + Bucket: s3Config.Bucket, + URLPrefix: s3Config.URLPrefix, + URLSuffix: s3Config.URLSuffix, + PreSign: s3Config.PreSign, + }) +} diff --git a/plugin/storage/s3/s3.go b/plugin/storage/s3/s3.go index 8618fd62..842032c4 100644 --- a/plugin/storage/s3/s3.go +++ b/plugin/storage/s3/s3.go @@ -7,6 +7,7 @@ import ( "io" "net/url" "strings" + "time" "github.com/aws/aws-sdk-go-v2/aws" s3config "github.com/aws/aws-sdk-go-v2/config" @@ -14,8 +15,11 @@ import ( "github.com/aws/aws-sdk-go-v2/feature/s3/manager" awss3 "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" + errors2 "github.com/pkg/errors" ) +const LinkLifetime = 24 * time.Hour + type Config struct { AccessKey string SecretKey string @@ -24,6 +28,7 @@ type Config struct { Region string URLPrefix string URLSuffix string + PreSign bool } type Client struct { @@ -93,5 +98,44 @@ func (client *Client) UploadFile(ctx context.Context, filename string, fileType if link == "" { return "", errors.New("failed to get file link") } + if client.Config.PreSign { + return client.PreSignLink(ctx, link) + } return link, nil } + +// PreSignLink generates a pre-signed URL for the given sourceLink. +// If the link does not belong to the configured storage endpoint, it is returned as-is. +// If the link belongs to the storage, the function generates a pre-signed URL using the AWS S3 client. +func (client *Client) PreSignLink(ctx context.Context, sourceLink string) (string, error) { + u, err := url.Parse(sourceLink) + if err != nil { + return "", errors2.Wrapf(err, "parse URL") + } + // if link doesn't belong to storage, then return as-is. + // the empty hostname is corner-case for AWS native endpoint. + if client.Config.EndPoint != "" && !strings.Contains(client.Config.EndPoint, u.Hostname()) { + return sourceLink, nil + } + + filename := u.Path + if prefixLen := len(client.Config.URLPrefix); len(filename) >= prefixLen { + filename = filename[prefixLen:] + } + if suffixLen := len(client.Config.URLSuffix); len(filename) >= suffixLen { + filename = filename[:len(filename)-suffixLen] + } + filename = strings.Trim(filename, "/") + if strings.HasPrefix(filename, client.Config.Bucket) { + filename = strings.Trim(filename[len(client.Config.Bucket):], "/") + } + + req, err := awss3.NewPresignClient(client.Client).PresignGetObject(ctx, &awss3.GetObjectInput{ + Bucket: aws.String(client.Config.Bucket), + Key: aws.String(filename), + }, awss3.WithPresignExpires(LinkLifetime)) + if err != nil { + return "", errors2.Wrapf(err, "pre-sign link") + } + return req.URL, nil +} diff --git a/store/db/mysql/resource.go b/store/db/mysql/resource.go index 76321e48..e2c570ea 100644 --- a/store/db/mysql/resource.go +++ b/store/db/mysql/resource.go @@ -133,6 +133,9 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) ( if v := update.InternalPath; v != nil { set, args = append(set, "`internal_path` = ?"), append(args, *v) } + if v := update.ExternalLink; v != nil { + set, args = append(set, "`external_link` = ?"), append(args, *v) + } if v := update.MemoID; v != nil { set, args = append(set, "`memo_id` = ?"), append(args, *v) } diff --git a/store/db/postgres/resource.go b/store/db/postgres/resource.go index 9409b14d..3ced7461 100644 --- a/store/db/postgres/resource.go +++ b/store/db/postgres/resource.go @@ -118,6 +118,9 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) ( if v := update.InternalPath; v != nil { set, args = append(set, "internal_path = "+placeholder(len(args)+1)), append(args, *v) } + if v := update.ExternalLink; v != nil { + set, args = append(set, "external_link = "+placeholder(len(args)+1)), append(args, *v) + } if v := update.MemoID; v != nil { set, args = append(set, "memo_id = "+placeholder(len(args)+1)), append(args, *v) } diff --git a/store/db/sqlite/resource.go b/store/db/sqlite/resource.go index 13609898..bb0d6ee7 100644 --- a/store/db/sqlite/resource.go +++ b/store/db/sqlite/resource.go @@ -114,6 +114,9 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) ( if v := update.InternalPath; v != nil { set, args = append(set, "`internal_path` = ?"), append(args, *v) } + if v := update.ExternalLink; v != nil { + set, args = append(set, "`external_link` = ?"), append(args, *v) + } if v := update.MemoID; v != nil { set, args = append(set, "`memo_id` = ?"), append(args, *v) } diff --git a/store/resource.go b/store/resource.go index 37170084..fae1cd1b 100644 --- a/store/resource.go +++ b/store/resource.go @@ -53,6 +53,7 @@ type UpdateResource struct { UpdatedTs *int64 Filename *string InternalPath *string + ExternalLink *string MemoID *int32 Blob []byte } diff --git a/web/src/components/CreateStorageServiceDialog.tsx b/web/src/components/CreateStorageServiceDialog.tsx index c3fe6829..d9be9dac 100644 --- a/web/src/components/CreateStorageServiceDialog.tsx +++ b/web/src/components/CreateStorageServiceDialog.tsx @@ -1,5 +1,5 @@ -import { Button, IconButton, Input, Typography } from "@mui/joy"; -import { useEffect, useState } from "react"; +import { Button, IconButton, Input, Checkbox, Typography } from "@mui/joy"; +import React, { useEffect, useState } from "react"; import { toast } from "react-hot-toast"; import * as api from "@/helpers/api"; import { useTranslate } from "@/utils/i18n"; @@ -29,6 +29,7 @@ const CreateStorageServiceDialog: React.FC = (props: Props) => { bucket: "", urlPrefix: "", urlSuffix: "", + presign: false, }); const isCreating = storage === undefined; @@ -220,6 +221,12 @@ const CreateStorageServiceDialog: React.FC = (props: Props) => { onChange={(e) => setPartialS3Config({ urlSuffix: e.target.value })} fullWidth /> + setPartialS3Config({ presign: e.target.checked })} + />