From d8f415f8abd0c4301803bd968c54429dd3fe4b59 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Tue, 2 Mar 2021 06:00:18 -0500 Subject: [PATCH] feat: allow to password protect shares (#1252) This changes allows to password protect shares. It works by: * Allowing to optionally pass a password when creating a share * If set, the password + salt that is configured via a new flag will be hashed via bcrypt and the hash stored together with the rest of the share * Additionally, a random 96 byte long token gets generated and stored as part of the share * When the backend retrieves an unauthenticated request for a share that has authentication configured, it will return a http 401 * The frontend detects this and will show a login prompt * The actual download links are protected via an url arg that contains the previously generated token. This allows us to avoid buffering the download in the browser and allows pasting the link without breaking it --- .circleci/config.yml | 4 +- auth/auth.go | 2 +- auth/json.go | 2 +- auth/none.go | 2 +- auth/proxy.go | 2 +- files/file.go | 3 + frontend/src/api/files.js | 7 +- frontend/src/api/share.js | 16 ++- frontend/src/components/prompts/Share.vue | 21 ++-- frontend/src/css/_share.css | 15 ++- frontend/src/i18n/en.json | 4 +- frontend/src/store/index.js | 3 +- frontend/src/store/mutations.js | 1 + frontend/src/views/Share.vue | 35 +++++- http/data.go | 2 +- http/public.go | 33 ++++++ http/public_test.go | 136 ++++++++++++++++++++++ http/share.go | 59 ++++++++-- share/share.go | 19 ++- storage/storage.go | 2 +- users/storage.go | 9 ++ users/storage_test.go | 4 + 22 files changed, 340 insertions(+), 41 deletions(-) create mode 100644 http/public_test.go create mode 100644 users/storage_test.go diff --git a/.circleci/config.yml b/.circleci/config.yml index 643276b0..09d44117 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -2,7 +2,7 @@ version: 2 jobs: lint: docker: - - image: golangci/golangci-lint:v1.27.0 + - image: golangci/golangci-lint:v1.31.0 steps: - checkout - run: golangci-lint run -v @@ -89,4 +89,4 @@ workflows: tags: only: /^v.*/ branches: - ignore: /.*/ \ No newline at end of file + ignore: /.*/ diff --git a/auth/auth.go b/auth/auth.go index 72dc8f41..c15cb9ab 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -9,7 +9,7 @@ import ( // Auther is the authentication interface. type Auther interface { // Auth is called to authenticate a request. - Auth(r *http.Request, s *users.Storage, root string) (*users.User, error) + Auth(r *http.Request, s users.Store, root string) (*users.User, error) // LoginPage indicates if this auther needs a login page. LoginPage() bool } diff --git a/auth/json.go b/auth/json.go index 641b73cd..925ea4e0 100644 --- a/auth/json.go +++ b/auth/json.go @@ -26,7 +26,7 @@ type JSONAuth struct { } // Auth authenticates the user via a json in content body. -func (a JSONAuth) Auth(r *http.Request, sto *users.Storage, root string) (*users.User, error) { +func (a JSONAuth) Auth(r *http.Request, sto users.Store, root string) (*users.User, error) { var cred jsonCred if r.Body == nil { diff --git a/auth/none.go b/auth/none.go index 7e47baf7..43b60f06 100644 --- a/auth/none.go +++ b/auth/none.go @@ -14,7 +14,7 @@ const MethodNoAuth settings.AuthMethod = "noauth" type NoAuth struct{} // Auth uses authenticates user 1. -func (a NoAuth) Auth(r *http.Request, sto *users.Storage, root string) (*users.User, error) { +func (a NoAuth) Auth(r *http.Request, sto users.Store, root string) (*users.User, error) { return sto.Get(root, uint(1)) } diff --git a/auth/proxy.go b/auth/proxy.go index 21d072c7..f9387509 100644 --- a/auth/proxy.go +++ b/auth/proxy.go @@ -18,7 +18,7 @@ type ProxyAuth struct { } // Auth authenticates the user via an HTTP header. -func (a ProxyAuth) Auth(r *http.Request, sto *users.Storage, root string) (*users.User, error) { +func (a ProxyAuth) Auth(r *http.Request, sto users.Store, root string) (*users.User, error) { username := r.Header.Get(a.Header) user, err := sto.Get(root, username) if err == errors.ErrNotExist { diff --git a/files/file.go b/files/file.go index fa102049..fed35dce 100644 --- a/files/file.go +++ b/files/file.go @@ -38,6 +38,7 @@ type FileInfo struct { Subtitles []string `json:"subtitles,omitempty"` Content string `json:"content,omitempty"` Checksums map[string]string `json:"checksums,omitempty"` + Token string `json:"token,omitempty"` } // FileOptions are the options when getting a file info. @@ -47,6 +48,7 @@ type FileOptions struct { Modify bool Expand bool ReadHeader bool + Token string Checker rules.Checker } @@ -72,6 +74,7 @@ func NewFileInfo(opts FileOptions) (*FileInfo, error) { IsDir: info.IsDir(), Size: info.Size(), Extension: filepath.Ext(info.Name()), + Token: opts.Token, } if opts.Expand { diff --git a/frontend/src/api/files.js b/frontend/src/api/files.js index d12a4237..bada7c7c 100644 --- a/frontend/src/api/files.js +++ b/frontend/src/api/files.js @@ -77,8 +77,13 @@ export function download (format, ...files) { if (format !== null) { url += `algo=${format}&` } + if (store.state.jwt !== ''){ + url += `auth=${store.state.jwt}&` + } + if (store.state.token !== ''){ + url += `token=${store.state.token}` + } - url += `auth=${store.state.jwt}` window.open(url) } diff --git a/frontend/src/api/share.js b/frontend/src/api/share.js index 6f8a2a17..4e6c40db 100644 --- a/frontend/src/api/share.js +++ b/frontend/src/api/share.js @@ -4,8 +4,10 @@ export async function list() { return fetchJSON('/api/shares') } -export async function getHash(hash) { - return fetchJSON(`/api/public/share/${hash}`) +export async function getHash(hash, password = "") { + return fetchJSON(`/api/public/share/${hash}`, { + headers: {'X-SHARE-PASSWORD': password}, + }) } export async function get(url) { @@ -23,14 +25,18 @@ export async function remove(hash) { } } -export async function create(url, expires = '', unit = 'hours') { +export async function create(url, password = '', expires = '', unit = 'hours') { url = removePrefix(url) url = `/api/share${url}` if (expires !== '') { url += `?expires=${expires}&unit=${unit}` } - + let body = '{}'; + if (password != '' || expires !== '' || unit !== 'hours') { + body = JSON.stringify({password: password, expires: expires, unit: unit}) + } return fetchJSON(url, { - method: 'POST' + method: 'POST', + body: body, }) } diff --git a/frontend/src/components/prompts/Share.vue b/frontend/src/components/prompts/Share.vue index 456b51e3..043847d8 100644 --- a/frontend/src/components/prompts/Share.vue +++ b/frontend/src/components/prompts/Share.vue @@ -1,14 +1,11 @@ @@ -102,7 +120,9 @@ export default { data: () => ({ error: null, path: '', - showLimit: 500 + showLimit: 500, + password: '', + attemptedPasswordLogin: false }), watch: { '$route': 'fetchData' @@ -129,7 +149,11 @@ export default { return 'insert_drive_file' }, link: function () { - return `${baseURL}/api/public/dl/${this.hash}${this.path}` + let queryArg = ''; + if (this.token !== ''){ + queryArg = `?token=${this.token}` + } + return `${baseURL}/api/public/dl/${this.hash}${this.path}${queryArg}` }, fullLink: function () { return window.location.origin + this.link @@ -193,8 +217,13 @@ export default { this.error = null try { - let file = await api.getHash(encodeURIComponent(this.$route.params.pathMatch)) + if (this.password !== ''){ + this.attemptedPasswordLogin = true + } + let file = await api.getHash(encodeURIComponent(this.$route.params.pathMatch), this.password) this.path = file.path + this.token = file.token || '' + this.$store.commit('setToken', this.token) if (file.isDir) file.items = file.items.map((item, index) => { item.index = index item.url = `/share/${this.hash}${this.path}/${encodeURIComponent(item.name)}` diff --git a/http/data.go b/http/data.go index 1bdafe3f..bc9bdd56 100644 --- a/http/data.go +++ b/http/data.go @@ -51,7 +51,7 @@ func handle(fn handleFunc, prefix string, store *storage.Storage, server *settin handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { settings, err := store.Settings.Get() if err != nil { - log.Fatalln("ERROR: couldn't get settings") + log.Fatalf("ERROR: couldn't get settings: %v\n", err) return } diff --git a/http/public.go b/http/public.go index d34b97e3..c24be0d9 100644 --- a/http/public.go +++ b/http/public.go @@ -1,14 +1,17 @@ package http import ( + "errors" "net/http" "path" "path/filepath" "strings" "github.com/spf13/afero" + "golang.org/x/crypto/bcrypt" "github.com/filebrowser/filebrowser/v2/files" + "github.com/filebrowser/filebrowser/v2/share" ) var withHashFile = func(fn handleFunc) handleFunc { @@ -19,6 +22,11 @@ var withHashFile = func(fn handleFunc) handleFunc { return errToStatus(err), err } + status, err := authenticateShareRequest(r, link) + if status != 0 || err != nil { + return status, err + } + user, err := d.store.Users.Get(d.server.Root, link.UserID) if err != nil { return errToStatus(err), err @@ -33,6 +41,7 @@ var withHashFile = func(fn handleFunc) handleFunc { Expand: true, ReadHeader: d.server.TypeDetectionByHeader, Checker: d, + Token: link.Token, }) if err != nil { return errToStatus(err), err @@ -48,6 +57,7 @@ var withHashFile = func(fn handleFunc) handleFunc { Modify: d.user.Perm.Modify, Expand: true, Checker: d, + Token: link.Token, }) if err != nil { return errToStatus(err), err @@ -94,3 +104,26 @@ var publicDlHandler = withHashFile(func(w http.ResponseWriter, r *http.Request, return rawDirHandler(w, r, d, file) }) + +func authenticateShareRequest(r *http.Request, l *share.Link) (int, error) { + if l.PasswordHash == "" { + return 0, nil + } + + if r.URL.Query().Get("token") == l.Token { + return 0, nil + } + + password := r.Header.Get("X-SHARE-PASSWORD") + if password == "" { + return http.StatusUnauthorized, nil + } + if err := bcrypt.CompareHashAndPassword([]byte(l.PasswordHash), []byte(password)); err != nil { + if errors.Is(err, bcrypt.ErrMismatchedHashAndPassword) { + return http.StatusUnauthorized, nil + } + return 0, err + } + + return 0, nil +} diff --git a/http/public_test.go b/http/public_test.go new file mode 100644 index 00000000..19772cfb --- /dev/null +++ b/http/public_test.go @@ -0,0 +1,136 @@ +package http + +import ( + "fmt" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/asdine/storm" + "github.com/spf13/afero" + + "github.com/filebrowser/filebrowser/v2/settings" + "github.com/filebrowser/filebrowser/v2/share" + "github.com/filebrowser/filebrowser/v2/storage/bolt" + "github.com/filebrowser/filebrowser/v2/users" +) + +func TestPublicShareHandlerAuthentication(t *testing.T) { + t.Parallel() + + const passwordBcrypt = "$2y$10$TFAmdCbyd/mEZDe5fUeZJu.MaJQXRTwdqb/IQV.eTn6dWrF58gCSe" //nolint:gosec + testCases := map[string]struct { + share *share.Link + req *http.Request + expectedStatusCode int + }{ + "Public share, no auth required": { + share: &share.Link{Hash: "h", UserID: 1}, + req: newHTTPRequest(t), + expectedStatusCode: 200, + }, + "Private share, no auth provided, 401": { + share: &share.Link{Hash: "h", UserID: 1, PasswordHash: passwordBcrypt, Token: "123"}, + req: newHTTPRequest(t), + expectedStatusCode: 401, + }, + "Private share, authentication via token": { + share: &share.Link{Hash: "h", UserID: 1, PasswordHash: passwordBcrypt, Token: "123"}, + req: newHTTPRequest(t, func(r *http.Request) { r.URL.RawQuery = "token=123" }), + expectedStatusCode: 200, + }, + "Private share, authentication via invalid token, 401": { + share: &share.Link{Hash: "h", UserID: 1, PasswordHash: passwordBcrypt, Token: "123"}, + req: newHTTPRequest(t, func(r *http.Request) { r.URL.RawQuery = "token=1234" }), + expectedStatusCode: 401, + }, + "Private share, authentication via password": { + share: &share.Link{Hash: "h", UserID: 1, PasswordHash: passwordBcrypt, Token: "123"}, + req: newHTTPRequest(t, func(r *http.Request) { r.Header.Set("X-SHARE-PASSWORD", "password") }), + expectedStatusCode: 200, + }, + "Private share, authentication via invalid password, 401": { + share: &share.Link{Hash: "h", UserID: 1, PasswordHash: passwordBcrypt, Token: "123"}, + req: newHTTPRequest(t, func(r *http.Request) { r.Header.Set("X-SHARE-PASSWORD", "wrong-password") }), + expectedStatusCode: 401, + }, + } + + for name, tc := range testCases { + for handlerName, handler := range map[string]handleFunc{"public share handler": publicShareHandler, "public dl handler": publicDlHandler} { + name, tc, handlerName, handler := name, tc, handlerName, handler + t.Run(fmt.Sprintf("%s: %s", handlerName, name), func(t *testing.T) { + t.Parallel() + + dbPath := filepath.Join(t.TempDir(), "db") + db, err := storm.Open(dbPath) + if err != nil { + t.Fatalf("failed to open db: %v", err) + } + + t.Cleanup(func() { + if err := db.Close(); err != nil { //nolint:shadow + t.Errorf("failed to close db: %v", err) + } + }) + + storage, err := bolt.NewStorage(db) + if err != nil { + t.Fatalf("failed to get storage: %v", err) + } + if err := storage.Share.Save(tc.share); err != nil { + t.Fatalf("failed to save share: %v", err) + } + if err := storage.Users.Save(&users.User{Username: "username", Password: "pw"}); err != nil { + t.Fatalf("failed to save user: %v", err) + } + if err := storage.Settings.Save(&settings.Settings{Key: []byte("key")}); err != nil { + t.Fatalf("failed to save settings: %v", err) + } + + storage.Users = &customFSUser{ + Store: storage.Users, + fs: &afero.MemMapFs{}, + } + + recorder := httptest.NewRecorder() + handler := handle(handler, "", storage, &settings.Server{}) + + handler.ServeHTTP(recorder, tc.req) + result := recorder.Result() + defer result.Body.Close() + if result.StatusCode != tc.expectedStatusCode { + t.Errorf("expected status code %d, got status code %d", tc.expectedStatusCode, result.StatusCode) + } + }) + } + } +} + +func newHTTPRequest(t *testing.T, requestModifiers ...func(*http.Request)) *http.Request { + t.Helper() + r, err := http.NewRequest(http.MethodGet, "h", nil) + if err != nil { + t.Fatalf("failed to construct request: %v", err) + } + for _, modify := range requestModifiers { + modify(r) + } + return r +} + +type customFSUser struct { + users.Store + fs afero.Fs +} + +func (cu *customFSUser) Get(baseScope string, id interface{}) (*users.User, error) { + user, err := cu.Store.Get(baseScope, id) + if err != nil { + return nil, err + } + user.Fs = cu.fs + + return user, nil +} diff --git a/http/share.go b/http/share.go index 853a178e..9ee28587 100644 --- a/http/share.go +++ b/http/share.go @@ -3,6 +3,8 @@ package http import ( "crypto/rand" "encoding/base64" + "encoding/json" + "fmt" "net/http" "path" "sort" @@ -10,6 +12,8 @@ import ( "strings" "time" + "golang.org/x/crypto/bcrypt" + "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/share" ) @@ -79,10 +83,15 @@ var shareDeleteHandler = withPermShare(func(w http.ResponseWriter, r *http.Reque var sharePostHandler = withPermShare(func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { var s *share.Link - rawExpire := r.URL.Query().Get("expires") - unit := r.URL.Query().Get("unit") + var body share.CreateBody + if r.Body != nil { + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + return http.StatusBadRequest, fmt.Errorf("failed to decode body: %w", err) + } + defer r.Body.Close() + } - if rawExpire == "" { + if body.Expires == "" { var err error s, err = d.store.Share.GetPermanent(r.URL.Path, d.user.ID) if err == nil { @@ -103,14 +112,15 @@ var sharePostHandler = withPermShare(func(w http.ResponseWriter, r *http.Request var expire int64 = 0 - if rawExpire != "" { - num, err := strconv.Atoi(rawExpire) + if body.Expires != "" { + //nolint:govet + num, err := strconv.Atoi(body.Expires) if err != nil { return http.StatusInternalServerError, err } var add time.Duration - switch unit { + switch body.Unit { case "seconds": add = time.Second * time.Duration(num) case "minutes": @@ -124,11 +134,27 @@ var sharePostHandler = withPermShare(func(w http.ResponseWriter, r *http.Request expire = time.Now().Add(add).Unix() } + hash, status, err := getSharePasswordHash(body) + if err != nil { + return status, err + } + + var token string + if len(hash) > 0 { + tokenBuffer := make([]byte, 96) + if _, err := rand.Read(tokenBuffer); err != nil { + return http.StatusInternalServerError, err + } + token = base64.URLEncoding.EncodeToString(tokenBuffer) + } + s = &share.Link{ - Path: r.URL.Path, - Hash: str, - Expire: expire, - UserID: d.user.ID, + Path: r.URL.Path, + Hash: str, + Expire: expire, + UserID: d.user.ID, + PasswordHash: string(hash), + Token: token, } if err := d.store.Share.Save(s); err != nil { @@ -137,3 +163,16 @@ var sharePostHandler = withPermShare(func(w http.ResponseWriter, r *http.Request return renderJSON(w, r, s) }) + +func getSharePasswordHash(body share.CreateBody) (data []byte, statuscode int, err error) { + if body.Password == "" { + return nil, 0, nil + } + + hash, err := bcrypt.GenerateFromPassword([]byte(body.Password), bcrypt.DefaultCost) + if err != nil { + return nil, http.StatusInternalServerError, fmt.Errorf("failed to hash password: %w", err) + } + + return hash, 0, nil +} diff --git a/share/share.go b/share/share.go index fb4329f1..f14c543d 100644 --- a/share/share.go +++ b/share/share.go @@ -1,9 +1,20 @@ package share +type CreateBody struct { + Password string `json:"password"` + Expires string `json:"expires"` + Unit string `json:"unit"` +} + // Link is the information needed to build a shareable link. type Link struct { - Hash string `json:"hash" storm:"id,index"` - Path string `json:"path" storm:"index"` - UserID uint `json:"userID"` - Expire int64 `json:"expire"` + Hash string `json:"hash" storm:"id,index"` + Path string `json:"path" storm:"index"` + UserID uint `json:"userID"` + Expire int64 `json:"expire"` + PasswordHash string `json:"password_hash,omitempty"` + // Token is a random value that will only be set when PasswordHash is set. It is + // URL-Safe and is used to download links in password-protected shares via a + // query arg. + Token string `json:"token,omitempty"` } diff --git a/storage/storage.go b/storage/storage.go index 79f7bcdd..d4f1a652 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -10,7 +10,7 @@ import ( // Storage is a storage powered by a Backend which makes the necessary // verifications when fetching and saving data to ensure consistency. type Storage struct { - Users *users.Storage + Users users.Store Share *share.Storage Auth *auth.Storage Settings *settings.Storage diff --git a/users/storage.go b/users/storage.go index 76a8a200..d18c191e 100644 --- a/users/storage.go +++ b/users/storage.go @@ -17,6 +17,15 @@ type StorageBackend interface { DeleteByUsername(string) error } +type Store interface { + Get(baseScope string, id interface{}) (user *User, err error) + Gets(baseScope string) ([]*User, error) + Update(user *User, fields ...string) error + Save(user *User) error + Delete(id interface{}) error + LastUpdate(id uint) int64 +} + // Storage is a users storage. type Storage struct { back StorageBackend diff --git a/users/storage_test.go b/users/storage_test.go new file mode 100644 index 00000000..65d13de8 --- /dev/null +++ b/users/storage_test.go @@ -0,0 +1,4 @@ +package users + +// Interface is implemented by storage +var _ Store = &Storage{}