Mirrors/photoprism
1
0
Fork 0
mirror of https://github.com/photoprism/photoprism.git synced 2026-07-17 16:49:04 +00:00
Code Issues Projects Releases Wiki Activity

Users: Apply 2FA changes for the cluster service principal

Browse source
This commit is contained in:
Michael Mayer 2026-06-29 21:59:48 +00:00
parent 3e5ea71fb6
commit d330f0e05c
3 changed files with 70 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

9
internal/api/users_update.go
View file

@ -153,11 +153,10 @@ func UpdateUser(router *gin.RouterGroup) {
}
}
// Persist form values. SaveForm gates privilege-level fields on admin
// authorization; the cluster JWT is a user-less service principal that
// u.IsAdmin() rejects, so pass that decision explicitly or its login and
// role sync would be silently dropped.
if err = m.SaveForm(f, u, u.IsAdmin() || isClusterJWT); err != nil {
// Persist form values. The cluster JWT is a user-less principal that
// u.IsAdmin()/u.IsSuperAdmin() reject, so authorize it explicitly for both admin and
// super-admin-level changes (else its role, login, and 2FA edits are silently dropped).
if err = m.SaveForm(f, u, u.IsAdmin() || isClusterJWT, isClusterJWT); err != nil {
event.AuditErr([]string{ClientIP(c), "session %s", "users", m.UserName, "update", err.Error()}, s.RefID)
AbortSaveFailed(c)
return

15
internal/entity/auth_user.go
View file

@ -1436,11 +1436,11 @@ func (m *User) PrivilegeLevelChange(frm form.User) bool {
}
// SaveForm updates the entity using form data and stores it in the database.
// Privilege-level fields (role, login, WebDAV, paths) are applied only when
// byAdmin is true. The caller decides this: a regular admin actor, or a trusted
// cluster service principal that has no end-user identity of its own and so
// cannot satisfy u.IsAdmin() despite being authorized for user management.
func (m *User) SaveForm(frm form.User, u *User, byAdmin bool) error {
// Privilege-level fields (role, login, WebDAV, paths) are applied only when byAdmin;
// the most sensitive ones (super-admin status, auth provider/method such as 2FA,
// external identity) also require bySuperAdmin. The caller passes both explicitly so a
// user-less cluster service principal can act despite failing u.IsAdmin()/IsSuperAdmin().
func (m *User) SaveForm(frm form.User, u *User, byAdmin, bySuperAdmin bool) error {
if m.UserName == "" || m.IsSystemOrInvalid() {
return fmt.Errorf("system users cannot be modified")
} else if frm.SuperAdmin && !acl.IsAdminRole(acl.Role(frm.Role())) {
@ -1504,8 +1504,9 @@ func (m *User) SaveForm(frm form.User, u *User, byAdmin bool) error {
m.WebDAV = frm.WebDAV
m.UserAttr = frm.Attr()
// Only allow super admins to change the authentication method and make other users super admins.
if u.IsSuperAdmin() {
// Only super-admin-level authority may change the auth method or grant super admin:
// an actual super admin, or bySuperAdmin (the trusted cluster service principal).
if u.IsSuperAdmin() || bySuperAdmin {
if !u.Equal(m) {
m.SuperAdmin = frm.SuperAdmin
}

73
internal/entity/auth_user_test.go
View file

@ -1506,7 +1506,7 @@ func TestUser_SaveForm(t *testing.T) {
frm, err := UnknownUser.Form()
assert.NoError(t, err)
err = UnknownUser.SaveForm(frm, UserFixtures.Pointer("guest"), false)
err = UnknownUser.SaveForm(frm, UserFixtures.Pointer("guest"), false, false)
assert.Error(t, err)
})
t.Run("Admin", func(t *testing.T) {
@ -1524,7 +1524,7 @@ func TestUser_SaveForm(t *testing.T) {
frm.UserEmail = "admin@example.com"
frm.UserDetails.UserLocation = "GoLand"
err = Admin.SaveForm(frm, UserFixtures.Pointer("guest"), false)
err = Admin.SaveForm(frm, UserFixtures.Pointer("guest"), false, false)
assert.NoError(t, err)
assert.Equal(t, "admin@example.com", Admin.UserEmail)
@ -1549,7 +1549,7 @@ func TestUser_SaveForm(t *testing.T) {
frm.UserEmail = "admin@example.com"
frm.UserDetails.UserLocation = "GoLand"
err = Admin.SaveForm(frm, UserFixtures.Pointer("alice"), true)
err = Admin.SaveForm(frm, UserFixtures.Pointer("alice"), true, false)
assert.NoError(t, err)
assert.Equal(t, "admin@example.com", Admin.UserEmail)
@ -1573,7 +1573,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.DisplayName = "New Name"
err = Admin.SaveForm(frm, UserFixtures.Pointer("alice"), true)
err = Admin.SaveForm(frm, UserFixtures.Pointer("alice"), true, false)
assert.NoError(t, err)
assert.Equal(t, "New Name", Admin.DisplayName)
@ -1598,7 +1598,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.CanLogin = false
err = Admin.SaveForm(frm, UserFixtures.Pointer("alice"), true)
err = Admin.SaveForm(frm, UserFixtures.Pointer("alice"), true, false)
assert.NoError(t, err)
assert.False(t, Admin.CanLogin)
@ -1632,7 +1632,7 @@ func TestUser_SaveForm(t *testing.T) {
frm.UserRole = acl.RoleGuest.String()
frm.SuperAdmin = false
frm.CanLogin = false
err = Admin.SaveForm(frm, UserFixtures.Pointer("alice"), true)
err = Admin.SaveForm(frm, UserFixtures.Pointer("alice"), true, false)
assert.NoError(t, err)
assert.False(t, Admin.SuperAdmin)
@ -1666,7 +1666,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.CanLogin = false
err = Admin.SaveForm(frm, &Admin, true)
err = Admin.SaveForm(frm, &Admin, true, false)
assert.NoError(t, err)
assert.Equal(t, true, Admin.CanLogin)
@ -1688,7 +1688,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.AuthProvider = authn.ProviderNone.String()
err = Admin.SaveForm(frm, &Admin, true)
err = Admin.SaveForm(frm, &Admin, true, false)
assert.NoError(t, err)
assert.Equal(t, "local", Admin.AuthProvider)
@ -1712,7 +1712,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.AuthProvider = authn.ProviderNone.String()
err = alice.SaveForm(frm, &alice, true)
err = alice.SaveForm(frm, &alice, true, false)
assert.NoError(t, err)
assert.Equal(t, "local", alice.AuthProvider)
@ -1746,7 +1746,7 @@ func TestUser_SaveForm(t *testing.T) {
frm.SuperAdmin = false
frm.CanLogin = false
err = user.SaveForm(frm, &user, true)
err = user.SaveForm(frm, &user, true, false)
assert.NoError(t, err)
assert.Equal(t, "admin", m.UserRole)
@ -1767,7 +1767,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.UserRole = "user"
err = Admin.SaveForm(frm, UserFixtures.Pointer("guest"), false)
err = Admin.SaveForm(frm, UserFixtures.Pointer("guest"), false, false)
assert.Error(t, err)
assert.Equal(t, "super admin must not have a non-admin role", err.Error())
@ -1789,7 +1789,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.BasePath = "//*?"
err = Admin.SaveForm(frm, &Admin, true)
err = Admin.SaveForm(frm, &Admin, true, false)
assert.Error(t, err)
assert.Equal(t, "invalid base folder", err.Error())
@ -1811,7 +1811,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.UploadPath = "//*?"
err = Admin.SaveForm(frm, &Admin, true)
err = Admin.SaveForm(frm, &Admin, true, false)
assert.Error(t, err)
assert.Equal(t, "invalid upload folder", err.Error())
@ -1835,7 +1835,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.CanLogin = false
err = u.SaveForm(frm, &User{}, true)
err = u.SaveForm(frm, &User{}, true, false)
assert.NoError(t, err)
assert.False(t, u.CanLogin)
@ -1857,7 +1857,7 @@ func TestUser_SaveForm(t *testing.T) {
}
frm.CanLogin = false
err = u.SaveForm(frm, &User{}, false)
err = u.SaveForm(frm, &User{}, false, false)
assert.NoError(t, err)
assert.True(t, u.CanLogin)
@ -1865,6 +1865,49 @@ func TestUser_SaveForm(t *testing.T) {
m := FindUserByUID(u.UserUID)
assert.True(t, m.CanLogin)
})
t.Run("ClusterServicePrincipalDisables2FA", func(t *testing.T) {
// Disabling 2FA is super-admin-level; the cluster principal isn't a super admin, so
// without bySuperAdmin the auth method change would be dropped.
u := &User{UserName: "cluster-2fa-off", UserRole: acl.RoleUser.String(), CanLogin: true, AuthMethod: authn.Method2FA.String()}
if err := u.Create(); err != nil {
t.Fatal(err)
}
frm, err := u.Form()
if err != nil {
t.Fatal(err)
}
frm.AuthMethod = authn.MethodDefault.String()
err = u.SaveForm(frm, &User{}, true, true)
assert.NoError(t, err)
assert.False(t, u.Method().Is(authn.Method2FA))
m := FindUserByUID(u.UserUID)
assert.False(t, m.Method().Is(authn.Method2FA))
})
t.Run("UnprivilegedAdminKeeps2FA", func(t *testing.T) {
// A regular admin (byAdmin, not super-admin-level) must not disable another user's 2FA.
u := &User{UserName: "admin-2fa-keep", UserRole: acl.RoleUser.String(), CanLogin: true, AuthMethod: authn.Method2FA.String()}
if err := u.Create(); err != nil {
t.Fatal(err)
}
frm, err := u.Form()
if err != nil {
t.Fatal(err)
}
frm.AuthMethod = authn.MethodDefault.String()
err = u.SaveForm(frm, &User{}, true, false)
assert.NoError(t, err)
assert.True(t, u.Method().Is(authn.Method2FA))
m := FindUserByUID(u.UserUID)
assert.True(t, m.Method().Is(authn.Method2FA))
})
}
func TestUser_SetDisplayName(t *testing.T) {