diff --git a/CHANGELOG.md b/CHANGELOG.md index 785d7d855..4206bfe6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ keys remain all-access. ### Changes +- Filtering nodes by user (e.g. `headscale nodes list -u `) now resolves OIDC users by the email shown in `headscale users list`, not only the local name [#3354](https://github.com/juanfont/headscale/pull/3354) - Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324) - Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341) diff --git a/hscontrol/db/issue_3219_test.go b/hscontrol/db/issue_3219_test.go new file mode 100644 index 000000000..106fc27f9 --- /dev/null +++ b/hscontrol/db/issue_3219_test.go @@ -0,0 +1,44 @@ +package db + +import ( + "database/sql" + "testing" + + "github.com/juanfont/headscale/hscontrol/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestGetUserByNameMatchesOIDCEmail is a regression test for +// https://github.com/juanfont/headscale/issues/3219: "headscale nodes list -u +// " failed for OIDC users. "headscale users list" shows the email as +// the username (User.Username returns the Email for OIDC users), but +// GetUserByName only matched the local Name column, so filtering by the shown +// identifier returned "user not found". This mirrors the policy engine's +// Username.resolveUser, which already matches both Email and Name. +func TestGetUserByNameMatchesOIDCEmail(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + oidcUser := types.User{ + Name: "alice", + Email: "alice@example.com", + ProviderIdentifier: sql.NullString{String: "https://oidc.example.com/alice", Valid: true}, + } + require.NoError(t, db.DB.Save(&oidcUser).Error) + + // The email is what "headscale users list" displays for an OIDC user, so + // filtering by it must resolve the user. + got, err := db.GetUserByName("alice@example.com") + require.NoError(t, err) + assert.Equal(t, oidcUser.ID, got.ID) + + // The local Name still resolves as before. + got, err = db.GetUserByName("alice") + require.NoError(t, err) + assert.Equal(t, oidcUser.ID, got.ID) + + // Unknown identifiers still error out. + _, err = db.GetUserByName("does-not-exist") + assert.ErrorIs(t, err, ErrUserNotFound) +} diff --git a/hscontrol/db/users.go b/hscontrol/db/users.go index 86ee936fe..ab1acad69 100644 --- a/hscontrol/db/users.go +++ b/hscontrol/db/users.go @@ -169,9 +169,16 @@ func ListUsers(tx *gorm.DB, filter *types.User) ([]types.User, error) { // GetUserByName returns a user if the provided username is // unique, and otherwise an error. +// +// The username is matched against both the local Name and the OIDC Email, +// because [types.User.Username] (the identifier shown to operators, e.g. in +// "headscale users list") returns the Email for OIDC-provisioned users. This +// mirrors the username resolution already used by the policy engine +// (Username.resolveUser), so filters like "headscale nodes list -u " +// resolve OIDC users the same way ACLs do. func (hsdb *HSDatabase) GetUserByName(name string) (*types.User, error) { - users, err := hsdb.ListUsers(&types.User{Name: name}) - if err != nil { + var users []types.User + if err := hsdb.DB.Where("name = ? OR email = ?", name, name).Find(&users).Error; err != nil { return nil, err }