From 55250407dd0b6bb03e6111aefb1bb2ab08273bd1 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 14 Jun 2026 21:07:25 +0200 Subject: [PATCH] feat(org): add team visibility so org members can discover teams (#37680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #37670. Today, org members in Gitea only see teams they're a member of. In larger orgs that hurts onboarding and discoverability — there's no way to look up which team owns what without asking around. GitHub solves this with a per-team visibility setting; this PR brings the same model to Gitea. ## What changes - Every team gets a `visibility` setting: - `private` *(default)* — only team members and org owners can see the team. Same as today's behavior. - `limited` — listable by any member of the organization. Members and the repos the team has access to are visible too. Non-org-members still see nothing. - `public` — listable by any signed-in user. - The Owners team visibility is fixed and cannot be changed via settings. - Existing teams default to `private`, so this is a no-op for anyone who doesn't change anything. ## API - `Team`, `CreateTeamOption`, `EditTeamOption` all gain a `visibility` field (string enum: `private` | `limited` | `public`). - `GET /orgs/{org}/teams` and `/orgs/{org}/teams/search` now apply the same visibility rules as the web UI: - site admins and org owners still see every team - other org members see their own teams plus any `limited` or `public` team - `private` teams are no longer leaked through these endpoints - Swagger/OpenAPI specs regenerated. ## UI View from admin2 (not an owner): View from admin (owner): --------- Signed-off-by: bircni Co-authored-by: TheFox0x7 Co-authored-by: wxiaoguang --- build/generate-openapi.go | 4 +- build/openapi3gen/convert.go | 141 +++++++++++++++++++++--- build/openapi3gen/convert_test.go | 58 +++++++++- build/openapi3gen/enumscan.go | 26 +++-- build/openapi3gen/enumscan_test.go | 32 ++++-- models/db/engine.go | 19 ++-- models/db/list.go | 5 +- models/issues/pull_list.go | 2 +- models/migrations/migrations.go | 1 + models/migrations/v1_27/v337.go | 36 ++++++ models/organization/org.go | 1 + models/organization/team.go | 34 +++++- models/organization/team_list.go | 61 ++++++++-- models/organization/team_test.go | 85 ++++++++++++++ models/repo/repo_list.go | 3 +- modules/structs/org_team.go | 24 ++++ options/locale/locale_en-US.json | 8 ++ routers/api/v1/api.go | 119 ++++++++++++++------ routers/api/v1/org/team.go | 47 +++++--- routers/web/org/home.go | 21 +++- routers/web/org/teams.go | 18 ++- services/context/org.go | 48 +++++--- services/convert/convert.go | 1 + services/forms/org.go | 1 + services/org/team.go | 2 +- templates/org/team/new.tmpl | 48 +++++++- templates/org/team/sidebar.tmpl | 13 ++- templates/org/team/teams.tmpl | 11 +- templates/swagger/v1_json.tmpl | 33 ++++++ templates/swagger/v1_openapi3_json.tmpl | 32 ++++++ tests/integration/api_team_test.go | 60 ++++++++++ 31 files changed, 850 insertions(+), 144 deletions(-) create mode 100644 models/migrations/v1_27/v337.go diff --git a/build/generate-openapi.go b/build/generate-openapi.go index 3c65781481d..f0cdd866345 100644 --- a/build/generate-openapi.go +++ b/build/generate-openapi.go @@ -57,8 +57,8 @@ func main() { log.Fatalf("scanning swagger:enum annotations: %v", err) } names := make([]string, 0, len(astEnumMap)) - for _, n := range astEnumMap { - names = append(names, n) + for _, ns := range astEnumMap { + names = append(names, ns...) } sort.Strings(names) fmt.Fprintf(os.Stderr, "discovered %d swagger:enum types: %s\n", len(names), strings.Join(names, ", ")) diff --git a/build/openapi3gen/convert.go b/build/openapi3gen/convert.go index 9d446ff45b8..d1ee1a3a68f 100644 --- a/build/openapi3gen/convert.go +++ b/build/openapi3gen/convert.go @@ -6,6 +6,7 @@ package openapi3gen import ( "fmt" "regexp" + "sort" "strings" "gitea.dev/modules/json" @@ -25,10 +26,12 @@ var rxDeprecated = regexp.MustCompile(`(?i)(?:^|[\n.;])\s*deprecated\b`) // Gitea-specific post-processing: file-schema fixups, URI formats, // deprecated flags, and shared-enum extraction. // -// astEnumMap is a value-set-key → Go-type-name map (built by -// ScanSwaggerEnumTypes). If a shared enum in the spec has no entry in the -// map, Convert returns an error — no fallback naming. -func Convert(swaggerJSON []byte, astEnumMap map[string]string) (*openapi3.T, error) { +// astEnumMap is a value-set-key → Go-type-name(s) map (built by +// ScanSwaggerEnumTypes). When a value set is shared by multiple Go types, +// per-property disambiguation uses the x-go-enum-desc extension. If a shared +// enum in the spec has no matching entry, Convert returns an error — no +// fallback naming. +func Convert(swaggerJSON []byte, astEnumMap map[string][]string) (*openapi3.T, error) { var swagger2 openapi2.T if err := json.Unmarshal(swaggerJSON, &swagger2); err != nil { return nil, fmt.Errorf("parsing swagger 2.0: %w", err) @@ -176,12 +179,24 @@ type enumUsage struct { // If the derived enum name collides with an existing component schema, or // no // swagger:enum annotation matches the value set, generation aborts // with an actionable error — there are no silent fallbacks. -func extractSharedEnums(doc *openapi3.T, astEnumMap map[string]string) error { +func extractSharedEnums(doc *openapi3.T, astEnumMap map[string][]string) error { if doc.Components == nil { return nil } - enumGroups := map[string][]enumUsage{} + type groupKey struct { + valueSet string + typeName string + } + enumGroups := map[groupKey][]enumUsage{} + groupOrder := []groupKey{} // deterministic iteration + + addUsage := func(key groupKey, u enumUsage) { + if _, seen := enumGroups[key]; !seen { + groupOrder = append(groupOrder, key) + } + enumGroups[key] = append(enumGroups[key], u) + } for schemaName, schemaRef := range doc.Components.Schemas { if schemaRef.Value == nil { @@ -192,24 +207,31 @@ func extractSharedEnums(doc *openapi3.T, astEnumMap map[string]string) error { continue } if len(propRef.Value.Enum) > 1 && propRef.Value.Type.Is("string") { - key := EnumKey(propRef.Value.Enum) - enumGroups[key] = append(enumGroups[key], enumUsage{schemaName, propName, propRef, false}) + key := groupKey{ + valueSet: EnumKey(propRef.Value.Enum), + typeName: extractEnumTypeName(propRef.Value, astEnumMap), + } + addUsage(key, enumUsage{schemaName, propName, propRef, false}) } if propRef.Value.Type.Is("array") && propRef.Value.Items != nil && propRef.Value.Items.Value != nil && propRef.Value.Items.Ref == "" && len(propRef.Value.Items.Value.Enum) > 1 && propRef.Value.Items.Value.Type.Is("string") { - key := EnumKey(propRef.Value.Items.Value.Enum) - enumGroups[key] = append(enumGroups[key], enumUsage{schemaName, propName, propRef, true}) + key := groupKey{ + valueSet: EnumKey(propRef.Value.Items.Value.Enum), + typeName: extractEnumTypeName(propRef.Value.Items.Value, astEnumMap), + } + addUsage(key, enumUsage{schemaName, propName, propRef, true}) } } } - for key, usages := range enumGroups { + for _, key := range groupOrder { + usages := enumGroups[key] if len(usages) < 2 { continue } - enumName, err := deriveEnumName(key, usages, astEnumMap) + enumName, err := deriveEnumName(key.valueSet, key.typeName, usages, astEnumMap) if err != nil { return err } @@ -257,12 +279,17 @@ func extractSharedEnums(doc *openapi3.T, astEnumMap map[string]string) error { return nil } -// deriveEnumName looks up a shared enum's Go type name from astEnumMap by -// value-set key. If no annotation matches, returns an error identifying the -// offending properties and the fix. -func deriveEnumName(key string, usages []enumUsage, astEnumMap map[string]string) (string, error) { - if name, ok := astEnumMap[key]; ok { - return name, nil +// deriveEnumName looks up a shared enum's Go type name. If typeName is +// non-empty (because we recovered it from x-go-enum-desc), it is used +// directly. Otherwise the value-set must map to exactly one known type. On +// failure, returns an error identifying the offending properties. +func deriveEnumName(key, typeName string, usages []enumUsage, astEnumMap map[string][]string) (string, error) { + if typeName != "" { + return typeName, nil + } + names := astEnumMap[key] + if len(names) == 1 { + return names[0], nil } props := map[string]bool{} @@ -273,9 +300,87 @@ func deriveEnumName(key string, usages []enumUsage, astEnumMap map[string]string for p := range props { propList = append(propList, p) } + if len(names) > 1 { + return "", fmt.Errorf( + "value-set %q is shared by multiple swagger:enum types %v and could not be disambiguated for properties: %v; "+ + "ensure go-swagger emits x-go-enum-desc for those properties", + key, names, propList, + ) + } return "", fmt.Errorf( "no swagger:enum annotation matches value-set %q used by %d properties: %v; "+ "fix by adding a named string type with // swagger:enum to modules/structs or modules/commitstatus", key, len(usages), propList, ) } + +// extractEnumTypeName recovers the Go type name a schema's enum came from by +// parsing the property's x-go-enum-desc extension. go-swagger emits one line +// per value as " [ ]"; the type is the longest +// common prefix of the const names, narrowed to the candidate set in +// astEnumMap. Returns "" if extraction is inconclusive. +func extractEnumTypeName(s *openapi3.Schema, astEnumMap map[string][]string) string { + if s == nil || s.Extensions == nil { + return "" + } + raw, ok := s.Extensions["x-go-enum-desc"] + if !ok { + return "" + } + desc, ok := raw.(string) + if !ok { + return "" + } + candidates := astEnumMap[EnumKey(s.Enum)] + if len(candidates) == 0 { + return "" + } + // Collect the const names (second whitespace-separated field per line). + var consts []string + for line := range strings.SplitSeq(desc, "\n") { + fields := strings.Fields(line) + if len(fields) >= 2 { + consts = append(consts, fields[1]) + } + } + if len(consts) == 0 { + return "" + } + // A candidate matches when it is a prefix of every const name AND the + // first character after the prefix is an uppercase ASCII letter — this + // rejects e.g. "Alpha" matching "Alphabet" (suffix "bet" starts lower) + // while still accepting both "Alpha" and "AlphaPlus" against "AlphaPlusX" + // (both prefixes valid). The most specific (longest) wins; ties return + // "" so deriveEnumName surfaces the ambiguity rather than silently + // picking a winner. + ordered := append([]string(nil), candidates...) + sort.Slice(ordered, func(i, j int) bool { return len(ordered[i]) > len(ordered[j]) }) + var matches []string + for _, name := range ordered { + ok := true + for _, c := range consts { + if !strings.HasPrefix(c, name) { + ok = false + break + } + suffix := c[len(name):] + // Empty suffix means the const name exactly equals the type name — valid exact match. + // A non-empty suffix must begin with an uppercase letter to reject incidental + // prefix matches (e.g. "Alpha" should not match "Alphabet"). + if len(suffix) > 0 && (suffix[0] < 'A' || suffix[0] > 'Z') { + ok = false + break + } + } + if ok { + matches = append(matches, name) + } + } + if len(matches) == 0 { + return "" + } + if len(matches) > 1 && len(matches[0]) == len(matches[1]) { + return "" + } + return matches[0] +} diff --git a/build/openapi3gen/convert_test.go b/build/openapi3gen/convert_test.go index a9a715e6c2a..1cf73bd1f0e 100644 --- a/build/openapi3gen/convert_test.go +++ b/build/openapi3gen/convert_test.go @@ -12,9 +12,9 @@ import ( func TestDeriveEnumName_hit(t *testing.T) { key := EnumKey([]any{"red", "green", "blue"}) - astMap := map[string]string{key: "Color"} + astMap := map[string][]string{key: {"Color"}} usages := []enumUsage{{schemaName: "Paint", propName: "color"}} - got, err := deriveEnumName(key, usages, astMap) + got, err := deriveEnumName(key, "", usages, astMap) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -26,7 +26,7 @@ func TestDeriveEnumName_hit(t *testing.T) { func TestDeriveEnumName_miss(t *testing.T) { key := EnumKey([]any{"x", "y"}) usages := []enumUsage{{schemaName: "Thing", propName: "kind"}} - _, err := deriveEnumName(key, usages, map[string]string{}) + _, err := deriveEnumName(key, "", usages, map[string][]string{}) if err == nil { t.Fatal("expected miss error, got nil") } @@ -64,7 +64,7 @@ func TestExtractSharedEnums_usesASTMap(t *testing.T) { }, }, } - astMap := map[string]string{EnumKey([]any{"red", "green", "blue"}): "Color"} + astMap := map[string][]string{EnumKey([]any{"red", "green", "blue"}): {"Color"}} if err := extractSharedEnums(doc, astMap); err != nil { t.Fatalf("extractSharedEnums: %v", err) } @@ -139,6 +139,54 @@ func TestFixFileSchemas_recursesIntoNested(t *testing.T) { } } +func TestExtractEnumTypeName_TeamVisibility(t *testing.T) { + enum := []any{"public", "limited", "private"} + key := EnumKey(enum) + astMap := map[string][]string{key: {"UserVisibility", "TeamVisibility"}} + schema := &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + Enum: enum, + Extensions: map[string]any{ + "x-go-enum-desc": "public TeamVisibilityPublic\nlimited TeamVisibilityLimited\nprivate TeamVisibilityPrivate", + }, + } + if got := extractEnumTypeName(schema, astMap); got != "TeamVisibility" { + t.Fatalf("got %q, want %q", got, "TeamVisibility") + } +} + +func TestExtractEnumTypeName_ambiguousPrefixTie(t *testing.T) { + enum := []any{"one", "two"} + key := EnumKey(enum) + astMap := map[string][]string{key: {"AB", "AC"}} + schema := &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + Enum: enum, + Extensions: map[string]any{ + "x-go-enum-desc": "one ABOne\ntwo ACTwo", + }, + } + if got := extractEnumTypeName(schema, astMap); got != "" { + t.Fatalf("got %q, want empty string for ambiguous tie", got) + } +} + +func TestExtractEnumTypeName_rejectsIncidentalPrefix(t *testing.T) { + enum := []any{"a", "b"} + key := EnumKey(enum) + astMap := map[string][]string{key: {"Alpha", "Alphabet"}} + schema := &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + Enum: enum, + Extensions: map[string]any{ + "x-go-enum-desc": "a AlphabetA\nb AlphabetB", + }, + } + if got := extractEnumTypeName(schema, astMap); got != "Alphabet" { + t.Fatalf("got %q, want %q", got, "Alphabet") + } +} + func TestExtractSharedEnums_missReturnsError(t *testing.T) { doc := &openapi3.T{ Components: &openapi3.Components{ @@ -164,7 +212,7 @@ func TestExtractSharedEnums_missReturnsError(t *testing.T) { }, }, } - if err := extractSharedEnums(doc, map[string]string{}); err == nil { + if err := extractSharedEnums(doc, map[string][]string{}); err == nil { t.Fatal("expected miss error") } } diff --git a/build/openapi3gen/enumscan.go b/build/openapi3gen/enumscan.go index dd116205493..73bbcb5e150 100644 --- a/build/openapi3gen/enumscan.go +++ b/build/openapi3gen/enumscan.go @@ -34,13 +34,16 @@ func EnumKey(values []any) string { var rxSwaggerEnum = regexp.MustCompile(`swagger:enum\s+(\w+)`) // ScanSwaggerEnumTypes walks .go files under each dir and returns a map from -// a canonical value-set key (see EnumKey) to the Go type name declared with -// // swagger:enum TypeName. +// a canonical value-set key (see EnumKey) to the Go type names declared with +// // swagger:enum TypeName. Multiple type names per key are allowed (e.g. +// distinct enum types that happen to share a value set such as +// {public, limited, private}); callers must disambiguate per-usage (typically +// by parsing the property's x-go-enum-desc extension to recover the const +// type prefix). // -// Returns an error on parse failure, on an annotation for a type whose -// constants can't be extracted, or on value-set collisions between two -// different enum types. -func ScanSwaggerEnumTypes(dirs []string) (map[string]string, error) { +// Returns an error on parse failure or on an annotation for a type whose +// constants can't be extracted. +func ScanSwaggerEnumTypes(dirs []string) (map[string][]string, error) { fset := token.NewFileSet() parsed := []*ast.File{} @@ -92,17 +95,18 @@ func ScanSwaggerEnumTypes(dirs []string) (map[string]string, error) { } } - result := map[string]string{} + result := map[string][]string{} for typeName := range enumTypes { values, ok := enumValues[typeName] if !ok || len(values) == 0 { return nil, fmt.Errorf("swagger:enum %s has no const block with typed string values", typeName) } key := EnumKey(values) - if existing, ok := result[key]; ok && existing != typeName { - return nil, fmt.Errorf("swagger:enum value-set collision: %s and %s both use %q", existing, typeName, key) - } - result[key] = typeName + result[key] = append(result[key], typeName) + } + for key, names := range result { + sort.Strings(names) + result[key] = names } return result, nil } diff --git a/build/openapi3gen/enumscan_test.go b/build/openapi3gen/enumscan_test.go index 2e5fe99db0f..8cb16b721d1 100644 --- a/build/openapi3gen/enumscan_test.go +++ b/build/openapi3gen/enumscan_test.go @@ -6,10 +6,19 @@ package openapi3gen import ( "os" "path/filepath" + "slices" "strings" "testing" ) +func single(got map[string][]string, key string) string { + v := got[key] + if len(v) != 1 { + return "" + } + return v[0] +} + func TestEnumKey_sortsAndJoins(t *testing.T) { key := EnumKey([]any{"b", "a", "c"}) if key != "a|b|c" { @@ -47,7 +56,7 @@ const ( t.Fatalf("ScanSwaggerEnumTypes: %v", err) } wantKey := EnumKey([]any{"red", "green", "blue"}) - if got[wantKey] != "Color" { + if single(got, wantKey) != "Color" { t.Fatalf("map[%q] = %q, want %q", wantKey, got[wantKey], "Color") } } @@ -98,13 +107,14 @@ const ( t.Fatal(err) } - _, err := ScanSwaggerEnumTypes([]string{dir}) - if err == nil { - t.Fatal("expected collision error, got nil") + got, err := ScanSwaggerEnumTypes([]string{dir}) + if err != nil { + t.Fatalf("ScanSwaggerEnumTypes: %v", err) } - msg := err.Error() - if !strings.Contains(msg, "Alpha") || !strings.Contains(msg, "Beta") { - t.Fatalf("error %q should mention both Alpha and Beta", msg) + key := EnumKey([]any{"x", "y"}) + names := got[key] + if !slices.Equal(names, []string{"Alpha", "Beta"}) { + t.Fatalf("map[%q] = %v, want [Alpha Beta]", key, names) } } @@ -168,7 +178,7 @@ type Hue string t.Fatalf("ScanSwaggerEnumTypes: %v", err) } wantKey := EnumKey([]any{"a", "b"}) - if got[wantKey] != "Hue" { + if single(got, wantKey) != "Hue" { t.Fatalf("map[%q] = %q, want %q", wantKey, got[wantKey], "Hue") } } @@ -194,7 +204,7 @@ type Shade string t.Fatalf("ScanSwaggerEnumTypes: %v", err) } wantKey := EnumKey([]any{"dark", "light"}) - if got[wantKey] != "Shade" { + if single(got, wantKey) != "Shade" { t.Fatalf("map[%q] = %q, want %q", wantKey, got[wantKey], "Shade") } } @@ -230,10 +240,10 @@ const ( } colorKey := EnumKey([]any{"red", "blue"}) shadeKey := EnumKey([]any{"dark", "light"}) - if got[colorKey] != "Color" { + if single(got, colorKey) != "Color" { t.Fatalf("Color: map[%q] = %q, want %q", colorKey, got[colorKey], "Color") } - if got[shadeKey] != "Shade" { + if single(got, shadeKey) != "Shade" { t.Fatalf("Shade: map[%q] = %q, want %q", shadeKey, got[shadeKey], "Shade") } } diff --git a/models/db/engine.go b/models/db/engine.go index d4ac0b4aca0..ef0636ca607 100755 --- a/models/db/engine.go +++ b/models/db/engine.go @@ -28,9 +28,8 @@ var ( registeredInitFuncs []func() error ) -// Engine represents a xorm engine or session. -type Engine interface { - Table(tableNameOrBean any) *xorm.Session +// SQLSession represents a common interface for engine and session to execute SQLs +type SQLSession interface { Count(...any) (int64, error) Decr(column string, arg ...any) *xorm.Session Delete(...any) (int64, error) @@ -52,7 +51,6 @@ type Engine interface { Limit(limit int, start ...int) *xorm.Session NoAutoTime() *xorm.Session SumInt(bean any, columnName string) (res int64, err error) - Sync(...any) error Select(string) *xorm.Session SetExpr(string, any) *xorm.Session NotIn(string, ...any) *xorm.Session @@ -61,12 +59,20 @@ type Engine interface { Distinct(...string) *xorm.Session Query(...any) ([]map[string][]byte, error) Cols(...string) *xorm.Session + Table(tableNameOrBean any) *xorm.Session Context(ctx context.Context) *xorm.Session - Ping() error + QueryInterface(sqlOrArgs ...any) ([]map[string]any, error) IsTableExist(tableNameOrBean any) (bool, error) } -// Session represents a xorm session interface, used as an abstraction over *xorm.Session. +// Engine represents a xorm engine +type Engine interface { + SQLSession + Sync(...any) error + Ping() error +} + +// Session represents a xorm session interface type Session interface { Engine And(query any, args ...any) *xorm.Session @@ -89,7 +95,6 @@ type EngineMigration interface { Dialect() dialects.Dialect DropTables(beans ...any) error NewSession() *xorm.Session - QueryInterface(sqlOrArgs ...any) ([]map[string]any, error) SetMapper(mapper names.Mapper) SyncWithOptions(opts xorm.SyncOptions, beans ...any) (*xorm.SyncResult, error) TableInfo(bean any) (*schemas.Table, error) diff --git a/models/db/list.go b/models/db/list.go index 47c163c1d72..e91a7054112 100644 --- a/models/db/list.go +++ b/models/db/list.go @@ -24,10 +24,9 @@ type Paginator interface { } // SetSessionPagination sets pagination for a database session -func SetSessionPagination(sess Engine, p Paginator) Session { +func SetSessionPagination(sess Engine, p Paginator) { skip, take := p.GetSkipTake() - - return sess.Limit(take, skip) + sess.Limit(take, skip) } // ListOptions options to paginate results diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go index f271850a147..b734c81af9c 100644 --- a/models/issues/pull_list.go +++ b/models/issues/pull_list.go @@ -181,7 +181,7 @@ func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptio findSession := listPullRequestStatement(ctx, baseRepoID, opts) applySorts(findSession, opts.SortType, 0) - findSession = db.SetSessionPagination(findSession, opts) + db.SetSessionPagination(findSession, opts) prs := make([]*PullRequest, 0, opts.PageSize) found := findSession.Find(&prs) return prs, maxResults, found diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index aedd679c57f..81a618a2076 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -414,6 +414,7 @@ func prepareMigrationTasks() []*migration { newMigration(334, "Add cancelling support to action runners", v1_27.AddCancellingSupportToActionRunner), newMigration(335, "Add reusable workflow fields and action_run_attempt_job_id_index table for ActionRunJob", v1_27.AddReusableWorkflowFieldsToActionRunJob), newMigration(336, "Add ActionRunJobSummary table", v1_27.AddActionRunJobSummaryTable), + newMigration(337, "Add visibility to team", v1_27.AddVisibilityToTeam), } return preparedMigrations } diff --git a/models/migrations/v1_27/v337.go b/models/migrations/v1_27/v337.go new file mode 100644 index 00000000000..61fd2445ccf --- /dev/null +++ b/models/migrations/v1_27/v337.go @@ -0,0 +1,36 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_27 + +import ( + "gitea.dev/models/db" + + "xorm.io/xorm" +) + +type VisibleType int + +type teamWithVisibility struct { + Visibility VisibleType `xorm:"NOT NULL DEFAULT 2"` +} + +func (teamWithVisibility) TableName() string { + return "team" +} + +func AddVisibilityToTeam(x db.EngineMigration) error { + if _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreDropIndices: true, + IgnoreConstrains: true, + }, new(teamWithVisibility)); err != nil { + return err + } + + // Owner teams must remain listable to all org members; new orgs create + // them as "limited", so make existing owner teams limited too. + // Filter on authorize=4 (AccessModeOwner) so a user-created team that + // happens to share the name "owners" is not accidentally affected. + _, err := x.Exec("UPDATE `team` SET visibility = ? WHERE lower_name = ? AND authorize = ?", 1, "owners", 4) + return err +} diff --git a/models/organization/org.go b/models/organization/org.go index c2f77a83dde..9a28aa39181 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -370,6 +370,7 @@ func CreateOrganization(ctx context.Context, org *Organization, owner *user_mode NumMembers: 1, IncludesAllRepositories: true, CanCreateOrgRepo: true, + Visibility: structs.VisibleTypeLimited, } if err = db.Insert(ctx, t); err != nil { return fmt.Errorf("insert owner team: %w", err) diff --git a/models/organization/team.go b/models/organization/team.go index fd6365e7319..ea2f52a1edb 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -14,6 +14,7 @@ import ( "gitea.dev/models/unit" user_model "gitea.dev/models/user" "gitea.dev/modules/log" + "gitea.dev/modules/structs" "gitea.dev/modules/util" "xorm.io/builder" @@ -81,9 +82,36 @@ type Team struct { Members []*user_model.User `xorm:"-"` NumRepos int NumMembers int - Units []*TeamUnit `xorm:"-"` - IncludesAllRepositories bool `xorm:"NOT NULL DEFAULT false"` - CanCreateOrgRepo bool `xorm:"NOT NULL DEFAULT false"` + Units []*TeamUnit `xorm:"-"` + IncludesAllRepositories bool `xorm:"NOT NULL DEFAULT false"` + CanCreateOrgRepo bool `xorm:"NOT NULL DEFAULT false"` + Visibility structs.VisibleType `xorm:"NOT NULL DEFAULT 2"` +} + +func (t *Team) IsPublic() bool { return t.Visibility.IsPublic() } +func (t *Team) IsLimited() bool { return t.Visibility.IsLimited() } +func (t *Team) IsPrivate() bool { return t.Visibility.IsPrivate() } + +// CanNonMemberReadMeta reports whether a non-member, non-owner doer may read +// the team's metadata, based on the team's visibility tier and the parent org's +// visibility. Privileged callers (site admins, org owners, team members) are +// decided by the caller before reaching here. +func (t *Team) CanNonMemberReadMeta(ctx context.Context, org, doer *user_model.User) (bool, error) { + switch t.Visibility { + case structs.VisibleTypePublic: + return HasOrgOrUserVisible(ctx, org, doer), nil + case structs.VisibleTypeLimited: + return IsOrganizationMember(ctx, t.OrgID, doer.ID) + default: + return false, nil + } +} + +func NormalizeTeamVisibility(s string) structs.VisibleType { + if vt, ok := structs.VisibilityModes[s]; ok { + return vt + } + return structs.VisibleTypePrivate } func init() { diff --git a/models/organization/team_list.go b/models/organization/team_list.go index 64b084eaaaf..bb2f9f224d1 100644 --- a/models/organization/team_list.go +++ b/models/organization/team_list.go @@ -10,6 +10,8 @@ import ( "gitea.dev/models/db" "gitea.dev/models/perm" "gitea.dev/models/unit" + user_model "gitea.dev/models/user" + "gitea.dev/modules/structs" "xorm.io/builder" ) @@ -50,9 +52,15 @@ type SearchTeamOptions struct { Keyword string OrgID int64 IncludeDesc bool + // IncludeVisibilities, when combined with UserID, also returns teams whose + // visibility is in this list, even if UserID is not a member. Typical values: + // - {limited,public} for org members + // - {public} for signed-in users who are not org members + // Leave empty to return only teams the user is a member of. + IncludeVisibilities []structs.VisibleType } -func (opts *SearchTeamOptions) toCond() builder.Cond { +func (opts *SearchTeamOptions) applyToSession(sess db.SQLSession) { cond := builder.NewCond() if len(opts.Keyword) > 0 { @@ -68,11 +76,51 @@ func (opts *SearchTeamOptions) toCond() builder.Cond { cond = cond.And(builder.Eq{"`team`.org_id": opts.OrgID}) } - if opts.UserID > 0 { + switch { + case opts.UserID > 0 && len(opts.IncludeVisibilities) > 0: + sess = sess.Join("LEFT", "team_user", "team_user.team_id = team.id AND team_user.uid = ?", opts.UserID) + cond = cond.And(builder.Or( + builder.Eq{"team_user.uid": opts.UserID}, + builder.In("`team`.visibility", opts.IncludeVisibilities), + )) + case opts.UserID > 0: + sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id") cond = cond.And(builder.Eq{"team_user.uid": opts.UserID}) + case len(opts.IncludeVisibilities) > 0: + cond = cond.And(builder.In("`team`.visibility", opts.IncludeVisibilities)) } + sess.Where(cond) +} - return cond +func VisibleTeamVisibilitiesFor(isOrgMember, isSignedIn bool) []structs.VisibleType { + switch { + case isOrgMember: + return []structs.VisibleType{structs.VisibleTypeLimited, structs.VisibleTypePublic} + case isSignedIn: + return []structs.VisibleType{structs.VisibleTypePublic} + default: + return nil + } +} + +func ApplyTeamListFilter(ctx context.Context, orgID int64, viewer *user_model.User, isSignedIn bool, opts *SearchTeamOptions) error { + if viewer.IsAdmin { + return nil + } + isOwner, err := IsOrganizationOwner(ctx, orgID, viewer.ID) + if err != nil { + return err + } + if isOwner { + return nil + } + isOrgMember, err := IsOrganizationMember(ctx, orgID, viewer.ID) + if err != nil { + return err + } + opts.UserID = viewer.ID + opts.IncludeVisibilities = VisibleTeamVisibilitiesFor(isOrgMember, isSignedIn) + return nil } // SearchTeam search for teams. Caller is responsible to check permissions. @@ -80,15 +128,12 @@ func SearchTeam(ctx context.Context, opts *SearchTeamOptions) (TeamList, int64, sess := db.GetEngine(ctx) opts.SetDefaultValues() - cond := opts.toCond() + opts.applyToSession(sess) - if opts.UserID > 0 { - sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id") - } db.SetSessionPagination(sess, opts) teams := make([]*Team, 0, opts.PageSize) - count, err := sess.Where(cond).OrderBy("CASE WHEN name=? THEN '' ELSE lower_name END", OwnerTeamName).FindAndCount(&teams) + count, err := sess.OrderBy("CASE WHEN name=? THEN '' ELSE lower_name END", OwnerTeamName).FindAndCount(&teams) if err != nil { return nil, 0, err } diff --git a/models/organization/team_test.go b/models/organization/team_test.go index aa1cc90caa3..30fe6e82290 100644 --- a/models/organization/team_test.go +++ b/models/organization/team_test.go @@ -10,6 +10,8 @@ import ( "gitea.dev/models/organization" repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" + user_model "gitea.dev/models/user" + "gitea.dev/modules/structs" "github.com/stretchr/testify/assert" ) @@ -38,6 +40,43 @@ func TestTeam_IsMember(t *testing.T) { assert.False(t, team.IsMember(t.Context(), unittest.NonexistentID)) } +func TestTeam_CanNonMemberReadMeta(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) // public org + org35 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 35}) // private org + member := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // member of org 3 and org 35 + outsider := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) // member of neither org + + test := func(name string, team *organization.Team, org, doer *user_model.User, expected bool) { + t.Run(name, func(t *testing.T) { + ok, err := team.CanNonMemberReadMeta(t.Context(), org, doer) + assert.NoError(t, err) + assert.Equal(t, expected, ok) + }) + } + + // Public team is gated only by the parent org's visibility. + publicTeam := &organization.Team{OrgID: 3, Visibility: structs.VisibleTypePublic} + test("public team, public org, member", publicTeam, org3, member, true) + test("public team, public org, outsider", publicTeam, org3, outsider, true) + + // Public team inside a private org: only org members may see it. + publicTeamPrivOrg := &organization.Team{OrgID: 35, Visibility: structs.VisibleTypePublic} + test("public team, private org, org member", publicTeamPrivOrg, org35, member, true) + test("public team, private org, outsider", publicTeamPrivOrg, org35, outsider, false) + + // Limited team: any org member, but never outsiders. + limitedTeam := &organization.Team{OrgID: 3, Visibility: structs.VisibleTypeLimited} + test("limited team, org member", limitedTeam, org3, member, true) + test("limited team, outsider", limitedTeam, org3, outsider, false) + + // Private team is never visible to non-members; members/owners are admitted by the caller. + privateTeam := &organization.Team{OrgID: 3, Visibility: structs.VisibleTypePrivate} + test("private team, org member", privateTeam, org3, member, false) + test("private team, outsider", privateTeam, org3, outsider, false) +} + func TestTeam_GetRepositories(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) @@ -172,6 +211,52 @@ func TestGetUserOrgTeams(t *testing.T) { test(3, unittest.NonexistentID) } +func TestSearchTeamIncludeVisible(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + const orgID int64 = 3 + // User 5 is an org member but only belongs to team 1 (Owners) — make sure + // they don't see team 2 (default private) but do see a freshly added + // limited team they are not a member of. + visible := &organization.Team{ + OrgID: orgID, + LowerName: "visible-team", + Name: "visible-team", + AccessMode: 1, // read + Visibility: structs.VisibleTypeLimited, + } + assert.NoError(t, db.Insert(t.Context(), visible)) + teams, _, err := organization.SearchTeam(t.Context(), &organization.SearchTeamOptions{ + OrgID: orgID, + UserID: 2, + IncludeVisibilities: organization.VisibleTeamVisibilitiesFor(true, true), + }) + assert.NoError(t, err) + ids := make(map[int64]bool, len(teams)) + for _, team := range teams { + assert.Equal(t, orgID, team.OrgID) + ids[team.ID] = true + } + // user 2 is in team 1 and team 2 in org 3, plus should see the new visible team. + assert.True(t, ids[1], "expected to see team 1 (member)") + assert.True(t, ids[2], "expected to see team 2 (member)") + assert.True(t, ids[visible.ID], "expected to see visible team") + + // user 5 is only an org member in team 1, must not see secret team 2 but must see the visible one. + teams, _, err = organization.SearchTeam(t.Context(), &organization.SearchTeamOptions{ + OrgID: orgID, + UserID: 5, + IncludeVisibilities: organization.VisibleTeamVisibilitiesFor(true, true), + }) + assert.NoError(t, err) + ids = make(map[int64]bool, len(teams)) + for _, team := range teams { + ids[team.ID] = true + } + assert.False(t, ids[2], "user 5 must not see private team 2") + assert.True(t, ids[visible.ID], "user 5 must see the limited team") +} + func TestHasTeamRepo(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index d0e5dd8f60f..57f9e788332 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -783,7 +783,8 @@ func GetUserRepositories(ctx context.Context, opts SearchRepoOptions) (Repositor sess = sess.Where(cond).OrderBy(opts.OrderBy.String()) repos := make(RepositoryList, 0, opts.PageSize) - return repos, count, db.SetSessionPagination(sess, &opts).Find(&repos) + db.SetSessionPagination(sess, &opts) + return repos, count, sess.Find(&repos) } func GetOwnerRepositoriesByIDs(ctx context.Context, ownerID int64, repoIDs []int64) (RepositoryList, error) { diff --git a/modules/structs/org_team.go b/modules/structs/org_team.go index 20959931d33..c9542951a02 100644 --- a/modules/structs/org_team.go +++ b/modules/structs/org_team.go @@ -4,6 +4,20 @@ package structs +// TeamVisibility controls who can list a team within its organization. +// - "public": visible to any signed-in user (still bounded by org visibility) +// - "limited": visible to any member of the parent organization +// - "private": visible only to team members and org owners +// +// swagger:enum TeamVisibility +type TeamVisibility string + +const ( + TeamVisibilityPublic TeamVisibility = "public" + TeamVisibilityLimited TeamVisibility = "limited" + TeamVisibilityPrivate TeamVisibility = "private" +) + // Team represents a team in an organization type Team struct { // The unique identifier of the team @@ -24,6 +38,11 @@ type Team struct { UnitsMap map[string]string `json:"units_map"` // Whether the team can create repositories in the organization CanCreateOrgRepo bool `json:"can_create_org_repo"` + // Team visibility within the organization. "private" teams are only + // listable by members and org owners; "limited" teams are listable by + // any organization member; "public" teams are listable by any signed-in + // user. + Visibility TeamVisibility `json:"visibility"` } // CreateTeamOption options for creating a team @@ -42,6 +61,8 @@ type CreateTeamOption struct { UnitsMap map[string]string `json:"units_map"` // Whether the team can create repositories in the organization CanCreateOrgRepo bool `json:"can_create_org_repo"` + // Team visibility within the organization. Defaults to "private". + Visibility TeamVisibility `json:"visibility" binding:"OmitEmpty;In(public,limited,private)"` } // EditTeamOption options for editing a team @@ -60,4 +81,7 @@ type EditTeamOption struct { UnitsMap map[string]string `json:"units_map"` // Whether the team can create repositories in the organization CanCreateOrgRepo *bool `json:"can_create_org_repo"` + // Team visibility within the organization. When omitted, visibility is + // left unchanged. + Visibility *TeamVisibility `json:"visibility" binding:"OmitEmpty;In(public,limited,private)"` } diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 90c7c71fb7d..f6e71cccb3e 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -2865,6 +2865,14 @@ "org.teams.all_repositories_read_permission_desc": "This team grants Read access to all repositories: members can view and clone repositories.", "org.teams.all_repositories_write_permission_desc": "This team grants Write access to all repositories: members can read from and push to repositories.", "org.teams.all_repositories_admin_permission_desc": "This team grants Admin access to all repositories: members can read from, push to and add collaborators to repositories.", + "org.teams.visibility": "Visibility", + "org.teams.visibility_private": "Private", + "org.teams.visibility_private_helper": "Visible only to team members and organization owners.", + "org.teams.visibility_limited": "Limited", + "org.teams.visibility_limited_helper": "Visible to all members of this organization.", + "org.teams.visibility_public": "Public", + "org.teams.visibility_public_helper": "Visible to any signed-in user.", + "org.teams.owners_visibility_fixed": "The Owners team visibility cannot be changed.", "org.teams.invite.title": "You have been invited to join team %s in organization %s.", "org.teams.invite.by": "Invited by %s", "org.teams.invite.description": "Please click the button below to join the team.", diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 4715ca1d672..02cabc55d16 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -505,41 +505,79 @@ func reqOrgOwnership() func(ctx *context.APIContext) { } } -// reqTeamMembership user should be an team member, or a site admin -func reqTeamMembership() func(ctx *context.APIContext) { +func teamAccessPrivileged(ctx *context.APIContext) (orgID int64, privileged, ok bool) { + if ctx.IsUserSiteAdmin() { + return 0, true, true + } + if ctx.Org.Team == nil { + setting.PanicInDevOrTesting("teamAccess: unprepared context") + ctx.APIErrorInternal(errors.New("teamAccess: unprepared context")) + return 0, false, false + } + + orgID = ctx.Org.Team.OrgID + isOwner, err := organization.IsOrganizationOwner(ctx, orgID, ctx.Doer.ID) + if err != nil { + ctx.APIErrorInternal(err) + return 0, false, false + } else if isOwner { + return orgID, true, true + } + + isTeamMember, err := organization.IsTeamMember(ctx, orgID, ctx.Org.Team.ID, ctx.Doer.ID) + if err != nil { + ctx.APIErrorInternal(err) + return 0, false, false + } + return orgID, isTeamMember, true +} + +func denyNonTeamMember(ctx *context.APIContext, orgID int64) { + isOrgMember, err := organization.IsOrganizationMember(ctx, orgID, ctx.Doer.ID) + if err != nil { + ctx.APIErrorInternal(err) + } else if isOrgMember { + ctx.APIError(http.StatusForbidden, "Must be a team member") + } else { + ctx.APIErrorNotFound() + } +} + +// reqTeamReadAccess allows callers who can list the team to read its metadata. +// Non-members are admitted by the team's visibility tier and parent org visibility. +// Not sufficient for mutations — use reqOrgOwnership() or reqTeamMembership() for those. +func reqTeamReadAccess() func(ctx *context.APIContext) { return func(ctx *context.APIContext) { - if ctx.IsUserSiteAdmin() { + orgID, privileged, ok := teamAccessPrivileged(ctx) + if !ok || privileged { return } - if ctx.Org.Team == nil { - setting.PanicInDevOrTesting("reqTeamMembership: unprepared context") - ctx.APIErrorInternal(errors.New("reqTeamMembership: unprepared context")) + if ctx.Org.Organization == nil { + setting.PanicInDevOrTesting("reqTeamReadAccess: organization not loaded") + ctx.APIErrorInternal(errors.New("reqTeamReadAccess: organization not loaded")) return } - orgID := ctx.Org.Team.OrgID - isOwner, err := organization.IsOrganizationOwner(ctx, orgID, ctx.Doer.ID) + visible, err := ctx.Org.Team.CanNonMemberReadMeta(ctx, ctx.Org.Organization.AsUser(), ctx.Doer) if err != nil { ctx.APIErrorInternal(err) return - } else if isOwner { - return } + if !visible { + // Not admitted by visibility: 403 for org members, 404 otherwise. + denyNonTeamMember(ctx, orgID) + } + } +} - if isTeamMember, err := organization.IsTeamMember(ctx, orgID, ctx.Org.Team.ID, ctx.Doer.ID); err != nil { - ctx.APIErrorInternal(err) - return - } else if !isTeamMember { - isOrgMember, err := organization.IsOrganizationMember(ctx, orgID, ctx.Doer.ID) - if err != nil { - ctx.APIErrorInternal(err) - } else if isOrgMember { - ctx.APIError(http.StatusForbidden, "Must be a team member") - } else { - ctx.APIErrorNotFound() - } +// reqTeamMembership user should be a team member, or a site admin +func reqTeamMembership() func(ctx *context.APIContext) { + return func(ctx *context.APIContext) { + orgID, privileged, ok := teamAccessPrivileged(ctx) + if !ok || privileged { return } + denyNonTeamMember(ctx, orgID) } } @@ -649,6 +687,17 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) { } return } + if ctx.Org.Organization == nil { + ctx.Org.Organization, err = organization.GetOrgByID(ctx, ctx.Org.Team.OrgID) + if err != nil { + if organization.IsErrOrgNotExist(err) { + ctx.APIErrorNotFound() + } else { + ctx.APIErrorInternal(err) + } + return + } + } } } } @@ -1703,25 +1752,31 @@ func Routes() *web.Router { }, reqToken(), reqOrgOwnership()) }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryOrganization), orgAssignment(true), checkTokenPublicOnly()) m.Group("/teams/{teamid}", func() { - m.Combo("").Get(reqToken(), org.GetTeam). - Patch(reqToken(), reqOrgOwnership(), bind(api.EditTeamOption{}), org.EditTeam). + m.Combo("").Patch(reqToken(), reqOrgOwnership(), bind(api.EditTeamOption{}), org.EditTeam). Delete(reqToken(), reqOrgOwnership(), org.DeleteTeam) + m.Group("", func() { + m.Get("", org.GetTeam) + m.Group("/members", func() { + m.Get("", reqOrgMembership(), org.GetTeamMembers) + m.Combo("/{username}").Get(reqOrgMembership(), org.GetTeamMember) + }) + m.Group("/repos", func() { + m.Get("", org.GetTeamRepos) + m.Combo("/{org}/{reponame}").Get(org.GetTeamRepo) + }) + m.Get("/activities/feeds", org.ListTeamActivityFeeds) + }, reqTeamReadAccess()) m.Group("/members", func() { - m.Get("", reqToken(), org.GetTeamMembers) m.Combo("/{username}"). - Get(reqToken(), org.GetTeamMember). Put(reqToken(), reqOrgOwnership(), org.AddTeamMember). Delete(reqToken(), reqOrgOwnership(), org.RemoveTeamMember) }) m.Group("/repos", func() { - m.Get("", reqToken(), org.GetTeamRepos) m.Combo("/{org}/{reponame}"). - Put(reqToken(), org.AddTeamRepository). - Delete(reqToken(), org.RemoveTeamRepository). - Get(reqToken(), org.GetTeamRepo) + Put(reqToken(), reqTeamMembership(), org.AddTeamRepository). + Delete(reqToken(), reqTeamMembership(), org.RemoveTeamRepository) }) - m.Get("/activities/feeds", org.ListTeamActivityFeeds) - }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryOrganization), orgAssignment(false, true), reqToken(), reqTeamMembership(), checkTokenPublicOnly()) + }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryOrganization), orgAssignment(false, true), reqToken(), checkTokenPublicOnly()) m.Group("/admin", func() { m.Group("/cron", func() { diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 4373b90f2fa..bc6bb54e90c 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -55,10 +55,15 @@ func ListTeams(ctx *context.APIContext) { // "$ref": "#/responses/notFound" listOptions := utils.GetListOptions(ctx) - teams, count, err := organization.SearchTeam(ctx, &organization.SearchTeamOptions{ + opts := &organization.SearchTeamOptions{ ListOptions: listOptions, OrgID: ctx.Org.Organization.ID, - }) + } + if err := organization.ApplyTeamListFilter(ctx, ctx.Org.Organization.ID, ctx.Doer, ctx.IsSigned, opts); err != nil { + ctx.APIErrorInternal(err) + return + } + teams, count, err := organization.SearchTeam(ctx, opts) if err != nil { ctx.APIErrorInternal(err) return @@ -218,6 +223,7 @@ func CreateTeam(ctx *context.APIContext) { IncludesAllRepositories: form.IncludesAllRepositories, CanCreateOrgRepo: form.CanCreateOrgRepo, AccessMode: teamPermission, + Visibility: organization.NormalizeTeamVisibility(string(form.Visibility)), } if team.AccessMode < perm.AccessModeAdmin { @@ -295,6 +301,10 @@ func EditTeam(ctx *context.APIContext) { team.Description = *form.Description } + if form.Visibility != nil && !team.IsOwnerTeam() { + team.Visibility = organization.NormalizeTeamVisibility(string(*form.Visibility)) + } + isAuthChanged := false isIncludeAllChanged := false if !team.IsOwnerTeam() && len(form.Permission) != 0 { @@ -387,15 +397,6 @@ func GetTeamMembers(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - isMember, err := organization.IsOrganizationMember(ctx, ctx.Org.Team.OrgID, ctx.Doer.ID) - if err != nil { - ctx.APIErrorInternal(err) - return - } else if !isMember && !ctx.Doer.IsAdmin { - ctx.APIErrorNotFound() - return - } - listOptions := utils.GetListOptions(ctx) teamMembers, err := organization.GetTeamMembers(ctx, &organization.SearchMembersOptions{ ListOptions: listOptions, @@ -574,14 +575,20 @@ func GetTeamRepos(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } - repos := make([]*api.Repository, len(teamRepos)) - for i, repo := range teamRepos { + repos := make([]*api.Repository, 0, len(teamRepos)) + for _, repo := range teamRepos { permission, err := access_model.GetDoerRepoPermission(ctx, repo, ctx.Doer) if err != nil { ctx.APIErrorInternal(err) return } - repos[i] = convert.ToRepo(ctx, repo, permission) + // A team's repo list is reachable by non-team-members through the team's + // visibility tier, so never expose repos (incl. their names) the doer + // cannot access. + if !permission.HasAnyUnitAccessOrPublicAccess() { + continue + } + repos = append(repos, convert.ToRepo(ctx, repo, permission)) } ctx.SetLinkHeader(int64(team.NumRepos), listOptions.PageSize) ctx.SetTotalCountHeader(int64(team.NumRepos)) @@ -633,6 +640,12 @@ func GetTeamRepo(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } + // The team may be reachable by a non-team-member via its visibility tier; + // don't confirm the existence of a repo the doer cannot access. + if !permission.HasAnyUnitAccessOrPublicAccess() { + ctx.APIErrorNotFound() + return + } ctx.JSON(http.StatusOK, convert.ToRepo(ctx, repo, permission)) } @@ -806,9 +819,9 @@ func SearchTeam(ctx *context.APIContext) { ListOptions: listOptions, } - // Only admin is allowed to search for all teams - if !ctx.Doer.IsAdmin { - opts.UserID = ctx.Doer.ID + if err := organization.ApplyTeamListFilter(ctx, ctx.Org.Organization.ID, ctx.Doer, ctx.IsSigned, opts); err != nil { + ctx.APIErrorInternal(err) + return } teams, maxResults, err := organization.SearchTeam(ctx, opts) diff --git a/routers/web/org/home.go b/routers/web/org/home.go index a88c8461767..7c52455d31d 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -101,7 +101,26 @@ func home(ctx *context.Context, viewRepositories bool) { const orgOverviewTeamsLimit = 5 ctx.Data["OrgOverviewMembers"] = members - ctx.Data["OrgOverviewTeams"] = ctx.Org.Teams[:min(len(ctx.Org.Teams), orgOverviewTeamsLimit)] + // The overview widget shows only teams the viewer belongs to. ctx.Org.Teams + // may include visible-but-not-joined teams (via IncludeVisibilities for + // signed-in non-members), so re-query the viewer's own membership; owners + // keep the full list they are entitled to manage. + overviewTeams := ctx.Org.Teams + if !ctx.Org.IsOwner { + overviewTeams = nil + if ctx.Org.IsMember { + overviewTeams, _, err = organization.SearchTeam(ctx, &organization.SearchTeamOptions{ + OrgID: org.ID, + UserID: ctx.Doer.ID, + ListOptions: db.ListOptions{Page: 1, PageSize: orgOverviewTeamsLimit}, + }) + if err != nil { + ctx.ServerError("SearchTeam", err) + return + } + } + } + ctx.Data["OrgOverviewTeams"] = overviewTeams[:min(len(overviewTeams), orgOverviewTeamsLimit)] ctx.Data["DisableNewPullMirrors"] = setting.Mirror.DisableNewPull ctx.Data["ShowMemberAndTeamTab"] = ctx.Org.IsMember || len(members) > 0 diff --git a/routers/web/org/teams.go b/routers/web/org/teams.go index a026f4aa073..11c4a1da3a5 100644 --- a/routers/web/org/teams.go +++ b/routers/web/org/teams.go @@ -21,6 +21,7 @@ import ( user_model "gitea.dev/models/user" "gitea.dev/modules/log" "gitea.dev/modules/setting" + "gitea.dev/modules/structs" "gitea.dev/modules/templates" "gitea.dev/modules/util" "gitea.dev/modules/web" @@ -80,6 +81,8 @@ func Teams(ctx *context.Context) { UserID: util.Iif(shouldSeeAllOrgTeams, 0, ctx.Doer.ID), Keyword: keyword, IncludeDesc: true, + IncludeVisibilities: util.Iif(shouldSeeAllOrgTeams, nil, + org_model.VisibleTeamVisibilitiesFor(ctx.Org.IsMember, ctx.IsSigned)), ListOptions: db.ListOptions{Page: page, PageSize: pagingNum}, } return org_model.SearchTeam(ctx, opts) @@ -377,6 +380,7 @@ func NewTeamPost(ctx *context.Context) { AccessMode: teamPermission, IncludesAllRepositories: includesAllRepositories, CanCreateOrgRepo: form.CanCreateOrgRepo, + Visibility: org_model.NormalizeTeamVisibility(form.Visibility), } units := make([]*org_model.TeamUnit, 0, len(unitPerms)) @@ -477,13 +481,22 @@ func SearchTeam(ctx *context.Context) { PageSize: convert.ToCorrectPageSize(ctx.FormInt("limit")), } + shouldSeeAll, err := context.UserShouldSeeAllOrgTeams(ctx) + if err != nil { + ctx.ServerError("UserShouldSeeAllOrgTeams", err) + return + } + opts := &org_model.SearchTeamOptions{ - // UserID is not set because the router already requires the doer to be an org admin. Thus, we don't need to restrict to teams that the user belongs in Keyword: ctx.FormTrim("q"), OrgID: ctx.Org.Organization.ID, IncludeDesc: ctx.FormString("include_desc") == "" || ctx.FormBool("include_desc"), ListOptions: listOptions, } + if !shouldSeeAll { + opts.UserID = ctx.Doer.ID + opts.IncludeVisibilities = org_model.VisibleTeamVisibilitiesFor(ctx.Org.IsMember, ctx.IsSigned) + } teams, maxResults, err := org_model.SearchTeam(ctx, opts) if err != nil { @@ -556,8 +569,11 @@ func EditTeamPost(ctx *context.Context) { t.IncludesAllRepositories = includesAllRepositories } t.CanCreateOrgRepo = form.CanCreateOrgRepo + t.Visibility = org_model.NormalizeTeamVisibility(form.Visibility) } else { t.CanCreateOrgRepo = true + // The owner team must remain listable to all org members. + t.Visibility = structs.VisibleTypeLimited } t.Description = form.Description diff --git a/services/context/org.go b/services/context/org.go index 79dcfab5f2b..0f6a06e3269 100644 --- a/services/context/org.go +++ b/services/context/org.go @@ -179,20 +179,28 @@ func OrgAssignment(orgAssignmentOpts OrgAssignmentOptions) func(ctx *Context) { ctx.ServerError("UserShouldSeeAllOrgTeams", err) return } - if ctx.Org.IsMember { - if shouldSeeAllTeams { - ctx.Org.Teams, err = org.LoadTeams(ctx) - if err != nil { - ctx.ServerError("LoadTeams", err) - return - } - } else { - ctx.Org.Teams, err = org.GetUserTeams(ctx, ctx.Doer.ID) - if err != nil { - ctx.ServerError("GetUserTeams", err) - return - } + switch { + case shouldSeeAllTeams: + ctx.Org.Teams, err = org.LoadTeams(ctx) + if err != nil { + ctx.ServerError("LoadTeams", err) + return } + case ctx.IsSigned: + // Signed-in non-members still see teams whose visibility tier + // includes them (public for any signed-in user, plus limited + // for org members), and any team they directly belong to. + ctx.Org.Teams, _, err = organization.SearchTeam(ctx, &organization.SearchTeamOptions{ + OrgID: org.ID, + UserID: ctx.Doer.ID, + IncludeVisibilities: organization.VisibleTeamVisibilitiesFor(ctx.Org.IsMember, true), + }) + if err != nil { + ctx.ServerError("SearchTeam", err) + return + } + } + if ctx.Org.IsMember { ctx.Data["NumTeams"] = len(ctx.Org.Teams) } @@ -203,7 +211,6 @@ func OrgAssignment(orgAssignmentOpts OrgAssignmentOptions) func(ctx *Context) { if strings.EqualFold(team.LowerName, teamName) { teamExists = true ctx.Org.Team = team - ctx.Org.IsTeamMember = true ctx.Data["Team"] = ctx.Org.Team break } @@ -214,13 +221,24 @@ func OrgAssignment(orgAssignmentOpts OrgAssignmentOptions) func(ctx *Context) { return } + // Membership in a visible team is not implied by its presence in + // ctx.Org.Teams; admins/org owners keep the privileged flag set + // earlier in this function. + if !ctx.Org.IsOwner { + ctx.Org.IsTeamMember, err = organization.IsTeamMember(ctx, org.ID, ctx.Org.Team.ID, ctx.Doer.ID) + if err != nil { + ctx.ServerError("IsTeamMember", err) + return + } + } ctx.Data["IsTeamMember"] = ctx.Org.IsTeamMember if opts.RequireTeamMember && !ctx.Org.IsTeamMember { ctx.NotFound(err) return } - ctx.Org.IsTeamAdmin = ctx.Org.Team.IsOwnerTeam() || ctx.Org.Team.HasAdminAccess() + isTeamOwnerOrAdmin := ctx.Org.Team.IsOwnerTeam() || ctx.Org.Team.HasAdminAccess() + ctx.Org.IsTeamAdmin = ctx.Org.IsOwner || (ctx.Org.IsTeamMember && isTeamOwnerOrAdmin) ctx.Data["IsTeamAdmin"] = ctx.Org.IsTeamAdmin if opts.RequireTeamAdmin && !ctx.Org.IsTeamAdmin { ctx.NotFound(err) diff --git a/services/convert/convert.go b/services/convert/convert.go index 49d74f7d109..d1437de286f 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -836,6 +836,7 @@ func ToTeams(ctx context.Context, teams []*organization.Team, loadOrgs bool) ([] Permission: api.AccessLevelName(t.AccessMode.ToString()), Units: t.GetUnitNames(), UnitsMap: t.GetUnitsMap(), + Visibility: api.TeamVisibility(t.Visibility.String()), } if loadOrgs { diff --git a/services/forms/org.go b/services/forms/org.go index 2aacf1b645d..be3f64d630e 100644 --- a/services/forms/org.go +++ b/services/forms/org.go @@ -70,6 +70,7 @@ type CreateTeamForm struct { Permission string RepoAccess string CanCreateOrgRepo bool + Visibility string `binding:"OmitEmpty;In(public,limited,private)"` } // Validate validates the fields diff --git a/services/org/team.go b/services/org/team.go index f3313e0edf5..b079313306d 100644 --- a/services/org/team.go +++ b/services/org/team.go @@ -110,7 +110,7 @@ func UpdateTeam(ctx context.Context, t *organization.Team, authChanged, includeA sess := db.GetEngine(ctx) if _, err = sess.ID(t.ID).Cols("name", "lower_name", "description", - "can_create_org_repo", "authorize", "includes_all_repositories").Update(t); err != nil { + "can_create_org_repo", "authorize", "includes_all_repositories", "visibility").Update(t); err != nil { return fmt.Errorf("update: %w", err) } diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index f8785bb466a..7e0403a2f56 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -12,10 +12,10 @@ {{template "base/alert" .}}
- {{if eq .Team.LowerName "owners"}} + {{if .Team.IsOwnerTeam}} {{end}} - + {{ctx.Locale.Tr "org.team_name_helper"}}
@@ -23,7 +23,47 @@ {{ctx.Locale.Tr "org.team_desc_helper"}}
- {{if not (eq .Team.LowerName "owners")}} + {{if .Team.IsOwnerTeam}} +
+ +
+ {{if .Team.IsPrivate}} + {{ctx.Locale.Tr "org.teams.visibility_private"}} + {{else if .Team.IsLimited}} + {{ctx.Locale.Tr "org.teams.visibility_limited"}} + {{else if .Team.IsPublic}} + {{ctx.Locale.Tr "org.teams.visibility_public"}} + {{end}} +
+ {{ctx.Locale.Tr "org.teams.owners_visibility_fixed"}} +
+ {{end}} + {{if not .Team.IsOwnerTeam}} +
+ +
+
+
+ + + {{ctx.Locale.Tr "org.teams.visibility_private_helper"}} +
+
+
+
+ + + {{ctx.Locale.Tr "org.teams.visibility_limited_helper"}} +
+
+
+
+ + + {{ctx.Locale.Tr "org.teams.visibility_public_helper"}} +
+
+

@@ -135,7 +175,7 @@ {{else}} - {{if not (eq .Team.LowerName "owners")}} + {{if not .Team.IsOwnerTeam}} {{end}} {{end}} diff --git a/templates/org/team/sidebar.tmpl b/templates/org/team/sidebar.tmpl index 1036e886da7..a0b936285e6 100644 --- a/templates/org/team/sidebar.tmpl +++ b/templates/org/team/sidebar.tmpl @@ -1,6 +1,15 @@

- {{.Team.Name}} +
+ {{.Team.Name}} + {{if .Team.IsPrivate}} + {{ctx.Locale.Tr "org.teams.visibility_private"}} + {{else if .Team.IsLimited}} + {{ctx.Locale.Tr "org.teams.visibility_limited"}} + {{else if .Team.IsPublic}} + {{ctx.Locale.Tr "org.teams.visibility_public"}} + {{end}} +
{{if .Team.IsMember ctx $.SignedUser.ID}}