From 888d053b2da64c694df9b3eb5dc34d0a0020aad5 Mon Sep 17 00:00:00 2001 From: Felipe Martin <812088+fmartingr@users.noreply.github.com> Date: Fri, 21 Jul 2023 07:57:42 +0200 Subject: [PATCH] Allow JWT authentication into legacy APIs (#651) * typo: letter in login page * httpconfig set defaults for secret key with warn * allow new authentication in old api * Updated warn log --- internal/cmd/server.go | 12 +- internal/config/config.go | 15 ++- internal/http/routes/legacy.go | 2 +- internal/view/login.html | 2 +- internal/webserver/handler-api.go | 95 ------------- internal/webserver/handler-ui.go | 70 ---------- internal/webserver/handler.go | 38 ++++-- internal/webserver/server.go | 214 +----------------------------- internal/webserver/utils.go | 67 +--------- 9 files changed, 48 insertions(+), 467 deletions(-) diff --git a/internal/cmd/server.go b/internal/cmd/server.go index 7637ee5ea..d32eccb98 100644 --- a/internal/cmd/server.go +++ b/internal/cmd/server.go @@ -19,7 +19,7 @@ func newServerCommand() *cobra.Command { cmd.Flags().IntP("port", "p", 8080, "Port used by the server") cmd.Flags().StringP("address", "a", "", "Address the server listens to") cmd.Flags().StringP("webroot", "r", "/", "Root path that used by server") - cmd.Flags().Bool("access-log", true, "Print out a non-standard access log") + cmd.Flags().Bool("access-log", false, "Print out a non-standard access log") cmd.Flags().Bool("serve-web-ui", true, "Serve static files from the webroot path") cmd.Flags().String("secret-key", "", "Secret key used for encrypting session data") @@ -40,15 +40,7 @@ func newServerCommandHandler() func(cmd *cobra.Command, args []string) { cfg, dependencies := initShiori(ctx, cmd) - // Check HTTP configuration - // For now it will just log to the console, but in the future it will be fatal. The only required - // setting for now is the secret key. - if errs, isValid := cfg.Http.IsValid(); !isValid { - dependencies.Log.Error("Found some errors in configuration.For now server will start but this will be fatal in the future.") - for _, err := range errs { - dependencies.Log.WithError(err).Error("found invalid configuration") - } - } + cfg.Http.SetDefaults(dependencies.Log) // Validate root path if rootPath == "" { diff --git a/internal/config/config.go b/internal/config/config.go index 336ba5b76..4e8abef13 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/gofrs/uuid" "github.com/sethvargo/go-envconfig" "github.com/sirupsen/logrus" ) @@ -78,13 +79,17 @@ type Config struct { Http *HttpConfig } -// IsValid checks if the configuration is valid -func (c HttpConfig) IsValid() (errs []error, isValid bool) { +// SetDefaults sets the default values for the configuration +func (c *HttpConfig) SetDefaults(logger *logrus.Logger) { + // Set a random secret key if not set if c.SecretKey == "" { - errs = append(errs, fmt.Errorf("SHIORI_HTTP_SECRET_KEY is required")) + logger.Warn("SHIORI_HTTP_SECRET_KEY is not set, using random value. This means that all sessions will be invalidated on server restart.") + randomUUID, err := uuid.NewV4() + if err != nil { + logger.WithError(err).Fatal("couldn't generate a random UUID") + } + c.SecretKey = randomUUID.String() } - - return errs, len(errs) == 0 } // SetDefaults sets the default values for the configuration diff --git a/internal/http/routes/legacy.go b/internal/http/routes/legacy.go index 7b7bea271..d9d3079bb 100644 --- a/internal/http/routes/legacy.go +++ b/internal/http/routes/legacy.go @@ -67,7 +67,7 @@ func (r *LegacyAPIRoutes) Setup(g *gin.Engine) { DataDir: r.cfg.Storage.DataDir, RootPath: r.cfg.Http.RootPath, Log: false, // Already done by gin - }) + }, r.deps) r.legacyHandler.PrepareSessionCache() r.legacyHandler.PrepareTemplates() diff --git a/internal/view/login.html b/internal/view/login.html index 21e59d9b9..907b1fdf0 100644 --- a/internal/view/login.html +++ b/internal/view/login.html @@ -15,7 +15,7 @@ - w + diff --git a/internal/webserver/handler-api.go b/internal/webserver/handler-api.go index ac872b07d..92da0f4a9 100644 --- a/internal/webserver/handler-api.go +++ b/internal/webserver/handler-api.go @@ -13,12 +13,10 @@ import ( "strconv" "strings" "sync" - "time" "github.com/go-shiori/shiori/internal/core" "github.com/go-shiori/shiori/internal/database" "github.com/go-shiori/shiori/internal/model" - "github.com/gofrs/uuid" "github.com/julienschmidt/httprouter" "golang.org/x/crypto/bcrypt" ) @@ -48,99 +46,6 @@ func downloadBookmarkContent(book *model.Bookmark, dataDir string, request *http return &result, err } -// apiLogin is handler for POST /api/login -func (h *Handler) apiLogin(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - ctx := r.Context() - - // Decode request - request := struct { - Username string `json:"username"` - Password string `json:"password"` - Remember bool `json:"remember"` - Owner bool `json:"owner"` - }{} - - err := json.NewDecoder(r.Body).Decode(&request) - checkError(err) - - // Prepare function to generate session - genSession := func(account model.Account, expTime time.Duration) { - // Create session ID - sessionID, err := uuid.NewV4() - checkError(err) - - // Save session ID to cache - strSessionID := sessionID.String() - h.SessionCache.Set(strSessionID, account, expTime) - - // Save user's session IDs to cache as well - // useful for mass logout - sessionIDs := []string{strSessionID} - if val, found := h.UserCache.Get(request.Username); found { - sessionIDs = val.([]string) - sessionIDs = append(sessionIDs, strSessionID) - } - h.UserCache.Set(request.Username, sessionIDs, -1) - - // Send login result - account.Password = "" - loginResult := struct { - Session string `json:"session"` - Account model.Account `json:"account"` - Expires string `json:"expires"` - }{strSessionID, account, time.Now().UTC().Add(expTime).Format(time.RFC1123)} - - w.Header().Set("Content-Type", "application/json") - err = json.NewEncoder(w).Encode(&loginResult) - checkError(err) - } - - // Check if user's database is empty or there are no owner. - // If yes, and user uses default account, let him in. - searchOptions := database.GetAccountsOptions{ - Owner: true, - } - - accounts, err := h.DB.GetAccounts(ctx, searchOptions) - checkError(err) - - if len(accounts) == 0 && request.Username == "shiori" && request.Password == "gopher" { - genSession(model.Account{ - Username: "shiori", - Owner: true, - }, time.Hour) - return - } - - // Get account data from database - account, exist, err := h.DB.GetAccount(ctx, request.Username) - checkError(err) - - if !exist { - panic(fmt.Errorf("username doesn't exist")) - } - - // Compare password with database - err = bcrypt.CompareHashAndPassword([]byte(account.Password), []byte(request.Password)) - if err != nil { - panic(fmt.Errorf("username and password don't match")) - } - - // If login request is as owner, make sure this account is owner - if request.Owner && !account.Owner { - panic(fmt.Errorf("account level is not sufficient as owner")) - } - - // Calculate expiration time - expTime := time.Hour - if request.Remember { - expTime = time.Hour * 24 * 30 - } - - // Create session - genSession(account, expTime) -} - // ApiLogout is handler for POST /api/logout func (h *Handler) ApiLogout(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { // Get session ID diff --git a/internal/webserver/handler-ui.go b/internal/webserver/handler-ui.go index ca9543423..2f300aea4 100644 --- a/internal/webserver/handler-ui.go +++ b/internal/webserver/handler-ui.go @@ -20,76 +20,6 @@ import ( "github.com/go-shiori/shiori/internal/model" ) -// serveFile is handler for general file request -func (h *Handler) serveFile(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - rootPath := strings.Trim(h.RootPath, "/") - urlPath := strings.Trim(r.URL.Path, "/") - filePath := strings.TrimPrefix(urlPath, rootPath) - filePath = strings.Trim(filePath, "/") - - err := serveFile(w, filePath, true) - checkError(err) -} - -// serveJsFile is handler for GET /js/*filepath -func (h *Handler) serveJsFile(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - jsFilePath := ps.ByName("filepath") - jsFilePath = path.Join("js", jsFilePath) - jsDir, jsName := path.Split(jsFilePath) - - if developmentMode && fp.Ext(jsName) == ".js" && strings.HasSuffix(jsName, ".min.js") { - jsName = strings.TrimSuffix(jsName, ".min.js") + ".js" - tmpPath := path.Join(jsDir, jsName) - if assetExists(tmpPath) { - jsFilePath = tmpPath - } - } - - err := serveFile(w, jsFilePath, true) - checkError(err) -} - -// serveIndexPage is handler for GET / -func (h *Handler) serveIndexPage(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - // Make sure session still valid - err := h.validateSession(r) - if err != nil { - newPath := path.Join(h.RootPath, "/login") - redirectURL := createRedirectURL(newPath, r.URL.String()) - redirectPage(w, r, redirectURL) - return - } - - if developmentMode { - if err := h.PrepareTemplates(); err != nil { - log.Printf("error during template preparation: %s", err) - } - } - - err = h.templates["index"].Execute(w, h.RootPath) - checkError(err) -} - -// serveLoginPage is handler for GET /login -func (h *Handler) serveLoginPage(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - // Make sure session is not valid - err := h.validateSession(r) - if err == nil { - redirectURL := path.Join(h.RootPath, "/") - redirectPage(w, r, redirectURL) - return - } - - if developmentMode { - if err := h.PrepareTemplates(); err != nil { - log.Printf("error during template preparation: %s", err) - } - } - - err = h.templates["login"].Execute(w, h.RootPath) - checkError(err) -} - // ServeBookmarkContent is handler for GET /bookmark/:id/content func (h *Handler) ServeBookmarkContent(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { ctx := r.Context() diff --git a/internal/webserver/handler.go b/internal/webserver/handler.go index 4b996d581..442466ee7 100644 --- a/internal/webserver/handler.go +++ b/internal/webserver/handler.go @@ -4,11 +4,13 @@ import ( "fmt" "html/template" "net/http" + "strings" + "github.com/go-shiori/shiori/internal/config" "github.com/go-shiori/shiori/internal/database" "github.com/go-shiori/shiori/internal/model" - "github.com/go-shiori/warc" cch "github.com/patrickmn/go-cache" + "github.com/sirupsen/logrus" ) var developmentMode = false @@ -23,6 +25,8 @@ type Handler struct { ArchiveCache *cch.Cache Log bool + depenencies *config.Dependencies + templates map[string]*template.Template } @@ -46,13 +50,6 @@ func (h *Handler) PrepareSessionCache() { }) } -func (h *Handler) prepareArchiveCache() { - h.ArchiveCache.OnEvicted(func(key string, data interface{}) { - archive := data.(*warc.Archive) - archive.Close() - }) -} - func (h *Handler) PrepareTemplates() error { // Prepare variables var err error @@ -109,6 +106,31 @@ func (h *Handler) GetSessionID(r *http.Request) string { // validateSession checks whether user session is still valid or not func (h *Handler) validateSession(r *http.Request) error { + authorization := r.Header.Get(model.AuthorizationHeader) + if authorization != "" { + authParts := strings.SplitN(authorization, " ", 2) + if len(authParts) != 2 && authParts[0] != model.AuthorizationTokenType { + return fmt.Errorf("session has been expired") + } + + account, err := h.depenencies.Domains.Auth.CheckToken(r.Context(), authParts[1]) + if err != nil { + return err + } + + if r.Method != "" && r.Method != "GET" && !account.Owner { + return fmt.Errorf("account level is not sufficient") + } + + h.depenencies.Log.WithFields(logrus.Fields{ + "username": account.Username, + "method": r.Method, + "path": r.URL.Path, + }).Info("allowing legacy api access using JWT token") + + return nil + } + sessionID := h.GetSessionID(r) if sessionID == "" { return fmt.Errorf("session is not exist") diff --git a/internal/webserver/server.go b/internal/webserver/server.go index aaf567e95..593255dfc 100644 --- a/internal/webserver/server.go +++ b/internal/webserver/server.go @@ -1,15 +1,11 @@ package webserver import ( - "fmt" - "net/http" - "path" "time" + "github.com/go-shiori/shiori/internal/config" "github.com/go-shiori/shiori/internal/database" - "github.com/julienschmidt/httprouter" cch "github.com/patrickmn/go-cache" - "github.com/sirupsen/logrus" ) // Config is parameter that used for starting web server @@ -22,89 +18,7 @@ type Config struct { Log bool } -// ErrorResponse defines a single HTTP error response. -type ErrorResponse struct { - Code int - Body string - contentType string - errorText string - Log bool -} - -func (e *ErrorResponse) Error() string { - return e.errorText -} - -func (e *ErrorResponse) ServeHTTP(w http.ResponseWriter, r *http.Request) { - if e.contentType != "" { - w.Header().Set("Content-Type", e.contentType) - } - body := e.Body - if e.Code != 0 { - w.WriteHeader(e.Code) - } - written := 0 - if len(body) > 0 { - written, _ = w.Write([]byte(body)) - } - if e.Log { - Logger(r, e.Code, written) - } -} - -// responseData will hold response details that we are interested in for logging -type responseData struct { - status int - size int -} - -// Wrapper around http.ResponseWriter to be able to catch calls to Write*() -type loggingResponseWriter struct { - http.ResponseWriter - responseData *responseData -} - -// Collect response size for each Write(). Also behave as the internal -// http.ResponseWriter by implicitely setting the status code to 200 at the -// first write. -func (r *loggingResponseWriter) Write(b []byte) (int, error) { - size, err := r.ResponseWriter.Write(b) // write response using original http.ResponseWriter - r.responseData.size += size // capture size - // Documented implicit WriteHeader(http.StatusOK) with first call to Write - if r.responseData.status == 0 { - r.responseData.status = http.StatusOK - } - return size, err -} - -// Capture calls to WriteHeader, might be called on errors. -func (r *loggingResponseWriter) WriteHeader(statusCode int) { - r.ResponseWriter.WriteHeader(statusCode) // write status code using original http.ResponseWriter - r.responseData.status = statusCode // capture status code -} - -// Logger Log through logrus, 200 will log as info, anything else as an error. -func Logger(r *http.Request, statusCode int, size int) { - if statusCode == http.StatusOK { - logrus.WithFields(logrus.Fields{ - "proto": r.Proto, - "remote": GetUserRealIP(r), - "reqlen": r.ContentLength, - "size": size, - "status": statusCode, - }).Info(r.Method, " ", r.RequestURI) - } else { - logrus.WithFields(logrus.Fields{ - "proto": r.Proto, - "remote": GetUserRealIP(r), - "reqlen": r.ContentLength, - "size": size, - "status": statusCode, - }).Warn(r.Method, " ", r.RequestURI) - } -} - -func GetLegacyHandler(cfg Config) *Handler { +func GetLegacyHandler(cfg Config, dependencies *config.Dependencies) *Handler { return &Handler{ DB: cfg.DB, DataDir: cfg.DataDir, @@ -113,128 +27,6 @@ func GetLegacyHandler(cfg Config) *Handler { ArchiveCache: cch.New(time.Minute, 5*time.Minute), RootPath: cfg.RootPath, Log: cfg.Log, + depenencies: dependencies, } } - -// ServeApp serves web interface in specified port -func ServeApp(cfg Config) error { - // Create handler - hdl := GetLegacyHandler(cfg) - - hdl.PrepareSessionCache() - hdl.prepareArchiveCache() - - err := hdl.PrepareTemplates() - if err != nil { - return fmt.Errorf("failed to prepare templates: %v", err) - } - - // Prepare errors - var ( - ErrorNotAllowed = &ErrorResponse{ - http.StatusMethodNotAllowed, - "Method is not allowed", - "text/plain; charset=UTF-8", - "MethodNotAllowedError", - cfg.Log, - } - ErrorNotFound = &ErrorResponse{ - http.StatusNotFound, - "Resource Not Found", - "text/plain; charset=UTF-8", - "NotFoundError", - cfg.Log, - } - ) - - // Create router and register error handlers - router := httprouter.New() - router.NotFound = ErrorNotFound - router.MethodNotAllowed = ErrorNotAllowed - - // withLogging will inject our own (compatible) http.ResponseWriter in order - // to collect details about the answer, i.e. the status code and the size of - // data in the response. Once done, these are passed further for logging, if - // relevant. - withLogging := func(req func(http.ResponseWriter, *http.Request, httprouter.Params)) func(http.ResponseWriter, *http.Request, httprouter.Params) { - return func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - d := &responseData{ - status: 0, - size: 0, - } - lrw := loggingResponseWriter{ - ResponseWriter: w, - responseData: d, - } - req(&lrw, r, ps) - if hdl.Log { - Logger(r, d.status, d.size) - } - } - } - - // jp here means "join path", as in "join route with root path" - jp := func(route string) string { - return path.Join(cfg.RootPath, route) - } - - router.GET(jp("/js/*filepath"), withLogging(hdl.serveJsFile)) - router.GET(jp("/res/*filepath"), withLogging(hdl.serveFile)) - router.GET(jp("/css/*filepath"), withLogging(hdl.serveFile)) - router.GET(jp("/fonts/*filepath"), withLogging(hdl.serveFile)) - - router.GET(cfg.RootPath, withLogging(hdl.serveIndexPage)) - router.GET(jp("/login"), withLogging(hdl.serveLoginPage)) - router.GET(jp("/bookmark/:id/thumb"), withLogging(hdl.ServeThumbnailImage)) - router.GET(jp("/bookmark/:id/content"), withLogging(hdl.ServeBookmarkContent)) - router.GET(jp("/bookmark/:id/ebook"), withLogging(hdl.ServeBookmarkEbook)) - router.GET(jp("/bookmark/:id/archive/*filepath"), withLogging(hdl.ServeBookmarkArchive)) - - router.POST(jp("/api/login"), withLogging(hdl.apiLogin)) - router.POST(jp("/api/logout"), withLogging(hdl.ApiLogout)) - router.GET(jp("/api/bookmarks"), withLogging(hdl.ApiGetBookmarks)) - router.GET(jp("/api/tags"), withLogging(hdl.ApiGetTags)) - router.PUT(jp("/api/tag"), withLogging(hdl.ApiRenameTag)) - router.POST(jp("/api/bookmarks"), withLogging(hdl.ApiInsertBookmark)) - router.DELETE(jp("/api/bookmarks"), withLogging(hdl.ApiDeleteBookmark)) - router.PUT(jp("/api/bookmarks"), withLogging(hdl.ApiUpdateBookmark)) - router.PUT(jp("/api/cache"), withLogging(hdl.ApiUpdateCache)) - router.PUT(jp("/api/ebook"), withLogging(hdl.ApiDownloadEbook)) - router.PUT(jp("/api/bookmarks/tags"), withLogging(hdl.ApiUpdateBookmarkTags)) - router.POST(jp("/api/bookmarks/ext"), withLogging(hdl.ApiInsertViaExtension)) - router.DELETE(jp("/api/bookmarks/ext"), withLogging(hdl.ApiDeleteViaExtension)) - - router.GET(jp("/api/accounts"), withLogging(hdl.ApiGetAccounts)) - router.PUT(jp("/api/accounts"), withLogging(hdl.ApiUpdateAccount)) - router.POST(jp("/api/accounts"), withLogging(hdl.ApiInsertAccount)) - router.DELETE(jp("/api/accounts"), withLogging(hdl.ApiDeleteAccount)) - - // Route for panic, keep logging anyhow - router.PanicHandler = func(w http.ResponseWriter, r *http.Request, arg interface{}) { - d := &responseData{ - status: 0, - size: 0, - } - lrw := loggingResponseWriter{ - ResponseWriter: w, - responseData: d, - } - http.Error(&lrw, fmt.Sprint(arg), 500) - if hdl.Log { - Logger(r, d.status, d.size) - } - } - - // Create server - url := fmt.Sprintf("%s:%d", cfg.ServerAddress, cfg.ServerPort) - svr := &http.Server{ - Addr: url, - Handler: router, - ReadTimeout: 10 * time.Second, - WriteTimeout: time.Minute, - } - - // Serve app - logrus.Infoln("Serve shiori in", url, cfg.RootPath) - return svr.ListenAndServe() -} diff --git a/internal/webserver/utils.go b/internal/webserver/utils.go index 08086fa8f..c4e69d976 100644 --- a/internal/webserver/utils.go +++ b/internal/webserver/utils.go @@ -1,75 +1,18 @@ package webserver import ( - "fmt" "html/template" "io" - "mime" "net" "net/http" nurl "net/url" "os" - fp "path/filepath" "regexp" "strings" "syscall" ) -var ( - rxRepeatedStrip = regexp.MustCompile(`(?i)-+`) - - presetMimeTypes = map[string]string{ - ".css": "text/css; charset=utf-8", - ".html": "text/html; charset=utf-8", - ".js": "application/javascript", - ".png": "image/png", - } -) - -func guessTypeByExtension(ext string) string { - ext = strings.ToLower(ext) - - if v, ok := presetMimeTypes[ext]; ok { - return v - } - - return mime.TypeByExtension(ext) -} - -func serveFile(w http.ResponseWriter, filePath string, cache bool) error { - // Open file - src, err := assets.Open(filePath) - if err != nil { - return err - } - defer src.Close() - - // Cache this file if needed - if cache { - info, err := src.Stat() - if err != nil { - return err - } - - etag := fmt.Sprintf(`W/"%x-%x"`, info.ModTime().Unix(), info.Size()) - w.Header().Set("ETag", etag) - w.Header().Set("Cache-Control", "max-age=86400") - } else { - w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate") - } - - // Set content type - ext := fp.Ext(filePath) - mimeType := guessTypeByExtension(ext) - if mimeType != "" { - w.Header().Set("Content-Type", mimeType) - w.Header().Set("X-Content-Type-Options", "nosniff") - } - - // Serve file - _, err = io.Copy(w, src) - return err -} +var rxRepeatedStrip = regexp.MustCompile(`(?i)-+`) func createRedirectURL(newPath, previousPath string) string { urlQueries := nurl.Values{} @@ -87,14 +30,6 @@ func redirectPage(w http.ResponseWriter, r *http.Request, url string) { http.Redirect(w, r, url, http.StatusMovedPermanently) } -func assetExists(filePath string) bool { - f, err := assets.Open(filePath) - if f != nil { - f.Close() - } - return err == nil || !os.IsNotExist(err) -} - func fileExists(filePath string) bool { info, err := os.Stat(filePath) return err == nil && !info.IsDir()