diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c26901cf..4cca5bb76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,7 @@ the ACME DNS-01 challenge through a built-in DNS provider: `acmedns`, - API errors are now RFC 7807 `application/problem+json`, including authentication failures, instead of the previous gRPC-status JSON shape [#3324](https://github.com/juanfont/headscale/pull/3324) - Errors that previously returned HTTP 500 — unknown users or nodes, malformed input, duplicate names — now return the correct 404, 400 or 409 [#3324](https://github.com/juanfont/headscale/pull/3324) - The OpenAPI document is OpenAPI 3.1 at `/api/v1/openapi.yaml` (docs at `/api/v1/docs`), replacing Swagger 2.0 at `/swagger` [#3324](https://github.com/juanfont/headscale/pull/3324) +- Renaming a user (`headscale users rename`, `POST /api/v1/user/{id}/rename/{name}`) now returns 400 for an OIDC user, whose name comes from the identity provider, and for an invalid user name; both were previously accepted - V2 hides spent, expired, and revoked auth keys. OAuth key lists need `all=true` for keys created without tokens. Tags do not limit lists. V1 and `headscale preauthkeys list` hide revoked keys. Deleting referenced ephemeral keys revokes them. Referenced rows stay. [#3619](https://github.com/juanfont/headscale/pull/3619) - Tagged OAuth clients with `all` cannot issue tokens outside their tags. OAuth retagging requires permission for all current and requested tags. It replaces the set. Disallowed tags return HTTP 400. [#3619](https://github.com/juanfont/headscale/pull/3619) diff --git a/hscontrol/api/v1/errors.go b/hscontrol/api/v1/errors.go index 91c426b2a..b83b04bdb 100644 --- a/hscontrol/api/v1/errors.go +++ b/hscontrol/api/v1/errors.go @@ -7,6 +7,7 @@ import ( "github.com/juanfont/headscale/hscontrol/db" "github.com/juanfont/headscale/hscontrol/state" "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util" "gorm.io/gorm" ) @@ -45,6 +46,10 @@ func mapError(msg string, err error) error { errors.Is(err, types.ErrHostinfoTooLarge), errors.Is(err, db.ErrUserStillHasNodes), errors.Is(err, db.ErrCannotChangeOIDCUser), + errors.Is(err, util.ErrUsernameTooShort), + errors.Is(err, util.ErrUsernameMustStartLetter), + errors.Is(err, util.ErrUsernameTooManyAt), + errors.Is(err, util.ErrUsernameInvalidChar), errors.Is(err, db.ErrPreAuthKeyNotTaggedOrOwned), errors.Is(err, db.ErrSingleUseAuthKeyHasBeenUsed), errors.Is(err, state.ErrInvalidPolicy): diff --git a/hscontrol/apiv1_users_test.go b/hscontrol/apiv1_users_test.go index 428ba21c6..dd8d171ea 100644 --- a/hscontrol/apiv1_users_test.go +++ b/hscontrol/apiv1_users_test.go @@ -5,6 +5,8 @@ import ( "net/http" "testing" + "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -68,6 +70,34 @@ func TestAPIV1RenameUser(t *testing.T) { res := h.assertParity(t, http.MethodPost, "/api/v1/user/abc/rename/bob", nil) assertStatus(t, res, http.StatusBadRequest) }) + + t.Run("oidc user refused", func(t *testing.T) { + h := newAPIV1Harness(t) + user := h.app.state.CreateUserForTest("alice") + require.NoError(t, h.app.state.DB().DB.Model(user). + Update("provider", util.RegisterMethodOIDC).Error) + + res := h.callHuma(http.MethodPost, "/api/v1/user/1/rename/bob", nil) + assertStatus(t, res, http.StatusBadRequest) + + got, err := h.app.state.GetUserByID(types.UserID(user.ID)) + require.NoError(t, err) + assert.Equal(t, "alice", got.Name) + }) + + for _, name := range []string{"b", "1bob", "bob!", "bob@@example.com"} { + t.Run("invalid name "+name+" refused", func(t *testing.T) { + h := newAPIV1Harness(t) + seedUsers("alice")(t, h.app) + + res := h.callHuma(http.MethodPost, "/api/v1/user/1/rename/"+name, nil) + assertStatus(t, res, http.StatusBadRequest) + + got, err := h.app.state.GetUserByID(1) + require.NoError(t, err) + assert.Equal(t, "alice", got.Name) + }) + } } func TestAPIV1DeleteUser(t *testing.T) { diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 08a68270a..57ef214e6 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -733,10 +733,21 @@ func (s *State) DeleteUser(userID types.UserID) (change.Change, error) { return c, nil } -// RenameUser changes a user's name. The new name must be unique. +// RenameUser changes a user's name. The new name must be unique and a valid +// username. OIDC users are refused: their name comes from the identity provider. func (s *State) RenameUser(userID types.UserID, newName string) (*types.User, change.Change, error) { return s.UpdateUser(userID, func(user *types.User) error { + err := util.ValidateUsername(newName) + if err != nil { + return err + } + + if user.Provider == util.RegisterMethodOIDC { + return hsdb.ErrCannotChangeOIDCUser + } + user.Name = newName + return nil }) }