Skip to content

Commit 0d2d748

Browse files
committed
fix: trim and validate default admin password input
- Trim whitespace from the configured default admin password before use; treat whitespace-only or empty passwords as unset and generate a random password instead - Replace direct checks for non-empty password with checks on the trimmed value in the password-creation logic - Add a test to verify handling of whitespace in the default admin password, including cases with only spaces/tabs/mixed whitespace Signed-off-by: appleboy <appleboy.tw@gmail.com>
1 parent ba2662f commit 0d2d748

File tree

2 files changed

+88
-4
lines changed

2 files changed

+88
-4
lines changed

internal/store/sqlite.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"log"
9+
"strings"
910
"time"
1011

1112
"github.com/appleboy/authgate/internal/config"
@@ -75,9 +76,10 @@ func (s *Store) seedData(cfg *config.Config) error {
7576
var password string
7677
var err error
7778

78-
// Use configured password if set, otherwise generate random password
79-
if cfg.DefaultAdminPassword != "" {
80-
password = cfg.DefaultAdminPassword
79+
// Use configured password if set (after trimming whitespace), otherwise generate random password
80+
configuredPassword := strings.TrimSpace(cfg.DefaultAdminPassword)
81+
if configuredPassword != "" {
82+
password = configuredPassword
8183
} else {
8284
password, err = generateRandomPassword(16)
8385
if err != nil {
@@ -101,7 +103,7 @@ func (s *Store) seedData(cfg *config.Config) error {
101103
}
102104

103105
// Log password creation differently based on source
104-
if cfg.DefaultAdminPassword != "" {
106+
if configuredPassword != "" {
105107
log.Printf("Created default user: admin / [configured password] (role: admin)")
106108
} else {
107109
log.Printf("Created default user: admin / %s (role: admin)", password)

internal/store/store_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package store
33
import (
44
"context"
55
"fmt"
6+
"strings"
67
"testing"
78
"time"
89

@@ -15,6 +16,7 @@ import (
1516
"github.com/testcontainers/testcontainers-go"
1617
"github.com/testcontainers/testcontainers-go/modules/postgres"
1718
"github.com/testcontainers/testcontainers-go/wait"
19+
"golang.org/x/crypto/bcrypt"
1820
"gorm.io/gorm"
1921
)
2022

@@ -525,3 +527,83 @@ func TestUpsertExternalUser_Success_UpdateExisting(t *testing.T) {
525527
assert.Equal(t, "bob.builder@example.com", updatedUser.Email)
526528
assert.Equal(t, "Robert Builder", updatedUser.FullName)
527529
}
530+
531+
// TestDefaultAdminPassword_WhitespaceHandling tests that whitespace-only passwords are treated as empty
532+
func TestDefaultAdminPassword_WhitespaceHandling(t *testing.T) {
533+
tests := []struct {
534+
name string
535+
defaultAdminPassword string
536+
shouldUseConfigured bool
537+
}{
538+
{
539+
name: "valid password",
540+
defaultAdminPassword: "MyPassword123",
541+
shouldUseConfigured: true,
542+
},
543+
{
544+
name: "password with leading/trailing spaces",
545+
defaultAdminPassword: " MyPassword123 ",
546+
shouldUseConfigured: true,
547+
},
548+
{
549+
name: "empty string",
550+
defaultAdminPassword: "",
551+
shouldUseConfigured: false,
552+
},
553+
{
554+
name: "only spaces",
555+
defaultAdminPassword: " ",
556+
shouldUseConfigured: false,
557+
},
558+
{
559+
name: "only tabs",
560+
defaultAdminPassword: "\t\t\t",
561+
shouldUseConfigured: false,
562+
},
563+
{
564+
name: "mixed whitespace",
565+
defaultAdminPassword: " \t\n\r ",
566+
shouldUseConfigured: false,
567+
},
568+
}
569+
570+
for _, tt := range tests {
571+
t.Run(tt.name, func(t *testing.T) {
572+
cfg := &config.Config{
573+
DefaultAdminPassword: tt.defaultAdminPassword,
574+
}
575+
576+
store, err := New("sqlite", ":memory:", cfg)
577+
require.NoError(t, err)
578+
require.NotNil(t, store)
579+
580+
// Get the created admin user
581+
admin, err := store.GetUserByUsername("admin")
582+
require.NoError(t, err)
583+
require.NotNil(t, admin)
584+
585+
// Verify the password works
586+
if tt.shouldUseConfigured {
587+
// Should use the trimmed configured password
588+
err = bcrypt.CompareHashAndPassword(
589+
[]byte(admin.PasswordHash),
590+
[]byte(strings.TrimSpace(tt.defaultAdminPassword)),
591+
)
592+
assert.NoError(t, err, "configured password should work after trimming")
593+
} else {
594+
// Should have generated a random password (we can't verify the exact password,
595+
// but we can verify it's not an empty password)
596+
assert.NotEmpty(t, admin.PasswordHash)
597+
598+
// Verify that whitespace-only password does NOT work
599+
if tt.defaultAdminPassword != "" {
600+
err = bcrypt.CompareHashAndPassword(
601+
[]byte(admin.PasswordHash),
602+
[]byte(tt.defaultAdminPassword),
603+
)
604+
assert.Error(t, err, "whitespace-only password should not work")
605+
}
606+
}
607+
})
608+
}
609+
}

0 commit comments

Comments
 (0)