Skip to content

Commit

Permalink
Add ContextUser to http request context (go-gitea#18798)
Browse files Browse the repository at this point in the history
This PR adds a middleware which sets a ContextUser (like GetUserByParams before) in a single place which can be used by other methods. For routes which represent a repo or org the respective middlewares set the field too.

Also fix a bug in modules/context/org.go during refactoring.
  • Loading branch information
KN4CK3R authored and Stelios Malathouras committed Mar 28, 2022
1 parent c68ad4c commit 92ab3bc
Show file tree
Hide file tree
Showing 23 changed files with 243 additions and 319 deletions.
2 changes: 1 addition & 1 deletion integrations/api_user_org_perm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func TestUnknowUser(t *testing.T) {

var apiError api.APIError
DecodeJSON(t, resp, &apiError)
assert.Equal(t, "GetUserByName", apiError.Message)
assert.Equal(t, "user redirect does not exist [name: unknow]", apiError.Message)
}

func TestUnknowOrganization(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions modules/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ type Context struct {
IsSigned bool
IsBasicAuth bool

Repo *Repository
Org *Organization
ContextUser *user_model.User
Repo *Repository
Org *Organization
}

// TrHTMLEscapeArgs runs Tr but pre-escapes all arguments with html.EscapeString.
Expand Down
3 changes: 2 additions & 1 deletion modules/context/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
var err error
ctx.Org.Organization, err = models.GetOrgByName(orgName)
if err != nil {
if user_model.IsErrUserNotExist(err) {
if models.IsErrOrgNotExist(err) {
redirectUserID, err := user_model.LookupUserRedirect(orgName)
if err == nil {
RedirectToUser(ctx, orgName, redirectUserID)
Expand All @@ -68,6 +68,7 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
return
}
org := ctx.Org.Organization
ctx.ContextUser = org.AsUser()
ctx.Data["Org"] = org

teams, err := org.LoadTeams()
Expand Down
1 change: 1 addition & 0 deletions modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) {
}
}
ctx.Repo.Owner = owner
ctx.ContextUser = owner
ctx.Data["Username"] = ctx.Repo.Owner.Name

// redirect link to wiki
Expand Down
8 changes: 2 additions & 6 deletions routers/api/v1/admin/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"code.gitea.io/gitea/modules/convert"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/routers/api/v1/user"
"code.gitea.io/gitea/routers/api/v1/utils"
)

Expand Down Expand Up @@ -45,11 +44,8 @@ func CreateOrg(ctx *context.APIContext) {
// "$ref": "#/responses/forbidden"
// "422":
// "$ref": "#/responses/validationError"

form := web.GetForm(ctx).(*api.CreateOrgOption)
u := user.GetUserByParams(ctx)
if ctx.Written() {
return
}

visibility := api.VisibleTypePublic
if form.Visibility != "" {
Expand All @@ -67,7 +63,7 @@ func CreateOrg(ctx *context.APIContext) {
Visibility: visibility,
}

if err := models.CreateOrganization(org, u); err != nil {
if err := models.CreateOrganization(org, ctx.ContextUser); err != nil {
if user_model.IsErrUserAlreadyExist(err) ||
db.IsErrNameReserved(err) ||
db.IsErrNameCharsNotAllowed(err) ||
Expand Down
8 changes: 2 additions & 6 deletions routers/api/v1/admin/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/routers/api/v1/repo"
"code.gitea.io/gitea/routers/api/v1/user"
)

// CreateRepo api for creating a repository
Expand Down Expand Up @@ -42,11 +41,8 @@ func CreateRepo(ctx *context.APIContext) {
// "$ref": "#/responses/error"
// "422":
// "$ref": "#/responses/validationError"

form := web.GetForm(ctx).(*api.CreateRepoOption)
owner := user.GetUserByParams(ctx)
if ctx.Written() {
return
}

repo.CreateUserRepo(ctx, owner, *form)
repo.CreateUserRepo(ctx, ctx.ContextUser, *form)
}
82 changes: 34 additions & 48 deletions routers/api/v1/admin/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func CreateUser(ctx *context.APIContext) {
// "$ref": "#/responses/forbidden"
// "422":
// "$ref": "#/responses/validationError"

form := web.GetForm(ctx).(*api.CreateUserOption)

u := &user_model.User{
Expand Down Expand Up @@ -163,13 +164,10 @@ func EditUser(ctx *context.APIContext) {
// "$ref": "#/responses/forbidden"
// "422":
// "$ref": "#/responses/validationError"

form := web.GetForm(ctx).(*api.EditUserOption)
u := user.GetUserByParams(ctx)
if ctx.Written() {
return
}

parseAuthSource(ctx, u, form.SourceID, form.LoginName)
parseAuthSource(ctx, ctx.ContextUser, form.SourceID, form.LoginName)
if ctx.Written() {
return
}
Expand All @@ -193,24 +191,24 @@ func EditUser(ctx *context.APIContext) {
ctx.Error(http.StatusBadRequest, "PasswordPwned", errors.New("PasswordPwned"))
return
}
if u.Salt, err = user_model.GetUserSalt(); err != nil {
if ctx.ContextUser.Salt, err = user_model.GetUserSalt(); err != nil {
ctx.Error(http.StatusInternalServerError, "UpdateUser", err)
return
}
if err = u.SetPassword(form.Password); err != nil {
if err = ctx.ContextUser.SetPassword(form.Password); err != nil {
ctx.InternalServerError(err)
return
}
}

if form.MustChangePassword != nil {
u.MustChangePassword = *form.MustChangePassword
ctx.ContextUser.MustChangePassword = *form.MustChangePassword
}

u.LoginName = form.LoginName
ctx.ContextUser.LoginName = form.LoginName

if form.FullName != nil {
u.FullName = *form.FullName
ctx.ContextUser.FullName = *form.FullName
}
var emailChanged bool
if form.Email != nil {
Expand All @@ -225,47 +223,47 @@ func EditUser(ctx *context.APIContext) {
return
}

emailChanged = !strings.EqualFold(u.Email, email)
u.Email = email
emailChanged = !strings.EqualFold(ctx.ContextUser.Email, email)
ctx.ContextUser.Email = email
}
if form.Website != nil {
u.Website = *form.Website
ctx.ContextUser.Website = *form.Website
}
if form.Location != nil {
u.Location = *form.Location
ctx.ContextUser.Location = *form.Location
}
if form.Description != nil {
u.Description = *form.Description
ctx.ContextUser.Description = *form.Description
}
if form.Active != nil {
u.IsActive = *form.Active
ctx.ContextUser.IsActive = *form.Active
}
if len(form.Visibility) != 0 {
u.Visibility = api.VisibilityModes[form.Visibility]
ctx.ContextUser.Visibility = api.VisibilityModes[form.Visibility]
}
if form.Admin != nil {
u.IsAdmin = *form.Admin
ctx.ContextUser.IsAdmin = *form.Admin
}
if form.AllowGitHook != nil {
u.AllowGitHook = *form.AllowGitHook
ctx.ContextUser.AllowGitHook = *form.AllowGitHook
}
if form.AllowImportLocal != nil {
u.AllowImportLocal = *form.AllowImportLocal
ctx.ContextUser.AllowImportLocal = *form.AllowImportLocal
}
if form.MaxRepoCreation != nil {
u.MaxRepoCreation = *form.MaxRepoCreation
ctx.ContextUser.MaxRepoCreation = *form.MaxRepoCreation
}
if form.AllowCreateOrganization != nil {
u.AllowCreateOrganization = *form.AllowCreateOrganization
ctx.ContextUser.AllowCreateOrganization = *form.AllowCreateOrganization
}
if form.ProhibitLogin != nil {
u.ProhibitLogin = *form.ProhibitLogin
ctx.ContextUser.ProhibitLogin = *form.ProhibitLogin
}
if form.Restricted != nil {
u.IsRestricted = *form.Restricted
ctx.ContextUser.IsRestricted = *form.Restricted
}

if err := user_model.UpdateUser(u, emailChanged); err != nil {
if err := user_model.UpdateUser(ctx.ContextUser, emailChanged); err != nil {
if user_model.IsErrEmailAlreadyUsed(err) ||
user_model.IsErrEmailCharIsNotSupported(err) ||
user_model.IsErrEmailInvalid(err) {
Expand All @@ -275,9 +273,9 @@ func EditUser(ctx *context.APIContext) {
}
return
}
log.Trace("Account profile updated by admin (%s): %s", ctx.Doer.Name, u.Name)
log.Trace("Account profile updated by admin (%s): %s", ctx.Doer.Name, ctx.ContextUser.Name)

ctx.JSON(http.StatusOK, convert.ToUser(u, ctx.Doer))
ctx.JSON(http.StatusOK, convert.ToUser(ctx.ContextUser, ctx.Doer))
}

// DeleteUser api for deleting a user
Expand All @@ -301,17 +299,12 @@ func DeleteUser(ctx *context.APIContext) {
// "422":
// "$ref": "#/responses/validationError"

u := user.GetUserByParams(ctx)
if ctx.Written() {
return
}

if u.IsOrganization() {
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("%s is an organization not a user", u.Name))
if ctx.ContextUser.IsOrganization() {
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("%s is an organization not a user", ctx.ContextUser.Name))
return
}

if err := user_service.DeleteUser(u); err != nil {
if err := user_service.DeleteUser(ctx.ContextUser); err != nil {
if models.IsErrUserOwnRepos(err) ||
models.IsErrUserHasOrgs(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
Expand All @@ -320,7 +313,7 @@ func DeleteUser(ctx *context.APIContext) {
}
return
}
log.Trace("Account deleted by admin(%s): %s", ctx.Doer.Name, u.Name)
log.Trace("Account deleted by admin(%s): %s", ctx.Doer.Name, ctx.ContextUser.Name)

ctx.Status(http.StatusNoContent)
}
Expand Down Expand Up @@ -351,12 +344,10 @@ func CreatePublicKey(ctx *context.APIContext) {
// "$ref": "#/responses/forbidden"
// "422":
// "$ref": "#/responses/validationError"

form := web.GetForm(ctx).(*api.CreateKeyOption)
u := user.GetUserByParams(ctx)
if ctx.Written() {
return
}
user.CreateUserPublicKey(ctx, *form, u.ID)

user.CreateUserPublicKey(ctx, *form, ctx.ContextUser.ID)
}

// DeleteUserPublicKey api for deleting a user's public key
Expand Down Expand Up @@ -386,12 +377,7 @@ func DeleteUserPublicKey(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

u := user.GetUserByParams(ctx)
if ctx.Written() {
return
}

if err := asymkey_service.DeletePublicKey(u, ctx.ParamsInt64(":id")); err != nil {
if err := asymkey_service.DeletePublicKey(ctx.ContextUser, ctx.ParamsInt64(":id")); err != nil {
if asymkey_model.IsErrKeyNotExist(err) {
ctx.NotFound()
} else if asymkey_model.IsErrKeyAccessDenied(err) {
Expand All @@ -401,7 +387,7 @@ func DeleteUserPublicKey(ctx *context.APIContext) {
}
return
}
log.Trace("Key deleted by admin(%s): %s", ctx.Doer.Name, u.Name)
log.Trace("Key deleted by admin(%s): %s", ctx.Doer.Name, ctx.ContextUser.Name)

ctx.Status(http.StatusNoContent)
}
Expand Down
17 changes: 12 additions & 5 deletions routers/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import (
"code.gitea.io/gitea/routers/api/v1/settings"
"code.gitea.io/gitea/routers/api/v1/user"
"code.gitea.io/gitea/services/auth"
context_service "code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/forms"

_ "code.gitea.io/gitea/routers/api/v1/swagger" // for swagger generation
Expand Down Expand Up @@ -156,6 +157,7 @@ func repoAssignment() func(ctx *context.APIContext) {
}
}
ctx.Repo.Owner = owner
ctx.ContextUser = owner

// Get repository.
repo, err := repo_model.GetRepositoryByName(owner.ID, repoName)
Expand Down Expand Up @@ -441,6 +443,7 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) {
}
return
}
ctx.ContextUser = ctx.Org.Organization.AsUser()
}

if assignTeam {
Expand Down Expand Up @@ -636,7 +639,7 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route {
Post(bind(api.CreateAccessTokenOption{}), user.CreateAccessToken)
m.Combo("/{id}").Delete(user.DeleteAccessToken)
}, reqBasicOrRevProxyAuth())
})
}, context_service.UserAssignmentAPI())
})

m.Group("/users", func() {
Expand All @@ -653,7 +656,7 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route {
m.Get("/starred", user.GetStarredRepos)

m.Get("/subscriptions", user.GetWatchedRepos)
})
}, context_service.UserAssignmentAPI())
}, reqToken())

m.Group("/user", func() {
Expand All @@ -669,7 +672,11 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route {
m.Get("/followers", user.ListMyFollowers)
m.Group("/following", func() {
m.Get("", user.ListMyFollowing)
m.Combo("/{username}").Get(user.CheckMyFollowing).Put(user.Follow).Delete(user.Unfollow)
m.Group("/{username}", func() {
m.Get("", user.CheckMyFollowing)
m.Put("", user.Follow)
m.Delete("", user.Unfollow)
}, context_service.UserAssignmentAPI())
})

m.Group("/keys", func() {
Expand Down Expand Up @@ -1005,7 +1012,7 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route {
m.Group("/users/{username}/orgs", func() {
m.Get("", org.ListUserOrgs)
m.Get("/{org}/permissions", reqToken(), org.GetUserOrgsPermissions)
})
}, context_service.UserAssignmentAPI())
m.Post("/orgs", reqToken(), bind(api.CreateOrgOption{}), org.Create)
m.Get("/orgs", org.GetAll)
m.Group("/orgs/{org}", func() {
Expand Down Expand Up @@ -1083,7 +1090,7 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route {
m.Get("/orgs", org.ListUserOrgs)
m.Post("/orgs", bind(api.CreateOrgOption{}), admin.CreateOrg)
m.Post("/repos", bind(api.CreateRepoOption{}), admin.CreateRepo)
})
}, context_service.UserAssignmentAPI())
})
m.Group("/unadopted", func() {
m.Get("", admin.ListUnadoptedRepositories)
Expand Down
Loading

0 comments on commit 92ab3bc

Please sign in to comment.