From 5c7bb56b8a0bbb29faff73971a308d984268a919 Mon Sep 17 00:00:00 2001 From: Richard Mahn Date: Thu, 30 May 2019 13:57:55 -0400 Subject: [PATCH] Fixes #7023 - API Org Visibility (#7028) --- integrations/api_admin_org_test.go | 86 ++++++++++++++++++++++++++++++ integrations/api_org_test.go | 48 ++++++++++++++++- integrations/api_user_orgs_test.go | 2 + modules/structs/org.go | 33 +++++++----- modules/structs/org_type.go | 10 ++++ modules/structs/repo_file.go | 26 ++++++--- routers/api/v1/admin/org.go | 7 +++ routers/api/v1/convert/convert.go | 1 + routers/api/v1/org/org.go | 14 ++++- routers/api/v1/repo/file.go | 6 +-- templates/swagger/v1_json.tmpl | 65 +++++++++++++++++----- 11 files changed, 258 insertions(+), 40 deletions(-) create mode 100644 integrations/api_admin_org_test.go diff --git a/integrations/api_admin_org_test.go b/integrations/api_admin_org_test.go new file mode 100644 index 0000000000000..546ed861c255a --- /dev/null +++ b/integrations/api_admin_org_test.go @@ -0,0 +1,86 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "net/http" + "net/url" + "strings" + "testing" + + "code.gitea.io/gitea/models" + api "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" +) + +func TestAPIAdminOrgCreate(t *testing.T) { + onGiteaRun(t, func(*testing.T, *url.URL) { + session := loginUser(t, "user1") + token := getTokenForLoggedInUser(t, session) + + var org = api.CreateOrgOption{ + UserName: "user2_org", + FullName: "User2's organization", + Description: "This organization created by admin for user2", + Website: "https://try.gitea.io", + Location: "Shanghai", + Visibility: "private", + } + req := NewRequestWithJSON(t, "POST", "/api/v1/admin/users/user2/orgs?token="+token, &org) + resp := session.MakeRequest(t, req, http.StatusCreated) + + var apiOrg api.Organization + DecodeJSON(t, resp, &apiOrg) + + assert.Equal(t, org.UserName, apiOrg.UserName) + assert.Equal(t, org.FullName, apiOrg.FullName) + assert.Equal(t, org.Description, apiOrg.Description) + assert.Equal(t, org.Website, apiOrg.Website) + assert.Equal(t, org.Location, apiOrg.Location) + assert.Equal(t, org.Visibility, apiOrg.Visibility) + + models.AssertExistsAndLoadBean(t, &models.User{ + Name: org.UserName, + LowerName: strings.ToLower(org.UserName), + FullName: org.FullName, + }) + }) +} + +func TestAPIAdminOrgCreateBadVisibility(t *testing.T) { + onGiteaRun(t, func(*testing.T, *url.URL) { + session := loginUser(t, "user1") + token := getTokenForLoggedInUser(t, session) + + var org = api.CreateOrgOption{ + UserName: "user2_org", + FullName: "User2's organization", + Description: "This organization created by admin for user2", + Website: "https://try.gitea.io", + Location: "Shanghai", + Visibility: "notvalid", + } + req := NewRequestWithJSON(t, "POST", "/api/v1/admin/users/user2/orgs?token="+token, &org) + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + }) +} + +func TestAPIAdminOrgCreateNotAdmin(t *testing.T) { + prepareTestEnv(t) + nonAdminUsername := "user2" + session := loginUser(t, nonAdminUsername) + token := getTokenForLoggedInUser(t, session) + var org = api.CreateOrgOption{ + UserName: "user2_org", + FullName: "User2's organization", + Description: "This organization created by admin for user2", + Website: "https://try.gitea.io", + Location: "Shanghai", + Visibility: "public", + } + req := NewRequestWithJSON(t, "POST", "/api/v1/admin/users/user2/orgs?token="+token, &org) + session.MakeRequest(t, req, http.StatusForbidden) +} diff --git a/integrations/api_org_test.go b/integrations/api_org_test.go index b36650f2e8b5f..34579aa1ea9c1 100644 --- a/integrations/api_org_test.go +++ b/integrations/api_org_test.go @@ -17,7 +17,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestAPIOrg(t *testing.T) { +func TestAPIOrgCreate(t *testing.T) { onGiteaRun(t, func(*testing.T, *url.URL) { session := loginUser(t, "user1") @@ -28,6 +28,7 @@ func TestAPIOrg(t *testing.T) { Description: "This organization created by user1", Website: "https://try.gitea.io", Location: "Shanghai", + Visibility: "limited", } req := NewRequestWithJSON(t, "POST", "/api/v1/orgs?token="+token, &org) resp := session.MakeRequest(t, req, http.StatusCreated) @@ -40,6 +41,7 @@ func TestAPIOrg(t *testing.T) { assert.Equal(t, org.Description, apiOrg.Description) assert.Equal(t, org.Website, apiOrg.Website) assert.Equal(t, org.Location, apiOrg.Location) + assert.Equal(t, org.Visibility, apiOrg.Visibility) models.AssertExistsAndLoadBean(t, &models.User{ Name: org.UserName, @@ -72,6 +74,50 @@ func TestAPIOrg(t *testing.T) { }) } +func TestAPIOrgEdit(t *testing.T) { + onGiteaRun(t, func(*testing.T, *url.URL) { + session := loginUser(t, "user1") + + token := getTokenForLoggedInUser(t, session) + var org = api.EditOrgOption{ + FullName: "User3 organization new full name", + Description: "A new description", + Website: "https://try.gitea.io/new", + Location: "Beijing", + Visibility: "private", + } + req := NewRequestWithJSON(t, "PATCH", "/api/v1/orgs/user3?token="+token, &org) + resp := session.MakeRequest(t, req, http.StatusOK) + + var apiOrg api.Organization + DecodeJSON(t, resp, &apiOrg) + + assert.Equal(t, "user3", apiOrg.UserName) + assert.Equal(t, org.FullName, apiOrg.FullName) + assert.Equal(t, org.Description, apiOrg.Description) + assert.Equal(t, org.Website, apiOrg.Website) + assert.Equal(t, org.Location, apiOrg.Location) + assert.Equal(t, org.Visibility, apiOrg.Visibility) + }) +} + +func TestAPIOrgEditBadVisibility(t *testing.T) { + onGiteaRun(t, func(*testing.T, *url.URL) { + session := loginUser(t, "user1") + + token := getTokenForLoggedInUser(t, session) + var org = api.EditOrgOption{ + FullName: "User3 organization new full name", + Description: "A new description", + Website: "https://try.gitea.io/new", + Location: "Beijing", + Visibility: "badvisibility", + } + req := NewRequestWithJSON(t, "PATCH", "/api/v1/orgs/user3?token="+token, &org) + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + }) +} + func TestAPIOrgDeny(t *testing.T) { onGiteaRun(t, func(*testing.T, *url.URL) { setting.Service.RequireSignInView = true diff --git a/integrations/api_user_orgs_test.go b/integrations/api_user_orgs_test.go index 63e67f4356a30..6611a429d1f68 100644 --- a/integrations/api_user_orgs_test.go +++ b/integrations/api_user_orgs_test.go @@ -38,6 +38,7 @@ func TestUserOrgs(t *testing.T) { Description: "", Website: "", Location: "", + Visibility: "public", }, }, orgs) } @@ -63,6 +64,7 @@ func TestMyOrgs(t *testing.T) { Description: "", Website: "", Location: "", + Visibility: "public", }, }, orgs) } diff --git a/modules/structs/org.go b/modules/structs/org.go index fd15da1ce946c..08ab139975283 100644 --- a/modules/structs/org.go +++ b/modules/structs/org.go @@ -6,25 +6,27 @@ package structs // Organization represents an organization type Organization struct { - ID int64 `json:"id"` - UserName string `json:"username"` - FullName string `json:"full_name"` - AvatarURL string `json:"avatar_url"` - Description string `json:"description"` - Website string `json:"website"` - Location string `json:"location"` - Visibility VisibleType `json:"visibility"` + ID int64 `json:"id"` + UserName string `json:"username"` + FullName string `json:"full_name"` + AvatarURL string `json:"avatar_url"` + Description string `json:"description"` + Website string `json:"website"` + Location string `json:"location"` + Visibility string `json:"visibility"` } // CreateOrgOption options for creating an organization type CreateOrgOption struct { // required: true - UserName string `json:"username" binding:"Required"` - FullName string `json:"full_name"` - Description string `json:"description"` - Website string `json:"website"` - Location string `json:"location"` - Visibility VisibleType `json:"visibility"` + UserName string `json:"username" binding:"Required"` + FullName string `json:"full_name"` + Description string `json:"description"` + Website string `json:"website"` + Location string `json:"location"` + // possible values are `public` (default), `limited` or `private` + // enum: public,limited,private + Visibility string `json:"visibility" binding:"In(,public,limited,private)"` } // EditOrgOption options for editing an organization @@ -33,4 +35,7 @@ type EditOrgOption struct { Description string `json:"description"` Website string `json:"website"` Location string `json:"location"` + // possible values are `public`, `limited` or `private` + // enum: public,limited,private + Visibility string `json:"visibility" binding:"In(,public,limited,private)"` } diff --git a/modules/structs/org_type.go b/modules/structs/org_type.go index 86dc5c81cd6ab..4fb9b6fc0fdc8 100644 --- a/modules/structs/org_type.go +++ b/modules/structs/org_type.go @@ -40,6 +40,16 @@ func (vt VisibleType) IsPrivate() bool { return vt == VisibleTypePrivate } +// VisibilityString provides the mode string of the visibility type (public, limited, private) +func (vt VisibleType) String() string { + for k, v := range VisibilityModes { + if vt == v { + return k + } + } + return "" +} + // ExtractKeysFromMapString provides a slice of keys from map func ExtractKeysFromMapString(in map[string]VisibleType) (keys []string) { for k := range in { diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index ac8b9333fe57c..c447d267244c9 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -7,29 +7,43 @@ package structs // FileOptions options for all file APIs type FileOptions struct { - Message string `json:"message" binding:"Required"` - BranchName string `json:"branch"` - NewBranchName string `json:"new_branch"` - Author Identity `json:"author"` - Committer Identity `json:"committer"` + // message (optional) for the commit of this file. if not supplied, a default message will be used + Message string `json:"message" binding:"Required"` + // branch (optional) to base this file from. if not given, the default branch is used + BranchName string `json:"branch"` + // new_branch (optional) will make a new branch from `branch` before creating the file + NewBranchName string `json:"new_branch"` + // `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) + Author Identity `json:"author"` + Committer Identity `json:"committer"` } // CreateFileOptions options for creating files +// Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type CreateFileOptions struct { FileOptions + // content must be base64 encoded + // required: true Content string `json:"content"` } // DeleteFileOptions options for deleting files (used for other File structs below) +// Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type DeleteFileOptions struct { FileOptions + // sha is the SHA for the file that already exists + // required: true SHA string `json:"sha" binding:"Required"` } // UpdateFileOptions options for updating files +// Note: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) type UpdateFileOptions struct { DeleteFileOptions - Content string `json:"content"` + // content must be base64 encoded + // required: true + Content string `json:"content"` + // from_path (optional) is the path of the original file which will be moved/renamed to the path in the URL FromPath string `json:"from_path" binding:"MaxSize(500)"` } diff --git a/routers/api/v1/admin/org.go b/routers/api/v1/admin/org.go index fba41a8cfece8..d740647cd4f20 100644 --- a/routers/api/v1/admin/org.go +++ b/routers/api/v1/admin/org.go @@ -45,6 +45,11 @@ func CreateOrg(ctx *context.APIContext, form api.CreateOrgOption) { return } + visibility := api.VisibleTypePublic + if form.Visibility != "" { + visibility = api.VisibilityModes[form.Visibility] + } + org := &models.User{ Name: form.UserName, FullName: form.FullName, @@ -53,7 +58,9 @@ func CreateOrg(ctx *context.APIContext, form api.CreateOrgOption) { Location: form.Location, IsActive: true, Type: models.UserTypeOrganization, + Visibility: visibility, } + if err := models.CreateOrganization(org, u); err != nil { if models.IsErrUserAlreadyExist(err) || models.IsErrNameReserved(err) || diff --git a/routers/api/v1/convert/convert.go b/routers/api/v1/convert/convert.go index 74fd9b3afd354..ba61c7e46c938 100644 --- a/routers/api/v1/convert/convert.go +++ b/routers/api/v1/convert/convert.go @@ -213,6 +213,7 @@ func ToOrganization(org *models.User) *api.Organization { Description: org.Description, Website: org.Website, Location: org.Location, + Visibility: org.Visibility.String(), } } diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index e1d0663f05aa4..2893887a4bf14 100644 --- a/routers/api/v1/org/org.go +++ b/routers/api/v1/org/org.go @@ -90,6 +90,11 @@ func Create(ctx *context.APIContext, form api.CreateOrgOption) { return } + visibility := api.VisibleTypePublic + if form.Visibility != "" { + visibility = api.VisibilityModes[form.Visibility] + } + org := &models.User{ Name: form.UserName, FullName: form.FullName, @@ -98,6 +103,7 @@ func Create(ctx *context.APIContext, form api.CreateOrgOption) { Location: form.Location, IsActive: true, Type: models.UserTypeOrganization, + Visibility: visibility, } if err := models.CreateOrganization(org, ctx.User); err != nil { if models.IsErrUserAlreadyExist(err) || @@ -153,6 +159,7 @@ func Edit(ctx *context.APIContext, form api.EditOrgOption) { // required: true // - name: body // in: body + // required: true // schema: // "$ref": "#/definitions/EditOrgOption" // responses: @@ -163,8 +170,11 @@ func Edit(ctx *context.APIContext, form api.EditOrgOption) { org.Description = form.Description org.Website = form.Website org.Location = form.Location - if err := models.UpdateUserCols(org, "full_name", "description", "website", "location"); err != nil { - ctx.Error(500, "UpdateUser", err) + if form.Visibility != "" { + org.Visibility = api.VisibilityModes[form.Visibility] + } + if err := models.UpdateUserCols(org, "full_name", "description", "website", "location", "visibility"); err != nil { + ctx.Error(500, "EditOrganization", err) return } diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index db952263e2f23..20f80f37f4c67 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -181,7 +181,7 @@ func CreateFile(ctx *context.APIContext, apiOpts api.CreateFileOptions) { // required: true // - name: body // in: body - // description: "'content' must be base64 encoded\n\n 'author' and 'committer' are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)\n\n If 'branch' is not given, default branch will be used\n\n 'sha' is the SHA for the file that already exists\n\n 'new_branch' (optional) will make a new branch from 'branch' before creating the file" + // required: true // schema: // "$ref": "#/definitions/CreateFileOptions" // responses: @@ -238,7 +238,7 @@ func UpdateFile(ctx *context.APIContext, apiOpts api.UpdateFileOptions) { // required: true // - name: body // in: body - // description: "'content' must be base64 encoded\n\n 'sha' is the SHA for the file that already exists\n\n 'author' and 'committer' are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)\n\n If 'branch' is not given, default branch will be used\n\n 'new_branch' (optional) will make a new branch from 'branch' before updating the file" + // required: true // schema: // "$ref": "#/definitions/UpdateFileOptions" // responses: @@ -316,7 +316,7 @@ func DeleteFile(ctx *context.APIContext, apiOpts api.DeleteFileOptions) { // required: true // - name: body // in: body - // description: "'sha' is the SHA for the file to be deleted\n\n 'author' and 'committer' are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)\n\n If 'branch' is not given, default branch will be used\n\n 'new_branch' (optional) will make a new branch from 'branch' before deleting the file" + // required: true // schema: // "$ref": "#/definitions/DeleteFileOptions" // responses: diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index a3090d1d52eb5..77515bb139e61 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -552,6 +552,7 @@ { "name": "body", "in": "body", + "required": true, "schema": { "$ref": "#/definitions/EditOrgOption" } @@ -1649,9 +1650,9 @@ "required": true }, { - "description": "'content' must be base64 encoded\n\n 'sha' is the SHA for the file that already exists\n\n 'author' and 'committer' are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)\n\n If 'branch' is not given, default branch will be used\n\n 'new_branch' (optional) will make a new branch from 'branch' before updating the file", "name": "body", "in": "body", + "required": true, "schema": { "$ref": "#/definitions/UpdateFileOptions" } @@ -1698,9 +1699,9 @@ "required": true }, { - "description": "'content' must be base64 encoded\n\n 'author' and 'committer' are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)\n\n If 'branch' is not given, default branch will be used\n\n 'sha' is the SHA for the file that already exists\n\n 'new_branch' (optional) will make a new branch from 'branch' before creating the file", "name": "body", "in": "body", + "required": true, "schema": { "$ref": "#/definitions/CreateFileOptions" } @@ -1747,9 +1748,9 @@ "required": true }, { - "description": "'sha' is the SHA for the file to be deleted\n\n 'author' and 'committer' are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)\n\n If 'branch' is not given, default branch will be used\n\n 'new_branch' (optional) will make a new branch from 'branch' before deleting the file", "name": "body", "in": "body", + "required": true, "schema": { "$ref": "#/definitions/DeleteFileOptions" } @@ -6935,13 +6936,17 @@ "x-go-package": "code.gitea.io/gitea/modules/structs" }, "CreateFileOptions": { - "description": "CreateFileOptions options for creating files", + "description": "CreateFileOptions options for creating files\nNote: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)", "type": "object", + "required": [ + "content" + ], "properties": { "author": { "$ref": "#/definitions/Identity" }, "branch": { + "description": "branch (optional) to base this file from. if not given, the default branch is used", "type": "string", "x-go-name": "BranchName" }, @@ -6949,14 +6954,17 @@ "$ref": "#/definitions/Identity" }, "content": { + "description": "content must be base64 encoded", "type": "string", "x-go-name": "Content" }, "message": { + "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", "type": "string", "x-go-name": "Message" }, "new_branch": { + "description": "new_branch (optional) will make a new branch from `branch` before creating the file", "type": "string", "x-go-name": "NewBranchName" } @@ -7191,7 +7199,14 @@ "x-go-name": "UserName" }, "visibility": { - "$ref": "#/definitions/VisibleType" + "description": "possible values are `public` (default), `limited` or `private`", + "type": "string", + "enum": [ + "public", + "limited", + "private" + ], + "x-go-name": "Visibility" }, "website": { "type": "string", @@ -7459,13 +7474,17 @@ "x-go-package": "code.gitea.io/gitea/modules/structs" }, "DeleteFileOptions": { - "description": "DeleteFileOptions options for deleting files (used for other File structs below)", + "description": "DeleteFileOptions options for deleting files (used for other File structs below)\nNote: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)", "type": "object", + "required": [ + "sha" + ], "properties": { "author": { "$ref": "#/definitions/Identity" }, "branch": { + "description": "branch (optional) to base this file from. if not given, the default branch is used", "type": "string", "x-go-name": "BranchName" }, @@ -7473,14 +7492,17 @@ "$ref": "#/definitions/Identity" }, "message": { + "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", "type": "string", "x-go-name": "Message" }, "new_branch": { + "description": "new_branch (optional) will make a new branch from `branch` before creating the file", "type": "string", "x-go-name": "NewBranchName" }, "sha": { + "description": "sha is the SHA for the file that already exists", "type": "string", "x-go-name": "SHA" } @@ -7703,6 +7725,16 @@ "type": "string", "x-go-name": "Location" }, + "visibility": { + "description": "possible values are `public`, `limited` or `private`", + "type": "string", + "enum": [ + "public", + "limited", + "private" + ], + "x-go-name": "Visibility" + }, "website": { "type": "string", "x-go-name": "Website" @@ -8741,7 +8773,8 @@ "x-go-name": "UserName" }, "visibility": { - "$ref": "#/definitions/VisibleType" + "type": "string", + "x-go-name": "Visibility" }, "website": { "type": "string", @@ -9537,13 +9570,18 @@ "x-go-package": "code.gitea.io/gitea/modules/structs" }, "UpdateFileOptions": { - "description": "UpdateFileOptions options for updating files", + "description": "UpdateFileOptions options for updating files\nNote: `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used)", "type": "object", + "required": [ + "sha", + "content" + ], "properties": { "author": { "$ref": "#/definitions/Identity" }, "branch": { + "description": "branch (optional) to base this file from. if not given, the default branch is used", "type": "string", "x-go-name": "BranchName" }, @@ -9551,22 +9589,27 @@ "$ref": "#/definitions/Identity" }, "content": { + "description": "content must be base64 encoded", "type": "string", "x-go-name": "Content" }, "from_path": { + "description": "from_path (optional) is the path of the original file which will be moved/renamed to the path in the URL", "type": "string", "x-go-name": "FromPath" }, "message": { + "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", "type": "string", "x-go-name": "Message" }, "new_branch": { + "description": "new_branch (optional) will make a new branch from `branch` before creating the file", "type": "string", "x-go-name": "NewBranchName" }, "sha": { + "description": "sha is the SHA for the file that already exists", "type": "string", "x-go-name": "SHA" } @@ -9631,12 +9674,6 @@ }, "x-go-package": "code.gitea.io/gitea/models" }, - "VisibleType": { - "description": "VisibleType defines the visibility (Organization only)", - "type": "integer", - "format": "int64", - "x-go-package": "code.gitea.io/gitea/modules/structs" - }, "WatchInfo": { "description": "WatchInfo represents an API watch status of one repository", "type": "object",