diff --git a/models/db/name.go b/models/db/name.go index 5f98edbb28d..e2165fd76bb 100644 --- a/models/db/name.go +++ b/models/db/name.go @@ -11,8 +11,6 @@ import ( "code.gitea.io/gitea/modules/util" ) -var ErrNameEmpty = util.SilentWrap{Message: "name is empty", Err: util.ErrInvalidArgument} - // ErrNameReserved represents a "reserved name" error. type ErrNameReserved struct { Name string @@ -79,7 +77,7 @@ func (err ErrNameCharsNotAllowed) Unwrap() error { func IsUsableName(reservedNames, reservedPatterns []string, name string) error { name = strings.TrimSpace(strings.ToLower(name)) if utf8.RuneCountInString(name) == 0 { - return ErrNameEmpty + return util.SilentWrap{Message: "name is empty", Err: util.ErrInvalidArgument} } for i := range reservedNames { diff --git a/models/repo/repo.go b/models/repo/repo.go index fb8a6642f50..4e27dbaf146 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -14,6 +14,7 @@ import ( "regexp" "strconv" "strings" + "sync" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unit" @@ -61,20 +62,30 @@ func (err ErrRepoIsArchived) Error() string { return fmt.Sprintf("%s is archived", err.Repo.LogString()) } -var ( - validRepoNamePattern = regexp.MustCompile(`[-.\w]+`) - invalidRepoNamePattern = regexp.MustCompile(`[.]{2,}`) - reservedRepoNames = []string{".", "..", "-"} - reservedRepoPatterns = []string{"*.git", "*.wiki", "*.rss", "*.atom"} -) +type globalVarsStruct struct { + validRepoNamePattern *regexp.Regexp + invalidRepoNamePattern *regexp.Regexp + reservedRepoNames []string + reservedRepoPatterns []string +} + +var globalVars = sync.OnceValue(func() *globalVarsStruct { + return &globalVarsStruct{ + validRepoNamePattern: regexp.MustCompile(`[-.\w]+`), + invalidRepoNamePattern: regexp.MustCompile(`[.]{2,}`), + reservedRepoNames: []string{".", "..", "-"}, + reservedRepoPatterns: []string{"*.git", "*.wiki", "*.rss", "*.atom"}, + } +}) // IsUsableRepoName returns true when name is usable func IsUsableRepoName(name string) error { - if !validRepoNamePattern.MatchString(name) || invalidRepoNamePattern.MatchString(name) { + vars := globalVars() + if !vars.validRepoNamePattern.MatchString(name) || vars.invalidRepoNamePattern.MatchString(name) { // Note: usually this error is normally caught up earlier in the UI return db.ErrNameCharsNotAllowed{Name: name} } - return db.IsUsableName(reservedRepoNames, reservedRepoPatterns, name) + return db.IsUsableName(vars.reservedRepoNames, vars.reservedRepoPatterns, name) } // TrustModelType defines the types of trust model for this repository diff --git a/models/repo/repo_test.go b/models/repo/repo_test.go index a9e2cdfb757..dffbd182613 100644 --- a/models/repo/repo_test.go +++ b/models/repo/repo_test.go @@ -219,4 +219,5 @@ func TestIsUsableRepoName(t *testing.T) { assert.Error(t, IsUsableRepoName("the..repo")) assert.Error(t, IsUsableRepoName("foo.wiki")) assert.Error(t, IsUsableRepoName("foo.git")) + assert.Error(t, IsUsableRepoName("foo.RSS")) } diff --git a/models/user/user.go b/models/user/user.go index 19879fbcc79..f790392a9bd 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -502,10 +502,10 @@ func (u *User) IsMailable() bool { return u.IsActive } -// IsUserExist checks if given user name exist, -// the user name should be noncased unique. +// IsUserExist checks if given username exist, +// the username should be non-cased unique. // If uid is presented, then check will rule out that one, -// it is used when update a user name in settings page. +// it is used when update a username in settings page. func IsUserExist(ctx context.Context, uid int64, name string) (bool, error) { if len(name) == 0 { return false, nil @@ -515,7 +515,7 @@ func IsUserExist(ctx context.Context, uid int64, name string) (bool, error) { Get(&User{LowerName: strings.ToLower(name)}) } -// Note: As of the beginning of 2022, it is recommended to use at least +// SaltByteLength as of the beginning of 2022, it is recommended to use at least // 64 bits of salt, but NIST is already recommending to use to 128 bits. // (16 bytes = 16 * 8 = 128 bits) const SaltByteLength = 16 diff --git a/models/user/user_test.go b/models/user/user_test.go index 7ebc64f69e7..cad1a64d6e8 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -25,6 +25,21 @@ import ( "github.com/stretchr/testify/assert" ) +func TestIsUsableUsername(t *testing.T) { + assert.NoError(t, user_model.IsUsableUsername("a")) + assert.NoError(t, user_model.IsUsableUsername("foo.wiki")) + assert.NoError(t, user_model.IsUsableUsername("foo.git")) + + assert.Error(t, user_model.IsUsableUsername("a--b")) + assert.Error(t, user_model.IsUsableUsername("-1_.")) + assert.Error(t, user_model.IsUsableUsername(".profile")) + assert.Error(t, user_model.IsUsableUsername("-")) + assert.Error(t, user_model.IsUsableUsername("🌞")) + assert.Error(t, user_model.IsUsableUsername("the..repo")) + assert.Error(t, user_model.IsUsableUsername("foo.RSS")) + assert.Error(t, user_model.IsUsableUsername("foo.PnG")) +} + func TestOAuth2Application_LoadUser(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) app := unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ID: 1}) diff --git a/modules/validation/glob_pattern_test.go b/modules/validation/glob_pattern_test.go index 1bf622e61d6..7f3e609acfa 100644 --- a/modules/validation/glob_pattern_test.go +++ b/modules/validation/glob_pattern_test.go @@ -19,39 +19,39 @@ func getGlobPatternErrorString(pattern string) string { return "" } -var globValidationTestCases = []validationTestCase{ - { - description: "Empty glob pattern", - data: TestForm{ - GlobPattern: "", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "Valid glob", - data: TestForm{ - GlobPattern: "{master,release*}", - }, - expectedErrors: binding.Errors{}, - }, +func Test_GlobPatternValidation(t *testing.T) { + AddBindingRules() - { - description: "Invalid glob", - data: TestForm{ - GlobPattern: "[a-", + globValidationTestCases := []validationTestCase{ + { + description: "Empty glob pattern", + data: TestForm{ + GlobPattern: "", + }, + expectedErrors: binding.Errors{}, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"GlobPattern"}, - Classification: ErrGlobPattern, - Message: getGlobPatternErrorString("[a-"), + { + description: "Valid glob", + data: TestForm{ + GlobPattern: "{master,release*}", }, + expectedErrors: binding.Errors{}, }, - }, -} -func Test_GlobPatternValidation(t *testing.T) { - AddBindingRules() + { + description: "Invalid glob", + data: TestForm{ + GlobPattern: "[a-", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"GlobPattern"}, + Classification: ErrGlobPattern, + Message: getGlobPatternErrorString("[a-"), + }, + }, + }, + } for _, testCase := range globValidationTestCases { t.Run(testCase.description, func(t *testing.T) { diff --git a/modules/validation/helpers.go b/modules/validation/helpers.go index f6e00f3887a..9f6cf5201a2 100644 --- a/modules/validation/helpers.go +++ b/modules/validation/helpers.go @@ -8,13 +8,26 @@ import ( "net/url" "regexp" "strings" + "sync" "code.gitea.io/gitea/modules/setting" "github.com/gobwas/glob" ) -var externalTrackerRegex = regexp.MustCompile(`({?)(?:user|repo|index)+?(}?)`) +type globalVarsStruct struct { + externalTrackerRegex *regexp.Regexp + validUsernamePattern *regexp.Regexp + invalidUsernamePattern *regexp.Regexp +} + +var globalVars = sync.OnceValue(func() *globalVarsStruct { + return &globalVarsStruct{ + externalTrackerRegex: regexp.MustCompile(`({?)(?:user|repo|index)+?(}?)`), + validUsernamePattern: regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`), + invalidUsernamePattern: regexp.MustCompile(`[-._]{2,}|[-._]$`), // No consecutive or trailing non-alphanumeric chars + } +}) func isLoopbackIP(ip string) bool { return net.ParseIP(ip).IsLoopback() @@ -105,9 +118,9 @@ func IsValidExternalTrackerURLFormat(uri string) bool { if !IsValidExternalURL(uri) { return false } - + vars := globalVars() // check for typoed variables like /{index/ or /[repo} - for _, match := range externalTrackerRegex.FindAllStringSubmatch(uri, -1) { + for _, match := range vars.externalTrackerRegex.FindAllStringSubmatch(uri, -1) { if (match[1] == "{" || match[2] == "}") && (match[1] != "{" || match[2] != "}") { return false } @@ -116,14 +129,10 @@ func IsValidExternalTrackerURLFormat(uri string) bool { return true } -var ( - validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) - invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) // No consecutive or trailing non-alphanumeric chars -) - // IsValidUsername checks if username is valid func IsValidUsername(name string) bool { // It is difficult to find a single pattern that is both readable and effective, // but it's easier to use positive and negative checks. - return validUsernamePattern.MatchString(name) && !invalidUsernamePattern.MatchString(name) + vars := globalVars() + return vars.validUsernamePattern.MatchString(name) && !vars.invalidUsernamePattern.MatchString(name) } diff --git a/modules/validation/refname_test.go b/modules/validation/refname_test.go index 3af7387c474..93703761cf9 100644 --- a/modules/validation/refname_test.go +++ b/modules/validation/refname_test.go @@ -9,253 +9,252 @@ import ( "gitea.com/go-chi/binding" ) -var gitRefNameValidationTestCases = []validationTestCase{ - { - description: "Reference name contains only characters", - data: TestForm{ - BranchName: "test", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "Reference name contains single slash", - data: TestForm{ - BranchName: "feature/test", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "Reference name has allowed special characters", - data: TestForm{ - BranchName: "debian/1%1.6.0-2", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "Reference name contains backslash", - data: TestForm{ - BranchName: "feature\\test", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", +func Test_GitRefNameValidation(t *testing.T) { + AddBindingRules() + gitRefNameValidationTestCases := []validationTestCase{ + { + description: "Reference name contains only characters", + data: TestForm{ + BranchName: "test", }, + expectedErrors: binding.Errors{}, }, - }, - { - description: "Reference name starts with dot", - data: TestForm{ - BranchName: ".test", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name contains single slash", + data: TestForm{ + BranchName: "feature/test", }, + expectedErrors: binding.Errors{}, }, - }, - { - description: "Reference name ends with dot", - data: TestForm{ - BranchName: "test.", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name has allowed special characters", + data: TestForm{ + BranchName: "debian/1%1.6.0-2", }, + expectedErrors: binding.Errors{}, }, - }, - { - description: "Reference name starts with slash", - data: TestForm{ - BranchName: "/test", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name contains backslash", + data: TestForm{ + BranchName: "feature\\test", }, - }, - }, - { - description: "Reference name ends with slash", - data: TestForm{ - BranchName: "test/", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name ends with .lock", - data: TestForm{ - BranchName: "test.lock", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name starts with dot", + data: TestForm{ + BranchName: ".test", }, - }, - }, - { - description: "Reference name contains multiple consecutive dots", - data: TestForm{ - BranchName: "te..st", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name contains multiple consecutive slashes", - data: TestForm{ - BranchName: "te//st", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name ends with dot", + data: TestForm{ + BranchName: "test.", }, - }, - }, - { - description: "Reference name is single @", - data: TestForm{ - BranchName: "@", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name has @{", - data: TestForm{ - BranchName: "branch@{", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name starts with slash", + data: TestForm{ + BranchName: "/test", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name has unallowed special character ~", - data: TestForm{ - BranchName: "~debian/1%1.6.0-2", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name ends with slash", + data: TestForm{ + BranchName: "test/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name has unallowed special character *", - data: TestForm{ - BranchName: "*debian/1%1.6.0-2", + { + description: "Reference name ends with .lock", + data: TestForm{ + BranchName: "test.lock", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name contains multiple consecutive dots", + data: TestForm{ + BranchName: "te..st", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name has unallowed special character ?", - data: TestForm{ - BranchName: "?debian/1%1.6.0-2", + { + description: "Reference name contains multiple consecutive slashes", + data: TestForm{ + BranchName: "te//st", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name is single @", + data: TestForm{ + BranchName: "@", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name has unallowed special character ^", - data: TestForm{ - BranchName: "^debian/1%1.6.0-2", + { + description: "Reference name has @{", + data: TestForm{ + BranchName: "branch@{", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name has unallowed special character ~", + data: TestForm{ + BranchName: "~debian/1%1.6.0-2", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name has unallowed special character :", - data: TestForm{ - BranchName: "debian:jessie", + { + description: "Reference name has unallowed special character *", + data: TestForm{ + BranchName: "*debian/1%1.6.0-2", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name has unallowed special character ?", + data: TestForm{ + BranchName: "?debian/1%1.6.0-2", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name has unallowed special character (whitespace)", - data: TestForm{ - BranchName: "debian jessie", + { + description: "Reference name has unallowed special character ^", + data: TestForm{ + BranchName: "^debian/1%1.6.0-2", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name has unallowed special character :", + data: TestForm{ + BranchName: "debian:jessie", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, - { - description: "Reference name has unallowed special character [", - data: TestForm{ - BranchName: "debian[jessie", + { + description: "Reference name has unallowed special character (whitespace)", + data: TestForm{ + BranchName: "debian jessie", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"BranchName"}, - Classification: ErrGitRefName, - Message: "GitRefName", + { + description: "Reference name has unallowed special character [", + data: TestForm{ + BranchName: "debian[jessie", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"BranchName"}, + Classification: ErrGitRefName, + Message: "GitRefName", + }, }, }, - }, -} - -func Test_GitRefNameValidation(t *testing.T) { - AddBindingRules() + } for _, testCase := range gitRefNameValidationTestCases { t.Run(testCase.description, func(t *testing.T) { diff --git a/modules/validation/regex_pattern_test.go b/modules/validation/regex_pattern_test.go index efcb2767340..80790a23b16 100644 --- a/modules/validation/regex_pattern_test.go +++ b/modules/validation/regex_pattern_test.go @@ -17,39 +17,39 @@ func getRegexPatternErrorString(pattern string) string { return "" } -var regexValidationTestCases = []validationTestCase{ - { - description: "Empty regex pattern", - data: TestForm{ - RegexPattern: "", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "Valid regex", - data: TestForm{ - RegexPattern: `(\d{1,3})+`, - }, - expectedErrors: binding.Errors{}, - }, +func Test_RegexPatternValidation(t *testing.T) { + AddBindingRules() - { - description: "Invalid regex", - data: TestForm{ - RegexPattern: "[a-", + regexValidationTestCases := []validationTestCase{ + { + description: "Empty regex pattern", + data: TestForm{ + RegexPattern: "", + }, + expectedErrors: binding.Errors{}, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"RegexPattern"}, - Classification: ErrRegexPattern, - Message: getRegexPatternErrorString("[a-"), + { + description: "Valid regex", + data: TestForm{ + RegexPattern: `(\d{1,3})+`, }, + expectedErrors: binding.Errors{}, }, - }, -} -func Test_RegexPatternValidation(t *testing.T) { - AddBindingRules() + { + description: "Invalid regex", + data: TestForm{ + RegexPattern: "[a-", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"RegexPattern"}, + Classification: ErrRegexPattern, + Message: getRegexPatternErrorString("[a-"), + }, + }, + }, + } for _, testCase := range regexValidationTestCases { t.Run(testCase.description, func(t *testing.T) { diff --git a/modules/validation/validurl_test.go b/modules/validation/validurl_test.go index 39f7fa5d65a..ce4898b2716 100644 --- a/modules/validation/validurl_test.go +++ b/modules/validation/validurl_test.go @@ -9,98 +9,98 @@ import ( "gitea.com/go-chi/binding" ) -var urlValidationTestCases = []validationTestCase{ - { - description: "Empty URL", - data: TestForm{ - URL: "", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "URL without port", - data: TestForm{ - URL: "http://test.lan/", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "URL with port", - data: TestForm{ - URL: "http://test.lan:3000/", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "URL with IPv6 address without port", - data: TestForm{ - URL: "http://[::1]/", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "URL with IPv6 address with port", - data: TestForm{ - URL: "http://[::1]:3000/", +func Test_ValidURLValidation(t *testing.T) { + AddBindingRules() + + urlValidationTestCases := []validationTestCase{ + { + description: "Empty URL", + data: TestForm{ + URL: "", + }, + expectedErrors: binding.Errors{}, }, - expectedErrors: binding.Errors{}, - }, - { - description: "Invalid URL", - data: TestForm{ - URL: "http//test.lan/", + { + description: "URL without port", + data: TestForm{ + URL: "http://test.lan/", + }, + expectedErrors: binding.Errors{}, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URL"}, - Classification: binding.ERR_URL, - Message: "Url", + { + description: "URL with port", + data: TestForm{ + URL: "http://test.lan:3000/", }, + expectedErrors: binding.Errors{}, }, - }, - { - description: "Invalid schema", - data: TestForm{ - URL: "ftp://test.lan/", + { + description: "URL with IPv6 address without port", + data: TestForm{ + URL: "http://[::1]/", + }, + expectedErrors: binding.Errors{}, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URL"}, - Classification: binding.ERR_URL, - Message: "Url", + { + description: "URL with IPv6 address with port", + data: TestForm{ + URL: "http://[::1]:3000/", }, + expectedErrors: binding.Errors{}, }, - }, - { - description: "Invalid port", - data: TestForm{ - URL: "http://test.lan:3x4/", + { + description: "Invalid URL", + data: TestForm{ + URL: "http//test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URL"}, + Classification: binding.ERR_URL, + Message: "Url", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URL"}, - Classification: binding.ERR_URL, - Message: "Url", + { + description: "Invalid schema", + data: TestForm{ + URL: "ftp://test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URL"}, + Classification: binding.ERR_URL, + Message: "Url", + }, }, }, - }, - { - description: "Invalid port with IPv6 address", - data: TestForm{ - URL: "http://[::1]:3x4/", + { + description: "Invalid port", + data: TestForm{ + URL: "http://test.lan:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URL"}, + Classification: binding.ERR_URL, + Message: "Url", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URL"}, - Classification: binding.ERR_URL, - Message: "Url", + { + description: "Invalid port with IPv6 address", + data: TestForm{ + URL: "http://[::1]:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URL"}, + Classification: binding.ERR_URL, + Message: "Url", + }, }, }, - }, -} - -func Test_ValidURLValidation(t *testing.T) { - AddBindingRules() + } for _, testCase := range urlValidationTestCases { t.Run(testCase.description, func(t *testing.T) { diff --git a/modules/validation/validurllist_test.go b/modules/validation/validurllist_test.go index c6f862a962b..cccc570a1a8 100644 --- a/modules/validation/validurllist_test.go +++ b/modules/validation/validurllist_test.go @@ -9,145 +9,145 @@ import ( "gitea.com/go-chi/binding" ) -// This is a copy of all the URL tests cases, plus additional ones to -// account for multiple URLs -var urlListValidationTestCases = []validationTestCase{ - { - description: "Empty URL", - data: TestForm{ - URLs: "", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "URL without port", - data: TestForm{ - URLs: "http://test.lan/", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "URL with port", - data: TestForm{ - URLs: "http://test.lan:3000/", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "URL with IPv6 address without port", - data: TestForm{ - URLs: "http://[::1]/", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "URL with IPv6 address with port", - data: TestForm{ - URLs: "http://[::1]:3000/", - }, - expectedErrors: binding.Errors{}, - }, - { - description: "Invalid URL", - data: TestForm{ - URLs: "http//test.lan/", - }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URLs"}, - Classification: binding.ERR_URL, - Message: "http//test.lan/", +func Test_ValidURLListValidation(t *testing.T) { + AddBindingRules() + + // This is a copy of all the URL tests cases, plus additional ones to + // account for multiple URLs + urlListValidationTestCases := []validationTestCase{ + { + description: "Empty URL", + data: TestForm{ + URLs: "", }, + expectedErrors: binding.Errors{}, }, - }, - { - description: "Invalid schema", - data: TestForm{ - URLs: "ftp://test.lan/", + { + description: "URL without port", + data: TestForm{ + URLs: "http://test.lan/", + }, + expectedErrors: binding.Errors{}, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URLs"}, - Classification: binding.ERR_URL, - Message: "ftp://test.lan/", + { + description: "URL with port", + data: TestForm{ + URLs: "http://test.lan:3000/", }, + expectedErrors: binding.Errors{}, }, - }, - { - description: "Invalid port", - data: TestForm{ - URLs: "http://test.lan:3x4/", + { + description: "URL with IPv6 address without port", + data: TestForm{ + URLs: "http://[::1]/", + }, + expectedErrors: binding.Errors{}, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URLs"}, - Classification: binding.ERR_URL, - Message: "http://test.lan:3x4/", + { + description: "URL with IPv6 address with port", + data: TestForm{ + URLs: "http://[::1]:3000/", }, + expectedErrors: binding.Errors{}, }, - }, - { - description: "Invalid port with IPv6 address", - data: TestForm{ - URLs: "http://[::1]:3x4/", + { + description: "Invalid URL", + data: TestForm{ + URLs: "http//test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http//test.lan/", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URLs"}, - Classification: binding.ERR_URL, - Message: "http://[::1]:3x4/", + { + description: "Invalid schema", + data: TestForm{ + URLs: "ftp://test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "ftp://test.lan/", + }, }, }, - }, - { - description: "Multi URLs", - data: TestForm{ - URLs: "http://test.lan:3000/\nhttp://test.local/", + { + description: "Invalid port", + data: TestForm{ + URLs: "http://test.lan:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://test.lan:3x4/", + }, + }, }, - expectedErrors: binding.Errors{}, - }, - { - description: "Multi URLs with newline", - data: TestForm{ - URLs: "http://test.lan:3000/\nhttp://test.local/\n", + { + description: "Invalid port with IPv6 address", + data: TestForm{ + URLs: "http://[::1]:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://[::1]:3x4/", + }, + }, }, - expectedErrors: binding.Errors{}, - }, - { - description: "List with invalid entry", - data: TestForm{ - URLs: "http://test.lan:3000/\nhttp://[::1]:3x4/", + { + description: "Multi URLs", + data: TestForm{ + URLs: "http://test.lan:3000/\nhttp://test.local/", + }, + expectedErrors: binding.Errors{}, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URLs"}, - Classification: binding.ERR_URL, - Message: "http://[::1]:3x4/", + { + description: "Multi URLs with newline", + data: TestForm{ + URLs: "http://test.lan:3000/\nhttp://test.local/\n", }, + expectedErrors: binding.Errors{}, }, - }, - { - description: "List with two invalid entries", - data: TestForm{ - URLs: "ftp://test.lan:3000/\nhttp://[::1]:3x4/\n", + { + description: "List with invalid entry", + data: TestForm{ + URLs: "http://test.lan:3000/\nhttp://[::1]:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://[::1]:3x4/", + }, + }, }, - expectedErrors: binding.Errors{ - binding.Error{ - FieldNames: []string{"URLs"}, - Classification: binding.ERR_URL, - Message: "ftp://test.lan:3000/", + { + description: "List with two invalid entries", + data: TestForm{ + URLs: "ftp://test.lan:3000/\nhttp://[::1]:3x4/\n", }, - binding.Error{ - FieldNames: []string{"URLs"}, - Classification: binding.ERR_URL, - Message: "http://[::1]:3x4/", + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "ftp://test.lan:3000/", + }, + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://[::1]:3x4/", + }, }, }, - }, -} - -func Test_ValidURLListValidation(t *testing.T) { - AddBindingRules() + } for _, testCase := range urlListValidationTestCases { t.Run(testCase.description, func(t *testing.T) { diff --git a/modules/web/middleware/flash.go b/modules/web/middleware/flash.go index 0caaa8c0364..0e848c79029 100644 --- a/modules/web/middleware/flash.go +++ b/modules/web/middleware/flash.go @@ -6,6 +6,7 @@ package middleware import ( "fmt" "html/template" + "net/http" "net/url" "code.gitea.io/gitea/modules/reqctx" @@ -65,3 +66,27 @@ func (f *Flash) Success(msg any, current ...bool) { f.SuccessMsg = flashMsgStringOrHTML(msg) f.set("success", f.SuccessMsg, current...) } + +func ParseCookieFlashMessage(val string) *Flash { + if vals, _ := url.ParseQuery(val); len(vals) > 0 { + return &Flash{ + Values: vals, + ErrorMsg: vals.Get("error"), + SuccessMsg: vals.Get("success"), + InfoMsg: vals.Get("info"), + WarningMsg: vals.Get("warning"), + } + } + return nil +} + +func GetSiteCookieFlashMessage(dataStore reqctx.RequestDataStore, req *http.Request, cookieName string) (string, *Flash) { + // Get the last flash message from cookie + lastFlashCookie := GetSiteCookie(req, cookieName) + lastFlashMsg := ParseCookieFlashMessage(lastFlashCookie) + if lastFlashMsg != nil { + lastFlashMsg.DataStore = dataStore + return lastFlashCookie, lastFlashMsg + } + return lastFlashCookie, nil +} diff --git a/modules/web/middleware/request.go b/modules/web/middleware/request.go deleted file mode 100644 index 0bb155df703..00000000000 --- a/modules/web/middleware/request.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package middleware - -import ( - "net/http" - "strings" -) - -// IsAPIPath returns true if the specified URL is an API path -func IsAPIPath(req *http.Request) bool { - return strings.HasPrefix(req.URL.Path, "/api/") -} diff --git a/routers/web/feed/convert.go b/routers/web/feed/convert.go index b7f849dc659..4ede5796b18 100644 --- a/routers/web/feed/convert.go +++ b/routers/web/feed/convert.go @@ -258,18 +258,18 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio } // GetFeedType return if it is a feed request and altered name and feed type. -func GetFeedType(name string, req *http.Request) (bool, string, string) { +func GetFeedType(name string, req *http.Request) (showFeed bool, feedType string) { if strings.HasSuffix(name, ".rss") || strings.Contains(req.Header.Get("Accept"), "application/rss+xml") { - return true, strings.TrimSuffix(name, ".rss"), "rss" + return true, "rss" } if strings.HasSuffix(name, ".atom") || strings.Contains(req.Header.Get("Accept"), "application/atom+xml") { - return true, strings.TrimSuffix(name, ".atom"), "atom" + return true, "atom" } - return false, name, "" + return false, "" } // feedActionsToFeedItems convert gitea's Repo's Releases to feeds Item diff --git a/routers/web/feed/render.go b/routers/web/feed/render.go index 462ebb97b5a..d1a229e5b27 100644 --- a/routers/web/feed/render.go +++ b/routers/web/feed/render.go @@ -9,7 +9,7 @@ import ( // RenderBranchFeed render format for branch or file func RenderBranchFeed(ctx *context.Context) { - _, _, showFeedType := GetFeedType(ctx.PathParam("reponame"), ctx.Req) + _, showFeedType := GetFeedType(ctx.PathParam("reponame"), ctx.Req) if ctx.Repo.TreePath == "" { ShowBranchFeed(ctx, ctx.Repo.Repository, showFeedType) } else { diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index d141df181ce..456efb96f6e 100644 --- a/routers/web/repo/view_home.go +++ b/routers/web/repo/view_home.go @@ -311,21 +311,21 @@ func prepareToRenderDirOrFile(entry *git.TreeEntry) func(ctx *context.Context) { } func handleRepoHomeFeed(ctx *context.Context) bool { - if setting.Other.EnableFeed { - isFeed, _, showFeedType := feed.GetFeedType(ctx.PathParam("reponame"), ctx.Req) - if isFeed { - switch { - case ctx.Link == fmt.Sprintf("%s.%s", ctx.Repo.RepoLink, showFeedType): - feed.ShowRepoFeed(ctx, ctx.Repo.Repository, showFeedType) - case ctx.Repo.TreePath == "": - feed.ShowBranchFeed(ctx, ctx.Repo.Repository, showFeedType) - case ctx.Repo.TreePath != "": - feed.ShowFileFeed(ctx, ctx.Repo.Repository, showFeedType) - } - return true - } + if !setting.Other.EnableFeed { + return false + } + isFeed, showFeedType := feed.GetFeedType(ctx.PathParam("reponame"), ctx.Req) + if !isFeed { + return false + } + if ctx.Link == fmt.Sprintf("%s.%s", ctx.Repo.RepoLink, showFeedType) { + feed.ShowRepoFeed(ctx, ctx.Repo.Repository, showFeedType) + } else if ctx.Repo.TreePath == "" { + feed.ShowBranchFeed(ctx, ctx.Repo.Repository, showFeedType) + } else { + feed.ShowFileFeed(ctx, ctx.Repo.Repository, showFeedType) } - return false + return true } // Home render repository home page diff --git a/routers/web/web.go b/routers/web/web.go index ef26dd9a743..5330b0f3c1b 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1152,7 +1152,7 @@ func registerRoutes(m *web.Router) { ) // end "/{username}/{reponame}/settings" - // user/org home, including rss feeds + // user/org home, including rss feeds like "/{username}/{reponame}.rss" m.Get("/{username}/{reponame}", optSignIn, context.RepoAssignment, context.RepoRefByType(git.RefTypeBranch), repo.SetEditorconfigIfExists, repo.Home) m.Post("/{username}/{reponame}/markup", optSignIn, context.RepoAssignment, reqUnitsWithMarkdown, web.Bind(structs.MarkupOption{}), misc.Markup) diff --git a/services/auth/auth.go b/services/auth/auth.go index 43ff95f0530..eb90202d24d 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -9,6 +9,7 @@ import ( "net/http" "regexp" "strings" + "sync" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/auth/webauthn" @@ -21,44 +22,74 @@ import ( user_service "code.gitea.io/gitea/services/user" ) +type globalVarsStruct struct { + gitRawOrAttachPathRe *regexp.Regexp + lfsPathRe *regexp.Regexp + archivePathRe *regexp.Regexp +} + +var globalVars = sync.OnceValue(func() *globalVarsStruct { + return &globalVarsStruct{ + gitRawOrAttachPathRe: regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/(?:(?:git-(?:(?:upload)|(?:receive))-pack$)|(?:info/refs$)|(?:HEAD$)|(?:objects/)|(?:raw/)|(?:releases/download/)|(?:attachments/))`), + lfsPathRe: regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/info/lfs/`), + archivePathRe: regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/archive/`), + } +}) + // Init should be called exactly once when the application starts to allow plugins // to allocate necessary resources func Init() { webauthn.Init() } +type authPathDetector struct { + req *http.Request + vars *globalVarsStruct +} + +func newAuthPathDetector(req *http.Request) *authPathDetector { + return &authPathDetector{req: req, vars: globalVars()} +} + +// isAPIPath returns true if the specified URL is an API path +func (a *authPathDetector) isAPIPath() bool { + return strings.HasPrefix(a.req.URL.Path, "/api/") +} + // isAttachmentDownload check if request is a file download (GET) with URL to an attachment -func isAttachmentDownload(req *http.Request) bool { - return strings.HasPrefix(req.URL.Path, "/attachments/") && req.Method == "GET" +func (a *authPathDetector) isAttachmentDownload() bool { + return strings.HasPrefix(a.req.URL.Path, "/attachments/") && a.req.Method == "GET" } // isContainerPath checks if the request targets the container endpoint -func isContainerPath(req *http.Request) bool { - return strings.HasPrefix(req.URL.Path, "/v2/") +func (a *authPathDetector) isContainerPath() bool { + return strings.HasPrefix(a.req.URL.Path, "/v2/") } -var ( - gitRawOrAttachPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/(?:(?:git-(?:(?:upload)|(?:receive))-pack$)|(?:info/refs$)|(?:HEAD$)|(?:objects/)|(?:raw/)|(?:releases/download/)|(?:attachments/))`) - lfsPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/info/lfs/`) - archivePathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/archive/`) -) - -func isGitRawOrAttachPath(req *http.Request) bool { - return gitRawOrAttachPathRe.MatchString(req.URL.Path) +func (a *authPathDetector) isGitRawOrAttachPath() bool { + return a.vars.gitRawOrAttachPathRe.MatchString(a.req.URL.Path) } -func isGitRawOrAttachOrLFSPath(req *http.Request) bool { - if isGitRawOrAttachPath(req) { +func (a *authPathDetector) isGitRawOrAttachOrLFSPath() bool { + if a.isGitRawOrAttachPath() { return true } if setting.LFS.StartServer { - return lfsPathRe.MatchString(req.URL.Path) + return a.vars.lfsPathRe.MatchString(a.req.URL.Path) } return false } -func isArchivePath(req *http.Request) bool { - return archivePathRe.MatchString(req.URL.Path) +func (a *authPathDetector) isArchivePath() bool { + return a.vars.archivePathRe.MatchString(a.req.URL.Path) +} + +func (a *authPathDetector) isAuthenticatedTokenRequest() bool { + switch a.req.URL.Path { + case "/login/oauth/userinfo", "/login/oauth/introspect": + return true + } + return false } // handleSignIn clears existing session variables and stores new ones for the specified user object diff --git a/services/auth/auth_test.go b/services/auth/auth_test.go index f1e9e6753fc..55ffdebe2d3 100644 --- a/services/auth/auth_test.go +++ b/services/auth/auth_test.go @@ -110,21 +110,21 @@ func Test_isGitRawOrLFSPath(t *testing.T) { t.Run(tt.path, func(t *testing.T) { req, _ := http.NewRequest("POST", "http://localhost"+tt.path, nil) setting.LFS.StartServer = false - assert.Equal(t, tt.want, isGitRawOrAttachOrLFSPath(req)) + assert.Equal(t, tt.want, newAuthPathDetector(req).isGitRawOrAttachOrLFSPath()) setting.LFS.StartServer = true - assert.Equal(t, tt.want, isGitRawOrAttachOrLFSPath(req)) + assert.Equal(t, tt.want, newAuthPathDetector(req).isGitRawOrAttachOrLFSPath()) }) } for _, tt := range lfsTests { t.Run(tt, func(t *testing.T) { req, _ := http.NewRequest("POST", tt, nil) setting.LFS.StartServer = false - got := isGitRawOrAttachOrLFSPath(req) - assert.Equalf(t, setting.LFS.StartServer, got, "isGitOrLFSPath(%q) = %v, want %v, %v", tt, got, setting.LFS.StartServer, gitRawOrAttachPathRe.MatchString(tt)) + got := newAuthPathDetector(req).isGitRawOrAttachOrLFSPath() + assert.Equalf(t, setting.LFS.StartServer, got, "isGitOrLFSPath(%q) = %v, want %v, %v", tt, got, setting.LFS.StartServer, globalVars().gitRawOrAttachPathRe.MatchString(tt)) setting.LFS.StartServer = true - got = isGitRawOrAttachOrLFSPath(req) + got = newAuthPathDetector(req).isGitRawOrAttachOrLFSPath() assert.Equalf(t, setting.LFS.StartServer, got, "isGitOrLFSPath(%q) = %v, want %v", tt, got, setting.LFS.StartServer) }) } diff --git a/services/auth/basic.go b/services/auth/basic.go index 6a05b2fe530..67987206a74 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" - "code.gitea.io/gitea/modules/web/middleware" ) // Ensure the struct implements the interface. @@ -49,7 +48,8 @@ func (b *Basic) Name() string { // Returns nil if header is empty or validation fails. func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) { // Basic authentication should only fire on API, Download or on Git or LFSPaths - if !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) { + detector := newAuthPathDetector(req) + if !detector.isAPIPath() && !detector.isContainerPath() && !detector.isAttachmentDownload() && !detector.isGitRawOrAttachOrLFSPath() { return nil, nil } diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 6f2cadd4abb..66cc6868093 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -16,7 +16,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" - "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/services/actions" "code.gitea.io/gitea/services/oauth2_provider" ) @@ -162,8 +161,9 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store Dat // Returns nil if verification fails. func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) { // These paths are not API paths, but we still want to check for tokens because they maybe in the API returned URLs - if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) && - !isGitRawOrAttachPath(req) && !isArchivePath(req) { + detector := newAuthPathDetector(req) + if !detector.isAPIPath() && !detector.isAttachmentDownload() && !detector.isAuthenticatedTokenRequest() && + !detector.isGitRawOrAttachPath() && !detector.isArchivePath() { return nil, nil } @@ -190,13 +190,3 @@ func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStor log.Trace("OAuth2 Authorization: Logged in user %-v", user) return user, nil } - -func isAuthenticatedTokenRequest(req *http.Request) bool { - switch req.URL.Path { - case "/login/oauth/userinfo": - fallthrough - case "/login/oauth/introspect": - return true - } - return false -} diff --git a/services/auth/reverseproxy.go b/services/auth/reverseproxy.go index 36b4ef68f42..d6664d738de 100644 --- a/services/auth/reverseproxy.go +++ b/services/auth/reverseproxy.go @@ -12,7 +12,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/web/middleware" gouuid "github.com/google/uuid" ) @@ -117,7 +116,8 @@ func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, store Da } // Make sure requests to API paths, attachment downloads, git and LFS do not create a new session - if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) { + detector := newAuthPathDetector(req) + if !detector.isAPIPath() && !detector.isAttachmentDownload() && !detector.isGitRawOrAttachOrLFSPath() { if sess != nil && (sess.Get("uid") == nil || sess.Get("uid").(int64) != user.ID) { handleSignIn(w, req, sess, user) } diff --git a/services/auth/sspi.go b/services/auth/sspi.go index 8ac83dcb044..3882740ae37 100644 --- a/services/auth/sspi.go +++ b/services/auth/sspi.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" - "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/services/auth/source/sspi" gitea_context "code.gitea.io/gitea/services/context" @@ -120,7 +119,8 @@ func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore, } // Make sure requests to API paths and PWA resources do not create a new session - if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) { + detector := newAuthPathDetector(req) + if !detector.isAPIPath() && !detector.isAttachmentDownload() { handleSignIn(w, req, sess, user) } @@ -155,8 +155,9 @@ func (s *SSPI) shouldAuthenticate(req *http.Request) (shouldAuth bool) { } else if req.FormValue("auth_with_sspi") == "1" { shouldAuth = true } - } else if middleware.IsAPIPath(req) || isAttachmentDownload(req) { - shouldAuth = true + } else { + detector := newAuthPathDetector(req) + shouldAuth = detector.isAPIPath() || detector.isAttachmentDownload() } return shouldAuth } diff --git a/services/context/context.go b/services/context/context.go index 6715c5663dc..5b16f9be982 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -165,18 +165,10 @@ func Contexter() func(next http.Handler) http.Handler { ctx.Base.SetContextValue(WebContextKey, ctx) ctx.Csrf = NewCSRFProtector(csrfOpts) - // Get the last flash message from cookie - lastFlashCookie := middleware.GetSiteCookie(ctx.Req, CookieNameFlash) + // get the last flash message from cookie + lastFlashCookie, lastFlashMsg := middleware.GetSiteCookieFlashMessage(ctx, ctx.Req, CookieNameFlash) if vals, _ := url.ParseQuery(lastFlashCookie); len(vals) > 0 { - // store last Flash message into the template data, to render it - ctx.Data["Flash"] = &middleware.Flash{ - DataStore: ctx, - Values: vals, - ErrorMsg: vals.Get("error"), - SuccessMsg: vals.Get("success"), - InfoMsg: vals.Get("info"), - WarningMsg: vals.Get("warning"), - } + ctx.Data["Flash"] = lastFlashMsg // store last Flash message into the template data, to render it } // if there are new messages in the ctx.Flash, write them into cookie diff --git a/tests/integration/editor_test.go b/tests/integration/editor_test.go index f0f71b80d1b..1999d1661ea 100644 --- a/tests/integration/editor_test.go +++ b/tests/integration/editor_test.go @@ -12,7 +12,6 @@ import ( "testing" "code.gitea.io/gitea/modules/json" - gitea_context "code.gitea.io/gitea/services/context" "github.com/stretchr/testify/assert" ) @@ -58,9 +57,8 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { }) session.MakeRequest(t, req, http.StatusSeeOther) // Check if master branch has been locked successfully - flashCookie := session.GetCookie(gitea_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.EqualValues(t, "success%3DBranch%2Bprotection%2Bfor%2Brule%2B%2522master%2522%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value) + flashMsg := session.GetCookieFlashMessage() + assert.EqualValues(t, `Branch protection for rule "master" has been updated.`, flashMsg.SuccessMsg) // Request editor page req = NewRequest(t, "GET", "/user2/repo1/_new/master/") @@ -98,9 +96,8 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { assert.EqualValues(t, "/user2/repo1/settings/branches", res["redirect"]) // Check if master branch has been locked successfully - flashCookie = session.GetCookie(gitea_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.EqualValues(t, "error%3DRemoving%2Bbranch%2Bprotection%2Brule%2B%25221%2522%2Bfailed.", flashCookie.Value) + flashMsg = session.GetCookieFlashMessage() + assert.EqualValues(t, `Removing branch protection rule "1" failed.`, flashMsg.ErrorMsg) }) } diff --git a/tests/integration/git_general_test.go b/tests/integration/git_general_test.go index 5d915d8a513..6dd29a438a7 100644 --- a/tests/integration/git_general_test.go +++ b/tests/integration/git_general_test.go @@ -28,7 +28,6 @@ import ( "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" - gitea_context "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -464,9 +463,8 @@ func doProtectBranch(ctx APITestContext, branch, userToWhitelistPush, userToWhit ctx.Session.MakeRequest(t, req, http.StatusSeeOther) // Check if master branch has been locked successfully - flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.EqualValues(t, "success%3DBranch%2Bprotection%2Bfor%2Brule%2B%2522"+url.QueryEscape(branch)+"%2522%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value) + flashMsg := ctx.Session.GetCookieFlashMessage() + assert.EqualValues(t, `Branch protection for rule "`+branch+`" has been updated.`, flashMsg.SuccessMsg) } } diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 46b93b0a108..9e487924d1f 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -29,6 +29,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/routers" gitea_context "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/tests" @@ -118,12 +119,11 @@ type TestSession struct { jar http.CookieJar } -func (s *TestSession) GetCookie(name string) *http.Cookie { +func (s *TestSession) GetRawCookie(name string) *http.Cookie { baseURL, err := url.Parse(setting.AppURL) if err != nil { return nil } - for _, c := range s.jar.Cookies(baseURL) { if c.Name == name { return c @@ -132,6 +132,20 @@ func (s *TestSession) GetCookie(name string) *http.Cookie { return nil } +func (s *TestSession) GetSiteCookie(name string) string { + c := s.GetRawCookie(name) + if c != nil { + v, _ := url.QueryUnescape(c.Value) + return v + } + return "" +} + +func (s *TestSession) GetCookieFlashMessage() *middleware.Flash { + cookie := s.GetSiteCookie(gitea_context.CookieNameFlash) + return middleware.ParseCookieFlashMessage(cookie) +} + func (s *TestSession) MakeRequest(t testing.TB, rw *RequestWrapper, expectedStatus int) *httptest.ResponseRecorder { t.Helper() req := rw.Request @@ -458,9 +472,9 @@ func VerifyJSONSchema(t testing.TB, resp *httptest.ResponseRecorder, schemaFile // GetUserCSRFToken returns CSRF token for current user func GetUserCSRFToken(t testing.TB, session *TestSession) string { t.Helper() - cookie := session.GetCookie("_csrf") + cookie := session.GetSiteCookie("_csrf") require.NotEmpty(t, cookie) - return cookie.Value + return cookie } // GetUserCSRFToken returns CSRF token for anonymous user (not logged in) diff --git a/tests/integration/mirror_push_test.go b/tests/integration/mirror_push_test.go index 0dd8919bff9..dd3ad18a5cd 100644 --- a/tests/integration/mirror_push_test.go +++ b/tests/integration/mirror_push_test.go @@ -9,7 +9,6 @@ import ( "net/http" "net/url" "strconv" - "strings" "testing" "time" @@ -20,7 +19,6 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" - gitea_context "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/migrations" mirror_service "code.gitea.io/gitea/services/mirror" repo_service "code.gitea.io/gitea/services/repository" @@ -92,9 +90,8 @@ func testCreatePushMirror(t *testing.T, session *TestSession, owner, repo, addre }) session.MakeRequest(t, req, http.StatusSeeOther) - flashCookie := session.GetCookie(gitea_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.Contains(t, flashCookie.Value, "success") + flashMsg := session.GetCookieFlashMessage() + assert.NotEmpty(t, flashMsg.SuccessMsg) } func doRemovePushMirror(t *testing.T, session *TestSession, owner, repo string, pushMirrorID int64) bool { @@ -104,8 +101,8 @@ func doRemovePushMirror(t *testing.T, session *TestSession, owner, repo string, "push_mirror_id": strconv.FormatInt(pushMirrorID, 10), }) resp := session.MakeRequest(t, req, NoExpectedStatus) - flashCookie := session.GetCookie(gitea_context.CookieNameFlash) - return resp.Code == http.StatusSeeOther && flashCookie != nil && strings.Contains(flashCookie.Value, "success") + flashMsg := session.GetCookieFlashMessage() + return resp.Code == http.StatusSeeOther && assert.NotEmpty(t, flashMsg.SuccessMsg) } func doUpdatePushMirror(t *testing.T, session *TestSession, owner, repo string, pushMirrorID int64, interval string) bool { diff --git a/tests/integration/rename_branch_test.go b/tests/integration/rename_branch_test.go index 576264ba951..492fdf781bc 100644 --- a/tests/integration/rename_branch_test.go +++ b/tests/integration/rename_branch_test.go @@ -11,7 +11,6 @@ import ( git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" - gitea_context "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -82,9 +81,8 @@ func testRenameBranch(t *testing.T, u *url.URL) { "to": "branch1", }) session.MakeRequest(t, req, http.StatusSeeOther) - flashCookie := session.GetCookie(gitea_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.Contains(t, flashCookie.Value, "error") + flashMsg := session.GetCookieFlashMessage() + assert.NotEmpty(t, flashMsg.ErrorMsg) branch2 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"}) assert.Equal(t, "branch2", branch2.Name) @@ -110,9 +108,8 @@ func testRenameBranch(t *testing.T, u *url.URL) { }) session.MakeRequest(t, req, http.StatusSeeOther) - flashCookie = session.GetCookie(gitea_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.Contains(t, flashCookie.Value, "success") + flashMsg = session.GetCookieFlashMessage() + assert.NotEmpty(t, flashMsg.SuccessMsg) unittest.AssertNotExistsBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch2"}) branch1 = unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo1.ID, Name: "branch1"}) diff --git a/tests/integration/signin_test.go b/tests/integration/signin_test.go index 5c041099450..6738d998d7a 100644 --- a/tests/integration/signin_test.go +++ b/tests/integration/signin_test.go @@ -79,7 +79,7 @@ func TestSigninWithRememberMe(t *testing.T) { }) session.MakeRequest(t, req, http.StatusSeeOther) - c := session.GetCookie(setting.CookieRememberName) + c := session.GetRawCookie(setting.CookieRememberName) assert.NotNil(t, c) session = emptyTestSession(t)