From 4eac10c9d14a452d120d94fc333bd32d139e3a4e Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Sat, 22 Nov 2025 09:25:01 +0100 Subject: [PATCH] API: Apply "golangci-lint" recommendations #5330 Signed-off-by: Michael Mayer --- internal/api/albums.go | 8 +++---- internal/api/api.go | 7 +++--- internal/api/api_auth.go | 11 ++++------ internal/api/api_test.go | 4 ---- internal/api/batch_photos.go | 16 ++++++++------ internal/api/batch_photos_edit.go | 4 ++-- internal/api/batch_photos_edit_test.go | 12 +++++------ internal/api/cluster_nodes_register.go | 2 +- internal/api/config_options.go | 2 +- internal/api/covers_test.go | 4 ++-- internal/api/download_album.go | 2 +- internal/api/oauth_revoke.go | 15 +++++++------ internal/api/oauth_revoke_test.go | 2 +- internal/api/oauth_token.go | 16 ++++++++------ internal/api/oidc_login.go | 3 +-- internal/api/oidc_redirect.go | 22 +++++++++---------- internal/api/photo_label.go | 18 +++++++++++++--- internal/api/photo_unstack.go | 14 ++++++------ internal/api/server.go | 4 +--- internal/api/services_upload.go | 2 +- internal/api/session_create.go | 24 ++++++++++++--------- internal/api/session_test.go | 10 ++++----- internal/api/share.go | 4 ++-- internal/api/share_preview.go | 7 +++++- internal/api/share_test.go | 4 ++-- internal/api/users_passcode.go | 15 ++++++++----- internal/api/users_passcode_test.go | 8 +++---- internal/api/users_upload.go | 14 ++++++------ internal/api/users_upload_multipart_test.go | 12 ----------- internal/api/video.go | 4 +--- internal/api/vision_caption.go | 7 +++--- internal/api/vision_face_test.go | 4 ---- internal/api/vision_labels_test.go | 4 ---- internal/api/vision_nsfw_test.go | 4 ---- internal/api/zip.go | 3 ++- 35 files changed, 146 insertions(+), 146 deletions(-) diff --git a/internal/api/albums.go b/internal/api/albums.go index 2f50da1f3..bb08b8c54 100644 --- a/internal/api/albums.go +++ b/internal/api/albums.go @@ -565,13 +565,11 @@ func AddPhotosToAlbum(router *gin.RouterGroup) { // Find album by UID. album, err := query.AlbumByUID(uid) - if err != nil { + switch { + case err != nil, !album.HasID(): AbortAlbumNotFound(c) return - } else if !album.HasID() { - AbortAlbumNotFound(c) - return - } else if frm.Empty() { + case frm.Empty(): Abort(c, http.StatusBadRequest, i18n.ErrNoItemsSelected) return } diff --git a/internal/api/api.go b/internal/api/api.go index 3118fcdb8..08cfd33ef 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -25,11 +25,12 @@ Additional information can be found in our Developer Guide: package api import ( - _ "net/http" + // Blank imports register HTTP handlers, models, translations, and form bindings via init side effects. + _ "net/http" // register net/http handlers referenced by swagger - _ "github.com/gin-gonic/gin" + _ "github.com/gin-gonic/gin" // register gin metadata used by swaggo - _ "github.com/photoprism/photoprism/internal/auth/acl" + _ "github.com/photoprism/photoprism/internal/auth/acl" // embed ACL docs _ "github.com/photoprism/photoprism/internal/entity" _ "github.com/photoprism/photoprism/internal/entity/query" _ "github.com/photoprism/photoprism/internal/event" diff --git a/internal/api/api_auth.go b/internal/api/api_auth.go index 05ab72732..1b9bd11f4 100644 --- a/internal/api/api_auth.go +++ b/internal/api/api_auth.go @@ -35,13 +35,10 @@ func AuthAny(c *gin.Context, resource acl.Resource, perms acl.Permissions) (s *e c.Header(header.CacheControl, header.CacheControlNoStore) // Allow requests based on an access token for specific resources. - switch resource { - case acl.ResourceVision: - if perms.Contains(acl.ActionUse) && vision.ServiceApi && vision.ServiceKey != "" && vision.ServiceKey == authToken { - s = entity.NewSessionFromToken(c, authToken, acl.ResourceVision.String(), "service-key") - event.AuditInfo([]string{clientIp, "%s", "%s %s as %s", status.Granted}, s.RefID, perms.First(), string(resource), s.GetClientRole().String()) - return s - } + if resource == acl.ResourceVision && perms.Contains(acl.ActionUse) && vision.ServiceApi && vision.ServiceKey != "" && vision.ServiceKey == authToken { + s = entity.NewSessionFromToken(c, authToken, acl.ResourceVision.String(), "service-key") + event.AuditInfo([]string{clientIp, "%s", "%s %s as %s", status.Granted}, s.RefID, perms.First(), string(resource), s.GetClientRole().String()) + return s } // Find active session to perform authorization check or deny if no session was found. diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 3c41e0f80..974756446 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -58,10 +58,6 @@ func (r *CloseableResponseRecorder) CloseNotify() <-chan bool { return r.closeCh } -func (r *CloseableResponseRecorder) closeClient() { - r.closeCh <- true -} - // NewApiTest returns new API test helper. func NewApiTest() (app *gin.Engine, router *gin.RouterGroup, conf *config.Config) { gin.SetMode(gin.TestMode) diff --git a/internal/api/batch_photos.go b/internal/api/batch_photos.go index 7b0483391..65bab6f25 100644 --- a/internal/api/batch_photos.go +++ b/internal/api/batch_photos.go @@ -329,25 +329,27 @@ func BatchPhotosDelete(router *gin.RouterGroup) { } // Get selection or all archived photos if f.All is true. - if len(frm.Photos) == 0 && !frm.All { + switch { + case len(frm.Photos) == 0 && !frm.All: Abort(c, http.StatusBadRequest, i18n.ErrNoItemsSelected) return - } else if frm.All { + case frm.All: photos, err = query.ArchivedPhotos(1000000, 0) - } else { + default: photos, err = query.SelectedPhotos(frm) } // Abort if the query failed or no photos were found. - if err != nil { + switch { + case err != nil: log.Errorf("archive: %s", err) Abort(c, http.StatusBadRequest, i18n.ErrNoItemsSelected) return - } else if len(photos) > 0 { - log.Infof("archive: deleting %s", english.Plural(len(photos), "photo", "photos")) - } else { + case len(photos) == 0: Abort(c, http.StatusBadRequest, i18n.ErrNoItemsSelected) return + default: + log.Infof("archive: deleting %s", english.Plural(len(photos), "photo", "photos")) } var deleted entity.Photos diff --git a/internal/api/batch_photos_edit.go b/internal/api/batch_photos_edit.go index f97612a85..7992c8229 100644 --- a/internal/api/batch_photos_edit.go +++ b/internal/api/batch_photos_edit.go @@ -73,7 +73,7 @@ func BatchPhotosEdit(router *gin.RouterGroup) { return } - preloadedPhotos := map[string]*entity.Photo{} + var preloadedPhotos map[string]*entity.Photo if hydrated, err := query.PhotoPreloadByUIDs(photos.UIDs()); err != nil { log.Errorf("batch: failed to preload photo selection: %s", err) @@ -107,7 +107,7 @@ func BatchPhotosEdit(router *gin.RouterGroup) { // Refresh selected photos from database? if !savedAny { // Don't refresh. - } else if photos, count, err = search.BatchPhotos(frm.Photos, s); err != nil { + } else if photos, _, err = search.BatchPhotos(frm.Photos, s); err != nil { log.Errorf("batch: %s (refresh selection)", clean.Error(err)) } diff --git a/internal/api/batch_photos_edit_test.go b/internal/api/batch_photos_edit_test.go index cfc7032e1..5ee16a080 100644 --- a/internal/api/batch_photos_edit_test.go +++ b/internal/api/batch_photos_edit_test.go @@ -141,7 +141,7 @@ func TestBatchPhotosEdit(t *testing.T) { // Check the save response values. saveValues := gjson.Get(saveBody, "values").Raw - //t.Logf("save values: %#v", saveValues) + // t.Logf("save values: %#v", saveValues) timezoneAfter := gjson.Get(saveValues, "TimeZone") assert.Equal(t, timezoneAfter.String(), timezoneBefore.String()) altitudeAfter := gjson.Get(saveValues, "Altitude") @@ -200,10 +200,10 @@ func TestBatchPhotosEdit(t *testing.T) { assert.Equal(t, takenAfter.String(), takenBefore.String()) takenLocalAfter := gjson.Get(saveValues, "TakenAtLocal") assert.Equal(t, takenLocalAfter.String(), takenLocalBefore.String()) - //TODO Uncomment once keywords may be supported - //keywordsAfter := gjson.Get(saveValues, "DetailsKeywords") - //assert.Equal(t, keywordsAfter.String(), keywordsBefore.String()) - //assert.Equal(t, editValues, saveValues) + // TODO Uncomment once keywords may be supported + // keywordsAfter := gjson.Get(saveValues, "DetailsKeywords") + // assert.Equal(t, keywordsAfter.String(), keywordsBefore.String()) + // assert.Equal(t, editValues, saveValues) }) t.Run("SuccessChangeLocationValues", func(t *testing.T) { // Create new API test instance. @@ -471,7 +471,7 @@ func TestBatchPhotosEdit(t *testing.T) { editPhotos := gjson.Get(editBody, "models").Array() assert.Equal(t, len(editPhotos), 2) editValues := gjson.Get(editBody, "values").Raw - //t.Logf(editValues) + // t.Logf(editValues) albumsBefore := gjson.Get(editValues, "Albums") assert.Contains(t, albumsBefore.String(), "{\"value\":\"as6sg6bipotaab19\",\"title\":\"IlikeFood\",\"mixed\":false,\"action\":\"none\"}") assert.Contains(t, albumsBefore.String(), "{\"value\":\"as6sg6bxpogaaba7\",\"title\":\"Christmas 2030\",\"mixed\":true,\"action\":\"none\"}") diff --git a/internal/api/cluster_nodes_register.go b/internal/api/cluster_nodes_register.go index 7afdcd5c8..1cc96c5ff 100644 --- a/internal/api/cluster_nodes_register.go +++ b/internal/api/cluster_nodes_register.go @@ -111,7 +111,7 @@ func ClusterNodesRegister(router *gin.RouterGroup) { } for i := 0; i < len(name); i++ { b := name[i] - if !(b == '-' || (b >= 'a' && b <= 'z') || (b >= '0' && b <= '9')) { + if b != '-' && (b < 'a' || b > 'z') && (b < '0' || b > '9') { event.AuditWarn([]string{clientIp, string(acl.ResourceCluster), "register", "invalid name chars", status.Failed}) AbortBadRequest(c) return diff --git a/internal/api/config_options.go b/internal/api/config_options.go index 3b34e906b..5d25e64ff 100644 --- a/internal/api/config_options.go +++ b/internal/api/config_options.go @@ -72,7 +72,7 @@ func SaveConfigOptions(router *gin.RouterGroup) { v := make(entity.Values) if fs.FileExists(fileName) { - yamlData, err := os.ReadFile(fileName) + yamlData, err := os.ReadFile(fileName) // #nosec G304 file path validated above if err != nil { log.Errorf("config: failed loading values from %s (%s)", clean.Log(fileName), err) diff --git a/internal/api/covers_test.go b/internal/api/covers_test.go index cd655d943..7e21e9945 100644 --- a/internal/api/covers_test.go +++ b/internal/api/covers_test.go @@ -56,8 +56,8 @@ func TestLabelCover(t *testing.T) { t.Run("CouldNotFindOriginal", func(t *testing.T) { app, router, conf := NewApiTest() LabelCover(router) - //r := PerformRequest(app, "GET", "/api/v1/labels/ls6sg6b1wowuy3c3/t/"+conf.PreviewToken()+"/tile_500") - //ls6sg6b1wowuy3c2 + // r := PerformRequest(app, "GET", "/api/v1/labels/ls6sg6b1wowuy3c3/t/"+conf.PreviewToken()+"/tile_500") + // ls6sg6b1wowuy3c2 r := PerformRequest(app, "GET", "/api/v1/labels/ls6sg6b1wowuy3c2/t/"+conf.PreviewToken()+"/tile_500") assert.Equal(t, http.StatusOK, r.Code) }) diff --git a/internal/api/download_album.go b/internal/api/download_album.go index 8d41b4818..ea50436a6 100644 --- a/internal/api/download_album.go +++ b/internal/api/download_album.go @@ -130,7 +130,7 @@ func DownloadAlbum(router *gin.RouterGroup) { alias = file.DownloadName(dlName, seq) } - aliases[key] += 1 + aliases[key]++ if fs.FileExists(fileName) { if zipErr := fs.ZipFile(zipWriter, fileName, alias, false); zipErr != nil { diff --git a/internal/api/oauth_revoke.go b/internal/api/oauth_revoke.go index 926731c5c..dd37e5b8b 100644 --- a/internal/api/oauth_revoke.go +++ b/internal/api/oauth_revoke.go @@ -133,26 +133,27 @@ func OAuthRevoke(router *gin.RouterGroup) { } // Check revocation request and abort if invalid. - if err != nil { + switch { + case err != nil: event.AuditErr([]string{clientIp, "oauth2", actor, action, "delete %s as %s", status.Error(err)}, clean.Log(sess.RefID), role.String()) AbortInvalidCredentials(c) return - } else if sess == nil { - event.AuditErr([]string{clientIp, "oauth2", actor, action, "delete %s as %s", status.Denied}, clean.Log(sess.RefID), role.String()) + case sess == nil: + event.AuditErr([]string{clientIp, "oauth2", actor, action, "delete %s as %s", status.Denied}, "", role.String()) AbortInvalidCredentials(c) return - } else if sess.Abort(c) { + case sess.Abort(c): event.AuditErr([]string{clientIp, "oauth2", actor, action, "delete %s as %s", status.Denied}, clean.Log(sess.RefID), role.String()) return - } else if !sess.IsClient() { + case !sess.IsClient(): event.AuditErr([]string{clientIp, "oauth2", actor, action, "delete %s as %s", status.Denied}, clean.Log(sess.RefID), role.String()) c.AbortWithStatusJSON(http.StatusForbidden, i18n.NewResponse(http.StatusForbidden, i18n.ErrForbidden)) return - } else if sUserUID != "" && sess.UserUID != sUserUID { + case sUserUID != "" && sess.UserUID != sUserUID: event.AuditErr([]string{clientIp, "oauth2", actor, action, "delete %s as %s", authn.ErrUnauthorized.Error()}, clean.Log(sess.RefID), role.String()) AbortInvalidCredentials(c) return - } else { + default: event.AuditInfo([]string{clientIp, "oauth2", actor, action, "delete %s as %s", status.Granted}, clean.Log(sess.RefID), role.String()) } diff --git a/internal/api/oauth_revoke_test.go b/internal/api/oauth_revoke_test.go index 04948c590..730a776f6 100644 --- a/internal/api/oauth_revoke_test.go +++ b/internal/api/oauth_revoke_test.go @@ -18,7 +18,7 @@ import ( ) func TestOAuthRevoke(t *testing.T) { - const tokenPath = "/api/v1/oauth/token" + const tokenPath = "/api/v1/oauth/token" // #nosec G101 test constant, not a credential const revokePath = "/api/v1/oauth/revoke" t.Run("ClientSuccessToken", func(t *testing.T) { diff --git a/internal/api/oauth_token.go b/internal/api/oauth_token.go index 6cf796120..a0c1002c4 100644 --- a/internal/api/oauth_token.go +++ b/internal/api/oauth_token.go @@ -86,11 +86,12 @@ func OAuthToken(router *gin.RouterGroup) { return } - if frm.ClientID != "" { + switch { + case frm.ClientID != "": actor = fmt.Sprintf("client %s", clean.Log(frm.ClientID)) - } else if frm.Username != "" { + case frm.Username != "": actor = fmt.Sprintf("user %s", clean.Log(frm.Username)) - } else if frm.GrantType == authn.GrantPassword { + case frm.GrantType == authn.GrantPassword: actor = "unknown user" } @@ -150,17 +151,18 @@ func OAuthToken(router *gin.RouterGroup) { authUser, authProvider, authMethod, authErr := entity.Auth(loginForm, nil, c) - if authProvider.IsClient() { + switch { + case authProvider.IsClient(): event.AuditErr([]string{clientIp, "oauth2", actor, action, status.Denied}) AbortInvalidCredentials(c) return - } else if authMethod.Is(authn.Method2FA) && errors.Is(authErr, authn.ErrPasscodeRequired) { + case authMethod.Is(authn.Method2FA) && errors.Is(authErr, authn.ErrPasscodeRequired): // Ok. - } else if authErr != nil { + case authErr != nil: event.AuditErr([]string{clientIp, "oauth2", actor, action, status.Error(authErr)}) AbortInvalidCredentials(c) return - } else if !authUser.Equal(s.GetUser()) { + case !authUser.Equal(s.GetUser()): event.AuditErr([]string{clientIp, "oauth2", actor, action, authn.ErrUserDoesNotMatch.Error()}) AbortInvalidCredentials(c) return diff --git a/internal/api/oidc_login.go b/internal/api/oidc_login.go index 628ad8cfb..615802450 100644 --- a/internal/api/oidc_login.go +++ b/internal/api/oidc_login.go @@ -51,8 +51,7 @@ func OIDCLogin(router *gin.RouterGroup) { } // Check request rate limit. - var r *limiter.Request - r = limiter.Login.Request(clientIp) + r := limiter.Login.Request(clientIp) // Abort if failure rate limit is exceeded. if r.Reject() || limiter.Auth.Reject(clientIp) { diff --git a/internal/api/oidc_redirect.go b/internal/api/oidc_redirect.go index c5a1b783f..30ad88d0d 100644 --- a/internal/api/oidc_redirect.go +++ b/internal/api/oidc_redirect.go @@ -66,8 +66,7 @@ func OIDCRedirect(router *gin.RouterGroup) { } // Check request rate limit. - var r *limiter.Request - r = limiter.Login.Request(clientIp) + r := limiter.Login.Request(clientIp) // Abort if failure rate limit is exceeded. if r.Reject() || limiter.Auth.Reject(clientIp) { @@ -145,17 +144,18 @@ func OIDCRedirect(router *gin.RouterGroup) { event.AuditInfo([]string{clientIp, "create session", "oidc", "found user", userName}) // Check if the account is enabled and the OIDC Subject ID matches. - if !user.CanLogIn() { + switch { + case !user.CanLogIn(): event.AuditErr([]string{clientIp, "create session", "oidc", userName, authn.ErrAccountDisabled.Error()}) event.LoginError(clientIp, "oidc", userName, userAgent, authn.ErrAccountDisabled.Error()) c.HTML(http.StatusUnauthorized, "auth.gohtml", CreateSessionError(http.StatusUnauthorized, i18n.Error(i18n.ErrInvalidCredentials))) return - } else if authn.ProviderOIDC.NotEqual(user.AuthProvider) { + case authn.ProviderOIDC.NotEqual(user.AuthProvider): event.AuditErr([]string{clientIp, "create session", "oidc", userName, authn.ErrAuthProviderIsNotOIDC.Error()}) event.LoginError(clientIp, "oidc", userName, userAgent, authn.ErrAuthProviderIsNotOIDC.Error()) c.HTML(http.StatusUnauthorized, "auth.gohtml", CreateSessionError(http.StatusUnauthorized, i18n.Error(i18n.ErrInvalidCredentials))) return - } else if user.AuthID == "" || oidcUser.AuthID == "" || user.AuthID != oidcUser.AuthID { + case user.AuthID == "" || oidcUser.AuthID == "" || user.AuthID != oidcUser.AuthID: event.AuditErr([]string{clientIp, "create session", "oidc", userName, authn.ErrInvalidAuthID.Error()}) event.LoginError(clientIp, "oidc", userName, userAgent, authn.ErrInvalidAuthID.Error()) c.HTML(http.StatusUnauthorized, "auth.gohtml", CreateSessionError(http.StatusUnauthorized, i18n.Error(i18n.ErrInvalidCredentials))) @@ -228,12 +228,12 @@ func OIDCRedirect(router *gin.RouterGroup) { } // Set user avatar image? - if avatarUrl := userInfo.Picture; avatarUrl == "" || user.HasAvatar() { - // Do nothing. - } else if err = avatar.SetUserImageURL(user, avatarUrl, entity.SrcOIDC, conf.ThumbCachePath()); err != nil { - event.AuditWarn([]string{clientIp, "create session", "oidc", userName, "failed to set avatar image", err.Error()}) + if avatarUrl := userInfo.Picture; avatarUrl != "" && !user.HasAvatar() { + if err = avatar.SetUserImageURL(user, avatarUrl, entity.SrcOIDC, conf.ThumbCachePath()); err != nil { + event.AuditWarn([]string{clientIp, "create session", "oidc", userName, "failed to set avatar image", err.Error()}) + } } - } else if conf.UsersQuotaReached(conf.OIDCRole()) { + } else if conf.UsersQuotaReached(conf.OIDCRole()) { //nolint:gocritic userName = oidcUser.Username() event.AuditWarn([]string{clientIp, "create session", "oidc", "create user", userName, authn.ErrUsersQuotaExceeded.Error()}) event.LoginError(clientIp, "oidc", userName, userAgent, authn.ErrUsersQuotaExceeded.Error()) @@ -247,7 +247,7 @@ func OIDCRedirect(router *gin.RouterGroup) { // Resolve potential naming conflict by adding a random number to the username. if found := entity.FindUserByName(userName); found != nil { - userName = userName + rnd.Base10(6) + userName += rnd.Base10(6) } event.AuditInfo([]string{clientIp, "create session", "oidc", "create user", userName}) diff --git a/internal/api/photo_label.go b/internal/api/photo_label.go index 9d98adb70..c59057254 100644 --- a/internal/api/photo_label.go +++ b/internal/api/photo_label.go @@ -1,6 +1,7 @@ package api import ( + "errors" "net/http" "strconv" @@ -136,6 +137,11 @@ func RemovePhotoLabel(router *gin.RouterGroup) { return } + if labelId < 0 { + AbortBadRequest(c, errors.New("invalid label id")) + return + } + label, err := query.PhotoLabel(m.ID, uint(labelId)) if err != nil { @@ -143,13 +149,14 @@ func RemovePhotoLabel(router *gin.RouterGroup) { return } - if (label.LabelSrc == classify.SrcManual || label.LabelSrc == entity.SrcBatch) && label.Uncertainty < 100 { + switch { + case (label.LabelSrc == classify.SrcManual || label.LabelSrc == entity.SrcBatch) && label.Uncertainty < 100: logErr("label", entity.Db().Delete(&label).Error) - } else if label.LabelSrc != classify.SrcManual && label.LabelSrc != entity.SrcBatch { + case label.LabelSrc != classify.SrcManual && label.LabelSrc != entity.SrcBatch: label.Uncertainty = 100 label.LabelSrc = entity.SrcManual logErr("label", entity.Db().Save(&label).Error) - } else { + default: logErr("label", entity.Db().Save(&label).Error) } @@ -212,6 +219,11 @@ func UpdatePhotoLabel(router *gin.RouterGroup) { return } + if labelId < 0 { + AbortBadRequest(c, errors.New("invalid label id")) + return + } + label, err := query.PhotoLabel(m.ID, uint(labelId)) if err != nil { diff --git a/internal/api/photo_unstack.go b/internal/api/photo_unstack.go index 573b5907d..b90d88fe4 100644 --- a/internal/api/photo_unstack.go +++ b/internal/api/photo_unstack.go @@ -47,15 +47,16 @@ func PhotoUnstack(router *gin.RouterGroup) { return } - if file.FilePrimary { + switch { + case file.FilePrimary: log.Errorf("photo: cannot unstack primary file") AbortBadRequest(c) return - } else if file.FileSidecar { + case file.FileSidecar: log.Errorf("photo: cannot unstack sidecar files") AbortBadRequest(c) return - } else if file.FileRoot != entity.RootOriginals { + case file.FileRoot != entity.RootOriginals: log.Errorf("photo: only originals can be unstacked") AbortBadRequest(c) return @@ -91,15 +92,16 @@ func PhotoUnstack(router *gin.RouterGroup) { related, err := unstackFile.RelatedFiles(false) - if err != nil { + switch { + case err != nil: log.Errorf("photo: %s (unstack %s)", err, clean.Log(baseName)) AbortEntityNotFound(c) return - } else if related.Len() == 0 { + case related.Len() == 0: log.Errorf("photo: found no files for %s (unstack)", clean.Log(baseName)) AbortEntityNotFound(c) return - } else if related.Main == nil { + case related.Main == nil: log.Errorf("photo: found no main media file for %s (unstack)", clean.Log(baseName)) AbortEntityNotFound(c) return diff --git a/internal/api/server.go b/internal/api/server.go index 872cef4b9..ed13cbf14 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -6,7 +6,6 @@ import ( "github.com/gin-gonic/gin" "github.com/photoprism/photoprism/internal/auth/acl" - "github.com/photoprism/photoprism/internal/config" "github.com/photoprism/photoprism/internal/photoprism/get" "github.com/photoprism/photoprism/internal/server/process" ) @@ -32,8 +31,7 @@ func StopServer(router *gin.RouterGroup) { return } - var options *config.Options - options = conf.Options() + options := conf.Options() // Trigger restart. // diff --git a/internal/api/services_upload.go b/internal/api/services_upload.go index 21466eea8..c07bcded5 100644 --- a/internal/api/services_upload.go +++ b/internal/api/services_upload.go @@ -74,7 +74,7 @@ func UploadToService(router *gin.RouterGroup) { alias = file.ShareBase(seq) } - aliases[key] += 1 + aliases[key]++ entity.FirstOrCreateFileShare(entity.NewFileShare(file.ID, m.ID, alias)) } diff --git a/internal/api/session_create.go b/internal/api/session_create.go index 5a5e0dbbc..9414274ce 100644 --- a/internal/api/session_create.go +++ b/internal/api/session_create.go @@ -100,13 +100,14 @@ func CreateSession(router *gin.RouterGroup) { // Check authentication credentials. if err = sess.LogIn(frm, c); err != nil { - if sess.GetMethod().IsNot(authn.Method2FA) { + switch { + case sess.GetMethod().IsNot(authn.Method2FA): c.AbortWithStatusJSON(sess.HttpStatus(), gin.H{"error": i18n.Msg(i18n.ErrInvalidCredentials)}) - } else if errors.Is(err, authn.ErrPasscodeRequired) { + case errors.Is(err, authn.ErrPasscodeRequired): c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": err.Error(), "code": 32, "message": i18n.Msg(i18n.ErrPasscodeRequired)}) // Return the reserved request rate limit tokens if password is correct, even if the verification code is missing. r.Success() - } else { + default: c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": err.Error(), "code": http.StatusUnauthorized, "message": i18n.Msg(i18n.ErrInvalidPasscode)}) } return @@ -119,17 +120,20 @@ func CreateSession(router *gin.RouterGroup) { } // Save session after successful authentication. - if sess, err = get.Session().Save(sess); err != nil { - event.AuditErr([]string{clientIp, status.Error(err)}) + switch saved, saveErr := get.Session().Save(sess); { + case saveErr != nil: + event.AuditErr([]string{clientIp, status.Error(saveErr)}) c.AbortWithStatusJSON(sess.HttpStatus(), gin.H{"error": i18n.Msg(i18n.ErrInvalidCredentials)}) return - } else if sess == nil { + case saved == nil: c.AbortWithStatusJSON(sess.HttpStatus(), gin.H{"error": i18n.Msg(i18n.ErrUnexpected)}) return - } else if isNew { - event.AuditInfo([]string{clientIp, "session %s", "created"}, sess.RefID) - } else { - event.AuditInfo([]string{clientIp, "session %s", "updated"}, sess.RefID) + case isNew: + event.AuditInfo([]string{clientIp, "session %s", "created"}, saved.RefID) + sess = saved + default: + event.AuditInfo([]string{clientIp, "session %s", "updated"}, saved.RefID) + sess = saved } // Return the reserved request rate limit tokens after successful authentication. diff --git a/internal/api/session_test.go b/internal/api/session_test.go index f35963f5c..35732744a 100644 --- a/internal/api/session_test.go +++ b/internal/api/session_test.go @@ -72,7 +72,7 @@ func TestCreateSession(t *testing.T) { CreateSession(router) r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"username": "admin", "password": "photoprism"}`) - //t.Logf("Response Body: %s", r.Body.String()) + // t.Logf("Response Body: %s", r.Body.String()) userName := gjson.Get(r.Body.String(), "user.Name").String() assert.Equal(t, "admin", userName) assert.Equal(t, http.StatusOK, r.Code) @@ -373,7 +373,7 @@ func TestDeleteSession(t *testing.T) { defer conf.SetAuthMode(config.AuthModePublic) DeleteSession(router) - bobToken := "69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7ac1" + bobToken := "69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7ac1" // #nosec G101 test token r := PerformRequest(app, http.MethodDelete, "/api/v1/session/"+rnd.SessionID(bobToken)) assert.Equal(t, http.StatusUnauthorized, r.Code) @@ -399,7 +399,7 @@ func TestDeleteSession(t *testing.T) { DeleteSession(router) bobToken := AuthenticateUser(app, router, "bob", "Bobbob123!") - aliceToken := "69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7ac0" + aliceToken := "69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7ac0" // #nosec G101 test token r := AuthenticatedRequest(app, http.MethodDelete, "/api/v1/session/"+rnd.SessionID(aliceToken), bobToken) assert.Equal(t, http.StatusForbidden, r.Code) @@ -411,7 +411,7 @@ func TestDeleteSession(t *testing.T) { DeleteSession(router) aliceToken := AuthenticateUser(app, router, "alice", "Alice123!") - bobToken := "69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7ac1" + bobToken := "69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7ac1" // #nosec G101 test token r := AuthenticatedRequest(app, http.MethodDelete, "/api/v1/session/"+rnd.SessionID(bobToken), aliceToken) assert.Equal(t, http.StatusForbidden, r.Code) @@ -423,7 +423,7 @@ func TestDeleteSession(t *testing.T) { DeleteSession(router) authToken := AuthenticateUser(app, router, "alice", "Alice123!") - deleteToken := "638bffc9b86a8fda0d908ebee84a43930cb8d1e3507f4aa0" + deleteToken := "638bffc9b86a8fda0d908ebee84a43930cb8d1e3507f4aa0" // #nosec G101 test token r := AuthenticatedRequest(app, http.MethodDelete, "/api/v1/session/"+rnd.SessionID(deleteToken), authToken) assert.Equal(t, http.StatusForbidden, r.Code) diff --git a/internal/api/share.go b/internal/api/share.go index d4048c8e4..12c394582 100644 --- a/internal/api/share.go +++ b/internal/api/share.go @@ -36,7 +36,7 @@ func ShareToken(router *gin.RouterGroup) { } clientConfig := conf.ClientShare() - clientConfig.SiteUrl = clientConfig.SiteUrl + path.Join("s", token) + clientConfig.SiteUrl += path.Join("s", token) uri := conf.LibraryUri("/albums") c.HTML(http.StatusOK, "share.gohtml", gin.H{"shared": gin.H{"token": token, "uri": uri}, "config": clientConfig}) @@ -71,7 +71,7 @@ func ShareTokenShared(router *gin.RouterGroup) { uid := links[0].ShareUID clientConfig := conf.ClientShare() - clientConfig.SiteUrl = clientConfig.SiteUrl + path.Join("s", token, uid) + clientConfig.SiteUrl += path.Join("s", token, uid) clientConfig.SitePreview = clientConfig.SiteUrl + "/preview" if a, err := query.AlbumByUID(uid); err == nil { diff --git a/internal/api/share_preview.go b/internal/api/share_preview.go index add59d183..538d55f66 100644 --- a/internal/api/share_preview.go +++ b/internal/api/share_preview.go @@ -114,7 +114,7 @@ func SharePreview(router *gin.RouterGroup) { return } - size, _ := thumb.Sizes[thumb.Tile500] + size := thumb.Sizes[thumb.Tile500] images := make([]image.Image, 0, len(p)) @@ -147,6 +147,11 @@ func SharePreview(router *gin.RouterGroup) { // Create album preview from thumbnail images. preview, err := frame.Collage(frame.Polaroid, images) + if err != nil { + log.Warnf("preview collage: %v", err) + c.Redirect(http.StatusTemporaryRedirect, conf.SitePreview()) + return + } // Downsize from 1600x900 to 1200x675. preview = imaging.Resize(preview, 1200, 0, imaging.Lanczos) diff --git a/internal/api/share_test.go b/internal/api/share_test.go index 6568b5cc1..71853c498 100644 --- a/internal/api/share_test.go +++ b/internal/api/share_test.go @@ -14,7 +14,7 @@ func TestShareToken(t *testing.T) { r := PerformRequest(app, "GET", "/api/v1/xxx") assert.Equal(t, http.StatusTemporaryRedirect, r.Code) }) - //TODO Why does it panic? + // TODO Why does it panic? /*t.Run("ValidToken", func(t *testing.T) { app, router, _ := NewApiTest() ShareToken(router) @@ -30,7 +30,7 @@ func TestShareTokenShared(t *testing.T) { r := PerformRequest(app, "GET", "/api/v1/1jxf3jfn2k/ss6sg6bxpogaaba7") assert.Equal(t, http.StatusTemporaryRedirect, r.Code) }) - //TODO Why does it panic? + // TODO Why does it panic? /*t.Run("ValidTokenAndShare", func(t *testing.T) { app, router, _ := NewApiTest() ShareTokenShared(router) diff --git a/internal/api/users_passcode.go b/internal/api/users_passcode.go index a06aeab4d..fe9da2c38 100644 --- a/internal/api/users_passcode.go +++ b/internal/api/users_passcode.go @@ -336,20 +336,25 @@ func checkUserPasscodePassword(c *gin.Context, user *entity.User, password strin } // Check if user login credentials are valid. - if authUser, provider, method, authErr := entity.Auth(f, nil, c); method == authn.Method2FA && errors.Is(authErr, authn.ErrPasscodeRequired) { + authUser, provider, method, authErr := entity.Auth(f, nil, c) + + switch { + case method == authn.Method2FA && errors.Is(authErr, authn.ErrPasscodeRequired): return http.StatusOK, i18n.MsgVerified, nil - } else if authErr != nil { + case authErr != nil: // Abort if authentication has failed otherwise. return code, msg, authErr - } else if authUser == nil { + case authUser == nil: // Abort if account was not found. return code, msg, authn.ErrAccountNotFound - } else if !authUser.Equal(user) { + case !authUser.Equal(user): // Abort if user accounts do not match. return code, msg, authn.ErrUserDoesNotMatch - } else if !provider.SupportsPasscodeAuthentication() || method != authn.MethodDefault { + case !provider.SupportsPasscodeAuthentication() || method != authn.MethodDefault: // Abort if e.g. an app password was provided. return code, msg, authn.ErrInvalidCredentials + default: + return http.StatusOK, i18n.MsgVerified, nil } } diff --git a/internal/api/users_passcode_test.go b/internal/api/users_passcode_test.go index 60a444712..31a3bf7ce 100644 --- a/internal/api/users_passcode_test.go +++ b/internal/api/users_passcode_test.go @@ -229,7 +229,7 @@ func TestDeactivateUserPasscode(t *testing.T) { } func TestUserPasscode(t *testing.T) { - //create + // create app, router, conf := NewApiTest() conf.SetAuthMode(config.AuthModePasswd) defer conf.SetAuthMode(config.AuthModePublic) @@ -260,7 +260,7 @@ func TestUserPasscode(t *testing.T) { code, err := totp.GenerateCode(secret, time.Now()) - //confirm + // confirm ConfirmUserPasscode(router) if err != nil { @@ -288,7 +288,7 @@ func TestUserPasscode(t *testing.T) { assert.Empty(t, activatedAt) assert.NotEmpty(t, verifiedAt) - //activate + // activate ActivateUserPasscode(router) r = AuthenticatedRequestWithBody(app, "POST", "/api/v1/users/uqxetse3cy5eo9z2/passcode/activate", string(pcStr), sessId) @@ -300,7 +300,7 @@ func TestUserPasscode(t *testing.T) { assert.NotEmpty(t, activatedAt) assert.NotEmpty(t, verifiedAt) - //deactivate + // deactivate DeactivateUserPasscode(router) r = AuthenticatedRequestWithBody(app, "POST", "/api/v1/users/uqxetse3cy5eo9z2/passcode/deactivate", string(pcStr), sessId) diff --git a/internal/api/users_upload.go b/internal/api/users_upload.go index 0c1ffb153..7618df6bd 100644 --- a/internal/api/users_upload.go +++ b/internal/api/users_upload.go @@ -117,13 +117,14 @@ func UploadUserFiles(router *gin.RouterGroup) { fileType := fs.FileType(baseName) // Reject unsupported files and files with extensions that aren't allowed. - if fileType == fs.TypeUnknown { + switch { + case fileType == fs.TypeUnknown: log.Errorf("upload: rejected %s because it has an unsupported file extension", clean.Log(baseName)) continue - } else if allowedExt.Excludes(fileType.DefaultExt()) { + case allowedExt.Excludes(fileType.DefaultExt()): log.Errorf("upload: rejected %s because its extension is not allowed", clean.Log(baseName)) continue - } else if fileSizeLimit > 0 && file.Size > fileSizeLimit { + case fileSizeLimit > 0 && file.Size > fileSizeLimit: log.Errorf("upload: rejected %s because its size exceeds the file size limit", clean.Log(baseName)) continue } @@ -203,13 +204,14 @@ func UploadUserFiles(router *gin.RouterGroup) { for _, filename := range uploads { labels, nsfwErr := vision.DetectNSFW([]string{filename}, media.SrcLocal) - if nsfwErr != nil { + switch { + case nsfwErr != nil: log.Debug(nsfwErr) continue - } else if len(labels) < 1 { + case len(labels) < 1: log.Errorf("nsfw: model returned no result") continue - } else if labels[0].IsSafe() { + case labels[0].IsSafe(): continue } diff --git a/internal/api/users_upload_multipart_test.go b/internal/api/users_upload_multipart_test.go index e3a36c337..541d87615 100644 --- a/internal/api/users_upload_multipart_test.go +++ b/internal/api/users_upload_multipart_test.go @@ -74,18 +74,6 @@ func buildZipWithDirsAndFiles(dirs []string, files map[string][]byte) []byte { return zbuf.Bytes() } -func findUploadedFiles(t *testing.T, base string) []string { - t.Helper() - var out []string - _ = filepath.Walk(base, func(path string, info os.FileInfo, err error) error { - if err == nil && !info.IsDir() { - out = append(out, path) - } - return nil - }) - return out -} - // findUploadedFilesForToken lists files only under upload subfolders whose name ends with token suffix. func findUploadedFilesForToken(t *testing.T, base string, tokenSuffix string) []string { t.Helper() diff --git a/internal/api/video.go b/internal/api/video.go index bf1570db2..187726e74 100644 --- a/internal/api/video.go +++ b/internal/api/video.go @@ -94,10 +94,10 @@ func GetVideo(router *gin.RouterGroup) { // Get video bitrate, codec, and file type. videoFileType := f.Type() - videoCodec := f.FileCodec videoBitrate := f.Bitrate() videoFileName := photoprism.FileName(f.FileRoot, f.FileName) videoContentType := f.ContentType() + var videoCodec string // If the file has a hybrid photo/video format, try to find and send the embedded video data. if f.MediaType == entity.MediaLive { @@ -201,7 +201,5 @@ func GetVideo(router *gin.RouterGroup) { } else { c.File(videoFileName) } - - return }) } diff --git a/internal/api/vision_caption.go b/internal/api/vision_caption.go index e2db1e954..c8ed0f7fd 100644 --- a/internal/api/vision_caption.go +++ b/internal/api/vision_caption.go @@ -55,15 +55,16 @@ func PostVisionCaption(router *gin.RouterGroup) { // Run inference to generate a caption. result, model, err := vision.GenerateCaption(request.Images, media.SrcRemote) - if err != nil { + switch { + case err != nil: log.Errorf("vision: %s (caption)", err) c.JSON(http.StatusBadRequest, vision.NewApiError(request.GetId(), http.StatusBadRequest)) return - } else if model == nil { + case model == nil: log.Errorf("vision: no model specified (caption)") c.JSON(http.StatusInternalServerError, vision.NewApiError(request.GetId(), http.StatusInternalServerError)) return - } else if result == nil { + case result == nil: log.Errorf("vision: no result (caption)") c.JSON(http.StatusInternalServerError, vision.NewApiError(request.GetId(), http.StatusInternalServerError)) return diff --git a/internal/api/vision_face_test.go b/internal/api/vision_face_test.go index 3a3f2f396..f6114718a 100644 --- a/internal/api/vision_face_test.go +++ b/internal/api/vision_face_test.go @@ -166,10 +166,6 @@ func TestPostVisionFace(t *testing.T) { t.Fatal(apiErr) } - if apiResponse == nil { - t.Fatal("api response expected") - } - // t.Logf("error: %s", apiResponse.Err()) assert.Error(t, apiResponse.Err()) diff --git a/internal/api/vision_labels_test.go b/internal/api/vision_labels_test.go index 999ee5c8d..5d403ce59 100644 --- a/internal/api/vision_labels_test.go +++ b/internal/api/vision_labels_test.go @@ -117,10 +117,6 @@ func TestPostVisionLabels(t *testing.T) { t.Fatal(apiErr) } - if apiResponse == nil { - t.Fatal("api response expected") - } - t.Logf("error: %s", apiResponse.Err()) assert.Error(t, apiResponse.Err()) diff --git a/internal/api/vision_nsfw_test.go b/internal/api/vision_nsfw_test.go index a5a6972d9..fd46a8118 100644 --- a/internal/api/vision_nsfw_test.go +++ b/internal/api/vision_nsfw_test.go @@ -132,10 +132,6 @@ func TestPostVisionNsfw(t *testing.T) { t.Fatal(apiErr) } - if apiResponse == nil { - t.Fatal("api response expected") - } - // t.Logf("error: %s", apiResponse.Err()) assert.Error(t, apiResponse.Err()) diff --git a/internal/api/zip.go b/internal/api/zip.go index 664ffbbce..4c6e3685e 100644 --- a/internal/api/zip.go +++ b/internal/api/zip.go @@ -96,6 +96,7 @@ func ZipCreate(router *gin.RouterGroup) { // Create new zip file. var newZipFile *os.File + // #nosec G304 zip name derived from request if newZipFile, err = os.Create(zipFileName); err != nil { Error(c, http.StatusInternalServerError, err, i18n.ErrZipFailed) return @@ -124,7 +125,7 @@ func ZipCreate(router *gin.RouterGroup) { alias = file.DownloadName(dlName, seq) } - aliases[key] += 1 + aliases[key]++ if fs.FileExists(fileName) { if zipErr := fs.ZipFile(zipWriter, fileName, alias, false); zipErr != nil {