From 02970068764755e0c570c0da0053e32031991821 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Wed, 12 Oct 2022 18:11:20 +0200 Subject: [PATCH] API: Refactor authentication tests to use conf.SetAuthMode() #98 Signed-off-by: Michael Mayer --- internal/api/auth_change_password_test.go | 29 +++---- internal/api/auth_session_create.go | 16 ++-- internal/api/auth_session_get.go | 11 +-- internal/api/auth_session_test.go | 97 ++++++++++++++++++----- internal/api/auth_test.go | 12 +-- internal/config/client_config.go | 4 + internal/config/client_config_test.go | 8 +- internal/config/config_auth.go | 17 +++- internal/config/config_auth_test.go | 9 +++ 9 files changed, 137 insertions(+), 66 deletions(-) diff --git a/internal/api/auth_change_password_test.go b/internal/api/auth_change_password_test.go index 881986424..503dc43b9 100644 --- a/internal/api/auth_change_password_test.go +++ b/internal/api/auth_change_password_test.go @@ -5,9 +5,10 @@ import ( "net/http" "testing" - "github.com/photoprism/photoprism/internal/form" - "github.com/stretchr/testify/assert" + + "github.com/photoprism/photoprism/internal/config" + "github.com/photoprism/photoprism/internal/form" ) func TestChangePassword(t *testing.T) { @@ -20,8 +21,8 @@ func TestChangePassword(t *testing.T) { t.Run("AliceProvidesWrongPassword", func(t *testing.T) { app, router, conf := NewApiTest() - conf.SetPublic(false) - defer conf.SetPublic(true) + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) ChangePassword(router) sessId := AuthenticateUser(app, router, "alice", "Alice123!") @@ -40,8 +41,8 @@ func TestChangePassword(t *testing.T) { t.Run("Success", func(t *testing.T) { app, router, conf := NewApiTest() - conf.SetPublic(false) - defer conf.SetPublic(true) + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) ChangePassword(router) oldPassword := "PleaseChange$42" @@ -78,8 +79,8 @@ func TestChangePassword(t *testing.T) { t.Run("AliceChangesOtherUsersPassword", func(t *testing.T) { app, router, conf := NewApiTest() - conf.SetPublic(false) - defer conf.SetPublic(true) + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) ChangePassword(router) sessId := AuthenticateUser(app, router, "alice", "Alice123!") @@ -98,8 +99,8 @@ func TestChangePassword(t *testing.T) { t.Run("BobProvidesWrongPassword", func(t *testing.T) { app, router, conf := NewApiTest() - conf.SetPublic(false) - defer conf.SetPublic(true) + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) ChangePassword(router) sessId := AuthenticateUser(app, router, "bob", "Bobbob123!") @@ -118,8 +119,8 @@ func TestChangePassword(t *testing.T) { t.Run("SameNewPassword", func(t *testing.T) { app, router, conf := NewApiTest() - conf.SetPublic(false) - defer conf.SetPublic(true) + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) ChangePassword(router) sessId := AuthenticateUser(app, router, "friend", "!Friend321") @@ -138,8 +139,8 @@ func TestChangePassword(t *testing.T) { t.Run("BobChangesOtherUsersPassword", func(t *testing.T) { app, router, conf := NewApiTest() - conf.SetPublic(false) - defer conf.SetPublic(true) + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) ChangePassword(router) sessId := AuthenticateUser(app, router, "bob", "Bobbob123!") diff --git a/internal/api/auth_session_create.go b/internal/api/auth_session_create.go index c78a5f478..adc23a485 100644 --- a/internal/api/auth_session_create.go +++ b/internal/api/auth_session_create.go @@ -19,6 +19,14 @@ import ( // POST /api/v1/session func CreateSession(router *gin.RouterGroup) { router.POST("/session", func(c *gin.Context) { + var f form.Login + + if err := c.BindJSON(&f); err != nil { + event.AuditWarn([]string{ClientIP(c), "create session", "invalid request", "%s"}, err) + AbortBadRequest(c) + return + } + conf := service.Config() // Skip authentication if app is running in public mode. @@ -34,14 +42,6 @@ func CreateSession(router *gin.RouterGroup) { return } - var f form.Login - - if err := c.BindJSON(&f); err != nil { - event.AuditWarn([]string{ClientIP(c), "create session", "invalid request", "%s"}, err) - AbortBadRequest(c) - return - } - var sess *entity.Session var isNew bool diff --git a/internal/api/auth_session_get.go b/internal/api/auth_session_get.go index a0fda69a5..f544daa62 100644 --- a/internal/api/auth_session_get.go +++ b/internal/api/auth_session_get.go @@ -15,15 +15,6 @@ import ( // GET /api/v1/session/:id func GetSession(router *gin.RouterGroup) { router.GET("/session/:id", func(c *gin.Context) { - conf := service.Config() - - // Skip authentication if app is running in public mode. - if conf.Public() { - sess := service.Session().Public() - c.JSON(http.StatusOK, gin.H{"status": "ok", "id": sess.ID, "user": sess.User(), "data": sess.Data(), "config": conf.ClientPublic()}) - return - } - id := clean.ID(c.Param("id")) if id == "" { @@ -56,7 +47,7 @@ func GetSession(router *gin.RouterGroup) { var clientConfig config.ClientConfig - if conf == nil { + if conf := service.Config(); conf == nil { log.Errorf("session: config is not set - possible bug") AbortUnexpected(c) return diff --git a/internal/api/auth_session_test.go b/internal/api/auth_session_test.go index e0b154193..d2797074e 100644 --- a/internal/api/auth_session_test.go +++ b/internal/api/auth_session_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/tidwall/gjson" + "github.com/photoprism/photoprism/internal/config" + "github.com/photoprism/photoprism/internal/form" "github.com/photoprism/photoprism/internal/i18n" "github.com/photoprism/photoprism/internal/session" ) @@ -27,7 +29,10 @@ func TestSession(t *testing.T) { func TestCreateSession(t *testing.T) { t.Run("Success", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + CreateSession(router) r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"name": "admin", "password": "photoprism"}`) log.Debugf("BODY: %s", r.Body.String()) @@ -36,32 +41,47 @@ func TestCreateSession(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) }) t.Run("BadRequest", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + CreateSession(router) r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"name": 123, "password": "xxx"}`) assert.Equal(t, http.StatusBadRequest, r.Code) }) t.Run("PublicInvalidToken", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + CreateSession(router) r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"name": "admin", "password": "photoprism", "token": "xxx"}`) assert.Equal(t, http.StatusNotFound, r.Code) }) t.Run("AdminInvalidToken", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + // CreateSession(router) sessId := AuthenticateUser(app, router, "alice", "Alice123!") r := AuthenticatedRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"token": "xxx"}`, sessId) assert.Equal(t, http.StatusNotFound, r.Code) }) t.Run("VisitorInvalidToken", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + CreateSession(router) r := AuthenticatedRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"token": "xxx"}`, "345346") assert.Equal(t, http.StatusNotFound, r.Code) }) t.Run("AdminValidToken", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + sessId := AuthenticateUser(app, router, "alice", "Alice123!") r := AuthenticatedRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"token": "1jxf3jfn2k"}`, sessId) assert.Equal(t, http.StatusOK, r.Code) @@ -73,16 +93,28 @@ func TestCreateSession(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) }) t.Run("AdminInvalidPassword", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + CreateSession(router) - r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"name": "admin", "password": "xxx"}`) + + r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", form.AsJson(form.Login{ + UserName: "admin", + Password: "xxx", + })) + val := gjson.Get(r.Body.String(), "error") assert.Equal(t, i18n.Msg(i18n.ErrInvalidCredentials), val.String()) assert.Equal(t, http.StatusUnauthorized, r.Code) }) t.Run("AliceSuccess", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + CreateSession(router) + r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"name": "alice", "password": "Alice123!"}`) userEmail := gjson.Get(r.Body.String(), "user.Email") userName := gjson.Get(r.Body.String(), "user.Name") @@ -91,7 +123,10 @@ func TestCreateSession(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) }) t.Run("BobSuccess", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + CreateSession(router) r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"name": "bob", "password": "Bobbob123!"}`) userEmail := gjson.Get(r.Body.String(), "user.Email") @@ -101,7 +136,10 @@ func TestCreateSession(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) }) t.Run("BobInvalidPassword", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + CreateSession(router) r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", `{"name": "bob", "password": "helloworld"}`) val := gjson.Get(r.Body.String(), "error") @@ -112,7 +150,10 @@ func TestCreateSession(t *testing.T) { func TestGetSession(t *testing.T) { t.Run("AdminWithoutAuthentication", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + GetSession(router) sessId := AuthenticateAdmin(app, router) @@ -120,7 +161,10 @@ func TestGetSession(t *testing.T) { assert.Equal(t, http.StatusForbidden, r.Code) }) t.Run("AdminAuthenticatedRequest", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + GetSession(router) sessId := AuthenticateAdmin(app, router) @@ -131,7 +175,10 @@ func TestGetSession(t *testing.T) { func TestDeleteSession(t *testing.T) { t.Run("AdminWithoutAuthentication", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + DeleteSession(router) sessId := AuthenticateAdmin(app, router) @@ -140,7 +187,10 @@ func TestDeleteSession(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) }) t.Run("AdminAuthenticatedRequest", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + DeleteSession(router) sessId := AuthenticateAdmin(app, router) @@ -149,7 +199,10 @@ func TestDeleteSession(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) }) t.Run("UserWithoutAuthentication", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + DeleteSession(router) sessId := AuthenticateUser(app, router, "alice", "Alice123!") @@ -158,7 +211,10 @@ func TestDeleteSession(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) }) t.Run("UserAuthenticatedRequest", func(t *testing.T) { - app, router, _ := NewApiTest() + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + DeleteSession(router) sessId := AuthenticateUser(app, router, "alice", "Alice123!") @@ -167,9 +223,14 @@ func TestDeleteSession(t *testing.T) { assert.Equal(t, http.StatusOK, r.Code) }) t.Run("InvalidSession", func(t *testing.T) { + app, router, conf := NewApiTest() + conf.SetAuthMode(config.AuthModePasswd) + defer conf.SetAuthMode(config.AuthModePublic) + sessId := "638bffc9b86a8fda0d908ebee84a43930cb8d1e3507f4aa0" - app, router, _ := NewApiTest() + DeleteSession(router) + r := PerformRequest(app, http.MethodDelete, "/api/v1/session/"+sessId) assert.Equal(t, http.StatusOK, r.Code) }) diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 56a960f4c..406b5877a 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -1,7 +1,6 @@ package api import ( - "encoding/json" "net/http" "net/http/httptest" "strings" @@ -23,18 +22,11 @@ func AuthenticateAdmin(app *gin.Engine, router *gin.RouterGroup) (sessId string) func AuthenticateUser(app *gin.Engine, router *gin.RouterGroup, name string, password string) (sessId string) { CreateSession(router) - f := form.Login{ + r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", form.AsJson(form.Login{ UserName: name, Password: password, - } + })) - loginStr, err := json.Marshal(f) - - if err != nil { - log.Fatal(err) - } - - r := PerformRequestWithBody(app, http.MethodPost, "/api/v1/session", string(loginStr)) sessId = r.Header().Get(session.Header) return diff --git a/internal/config/client_config.go b/internal/config/client_config.go index 7f4b0a583..67d13180b 100644 --- a/internal/config/client_config.go +++ b/internal/config/client_config.go @@ -57,6 +57,7 @@ type ClientConfig struct { ReadOnly bool `json:"readonly"` UploadNSFW bool `json:"uploadNSFW"` Public bool `json:"public"` + AuthMode string `json:"authMode"` Experimental bool `json:"experimental"` AlbumCategories []string `json:"albumCategories"` Albums entity.Albums `json:"albums"` @@ -256,6 +257,7 @@ func (c *Config) ClientPublic() ClientConfig { Sponsor: c.Sponsor(), ReadOnly: c.ReadOnly(), Public: c.Public(), + AuthMode: c.AuthMode(), Experimental: c.Experimental(), Albums: entity.Albums{}, Cameras: entity.Cameras{}, @@ -332,6 +334,7 @@ func (c *Config) ClientShare() ClientConfig { ReadOnly: c.ReadOnly(), UploadNSFW: c.UploadNSFW(), Public: c.Public(), + AuthMode: c.AuthMode(), Experimental: c.Experimental(), Albums: entity.Albums{}, Cameras: entity.Cameras{}, @@ -413,6 +416,7 @@ func (c *Config) ClientUser(withSettings bool) ClientConfig { ReadOnly: c.ReadOnly(), UploadNSFW: c.UploadNSFW(), Public: c.Public(), + AuthMode: c.AuthMode(), Experimental: c.Experimental(), Albums: entity.Albums{}, Cameras: entity.Cameras{}, diff --git a/internal/config/client_config_test.go b/internal/config/client_config_test.go index 19f4da86b..c1e9d3433 100644 --- a/internal/config/client_config_test.go +++ b/internal/config/client_config_test.go @@ -16,12 +16,14 @@ func TestConfig_ClientConfig(t *testing.T) { c := TestConfig() result := c.ClientPublic() assert.IsType(t, ClientConfig{}, result) + assert.Equal(t, AuthModePublic, result.AuthMode) assert.Equal(t, true, result.Public) }) t.Run("TestErrorConfig", func(t *testing.T) { c := NewTestErrorConfig() result2 := c.ClientPublic() assert.IsType(t, ClientConfig{}, result2) + assert.Equal(t, AuthModePasswd, result2.AuthMode) assert.Equal(t, false, result2.Public) }) t.Run("Values", func(t *testing.T) { @@ -45,6 +47,7 @@ func TestConfig_ClientConfig(t *testing.T) { assert.NotEmpty(t, cfg.Thumbs) assert.NotEmpty(t, cfg.ManifestUri) assert.Equal(t, true, cfg.Debug) + assert.Equal(t, AuthModePublic, cfg.AuthMode) assert.Equal(t, false, cfg.Demo) assert.Equal(t, true, cfg.Sponsor) assert.Equal(t, false, cfg.ReadOnly) @@ -69,6 +72,7 @@ func TestConfig_ClientShareConfig(t *testing.T) { result := config.ClientShare() assert.IsType(t, ClientConfig{}, result) assert.Equal(t, true, result.Public) + assert.Equal(t, AuthModePublic, result.AuthMode) assert.Equal(t, true, result.Experimental) assert.Equal(t, false, result.ReadOnly) } @@ -76,8 +80,7 @@ func TestConfig_ClientShareConfig(t *testing.T) { func TestConfig_ClientRoleConfig(t *testing.T) { c := NewTestConfig("config") - c.Options().AuthMode = AuthModePasswd - c.Options().Public = false + c.SetAuthMode(AuthModePasswd) assert.Equal(t, AuthModePasswd, c.AuthMode()) @@ -86,6 +89,7 @@ func TestConfig_ClientRoleConfig(t *testing.T) { t.Run("RoleAdmin", func(t *testing.T) { cfg := c.ClientRole(acl.RoleAdmin) assert.IsType(t, ClientConfig{}, cfg) + assert.Equal(t, AuthModePasswd, cfg.AuthMode) assert.Equal(t, false, cfg.Public) f := cfg.Settings.Features diff --git a/internal/config/config_auth.go b/internal/config/config_auth.go index 1cadb0393..e80ea0e9e 100644 --- a/internal/config/config_auth.go +++ b/internal/config/config_auth.go @@ -65,10 +65,19 @@ func (c *Config) Public() bool { return c.AuthMode() == AuthModePublic } -// SetPublic changes authentication while instance is running, for testing purposes only. -func (c *Config) SetPublic(enabled bool) { - if c.Debug() { - c.options.Public = enabled +// SetAuthMode changes the authentication mode (for use in tests only). +func (c *Config) SetAuthMode(mode string) { + if !c.Debug() { + return + } + + switch mode { + case AuthModePublic: + c.options.AuthMode = AuthModePublic + c.options.Public = true + default: + c.options.AuthMode = AuthModePasswd + c.options.Public = false } } diff --git a/internal/config/config_auth_test.go b/internal/config/config_auth_test.go index 88d14c49a..49bb98390 100644 --- a/internal/config/config_auth_test.go +++ b/internal/config/config_auth_test.go @@ -23,6 +23,15 @@ func TestAuthMode(t *testing.T) { assert.Equal(t, AuthModePasswd, c.AuthMode()) c.options.AuthMode = "password" assert.Equal(t, AuthModePasswd, c.AuthMode()) + c.options.Debug = false + c.SetAuthMode(AuthModePublic) + assert.Equal(t, AuthModePasswd, c.AuthMode()) + c.options.Debug = true + c.SetAuthMode(AuthModePublic) + assert.Equal(t, AuthModePublic, c.AuthMode()) + c.SetAuthMode(AuthModePasswd) + assert.Equal(t, AuthModePasswd, c.AuthMode()) + c.options.Debug = false } func TestAuth(t *testing.T) {