Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EscapeFilter the group dn membership #20200

Merged
merged 2 commits into from
Jul 5, 2022

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Jul 1, 2022

The uid provided to the group filter must be properly escaped using the provided
ldap.EscapeFilter function.

Fix #20181

Signed-off-by: Andrew Thornton art27@cantab.net

The uid provided to the group filter must be properly escaped using the provided
ldap.EscapeFilter function.

Fix go-gitea#20181

Signed-off-by: Andrew Thornton <art27@cantab.net>
Copy link
Contributor

@Gusted Gusted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it the only thing that must be escaped within this function or related functions? LGTM

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jul 2, 2022
@zeripath
Copy link
Contributor Author

zeripath commented Jul 3, 2022

Is it the only thing that must be escaped within this function or related functions? LGTM

I believe so currently but I note we have functions like:

func (source *Source) sanitizedUserQuery(username string) (string, bool) {
	// See http://tools.ietf.org/search/rfc4515
	badCharacters := "\x00()*\\"
	if strings.ContainsAny(username, badCharacters) {
		log.Debug("'%s' contains invalid query characters. Aborting.", username)
		return "", false
	}

	return fmt.Sprintf(source.Filter, username), true
}

Which should just be escaping things properly instead of declaring bad characters.

@zeripath
Copy link
Contributor Author

zeripath commented Jul 4, 2022

It's been confirmed that this PR solves the related issue.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 5, 2022
@zeripath zeripath merged commit 6efbe49 into go-gitea:main Jul 5, 2022
@zeripath zeripath deleted the fix-20181-use-ldap.EscapeFilter branch July 5, 2022 15:59
@zeripath zeripath added the backport/done All backports for this PR have been created label Jul 5, 2022
zeripath added a commit to zeripath/gitea that referenced this pull request Jul 5, 2022
Backport go-gitea#20200

The uid provided to the group filter must be properly escaped using the provided
ldap.EscapeFilter function.

Fix go-gitea#20181

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit that referenced this pull request Jul 6, 2022
Backport #20200

The uid provided to the group filter must be properly escaped using the provided
ldap.EscapeFilter function.

Fix #20181

Signed-off-by: Andrew Thornton <art27@cantab.net>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 7, 2022
* upstream/main:
  Modify milestone search keywords to be case insensitive (go-gitea#20266)
  Fix toolip on mobile notification bell (go-gitea#20270)
  Allow RSA 2047 bit keys (go-gitea#20272)
  Refix notification bell placement (go-gitea#20251)
  Bump mermaid from 9.1.1 to 9.1.2 (go-gitea#20256)
  EscapeFilter the group dn membership (go-gitea#20200)
  Only show Followers that current user can access (go-gitea#20220)
  Init popup for new code comment (go-gitea#20234)
  Bypass Firefox (iOS) bug (go-gitea#20244)
  Adjust max-widths for the repository file table (go-gitea#20243)
  Display full name (go-gitea#20171)
  Adjust class for mobile has the problem of double small bells (go-gitea#20236)
  Adjust template for go-gitea#20069 smallbell (go-gitea#20108)
  Add integration tests for the Gitea migration form (go-gitea#20121)
  Allow dev i18n to be more concurrent (go-gitea#20159)
  Allow enable LDAP source and disable user sync via CLI (go-gitea#20206)
dineshsalunke pushed a commit to dineshsalunke/gitea that referenced this pull request Jul 9, 2022
The uid provided to the group filter must be properly escaped using the provided
ldap.EscapeFilter function.

Fix go-gitea#20181

Signed-off-by: Andrew Thornton <art27@cantab.net>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
The uid provided to the group filter must be properly escaped using the provided
ldap.EscapeFilter function.

Fix go-gitea#20181

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[1.17.0-rc1] LDAP Group Sync for organization teams issue with escaped characters in LDAP
5 participants