From 36c2004bf67c0572b6cd48680f1079c7e8b45c2d Mon Sep 17 00:00:00 2001 From: Stavros Date: Sun, 29 Mar 2026 20:20:53 +0300 Subject: [PATCH] chore: more review comments --- internal/controller/oidc_controller_test.go | 71 +++++++++---------- internal/controller/proxy_controller_test.go | 26 +++---- .../controller/resources_controller_test.go | 27 +++---- internal/controller/user_controller_test.go | 26 +++---- .../controller/well_known_controller_test.go | 24 ++++--- internal/service/auth_service.go | 1 + 6 files changed, 87 insertions(+), 88 deletions(-) diff --git a/internal/controller/oidc_controller_test.go b/internal/controller/oidc_controller_test.go index a00458b..c3943f7 100644 --- a/internal/controller/oidc_controller_test.go +++ b/internal/controller/oidc_controller_test.go @@ -4,20 +4,24 @@ import ( "encoding/json" "net/http/httptest" "net/url" - "os" + "path" "strings" "testing" "github.com/gin-gonic/gin" + "github.com/google/go-querystring/query" "github.com/steveiliop56/tinyauth/internal/bootstrap" "github.com/steveiliop56/tinyauth/internal/config" "github.com/steveiliop56/tinyauth/internal/controller" "github.com/steveiliop56/tinyauth/internal/repository" "github.com/steveiliop56/tinyauth/internal/service" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestOIDCController(t *testing.T) { + tempDir := t.TempDir() + oidcServiceCfg := service.OIDCServiceConfig{ Clients: map[string]config.OIDCClientConfig{ "test": { @@ -27,8 +31,8 @@ func TestOIDCController(t *testing.T) { Name: "Test Client", }, }, - PrivateKeyPath: "/tmp/tinyauth_testing_key.pem", - PublicKeyPath: "/tmp/tinyauth_testing_key.pub", + PrivateKeyPath: path.Join(tempDir, "key.pem"), + PublicKeyPath: path.Join(tempDir, "key.pub"), Issuer: "https://tinyauth.example.com", SessionExpiry: 500, } @@ -170,11 +174,11 @@ func TestOIDCController(t *testing.T) { Code: "", RedirectURI: "https://test.example.com/callback", } - reqBodyBytes, err := json.Marshal(reqBody) + reqBodyEncoded, err := query.Values(reqBody) assert.NoError(t, err) - req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(string(reqBodyBytes))) - req.Header.Set("Content-Type", "application/json") + req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") router.ServeHTTP(recorder, req) var res map[string]any @@ -193,11 +197,11 @@ func TestOIDCController(t *testing.T) { Code: "some-code", RedirectURI: "https://test.example.com/callback", } - reqBodyBytes, err := json.Marshal(reqBody) + reqBodyEncoded, err := query.Values(reqBody) assert.NoError(t, err) - req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(string(reqBodyBytes))) - req.Header.Set("Content-Type", "application/json") + req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.SetBasicAuth("some-client-id", "some-client-secret") router.ServeHTTP(recorder, req) @@ -231,11 +235,11 @@ func TestOIDCController(t *testing.T) { Code: "some-code", RedirectURI: "https://test.example.com/callback", } - reqBodyBytes, err := json.Marshal(reqBody) + reqBodyEncoded, err := query.Values(reqBody) assert.NoError(t, err) - req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(string(reqBodyBytes))) - req.Header.Set("Content-Type", "application/json") + req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") router.ServeHTTP(recorder, req) authHeader := recorder.Header().Get("www-authenticate") @@ -270,11 +274,11 @@ func TestOIDCController(t *testing.T) { Code: code, RedirectURI: "https://test.example.com/callback", } - reqBodyBytes, err := json.Marshal(reqBody) + reqBodyEncoded, err := query.Values(reqBody) assert.NoError(t, err) - req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(string(reqBodyBytes))) - req.Header.Set("Content-Type", "application/json") + req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.SetBasicAuth("some-client-id", "some-client-secret") router.ServeHTTP(recorder, req) @@ -307,11 +311,11 @@ func TestOIDCController(t *testing.T) { ClientID: "some-client-id", ClientSecret: "some-client-secret", } - reqBodyBytes, err := json.Marshal(reqBody) + reqBodyEncoded, err := query.Values(reqBody) assert.NoError(t, err) - req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(string(reqBodyBytes))) - req.Header.Set("Content-Type", "application/json") + req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") router.ServeHTTP(recorder, req) assert.NotEmpty(t, recorder.Header().Get("cache-control")) @@ -356,19 +360,19 @@ func TestOIDCController(t *testing.T) { Code: code, RedirectURI: "https://test.example.com/callback", } - reqBodyBytes, err := json.Marshal(reqBody) + reqBodyEncoded, err := query.Values(reqBody) assert.NoError(t, err) - req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(string(reqBodyBytes))) - req.Header.Set("Content-Type", "application/json") + req := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.SetBasicAuth("some-client-id", "some-client-secret") router.ServeHTTP(recorder, req) assert.Equal(t, 200, recorder.Code) // Try to use the same code again - secondReq := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(string(reqBodyBytes))) - secondReq.Header.Set("Content-Type", "application/json") + secondReq := httptest.NewRequest("POST", "/api/oidc/token", strings.NewReader(reqBodyEncoded.Encode())) + secondReq.Header.Set("Content-Type", "application/x-www-form-urlencoded") secondReq.SetBasicAuth("some-client-id", "some-client-secret") secondRecorder := httptest.NewRecorder() router.ServeHTTP(secondRecorder, secondReq) @@ -431,13 +435,13 @@ func TestOIDCController(t *testing.T) { app := bootstrap.NewBootstrapApp(config.Config{}) - db, err := app.SetupDatabase("/tmp/tinyauth_test.db") - assert.NoError(t, err) + db, err := app.SetupDatabase(path.Join(tempDir, "tinyauth.db")) + require.NoError(t, err) queries := repository.New(db) oidcService := service.NewOIDCService(oidcServiceCfg, queries) err = oidcService.Init() - assert.NoError(t, err) + require.NoError(t, err) for _, test := range tests { t.Run(test.description, func(t *testing.T) { @@ -459,15 +463,8 @@ func TestOIDCController(t *testing.T) { }) } - err = db.Close() - assert.NoError(t, err) - - err = os.Remove("/tmp/tinyauth_test.db") - assert.NoError(t, err) - - err = os.Remove(oidcServiceCfg.PrivateKeyPath) - assert.NoError(t, err) - - err = os.Remove(oidcServiceCfg.PublicKeyPath) - assert.NoError(t, err) + t.Cleanup(func() { + err = db.Close() + require.NoError(t, err) + }) } diff --git a/internal/controller/proxy_controller_test.go b/internal/controller/proxy_controller_test.go index 70e5eb6..049b657 100644 --- a/internal/controller/proxy_controller_test.go +++ b/internal/controller/proxy_controller_test.go @@ -2,7 +2,7 @@ package controller_test import ( "net/http/httptest" - "os" + "path" "testing" "github.com/gin-gonic/gin" @@ -13,9 +13,12 @@ import ( "github.com/steveiliop56/tinyauth/internal/service" "github.com/steveiliop56/tinyauth/internal/utils/tlog" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestProxyController(t *testing.T) { + tempDir := t.TempDir() + authServiceCfg := service.AuthServiceConfig{ Users: []config.User{ { @@ -320,26 +323,26 @@ func TestProxyController(t *testing.T) { app := bootstrap.NewBootstrapApp(config.Config{}) - db, err := app.SetupDatabase("/tmp/tinyauth_test.db") - assert.NoError(t, err) + db, err := app.SetupDatabase(path.Join(tempDir, "tinyauth.db")) + require.NoError(t, err) queries := repository.New(db) docker := service.NewDockerService() err = docker.Init() - assert.NoError(t, err) + require.NoError(t, err) ldap := service.NewLdapService(service.LdapServiceConfig{}) err = ldap.Init() - assert.NoError(t, err) + require.NoError(t, err) broker := service.NewOAuthBrokerService(oauthBrokerCfgs) err = broker.Init() - assert.NoError(t, err) + require.NoError(t, err) authService := service.NewAuthService(authServiceCfg, docker, ldap, queries, broker) err = authService.Init() - assert.NoError(t, err) + require.NoError(t, err) aclsService := service.NewAccessControlsService(docker, acls) @@ -363,9 +366,8 @@ func TestProxyController(t *testing.T) { }) } - err = db.Close() - assert.NoError(t, err) - - err = os.Remove("/tmp/tinyauth_test.db") - assert.NoError(t, err) + t.Cleanup(func() { + err = db.Close() + require.NoError(t, err) + }) } diff --git a/internal/controller/resources_controller_test.go b/internal/controller/resources_controller_test.go index 7467fc6..2376aa9 100644 --- a/internal/controller/resources_controller_test.go +++ b/internal/controller/resources_controller_test.go @@ -3,19 +3,26 @@ package controller_test import ( "net/http/httptest" "os" + "path" "testing" "github.com/gin-gonic/gin" "github.com/steveiliop56/tinyauth/internal/controller" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestResourcesController(t *testing.T) { + tempDir := t.TempDir() + resourcesControllerCfg := controller.ResourcesControllerConfig{ - Path: "/tmp/testfiles", + Path: path.Join(tempDir, "resources"), Enabled: true, } + err := os.Mkdir(resourcesControllerCfg.Path, 0777) + require.NoError(t, err) + type testCase struct { description string run func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) @@ -52,16 +59,13 @@ func TestResourcesController(t *testing.T) { }, } - err := os.MkdirAll(resourcesControllerCfg.Path, 0777) - assert.NoError(t, err) - testFilePath := resourcesControllerCfg.Path + "/testfile.txt" err = os.WriteFile(testFilePath, []byte("This is a test file."), 0777) - assert.NoError(t, err) + require.NoError(t, err) - testFilePathParent := resourcesControllerCfg.Path + "/../somefile.txt" + testFilePathParent := tempDir + "/somefile.txt" err = os.WriteFile(testFilePathParent, []byte("This file should not be accessible."), 0777) - assert.NoError(t, err) + require.NoError(t, err) for _, test := range tests { t.Run(test.description, func(t *testing.T) { @@ -76,13 +80,4 @@ func TestResourcesController(t *testing.T) { test.run(t, router, recorder) }) } - - err = os.Remove(testFilePath) - assert.NoError(t, err) - - err = os.Remove(testFilePathParent) - assert.NoError(t, err) - - err = os.Remove(resourcesControllerCfg.Path) - assert.NoError(t, err) } diff --git a/internal/controller/user_controller_test.go b/internal/controller/user_controller_test.go index 697364f..e6bceec 100644 --- a/internal/controller/user_controller_test.go +++ b/internal/controller/user_controller_test.go @@ -3,7 +3,7 @@ package controller_test import ( "encoding/json" "net/http/httptest" - "os" + "path" "slices" "strings" "testing" @@ -18,9 +18,12 @@ import ( "github.com/steveiliop56/tinyauth/internal/service" "github.com/steveiliop56/tinyauth/internal/utils/tlog" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestUserController(t *testing.T) { + tempDir := t.TempDir() + authServiceCfg := service.AuthServiceConfig{ Users: []config.User{ { @@ -277,26 +280,26 @@ func TestUserController(t *testing.T) { app := bootstrap.NewBootstrapApp(config.Config{}) - db, err := app.SetupDatabase("/tmp/tinyauth_test.db") - assert.NoError(t, err) + db, err := app.SetupDatabase(path.Join(tempDir, "tinyauth.db")) + require.NoError(t, err) queries := repository.New(db) docker := service.NewDockerService() err = docker.Init() - assert.NoError(t, err) + require.NoError(t, err) ldap := service.NewLdapService(service.LdapServiceConfig{}) err = ldap.Init() - assert.NoError(t, err) + require.NoError(t, err) broker := service.NewOAuthBrokerService(oauthBrokerCfgs) err = broker.Init() - assert.NoError(t, err) + require.NoError(t, err) authService := service.NewAuthService(authServiceCfg, docker, ldap, queries, broker) err = authService.Init() - assert.NoError(t, err) + require.NoError(t, err) beforeEach := func() { // Clear failed login attempts before each test @@ -346,9 +349,8 @@ func TestUserController(t *testing.T) { }) } - err = db.Close() - assert.NoError(t, err) - - err = os.Remove("/tmp/tinyauth_test.db") - assert.NoError(t, err) + t.Cleanup(func() { + err = db.Close() + require.NoError(t, err) + }) } diff --git a/internal/controller/well_known_controller_test.go b/internal/controller/well_known_controller_test.go index 993e398..20dc2a1 100644 --- a/internal/controller/well_known_controller_test.go +++ b/internal/controller/well_known_controller_test.go @@ -4,7 +4,7 @@ import ( "encoding/json" "fmt" "net/http/httptest" - "os" + "path" "testing" "github.com/gin-gonic/gin" @@ -14,9 +14,12 @@ import ( "github.com/steveiliop56/tinyauth/internal/repository" "github.com/steveiliop56/tinyauth/internal/service" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestWellKnownController(t *testing.T) { + tempDir := t.TempDir() + oidcServiceCfg := service.OIDCServiceConfig{ Clients: map[string]config.OIDCClientConfig{ "test": { @@ -26,8 +29,8 @@ func TestWellKnownController(t *testing.T) { Name: "Test Client", }, }, - PrivateKeyPath: "/tmp/tinyauth_testing_key.pem", - PublicKeyPath: "/tmp/tinyauth_testing_key.pub", + PrivateKeyPath: path.Join(tempDir, "key.pem"), + PublicKeyPath: path.Join(tempDir, "key.pub"), Issuer: "https://tinyauth.example.com", SessionExpiry: 500, } @@ -96,14 +99,14 @@ func TestWellKnownController(t *testing.T) { app := bootstrap.NewBootstrapApp(config.Config{}) - db, err := app.SetupDatabase("/tmp/tinyauth_test.db") - assert.NoError(t, err) + db, err := app.SetupDatabase(path.Join(tempDir, "tinyauth.db")) + require.NoError(t, err) queries := repository.New(db) oidcService := service.NewOIDCService(oidcServiceCfg, queries) err = oidcService.Init() - assert.NoError(t, err) + require.NoError(t, err) for _, test := range tests { t.Run(test.description, func(t *testing.T) { @@ -119,9 +122,8 @@ func TestWellKnownController(t *testing.T) { }) } - err = db.Close() - assert.NoError(t, err) - - err = os.Remove("/tmp/tinyauth_test.db") - assert.NoError(t, err) + t.Cleanup(func() { + err = db.Close() + require.NoError(t, err) + }) } diff --git a/internal/service/auth_service.go b/internal/service/auth_service.go index 1af63d1..9733336 100644 --- a/internal/service/auth_service.go +++ b/internal/service/auth_service.go @@ -801,5 +801,6 @@ func (auth *AuthService) lockdownMode() { func (auth *AuthService) ClearRateLimitsTestingOnly() { auth.loginMutex.Lock() auth.loginAttempts = make(map[string]*LoginAttempt) + auth.lockdown = nil auth.loginMutex.Unlock() }