From d90182833e9f2479fb3aa32e798ea206bc86a0c5 Mon Sep 17 00:00:00 2001 From: Keith Martin Date: Fri, 7 Nov 2025 00:19:16 +1000 Subject: [PATCH 1/6] OIDC: Add handling to AuthID so that SQLite doesn't corrupt on save with long numbers --- internal/entity/auth_user.go | 48 ++++++++++++++++++++++++++- internal/entity/auth_user_test.go | 54 +++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/internal/entity/auth_user.go b/internal/entity/auth_user.go index 01f8e5c6d..062259ad5 100644 --- a/internal/entity/auth_user.go +++ b/internal/entity/auth_user.go @@ -52,7 +52,7 @@ type User struct { AuthProvider string `gorm:"type:VARBINARY(128);default:'';" json:"AuthProvider" yaml:"AuthProvider,omitempty"` AuthMethod string `gorm:"type:VARBINARY(128);default:'';" json:"AuthMethod" yaml:"AuthMethod,omitempty"` AuthIssuer string `gorm:"type:VARBINARY(255);default:'';" json:"AuthIssuer,omitempty" yaml:"AuthIssuer,omitempty"` - AuthID string `gorm:"type:VARBINARY(255);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` + AuthID string `gorm:"type:VARBINARY(264);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` UserName string `gorm:"size:200;index;" json:"Name" yaml:"Name,omitempty"` DisplayName string `gorm:"size:200;" json:"DisplayName" yaml:"DisplayName,omitempty"` UserEmail string `gorm:"size:255;index;" json:"Email" yaml:"Email,omitempty"` @@ -382,6 +382,20 @@ func (m *User) Updates(values interface{}) error { return UnscopedDb().Model(m).Updates(values).Error } +// Wraps the AuthID field so that SQLite will save it correctly +func (m *User) wrapAuthID() { + if m.AuthID != "" && !strings.HasPrefix(m.AuthID, "") && !strings.HasSuffix(m.AuthID, "") { + m.AuthID = fmt.Sprintf("%s", m.AuthID) + } +} + +// Unwraps the AuthID field so that PhotoPrism can use it correctly +func (m *User) unwrapAuthID() { + if m.AuthID != "" && strings.HasPrefix(m.AuthID, "") && strings.HasSuffix(m.AuthID, "") { + m.AuthID = strings.TrimSuffix(strings.TrimPrefix(m.AuthID, ""), "") + } +} + // BeforeCreate sets a random UID if needed before inserting a new row to the database. func (m *User) BeforeCreate(scope *gorm.Scope) error { if m.UserSettings != nil { @@ -403,10 +417,42 @@ func (m *User) BeforeCreate(scope *gorm.Scope) error { return nil } + m.wrapAuthID() + m.UserUID = rnd.GenerateUID(UserUID) return scope.SetColumn("UserUID", m.UserUID) } +// BeforeSave ensures that the AuthID will save correctly on SQLite +func (m *User) BeforeSave(scope *gorm.Scope) error { + m.wrapAuthID() + return nil +} + +// BeforeUpdate ensures that the AuthID will save correctly on SQLite +func (m *User) BeforeUpdate(scope *gorm.Scope) error { + m.wrapAuthID() + return nil +} + +// AfterSave ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite +func (m *User) AfterSave(scope *gorm.Scope) error { + m.unwrapAuthID() + return nil +} + +// AfterUpdate ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite +func (m *User) AfterUpdate(scope *gorm.Scope) error { + m.unwrapAuthID() + return nil +} + +// AfterFind ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite +func (m *User) AfterFind(scope *gorm.Scope) error { + m.unwrapAuthID() + return nil +} + // IsExpired checks if the user account has expired. func (m *User) IsExpired() bool { if m.ExpiresAt == nil { diff --git a/internal/entity/auth_user_test.go b/internal/entity/auth_user_test.go index 1d107aaa7..481bc6a49 100644 --- a/internal/entity/auth_user_test.go +++ b/internal/entity/auth_user_test.go @@ -70,6 +70,25 @@ func TestOidcUser(t *testing.T) { assert.Equal(t, "jane.doe", m.UserName) assert.Equal(t, "Jane Doe", m.DisplayName) }) + t.Run("LongNumberAsSubject", func(t *testing.T) { + info := &oidc.UserInfo{} + info.Name = "Jane Doe" + info.GivenName = "Jane" + info.FamilyName = "Doe" + info.Email = "jane@doe.com" + info.EmailVerified = true + info.Subject = "12345678901234567890" + info.PreferredUsername = "Jane Doe" + + m := OidcUser(info, "", "jane.doe") + + assert.Equal(t, "oidc", m.AuthProvider) + assert.Equal(t, "", m.AuthIssuer) + assert.Equal(t, "12345678901234567890", m.AuthID) + assert.Equal(t, "jane@doe.com", m.UserEmail) + assert.Equal(t, "jane.doe", m.UserName) + assert.Equal(t, "Jane Doe", m.DisplayName) + }) t.Run("NoUsername", func(t *testing.T) { info := &oidc.UserInfo{} info.Name = "Jane Doe" @@ -1808,6 +1827,7 @@ func TestUser_SetAuthID(t *testing.T) { func TestUser_UpdateAuthID(t *testing.T) { uuid := rnd.UUID() issuer := "http://dummy-oidc:9998" + longnumber := "12345678901234567890" t.Run("UUID", func(t *testing.T) { m := UserFixtures.Get("friend") @@ -1833,6 +1853,20 @@ func TestUser_UpdateAuthID(t *testing.T) { err := m.UpdateAuthID(uuid, "") assert.Error(t, err) }) + t.Run("LongNumber", func(t *testing.T) { + m := UserFixtures.Get("friend") + + m.SetAuthID("", issuer) + assert.Equal(t, "", m.AuthID) + assert.Equal(t, "", m.AuthIssuer) + m.SetAuthID(longnumber, issuer) + assert.Equal(t, longnumber, m.AuthID) + assert.Equal(t, issuer, m.AuthIssuer) + err := m.UpdateAuthID(longnumber, "") + assert.NoError(t, err) + assert.Equal(t, longnumber, m.AuthID) + assert.Equal(t, "", m.AuthIssuer) + }) } func TestUser_AuthInfo(t *testing.T) { @@ -2356,3 +2390,23 @@ func TestUser_SetValuesFromCliScope(t *testing.T) { require.NoError(t, user.SetValuesFromCli(ctx)) assert.Equal(t, "videos:view", user.UserScope) } + +func TestUser_AuthIDSQLite(t *testing.T) { + user := FindLocalUser("alice") + require.NotNil(t, user) + + original := user.AuthID + t.Cleanup(func() { + user.AuthID = original + user.Save() + }) + + expected := "012345678901234567890123456789" + user.AuthID = expected + user.Save() + + user2 := FindLocalUser("alice") + require.NotNil(t, user2) + + assert.Equal(t, expected, user2.AuthID) +} From babcb59d227e93d52f3e8414f04e9d76f6353ebe Mon Sep 17 00:00:00 2001 From: Keith Martin Date: Fri, 7 Nov 2025 23:52:49 +1000 Subject: [PATCH 2/6] Entity: Ensure that AuthID wrap/unwrap is used for auth_user and auth_sessions, and that auth_sessions wrap/unwrap on create/save/find as required --- internal/entity/auth_session.go | 54 ++++++++++++++- internal/entity/auth_session_delete.go | 2 +- internal/entity/auth_session_test.go | 95 ++++++++++++++++++++++---- internal/entity/auth_user.go | 20 +++--- internal/entity/auth_user_test.go | 66 ++++++++++++++++++ 5 files changed, 212 insertions(+), 25 deletions(-) diff --git a/internal/entity/auth_session.go b/internal/entity/auth_session.go index 47ce96c8c..fd61d3361 100644 --- a/internal/entity/auth_session.go +++ b/internal/entity/auth_session.go @@ -50,7 +50,7 @@ type Session struct { AuthProvider string `gorm:"type:VARBINARY(128);default:'';" json:"AuthProvider" yaml:"AuthProvider,omitempty"` AuthMethod string `gorm:"type:VARBINARY(128);default:'';" json:"AuthMethod" yaml:"AuthMethod,omitempty"` AuthIssuer string `gorm:"type:VARBINARY(255);default:'';" json:"AuthIssuer,omitempty" yaml:"AuthIssuer,omitempty"` - AuthID string `gorm:"type:VARBINARY(255);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` + AuthID string `gorm:"type:VARBINARY(264);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` // Make sure that you wrap and unwrap if using auth_id in a query. AuthScope string `gorm:"size:1024;default:'';" json:"AuthScope" yaml:"AuthScope,omitempty"` GrantType string `gorm:"type:VARBINARY(64);default:'';" json:"GrantType" yaml:"GrantType,omitempty"` LastActive int64 `json:"LastActive" yaml:"LastActive,omitempty"` @@ -276,6 +276,27 @@ func (m *Session) Updates(values interface{}) error { return UnscopedDb().Model(m).Updates(values).Error } +// Wraps a string value in pseudo XML to force type to string +func wrapString(s string) (r string) { + r = s + if s != "" && !strings.HasPrefix(s, "") && !strings.HasSuffix(s, "") { + r = fmt.Sprintf("%s", s) + } + return r +} + +// Wraps the AuthID field so that SQLite will save it correctly +func (m *Session) wrapAuthID() { + m.AuthID = wrapString(m.AuthID) +} + +// Unwraps the AuthID field so that PhotoPrism can use it correctly +func (m *Session) unwrapAuthID() { + if m.AuthID != "" && strings.HasPrefix(m.AuthID, "") && strings.HasSuffix(m.AuthID, "") { + m.AuthID = strings.TrimSuffix(strings.TrimPrefix(m.AuthID, ""), "") + } +} + // BeforeCreate creates a random UID if needed before inserting a new row to the database. func (m *Session) BeforeCreate(scope *gorm.Scope) error { if rnd.InvalidRefID(m.RefID) { @@ -283,6 +304,7 @@ func (m *Session) BeforeCreate(scope *gorm.Scope) error { Log("session", "set ref id", scope.SetColumn("RefID", m.RefID)) } + m.wrapAuthID() if rnd.IsSessionID(m.ID) { return nil } @@ -292,6 +314,36 @@ func (m *Session) BeforeCreate(scope *gorm.Scope) error { return scope.SetColumn("ID", m.ID) } +// BeforeSave ensures that the AuthID will save correctly on SQLite +func (m *Session) BeforeSave(scope *gorm.Scope) error { + m.wrapAuthID() + return nil +} + +// BeforeUpdate ensures that the AuthID will save correctly on SQLite +func (m *Session) BeforeUpdate(scope *gorm.Scope) error { + m.wrapAuthID() + return nil +} + +// AfterSave ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite +func (m *Session) AfterSave(scope *gorm.Scope) error { + m.unwrapAuthID() + return nil +} + +// AfterUpdate ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite +func (m *Session) AfterUpdate(scope *gorm.Scope) error { + m.unwrapAuthID() + return nil +} + +// AfterFind ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite +func (m *Session) AfterFind(scope *gorm.Scope) error { + m.unwrapAuthID() + return nil +} + // SetClient sets the client of this session. func (m *Session) SetClient(c *Client) *Session { if c == nil { diff --git a/internal/entity/auth_session_delete.go b/internal/entity/auth_session_delete.go index 79d55daba..0d82c7fec 100644 --- a/internal/entity/auth_session_delete.go +++ b/internal/entity/auth_session_delete.go @@ -49,7 +49,7 @@ func DeleteChildSessions(s *Session) (deleted int) { found := Sessions{} - if err := Db().Where("auth_id = ? AND auth_method = ?", s.ID, authn.MethodSession.String()).Find(&found).Error; err != nil { + if err := Db().Where("auth_id = ? AND auth_method = ?", wrapString(s.ID), authn.MethodSession.String()).Find(&found).Error; err != nil { event.AuditErr([]string{"failed to find child sessions", status.Error(err)}) return deleted } diff --git a/internal/entity/auth_session_test.go b/internal/entity/auth_session_test.go index 7bd5a2d72..3524e83e1 100644 --- a/internal/entity/auth_session_test.go +++ b/internal/entity/auth_session_test.go @@ -8,6 +8,7 @@ import ( "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/photoprism/photoprism/internal/auth/acl" "github.com/photoprism/photoprism/pkg/authn" @@ -238,10 +239,11 @@ func TestSession_Create(t *testing.T) { s.SetAuthToken("69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7xxx") err := s.Create() + require.Nil(t, err) - if err != nil { - t.Fatal(err) - } + t.Cleanup(func() { + s.Delete() + }) m2 := FindSessionByRefID("sessxkkcxxxx") assert.Equal(t, "charles", m2.UserName) @@ -264,18 +266,19 @@ func TestSession_Create(t *testing.T) { s.SetAuthToken(authToken) err := s.Create() + require.Nil(t, err) - if err != nil { - t.Fatal(err) - } + t.Cleanup(func() { + s.Delete() + }) m2, _ := FindSession(id) assert.NotEqual(t, "123", m2.RefID) }) t.Run("IdAlreadyExists", func(t *testing.T) { - authToken := "69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7ac0" - + m := FindSessionByRefID("sessxkkcxxxx") + assert.Empty(t, m) s := &Session{ UserName: "charles", SessExpires: unix.Day * 3, @@ -283,11 +286,54 @@ func TestSession_Create(t *testing.T) { RefID: "sessxkkcxxxx", } - s.SetAuthToken(authToken) + s.SetAuthToken("69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7xxx") err := s.Create() + require.Nil(t, err) + + t.Cleanup(func() { + s.Delete() + }) + + authToken := "69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7ac0" + + s2 := &Session{ + UserName: "charles", + SessExpires: unix.Day * 3, + SessTimeout: unix.Now() + unix.Week, + RefID: "sessxkkcxxxx", + } + + s2.SetAuthToken(authToken) + + err = s2.Create() assert.Error(t, err) }) + t.Run("LongNumericAuthID", func(t *testing.T) { + refID := rnd.RefID("ts") + m := FindSessionByRefID(refID) + assert.Empty(t, m) + s := &Session{ + UserName: "charles", + SessExpires: unix.Day * 3, + SessTimeout: unix.Now() + unix.Week, + RefID: refID, + AuthID: "012345678901234567890", + } + + s.SetAuthToken("69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7xxs") + + err := s.Create() + require.Nil(t, err) + + t.Cleanup(func() { + s.Delete() + }) + + m2 := FindSessionByRefID(refID) + assert.Equal(t, "charles", m2.UserName) + assert.Equal(t, "012345678901234567890", m2.AuthID) + }) } func TestSession_Save(t *testing.T) { @@ -304,14 +350,37 @@ func TestSession_Save(t *testing.T) { s.SetAuthToken("69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7xxy") err := s.Save() - - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) m2 := FindSessionByRefID("sessxkkcxxxy") assert.Equal(t, "chris", m2.UserName) }) + + t.Run("LongNumericAuthID", func(t *testing.T) { + refID := rnd.RefID("ts") + m := FindSessionByRefID(refID) + assert.Empty(t, m) + s := &Session{ + UserName: "chris", + SessExpires: unix.Day * 3, + SessTimeout: unix.Now() + unix.Week, + RefID: refID, + AuthID: "012345678901234567890", + } + + s.SetAuthToken("69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7xxy") + + err := s.Save() + require.Nil(t, err) + + t.Cleanup(func() { + s.Delete() + }) + + m2 := FindSessionByRefID(refID) + assert.Equal(t, "chris", m2.UserName) + assert.Equal(t, "012345678901234567890", m2.AuthID) + }) } func TestSession_Updates(t *testing.T) { diff --git a/internal/entity/auth_user.go b/internal/entity/auth_user.go index 062259ad5..b8c9eb683 100644 --- a/internal/entity/auth_user.go +++ b/internal/entity/auth_user.go @@ -52,7 +52,7 @@ type User struct { AuthProvider string `gorm:"type:VARBINARY(128);default:'';" json:"AuthProvider" yaml:"AuthProvider,omitempty"` AuthMethod string `gorm:"type:VARBINARY(128);default:'';" json:"AuthMethod" yaml:"AuthMethod,omitempty"` AuthIssuer string `gorm:"type:VARBINARY(255);default:'';" json:"AuthIssuer,omitempty" yaml:"AuthIssuer,omitempty"` - AuthID string `gorm:"type:VARBINARY(264);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` + AuthID string `gorm:"type:VARBINARY(264);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` // Make sure that you wrap and unwrap if using auth_id in a query. See FindUser below. UserName string `gorm:"size:200;index;" json:"Name" yaml:"Name,omitempty"` DisplayName string `gorm:"size:200;" json:"DisplayName" yaml:"DisplayName,omitempty"` UserEmail string `gorm:"size:255;index;" json:"Email" yaml:"Email,omitempty"` @@ -148,18 +148,18 @@ func FindUser(find User) *User { stmt = stmt.Where("user_uid = ?", find.UserUID) } else if authn.ProviderOIDC.Equal(find.AuthProvider) && find.AuthID != "" { if find.AuthIssuer == "" { - stmt = stmt.Where("auth_provider = ? AND auth_id = ?", find.AuthProvider, find.AuthID) + stmt = stmt.Where("auth_provider = ? AND auth_id = ?", find.AuthProvider, wrapString(find.AuthID)) } else { - stmt = stmt.Where("auth_provider = ? AND (auth_issuer = '' OR auth_issuer = ?) AND auth_id = ?", find.AuthProvider, find.AuthIssuer, find.AuthID) + stmt = stmt.Where("auth_provider = ? AND (auth_issuer = '' OR auth_issuer = ?) AND auth_id = ?", find.AuthProvider, find.AuthIssuer, wrapString(find.AuthID)) } } else if find.AuthProvider != "" && find.AuthID != "" && find.UserName != "" { - stmt = stmt.Where("auth_provider = ? AND auth_id = ? OR user_name = ?", find.AuthProvider, find.AuthID, find.UserName) + stmt = stmt.Where("auth_provider = ? AND auth_id = ? OR user_name = ?", find.AuthProvider, wrapString(find.AuthID), find.UserName) } else if find.UserName != "" { stmt = stmt.Where("user_name = ?", find.UserName) } else if find.UserEmail != "" { stmt = stmt.Where("user_email = ?", find.UserEmail) } else if find.AuthProvider != "" && find.AuthID != "" { - stmt = stmt.Where("auth_provider = ? AND auth_id = ?", find.AuthProvider, find.AuthID) + stmt = stmt.Where("auth_provider = ? AND auth_id = ?", find.AuthProvider, wrapString(find.AuthID)) } else { return nil } @@ -413,12 +413,12 @@ func (m *User) BeforeCreate(scope *gorm.Scope) error { Log("user", "set ref id", scope.SetColumn("RefID", m.RefID)) } + m.wrapAuthID() + if rnd.IsUnique(m.UserUID, UserUID) { return nil } - m.wrapAuthID() - m.UserUID = rnd.GenerateUID(UserUID) return scope.SetColumn("UserUID", m.UserUID) } @@ -673,17 +673,17 @@ func (m *User) SetMethod(method authn.MethodType) *User { // SetAuthID sets a custom authentication identifier. func (m *User) SetAuthID(id, issuer string) *User { // Update auth id if not empty. - if authId := clean.Auth(id); authId == "" { + if authID := clean.Auth(id); authID == "" { return m } else { - m.AuthID = authId + m.AuthID = authID m.AuthIssuer = clean.Uri(issuer) } // Make sure other users do not use the same identifier. if m.HasUID() && m.AuthProvider != "" { if err := UnscopedDb().Model(&User{}). - Where("user_uid <> ? AND auth_provider = ? AND auth_id = ? AND super_admin = 0", m.UserUID, m.AuthProvider, m.AuthID). + Where("user_uid <> ? AND auth_provider = ? AND auth_id = ? AND super_admin = 0", m.UserUID, m.AuthProvider, wrapString(m.AuthID)). Updates(Values{"auth_id": "", "auth_provider": authn.ProviderNone}).Error; err != nil { event.AuditErr([]string{"user %s", "failed to resolve auth id conflicts", status.Error(err)}, m.RefID) } diff --git a/internal/entity/auth_user_test.go b/internal/entity/auth_user_test.go index 481bc6a49..9a948276d 100644 --- a/internal/entity/auth_user_test.go +++ b/internal/entity/auth_user_test.go @@ -324,6 +324,32 @@ func TestUser_Create(t *testing.T) { t.Fatal(err) } }) + t.Run("LongNumericAuthID", func(t *testing.T) { + useruid := rnd.GenerateUID(UserUID) + var m = User{ + UserUID: useruid, + UserName: "examplelong", + UserRole: string(acl.RoleGuest), + DisplayName: "Example Long", + SuperAdmin: false, + CanLogin: true, + AuthID: "012345678901234567890", + AuthProvider: string(authn.ProviderOIDC), + } + + if err := m.Create(); err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { + m.Delete() + UnscopedDb().Delete(m) + }) + + assert.Equal(t, "examplelong", m.Username()) + assert.Equal(t, "examplelong", m.UserName) + assert.Equal(t, "012345678901234567890", m.AuthID) + }) } func TestUser_UpdateUsername(t *testing.T) { @@ -551,6 +577,14 @@ func TestFindUser(t *testing.T) { assert.NotEmpty(t, m.UserUID) assert.Equal(t, "jane.doe", m.UserName) assert.Equal(t, "oidc", m.AuthProvider) + + n := FindUser(User{AuthProvider: authn.ProviderOIDC.String(), AuthID: info.Subject}) + + require.NotNil(t, n) + + assert.NotEmpty(t, n.UserUID) + assert.Equal(t, "jane.doe", n.UserName) + assert.Equal(t, "oidc", n.AuthProvider) }) t.Run("UserName", func(t *testing.T) { m := FindUser(User{UserName: "admin"}) @@ -1822,6 +1856,38 @@ func TestUser_SetAuthID(t *testing.T) { assert.Equal(t, uuid, m.AuthID) assert.Equal(t, "", m.AuthIssuer) }) + + t.Run("DupeAuthProviderAndID", func(t *testing.T) { + m := UserFixtures.Get("guest") + n := NewUser() + n.UserName = "guest2" + n.DisplayName = "Guest User2" + n.UserEmail = "guest2@example.com" + n.UserRole = acl.RoleGuest.String() + n.AuthProvider = authn.ProviderOIDC.String() + n.AuthMethod = authn.MethodDefault.String() + n.SuperAdmin = false + n.CanLogin = true + n.SetAuthID(uuid, issuer) + n.Save() + + t.Cleanup(func() { + n.Delete() + UnscopedDb().Delete(n) + }) + + newUserUID := n.UserUID + m.SetAuthID(uuid, issuer) + assert.Equal(t, uuid, m.AuthID) + assert.Equal(t, issuer, m.AuthIssuer) + + n = FindUserByUID(newUserUID) + require.NotNil(t, n) + + assert.Equal(t, "guest2", n.UserName) + assert.Equal(t, "", n.AuthID) + assert.Equal(t, authn.ProviderNone.String(), n.AuthProvider) + }) } func TestUser_UpdateAuthID(t *testing.T) { From 723697cb1ca3334c71ac7be0294ae0f2a1883aab Mon Sep 17 00:00:00 2001 From: Keith Martin Date: Sat, 8 Nov 2025 22:32:27 +1000 Subject: [PATCH 3/6] Entity: hard code toggle off wrap/unwrap, implement dbms migration change to pre-create/alter tables with auth_id in SQLite and pre-create for MariaDB --- internal/entity/auth_session.go | 3 + internal/entity/auth_user.go | 2 + internal/entity/entity_tables.go | 15 ++ .../entity/migrate/dbms_authid_migration.go | 169 ++++++++++++++++++ 4 files changed, 189 insertions(+) create mode 100644 internal/entity/migrate/dbms_authid_migration.go diff --git a/internal/entity/auth_session.go b/internal/entity/auth_session.go index fd61d3361..b208a2d82 100644 --- a/internal/entity/auth_session.go +++ b/internal/entity/auth_session.go @@ -278,6 +278,7 @@ func (m *Session) Updates(values interface{}) error { // Wraps a string value in pseudo XML to force type to string func wrapString(s string) (r string) { + return s r = s if s != "" && !strings.HasPrefix(s, "") && !strings.HasSuffix(s, "") { r = fmt.Sprintf("%s", s) @@ -287,11 +288,13 @@ func wrapString(s string) (r string) { // Wraps the AuthID field so that SQLite will save it correctly func (m *Session) wrapAuthID() { + return m.AuthID = wrapString(m.AuthID) } // Unwraps the AuthID field so that PhotoPrism can use it correctly func (m *Session) unwrapAuthID() { + return if m.AuthID != "" && strings.HasPrefix(m.AuthID, "") && strings.HasSuffix(m.AuthID, "") { m.AuthID = strings.TrimSuffix(strings.TrimPrefix(m.AuthID, ""), "") } diff --git a/internal/entity/auth_user.go b/internal/entity/auth_user.go index b8c9eb683..f3507b985 100644 --- a/internal/entity/auth_user.go +++ b/internal/entity/auth_user.go @@ -384,6 +384,7 @@ func (m *User) Updates(values interface{}) error { // Wraps the AuthID field so that SQLite will save it correctly func (m *User) wrapAuthID() { + return if m.AuthID != "" && !strings.HasPrefix(m.AuthID, "") && !strings.HasSuffix(m.AuthID, "") { m.AuthID = fmt.Sprintf("%s", m.AuthID) } @@ -391,6 +392,7 @@ func (m *User) wrapAuthID() { // Unwraps the AuthID field so that PhotoPrism can use it correctly func (m *User) unwrapAuthID() { + return if m.AuthID != "" && strings.HasPrefix(m.AuthID, "") && strings.HasSuffix(m.AuthID, "") { m.AuthID = strings.TrimSuffix(strings.TrimPrefix(m.AuthID, ""), "") } diff --git a/internal/entity/entity_tables.go b/internal/entity/entity_tables.go index cfcd5257c..c81830a6c 100644 --- a/internal/entity/entity_tables.go +++ b/internal/entity/entity_tables.go @@ -122,6 +122,21 @@ func (list Tables) Migrate(db *gorm.DB, opt migrate.Options) { // Run ORM auto migrations. if opt.AutoMigrate { + // Check if the DBMS AuthID fix has been applied? + version := migrate.FirstOrCreateVersion(db, migrate.NewVersion("DBMS AuthID Fix", "Any Editions")) + if version.NeedsMigration() { + if err := migrate.ConvertDBMSAuthIDDataTypes(db); err != nil { + log.Errorf("migrate: could not apply dbms auth_id fix : %v", err) + version.Error = err.Error() + version.Save(db) + } else { + version.Migrated(db) + log.Debug("migrate: DBMS AuthID fix migrated") + } + } else { + log.Debug("migrate: DBMS AuthID fix skipped") + } + for name, entity = range list { if err := db.AutoMigrate(entity).Error; err != nil { log.Debugf("migrate: %s (waiting 1s)", err.Error()) diff --git a/internal/entity/migrate/dbms_authid_migration.go b/internal/entity/migrate/dbms_authid_migration.go new file mode 100644 index 000000000..90e9b3978 --- /dev/null +++ b/internal/entity/migrate/dbms_authid_migration.go @@ -0,0 +1,169 @@ +package migrate + +import ( + "fmt" + "strings" + + "github.com/jinzhu/gorm" +) + +// ConvertDBMSAuthIDDataTypes applies the data type conversion for auth_id columns for SQLite +// Conversion is from VARBINARY(255) DEFAULT ” to TEXT NOT NULL COLLATE BINARY DEFAULT ” +// This can't be done in a script as the order of columns is not known, and is determined by the age of the database +func ConvertDBMSAuthIDDataTypes(db *gorm.DB) (err error) { + switch db.Dialect().GetName() { + case SQLite3: + // These create statements will get out of date, but that is ok, as the main migrate path will add any missing columns/indexes in later. + authSessionsCreate := `CREATE TABLE "auth_sessions" ("id" VARBINARY(2048),"user_uid" VARBINARY(42) DEFAULT '',"user_name" varchar(200),"client_uid" VARBINARY(42) DEFAULT '',"client_name" varchar(200) DEFAULT '',"client_ip" varchar(64),"auth_provider" VARBINARY(128) DEFAULT '',"auth_method" VARBINARY(128) DEFAULT '',"auth_issuer" VARBINARY(255) DEFAULT '',"auth_id" TEXT NOT NULL COLLATE BINARY DEFAULT '',"auth_scope" varchar(1024) DEFAULT '',"grant_type" VARBINARY(64) DEFAULT '',"last_active" bigint,"sess_expires" bigint,"sess_timeout" bigint,"preview_token" VARBINARY(64) DEFAULT '',"download_token" VARBINARY(64) DEFAULT '',"access_token" VARBINARY(4096) DEFAULT '',"refresh_token" VARBINARY(2048) DEFAULT '',"id_token" VARBINARY(2048) DEFAULT '',"user_agent" varchar(512),"data_json" VARBINARY(4096),"ref_id" VARBINARY(16) DEFAULT '',"login_ip" varchar(64),"login_at" datetime,"created_at" datetime,"updated_at" datetime , PRIMARY KEY ("id"))` + authUsersCreate := `CREATE TABLE "auth_users" ("id" integer primary key autoincrement,"user_uuid" VARBINARY(64),"user_uid" VARBINARY(42),"auth_provider" VARBINARY(128) DEFAULT '',"auth_method" VARBINARY(128) DEFAULT '',"auth_issuer" VARBINARY(255) DEFAULT '',"auth_id" TEXT NOT NULL COLLATE BINARY DEFAULT '',"user_name" varchar(200),"display_name" varchar(200),"user_email" varchar(255),"backup_email" varchar(255),"user_role" varchar(64) DEFAULT '',"user_scope" varchar(1024) DEFAULT '*',"user_attr" varchar(1024) DEFAULT '',"super_admin" bool,"can_login" bool,"login_at" datetime,"expires_at" datetime,"webdav" bool,"base_path" VARBINARY(1024),"upload_path" VARBINARY(1024),"can_invite" bool,"invite_token" VARBINARY(64),"invited_by" varchar(64),"verify_token" VARBINARY(64),"verified_at" datetime,"consent_at" datetime,"born_at" datetime,"reset_token" VARBINARY(64),"preview_token" VARBINARY(64),"download_token" VARBINARY(64),"thumb" VARBINARY(128) DEFAULT '',"thumb_src" VARBINARY(8) DEFAULT '',"ref_id" VARBINARY(16),"created_at" datetime,"updated_at" datetime,"deleted_at" datetime )` + + type resultIndex struct { + Name string + } + + type pragmaTable struct { + Cid int + Name string + Type string + Notnull int + DfltValue string + Pk int + } + + if !db.HasTable("auth_sessions") { + if err := db.Exec(authSessionsCreate).Error; err != nil { + return fmt.Errorf("migrate: error creating auth_sessions %w", err) + } + } else { + // Data Migration here, by rename, create new, data transfer, drop indexes + if err := db.Exec(`ALTER TABLE "auth_sessions" RENAME TO "migrate_auth_sessions"`).Error; err != nil { + return fmt.Errorf("migrate: error renaming auth_sessions %w", err) + } + if err := db.Exec(authSessionsCreate).Error; err != nil { + return fmt.Errorf("migrate: error creating auth_sessions %w", err) + } + + // Get the columns of both old and new table, and find the columns that are in the old and new table + var oldPragmaColumns []pragmaTable + var newPragmaColumns []pragmaTable + oldColumns := make(map[string]bool) + + if err := db.Raw("PRAGMA table_info(migrate_auth_sessions)").Scan(&oldPragmaColumns).Error; err != nil { + return fmt.Errorf("migrate: error getting column list for migrate_auth_sessions with %w", err) + } + for _, pragma := range oldPragmaColumns { + oldColumns[pragma.Name] = false + } + + if err := db.Raw("PRAGMA table_info(auth_sessions)").Scan(&newPragmaColumns).Error; err != nil { + return fmt.Errorf("migrate: error getting column list for auth_sessions with %w", err) + } + for _, pragma := range newPragmaColumns { + if _, present := oldColumns[pragma.Name]; present { + oldColumns[pragma.Name] = true + } + } + // Build the select into statement + var columns []string + for key, value := range oldColumns { + if value { + columns = append(columns, key) + } + } + + populateStmt := fmt.Sprintf("INSERT INTO auth_sessions (%s) SELECT %s FROM migrate_auth_sessions", strings.Join(columns, ", "), strings.Join(columns, ", ")) + + if err := db.Exec(populateStmt).Error; err != nil { + return fmt.Errorf("migrate: error migrating with stmt %s with %w", populateStmt, err) + } + + var indexes []resultIndex + if err := db.Raw("SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = ? AND sql IS NOT NULL", "migrate_auth_sessions").Scan(&indexes).Error; err != nil { + return fmt.Errorf("migrate: error getting index list %w", err) + } + for _, index := range indexes { + dropStatement := fmt.Sprintf(`DROP INDEX IF EXISTS "%s"`, index.Name) + if err := db.Exec(dropStatement).Error; err != nil { + return fmt.Errorf("migrate: error dropping index %s was %w", index.Name, err) + } + } + } + if !db.HasTable("auth_users") { + if err := db.Exec(authUsersCreate).Error; err != nil { + return fmt.Errorf("migrate: error creating auth_users %w", err) + } + } else { + // Data Migration here, by rename, create new, data transfer, drop indexes + if err := db.Exec(`ALTER TABLE "auth_users" RENAME TO "migrate_auth_users"`).Error; err != nil { + return fmt.Errorf("migrate: error renaming auth_users %w", err) + } + if err := db.Exec(authUsersCreate).Error; err != nil { + return fmt.Errorf("migrate: error creating auth_users %w", err) + } + + // Get the columns of both old and new table, and find the columns that are in the old and new table + var oldPragmaColumns []pragmaTable + var newPragmaColumns []pragmaTable + oldColumns := make(map[string]bool) + + if err := db.Raw("PRAGMA table_info(migrate_auth_users)").Scan(&oldPragmaColumns).Error; err != nil { + return fmt.Errorf("migrate: error getting column list for migrate_auth_users with %w", err) + } + for _, pragma := range oldPragmaColumns { + oldColumns[pragma.Name] = false + } + + if err := db.Raw("PRAGMA table_info(auth_users)").Scan(&newPragmaColumns).Error; err != nil { + return fmt.Errorf("migrate: error getting column list for auth_users with %w", err) + } + for _, pragma := range newPragmaColumns { + if _, present := oldColumns[pragma.Name]; present { + oldColumns[pragma.Name] = true + } + } + // Build the select into statement + var columns []string + for key, value := range oldColumns { + if value { + columns = append(columns, key) + } + } + + populateStmt := fmt.Sprintf("INSERT INTO auth_users (%s) SELECT %s FROM migrate_auth_users", strings.Join(columns, ", "), strings.Join(columns, ", ")) + + if err := db.Exec(populateStmt).Error; err != nil { + return fmt.Errorf("migrate: error migrating with stmt %s with %w", populateStmt, err) + } + + var indexes []resultIndex + if err := db.Raw("SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = ? AND sql IS NOT NULL", "migrate_auth_users").Scan(&indexes).Error; err != nil { + return fmt.Errorf("migrate: error getting index list %w", err) + } + for _, index := range indexes { + dropStatement := fmt.Sprintf(`DROP INDEX IF EXISTS "%s"`, index.Name) + if err := db.Exec(dropStatement).Error; err != nil { + return fmt.Errorf("migrate: error dropping index %s was %w", index.Name, err) + } + } + } + case MySQL: // MySQL + // These create statements will get out of date, but that is ok, as the main migrate path will add any missing columns/indexes in later. + authSessionsCreate := "CREATE TABLE `auth_sessions` (`id` VARBINARY(2048),`user_uid` VARBINARY(42) DEFAULT '',`user_name` varchar(200),`client_uid` VARBINARY(42) DEFAULT '',`client_name` varchar(200) DEFAULT '',`client_ip` varchar(64),`auth_provider` VARBINARY(128) DEFAULT '',`auth_method` VARBINARY(128) DEFAULT '',`auth_issuer` VARBINARY(255) DEFAULT '',`auth_id` VARBINARY(264) DEFAULT '',`auth_scope` varchar(1024) DEFAULT '',`grant_type` VARBINARY(64) DEFAULT '',`last_active` bigint,`sess_expires` bigint,`sess_timeout` bigint,`preview_token` VARBINARY(64) DEFAULT '',`download_token` VARBINARY(64) DEFAULT '',`access_token` VARBINARY(4096) DEFAULT '',`refresh_token` VARBINARY(2048) DEFAULT '',`id_token` VARBINARY(2048) DEFAULT '',`user_agent` varchar(512),`data_json` VARBINARY(4096),`ref_id` VARBINARY(16) DEFAULT '',`login_ip` varchar(64),`login_at` DATETIME NULL,`created_at` DATETIME NULL,`updated_at` DATETIME NULL , PRIMARY KEY (`id`))" + authUsersCreate := "CREATE TABLE `auth_users` (`id` int AUTO_INCREMENT,`user_uuid` VARBINARY(64),`user_uid` VARBINARY(42),`auth_provider` VARBINARY(128) DEFAULT '',`auth_method` VARBINARY(128) DEFAULT '',`auth_issuer` VARBINARY(255) DEFAULT '',`auth_id` VARBINARY(264) DEFAULT '',`user_name` varchar(200),`display_name` varchar(200),`user_email` varchar(255),`backup_email` varchar(255),`user_role` varchar(64) DEFAULT '',`user_scope` varchar(1024) DEFAULT '*',`user_attr` varchar(1024) DEFAULT '',`super_admin` boolean,`can_login` boolean,`login_at` DATETIME NULL,`expires_at` DATETIME NULL,`webdav` boolean,`base_path` VARBINARY(1024),`upload_path` VARBINARY(1024),`can_invite` boolean,`invite_token` VARBINARY(64),`invited_by` varchar(64),`verify_token` VARBINARY(64),`verified_at` DATETIME NULL,`consent_at` DATETIME NULL,`born_at` DATETIME NULL,`reset_token` VARBINARY(64),`preview_token` VARBINARY(64),`download_token` VARBINARY(64),`thumb` VARBINARY(128) DEFAULT '',`thumb_src` VARBINARY(8) DEFAULT '',`ref_id` VARBINARY(16),`created_at` DATETIME NULL,`updated_at` DATETIME NULL,`deleted_at` DATETIME NULL , PRIMARY KEY (`id`))" + if !db.HasTable("auth_sessions") { + if err := db.Exec(authSessionsCreate).Error; err != nil { + return fmt.Errorf("migrate: error creating auth_sessions %w", err) + } + } + if !db.HasTable("auth_users") { + if err := db.Exec(authUsersCreate).Error; err != nil { + return fmt.Errorf("migrate: error creating auth_users %w", err) + } + } + // There are no migration needs for MariaDB as the structure is not being manipulated. + // case Postgres: + // Nothing required for Gorm V1 + default: + } + return nil +} From a691ddb98bbec1d4679870eb63941d4d9e08eca7 Mon Sep 17 00:00:00 2001 From: Keith Martin Date: Tue, 18 Nov 2025 22:32:39 +1000 Subject: [PATCH 4/6] Entity: revert wrap/unwrap for AuthID --- internal/entity/auth_session.go | 57 +----------------------- internal/entity/auth_session_delete.go | 2 +- internal/entity/auth_user.go | 60 +++----------------------- 3 files changed, 8 insertions(+), 111 deletions(-) diff --git a/internal/entity/auth_session.go b/internal/entity/auth_session.go index b208a2d82..47ce96c8c 100644 --- a/internal/entity/auth_session.go +++ b/internal/entity/auth_session.go @@ -50,7 +50,7 @@ type Session struct { AuthProvider string `gorm:"type:VARBINARY(128);default:'';" json:"AuthProvider" yaml:"AuthProvider,omitempty"` AuthMethod string `gorm:"type:VARBINARY(128);default:'';" json:"AuthMethod" yaml:"AuthMethod,omitempty"` AuthIssuer string `gorm:"type:VARBINARY(255);default:'';" json:"AuthIssuer,omitempty" yaml:"AuthIssuer,omitempty"` - AuthID string `gorm:"type:VARBINARY(264);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` // Make sure that you wrap and unwrap if using auth_id in a query. + AuthID string `gorm:"type:VARBINARY(255);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` AuthScope string `gorm:"size:1024;default:'';" json:"AuthScope" yaml:"AuthScope,omitempty"` GrantType string `gorm:"type:VARBINARY(64);default:'';" json:"GrantType" yaml:"GrantType,omitempty"` LastActive int64 `json:"LastActive" yaml:"LastActive,omitempty"` @@ -276,30 +276,6 @@ func (m *Session) Updates(values interface{}) error { return UnscopedDb().Model(m).Updates(values).Error } -// Wraps a string value in pseudo XML to force type to string -func wrapString(s string) (r string) { - return s - r = s - if s != "" && !strings.HasPrefix(s, "") && !strings.HasSuffix(s, "") { - r = fmt.Sprintf("%s", s) - } - return r -} - -// Wraps the AuthID field so that SQLite will save it correctly -func (m *Session) wrapAuthID() { - return - m.AuthID = wrapString(m.AuthID) -} - -// Unwraps the AuthID field so that PhotoPrism can use it correctly -func (m *Session) unwrapAuthID() { - return - if m.AuthID != "" && strings.HasPrefix(m.AuthID, "") && strings.HasSuffix(m.AuthID, "") { - m.AuthID = strings.TrimSuffix(strings.TrimPrefix(m.AuthID, ""), "") - } -} - // BeforeCreate creates a random UID if needed before inserting a new row to the database. func (m *Session) BeforeCreate(scope *gorm.Scope) error { if rnd.InvalidRefID(m.RefID) { @@ -307,7 +283,6 @@ func (m *Session) BeforeCreate(scope *gorm.Scope) error { Log("session", "set ref id", scope.SetColumn("RefID", m.RefID)) } - m.wrapAuthID() if rnd.IsSessionID(m.ID) { return nil } @@ -317,36 +292,6 @@ func (m *Session) BeforeCreate(scope *gorm.Scope) error { return scope.SetColumn("ID", m.ID) } -// BeforeSave ensures that the AuthID will save correctly on SQLite -func (m *Session) BeforeSave(scope *gorm.Scope) error { - m.wrapAuthID() - return nil -} - -// BeforeUpdate ensures that the AuthID will save correctly on SQLite -func (m *Session) BeforeUpdate(scope *gorm.Scope) error { - m.wrapAuthID() - return nil -} - -// AfterSave ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite -func (m *Session) AfterSave(scope *gorm.Scope) error { - m.unwrapAuthID() - return nil -} - -// AfterUpdate ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite -func (m *Session) AfterUpdate(scope *gorm.Scope) error { - m.unwrapAuthID() - return nil -} - -// AfterFind ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite -func (m *Session) AfterFind(scope *gorm.Scope) error { - m.unwrapAuthID() - return nil -} - // SetClient sets the client of this session. func (m *Session) SetClient(c *Client) *Session { if c == nil { diff --git a/internal/entity/auth_session_delete.go b/internal/entity/auth_session_delete.go index 0d82c7fec..79d55daba 100644 --- a/internal/entity/auth_session_delete.go +++ b/internal/entity/auth_session_delete.go @@ -49,7 +49,7 @@ func DeleteChildSessions(s *Session) (deleted int) { found := Sessions{} - if err := Db().Where("auth_id = ? AND auth_method = ?", wrapString(s.ID), authn.MethodSession.String()).Find(&found).Error; err != nil { + if err := Db().Where("auth_id = ? AND auth_method = ?", s.ID, authn.MethodSession.String()).Find(&found).Error; err != nil { event.AuditErr([]string{"failed to find child sessions", status.Error(err)}) return deleted } diff --git a/internal/entity/auth_user.go b/internal/entity/auth_user.go index f3507b985..e57ed8709 100644 --- a/internal/entity/auth_user.go +++ b/internal/entity/auth_user.go @@ -52,7 +52,7 @@ type User struct { AuthProvider string `gorm:"type:VARBINARY(128);default:'';" json:"AuthProvider" yaml:"AuthProvider,omitempty"` AuthMethod string `gorm:"type:VARBINARY(128);default:'';" json:"AuthMethod" yaml:"AuthMethod,omitempty"` AuthIssuer string `gorm:"type:VARBINARY(255);default:'';" json:"AuthIssuer,omitempty" yaml:"AuthIssuer,omitempty"` - AuthID string `gorm:"type:VARBINARY(264);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` // Make sure that you wrap and unwrap if using auth_id in a query. See FindUser below. + AuthID string `gorm:"type:VARBINARY(255);index;default:'';" json:"AuthID" yaml:"AuthID,omitempty"` UserName string `gorm:"size:200;index;" json:"Name" yaml:"Name,omitempty"` DisplayName string `gorm:"size:200;" json:"DisplayName" yaml:"DisplayName,omitempty"` UserEmail string `gorm:"size:255;index;" json:"Email" yaml:"Email,omitempty"` @@ -148,18 +148,18 @@ func FindUser(find User) *User { stmt = stmt.Where("user_uid = ?", find.UserUID) } else if authn.ProviderOIDC.Equal(find.AuthProvider) && find.AuthID != "" { if find.AuthIssuer == "" { - stmt = stmt.Where("auth_provider = ? AND auth_id = ?", find.AuthProvider, wrapString(find.AuthID)) + stmt = stmt.Where("auth_provider = ? AND auth_id = ?", find.AuthProvider, find.AuthID) } else { - stmt = stmt.Where("auth_provider = ? AND (auth_issuer = '' OR auth_issuer = ?) AND auth_id = ?", find.AuthProvider, find.AuthIssuer, wrapString(find.AuthID)) + stmt = stmt.Where("auth_provider = ? AND (auth_issuer = '' OR auth_issuer = ?) AND auth_id = ?", find.AuthProvider, find.AuthIssuer, find.AuthID) } } else if find.AuthProvider != "" && find.AuthID != "" && find.UserName != "" { - stmt = stmt.Where("auth_provider = ? AND auth_id = ? OR user_name = ?", find.AuthProvider, wrapString(find.AuthID), find.UserName) + stmt = stmt.Where("auth_provider = ? AND auth_id = ? OR user_name = ?", find.AuthProvider, find.AuthID, find.UserName) } else if find.UserName != "" { stmt = stmt.Where("user_name = ?", find.UserName) } else if find.UserEmail != "" { stmt = stmt.Where("user_email = ?", find.UserEmail) } else if find.AuthProvider != "" && find.AuthID != "" { - stmt = stmt.Where("auth_provider = ? AND auth_id = ?", find.AuthProvider, wrapString(find.AuthID)) + stmt = stmt.Where("auth_provider = ? AND auth_id = ?", find.AuthProvider, find.AuthID) } else { return nil } @@ -382,22 +382,6 @@ func (m *User) Updates(values interface{}) error { return UnscopedDb().Model(m).Updates(values).Error } -// Wraps the AuthID field so that SQLite will save it correctly -func (m *User) wrapAuthID() { - return - if m.AuthID != "" && !strings.HasPrefix(m.AuthID, "") && !strings.HasSuffix(m.AuthID, "") { - m.AuthID = fmt.Sprintf("%s", m.AuthID) - } -} - -// Unwraps the AuthID field so that PhotoPrism can use it correctly -func (m *User) unwrapAuthID() { - return - if m.AuthID != "" && strings.HasPrefix(m.AuthID, "") && strings.HasSuffix(m.AuthID, "") { - m.AuthID = strings.TrimSuffix(strings.TrimPrefix(m.AuthID, ""), "") - } -} - // BeforeCreate sets a random UID if needed before inserting a new row to the database. func (m *User) BeforeCreate(scope *gorm.Scope) error { if m.UserSettings != nil { @@ -415,8 +399,6 @@ func (m *User) BeforeCreate(scope *gorm.Scope) error { Log("user", "set ref id", scope.SetColumn("RefID", m.RefID)) } - m.wrapAuthID() - if rnd.IsUnique(m.UserUID, UserUID) { return nil } @@ -425,36 +407,6 @@ func (m *User) BeforeCreate(scope *gorm.Scope) error { return scope.SetColumn("UserUID", m.UserUID) } -// BeforeSave ensures that the AuthID will save correctly on SQLite -func (m *User) BeforeSave(scope *gorm.Scope) error { - m.wrapAuthID() - return nil -} - -// BeforeUpdate ensures that the AuthID will save correctly on SQLite -func (m *User) BeforeUpdate(scope *gorm.Scope) error { - m.wrapAuthID() - return nil -} - -// AfterSave ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite -func (m *User) AfterSave(scope *gorm.Scope) error { - m.unwrapAuthID() - return nil -} - -// AfterUpdate ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite -func (m *User) AfterUpdate(scope *gorm.Scope) error { - m.unwrapAuthID() - return nil -} - -// AfterFind ensures that the AuthID will not have the prefix and suffix added so that it will save correctly on SQLite -func (m *User) AfterFind(scope *gorm.Scope) error { - m.unwrapAuthID() - return nil -} - // IsExpired checks if the user account has expired. func (m *User) IsExpired() bool { if m.ExpiresAt == nil { @@ -685,7 +637,7 @@ func (m *User) SetAuthID(id, issuer string) *User { // Make sure other users do not use the same identifier. if m.HasUID() && m.AuthProvider != "" { if err := UnscopedDb().Model(&User{}). - Where("user_uid <> ? AND auth_provider = ? AND auth_id = ? AND super_admin = 0", m.UserUID, m.AuthProvider, wrapString(m.AuthID)). + Where("user_uid <> ? AND auth_provider = ? AND auth_id = ? AND super_admin = 0", m.UserUID, m.AuthProvider, m.AuthID). Updates(Values{"auth_id": "", "auth_provider": authn.ProviderNone}).Error; err != nil { event.AuditErr([]string{"user %s", "failed to resolve auth id conflicts", status.Error(err)}, m.RefID) } From 37e731a96dca0b3ceaf6dac5f8ecc7a8caf78b74 Mon Sep 17 00:00:00 2001 From: Keith Martin Date: Wed, 19 Nov 2025 20:24:00 +1000 Subject: [PATCH 5/6] Migrate: Comment out MySQL statements required for GormV2 --- .../entity/migrate/dbms_authid_migration.go | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/internal/entity/migrate/dbms_authid_migration.go b/internal/entity/migrate/dbms_authid_migration.go index 90e9b3978..f2956908d 100644 --- a/internal/entity/migrate/dbms_authid_migration.go +++ b/internal/entity/migrate/dbms_authid_migration.go @@ -146,21 +146,22 @@ func ConvertDBMSAuthIDDataTypes(db *gorm.DB) (err error) { } } } - case MySQL: // MySQL - // These create statements will get out of date, but that is ok, as the main migrate path will add any missing columns/indexes in later. - authSessionsCreate := "CREATE TABLE `auth_sessions` (`id` VARBINARY(2048),`user_uid` VARBINARY(42) DEFAULT '',`user_name` varchar(200),`client_uid` VARBINARY(42) DEFAULT '',`client_name` varchar(200) DEFAULT '',`client_ip` varchar(64),`auth_provider` VARBINARY(128) DEFAULT '',`auth_method` VARBINARY(128) DEFAULT '',`auth_issuer` VARBINARY(255) DEFAULT '',`auth_id` VARBINARY(264) DEFAULT '',`auth_scope` varchar(1024) DEFAULT '',`grant_type` VARBINARY(64) DEFAULT '',`last_active` bigint,`sess_expires` bigint,`sess_timeout` bigint,`preview_token` VARBINARY(64) DEFAULT '',`download_token` VARBINARY(64) DEFAULT '',`access_token` VARBINARY(4096) DEFAULT '',`refresh_token` VARBINARY(2048) DEFAULT '',`id_token` VARBINARY(2048) DEFAULT '',`user_agent` varchar(512),`data_json` VARBINARY(4096),`ref_id` VARBINARY(16) DEFAULT '',`login_ip` varchar(64),`login_at` DATETIME NULL,`created_at` DATETIME NULL,`updated_at` DATETIME NULL , PRIMARY KEY (`id`))" - authUsersCreate := "CREATE TABLE `auth_users` (`id` int AUTO_INCREMENT,`user_uuid` VARBINARY(64),`user_uid` VARBINARY(42),`auth_provider` VARBINARY(128) DEFAULT '',`auth_method` VARBINARY(128) DEFAULT '',`auth_issuer` VARBINARY(255) DEFAULT '',`auth_id` VARBINARY(264) DEFAULT '',`user_name` varchar(200),`display_name` varchar(200),`user_email` varchar(255),`backup_email` varchar(255),`user_role` varchar(64) DEFAULT '',`user_scope` varchar(1024) DEFAULT '*',`user_attr` varchar(1024) DEFAULT '',`super_admin` boolean,`can_login` boolean,`login_at` DATETIME NULL,`expires_at` DATETIME NULL,`webdav` boolean,`base_path` VARBINARY(1024),`upload_path` VARBINARY(1024),`can_invite` boolean,`invite_token` VARBINARY(64),`invited_by` varchar(64),`verify_token` VARBINARY(64),`verified_at` DATETIME NULL,`consent_at` DATETIME NULL,`born_at` DATETIME NULL,`reset_token` VARBINARY(64),`preview_token` VARBINARY(64),`download_token` VARBINARY(64),`thumb` VARBINARY(128) DEFAULT '',`thumb_src` VARBINARY(8) DEFAULT '',`ref_id` VARBINARY(16),`created_at` DATETIME NULL,`updated_at` DATETIME NULL,`deleted_at` DATETIME NULL , PRIMARY KEY (`id`))" - if !db.HasTable("auth_sessions") { - if err := db.Exec(authSessionsCreate).Error; err != nil { - return fmt.Errorf("migrate: error creating auth_sessions %w", err) - } - } - if !db.HasTable("auth_users") { - if err := db.Exec(authUsersCreate).Error; err != nil { - return fmt.Errorf("migrate: error creating auth_users %w", err) - } - } - // There are no migration needs for MariaDB as the structure is not being manipulated. + // case MySQL: // MySQL + // Nothing required for Gorm V1. Statements left in comments for Gorm V2 implementation. + // // These create statements will get out of date, but that is ok, as the main migrate path will add any missing columns/indexes in later. + // authSessionsCreate := "CREATE TABLE `auth_sessions` (`id` VARBINARY(2048),`user_uid` VARBINARY(42) DEFAULT '',`user_name` varchar(200),`client_uid` VARBINARY(42) DEFAULT '',`client_name` varchar(200) DEFAULT '',`client_ip` varchar(64),`auth_provider` VARBINARY(128) DEFAULT '',`auth_method` VARBINARY(128) DEFAULT '',`auth_issuer` VARBINARY(255) DEFAULT '',`auth_id` VARBINARY(255) DEFAULT '',`auth_scope` varchar(1024) DEFAULT '',`grant_type` VARBINARY(64) DEFAULT '',`last_active` bigint,`sess_expires` bigint,`sess_timeout` bigint,`preview_token` VARBINARY(64) DEFAULT '',`download_token` VARBINARY(64) DEFAULT '',`access_token` VARBINARY(4096) DEFAULT '',`refresh_token` VARBINARY(2048) DEFAULT '',`id_token` VARBINARY(2048) DEFAULT '',`user_agent` varchar(512),`data_json` VARBINARY(4096),`ref_id` VARBINARY(16) DEFAULT '',`login_ip` varchar(64),`login_at` DATETIME NULL,`created_at` DATETIME NULL,`updated_at` DATETIME NULL , PRIMARY KEY (`id`))" + // authUsersCreate := "CREATE TABLE `auth_users` (`id` int AUTO_INCREMENT,`user_uuid` VARBINARY(64),`user_uid` VARBINARY(42),`auth_provider` VARBINARY(128) DEFAULT '',`auth_method` VARBINARY(128) DEFAULT '',`auth_issuer` VARBINARY(255) DEFAULT '',`auth_id` VARBINARY(255) DEFAULT '',`user_name` varchar(200),`display_name` varchar(200),`user_email` varchar(255),`backup_email` varchar(255),`user_role` varchar(64) DEFAULT '',`user_scope` varchar(1024) DEFAULT '*',`user_attr` varchar(1024) DEFAULT '',`super_admin` boolean,`can_login` boolean,`login_at` DATETIME NULL,`expires_at` DATETIME NULL,`webdav` boolean,`base_path` VARBINARY(1024),`upload_path` VARBINARY(1024),`can_invite` boolean,`invite_token` VARBINARY(64),`invited_by` varchar(64),`verify_token` VARBINARY(64),`verified_at` DATETIME NULL,`consent_at` DATETIME NULL,`born_at` DATETIME NULL,`reset_token` VARBINARY(64),`preview_token` VARBINARY(64),`download_token` VARBINARY(64),`thumb` VARBINARY(128) DEFAULT '',`thumb_src` VARBINARY(8) DEFAULT '',`ref_id` VARBINARY(16),`created_at` DATETIME NULL,`updated_at` DATETIME NULL,`deleted_at` DATETIME NULL , PRIMARY KEY (`id`))" + // if !db.HasTable("auth_sessions") { + // if err := db.Exec(authSessionsCreate).Error; err != nil { + // return fmt.Errorf("migrate: error creating auth_sessions %w", err) + // } + // } + // if !db.HasTable("auth_users") { + // if err := db.Exec(authUsersCreate).Error; err != nil { + // return fmt.Errorf("migrate: error creating auth_users %w", err) + // } + // } + // // There are no migration needs for MariaDB as the structure is not being manipulated. // case Postgres: // Nothing required for Gorm V1 default: From 0a30f8127f5599542a372be0dbe284199ead9153 Mon Sep 17 00:00:00 2001 From: Keith Martin Date: Wed, 19 Nov 2025 20:47:16 +1000 Subject: [PATCH 6/6] Entity: revert authID to authId as per review --- internal/entity/auth_user.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/entity/auth_user.go b/internal/entity/auth_user.go index e57ed8709..01f8e5c6d 100644 --- a/internal/entity/auth_user.go +++ b/internal/entity/auth_user.go @@ -627,10 +627,10 @@ func (m *User) SetMethod(method authn.MethodType) *User { // SetAuthID sets a custom authentication identifier. func (m *User) SetAuthID(id, issuer string) *User { // Update auth id if not empty. - if authID := clean.Auth(id); authID == "" { + if authId := clean.Auth(id); authId == "" { return m } else { - m.AuthID = authID + m.AuthID = authId m.AuthIssuer = clean.Uri(issuer) }