diff --git a/pkg/models/team_members.go b/pkg/models/team_members.go index 8dd95d7bb..ac2654214 100644 --- a/pkg/models/team_members.go +++ b/pkg/models/team_members.go @@ -169,6 +169,10 @@ func (tm *TeamMember) Update(s *xorm.Session, _ web.Auth) (err error) { Where("team_id = ? AND user_id = ?", tm.TeamID, tm.UserID). Cols("admin"). Update(ttm) - tm.Admin = ttm.Admin // Since we're returning the updated permissions object + + // Carry the persisted row back onto tm so the response has id/created, keeping Username (xorm:"-"). + username := tm.Username + *tm = *ttm + tm.Username = username return } diff --git a/pkg/routes/api/v2/team_members.go b/pkg/routes/api/v2/team_members.go new file mode 100644 index 000000000..e1f1a2540 --- /dev/null +++ b/pkg/routes/api/v2/team_members.go @@ -0,0 +1,111 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package apiv2 + +import ( + "context" + "net/http" + + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/web/handler" + + "github.com/danielgtaylor/huma/v2" +) + +// RegisterTeamMemberRoutes wires team membership management onto the Huma API. +// +// Members are read through the team itself (Team.Members), so there is no +// list/read here — only add, remove and the admin toggle. +func RegisterTeamMemberRoutes(api huma.API) { + tags := []string{"teams"} + + Register(api, huma.Operation{ + OperationID: "teams-members-add", + Summary: "Add a member to a team", + Description: "Adds a user to a team by username. Only a team admin may add members.", + Method: http.MethodPost, + Path: "/teams/{team}/members", + Tags: tags, + }, teamMembersAdd) + + Register(api, huma.Operation{ + OperationID: "teams-members-remove", + Summary: "Remove a member from a team", + Description: "Removes a user from a team, revoking the access the team granted them. A team admin may remove anyone; a member may remove themselves. The last member of a team cannot be removed.", + Method: http.MethodDelete, + Path: "/teams/{team}/members/{user}", + Tags: tags, + }, teamMembersRemove) + + Register(api, huma.Operation{ + OperationID: "teams-members-toggle-admin", + Summary: "Toggle a team member's admin status", + Description: "Flips the member's admin flag: an admin becomes a regular member and vice-versa. The request body is ignored. Only a team admin may do this.", + Method: http.MethodPost, + Path: "/teams/{team}/members/{user}/admin", + DefaultStatus: http.StatusOK, + Tags: tags, + }, teamMembersToggleAdmin) +} + +func init() { AddRouteRegistrar(RegisterTeamMemberRoutes) } + +func teamMembersAdd(ctx context.Context, in *struct { + TeamID int64 `path:"team"` + Body models.TeamMember +}) (*singleBody[models.TeamMember], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + in.Body.TeamID = in.TeamID // URL wins over body + if err := handler.DoCreate(ctx, &in.Body, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.TeamMember]{Body: &in.Body}, nil +} + +func teamMembersRemove(ctx context.Context, in *struct { + TeamID int64 `path:"team"` + Username string `path:"user" doc:"The username of the member to remove."` +}) (*emptyBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + tm := &models.TeamMember{TeamID: in.TeamID, Username: in.Username} + if err := handler.DoDelete(ctx, tm, a); err != nil { + return nil, translateDomainError(err) + } + return &emptyBody{}, nil +} + +// teamMembersToggleAdmin shares v1's update pipeline (DoUpdate, the core UpdateWeb wraps). +func teamMembersToggleAdmin(ctx context.Context, in *struct { + TeamID int64 `path:"team"` + Username string `path:"user" doc:"The username of the member whose admin status to toggle."` +}) (*singleBody[models.TeamMember], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + tm := &models.TeamMember{TeamID: in.TeamID, Username: in.Username} + if err := handler.DoUpdate(ctx, tm, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.TeamMember]{Body: tm}, nil +} diff --git a/pkg/webtests/huma_team_member_test.go b/pkg/webtests/huma_team_member_test.go new file mode 100644 index 000000000..2c9550081 --- /dev/null +++ b/pkg/webtests/huma_team_member_test.go @@ -0,0 +1,257 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package webtests + +import ( + "net/http" + "testing" + + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/user" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHumaTeamMember ports the v1 model coverage (pkg/models/team_members_test.go: +// TestTeamMember_Create / _Delete / _Update) plus the permission matrix from +// CanCreate/CanDelete/CanUpdate to /api/v2. It drives the Echo+Huma stack +// directly (humaRequest/humaTokenFor) because these are action sub-paths under a +// team that webHandlerTestV2's buildURL does not model. +// +// Fixture facts (team_members.yml, teams.yml): user1 is an ADMIN of team 1 and a +// non-admin member of teams 2-8. user2 is a non-admin member of team 1. Team 9 +// lists only user2. Teams 2/3/4 each list only user1 (non-admin). Teams 14/15 +// are external (external_id set) — team 14 lists user10 and user11. +func TestHumaTeamMember(t *testing.T) { + // user11 is a member of external team 14; not pre-defined in integrations.go. + testuser11 := user.User{ + ID: 11, + Username: "user11", + Email: "user11@example.com", + Issuer: "local", + } + + t.Run("Add", func(t *testing.T) { + // v1's TestTeamMember_Create/normal: team admin adds a user by username. + t.Run("Normal", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) // admin of team 1 + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/1/members", `{"username":"user3"}`, token, "") + require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"username":"user3"`) + // user3 has id 3. + db.AssertExists(t, "team_members", map[string]interface{}{ + "team_id": 1, + "user_id": 3, + }, false) + }) + // v1's TestTeamMember_Create/normal also allows seeding an admin member. + t.Run("As admin", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/1/members", `{"username":"user3","admin":true}`, token, "") + require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"admin":true`) + db.AssertExists(t, "team_members", map[string]interface{}{ + "team_id": 1, + "user_id": 3, + "admin": true, + }, false) + }) + // v1's TestTeamMember_Create/"already existing" -> ErrUserIsMemberOfTeam (409). + t.Run("Already a member", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/1/members", `{"username":"user1"}`, token, "") + require.Equal(t, http.StatusConflict, rec.Code, "body: %s", rec.Body.String()) + }) + // v1's TestTeamMember_Create/"nonexisting user" -> ErrUserDoesNotExist (404). + t.Run("Nonexisting user", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/1/members", `{"username":"nonexistinguser"}`, token, "") + require.Equal(t, http.StatusNotFound, rec.Code, "body: %s", rec.Body.String()) + }) + // v1's TestTeamMember_Create/"empty username": required by valid: tag -> 422. + t.Run("Empty username", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/1/members", `{"username":""}`, token, "") + require.Equal(t, http.StatusUnprocessableEntity, rec.Code, "body: %s", rec.Body.String()) + }) + t.Run("Permissions check", func(t *testing.T) { + // CanCreate -> IsAdmin: a non-admin member cannot add members. + t.Run("Forbidden non-admin member", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser2) // member of team 1, not admin + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/1/members", `{"username":"user3"}`, token, "") + require.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + // A non-member is likewise forbidden (team 9, user1 not a member). + t.Run("Forbidden non-member", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/9/members", `{"username":"user3"}`, token, "") + require.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + }) + }) + + t.Run("Remove", func(t *testing.T) { + // v1's TestTeamMember_Delete/normal: team admin removes a member. + t.Run("Normal", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) // admin of team 1 + + rec := humaRequest(t, e, http.MethodDelete, "/api/v2/teams/1/members/user2", "", token, "") + require.Equal(t, http.StatusNoContent, rec.Code, "body: %s", rec.Body.String()) + assert.Empty(t, rec.Body.String()) + db.AssertMissing(t, "team_members", map[string]interface{}{ + "team_id": 1, + "user_id": 2, + }) + }) + // CanDelete grants self-removal even to a non-admin: user2 removes itself + // from team 1 (which still has user1 afterwards, so not the last member). + t.Run("Member can remove themselves", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser2) // non-admin member of team 1 + + rec := humaRequest(t, e, http.MethodDelete, "/api/v2/teams/1/members/user2", "", token, "") + require.Equal(t, http.StatusNoContent, rec.Code, "body: %s", rec.Body.String()) + db.AssertMissing(t, "team_members", map[string]interface{}{ + "team_id": 1, + "user_id": 2, + }) + }) + // Delete guard: removing the only member is refused. Team 2 lists just + // user1; CanDelete passes (self), then the last-member guard rejects it. + t.Run("Cannot remove last member", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodDelete, "/api/v2/teams/2/members/user1", "", token, "") + require.Equal(t, http.StatusBadRequest, rec.Code, "body: %s", rec.Body.String()) + }) + // Delete guard: members of an external (oidc/ldap) team cannot be removed. + // Team 14 has external_id set; user11 removing itself hits that guard. + t.Run("Cannot remove from external team", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser11) + + rec := humaRequest(t, e, http.MethodDelete, "/api/v2/teams/14/members/user11", "", token, "") + require.Equal(t, http.StatusPreconditionFailed, rec.Code, "body: %s", rec.Body.String()) + }) + t.Run("Permissions check", func(t *testing.T) { + // CanDelete -> IsAdmin (when not self): a non-admin member cannot remove + // someone else. user2 (non-admin) tries to remove user1 from team 1. + t.Run("Forbidden non-admin removing another", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser2) + + rec := humaRequest(t, e, http.MethodDelete, "/api/v2/teams/1/members/user1", "", token, "") + require.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + // A non-member is forbidden (team 9, user1 not a member). + t.Run("Forbidden non-member", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodDelete, "/api/v2/teams/9/members/user2", "", token, "") + require.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + }) + }) + + t.Run("ToggleAdmin", func(t *testing.T) { + // v1's TestTeamMember_Update/normal: the toggle flips the flag. user2 is a + // non-admin member of team 1, so toggling makes them an admin. + t.Run("Normal", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) // admin of team 1 + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/1/members/user2/admin", "", token, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"admin":true`) + db.AssertExists(t, "team_members", map[string]interface{}{ + "team_id": 1, + "user_id": 2, + "admin": true, + }, false) + }) + // v1's TestTeamMember_Update/"explicitly false in payload": the body is + // ignored and the flag toggled regardless — user1 (admin of team 1) becomes + // a non-admin despite admin:false in the payload. + t.Run("Body is ignored", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/1/members/user1/admin", `{"admin":false}`, token, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"admin":false`) + db.AssertExists(t, "team_members", map[string]interface{}{ + "team_id": 1, + "user_id": 1, + "admin": false, + }, false) + }) + t.Run("Permissions check", func(t *testing.T) { + // CanUpdate -> IsAdmin: a non-admin member cannot toggle admin status, + // not even their own. The handler checks this explicitly (non-CRUD action). + t.Run("Forbidden non-admin member", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser2) // non-admin member of team 1 + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/1/members/user2/admin", "", token, "") + require.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + // A non-member is forbidden (team 9, user1 not a member). + t.Run("Forbidden non-member", func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/teams/9/members/user2/admin", "", token, "") + require.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + }) + }) +}