From a3881ffa3d90f3f017024f701ca8ef420d492224 Mon Sep 17 00:00:00 2001 From: Marcell Mars Date: Fri, 22 Nov 2024 05:06:41 +0100 Subject: [PATCH 1/8] Enhancing Gitea OAuth2 Provider with Granular Scopes for Resource Access (#32573) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve #31609 This PR was initiated following my personal research to find the lightest possible Single Sign-On solution for self-hosted setups. The existing solutions often seemed too enterprise-oriented, involving many moving parts and services, demanding significant resources while promising planetary-scale capabilities. Others were adequate in supporting basic OAuth2 flows but lacked proper user management features, such as a change password UI. Gitea hits the sweet spot for me, provided it supports more granular access permissions for resources under users who accept the OAuth2 application. This PR aims to introduce granularity in handling user resources as nonintrusively and simply as possible. It allows third parties to inform users about their intent to not ask for the full access and instead request a specific, reduced scope. If the provided scopes are **only** the typical ones for OIDC/OAuth2—`openid`, `profile`, `email`, and `groups`—everything remains unchanged (currently full access to user's resources). Additionally, this PR supports processing scopes already introduced with [personal tokens](https://docs.gitea.com/development/oauth2-provider#scopes) (e.g. `read:user`, `write:issue`, `read:group`, `write:repository`...) Personal tokens define scopes around specific resources: user info, repositories, issues, packages, organizations, notifications, miscellaneous, admin, and activitypub, with access delineated by read and/or write permissions. The initial case I wanted to address was to have Gitea act as an OAuth2 Identity Provider. To achieve that, with this PR, I would only add `openid public-only` to provide access token to the third party to authenticate the Gitea's user but no further access to the API and users resources. Another example: if a third party wanted to interact solely with Issues, it would need to add `read:user` (for authorization) and `read:issue`/`write:issue` to manage Issues. My approach is based on my understanding of how scopes can be utilized, supported by examples like [Sample Use Cases: Scopes and Claims](https://auth0.com/docs/get-started/apis/scopes/sample-use-cases-scopes-and-claims) on auth0.com. I renamed `CheckOAuthAccessToken` to `GetOAuthAccessTokenScopeAndUserID` so now it returns AccessTokenScope and user's ID. In the case of additional scopes in `userIDFromToken` the default `all` would be reduced to whatever was asked via those scopes. The main difference is the opportunity to reduce the permissions from `all`, as is currently the case, to what is provided by the additional scopes described above. Screenshots: ![Screenshot_20241121_121405](https://github.com/user-attachments/assets/29deaed7-4333-4b02-8898-b822e6f2463e) ![Screenshot_20241121_120211](https://github.com/user-attachments/assets/7a4a4ef7-409c-4116-9d5f-2fe00eb37167) ![Screenshot_20241121_120119](https://github.com/user-attachments/assets/aa52c1a2-212d-4e64-bcdf-7122cee49eb6) ![Screenshot_20241121_120018](https://github.com/user-attachments/assets/9eac318c-e381-4ea9-9e2c-3a3f60319e47) --------- Co-authored-by: wxiaoguang --- options/locale/locale_en-US.ini | 1 + routers/web/auth/oauth2_provider.go | 16 +- services/auth/basic.go | 4 +- services/auth/oauth2.go | 24 +- services/oauth2_provider/access_token.go | 40 +- .../oauth2_provider/additional_scopes_test.go | 35 ++ templates/user/auth/grant.tmpl | 5 +- tests/integration/oauth_test.go | 430 ++++++++++++++++++ 8 files changed, 537 insertions(+), 18 deletions(-) create mode 100644 services/oauth2_provider/additional_scopes_test.go diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b21c376002e5e..9945eb4949d68 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -459,6 +459,7 @@ authorize_application = Authorize Application authorize_redirect_notice = You will be redirected to %s if you authorize this application. authorize_application_created_by = This application was created by %s. authorize_application_description = If you grant the access, it will be able to access and write to all your account information, including private repos and organisations. +authorize_application_with_scopes = With scopes: %s authorize_title = Authorize "%s" to access your account? authorization_failed = Authorization failed authorization_failed_desc = The authorization failed because we detected an invalid request. Please contact the maintainer of the app you have tried to authorize. diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 2ccc4a2253742..1aebc047bd3d7 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -104,7 +104,18 @@ func InfoOAuth(ctx *context.Context) { Picture: ctx.Doer.AvatarLink(ctx), } - groups, err := oauth2_provider.GetOAuthGroupsForUser(ctx, ctx.Doer) + var accessTokenScope auth.AccessTokenScope + if auHead := ctx.Req.Header.Get("Authorization"); auHead != "" { + auths := strings.Fields(auHead) + if len(auths) == 2 && (auths[0] == "token" || strings.ToLower(auths[0]) == "bearer") { + accessTokenScope, _ = auth_service.GetOAuthAccessTokenScopeAndUserID(ctx, auths[1]) + } + } + + // since version 1.22 does not verify if groups should be public-only, + // onlyPublicGroups will be set only if 'public-only' is included in a valid scope + onlyPublicGroups, _ := accessTokenScope.PublicOnly() + groups, err := oauth2_provider.GetOAuthGroupsForUser(ctx, ctx.Doer, onlyPublicGroups) if err != nil { ctx.ServerError("Oauth groups for user", err) return @@ -304,6 +315,9 @@ func AuthorizeOAuth(ctx *context.Context) { return } + // check if additional scopes + ctx.Data["AdditionalScopes"] = oauth2_provider.GrantAdditionalScopes(form.Scope) != auth.AccessTokenScopeAll + // show authorize page to grant access ctx.Data["Application"] = app ctx.Data["RedirectURI"] = form.RedirectURI diff --git a/services/auth/basic.go b/services/auth/basic.go index 1f6c3a442d1d8..6a05b2fe53043 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -77,8 +77,8 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore log.Trace("Basic Authorization: Attempting login with username as token") } - // check oauth2 token - uid := CheckOAuthAccessToken(req.Context(), authToken) + // get oauth2 token's user's ID + _, uid := GetOAuthAccessTokenScopeAndUserID(req.Context(), authToken) if uid != 0 { log.Trace("Basic Authorization: Valid OAuthAccessToken for user[%d]", uid) diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 251ae5a244b5c..6f2cadd4abb5e 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -26,33 +26,35 @@ var ( _ Method = &OAuth2{} ) -// CheckOAuthAccessToken returns uid of user from oauth token -func CheckOAuthAccessToken(ctx context.Context, accessToken string) int64 { +// GetOAuthAccessTokenScopeAndUserID returns access token scope and user id +func GetOAuthAccessTokenScopeAndUserID(ctx context.Context, accessToken string) (auth_model.AccessTokenScope, int64) { + var accessTokenScope auth_model.AccessTokenScope if !setting.OAuth2.Enabled { - return 0 + return accessTokenScope, 0 } // JWT tokens require a ".", if the token isn't like that, return early if !strings.Contains(accessToken, ".") { - return 0 + return accessTokenScope, 0 } token, err := oauth2_provider.ParseToken(accessToken, oauth2_provider.DefaultSigningKey) if err != nil { log.Trace("oauth2.ParseToken: %v", err) - return 0 + return accessTokenScope, 0 } var grant *auth_model.OAuth2Grant if grant, err = auth_model.GetOAuth2GrantByID(ctx, token.GrantID); err != nil || grant == nil { - return 0 + return accessTokenScope, 0 } if token.Kind != oauth2_provider.KindAccessToken { - return 0 + return accessTokenScope, 0 } if token.ExpiresAt.Before(time.Now()) || token.IssuedAt.After(time.Now()) { - return 0 + return accessTokenScope, 0 } - return grant.UserID + accessTokenScope = oauth2_provider.GrantAdditionalScopes(grant.Scope) + return accessTokenScope, grant.UserID } // CheckTaskIsRunning verifies that the TaskID corresponds to a running task @@ -120,10 +122,10 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store Dat } // Otherwise, check if this is an OAuth access token - uid := CheckOAuthAccessToken(ctx, tokenSHA) + accessTokenScope, uid := GetOAuthAccessTokenScopeAndUserID(ctx, tokenSHA) if uid != 0 { store.GetData()["IsApiToken"] = true - store.GetData()["ApiTokenScope"] = auth_model.AccessTokenScopeAll // fallback to all + store.GetData()["ApiTokenScope"] = accessTokenScope } return uid } diff --git a/services/oauth2_provider/access_token.go b/services/oauth2_provider/access_token.go index dd3f24eeef242..d94e15d5f2597 100644 --- a/services/oauth2_provider/access_token.go +++ b/services/oauth2_provider/access_token.go @@ -6,6 +6,8 @@ package oauth2_provider //nolint import ( "context" "fmt" + "slices" + "strings" auth "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" @@ -69,6 +71,32 @@ type AccessTokenResponse struct { IDToken string `json:"id_token,omitempty"` } +// GrantAdditionalScopes returns valid scopes coming from grant +func GrantAdditionalScopes(grantScopes string) auth.AccessTokenScope { + // scopes_supported from templates/user/auth/oidc_wellknown.tmpl + scopesSupported := []string{ + "openid", + "profile", + "email", + "groups", + } + + var tokenScopes []string + for _, tokenScope := range strings.Split(grantScopes, " ") { + if slices.Index(scopesSupported, tokenScope) == -1 { + tokenScopes = append(tokenScopes, tokenScope) + } + } + + // since version 1.22, access tokens grant full access to the API + // with this access is reduced only if additional scopes are provided + accessTokenScope := auth.AccessTokenScope(strings.Join(tokenScopes, ",")) + if accessTokenWithAdditionalScopes, err := accessTokenScope.Normalize(); err == nil && len(tokenScopes) > 0 { + return accessTokenWithAdditionalScopes + } + return auth.AccessTokenScopeAll +} + func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, serverKey, clientKey JWTSigningKey) (*AccessTokenResponse, *AccessTokenError) { if setting.OAuth2.InvalidateRefreshTokens { if err := grant.IncreaseCounter(ctx); err != nil { @@ -161,7 +189,13 @@ func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, server idToken.EmailVerified = user.IsActive } if grant.ScopeContains("groups") { - groups, err := GetOAuthGroupsForUser(ctx, user) + accessTokenScope := GrantAdditionalScopes(grant.Scope) + + // since version 1.22 does not verify if groups should be public-only, + // onlyPublicGroups will be set only if 'public-only' is included in a valid scope + onlyPublicGroups, _ := accessTokenScope.PublicOnly() + + groups, err := GetOAuthGroupsForUser(ctx, user, onlyPublicGroups) if err != nil { log.Error("Error getting groups: %v", err) return nil, &AccessTokenError{ @@ -192,10 +226,10 @@ func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, server // returns a list of "org" and "org:team" strings, // that the given user is a part of. -func GetOAuthGroupsForUser(ctx context.Context, user *user_model.User) ([]string, error) { +func GetOAuthGroupsForUser(ctx context.Context, user *user_model.User, onlyPublicGroups bool) ([]string, error) { orgs, err := db.Find[org_model.Organization](ctx, org_model.FindOrgOptions{ UserID: user.ID, - IncludePrivate: true, + IncludePrivate: !onlyPublicGroups, }) if err != nil { return nil, fmt.Errorf("GetUserOrgList: %w", err) diff --git a/services/oauth2_provider/additional_scopes_test.go b/services/oauth2_provider/additional_scopes_test.go new file mode 100644 index 0000000000000..d239229f4be78 --- /dev/null +++ b/services/oauth2_provider/additional_scopes_test.go @@ -0,0 +1,35 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package oauth2_provider //nolint + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGrantAdditionalScopes(t *testing.T) { + tests := []struct { + grantScopes string + expectedScopes string + }{ + {"openid profile email", "all"}, + {"openid profile email groups", "all"}, + {"openid profile email all", "all"}, + {"openid profile email read:user all", "all"}, + {"openid profile email groups read:user", "read:user"}, + {"read:user read:repository", "read:repository,read:user"}, + {"read:user write:issue public-only", "public-only,write:issue,read:user"}, + {"openid profile email read:user", "read:user"}, + {"read:invalid_scope", "all"}, + {"read:invalid_scope,write:scope_invalid,just-plain-wrong", "all"}, + } + + for _, test := range tests { + t.Run(test.grantScopes, func(t *testing.T) { + result := GrantAdditionalScopes(test.grantScopes) + assert.Equal(t, test.expectedScopes, string(result)) + }) + } +} diff --git a/templates/user/auth/grant.tmpl b/templates/user/auth/grant.tmpl index a18a3bd27a23c..4031dd7a632cd 100644 --- a/templates/user/auth/grant.tmpl +++ b/templates/user/auth/grant.tmpl @@ -8,8 +8,11 @@
{{template "base/alert" .}}

+ {{if not .AdditionalScopes}} {{ctx.Locale.Tr "auth.authorize_application_description"}}
- {{ctx.Locale.Tr "auth.authorize_application_created_by" .ApplicationCreatorLinkHTML}} + {{end}} + {{ctx.Locale.Tr "auth.authorize_application_created_by" .ApplicationCreatorLinkHTML}}
+ {{ctx.Locale.Tr "auth.authorize_application_with_scopes" (HTMLFormat "%s" .Scope)}}

diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index b32d365b04d15..feb262b50e2cf 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -5,16 +5,25 @@ package integration import ( "bytes" + "encoding/base64" + "fmt" "io" "net/http" + "strings" "testing" + auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" oauth2_provider "code.gitea.io/gitea/services/oauth2_provider" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAuthorizeNoClientID(t *testing.T) { @@ -477,3 +486,424 @@ func TestOAuthIntrospection(t *testing.T) { resp = MakeRequest(t, req, http.StatusUnauthorized) assert.Contains(t, resp.Body.String(), "no valid authorization") } + +func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + appBody := api.CreateOAuth2ApplicationOptions{ + Name: "oauth-provider-scopes-test", + RedirectURIs: []string{ + "a", + }, + ConfidentialClient: true, + } + + req := NewRequestWithJSON(t, "POST", "/api/v1/user/applications/oauth2", &appBody). + AddBasicAuth(user.Name) + resp := MakeRequest(t, req, http.StatusCreated) + + var app *api.OAuth2Application + DecodeJSON(t, resp, &app) + + grant := &auth_model.OAuth2Grant{ + ApplicationID: app.ID, + UserID: user.ID, + Scope: "openid read:user", + } + + err := db.Insert(db.DefaultContext, grant) + require.NoError(t, err) + + assert.Contains(t, grant.Scope, "openid read:user") + + ctx := loginUser(t, user.Name) + + authorizeURL := fmt.Sprintf("/login/oauth/authorize?client_id=%s&redirect_uri=a&response_type=code&state=thestate", app.ClientID) + authorizeReq := NewRequest(t, "GET", authorizeURL) + authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) + + authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] + + accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": app.ClientID, + "client_secret": app.ClientSecret, + "redirect_uri": "a", + "code": authcode, + }) + accessTokenResp := ctx.MakeRequest(t, accessTokenReq, 200) + type response struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresIn int64 `json:"expires_in"` + RefreshToken string `json:"refresh_token"` + } + parsed := new(response) + + require.NoError(t, json.Unmarshal(accessTokenResp.Body.Bytes(), parsed)) + userReq := NewRequest(t, "GET", "/api/v1/user") + userReq.SetHeader("Authorization", "Bearer "+parsed.AccessToken) + userResp := MakeRequest(t, userReq, http.StatusOK) + + type userResponse struct { + Login string `json:"login"` + Email string `json:"email"` + } + + userParsed := new(userResponse) + require.NoError(t, json.Unmarshal(userResp.Body.Bytes(), userParsed)) + assert.Contains(t, userParsed.Email, "user2@example.com") + + errorReq := NewRequest(t, "GET", "/api/v1/users/user2/repos") + errorReq.SetHeader("Authorization", "Bearer "+parsed.AccessToken) + errorResp := MakeRequest(t, errorReq, http.StatusForbidden) + + type errorResponse struct { + Message string `json:"message"` + } + + errorParsed := new(errorResponse) + require.NoError(t, json.Unmarshal(errorResp.Body.Bytes(), errorParsed)) + assert.Contains(t, errorParsed.Message, "token does not have at least one of required scope(s): [read:repository]") +} + +func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + appBody := api.CreateOAuth2ApplicationOptions{ + Name: "oauth-provider-scopes-test", + RedirectURIs: []string{ + "a", + }, + ConfidentialClient: true, + } + + req := NewRequestWithJSON(t, "POST", "/api/v1/user/applications/oauth2", &appBody). + AddBasicAuth(user.Name) + resp := MakeRequest(t, req, http.StatusCreated) + + var app *api.OAuth2Application + DecodeJSON(t, resp, &app) + + grant := &auth_model.OAuth2Grant{ + ApplicationID: app.ID, + UserID: user.ID, + Scope: "openid read:user read:repository", + } + + err := db.Insert(db.DefaultContext, grant) + require.NoError(t, err) + + assert.Contains(t, grant.Scope, "openid read:user read:repository") + + ctx := loginUser(t, user.Name) + + authorizeURL := fmt.Sprintf("/login/oauth/authorize?client_id=%s&redirect_uri=a&response_type=code&state=thestate", app.ClientID) + authorizeReq := NewRequest(t, "GET", authorizeURL) + authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) + + authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] + accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": app.ClientID, + "client_secret": app.ClientSecret, + "redirect_uri": "a", + "code": authcode, + }) + accessTokenResp := ctx.MakeRequest(t, accessTokenReq, http.StatusOK) + type response struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresIn int64 `json:"expires_in"` + RefreshToken string `json:"refresh_token"` + } + parsed := new(response) + + require.NoError(t, json.Unmarshal(accessTokenResp.Body.Bytes(), parsed)) + userReq := NewRequest(t, "GET", "/api/v1/users/user2/repos") + userReq.SetHeader("Authorization", "Bearer "+parsed.AccessToken) + userResp := MakeRequest(t, userReq, http.StatusOK) + + type repo struct { + FullRepoName string `json:"full_name"` + Private bool `json:"private"` + } + + var reposCaptured []repo + require.NoError(t, json.Unmarshal(userResp.Body.Bytes(), &reposCaptured)) + + reposExpected := []repo{ + { + FullRepoName: "user2/repo1", + Private: false, + }, + { + FullRepoName: "user2/repo2", + Private: true, + }, + { + FullRepoName: "user2/repo15", + Private: true, + }, + { + FullRepoName: "user2/repo16", + Private: true, + }, + { + FullRepoName: "user2/repo20", + Private: true, + }, + { + FullRepoName: "user2/utf8", + Private: false, + }, + { + FullRepoName: "user2/commits_search_test", + Private: false, + }, + { + FullRepoName: "user2/git_hooks_test", + Private: false, + }, + { + FullRepoName: "user2/glob", + Private: false, + }, + { + FullRepoName: "user2/lfs", + Private: true, + }, + { + FullRepoName: "user2/scoped_label", + Private: true, + }, + { + FullRepoName: "user2/readme-test", + Private: true, + }, + { + FullRepoName: "user2/repo-release", + Private: false, + }, + { + FullRepoName: "user2/commitsonpr", + Private: false, + }, + { + FullRepoName: "user2/test_commit_revert", + Private: true, + }, + } + assert.Equal(t, reposExpected, reposCaptured) + + errorReq := NewRequest(t, "GET", "/api/v1/users/user2/orgs") + errorReq.SetHeader("Authorization", "Bearer "+parsed.AccessToken) + errorResp := MakeRequest(t, errorReq, http.StatusForbidden) + + type errorResponse struct { + Message string `json:"message"` + } + + errorParsed := new(errorResponse) + require.NoError(t, json.Unmarshal(errorResp.Body.Bytes(), errorParsed)) + assert.Contains(t, errorParsed.Message, "token does not have at least one of required scope(s): [read:user read:organization]") +} + +func TestOAuth_GrantScopesClaimPublicOnlyGroups(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"}) + + appBody := api.CreateOAuth2ApplicationOptions{ + Name: "oauth-provider-scopes-test", + RedirectURIs: []string{ + "a", + }, + ConfidentialClient: true, + } + + appReq := NewRequestWithJSON(t, "POST", "/api/v1/user/applications/oauth2", &appBody). + AddBasicAuth(user.Name) + appResp := MakeRequest(t, appReq, http.StatusCreated) + + var app *api.OAuth2Application + DecodeJSON(t, appResp, &app) + + grant := &auth_model.OAuth2Grant{ + ApplicationID: app.ID, + UserID: user.ID, + Scope: "openid groups read:user public-only", + } + + err := db.Insert(db.DefaultContext, grant) + require.NoError(t, err) + + assert.ElementsMatch(t, []string{"openid", "groups", "read:user", "public-only"}, strings.Split(grant.Scope, " ")) + + ctx := loginUser(t, user.Name) + + authorizeURL := fmt.Sprintf("/login/oauth/authorize?client_id=%s&redirect_uri=a&response_type=code&state=thestate", app.ClientID) + authorizeReq := NewRequest(t, "GET", authorizeURL) + authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) + + authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] + + accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": app.ClientID, + "client_secret": app.ClientSecret, + "redirect_uri": "a", + "code": authcode, + }) + accessTokenResp := ctx.MakeRequest(t, accessTokenReq, http.StatusOK) + type response struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresIn int64 `json:"expires_in"` + RefreshToken string `json:"refresh_token"` + IDToken string `json:"id_token,omitempty"` + } + parsed := new(response) + require.NoError(t, json.Unmarshal(accessTokenResp.Body.Bytes(), parsed)) + parts := strings.Split(parsed.IDToken, ".") + + payload, _ := base64.RawURLEncoding.DecodeString(parts[1]) + type IDTokenClaims struct { + Groups []string `json:"groups"` + } + + claims := new(IDTokenClaims) + require.NoError(t, json.Unmarshal(payload, claims)) + + userinfoReq := NewRequest(t, "GET", "/login/oauth/userinfo") + userinfoReq.SetHeader("Authorization", "Bearer "+parsed.AccessToken) + userinfoResp := MakeRequest(t, userinfoReq, http.StatusOK) + + type userinfoResponse struct { + Login string `json:"login"` + Email string `json:"email"` + Groups []string `json:"groups"` + } + + userinfoParsed := new(userinfoResponse) + require.NoError(t, json.Unmarshal(userinfoResp.Body.Bytes(), userinfoParsed)) + assert.Contains(t, userinfoParsed.Email, "user2@example.com") + + // test both id_token and call to /login/oauth/userinfo + for _, publicGroup := range []string{ + "org17", + "org17:test_team", + "org3", + "org3:owners", + "org3:team1", + "org3:teamcreaterepo", + } { + assert.Contains(t, claims.Groups, publicGroup) + assert.Contains(t, userinfoParsed.Groups, publicGroup) + } + for _, privateGroup := range []string{ + "private_org35", + "private_org35_team24", + } { + assert.NotContains(t, claims.Groups, privateGroup) + assert.NotContains(t, userinfoParsed.Groups, privateGroup) + } +} + +func TestOAuth_GrantScopesClaimAllGroups(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"}) + + appBody := api.CreateOAuth2ApplicationOptions{ + Name: "oauth-provider-scopes-test", + RedirectURIs: []string{ + "a", + }, + ConfidentialClient: true, + } + + appReq := NewRequestWithJSON(t, "POST", "/api/v1/user/applications/oauth2", &appBody). + AddBasicAuth(user.Name) + appResp := MakeRequest(t, appReq, http.StatusCreated) + + var app *api.OAuth2Application + DecodeJSON(t, appResp, &app) + + grant := &auth_model.OAuth2Grant{ + ApplicationID: app.ID, + UserID: user.ID, + Scope: "openid groups", + } + + err := db.Insert(db.DefaultContext, grant) + require.NoError(t, err) + + assert.ElementsMatch(t, []string{"openid", "groups"}, strings.Split(grant.Scope, " ")) + + ctx := loginUser(t, user.Name) + + authorizeURL := fmt.Sprintf("/login/oauth/authorize?client_id=%s&redirect_uri=a&response_type=code&state=thestate", app.ClientID) + authorizeReq := NewRequest(t, "GET", authorizeURL) + authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) + + authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] + + accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": app.ClientID, + "client_secret": app.ClientSecret, + "redirect_uri": "a", + "code": authcode, + }) + accessTokenResp := ctx.MakeRequest(t, accessTokenReq, http.StatusOK) + type response struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresIn int64 `json:"expires_in"` + RefreshToken string `json:"refresh_token"` + IDToken string `json:"id_token,omitempty"` + } + parsed := new(response) + require.NoError(t, json.Unmarshal(accessTokenResp.Body.Bytes(), parsed)) + parts := strings.Split(parsed.IDToken, ".") + + payload, _ := base64.RawURLEncoding.DecodeString(parts[1]) + type IDTokenClaims struct { + Groups []string `json:"groups"` + } + + claims := new(IDTokenClaims) + require.NoError(t, json.Unmarshal(payload, claims)) + + userinfoReq := NewRequest(t, "GET", "/login/oauth/userinfo") + userinfoReq.SetHeader("Authorization", "Bearer "+parsed.AccessToken) + userinfoResp := MakeRequest(t, userinfoReq, http.StatusOK) + + type userinfoResponse struct { + Login string `json:"login"` + Email string `json:"email"` + Groups []string `json:"groups"` + } + + userinfoParsed := new(userinfoResponse) + require.NoError(t, json.Unmarshal(userinfoResp.Body.Bytes(), userinfoParsed)) + assert.Contains(t, userinfoParsed.Email, "user2@example.com") + + // test both id_token and call to /login/oauth/userinfo + for _, group := range []string{ + "org17", + "org17:test_team", + "org3", + "org3:owners", + "org3:team1", + "org3:teamcreaterepo", + "private_org35", + "private_org35:team24", + } { + assert.Contains(t, claims.Groups, group) + assert.Contains(t, userinfoParsed.Groups, group) + } +} From 81ac8d914cf5fdfaad3c206223ad0ace1e8c1dcd Mon Sep 17 00:00:00 2001 From: Kerwin Bryant Date: Fri, 22 Nov 2024 12:33:31 +0800 Subject: [PATCH 2/8] Style unification for the issue_management area (#32605) Style unification for the issue_management area (consistent across the layout before: ![1732237277916](https://github.com/user-attachments/assets/52a20b2d-d6a4-4118-9cdf-9b377115b7f7) ![1732237288802](https://github.com/user-attachments/assets/05592fe8-cab2-412b-99bc-f0a201c08413) ![1732237299849](https://github.com/user-attachments/assets/8be4a891-c514-4983-bad4-fcc5a7a9d838) after: ![1732237471086](https://github.com/user-attachments/assets/0bd19ef6-79c1-490a-8ffa-6a42208befd9) --------- Co-authored-by: wxiaoguang --- templates/repo/issue/sidebar/issue_management.tmpl | 4 ++-- templates/repo/issue/sidebar/milestone_list.tmpl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/repo/issue/sidebar/issue_management.tmpl b/templates/repo/issue/sidebar/issue_management.tmpl index 3342d992129cd..76edf16e03abe 100644 --- a/templates/repo/issue/sidebar/issue_management.tmpl +++ b/templates/repo/issue/sidebar/issue_management.tmpl @@ -6,10 +6,10 @@ {{$.CsrfTokenHtml}} diff --git a/templates/repo/issue/sidebar/milestone_list.tmpl b/templates/repo/issue/sidebar/milestone_list.tmpl index 4a7f3f8ad8808..2a7b6f6009f15 100644 --- a/templates/repo/issue/sidebar/milestone_list.tmpl +++ b/templates/repo/issue/sidebar/milestone_list.tmpl @@ -39,7 +39,7 @@ {{end}} {{end}}
- {{end}} + {{end}} From c4e27cb27b99dd9528c999fdc8b1073f28be6313 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 22 Nov 2024 13:48:09 +0800 Subject: [PATCH 3/8] Refactor markup render system (#32589) This PR mainly moves some code and introduces `RenderContext.WithXxx` functions --- models/issues/comment_code.go | 14 +- models/repo/repo.go | 5 +- modules/csv/csv.go | 4 +- modules/csv/csv_test.go | 7 +- modules/markup/asciicast/asciicast.go | 8 +- modules/markup/console/console_test.go | 5 +- modules/markup/csv/csv.go | 4 +- modules/markup/csv/csv_test.go | 5 +- modules/markup/external/external.go | 19 +-- modules/markup/html_codepreview.go | 4 +- modules/markup/html_codepreview_test.go | 6 +- modules/markup/html_commit.go | 26 ++-- modules/markup/html_internal_test.go | 78 +++++----- modules/markup/html_issue.go | 32 ++--- modules/markup/html_link.go | 10 +- modules/markup/html_mention.go | 10 +- modules/markup/html_node.go | 4 +- modules/markup/html_test.go | 124 ++++------------ modules/markup/markdown/goldmark.go | 2 +- modules/markup/markdown/markdown.go | 4 +- modules/markup/markdown/markdown_test.go | 93 ++++-------- modules/markup/markdown/transform_image.go | 2 +- modules/markup/orgmode/orgmode.go | 10 +- modules/markup/orgmode/orgmode_test.go | 43 ++---- modules/markup/render.go | 157 ++++++++++++++++----- modules/templates/util_render.go | 27 +--- routers/api/v1/misc/markup.go | 4 +- routers/common/markup.go | 32 ++--- routers/web/feed/convert.go | 25 ++-- routers/web/feed/profile.go | 11 +- routers/web/org/home.go | 13 +- routers/web/repo/commit.go | 15 +- routers/web/repo/compare.go | 4 +- routers/web/repo/issue.go | 15 +- routers/web/repo/issue_comment.go | 15 +- routers/web/repo/issue_view.go | 45 +++--- routers/web/repo/milestone.go | 30 ++-- routers/web/repo/projects.go | 30 ++-- routers/web/repo/release.go | 15 +- routers/web/repo/render.go | 17 ++- routers/web/repo/view.go | 51 ++++--- routers/web/repo/wiki.go | 10 +- routers/web/shared/user/header.go | 5 +- routers/web/user/home.go | 13 +- routers/web/user/profile.go | 11 +- services/context/org.go | 4 +- services/mailer/mail.go | 14 +- services/mailer/mail_release.go | 13 +- tests/fuzz/fuzz_test.go | 17 +-- 49 files changed, 486 insertions(+), 626 deletions(-) diff --git a/models/issues/comment_code.go b/models/issues/comment_code.go index 6f23d3326a223..751550f37a6b6 100644 --- a/models/issues/comment_code.go +++ b/models/issues/comment_code.go @@ -112,14 +112,12 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu } var err error - if comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - Repo: issue.Repo, - Links: markup.Links{ - Base: issue.Repo.Link(), - }, - Metas: issue.Repo.ComposeMetas(ctx), - }, comment.Content); err != nil { + rctx := markup.NewRenderContext(ctx). + WithRepoFacade(issue.Repo). + WithLinks(markup.Links{Base: issue.Repo.Link()}). + WithMetas(issue.Repo.ComposeMetas(ctx)) + if comment.RenderedContent, err = markdown.RenderString(rctx, + comment.Content); err != nil { return nil, err } } diff --git a/models/repo/repo.go b/models/repo/repo.go index 7d78cee2877d4..4a12de9d98d9d 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -617,10 +617,7 @@ func (repo *Repository) CanEnableEditor() bool { // DescriptionHTML does special handles to description and return HTML string. func (repo *Repository) DescriptionHTML(ctx context.Context) template.HTML { - desc, err := markup.RenderDescriptionHTML(&markup.RenderContext{ - Ctx: ctx, - // Don't use Metas to speedup requests - }, repo.Description) + desc, err := markup.RenderDescriptionHTML(markup.NewRenderContext(ctx), repo.Description) if err != nil { log.Error("Failed to render description for %s (ID: %d): %v", repo.Name, repo.ID, err) return template.HTML(markup.SanitizeDescription(repo.Description)) diff --git a/modules/csv/csv.go b/modules/csv/csv.go index 35c5d6ab67e0c..f1ca3b0923029 100644 --- a/modules/csv/csv.go +++ b/modules/csv/csv.go @@ -7,7 +7,7 @@ import ( "bytes" stdcsv "encoding/csv" "io" - "path/filepath" + "path" "regexp" "strings" @@ -53,7 +53,7 @@ func CreateReaderAndDetermineDelimiter(ctx *markup.RenderContext, rd io.Reader) func determineDelimiter(ctx *markup.RenderContext, data []byte) rune { extension := ".csv" if ctx != nil { - extension = strings.ToLower(filepath.Ext(ctx.RelativePath)) + extension = strings.ToLower(path.Ext(ctx.RenderOptions.RelativePath)) } var delimiter rune diff --git a/modules/csv/csv_test.go b/modules/csv/csv_test.go index 3ddb47acbbe77..29ed58db97cf6 100644 --- a/modules/csv/csv_test.go +++ b/modules/csv/csv_test.go @@ -5,13 +5,13 @@ package csv import ( "bytes" + "context" "encoding/csv" "io" "strconv" "strings" "testing" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/translation" @@ -231,10 +231,7 @@ John Doe john@doe.com This,note,had,a,lot,of,commas,to,test,delimiters`, } for n, c := range cases { - delimiter := determineDelimiter(&markup.RenderContext{ - Ctx: git.DefaultContext, - RelativePath: c.filename, - }, []byte(decodeSlashes(t, c.csv))) + delimiter := determineDelimiter(markup.NewRenderContext(context.Background()).WithRelativePath(c.filename), []byte(decodeSlashes(t, c.csv))) assert.EqualValues(t, c.expectedDelimiter, delimiter, "case %d: delimiter should be equal, expected '%c' got '%c'", n, c.expectedDelimiter, delimiter) } } diff --git a/modules/markup/asciicast/asciicast.go b/modules/markup/asciicast/asciicast.go index e92b78a4bcad6..1d0d631650c1a 100644 --- a/modules/markup/asciicast/asciicast.go +++ b/modules/markup/asciicast/asciicast.go @@ -44,10 +44,10 @@ func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { func (Renderer) Render(ctx *markup.RenderContext, _ io.Reader, output io.Writer) error { rawURL := fmt.Sprintf("%s/%s/%s/raw/%s/%s", setting.AppSubURL, - url.PathEscape(ctx.Metas["user"]), - url.PathEscape(ctx.Metas["repo"]), - ctx.Metas["BranchNameSubURL"], - url.PathEscape(ctx.RelativePath), + url.PathEscape(ctx.RenderOptions.Metas["user"]), + url.PathEscape(ctx.RenderOptions.Metas["repo"]), + ctx.RenderOptions.Metas["BranchNameSubURL"], + url.PathEscape(ctx.RenderOptions.RelativePath), ) return ctx.RenderInternal.FormatWithSafeAttrs(output, `
`, playerClassName, playerSrcAttr, rawURL) } diff --git a/modules/markup/console/console_test.go b/modules/markup/console/console_test.go index 2337d91ac5448..e1f0da1f01cef 100644 --- a/modules/markup/console/console_test.go +++ b/modules/markup/console/console_test.go @@ -4,10 +4,10 @@ package console import ( + "context" "strings" "testing" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" "github.com/stretchr/testify/assert" @@ -24,8 +24,7 @@ func TestRenderConsole(t *testing.T) { canRender := render.CanRender("test", strings.NewReader(k)) assert.True(t, canRender) - err := render.Render(&markup.RenderContext{Ctx: git.DefaultContext}, - strings.NewReader(k), &buf) + err := render.Render(markup.NewRenderContext(context.Background()), strings.NewReader(k), &buf) assert.NoError(t, err) assert.EqualValues(t, v, buf.String()) } diff --git a/modules/markup/csv/csv.go b/modules/markup/csv/csv.go index a3e6bbaac6830..b7d7a1b35be45 100644 --- a/modules/markup/csv/csv.go +++ b/modules/markup/csv/csv.go @@ -133,10 +133,10 @@ func (r Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.W // Check if maxRows or maxSize is reached, and if true, warn. if (row >= maxRows && maxRows != 0) || (rd.InputOffset() >= maxSize && maxSize != 0) { warn := `
` - rawLink := ` ` + rawLink := ` ` // Try to get the user translation - if locale, ok := ctx.Ctx.Value(translation.ContextKey).(translation.Locale); ok { + if locale, ok := ctx.Value(translation.ContextKey).(translation.Locale); ok { warn += locale.TrString("repo.file_too_large") rawLink += locale.TrString("repo.file_view_raw") } else { diff --git a/modules/markup/csv/csv_test.go b/modules/markup/csv/csv_test.go index 8c07184b21eeb..4c47170c30626 100644 --- a/modules/markup/csv/csv_test.go +++ b/modules/markup/csv/csv_test.go @@ -4,10 +4,10 @@ package markup import ( + "context" "strings" "testing" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" "github.com/stretchr/testify/assert" @@ -24,8 +24,7 @@ func TestRenderCSV(t *testing.T) { for k, v := range kases { var buf strings.Builder - err := render.Render(&markup.RenderContext{Ctx: git.DefaultContext}, - strings.NewReader(k), &buf) + err := render.Render(markup.NewRenderContext(context.Background()), strings.NewReader(k), &buf) assert.NoError(t, err) assert.EqualValues(t, v, buf.String()) } diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index d28dc9fa5d19a..98708e99b8622 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -12,7 +12,6 @@ import ( "runtime" "strings" - "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/process" @@ -80,8 +79,8 @@ func envMark(envName string) string { func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { var ( command = strings.NewReplacer( - envMark("GITEA_PREFIX_SRC"), ctx.Links.SrcLink(), - envMark("GITEA_PREFIX_RAW"), ctx.Links.RawLink(), + envMark("GITEA_PREFIX_SRC"), ctx.RenderOptions.Links.SrcLink(), + envMark("GITEA_PREFIX_RAW"), ctx.RenderOptions.Links.RawLink(), ).Replace(p.Command) commands = strings.Fields(command) args = commands[1:] @@ -113,22 +112,14 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io. args = append(args, f.Name()) } - if ctx.Ctx == nil { - if !setting.IsProd || setting.IsInTesting { - panic("RenderContext did not provide context") - } - log.Warn("RenderContext did not provide context, defaulting to Shutdown context") - ctx.Ctx = graceful.GetManager().ShutdownContext() - } - - processCtx, _, finished := process.GetManager().AddContext(ctx.Ctx, fmt.Sprintf("Render [%s] for %s", commands[0], ctx.Links.SrcLink())) + processCtx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("Render [%s] for %s", commands[0], ctx.RenderOptions.Links.SrcLink())) defer finished() cmd := exec.CommandContext(processCtx, commands[0], args...) cmd.Env = append( os.Environ(), - "GITEA_PREFIX_SRC="+ctx.Links.SrcLink(), - "GITEA_PREFIX_RAW="+ctx.Links.RawLink(), + "GITEA_PREFIX_SRC="+ctx.RenderOptions.Links.SrcLink(), + "GITEA_PREFIX_RAW="+ctx.RenderOptions.Links.RawLink(), ) if !p.IsInputFile { cmd.Stdin = input diff --git a/modules/markup/html_codepreview.go b/modules/markup/html_codepreview.go index 5c88481d769ef..68886a3434d0a 100644 --- a/modules/markup/html_codepreview.go +++ b/modules/markup/html_codepreview.go @@ -38,7 +38,7 @@ func renderCodeBlock(ctx *RenderContext, node *html.Node) (urlPosStart, urlPosSt CommitID: node.Data[m[6]:m[7]], FilePath: node.Data[m[8]:m[9]], } - if !httplib.IsCurrentGiteaSiteURL(ctx.Ctx, opts.FullURL) { + if !httplib.IsCurrentGiteaSiteURL(ctx, opts.FullURL) { return 0, 0, "", nil } u, err := url.Parse(opts.FilePath) @@ -51,7 +51,7 @@ func renderCodeBlock(ctx *RenderContext, node *html.Node) (urlPosStart, urlPosSt lineStart, _ := strconv.Atoi(strings.TrimPrefix(lineStartStr, "L")) lineStop, _ := strconv.Atoi(strings.TrimPrefix(lineStopStr, "L")) opts.LineStart, opts.LineStop = lineStart, lineStop - h, err := DefaultProcessorHelper.RenderRepoFileCodePreview(ctx.Ctx, opts) + h, err := DefaultProcessorHelper.RenderRepoFileCodePreview(ctx, opts) return m[0], m[1], h, err } diff --git a/modules/markup/html_codepreview_test.go b/modules/markup/html_codepreview_test.go index 5054627dde6c1..7c0db59d06acc 100644 --- a/modules/markup/html_codepreview_test.go +++ b/modules/markup/html_codepreview_test.go @@ -9,7 +9,6 @@ import ( "strings" "testing" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" @@ -23,10 +22,7 @@ func TestRenderCodePreview(t *testing.T) { }, }) test := func(input, expected string) { - buffer, err := markup.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - MarkupType: markdown.MarkupName, - }, input) + buffer, err := markup.RenderString(markup.NewRenderContext(context.Background()).WithMarkupType(markdown.MarkupName), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } diff --git a/modules/markup/html_commit.go b/modules/markup/html_commit.go index 0e674c83e1736..0649f84664073 100644 --- a/modules/markup/html_commit.go +++ b/modules/markup/html_commit.go @@ -84,7 +84,7 @@ func anyHashPatternExtract(s string) (ret anyHashPatternResult, ok bool) { // fullHashPatternProcessor renders SHA containing URLs func fullHashPatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil { + if ctx.RenderOptions.Metas == nil { return } nodeStop := node.NextSibling @@ -111,7 +111,7 @@ func fullHashPatternProcessor(ctx *RenderContext, node *html.Node) { } func comparePatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil { + if ctx.RenderOptions.Metas == nil { return } nodeStop := node.NextSibling @@ -163,14 +163,14 @@ func comparePatternProcessor(ctx *RenderContext, node *html.Node) { // hashCurrentPatternProcessor renders SHA1 strings to corresponding links that // are assumed to be in the same repository. func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil || ctx.Metas["user"] == "" || ctx.Metas["repo"] == "" || (ctx.Repo == nil && ctx.GitRepo == nil) { + if ctx.RenderOptions.Metas == nil || ctx.RenderOptions.Metas["user"] == "" || ctx.RenderOptions.Metas["repo"] == "" || (ctx.RenderHelper.repoFacade == nil && ctx.RenderHelper.gitRepo == nil) { return } start := 0 next := node.NextSibling - if ctx.ShaExistCache == nil { - ctx.ShaExistCache = make(map[string]bool) + if ctx.RenderHelper.shaExistCache == nil { + ctx.RenderHelper.shaExistCache = make(map[string]bool) } for node != nil && node != next && start < len(node.Data) { m := globalVars().hashCurrentPattern.FindStringSubmatchIndex(node.Data[start:]) @@ -191,25 +191,25 @@ func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) { // a commit in the repository before making it a link. // check cache first - exist, inCache := ctx.ShaExistCache[hash] + exist, inCache := ctx.RenderHelper.shaExistCache[hash] if !inCache { - if ctx.GitRepo == nil { + if ctx.RenderHelper.gitRepo == nil { var err error var closer io.Closer - ctx.GitRepo, closer, err = gitrepo.RepositoryFromContextOrOpen(ctx.Ctx, ctx.Repo) + ctx.RenderHelper.gitRepo, closer, err = gitrepo.RepositoryFromContextOrOpen(ctx, ctx.RenderHelper.repoFacade) if err != nil { - log.Error("unable to open repository: %s Error: %v", gitrepo.RepoGitURL(ctx.Repo), err) + log.Error("unable to open repository: %s Error: %v", gitrepo.RepoGitURL(ctx.RenderHelper.repoFacade), err) return } ctx.AddCancel(func() { _ = closer.Close() - ctx.GitRepo = nil + ctx.RenderHelper.gitRepo = nil }) } // Don't use IsObjectExist since it doesn't support short hashs with gogit edition. - exist = ctx.GitRepo.IsReferenceExist(hash) - ctx.ShaExistCache[hash] = exist + exist = ctx.RenderHelper.gitRepo.IsReferenceExist(hash) + ctx.RenderHelper.shaExistCache[hash] = exist } if !exist { @@ -217,7 +217,7 @@ func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) { continue } - link := util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], "commit", hash) + link := util.URLJoin(ctx.RenderOptions.Links.Prefix(), ctx.RenderOptions.Metas["user"], ctx.RenderOptions.Metas["repo"], "commit", hash) replaceContent(node, m[2], m[3], createCodeLink(link, base.ShortSha(hash), "commit")) start = 0 node = node.NextSibling.NextSibling diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index cdcc94d563131..7b143664fe81d 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -4,12 +4,12 @@ package markup import ( + "context" "fmt" "strconv" "strings" "testing" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" testModule "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" @@ -79,11 +79,11 @@ func TestRender_IssueIndexPattern(t *testing.T) { // numeric: render inputs without valid mentions test := func(s string) { testRenderIssueIndexPattern(t, s, s, &RenderContext{ - Ctx: git.DefaultContext, + ctx: context.Background(), }) testRenderIssueIndexPattern(t, s, s, &RenderContext{ - Ctx: git.DefaultContext, - Metas: numericMetas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: numericMetas}, }) } @@ -133,8 +133,8 @@ func TestRender_IssueIndexPattern2(t *testing.T) { } expectedNil := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNil, &RenderContext{ - Ctx: git.DefaultContext, - Metas: localMetas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: localMetas}, }) class := "ref-issue" @@ -147,8 +147,8 @@ func TestRender_IssueIndexPattern2(t *testing.T) { } expectedNum := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNum, &RenderContext{ - Ctx: git.DefaultContext, - Metas: numericMetas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: numericMetas}, }) } @@ -184,8 +184,8 @@ func TestRender_IssueIndexPattern3(t *testing.T) { // alphanumeric: render inputs without valid mentions test := func(s string) { testRenderIssueIndexPattern(t, s, s, &RenderContext{ - Ctx: git.DefaultContext, - Metas: alphanumericMetas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: alphanumericMetas}, }) } test("") @@ -217,8 +217,8 @@ func TestRender_IssueIndexPattern4(t *testing.T) { } expected := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expected, &RenderContext{ - Ctx: git.DefaultContext, - Metas: alphanumericMetas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: alphanumericMetas}, }) } test("OTT-1234 test", "%s test", "OTT-1234") @@ -240,8 +240,8 @@ func TestRender_IssueIndexPattern5(t *testing.T) { expected := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expected, &RenderContext{ - Ctx: git.DefaultContext, - Metas: metas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: metas}, }) } @@ -264,8 +264,8 @@ func TestRender_IssueIndexPattern5(t *testing.T) { ) testRenderIssueIndexPattern(t, "will not match", "will not match", &RenderContext{ - Ctx: git.DefaultContext, - Metas: regexpMetas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: regexpMetas}, }) } @@ -279,16 +279,16 @@ func TestRender_IssueIndexPattern_NoShortPattern(t *testing.T) { } testRenderIssueIndexPattern(t, "#1", "#1", &RenderContext{ - Ctx: git.DefaultContext, - Metas: metas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: metas}, }) testRenderIssueIndexPattern(t, "#1312", "#1312", &RenderContext{ - Ctx: git.DefaultContext, - Metas: metas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: metas}, }) testRenderIssueIndexPattern(t, "!1", "!1", &RenderContext{ - Ctx: git.DefaultContext, - Metas: metas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: metas}, }) } @@ -301,17 +301,17 @@ func TestRender_RenderIssueTitle(t *testing.T) { "style": IssueNameStyleNumeric, } actual, err := RenderIssueTitle(&RenderContext{ - Ctx: git.DefaultContext, - Metas: metas, + ctx: context.Background(), + RenderOptions: RenderOptions{Metas: metas}, }, "#1") assert.NoError(t, err) assert.Equal(t, "#1", actual) } func testRenderIssueIndexPattern(t *testing.T, input, expected string, ctx *RenderContext) { - ctx.Links.AbsolutePrefix = true - if ctx.Links.Base == "" { - ctx.Links.Base = TestRepoURL + ctx.RenderOptions.Links.AbsolutePrefix = true + if ctx.RenderOptions.Links.Base == "" { + ctx.RenderOptions.Links.Base = TestRepoURL } var buf strings.Builder @@ -326,22 +326,18 @@ func TestRender_AutoLink(t *testing.T) { test := func(input, expected string) { var buffer strings.Builder err := PostProcess(&RenderContext{ - Ctx: git.DefaultContext, - Links: Links{ - Base: TestRepoURL, - }, - Metas: localMetas, + ctx: context.Background(), + + RenderOptions: RenderOptions{Metas: localMetas, Links: Links{Base: TestRepoURL}}, }, strings.NewReader(input), &buffer) assert.Equal(t, err, nil) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String())) buffer.Reset() err = PostProcess(&RenderContext{ - Ctx: git.DefaultContext, - Links: Links{ - Base: TestRepoURL, - }, - Metas: localWikiMetas, + ctx: context.Background(), + + RenderOptions: RenderOptions{Metas: localWikiMetas, Links: Links{Base: TestRepoURL}}, }, strings.NewReader(input), &buffer) assert.Equal(t, err, nil) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String())) @@ -368,11 +364,9 @@ func TestRender_FullIssueURLs(t *testing.T) { test := func(input, expected string) { var result strings.Builder err := postProcess(&RenderContext{ - Ctx: git.DefaultContext, - Links: Links{ - Base: TestRepoURL, - }, - Metas: localMetas, + ctx: context.Background(), + + RenderOptions: RenderOptions{Metas: localMetas, Links: Links{Base: TestRepoURL}}, }, []processor{fullIssuePatternProcessor}, strings.NewReader(input), &result) assert.NoError(t, err) assert.Equal(t, expected, result.String()) diff --git a/modules/markup/html_issue.go b/modules/markup/html_issue.go index 7341af7eb697b..a75a8b3290066 100644 --- a/modules/markup/html_issue.go +++ b/modules/markup/html_issue.go @@ -19,7 +19,7 @@ import ( ) func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil { + if ctx.RenderOptions.Metas == nil { return } next := node.NextSibling @@ -36,14 +36,14 @@ func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { } link := node.Data[m[0]:m[1]] - if !httplib.IsCurrentGiteaSiteURL(ctx.Ctx, link) { + if !httplib.IsCurrentGiteaSiteURL(ctx, link) { return } text := "#" + node.Data[m[2]:m[3]] // if m[4] and m[5] is not -1, then link is to a comment // indicate that in the text by appending (comment) if m[4] != -1 && m[5] != -1 { - if locale, ok := ctx.Ctx.Value(translation.ContextKey).(translation.Locale); ok { + if locale, ok := ctx.Value(translation.ContextKey).(translation.Locale); ok { text += " " + locale.TrString("repo.from_comment") } else { text += " (comment)" @@ -56,7 +56,7 @@ func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { matchOrg := linkParts[len(linkParts)-4] matchRepo := linkParts[len(linkParts)-3] - if matchOrg == ctx.Metas["user"] && matchRepo == ctx.Metas["repo"] { + if matchOrg == ctx.RenderOptions.Metas["user"] && matchRepo == ctx.RenderOptions.Metas["repo"] { replaceContent(node, m[0], m[1], createLink(ctx, link, text, "ref-issue")) } else { text = matchOrg + "/" + matchRepo + text @@ -67,14 +67,14 @@ func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { } func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil { + if ctx.RenderOptions.Metas == nil { return } // crossLinkOnly: do not parse "#123", only parse "owner/repo#123" // if there is no repo in the context, then the "#123" format can't be parsed - // old logic: crossLinkOnly := ctx.Metas["mode"] == "document" && !ctx.IsWiki - crossLinkOnly := ctx.Metas["markupAllowShortIssuePattern"] != "true" + // old logic: crossLinkOnly := ctx.RenderOptions.Metas["mode"] == "document" && !ctx.IsWiki + crossLinkOnly := ctx.RenderOptions.Metas["markupAllowShortIssuePattern"] != "true" var ( found bool @@ -84,20 +84,20 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { next := node.NextSibling for node != nil && node != next { - _, hasExtTrackFormat := ctx.Metas["format"] + _, hasExtTrackFormat := ctx.RenderOptions.Metas["format"] // Repos with external issue trackers might still need to reference local PRs // We need to concern with the first one that shows up in the text, whichever it is - isNumericStyle := ctx.Metas["style"] == "" || ctx.Metas["style"] == IssueNameStyleNumeric + isNumericStyle := ctx.RenderOptions.Metas["style"] == "" || ctx.RenderOptions.Metas["style"] == IssueNameStyleNumeric foundNumeric, refNumeric := references.FindRenderizableReferenceNumeric(node.Data, hasExtTrackFormat && !isNumericStyle, crossLinkOnly) - switch ctx.Metas["style"] { + switch ctx.RenderOptions.Metas["style"] { case "", IssueNameStyleNumeric: found, ref = foundNumeric, refNumeric case IssueNameStyleAlphanumeric: found, ref = references.FindRenderizableReferenceAlphanumeric(node.Data) case IssueNameStyleRegexp: - pattern, err := regexplru.GetCompiled(ctx.Metas["regexp"]) + pattern, err := regexplru.GetCompiled(ctx.RenderOptions.Metas["regexp"]) if err != nil { return } @@ -121,9 +121,9 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { var link *html.Node reftext := node.Data[ref.RefLocation.Start:ref.RefLocation.End] if hasExtTrackFormat && !ref.IsPull { - ctx.Metas["index"] = ref.Issue + ctx.RenderOptions.Metas["index"] = ref.Issue - res, err := vars.Expand(ctx.Metas["format"], ctx.Metas) + res, err := vars.Expand(ctx.RenderOptions.Metas["format"], ctx.RenderOptions.Metas) if err != nil { // here we could just log the error and continue the rendering log.Error("unable to expand template vars for ref %s, err: %v", ref.Issue, err) @@ -136,9 +136,9 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { // Gitea will redirect on click as appropriate. issuePath := util.Iif(ref.IsPull, "pulls", "issues") if ref.Owner == "" { - link = createLink(ctx, util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], issuePath, ref.Issue), reftext, "ref-issue") + link = createLink(ctx, util.URLJoin(ctx.RenderOptions.Links.Prefix(), ctx.RenderOptions.Metas["user"], ctx.RenderOptions.Metas["repo"], issuePath, ref.Issue), reftext, "ref-issue") } else { - link = createLink(ctx, util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, issuePath, ref.Issue), reftext, "ref-issue") + link = createLink(ctx, util.URLJoin(ctx.RenderOptions.Links.Prefix(), ref.Owner, ref.Name, issuePath, ref.Issue), reftext, "ref-issue") } } @@ -177,7 +177,7 @@ func commitCrossReferencePatternProcessor(ctx *RenderContext, node *html.Node) { } reftext := ref.Owner + "/" + ref.Name + "@" + base.ShortSha(ref.CommitSha) - link := createLink(ctx, util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, "commit", ref.CommitSha), reftext, "commit") + link := createLink(ctx, util.URLJoin(ctx.RenderOptions.Links.Prefix(), ref.Owner, ref.Name, "commit", ref.CommitSha), reftext, "commit") replaceContent(node, ref.RefLocation.Start, ref.RefLocation.End, link) node = node.NextSibling.NextSibling diff --git a/modules/markup/html_link.go b/modules/markup/html_link.go index 32aa7dc614ca7..f6700161b53f8 100644 --- a/modules/markup/html_link.go +++ b/modules/markup/html_link.go @@ -19,15 +19,15 @@ import ( func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (result string, resolved bool) { isAnchorFragment := link != "" && link[0] == '#' if !isAnchorFragment && !IsFullURLString(link) { - linkBase := ctx.Links.Base + linkBase := ctx.RenderOptions.Links.Base if ctx.IsMarkupContentWiki() { // no need to check if the link should be resolved as a wiki link or a wiki raw link // just use wiki link here, and it will be redirected to a wiki raw link if necessary - linkBase = ctx.Links.WikiLink() - } else if ctx.Links.BranchPath != "" || ctx.Links.TreePath != "" { + linkBase = ctx.RenderOptions.Links.WikiLink() + } else if ctx.RenderOptions.Links.BranchPath != "" || ctx.RenderOptions.Links.TreePath != "" { // if there is no BranchPath, then the link will be something like "/owner/repo/src/{the-file-path}" // and then this link will be handled by the "legacy-ref" code and be redirected to the default branch like "/owner/repo/src/branch/main/{the-file-path}" - linkBase = ctx.Links.SrcLink() + linkBase = ctx.RenderOptions.Links.SrcLink() } link, resolved = util.URLJoin(linkBase, link), true } @@ -147,7 +147,7 @@ func shortLinkProcessor(ctx *RenderContext, node *html.Node) { } if image { if !absoluteLink { - link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), link) + link = util.URLJoin(ctx.RenderOptions.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), link) } title := props["title"] if title == "" { diff --git a/modules/markup/html_mention.go b/modules/markup/html_mention.go index f7e2ad50f139f..4243eeb20f93b 100644 --- a/modules/markup/html_mention.go +++ b/modules/markup/html_mention.go @@ -25,15 +25,15 @@ func mentionProcessor(ctx *RenderContext, node *html.Node) { loc.Start += start loc.End += start mention := node.Data[loc.Start:loc.End] - teams, ok := ctx.Metas["teams"] + teams, ok := ctx.RenderOptions.Metas["teams"] // FIXME: util.URLJoin may not be necessary here: // - setting.AppURL is defined to have a terminal '/' so unless mention[1:] // is an AppSubURL link we can probably fallback to concatenation. // team mention should follow @orgName/teamName style if ok && strings.Contains(mention, "/") { mentionOrgAndTeam := strings.Split(mention, "/") - if mentionOrgAndTeam[0][1:] == ctx.Metas["org"] && strings.Contains(teams, ","+strings.ToLower(mentionOrgAndTeam[1])+",") { - replaceContent(node, loc.Start, loc.End, createLink(ctx, util.URLJoin(ctx.Links.Prefix(), "org", ctx.Metas["org"], "teams", mentionOrgAndTeam[1]), mention, "" /*mention*/)) + if mentionOrgAndTeam[0][1:] == ctx.RenderOptions.Metas["org"] && strings.Contains(teams, ","+strings.ToLower(mentionOrgAndTeam[1])+",") { + replaceContent(node, loc.Start, loc.End, createLink(ctx, util.URLJoin(ctx.RenderOptions.Links.Prefix(), "org", ctx.RenderOptions.Metas["org"], "teams", mentionOrgAndTeam[1]), mention, "" /*mention*/)) node = node.NextSibling.NextSibling start = 0 continue @@ -43,8 +43,8 @@ func mentionProcessor(ctx *RenderContext, node *html.Node) { } mentionedUsername := mention[1:] - if DefaultProcessorHelper.IsUsernameMentionable != nil && DefaultProcessorHelper.IsUsernameMentionable(ctx.Ctx, mentionedUsername) { - replaceContent(node, loc.Start, loc.End, createLink(ctx, util.URLJoin(ctx.Links.Prefix(), mentionedUsername), mention, "" /*mention*/)) + if DefaultProcessorHelper.IsUsernameMentionable != nil && DefaultProcessorHelper.IsUsernameMentionable(ctx, mentionedUsername) { + replaceContent(node, loc.Start, loc.End, createLink(ctx, util.URLJoin(ctx.RenderOptions.Links.Prefix(), mentionedUsername), mention, "" /*mention*/)) node = node.NextSibling.NextSibling start = 0 } else { diff --git a/modules/markup/html_node.go b/modules/markup/html_node.go index 234adba2bfa8d..a7c323fcba8a9 100644 --- a/modules/markup/html_node.go +++ b/modules/markup/html_node.go @@ -17,7 +17,7 @@ func visitNodeImg(ctx *RenderContext, img *html.Node) (next *html.Node) { } if IsNonEmptyRelativePath(attr.Val) { - attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), attr.Val) + attr.Val = util.URLJoin(ctx.RenderOptions.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), attr.Val) // By default, the "" tag should also be clickable, // because frontend use `` to paste the re-scaled image into the markdown, @@ -53,7 +53,7 @@ func visitNodeVideo(ctx *RenderContext, node *html.Node) (next *html.Node) { continue } if IsNonEmptyRelativePath(attr.Val) { - attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), attr.Val) + attr.Val = util.URLJoin(ctx.RenderOptions.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), attr.Val) } attr.Val = camoHandleLink(attr.Val) node.Attr[i] = attr diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 67ac2758a31fb..7366965a9d5ce 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -9,7 +9,6 @@ import ( "testing" "code.gitea.io/gitea/modules/emoji" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" @@ -57,16 +56,10 @@ func newMockRepo(ownerName, repoName string) gitrepo.Repository { func TestRender_Commits(t *testing.T) { setting.AppURL = markup.TestAppURL test := func(input, expected string) { - buffer, err := markup.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - RelativePath: ".md", - Links: markup.Links{ - AbsolutePrefix: true, - Base: markup.TestRepoURL, - }, - Repo: newMockRepo(testRepoOwnerName, testRepoName), - Metas: localMetas, - }, input) + buffer, err := markup.RenderString(markup.NewTestRenderContext("a.md", localMetas, newMockRepo(testRepoOwnerName, testRepoName), markup.Links{ + AbsolutePrefix: true, + Base: markup.TestRepoURL, + }), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } @@ -112,15 +105,11 @@ func TestRender_CrossReferences(t *testing.T) { setting.AppURL = markup.TestAppURL defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { - buffer, err := markup.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - RelativePath: "a.md", - Links: markup.Links{ + buffer, err := markup.RenderString(markup.NewTestRenderContext("a.md", localMetas, + markup.Links{ AbsolutePrefix: true, Base: setting.AppSubURL, - }, - Metas: localMetas, - }, input) + }), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } @@ -154,13 +143,7 @@ func TestRender_links(t *testing.T) { setting.AppURL = markup.TestAppURL defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { - buffer, err := markup.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - RelativePath: "a.md", - Links: markup.Links{ - Base: markup.TestRepoURL, - }, - }, input) + buffer, err := markup.RenderString(markup.NewTestRenderContext("a.md", markup.Links{Base: markup.TestRepoURL}), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } @@ -265,13 +248,7 @@ func TestRender_email(t *testing.T) { setting.AppURL = markup.TestAppURL defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { - res, err := markup.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - RelativePath: "a.md", - Links: markup.Links{ - Base: markup.TestRepoURL, - }, - }, input) + res, err := markup.RenderString(markup.NewTestRenderContext("a.md", markup.Links{Base: markup.TestRepoURL}), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res)) } @@ -338,13 +315,7 @@ func TestRender_emoji(t *testing.T) { test := func(input, expected string) { expected = strings.ReplaceAll(expected, "&", "&") - buffer, err := markup.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - RelativePath: "a.md", - Links: markup.Links{ - Base: markup.TestRepoURL, - }, - }, input) + buffer, err := markup.RenderString(markup.NewTestRenderContext("a.md", markup.Links{Base: markup.TestRepoURL}), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } @@ -404,22 +375,10 @@ func TestRender_ShortLinks(t *testing.T) { tree := util.URLJoin(markup.TestRepoURL, "src", "master") test := func(input, expected, expectedWiki string) { - buffer, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ - Base: markup.TestRepoURL, - BranchPath: "master", - }, - }, input) + buffer, err := markdown.RenderString(markup.NewTestRenderContext(markup.Links{Base: markup.TestRepoURL, BranchPath: "master"}), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer))) - buffer, err = markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ - Base: markup.TestRepoURL, - }, - Metas: localWikiMetas, - }, input) + buffer, err = markdown.RenderString(markup.NewTestRenderContext(markup.Links{Base: markup.TestRepoURL}, localWikiMetas), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer))) } @@ -529,11 +488,7 @@ func TestRender_ShortLinks(t *testing.T) { func TestRender_RelativeMedias(t *testing.T) { render := func(input string, isWiki bool, links markup.Links) string { - buffer, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: links, - Metas: util.Iif(isWiki, localWikiMetas, localMetas), - }, input) + buffer, err := markdown.RenderString(markup.NewTestRenderContext(links, util.Iif(isWiki, localWikiMetas, localMetas)), input) assert.NoError(t, err) return strings.TrimSpace(string(buffer)) } @@ -574,26 +529,14 @@ func Test_ParseClusterFuzz(t *testing.T) { data := "
` var res strings.Builder - err := markup.PostProcess(&markup.RenderContext{ - Ctx: git.DefaultContext, - Metas: localMetas, - }, strings.NewReader(data), &res) + err := markup.PostProcess(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res) assert.NoError(t, err) assert.Equal(t, data, res.String()) } @@ -666,29 +605,23 @@ func BenchmarkEmojiPostprocess(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { var res strings.Builder - err := markup.PostProcess(&markup.RenderContext{ - Ctx: git.DefaultContext, - Metas: localMetas, - }, strings.NewReader(data), &res) + err := markup.PostProcess(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res) assert.NoError(b, err) } } func TestFuzz(t *testing.T) { s := "t/l/issues/8#/../../a" - renderContext := markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ + renderContext := markup.NewTestRenderContext( + markup.Links{ Base: "https://example.com/go-gitea/gitea", }, - Metas: map[string]string{ + map[string]string{ "user": "go-gitea", "repo": "gitea", }, - } - - err := markup.PostProcess(&renderContext, strings.NewReader(s), io.Discard) - + ) + err := markup.PostProcess(renderContext, strings.NewReader(s), io.Discard) assert.NoError(t, err) } @@ -696,10 +629,7 @@ func TestIssue18471(t *testing.T) { data := `http://domain/org/repo/compare/783b039...da951ce` var res strings.Builder - err := markup.PostProcess(&markup.RenderContext{ - Ctx: git.DefaultContext, - Metas: localMetas, - }, strings.NewReader(data), &res) + err := markup.PostProcess(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res) assert.NoError(t, err) assert.Equal(t, `783b039...da951ce`, res.String()) diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go index 47dcfa8b5afc1..45f8c266a227c 100644 --- a/modules/markup/markdown/goldmark.go +++ b/modules/markup/markdown/goldmark.go @@ -79,7 +79,7 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa // TODO: this was a quite unclear part, old code: `if metas["mode"] != "document" { use comment link break setting }` // many places render non-comment contents with no mode=document, then these contents also use comment's hard line break setting // especially in many tests. - markdownLineBreakStyle := ctx.Metas["markdownLineBreakStyle"] + markdownLineBreakStyle := ctx.RenderOptions.Metas["markdownLineBreakStyle"] if markup.RenderBehaviorForTesting.ForceHardLineBreak { v.SetHardLineBreak(true) } else if markdownLineBreakStyle == "comment" { diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index a3915ad4398d3..f77db9eb38e02 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -182,7 +182,7 @@ func render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error bufWithMetadataLength := len(buf) rc := &RenderConfig{ - Meta: renderMetaModeFromString(string(ctx.RenderMetaAs)), + Meta: markup.RenderMetaAsDetails, Icon: "table", Lang: "", } @@ -241,7 +241,7 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri // Render renders Markdown to HTML with all specific handling stuff. func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { - ctx.MarkupType = MarkupName + ctx.RenderOptions.MarkupType = MarkupName return markup.Render(ctx, input, output) } diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index e4889a75e59fd..634ec6301f5f3 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -4,12 +4,10 @@ package markdown_test import ( - "context" "html/template" "strings" "testing" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" @@ -67,22 +65,11 @@ func TestRender_StandardLinks(t *testing.T) { setting.AppURL = AppURL test := func(input, expected, expectedWiki string) { - buffer, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ - Base: FullURL, - }, - }, input) + buffer, err := markdown.RenderString(markup.NewTestRenderContext(markup.Links{Base: FullURL}), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer))) - buffer, err = markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ - Base: FullURL, - }, - Metas: localWikiMetas, - }, input) + buffer, err = markdown.RenderString(markup.NewTestRenderContext(markup.Links{Base: FullURL}, localWikiMetas), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer))) } @@ -101,12 +88,7 @@ func TestRender_Images(t *testing.T) { setting.AppURL = AppURL test := func(input, expected string) { - buffer, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ - Base: FullURL, - }, - }, input) + buffer, err := markdown.RenderString(markup.NewTestRenderContext(markup.Links{Base: FullURL}), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer))) } @@ -308,14 +290,11 @@ func TestTotal_RenderWiki(t *testing.T) { setting.AppURL = AppURL answers := testAnswers(util.URLJoin(FullURL, "wiki"), util.URLJoin(FullURL, "wiki", "raw")) for i := 0; i < len(sameCases); i++ { - line, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ - Base: FullURL, - }, - Repo: newMockRepo(testRepoOwnerName, testRepoName), - Metas: localWikiMetas, - }, sameCases[i]) + line, err := markdown.RenderString(markup.NewTestRenderContext( + markup.Links{Base: FullURL}, + newMockRepo(testRepoOwnerName, testRepoName), + localWikiMetas, + ), sameCases[i]) assert.NoError(t, err) assert.Equal(t, answers[i], string(line)) } @@ -334,13 +313,7 @@ func TestTotal_RenderWiki(t *testing.T) { } for i := 0; i < len(testCases); i += 2 { - line, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ - Base: FullURL, - }, - Metas: localWikiMetas, - }, testCases[i]) + line, err := markdown.RenderString(markup.NewTestRenderContext(markup.Links{Base: FullURL}, localWikiMetas), testCases[i]) assert.NoError(t, err) assert.EqualValues(t, testCases[i+1], string(line)) } @@ -352,15 +325,14 @@ func TestTotal_RenderString(t *testing.T) { setting.AppURL = AppURL answers := testAnswers(util.URLJoin(FullURL, "src", "master"), util.URLJoin(FullURL, "media", "master")) for i := 0; i < len(sameCases); i++ { - line, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ + line, err := markdown.RenderString(markup.NewTestRenderContext( + markup.Links{ Base: FullURL, BranchPath: "master", }, - Repo: newMockRepo(testRepoOwnerName, testRepoName), - Metas: localMetas, - }, sameCases[i]) + newMockRepo(testRepoOwnerName, testRepoName), + localMetas, + ), sameCases[i]) assert.NoError(t, err) assert.Equal(t, answers[i], string(line)) } @@ -368,12 +340,7 @@ func TestTotal_RenderString(t *testing.T) { testCases := []string{} for i := 0; i < len(testCases); i += 2 { - line, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ - Base: FullURL, - }, - }, testCases[i]) + line, err := markdown.RenderString(markup.NewTestRenderContext(markup.Links{Base: FullURL}), testCases[i]) assert.NoError(t, err) assert.Equal(t, template.HTML(testCases[i+1]), line) } @@ -381,17 +348,17 @@ func TestTotal_RenderString(t *testing.T) { func TestRender_RenderParagraphs(t *testing.T) { test := func(t *testing.T, str string, cnt int) { - res, err := markdown.RenderRawString(&markup.RenderContext{Ctx: git.DefaultContext}, str) + res, err := markdown.RenderRawString(markup.NewTestRenderContext(), str) assert.NoError(t, err) assert.Equal(t, cnt, strings.Count(res, "image2

` defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() - res, err := markdown.RenderRawString(&markup.RenderContext{Ctx: git.DefaultContext}, testcase) + res, err := markdown.RenderRawString(markup.NewTestRenderContext(), testcase) assert.NoError(t, err) assert.Equal(t, expected, res) } @@ -441,7 +408,7 @@ func TestRenderEmojiInLinks_Issue12331(t *testing.T) { testcase := `[Link with emoji :moon: in text](https://gitea.io)` expected := `

Link with emoji 🌔 in text

` - res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, testcase) + res, err := markdown.RenderString(markup.NewTestRenderContext(), testcase) assert.NoError(t, err) assert.Equal(t, template.HTML(expected), res) } @@ -479,7 +446,7 @@ func TestColorPreview(t *testing.T) { } for _, test := range positiveTests { - res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, test.testcase) + res, err := markdown.RenderString(markup.NewTestRenderContext(), test.testcase) assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase) assert.Equal(t, template.HTML(test.expected), res, "Unexpected result in testcase %q", test.testcase) } @@ -498,7 +465,7 @@ func TestColorPreview(t *testing.T) { } for _, test := range negativeTests { - res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, test) + res, err := markdown.RenderString(markup.NewTestRenderContext(), test) assert.NoError(t, err, "Unexpected error in testcase: %q", test) assert.NotContains(t, res, ` defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() for i, c := range cases { - result, err := markdown.RenderString(&markup.RenderContext{ - Ctx: context.Background(), - Links: c.Links, - Metas: util.Iif(c.IsWiki, map[string]string{"markupContentMode": "wiki"}, map[string]string{}), - }, input) + result, err := markdown.RenderString(markup.NewTestRenderContext(c.Links, util.Iif(c.IsWiki, map[string]string{"markupContentMode": "wiki"}, map[string]string{})), input) assert.NoError(t, err, "Unexpected error in testcase: %v", i) assert.Equal(t, c.Expected, string(result), "Unexpected result in testcase %v", i) } @@ -1029,7 +992,7 @@ func TestAttention(t *testing.T) { } test := func(input, expected string) { - result, err := markdown.RenderString(&markup.RenderContext{Ctx: context.Background()}, input) + result, err := markdown.RenderString(markup.NewTestRenderContext(), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(result))) } @@ -1062,6 +1025,6 @@ func BenchmarkSpecializedMarkdown(b *testing.B) { func BenchmarkMarkdownRender(b *testing.B) { // 23202 50840 ns/op for i := 0; i < b.N; i++ { - _, _ = markdown.RenderString(&markup.RenderContext{Ctx: context.Background()}, "https://example.com\n- a\n- b\n") + _, _ = markdown.RenderString(markup.NewTestRenderContext(), "https://example.com\n- a\n- b\n") } } diff --git a/modules/markup/markdown/transform_image.go b/modules/markup/markdown/transform_image.go index b2262c1c7885d..c2cbffc1c187b 100644 --- a/modules/markup/markdown/transform_image.go +++ b/modules/markup/markdown/transform_image.go @@ -21,7 +21,7 @@ func (g *ASTTransformer) transformImage(ctx *markup.RenderContext, v *ast.Image) // Check if the destination is a real link if len(v.Destination) > 0 && !markup.IsFullURLBytes(v.Destination) { v.Destination = []byte(giteautil.URLJoin( - ctx.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), + ctx.RenderOptions.Links.ResolveMediaLink(ctx.IsMarkupContentWiki()), strings.TrimLeft(string(v.Destination), "/"), )) } diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index c587a6ada5ef0..cf719cf4e906e 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -143,15 +143,15 @@ func (r *Writer) resolveLink(kind, link string) string { kind = org.RegularLink{URL: link}.Kind() } - base := r.Ctx.Links.Base + base := r.Ctx.RenderOptions.Links.Base if r.Ctx.IsMarkupContentWiki() { - base = r.Ctx.Links.WikiLink() - } else if r.Ctx.Links.HasBranchInfo() { - base = r.Ctx.Links.SrcLink() + base = r.Ctx.RenderOptions.Links.WikiLink() + } else if r.Ctx.RenderOptions.Links.HasBranchInfo() { + base = r.Ctx.RenderOptions.Links.SrcLink() } if kind == "image" || kind == "video" { - base = r.Ctx.Links.ResolveMediaLink(r.Ctx.IsMarkupContentWiki()) + base = r.Ctx.RenderOptions.Links.ResolveMediaLink(r.Ctx.IsMarkupContentWiki()) } link = util.URLJoin(base, link) diff --git a/modules/markup/orgmode/orgmode_test.go b/modules/markup/orgmode/orgmode_test.go index a3eefc3db3df9..4048ae2475ff7 100644 --- a/modules/markup/orgmode/orgmode_test.go +++ b/modules/markup/orgmode/orgmode_test.go @@ -4,10 +4,10 @@ package markup import ( + "os" "strings" "testing" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -15,20 +15,21 @@ import ( "github.com/stretchr/testify/assert" ) -const AppURL = "http://localhost:3000/" +func TestMain(m *testing.M) { + setting.AppURL = "http://localhost:3000/" + setting.IsInTesting = true + os.Exit(m.Run()) +} func TestRender_StandardLinks(t *testing.T) { - setting.AppURL = AppURL - test := func(input, expected string, isWiki bool) { - buffer, err := RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ + buffer, err := RenderString(markup.NewTestRenderContext( + markup.Links{ Base: "/relative-path", BranchPath: "branch/main", }, - Metas: map[string]string{"markupContentMode": util.Iif(isWiki, "wiki", "")}, - }, input) + map[string]string{"markupContentMode": util.Iif(isWiki, "wiki", "")}, + ), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } @@ -42,16 +43,13 @@ func TestRender_StandardLinks(t *testing.T) { } func TestRender_InternalLinks(t *testing.T) { - setting.AppURL = AppURL - test := func(input, expected string) { - buffer, err := RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ + buffer, err := RenderString(markup.NewTestRenderContext( + markup.Links{ Base: "/relative-path", BranchPath: "branch/main", }, - }, input) + ), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } @@ -67,15 +65,8 @@ func TestRender_InternalLinks(t *testing.T) { } func TestRender_Media(t *testing.T) { - setting.AppURL = AppURL - test := func(input, expected string) { - buffer, err := RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: markup.Links{ - Base: "./relative-path", - }, - }, input) + buffer, err := RenderString(markup.NewTestRenderContext(markup.Links{Base: "./relative-path"}), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } @@ -113,12 +104,8 @@ func TestRender_Media(t *testing.T) { } func TestRender_Source(t *testing.T) { - setting.AppURL = AppURL - test := func(input, expected string) { - buffer, err := RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - }, input) + buffer, err := RenderString(markup.NewTestRenderContext(), input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } diff --git a/modules/markup/render.go b/modules/markup/render.go index f05cb62626451..e251f47fc9107 100644 --- a/modules/markup/render.go +++ b/modules/markup/render.go @@ -9,6 +9,7 @@ import ( "io" "net/url" "strings" + "time" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" @@ -42,16 +43,16 @@ var RenderBehaviorForTesting struct { DisableInternalAttributes bool } -// RenderContext represents a render context -type RenderContext struct { - Ctx context.Context - RelativePath string // relative path from tree root of the branch +type RenderOptions struct { + // relative path from tree root of the branch + RelativePath string // eg: "orgmode", "asciicast", "console" // for file mode, it could be left as empty, and will be detected by file extension in RelativePath MarkupType string - Links Links // special link references for rendering, especially when there is a branch/tree path + // special link references for rendering, especially when there is a branch/tree path + Links Links // user&repo, format&style®exp (for external issue pattern), teams&org (for mention) // BranchNameSubURL (for iframe&asciicast) @@ -59,27 +60,95 @@ type RenderContext struct { // markdownLineBreakStyle (comment, document) Metas map[string]string - GitRepo *git.Repository - Repo gitrepo.Repository - ShaExistCache map[string]bool - cancelFn func() - SidebarTocNode ast.Node - RenderMetaAs RenderMetaMode - InStandalonePage bool // used by external render. the router "/org/repo/render/..." will output the rendered content in a standalone page + // used by external render. the router "/org/repo/render/..." will output the rendered content in a standalone page + InStandalonePage bool +} + +type RenderHelper struct { + gitRepo *git.Repository + repoFacade gitrepo.Repository + shaExistCache map[string]bool + cancelFn func() +} + +// RenderContext represents a render context +type RenderContext struct { + ctx context.Context + SidebarTocNode ast.Node + + RenderHelper RenderHelper + RenderOptions RenderOptions RenderInternal internal.RenderInternal } +func (ctx *RenderContext) Deadline() (deadline time.Time, ok bool) { + return ctx.ctx.Deadline() +} + +func (ctx *RenderContext) Done() <-chan struct{} { + return ctx.ctx.Done() +} + +func (ctx *RenderContext) Err() error { + return ctx.ctx.Err() +} + +func (ctx *RenderContext) Value(key any) any { + return ctx.ctx.Value(key) +} + +var _ context.Context = (*RenderContext)(nil) + +func NewRenderContext(ctx context.Context) *RenderContext { + return &RenderContext{ctx: ctx} +} + +func (ctx *RenderContext) WithMarkupType(typ string) *RenderContext { + ctx.RenderOptions.MarkupType = typ + return ctx +} + +func (ctx *RenderContext) WithRelativePath(path string) *RenderContext { + ctx.RenderOptions.RelativePath = path + return ctx +} + +func (ctx *RenderContext) WithLinks(links Links) *RenderContext { + ctx.RenderOptions.Links = links + return ctx +} + +func (ctx *RenderContext) WithMetas(metas map[string]string) *RenderContext { + ctx.RenderOptions.Metas = metas + return ctx +} + +func (ctx *RenderContext) WithInStandalonePage(v bool) *RenderContext { + ctx.RenderOptions.InStandalonePage = v + return ctx +} + +func (ctx *RenderContext) WithGitRepo(r *git.Repository) *RenderContext { + ctx.RenderHelper.gitRepo = r + return ctx +} + +func (ctx *RenderContext) WithRepoFacade(r gitrepo.Repository) *RenderContext { + ctx.RenderHelper.repoFacade = r + return ctx +} + // Cancel runs any cleanup functions that have been registered for this Ctx func (ctx *RenderContext) Cancel() { if ctx == nil { return } - ctx.ShaExistCache = map[string]bool{} - if ctx.cancelFn == nil { + ctx.RenderHelper.shaExistCache = map[string]bool{} + if ctx.RenderHelper.cancelFn == nil { return } - ctx.cancelFn() + ctx.RenderHelper.cancelFn() } // AddCancel adds the provided fn as a Cleanup for this Ctx @@ -87,38 +156,38 @@ func (ctx *RenderContext) AddCancel(fn func()) { if ctx == nil { return } - oldCancelFn := ctx.cancelFn + oldCancelFn := ctx.RenderHelper.cancelFn if oldCancelFn == nil { - ctx.cancelFn = fn + ctx.RenderHelper.cancelFn = fn return } - ctx.cancelFn = func() { + ctx.RenderHelper.cancelFn = func() { defer oldCancelFn() fn() } } func (ctx *RenderContext) IsMarkupContentWiki() bool { - return ctx.Metas != nil && ctx.Metas["markupContentMode"] == "wiki" + return ctx.RenderOptions.Metas != nil && ctx.RenderOptions.Metas["markupContentMode"] == "wiki" } // Render renders markup file to HTML with all specific handling stuff. func Render(ctx *RenderContext, input io.Reader, output io.Writer) error { - if ctx.MarkupType == "" && ctx.RelativePath != "" { - ctx.MarkupType = DetectMarkupTypeByFileName(ctx.RelativePath) - if ctx.MarkupType == "" { - return util.NewInvalidArgumentErrorf("unsupported file to render: %q", ctx.RelativePath) + if ctx.RenderOptions.MarkupType == "" && ctx.RenderOptions.RelativePath != "" { + ctx.RenderOptions.MarkupType = DetectMarkupTypeByFileName(ctx.RenderOptions.RelativePath) + if ctx.RenderOptions.MarkupType == "" { + return util.NewInvalidArgumentErrorf("unsupported file to render: %q", ctx.RenderOptions.RelativePath) } } - renderer := renderers[ctx.MarkupType] + renderer := renderers[ctx.RenderOptions.MarkupType] if renderer == nil { - return util.NewInvalidArgumentErrorf("unsupported markup type: %q", ctx.MarkupType) + return util.NewInvalidArgumentErrorf("unsupported markup type: %q", ctx.RenderOptions.MarkupType) } - if ctx.RelativePath != "" { + if ctx.RenderOptions.RelativePath != "" { if externalRender, ok := renderer.(ExternalRenderer); ok && externalRender.DisplayInIFrame() { - if !ctx.InStandalonePage { + if !ctx.RenderOptions.InStandalonePage { // for an external "DisplayInIFrame" render, it could only output its content in a standalone page // otherwise, a `, setting.AppSubURL, - url.PathEscape(ctx.Metas["user"]), - url.PathEscape(ctx.Metas["repo"]), - ctx.Metas["BranchNameSubURL"], - url.PathEscape(ctx.RelativePath), + url.PathEscape(ctx.RenderOptions.Metas["user"]), + url.PathEscape(ctx.RenderOptions.Metas["repo"]), + ctx.RenderOptions.Metas["BranchNameSubURL"], + url.PathEscape(ctx.RenderOptions.RelativePath), )) return err } @@ -176,7 +245,7 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr pr1, pw1, close1 := pipes() defer close1() - eg, _ := errgroup.WithContext(ctx.Ctx) + eg, _ := errgroup.WithContext(ctx) var pw2 io.WriteCloser = util.NopCloser{Writer: finalProcessor} if r, ok := renderer.(ExternalRenderer); !ok || !r.SanitizerDisabled() { @@ -230,3 +299,27 @@ func Init(ph *ProcessorHelper) { func ComposeSimpleDocumentMetas() map[string]string { return map[string]string{"markdownLineBreakStyle": "document"} } + +// NewTestRenderContext is a helper function to create a RenderContext for testing purpose +// It accepts string (RelativePath), Links, map[string]string (Metas), gitrepo.Repository +func NewTestRenderContext(a ...any) *RenderContext { + if !setting.IsInTesting { + panic("NewTestRenderContext should only be used in testing") + } + ctx := NewRenderContext(context.Background()) + for _, v := range a { + switch v := v.(type) { + case string: + ctx = ctx.WithRelativePath(v) + case Links: + ctx = ctx.WithLinks(v) + case map[string]string: + ctx = ctx.WithMetas(v) + case gitrepo.Repository: + ctx = ctx.WithRepoFacade(v) + default: + panic(fmt.Sprintf("unknown type %T", v)) + } + } + return ctx +} diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index 5776eefced961..3237de5ecb18b 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -38,10 +38,7 @@ func (ut *RenderUtils) RenderCommitMessage(msg string, metas map[string]string) cleanMsg := template.HTMLEscapeString(msg) // we can safely assume that it will not return any error, since there // shouldn't be any special HTML. - fullMessage, err := markup.RenderCommitMessage(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: metas, - }, cleanMsg) + fullMessage, err := markup.RenderCommitMessage(markup.NewRenderContext(ut.ctx).WithMetas(metas), cleanMsg) if err != nil { log.Error("RenderCommitMessage: %v", err) return "" @@ -68,10 +65,7 @@ func (ut *RenderUtils) RenderCommitMessageLinkSubject(msg, urlDefault string, me // we can safely assume that it will not return any error, since there // shouldn't be any special HTML. - renderedMessage, err := markup.RenderCommitMessageSubject(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: metas, - }, urlDefault, template.HTMLEscapeString(msgLine)) + renderedMessage, err := markup.RenderCommitMessageSubject(markup.NewRenderContext(ut.ctx).WithMetas(metas), urlDefault, template.HTMLEscapeString(msgLine)) if err != nil { log.Error("RenderCommitMessageSubject: %v", err) return "" @@ -93,10 +87,7 @@ func (ut *RenderUtils) RenderCommitBody(msg string, metas map[string]string) tem return "" } - renderedMessage, err := markup.RenderCommitMessage(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: metas, - }, template.HTMLEscapeString(msgLine)) + renderedMessage, err := markup.RenderCommitMessage(markup.NewRenderContext(ut.ctx).WithMetas(metas), template.HTMLEscapeString(msgLine)) if err != nil { log.Error("RenderCommitMessage: %v", err) return "" @@ -115,10 +106,7 @@ func renderCodeBlock(htmlEscapedTextToRender template.HTML) template.HTML { // RenderIssueTitle renders issue/pull title with defined post processors func (ut *RenderUtils) RenderIssueTitle(text string, metas map[string]string) template.HTML { - renderedText, err := markup.RenderIssueTitle(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: metas, - }, template.HTMLEscapeString(text)) + renderedText, err := markup.RenderIssueTitle(markup.NewRenderContext(ut.ctx).WithMetas(metas), template.HTMLEscapeString(text)) if err != nil { log.Error("RenderIssueTitle: %v", err) return "" @@ -186,7 +174,7 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML { // RenderEmoji renders html text with emoji post processors func (ut *RenderUtils) RenderEmoji(text string) template.HTML { - renderedText, err := markup.RenderEmoji(&markup.RenderContext{Ctx: ut.ctx}, template.HTMLEscapeString(text)) + renderedText, err := markup.RenderEmoji(markup.NewRenderContext(ut.ctx), template.HTMLEscapeString(text)) if err != nil { log.Error("RenderEmoji: %v", err) return "" @@ -208,10 +196,7 @@ func reactionToEmoji(reaction string) template.HTML { } func (ut *RenderUtils) MarkdownToHtml(input string) template.HTML { //nolint:revive - output, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: markup.ComposeSimpleDocumentMetas(), - }, input) + output, err := markdown.RenderString(markup.NewRenderContext(ut.ctx).WithMetas(markup.ComposeSimpleDocumentMetas()), input) if err != nil { log.Error("RenderString: %v", err) } diff --git a/routers/api/v1/misc/markup.go b/routers/api/v1/misc/markup.go index 868ed92519089..7b3633552fde5 100644 --- a/routers/api/v1/misc/markup.go +++ b/routers/api/v1/misc/markup.go @@ -99,9 +99,7 @@ func MarkdownRaw(ctx *context.APIContext) { // "422": // "$ref": "#/responses/validationError" defer ctx.Req.Body.Close() - if err := markdown.RenderRaw(&markup.RenderContext{ - Ctx: ctx, - }, ctx.Req.Body, ctx.Resp); err != nil { + if err := markdown.RenderRaw(markup.NewRenderContext(ctx), ctx.Req.Body, ctx.Resp); err != nil { ctx.InternalServerError(err) return } diff --git a/routers/common/markup.go b/routers/common/markup.go index dd6b286109f80..59f338c2bccc6 100644 --- a/routers/common/markup.go +++ b/routers/common/markup.go @@ -28,13 +28,12 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPa // for example, when previewing file "/gitea/owner/repo/src/branch/features/feat-123/doc/CHANGE.md", then filePath is "doc/CHANGE.md" // and the urlPathContext is "/gitea/owner/repo/src/branch/features/feat-123/doc" - renderCtx := &markup.RenderContext{ - Ctx: ctx, - Links: markup.Links{AbsolutePrefix: true}, - MarkupType: markdown.MarkupName, - } + renderCtx := markup.NewRenderContext(ctx). + WithLinks(markup.Links{AbsolutePrefix: true}). + WithMarkupType(markdown.MarkupName) + if urlPathContext != "" { - renderCtx.Links.Base = fmt.Sprintf("%s%s", httplib.GuessCurrentHostURL(ctx), urlPathContext) + renderCtx.RenderOptions.Links.Base = fmt.Sprintf("%s%s", httplib.GuessCurrentHostURL(ctx), urlPathContext) } if mode == "" || mode == "markdown" { @@ -47,15 +46,14 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPa switch mode { case "gfm": // legacy mode, do nothing case "comment": - renderCtx.Metas = map[string]string{"markdownLineBreakStyle": "comment"} + renderCtx = renderCtx.WithMetas(map[string]string{"markdownLineBreakStyle": "comment"}) case "wiki": - renderCtx.Metas = map[string]string{"markdownLineBreakStyle": "document", "markupContentMode": "wiki"} + renderCtx = renderCtx.WithMetas(map[string]string{"markdownLineBreakStyle": "document", "markupContentMode": "wiki"}) case "file": // render the repo file content by its extension - renderCtx.Metas = map[string]string{"markdownLineBreakStyle": "document"} - renderCtx.MarkupType = "" - renderCtx.RelativePath = filePath - renderCtx.InStandalonePage = true + renderCtx = renderCtx.WithMetas(map[string]string{"markdownLineBreakStyle": "document"}). + WithMarkupType(""). + WithRelativePath(filePath) default: ctx.Error(http.StatusUnprocessableEntity, fmt.Sprintf("Unknown mode: %s", mode)) return @@ -70,17 +68,17 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPa refPath := strings.Join(fields[3:], "/") // it is "branch/features/feat-12/doc" refPath = strings.TrimSuffix(refPath, "/"+fileDir) // now we get the correct branch path: "branch/features/feat-12" - renderCtx.Links = markup.Links{AbsolutePrefix: true, Base: absoluteBasePrefix, BranchPath: refPath, TreePath: fileDir} + renderCtx = renderCtx.WithLinks(markup.Links{AbsolutePrefix: true, Base: absoluteBasePrefix, BranchPath: refPath, TreePath: fileDir}) } if repo != nil && repo.Repository != nil { - renderCtx.Repo = repo.Repository + renderCtx = renderCtx.WithRepoFacade(repo.Repository) if mode == "file" { - renderCtx.Metas = repo.Repository.ComposeDocumentMetas(ctx) + renderCtx = renderCtx.WithMetas(repo.Repository.ComposeDocumentMetas(ctx)) } else if mode == "wiki" { - renderCtx.Metas = repo.Repository.ComposeWikiMetas(ctx) + renderCtx = renderCtx.WithMetas(repo.Repository.ComposeWikiMetas(ctx)) } else if mode == "comment" { - renderCtx.Metas = repo.Repository.ComposeMetas(ctx) + renderCtx = renderCtx.WithMetas(repo.Repository.ComposeMetas(ctx)) } } if err := markup.Render(renderCtx, strings.NewReader(text), ctx.Resp); err != nil { diff --git a/routers/web/feed/convert.go b/routers/web/feed/convert.go index afc2c343a68b3..fad7dfdf5e0ee 100644 --- a/routers/web/feed/convert.go +++ b/routers/web/feed/convert.go @@ -51,16 +51,14 @@ func toReleaseLink(ctx *context.Context, act *activities_model.Action) string { // renderMarkdown creates a minimal markdown render context from an action. // If rendering fails, the original markdown text is returned func renderMarkdown(ctx *context.Context, act *activities_model.Action, content string) template.HTML { - markdownCtx := &markup.RenderContext{ - Ctx: ctx, - Links: markup.Links{ + markdownCtx := markup.NewRenderContext(ctx). + WithLinks(markup.Links{ Base: act.GetRepoLink(ctx), - }, - Metas: map[string]string{ // FIXME: not right here, it should use issue to compose the metas + }). + WithMetas(map[string]string{ // FIXME: not right here, it should use issue to compose the metas "user": act.GetRepoUserName(ctx), "repo": act.GetRepoName(ctx), - }, - } + }) markdown, err := markdown.RenderString(markdownCtx, content) if err != nil { return templates.SanitizeHTML(content) // old code did so: use SanitizeHTML to render in tmpl @@ -296,14 +294,13 @@ func releasesToFeedItems(ctx *context.Context, releases []*repo_model.Release) ( } link := &feeds.Link{Href: rel.HTMLURL()} - content, err = markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - Repo: rel.Repo, - Links: markup.Links{ + content, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithRepoFacade(rel.Repo). + WithLinks(markup.Links{ Base: rel.Repo.Link(), - }, - Metas: rel.Repo.ComposeMetas(ctx), - }, rel.Note) + }). + WithMetas(rel.Repo.ComposeMetas(ctx)), + rel.Note) if err != nil { return nil, err } diff --git a/routers/web/feed/profile.go b/routers/web/feed/profile.go index 6dd2d14cc6bcf..7c4864b45ef15 100644 --- a/routers/web/feed/profile.go +++ b/routers/web/feed/profile.go @@ -41,13 +41,10 @@ func showUserFeed(ctx *context.Context, formatType string) { return } - ctxUserDescription, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - Links: markup.Links{ - Base: ctx.ContextUser.HTMLURL(), - }, - Metas: markup.ComposeSimpleDocumentMetas(), - }, ctx.ContextUser.Description) + ctxUserDescription, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.ContextUser.HTMLURL()}). + WithMetas(markup.ComposeSimpleDocumentMetas()), + ctx.ContextUser.Description) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/org/home.go b/routers/web/org/home.go index 18648d33cd79f..d0ac82b1b094e 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -180,17 +180,16 @@ func prepareOrgProfileReadme(ctx *context.Context, viewRepositories bool) bool { if bytes, err := profileReadme.GetBlobContent(setting.UI.MaxDisplayFileSize); err != nil { log.Error("failed to GetBlobContent: %v", err) } else { - if profileContent, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - GitRepo: profileGitRepo, - Links: markup.Links{ + if profileContent, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithGitRepo(profileGitRepo). + WithLinks(markup.Links{ // Pass repo link to markdown render for the full link of media elements. // The profile of default branch would be shown. Base: profileDbRepo.Link(), BranchPath: path.Join("branch", util.PathEscapeSegments(profileDbRepo.DefaultBranch)), - }, - Metas: markup.ComposeSimpleDocumentMetas(), - }, bytes); err != nil { + }). + WithMetas(markup.ComposeSimpleDocumentMetas()), + bytes); err != nil { log.Error("failed to RenderString: %v", err) } else { ctx.Data["ProfileReadme"] = profileContent diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index d7865e18d63f9..87b1f9019a00f 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -392,16 +392,15 @@ func Diff(ctx *context.Context) { if err == nil { ctx.Data["NoteCommit"] = note.Commit ctx.Data["NoteAuthor"] = user_model.ValidateCommitWithEmail(ctx, note.Commit) - ctx.Data["NoteRendered"], err = markup.RenderCommitMessage(&markup.RenderContext{ - Links: markup.Links{ + ctx.Data["NoteRendered"], err = markup.RenderCommitMessage(markup.NewRenderContext(ctx). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: path.Join("commit", util.PathEscapeSegments(commitID)), - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, template.HTMLEscapeString(string(charset.ToUTF8WithFallback(note.Message, charset.ConvertOpts{})))) + }). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + template.HTMLEscapeString(string(charset.ToUTF8WithFallback(note.Message, charset.ConvertOpts{})))) if err != nil { ctx.ServerError("RenderCommitMessage", err) return diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index a5fdba3fde696..278974bec3ecf 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -149,7 +149,7 @@ func setCsvCompareContext(ctx *context.Context) { return csvReader, reader, err } - baseReader, baseBlobCloser, err := csvReaderFromCommit(&markup.RenderContext{Ctx: ctx, RelativePath: diffFile.OldName}, baseBlob) + baseReader, baseBlobCloser, err := csvReaderFromCommit(markup.NewRenderContext(ctx).WithRelativePath(diffFile.OldName), baseBlob) if baseBlobCloser != nil { defer baseBlobCloser.Close() } @@ -161,7 +161,7 @@ func setCsvCompareContext(ctx *context.Context) { return CsvDiffResult{nil, "unable to load file"} } - headReader, headBlobCloser, err := csvReaderFromCommit(&markup.RenderContext{Ctx: ctx, RelativePath: diffFile.Name}, headBlob) + headReader, headBlobCloser, err := csvReaderFromCommit(markup.NewRenderContext(ctx).WithRelativePath(diffFile.Name), headBlob) if headBlobCloser != nil { defer headBlobCloser.Close() } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index d1dbdd6bff39f..d52dbf393945e 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -366,15 +366,12 @@ func UpdateIssueContent(ctx *context.Context) { } } - content, err := markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.FormString("context"), // FIXME: <- IS THIS SAFE ? - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, issue.Content) + content, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.FormString("context")}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + issue.Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/issue_comment.go b/routers/web/repo/issue_comment.go index 6f0fa938ce3ce..33105d67ca640 100644 --- a/routers/web/repo/issue_comment.go +++ b/routers/web/repo/issue_comment.go @@ -267,15 +267,12 @@ func UpdateCommentContent(ctx *context.Context) { var renderedContent template.HTML if comment.Content != "" { - renderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.FormString("context"), // FIXME: <- IS THIS SAFE ? - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, comment.Content) + renderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.FormString("context")}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + comment.Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 284928856f19a..55d36cfefa781 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -359,15 +359,12 @@ func ViewIssue(ctx *context.Context) { } } ctx.Data["IssueWatch"] = iw - issue.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, issue.Content) + issue.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + issue.Content) if err != nil { ctx.ServerError("RenderString", err) return @@ -467,15 +464,14 @@ func ViewIssue(ctx *context.Context) { comment.Issue = issue if comment.Type == issues_model.CommentTypeComment || comment.Type == issues_model.CommentTypeReview { - comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ + comment.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, comment.Content) + }). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + comment.Content) if err != nil { ctx.ServerError("RenderString", err) return @@ -550,15 +546,12 @@ func ViewIssue(ctx *context.Context) { } } } else if comment.Type.HasContentSupport() { - comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, comment.Content) + comment.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + comment.Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/milestone.go b/routers/web/repo/milestone.go index 5c0972188cecf..7361fe66bcc57 100644 --- a/routers/web/repo/milestone.go +++ b/routers/web/repo/milestone.go @@ -79,15 +79,12 @@ func Milestones(ctx *context.Context) { } } for _, m := range miles { - m.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, m.Content) + m.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + m.Content) if err != nil { ctx.ServerError("RenderString", err) return @@ -268,15 +265,12 @@ func MilestoneIssuesAndPulls(ctx *context.Context) { return } - milestone.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, milestone.Content) + milestone.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + milestone.Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 664ea7eb76d79..cce13df3be876 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -92,15 +92,12 @@ func Projects(ctx *context.Context) { } for i := range projects { - projects[i].RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, projects[i].Description) + projects[i].RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + projects[i].Description) if err != nil { ctx.ServerError("RenderString", err) return @@ -425,15 +422,12 @@ func ViewProject(ctx *context.Context) { ctx.Data["SelectLabels"] = selectLabels ctx.Data["AssigneeID"] = assigneeID - project.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, project.Description) + project.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + project.Description) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 566a82316f8e0..1b5305a90db5f 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -114,15 +114,12 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions) cacheUsers[r.PublisherID] = r.Publisher } - r.RenderedNote, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, r.Note) + r.RenderedNote, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + r.Note) if err != nil { return nil, err } diff --git a/routers/web/repo/render.go b/routers/web/repo/render.go index 6aba9e0ac1436..c551e44f46fc7 100644 --- a/routers/web/repo/render.go +++ b/routers/web/repo/render.go @@ -56,18 +56,17 @@ func RenderFile(ctx *context.Context) { return } - err = markup.Render(&markup.RenderContext{ - Ctx: ctx, - RelativePath: ctx.Repo.TreePath, - Links: markup.Links{ + err = markup.Render(markup.NewRenderContext(ctx). + WithRelativePath(ctx.Repo.TreePath). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: ctx.Repo.BranchNameSubURL(), TreePath: path.Dir(ctx.Repo.TreePath), - }, - Metas: ctx.Repo.Repository.ComposeDocumentMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - InStandalonePage: true, - }, rd, ctx.Resp) + }). + WithMetas(ctx.Repo.Repository.ComposeDocumentMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithInStandalonePage(true), + rd, ctx.Resp) if err != nil { log.Error("Failed to render file %q: %v", ctx.Repo.TreePath, err) http.Error(ctx.Resp, "Failed to render file", http.StatusInternalServerError) diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 5d68ace29b535..aacd7de6b1782 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -310,18 +310,17 @@ func renderReadmeFile(ctx *context.Context, subfolder string, readmeFile *git.Tr ctx.Data["IsMarkup"] = true ctx.Data["MarkupType"] = markupType - ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, &markup.RenderContext{ - Ctx: ctx, - MarkupType: markupType, - RelativePath: path.Join(ctx.Repo.TreePath, readmeFile.Name()), // ctx.Repo.TreePath is the directory not the Readme so we must append the Readme filename (and path). - Links: markup.Links{ + ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, markup.NewRenderContext(ctx). + WithMarkupType(markupType). + WithRelativePath(path.Join(ctx.Repo.TreePath, readmeFile.Name())). // ctx.Repo.TreePath is the directory not the Readme so we must append the Readme filename (and path). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: ctx.Repo.BranchNameSubURL(), TreePath: path.Join(ctx.Repo.TreePath, subfolder), - }, - Metas: ctx.Repo.Repository.ComposeDocumentMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - }, rd) + }). + WithMetas(ctx.Repo.Repository.ComposeDocumentMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo), + rd) if err != nil { log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.Name(), ctx.Repo.Repository, err) delete(ctx.Data, "IsMarkup") @@ -514,18 +513,17 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) { ctx.Data["MarkupType"] = markupType metas := ctx.Repo.Repository.ComposeDocumentMetas(ctx) metas["BranchNameSubURL"] = ctx.Repo.BranchNameSubURL() - ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, &markup.RenderContext{ - Ctx: ctx, - MarkupType: markupType, - RelativePath: ctx.Repo.TreePath, - Links: markup.Links{ + ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, markup.NewRenderContext(ctx). + WithMarkupType(markupType). + WithRelativePath(ctx.Repo.TreePath). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: ctx.Repo.BranchNameSubURL(), TreePath: path.Dir(ctx.Repo.TreePath), - }, - Metas: metas, - GitRepo: ctx.Repo.GitRepo, - }, rd) + }). + WithMetas(metas). + WithGitRepo(ctx.Repo.GitRepo), + rd) if err != nil { ctx.ServerError("Render", err) return @@ -606,18 +604,17 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) { rd := io.MultiReader(bytes.NewReader(buf), dataRc) ctx.Data["IsMarkup"] = true ctx.Data["MarkupType"] = markupType - ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, &markup.RenderContext{ - Ctx: ctx, - MarkupType: markupType, - RelativePath: ctx.Repo.TreePath, - Links: markup.Links{ + ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, markup.NewRenderContext(ctx). + WithMarkupType(markupType). + WithRelativePath(ctx.Repo.TreePath). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: ctx.Repo.BranchNameSubURL(), TreePath: path.Dir(ctx.Repo.TreePath), - }, - Metas: ctx.Repo.Repository.ComposeDocumentMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - }, rd) + }). + WithMetas(ctx.Repo.Repository.ComposeDocumentMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo), + rd) if err != nil { ctx.ServerError("Render", err) return diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 2732a67e714fd..eda3320ff0d27 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -288,13 +288,9 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { footerContent = data } - rctx := &markup.RenderContext{ - Ctx: ctx, - Metas: ctx.Repo.Repository.ComposeWikiMetas(ctx), - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - } + rctx := markup.NewRenderContext(ctx). + WithMetas(ctx.Repo.Repository.ComposeWikiMetas(ctx)). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}) buf := &strings.Builder{} renderFn := func(data []byte) (escaped *charset.EscapeStatus, output string, err error) { diff --git a/routers/web/shared/user/header.go b/routers/web/shared/user/header.go index 9467b0986b3d5..4cb0592b4b782 100644 --- a/routers/web/shared/user/header.go +++ b/routers/web/shared/user/header.go @@ -49,10 +49,7 @@ func PrepareContextForProfileBigAvatar(ctx *context.Context) { } ctx.Data["OpenIDs"] = openIDs if len(ctx.ContextUser.Description) != 0 { - content, err := markdown.RenderString(&markup.RenderContext{ - Metas: markup.ComposeSimpleDocumentMetas(), - Ctx: ctx, - }, ctx.ContextUser.Description) + content, err := markdown.RenderString(markup.NewRenderContext(ctx).WithMetas(markup.ComposeSimpleDocumentMetas()), ctx.ContextUser.Description) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 2b16142f6d591..0bd0371f14465 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -257,14 +257,11 @@ func Milestones(ctx *context.Context) { continue } - milestones[i].RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: milestones[i].Repo.Link(), - }, - Metas: milestones[i].Repo.ComposeMetas(ctx), - Ctx: ctx, - Repo: milestones[i].Repo, - }, milestones[i].Content) + milestones[i].RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: milestones[i].Repo.Link()}). + WithMetas(milestones[i].Repo.ComposeMetas(ctx)). + WithRepoFacade(milestones[i].Repo), + milestones[i].Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 4fbfc2bd175fb..2c9487bbc08d9 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -246,10 +246,9 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb if bytes, err := profileReadme.GetBlobContent(setting.UI.MaxDisplayFileSize); err != nil { log.Error("failed to GetBlobContent: %v", err) } else { - if profileContent, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - GitRepo: profileGitRepo, - Links: markup.Links{ + if profileContent, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithGitRepo(profileGitRepo). + WithLinks(markup.Links{ // Give the repo link to the markdown render for the full link of media element. // the media link usually be like /[user]/[repoName]/media/branch/[branchName], // Eg. /Tom/.profile/media/branch/main @@ -257,8 +256,8 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb // https://docs.gitea.com/usage/profile-readme Base: profileDbRepo.Link(), BranchPath: path.Join("branch", util.PathEscapeSegments(profileDbRepo.DefaultBranch)), - }, - }, bytes); err != nil { + }), + bytes); err != nil { log.Error("failed to RenderString: %v", err) } else { ctx.Data["ProfileReadme"] = profileContent diff --git a/services/context/org.go b/services/context/org.go index 132ce19a31b90..bf482fa7543ce 100644 --- a/services/context/org.go +++ b/services/context/org.go @@ -259,9 +259,7 @@ func HandleOrgAssignment(ctx *Context, args ...bool) { ctx.Data["IsFollowing"] = ctx.Doer != nil && user_model.IsFollowing(ctx, ctx.Doer.ID, ctx.ContextUser.ID) if len(ctx.ContextUser.Description) != 0 { - content, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - }, ctx.ContextUser.Description) + content, err := markdown.RenderString(markup.NewRenderContext(ctx), ctx.ContextUser.Description) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 23c91595b799f..162e497dc02be 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -219,15 +219,11 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient } // This is the body of the new issue or comment, not the mail body - body, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - Repo: ctx.Issue.Repo, - Links: markup.Links{ - AbsolutePrefix: true, - Base: ctx.Issue.Repo.HTMLURL(), - }, - Metas: ctx.Issue.Repo.ComposeMetas(ctx), - }, ctx.Content) + body, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithRepoFacade(ctx.Issue.Repo). + WithLinks(markup.Links{AbsolutePrefix: true, Base: ctx.Issue.Repo.HTMLURL()}). + WithMetas(ctx.Issue.Repo.ComposeMetas(ctx)), + ctx.Content) if err != nil { return nil, err } diff --git a/services/mailer/mail_release.go b/services/mailer/mail_release.go index 01a8929e2d028..3298c2273a945 100644 --- a/services/mailer/mail_release.go +++ b/services/mailer/mail_release.go @@ -56,14 +56,11 @@ func mailNewRelease(ctx context.Context, lang string, tos []*user_model.User, re locale := translation.NewLocale(lang) var err error - rel.RenderedNote, err = markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - Repo: rel.Repo, - Links: markup.Links{ - Base: rel.Repo.HTMLURL(), - }, - Metas: rel.Repo.ComposeMetas(ctx), - }, rel.Note) + rel.RenderedNote, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithRepoFacade(rel.Repo). + WithLinks(markup.Links{Base: rel.Repo.HTMLURL()}). + WithMetas(rel.Repo.ComposeMetas(ctx)), + rel.Note) if err != nil { log.Error("markdown.RenderString(%d): %v", rel.RepoID, err) return diff --git a/tests/fuzz/fuzz_test.go b/tests/fuzz/fuzz_test.go index 25a6ed8213191..78d3027547d84 100644 --- a/tests/fuzz/fuzz_test.go +++ b/tests/fuzz/fuzz_test.go @@ -14,27 +14,22 @@ import ( "code.gitea.io/gitea/modules/setting" ) -var renderContext = markup.RenderContext{ - Ctx: context.Background(), - Links: markup.Links{ - Base: "https://example.com/go-gitea/gitea", - }, - Metas: map[string]string{ - "user": "go-gitea", - "repo": "gitea", - }, +func newFuzzRenderContext() *markup.RenderContext { + return markup.NewRenderContext(context.Background()). + WithLinks(markup.Links{Base: "https://example.com/go-gitea/gitea"}). + WithMetas(map[string]string{"user": "go-gitea", "repo": "gitea"}) } func FuzzMarkdownRenderRaw(f *testing.F) { f.Fuzz(func(t *testing.T, data []byte) { setting.AppURL = "http://localhost:3000/" - markdown.RenderRaw(&renderContext, bytes.NewReader(data), io.Discard) + markdown.RenderRaw(newFuzzRenderContext(), bytes.NewReader(data), io.Discard) }) } func FuzzMarkupPostProcess(f *testing.F) { f.Fuzz(func(t *testing.T, data []byte) { setting.AppURL = "http://localhost:3000/" - markup.PostProcess(&renderContext, bytes.NewReader(data), io.Discard) + markup.PostProcess(newFuzzRenderContext(), bytes.NewReader(data), io.Discard) }) } From bc7d599030cad2e69139b79ad5a878504fbf8ed9 Mon Sep 17 00:00:00 2001 From: Kerwin Bryant Date: Fri, 22 Nov 2024 14:12:50 +0800 Subject: [PATCH 4/8] Fix issues with inconsistent spacing in areas (#32607) Fix issues with inconsistent spacing in areas where the branch_dropdown component is used. before: ![1732238359257](https://github.com/user-attachments/assets/38edda1f-ec4e-419e-9264-68009375d177) ![1732238334410](https://github.com/user-attachments/assets/c4770aea-bc83-477c-9b6a-632f984c0d7d) after: ![1732238273317](https://github.com/user-attachments/assets/4d05068e-db97-45af-86c4-29442dff1bdf) ![1732238723881](https://github.com/user-attachments/assets/69acd286-f79b-44fe-ad73-2d5fc6dfc98c) --------- Co-authored-by: wxiaoguang --- templates/repo/commits.tmpl | 4 ++-- templates/repo/home.tmpl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/repo/commits.tmpl b/templates/repo/commits.tmpl index e6efe1ff5426c..6bce58577442b 100644 --- a/templates/repo/commits.tmpl +++ b/templates/repo/commits.tmpl @@ -4,8 +4,8 @@
{{template "repo/sub_menu" .}}
-
- {{template "repo/branch_dropdown" dict "root" . "ContainerClasses" "tw-mr-1"}} +
+ {{template "repo/branch_dropdown" dict "root" .}} {{svg "octicon-git-branch"}} {{ctx.Locale.Tr "repo.commit_graph"}} diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl index 0a8391b55353b..12c4a17234d3a 100644 --- a/templates/repo/home.tmpl +++ b/templates/repo/home.tmpl @@ -47,7 +47,7 @@ {{$isHomepage := (eq $n 0)}}
- {{template "repo/branch_dropdown" dict "root" . "ContainerClasses" "tw-mr-1"}} + {{template "repo/branch_dropdown" dict "root" .}} {{if and .CanCompareOrPull .IsViewBranch (not .Repository.IsArchived)}} {{$cmpBranch := ""}} {{if ne .Repository.ID .BaseRepo.ID}} From fe49cb0243bed03f565269d84836bf21a0597665 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 22 Nov 2024 07:44:48 -0800 Subject: [PATCH 5/8] Fix get reviewers' bug (#32415) This PR rewrites `GetReviewer` function and move it to service layer. Reviewers should not be watchers, so that this PR removed all watchers from reviewers. When the repository is under an organization, the pull request unit read permission will be checked to resolve the bug of #32394 Fix #32394 --- models/organization/team_repo.go | 14 +++++ models/organization/team_repo_test.go | 31 ++++++++++ models/repo/user_repo.go | 52 ---------------- models/repo/user_repo_test.go | 43 ------------- routers/api/v1/repo/collaborators.go | 10 ++- routers/web/repo/issue_page_meta.go | 8 +-- services/issue/assignee.go | 8 +-- services/pull/reviewer.go | 89 +++++++++++++++++++++++++++ services/pull/reviewer_test.go | 72 ++++++++++++++++++++++ services/repository/review.go | 24 -------- services/repository/review_test.go | 28 --------- tests/integration/api_repo_test.go | 4 +- 12 files changed, 225 insertions(+), 158 deletions(-) create mode 100644 models/organization/team_repo_test.go create mode 100644 services/pull/reviewer.go create mode 100644 services/pull/reviewer_test.go delete mode 100644 services/repository/review.go delete mode 100644 services/repository/review_test.go diff --git a/models/organization/team_repo.go b/models/organization/team_repo.go index 1184e39263635..c90dfdeda04dc 100644 --- a/models/organization/team_repo.go +++ b/models/organization/team_repo.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" "xorm.io/builder" ) @@ -83,3 +84,16 @@ func GetTeamsWithAccessToRepo(ctx context.Context, orgID, repoID int64, mode per OrderBy("name"). Find(&teams) } + +// GetTeamsWithAccessToRepoUnit returns all teams in an organization that have given access level to the repository special unit. +func GetTeamsWithAccessToRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type) ([]*Team, error) { + teams := make([]*Team, 0, 5) + return teams, db.GetEngine(ctx).Where("team_unit.access_mode >= ?", mode). + Join("INNER", "team_repo", "team_repo.team_id = team.id"). + Join("INNER", "team_unit", "team_unit.team_id = team.id"). + And("team_repo.org_id = ?", orgID). + And("team_repo.repo_id = ?", repoID). + And("team_unit.type = ?", unitType). + OrderBy("name"). + Find(&teams) +} diff --git a/models/organization/team_repo_test.go b/models/organization/team_repo_test.go new file mode 100644 index 0000000000000..c0d6750df90cb --- /dev/null +++ b/models/organization/team_repo_test.go @@ -0,0 +1,31 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package organization_test + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/perm" + "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestGetTeamsWithAccessToRepoUnit(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + org41 := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 41}) + repo61 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 61}) + + teams, err := organization.GetTeamsWithAccessToRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests) + assert.NoError(t, err) + if assert.Len(t, teams, 2) { + assert.EqualValues(t, 21, teams[0].ID) + assert.EqualValues(t, 22, teams[1].ID) + } +} diff --git a/models/repo/user_repo.go b/models/repo/user_repo.go index ecc9216950738..a9b1360df1410 100644 --- a/models/repo/user_repo.go +++ b/models/repo/user_repo.go @@ -11,7 +11,6 @@ import ( "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" - api "code.gitea.io/gitea/modules/structs" "xorm.io/builder" ) @@ -146,57 +145,6 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us return users, nil } -// GetReviewers get all users can be requested to review: -// * for private repositories this returns all users that have read access or higher to the repository. -// * for public repositories this returns all users that have read access or higher to the repository, -// all repo watchers and all organization members. -// TODO: may be we should have a busy choice for users to block review request to them. -func GetReviewers(ctx context.Context, repo *Repository, doerID, posterID int64) ([]*user_model.User, error) { - // Get the owner of the repository - this often already pre-cached and if so saves complexity for the following queries - if err := repo.LoadOwner(ctx); err != nil { - return nil, err - } - - cond := builder.And(builder.Neq{"`user`.id": posterID}). - And(builder.Eq{"`user`.is_active": true}) - - if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate { - // This a private repository: - // Anyone who can read the repository is a requestable reviewer - - cond = cond.And(builder.In("`user`.id", - builder.Select("user_id").From("access").Where( - builder.Eq{"repo_id": repo.ID}. - And(builder.Gte{"mode": perm.AccessModeRead}), - ), - )) - - if repo.Owner.Type == user_model.UserTypeIndividual && repo.Owner.ID != posterID { - // as private *user* repos don't generate an entry in the `access` table, - // the owner of a private repo needs to be explicitly added. - cond = cond.Or(builder.Eq{"`user`.id": repo.Owner.ID}) - } - } else { - // This is a "public" repository: - // Any user that has read access, is a watcher or organization member can be requested to review - cond = cond.And(builder.And(builder.In("`user`.id", - builder.Select("user_id").From("access"). - Where(builder.Eq{"repo_id": repo.ID}. - And(builder.Gte{"mode": perm.AccessModeRead})), - ).Or(builder.In("`user`.id", - builder.Select("user_id").From("watch"). - Where(builder.Eq{"repo_id": repo.ID}. - And(builder.In("mode", WatchModeNormal, WatchModeAuto))), - ).Or(builder.In("`user`.id", - builder.Select("uid").From("org_user"). - Where(builder.Eq{"org_id": repo.OwnerID}), - ))))) - } - - users := make([]*user_model.User, 0, 8) - return users, db.GetEngine(ctx).Where(cond).OrderBy(user_model.GetOrderByName()).Find(&users) -} - // GetIssuePostersWithSearch returns users with limit of 30 whose username started with prefix that have authored an issue/pull request for the given repository // If isShowFullName is set to true, also include full name prefix search func GetIssuePostersWithSearch(ctx context.Context, repo *Repository, isPull bool, search string, isShowFullName bool) ([]*user_model.User, error) { diff --git a/models/repo/user_repo_test.go b/models/repo/user_repo_test.go index d2bf6dc9121c9..f2abc2ffa01b9 100644 --- a/models/repo/user_repo_test.go +++ b/models/repo/user_repo_test.go @@ -38,46 +38,3 @@ func TestRepoAssignees(t *testing.T) { assert.NotContains(t, []int64{users[0].ID, users[1].ID, users[2].ID}, 15) } } - -func TestRepoGetReviewers(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - // test public repo - repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - - ctx := db.DefaultContext - reviewers, err := repo_model.GetReviewers(ctx, repo1, 2, 2) - assert.NoError(t, err) - if assert.Len(t, reviewers, 3) { - assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID}) - } - - // should include doer if doer is not PR poster. - reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 2) - assert.NoError(t, err) - assert.Len(t, reviewers, 3) - - // should not include PR poster, if PR poster would be otherwise eligible - reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 4) - assert.NoError(t, err) - assert.Len(t, reviewers, 2) - - // test private user repo - repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - - reviewers, err = repo_model.GetReviewers(ctx, repo2, 2, 4) - assert.NoError(t, err) - assert.Len(t, reviewers, 1) - assert.EqualValues(t, reviewers[0].ID, 2) - - // test private org repo - repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) - - reviewers, err = repo_model.GetReviewers(ctx, repo3, 2, 1) - assert.NoError(t, err) - assert.Len(t, reviewers, 2) - - reviewers, err = repo_model.GetReviewers(ctx, repo3, 2, 2) - assert.NoError(t, err) - assert.Len(t, reviewers, 1) -} diff --git a/routers/api/v1/repo/collaborators.go b/routers/api/v1/repo/collaborators.go index ea9d8b0f37e9c..0bbf5a1ea49e8 100644 --- a/routers/api/v1/repo/collaborators.go +++ b/routers/api/v1/repo/collaborators.go @@ -17,6 +17,8 @@ import ( "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/convert" + issue_service "code.gitea.io/gitea/services/issue" + pull_service "code.gitea.io/gitea/services/pull" repo_service "code.gitea.io/gitea/services/repository" ) @@ -320,7 +322,13 @@ func GetReviewers(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - reviewers, err := repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0) + canChooseReviewer := issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, ctx.Repo.Repository, 0) + if !canChooseReviewer { + ctx.Error(http.StatusForbidden, "GetReviewers", errors.New("doer has no permission to get reviewers")) + return + } + + reviewers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0) if err != nil { ctx.Error(http.StatusInternalServerError, "ListCollaborators", err) return diff --git a/routers/web/repo/issue_page_meta.go b/routers/web/repo/issue_page_meta.go index ac0b1c6425dbd..e04d76b287d8f 100644 --- a/routers/web/repo/issue_page_meta.go +++ b/routers/web/repo/issue_page_meta.go @@ -19,7 +19,7 @@ import ( shared_user "code.gitea.io/gitea/routers/web/shared/user" "code.gitea.io/gitea/services/context" issue_service "code.gitea.io/gitea/services/issue" - repo_service "code.gitea.io/gitea/services/repository" + pull_service "code.gitea.io/gitea/services/pull" ) type issueSidebarMilestoneData struct { @@ -186,7 +186,7 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) { if d.Issue == nil { data.CanChooseReviewer = true } else { - data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue) + data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue.PosterID) } } @@ -231,13 +231,13 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) { if data.CanChooseReviewer { var err error - reviewers, err = repo_model.GetReviewers(ctx, repo, ctx.Doer.ID, posterID) + reviewers, err = pull_service.GetReviewers(ctx, repo, ctx.Doer.ID, posterID) if err != nil { ctx.ServerError("GetReviewers", err) return } - teamReviewers, err = repo_service.GetReviewerTeams(ctx, repo) + teamReviewers, err = pull_service.GetReviewerTeams(ctx, repo) if err != nil { ctx.ServerError("GetReviewerTeams", err) return diff --git a/services/issue/assignee.go b/services/issue/assignee.go index 52ee9f2b22cbf..c7e24955687f9 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -119,7 +119,7 @@ func isValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, return err } - canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue) + canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID) if isAdd { if !permReviewer.CanAccessAny(perm.AccessModeRead, unit.TypePullRequests) { @@ -178,7 +178,7 @@ func isValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team, } } - canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue) + canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID) if isAdd { if issue.Repo.IsPrivate { @@ -276,12 +276,12 @@ func teamReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doe } // CanDoerChangeReviewRequests returns if the doer can add/remove review requests of a PR -func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue) bool { +func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, posterID int64) bool { if repo.IsArchived { return false } // The poster of the PR can change the reviewers - if doer.ID == issue.PosterID { + if doer.ID == posterID { return true } diff --git a/services/pull/reviewer.go b/services/pull/reviewer.go new file mode 100644 index 0000000000000..bf0d8cb298c8b --- /dev/null +++ b/services/pull/reviewer.go @@ -0,0 +1,89 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "context" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/perm" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" + + "xorm.io/builder" +) + +// GetReviewers get all users can be requested to review: +// - Poster should not be listed +// - For collaborator, all users that have read access or higher to the repository. +// - For repository under organization, users under the teams which have read permission or higher of pull request unit +// - Owner will be listed if it's not an organization, not the poster and not in the list of reviewers +func GetReviewers(ctx context.Context, repo *repo_model.Repository, doerID, posterID int64) ([]*user_model.User, error) { + if err := repo.LoadOwner(ctx); err != nil { + return nil, err + } + + e := db.GetEngine(ctx) + uniqueUserIDs := make(container.Set[int64]) + + collaboratorIDs := make([]int64, 0, 10) + if err := e.Table("collaboration").Where("repo_id=?", repo.ID). + And("mode >= ?", perm.AccessModeRead). + Select("user_id"). + Find(&collaboratorIDs); err != nil { + return nil, err + } + uniqueUserIDs.AddMultiple(collaboratorIDs...) + + if repo.Owner.IsOrganization() { + additionalUserIDs := make([]int64, 0, 10) + if err := e.Table("team_user"). + Join("INNER", "team_repo", "`team_repo`.team_id = `team_user`.team_id"). + Join("INNER", "team_unit", "`team_unit`.team_id = `team_user`.team_id"). + Where("`team_repo`.repo_id = ? AND (`team_unit`.access_mode >= ? AND `team_unit`.`type` = ?)", + repo.ID, perm.AccessModeRead, unit.TypePullRequests). + Distinct("`team_user`.uid"). + Select("`team_user`.uid"). + Find(&additionalUserIDs); err != nil { + return nil, err + } + uniqueUserIDs.AddMultiple(additionalUserIDs...) + } + + uniqueUserIDs.Remove(posterID) // posterID should not be in the list of reviewers + + // Leave a seat for owner itself to append later, but if owner is an organization + // and just waste 1 unit is cheaper than re-allocate memory once. + users := make([]*user_model.User, 0, len(uniqueUserIDs)+1) + if len(uniqueUserIDs) > 0 { + if err := e.In("id", uniqueUserIDs.Values()). + Where(builder.Eq{"`user`.is_active": true}). + OrderBy(user_model.GetOrderByName()). + Find(&users); err != nil { + return nil, err + } + } + + // add owner after all users are loaded because we can avoid load owner twice + if repo.OwnerID != posterID && !repo.Owner.IsOrganization() && !uniqueUserIDs.Contains(repo.OwnerID) { + users = append(users, repo.Owner) + } + + return users, nil +} + +// GetReviewerTeams get all teams can be requested to review +func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*organization.Team, error) { + if err := repo.LoadOwner(ctx); err != nil { + return nil, err + } + if !repo.Owner.IsOrganization() { + return nil, nil + } + + return organization.GetTeamsWithAccessToRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests) +} diff --git a/services/pull/reviewer_test.go b/services/pull/reviewer_test.go new file mode 100644 index 0000000000000..1ff373bafb721 --- /dev/null +++ b/services/pull/reviewer_test.go @@ -0,0 +1,72 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull_test + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + pull_service "code.gitea.io/gitea/services/pull" + + "github.com/stretchr/testify/assert" +) + +func TestRepoGetReviewers(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + // test public repo + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + ctx := db.DefaultContext + reviewers, err := pull_service.GetReviewers(ctx, repo1, 2, 0) + assert.NoError(t, err) + if assert.Len(t, reviewers, 1) { + assert.ElementsMatch(t, []int64{2}, []int64{reviewers[0].ID}) + } + + // should not include doer and remove the poster + reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 2) + assert.NoError(t, err) + assert.Len(t, reviewers, 0) + + // should not include PR poster, if PR poster would be otherwise eligible + reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 4) + assert.NoError(t, err) + assert.Len(t, reviewers, 1) + + // test private user repo + repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + + reviewers, err = pull_service.GetReviewers(ctx, repo2, 2, 4) + assert.NoError(t, err) + assert.Len(t, reviewers, 1) + assert.EqualValues(t, reviewers[0].ID, 2) + + // test private org repo + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + + reviewers, err = pull_service.GetReviewers(ctx, repo3, 2, 1) + assert.NoError(t, err) + assert.Len(t, reviewers, 2) + + reviewers, err = pull_service.GetReviewers(ctx, repo3, 2, 2) + assert.NoError(t, err) + assert.Len(t, reviewers, 1) +} + +func TestRepoGetReviewerTeams(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + teams, err := pull_service.GetReviewerTeams(db.DefaultContext, repo2) + assert.NoError(t, err) + assert.Empty(t, teams) + + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + teams, err = pull_service.GetReviewerTeams(db.DefaultContext, repo3) + assert.NoError(t, err) + assert.Len(t, teams, 2) +} diff --git a/services/repository/review.go b/services/repository/review.go deleted file mode 100644 index 40513e6bc67ba..0000000000000 --- a/services/repository/review.go +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repository - -import ( - "context" - - "code.gitea.io/gitea/models/organization" - "code.gitea.io/gitea/models/perm" - repo_model "code.gitea.io/gitea/models/repo" -) - -// GetReviewerTeams get all teams can be requested to review -func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*organization.Team, error) { - if err := repo.LoadOwner(ctx); err != nil { - return nil, err - } - if !repo.Owner.IsOrganization() { - return nil, nil - } - - return organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead) -} diff --git a/services/repository/review_test.go b/services/repository/review_test.go deleted file mode 100644 index 2db56d4e8a9cc..0000000000000 --- a/services/repository/review_test.go +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repository - -import ( - "testing" - - "code.gitea.io/gitea/models/db" - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unittest" - - "github.com/stretchr/testify/assert" -) - -func TestRepoGetReviewerTeams(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - teams, err := GetReviewerTeams(db.DefaultContext, repo2) - assert.NoError(t, err) - assert.Empty(t, teams) - - repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) - teams, err = GetReviewerTeams(db.DefaultContext, repo3) - assert.NoError(t, err) - assert.Len(t, teams, 2) -} diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index 93c9ca0920d49..122afbfa08fb1 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -718,8 +718,8 @@ func TestAPIRepoGetReviewers(t *testing.T) { resp := MakeRequest(t, req, http.StatusOK) var reviewers []*api.User DecodeJSON(t, resp, &reviewers) - if assert.Len(t, reviewers, 3) { - assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID}) + if assert.Len(t, reviewers, 1) { + assert.ElementsMatch(t, []int64{2}, []int64{reviewers[0].ID}) } } From ae90b21db036507b8df9ed22e7ba416139037547 Mon Sep 17 00:00:00 2001 From: hiifong Date: Sat, 23 Nov 2024 02:26:05 +0800 Subject: [PATCH 6/8] Apply to became a maintainer (#32614) [PRs list](https://github.com/go-gitea/gitea/pulls?q=is%3Apr+author%3Ahiifong+is%3Aclosed+is%3Amerged) --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 160bffcdb77fc..426181cbcf139 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -63,3 +63,4 @@ Tim-Niclas Oelschläger (@zokkis) Yu Liu <1240335630@qq.com> (@HEREYUA) Kemal Zebari (@kemzeb) Rowan Bohde (@bohde) +hiifong (@hiifong) From f2a995174101b9fa9409acb2b47b065262098b28 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Fri, 22 Nov 2024 20:51:51 +0200 Subject: [PATCH 7/8] Update the list of watchers and stargazers when clicking watch/unwatch or star/unstar (#32570) We make sure the user cards are updated - Fixes https://github.com/go-gitea/gitea/issues/32561 I also removed `ctx.Data["PageIsWatchers"] = true` and `ctx.Data["PageIsStargazers"] = true` as they are not used anywhere. # Before ![before](https://github.com/user-attachments/assets/e3bc3235-35eb-4eda-862d-bdf2510282ea) # After ![after](https://github.com/user-attachments/assets/bc0488a5-8399-4cf6-95c9-17328a9702eb) --------- Signed-off-by: Yarden Shoham Co-authored-by: wxiaoguang Co-authored-by: silverwind --- routers/web/repo/repo.go | 3 +++ routers/web/repo/view.go | 3 --- templates/repo/user_cards.tmpl | 12 +++++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index be6f2d483ff31..b62fd21585184 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -352,6 +352,9 @@ func Action(ctx *context.Context) { ctx.Data["IsStaringRepo"] = repo_model.IsStaring(ctx, ctx.Doer.ID, ctx.Repo.Repository.ID) } + // see the `hx-trigger="refreshUserCards ..."` comments in tmpl + ctx.RespHeader().Add("hx-trigger", "refreshUserCards") + switch ctx.PathParam(":action") { case "watch", "unwatch", "star", "unstar": // we have to reload the repository because NumStars or NumWatching (used in the templates) has just changed diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index aacd7de6b1782..ec2ddfd79f47d 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -1123,8 +1123,6 @@ func RenderUserCards(ctx *context.Context, total int, getter func(opts db.ListOp func Watchers(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.watchers") ctx.Data["CardsTitle"] = ctx.Tr("repo.watchers") - ctx.Data["PageIsWatchers"] = true - RenderUserCards(ctx, ctx.Repo.Repository.NumWatches, func(opts db.ListOptions) ([]*user_model.User, error) { return repo_model.GetRepoWatchers(ctx, ctx.Repo.Repository.ID, opts) }, tplWatchers) @@ -1134,7 +1132,6 @@ func Watchers(ctx *context.Context) { func Stars(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.stargazers") ctx.Data["CardsTitle"] = ctx.Tr("repo.stargazers") - ctx.Data["PageIsStargazers"] = true RenderUserCards(ctx, ctx.Repo.Repository.NumStars, func(opts db.ListOptions) ([]*user_model.User, error) { return repo_model.GetStargazers(ctx, ctx.Repo.Repository, opts) }, tplWatchers) diff --git a/templates/repo/user_cards.tmpl b/templates/repo/user_cards.tmpl index 7cd3d4517ad9c..360aeaf619f8b 100644 --- a/templates/repo/user_cards.tmpl +++ b/templates/repo/user_cards.tmpl @@ -1,4 +1,14 @@ -
+ +
+
{{if .CardsTitle}}

{{.CardsTitle}} From 713364fc718d1d53840bd83ba6f6c307bd213fa8 Mon Sep 17 00:00:00 2001 From: Michael Owoc <130198442+mowoc-ocp@users.noreply.github.com> Date: Fri, 22 Nov 2024 15:12:06 -0500 Subject: [PATCH 8/8] Support optional/configurable IAMEndpoint for Minio Client (#32581) (#32581) Targeting issue #32271 This modification allows native Kubernetes + AWS (EKS) authentication with the Minio client, to Amazon S3 using the IRSA role assigned to a Service account by replacing the hard coded reference to the `DefaultIAMRoleEndpoint` with an optional configurable endpoint. Internally, Minio's `credentials.IAM` provider implements a discovery flow for IAM Endpoints if it is not set. For backwards compatibility: - We have added a configuration mechanism for an `IamEndpoint` to retain the unit test safety in `minio_test.go`. - We believe existing clients will continue to function the same without needing to provide a new config property since the internals of Minio client also often resolve to the `http://169.254.169.254` default endpoint that was being hard coded before To test, we were able to build a docker image from source and, observe it choosing the expected IAM endpoint, and see files uploaded via the client. --- custom/conf/app.example.ini | 14 ++++++++++++++ modules/setting/storage.go | 1 + modules/setting/storage_test.go | 13 +++++++++++++ modules/storage/minio.go | 8 +++++--- modules/storage/minio_test.go | 21 +++++++++++++-------- 5 files changed, 46 insertions(+), 11 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index ef5684237dc5b..c3b78e60bb47c 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1944,6 +1944,13 @@ LEVEL = Info ;; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio` ;MINIO_SECRET_ACCESS_KEY = ;; +;; Preferred IAM Endpoint to override Minio's default IAM Endpoint resolution only available when STORAGE_TYPE is `minio`. +;; If not provided and STORAGE_TYPE is `minio`, will search for and derive endpoint from known environment variables +;; (AWS_CONTAINER_AUTHORIZATION_TOKEN, AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE, AWS_CONTAINER_CREDENTIALS_RELATIVE_URI, +;; AWS_CONTAINER_CREDENTIALS_FULL_URI, AWS_WEB_IDENTITY_TOKEN_FILE, AWS_ROLE_ARN, AWS_ROLE_SESSION_NAME, AWS_REGION), +;; or the DefaultIAMRoleEndpoint if not provided otherwise. +;MINIO_IAM_ENDPOINT = +;; ;; Minio bucket to store the attachments only available when STORAGE_TYPE is `minio` ;MINIO_BUCKET = gitea ;; @@ -2688,6 +2695,13 @@ LEVEL = Info ;; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio` ;MINIO_SECRET_ACCESS_KEY = ;; +;; Preferred IAM Endpoint to override Minio's default IAM Endpoint resolution only available when STORAGE_TYPE is `minio`. +;; If not provided and STORAGE_TYPE is `minio`, will search for and derive endpoint from known environment variables +;; (AWS_CONTAINER_AUTHORIZATION_TOKEN, AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE, AWS_CONTAINER_CREDENTIALS_RELATIVE_URI, +;; AWS_CONTAINER_CREDENTIALS_FULL_URI, AWS_WEB_IDENTITY_TOKEN_FILE, AWS_ROLE_ARN, AWS_ROLE_SESSION_NAME, AWS_REGION), +;; or the DefaultIAMRoleEndpoint if not provided otherwise. +;MINIO_IAM_ENDPOINT = +;; ;; Minio bucket to store the attachments only available when STORAGE_TYPE is `minio` ;MINIO_BUCKET = gitea ;; diff --git a/modules/setting/storage.go b/modules/setting/storage.go index d6f7672b613d8..d3d1fb9f30b20 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -43,6 +43,7 @@ type MinioStorageConfig struct { Endpoint string `ini:"MINIO_ENDPOINT" json:",omitempty"` AccessKeyID string `ini:"MINIO_ACCESS_KEY_ID" json:",omitempty"` SecretAccessKey string `ini:"MINIO_SECRET_ACCESS_KEY" json:",omitempty"` + IamEndpoint string `ini:"MINIO_IAM_ENDPOINT" json:",omitempty"` Bucket string `ini:"MINIO_BUCKET" json:",omitempty"` Location string `ini:"MINIO_LOCATION" json:",omitempty"` BasePath string `ini:"MINIO_BASE_PATH" json:",omitempty"` diff --git a/modules/setting/storage_test.go b/modules/setting/storage_test.go index 44a5de68265ff..8ee37fd2b6d9c 100644 --- a/modules/setting/storage_test.go +++ b/modules/setting/storage_test.go @@ -470,6 +470,19 @@ MINIO_BASE_PATH = /prefix cfg, err = NewConfigProviderFromData(` [storage] STORAGE_TYPE = minio +MINIO_IAM_ENDPOINT = 127.0.0.1 +MINIO_USE_SSL = true +MINIO_BASE_PATH = /prefix +`) + assert.NoError(t, err) + assert.NoError(t, loadRepoArchiveFrom(cfg)) + assert.EqualValues(t, "127.0.0.1", RepoArchive.Storage.MinioConfig.IamEndpoint) + assert.EqualValues(t, true, RepoArchive.Storage.MinioConfig.UseSSL) + assert.EqualValues(t, "/prefix/repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) + + cfg, err = NewConfigProviderFromData(` +[storage] +STORAGE_TYPE = minio MINIO_ACCESS_KEY_ID = my_access_key MINIO_SECRET_ACCESS_KEY = my_secret_key MINIO_USE_SSL = true diff --git a/modules/storage/minio.go b/modules/storage/minio.go index 8acb7b0354c2e..6b92be61fb7d2 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -97,7 +97,7 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, } minioClient, err := minio.New(config.Endpoint, &minio.Options{ - Creds: buildMinioCredentials(config, credentials.DefaultIAMRoleEndpoint), + Creds: buildMinioCredentials(config), Secure: config.UseSSL, Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: config.InsecureSkipVerify}}, Region: config.Location, @@ -164,7 +164,7 @@ func (m *MinioStorage) buildMinioDirPrefix(p string) string { return p } -func buildMinioCredentials(config setting.MinioStorageConfig, iamEndpoint string) *credentials.Credentials { +func buildMinioCredentials(config setting.MinioStorageConfig) *credentials.Credentials { // If static credentials are provided, use those if config.AccessKeyID != "" { return credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, "") @@ -184,7 +184,9 @@ func buildMinioCredentials(config setting.MinioStorageConfig, iamEndpoint string &credentials.FileAWSCredentials{}, // read IAM role from EC2 metadata endpoint if available &credentials.IAM{ - Endpoint: iamEndpoint, + // passing in an empty Endpoint lets the IAM Provider + // decide which endpoint to resolve internally + Endpoint: config.IamEndpoint, Client: &http.Client{ Transport: http.DefaultTransport, }, diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go index 6eb03c4a45fdd..395da051e869b 100644 --- a/modules/storage/minio_test.go +++ b/modules/storage/minio_test.go @@ -107,8 +107,9 @@ func TestMinioCredentials(t *testing.T) { cfg := setting.MinioStorageConfig{ AccessKeyID: ExpectedAccessKey, SecretAccessKey: ExpectedSecretAccessKey, + IamEndpoint: FakeEndpoint, } - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -117,13 +118,15 @@ func TestMinioCredentials(t *testing.T) { }) t.Run("Chain", func(t *testing.T) { - cfg := setting.MinioStorageConfig{} + cfg := setting.MinioStorageConfig{ + IamEndpoint: FakeEndpoint, + } t.Run("EnvMinio", func(t *testing.T) { t.Setenv("MINIO_ACCESS_KEY", ExpectedAccessKey+"Minio") t.Setenv("MINIO_SECRET_KEY", ExpectedSecretAccessKey+"Minio") - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -135,7 +138,7 @@ func TestMinioCredentials(t *testing.T) { t.Setenv("AWS_ACCESS_KEY", ExpectedAccessKey+"AWS") t.Setenv("AWS_SECRET_KEY", ExpectedSecretAccessKey+"AWS") - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -144,11 +147,11 @@ func TestMinioCredentials(t *testing.T) { }) t.Run("FileMinio", func(t *testing.T) { - t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/minio.json") // prevent loading any actual credentials files from the user + t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/minio.json") t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "testdata/fake") - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -161,7 +164,7 @@ func TestMinioCredentials(t *testing.T) { t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/fake.json") t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "testdata/aws_credentials") - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -187,7 +190,9 @@ func TestMinioCredentials(t *testing.T) { defer server.Close() // Use the provided EC2 Instance Metadata server - creds := buildMinioCredentials(cfg, server.URL) + creds := buildMinioCredentials(setting.MinioStorageConfig{ + IamEndpoint: server.URL, + }) v, err := creds.Get() assert.NoError(t, err)