From dab6ac620dc10b862e400a52b537dde954790a0b Mon Sep 17 00:00:00 2001 From: kolaente Date: Wed, 3 Jun 2026 19:14:27 +0200 Subject: [PATCH] feat(api/v2): add team CRUD endpoints Adds Team CRUD on /api/v2 mirroring the labels reference resource: list, read, create, update, delete under /teams[/{id}]. - The list op exposes an include_public query param bound onto the model so Team.ReadAll can surface public teams (gated by the instance public-teams setting). - Read ops emit an ETag and honor If-None-Match (304). - Model fields gain doc: tags; server-controlled fields are marked readOnly:true. - Self-registers via init()/AddRouteRegistrar; no routes.go change. - New webtest TestHumaTeam (named to avoid clashing with the v1 model TestTeam) covers list/read/create/update/delete plus negatives (non-member 403, nonexistent 403/404) and ETag/304. --- pkg/models/teams.go | 18 ++-- pkg/routes/api/v2/teams.go | 171 +++++++++++++++++++++++++++++++++ pkg/webtests/huma_team_test.go | 163 +++++++++++++++++++++++++++++++ 3 files changed, 343 insertions(+), 9 deletions(-) create mode 100644 pkg/routes/api/v2/teams.go create mode 100644 pkg/webtests/huma_team_test.go diff --git a/pkg/models/teams.go b/pkg/models/teams.go index 1ed143590..508bcc793 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -33,32 +33,32 @@ import ( // Team holds a team object type Team struct { // The unique, numeric id of this team. - ID int64 `xorm:"bigint autoincr not null unique pk" json:"id" param:"team"` + ID int64 `xorm:"bigint autoincr not null unique pk" json:"id" param:"team" readOnly:"true" doc:"The unique, numeric id of this team."` // The name of this team. - Name string `xorm:"varchar(250) not null" json:"name" valid:"required,runelength(1|250)" minLength:"1" maxLength:"250"` + Name string `xorm:"varchar(250) not null" json:"name" valid:"required,runelength(1|250)" minLength:"1" maxLength:"250" doc:"The name of this team."` // The team's description. Description string `xorm:"longtext null" json:"description"` CreatedByID int64 `xorm:"bigint not null INDEX" json:"-"` // The team's external id provided by the openid or ldap provider - ExternalID string `xorm:"varchar(250) null" maxLength:"250" json:"external_id"` + ExternalID string `xorm:"varchar(250) null" maxLength:"250" json:"external_id" doc:"The team's external id, set by the openid or ldap provider that created it. Read-only for clients."` // Contains the issuer extracted from the vikunja_groups claim if this team was created through oidc Issuer string `xorm:"text null" json:"-"` // The user who created this team. - CreatedBy *user.User `xorm:"-" json:"created_by"` + CreatedBy *user.User `xorm:"-" json:"created_by" readOnly:"true" doc:"The user who created this team. Set by the server."` // An array of all members in this team. - Members []*TeamUser `xorm:"-" json:"members"` + Members []*TeamUser `xorm:"-" json:"members" readOnly:"true" doc:"All members of this team. Managed through the team members endpoints, not by writing to this field."` // A timestamp when this relation was created. You cannot change this value. - Created time.Time `xorm:"created" json:"created"` + Created time.Time `xorm:"created" json:"created" readOnly:"true" doc:"A timestamp when this team was created. You cannot change this value."` // A timestamp when this relation was last updated. You cannot change this value. - Updated time.Time `xorm:"updated" json:"updated"` + Updated time.Time `xorm:"updated" json:"updated" readOnly:"true" doc:"A timestamp when this team was last updated. You cannot change this value."` // Defines wether the team should be publicly discoverable when sharing a project - IsPublic bool `xorm:"not null default false" json:"is_public"` + IsPublic bool `xorm:"not null default false" json:"is_public" doc:"Whether the team should be publicly discoverable when sharing a project. Only effective if public teams are enabled on the instance."` // Query parameter controlling whether to include public projects or not - IncludePublic bool `xorm:"-" query:"include_public" json:"include_public"` + IncludePublic bool `xorm:"-" query:"include_public" json:"include_public" doc:"When listing teams, also include public teams the user is not a member of. Only honored when public teams are enabled on the instance."` web.CRUDable `xorm:"-" json:"-"` web.Permissions `xorm:"-" json:"-"` diff --git a/pkg/routes/api/v2/teams.go b/pkg/routes/api/v2/teams.go new file mode 100644 index 000000000..5abe4f5ff --- /dev/null +++ b/pkg/routes/api/v2/teams.go @@ -0,0 +1,171 @@ +// 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" + "fmt" + "net/http" + + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/web/handler" + + "github.com/danielgtaylor/huma/v2" + "github.com/danielgtaylor/huma/v2/conditional" +) + +// models.Team.ReadAll returns []*models.Team, so that's the element type. +type teamListBody struct { + Body Paginated[*models.Team] +} + +// RegisterTeamRoutes wires Team CRUD onto the Huma API. +func RegisterTeamRoutes(api huma.API) { + tags := []string{"teams"} + + Register(api, huma.Operation{ + OperationID: "teams-list", + Summary: "List teams", + Description: "Returns the teams the authenticated user is a member of, paginated. Set include_public=true to also surface public teams the user is not a member of, where the instance has public teams enabled.", + Method: http.MethodGet, + Path: "/teams", + Tags: tags, + }, teamsList) + + Register(api, huma.Operation{ + OperationID: "teams-read", + Summary: "Get a team", + Description: "Returns a single team the user is a member of. Sends an ETag; pass it as If-None-Match on a later read to get a 304 Not Modified.", + Method: http.MethodGet, + Path: "/teams/{id}", + Tags: tags, + }, teamsRead) + + Register(api, huma.Operation{ + OperationID: "teams-create", + Summary: "Create a team", + Description: "Creates a team; the authenticated user becomes its first member and an admin of it.", + Method: http.MethodPost, + Path: "/teams", + Tags: tags, + }, teamsCreate) + + Register(api, huma.Operation{ + OperationID: "teams-update", + Summary: "Update a team", + Description: "Replaces a team's fields — only a team admin may update it. Use PATCH for a partial update.", + Method: http.MethodPut, + Path: "/teams/{id}", + Tags: tags, + }, teamsUpdate) + + Register(api, huma.Operation{ + OperationID: "teams-delete", + Summary: "Delete a team", + Description: "Deletes a team and revokes the access it granted to all of its members. Only a team admin may delete it.", + Method: http.MethodDelete, + Path: "/teams/{id}", + Tags: tags, + }, teamsDelete) +} + +func init() { AddRouteRegistrar(RegisterTeamRoutes) } + +func teamsList(ctx context.Context, in *struct { + ListParams + // IncludePublic mirrors the model's include_public query param; bound + // onto the model below so ReadAll can honor it (gated by the instance + // public-teams setting). + IncludePublic bool `query:"include_public" doc:"Also include public teams the user is not a member of. Only honored when public teams are enabled on the instance."` +}) (*teamListBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + result, _, total, err := handler.DoReadAll(ctx, &models.Team{IncludePublic: in.IncludePublic}, a, in.Q, in.Page, in.PerPage) + if err != nil { + return nil, translateDomainError(err) + } + items, ok := result.([]*models.Team) + if !ok { + return nil, fmt.Errorf("teams.ReadAll returned unexpected type %T (expected []*models.Team)", result) + } + return &teamListBody{Body: NewPaginated(items, total, in.Page, in.PerPage)}, nil +} + +func teamsRead(ctx context.Context, in *struct { + ID int64 `path:"id"` + conditional.Params +}) (*singleReadBody[models.Team], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + team := &models.Team{ID: in.ID} + if _, err := handler.DoReadOne(ctx, team, a); err != nil { + return nil, translateDomainError(err) + } + // PreconditionFailed wants the unquoted etag; response header uses RFC 9110 quoted form. + etag := fmt.Sprintf("%d-%d", team.ID, team.Updated.UnixNano()) + if in.HasConditionalParams() { + if err := in.PreconditionFailed(etag, team.Updated); err != nil { + return nil, err + } + } + return &singleReadBody[models.Team]{ETag: `"` + etag + `"`, Body: team}, nil +} + +func teamsCreate(ctx context.Context, in *struct { + Body models.Team +}) (*singleBody[models.Team], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + if err := handler.DoCreate(ctx, &in.Body, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.Team]{Body: &in.Body}, nil +} + +func teamsUpdate(ctx context.Context, in *struct { + ID int64 `path:"id"` + Body models.Team +}) (*singleBody[models.Team], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + in.Body.ID = in.ID // URL wins over body + if err := handler.DoUpdate(ctx, &in.Body, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.Team]{Body: &in.Body}, nil +} + +func teamsDelete(ctx context.Context, in *struct { + ID int64 `path:"id"` +}) (*emptyBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + if err := handler.DoDelete(ctx, &models.Team{ID: in.ID}, a); err != nil { + return nil, translateDomainError(err) + } + return &emptyBody{}, nil +} diff --git a/pkg/webtests/huma_team_test.go b/pkg/webtests/huma_team_test.go new file mode 100644 index 000000000..1c1a347e2 --- /dev/null +++ b/pkg/webtests/huma_team_test.go @@ -0,0 +1,163 @@ +// 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" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHumaTeam mirrors v1's model TestTeam shape so v2 contract parity is +// readable side-by-side. Named TestHumaTeam to avoid clashing with the v1 +// model test (pkg/models/teams_test.go TestTeam). +// +// Fixture facts (pkg/db/fixtures/team_members.yml): testuser1 is an admin of +// team 1 and a non-admin member of teams 2-8. Team 9 (created by user 7) has +// only user 2 as a member, so user1 is not a member at all. +func TestHumaTeam(t *testing.T) { + testHandler := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/teams", + idParam: "team", + t: t, + } + + t.Run("ReadAll", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testReadAllWithUser(nil, nil) + require.NoError(t, err) + // User 1 is a member of teams 1-8. + assert.Contains(t, rec.Body.String(), `testteam1`) + // User 1 is not a member of team 9 (only user 2 is). + assert.NotContains(t, rec.Body.String(), `testteam9`) + }) + }) + + t.Run("ReadOne", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testReadOneWithUser(nil, map[string]string{"team": "1"}) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"name":"testteam1"`) + assert.NotEmpty(t, rec.Result().Header.Get("ETag")) + }) + t.Run("Nonexisting", func(t *testing.T) { + // CanRead refuses non-members before existence is checked, so a + // missing team returns 403, not 404. + _, err := testHandler.testReadOneWithUser(nil, map[string]string{"team": "9999"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Permissions check", func(t *testing.T) { + t.Run("Forbidden non-member", func(t *testing.T) { + // Team 9: user1 is not a member. + _, err := testHandler.testReadOneWithUser(nil, map[string]string{"team": "9"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + }) + + t.Run("Create", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, nil, `{"name":"Lorem","description":"Ipsum"}`) + require.NoError(t, err) + assert.Equal(t, http.StatusCreated, rec.Code) + assert.Contains(t, rec.Body.String(), `"name":"Lorem"`) + assert.Contains(t, rec.Body.String(), `"description":"Ipsum"`) + }) + t.Run("Empty name", func(t *testing.T) { + // Name has minLength:1, so Huma rejects an empty name with 422 + // before the model is touched. + _, err := testHandler.testCreateWithUser(nil, nil, `{"name":""}`) + require.Error(t, err) + assert.Equal(t, http.StatusUnprocessableEntity, getHTTPErrorCode(err)) + }) + }) + + t.Run("Update", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + // Team 1: user1 is admin. + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"team": "1"}, `{"name":"TestLoremIpsum"}`) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"name":"TestLoremIpsum"`) + }) + t.Run("Nonexisting", func(t *testing.T) { + // CanUpdate -> IsAdmin -> GetTeamByID surfaces ErrTeamDoesNotExist (404). + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"team": "9999"}, `{"name":"TestLoremIpsum"}`) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Permissions check", func(t *testing.T) { + t.Run("Forbidden non-admin", func(t *testing.T) { + // Team 2: user1 is a member but not an admin. + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"team": "2"}, `{"name":"TestLoremIpsum"}`) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + }) + + t.Run("Delete", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + // Team 1: user1 is admin. + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"team": "1"}) + require.NoError(t, err) + // v2 delete is 204 No Content; v1 returned 200 + a message body. + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.Empty(t, rec.Body.String()) + }) + t.Run("Nonexisting", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"team": "9999"}) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Permissions check", func(t *testing.T) { + t.Run("Forbidden non-admin", func(t *testing.T) { + // Team 2: user1 is a member but not an admin. + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"team": "2"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + }) +} + +// TestHumaTeam_ETagReturns304 covers the v2-only conditional-request behaviour +// (ETag + If-None-Match -> 304) with no v1 counterpart. +func TestHumaTeam_ETagReturns304(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodGet, "/api/v2/teams/1", "", token, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + etag := rec.Header().Get("ETag") + require.NotEmpty(t, etag, "GET must return an ETag header") + + req := httptest.NewRequest(http.MethodGet, "/api/v2/teams/1", strings.NewReader("")) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("If-None-Match", etag) + rec = httptest.NewRecorder() + e.ServeHTTP(rec, req) + require.Equal(t, http.StatusNotModified, rec.Code, "body: %s", rec.Body.String()) +}