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

User is not found when is searched by name and one of surnames #7510 #8178

Closed
wants to merge 66 commits into from

Conversation

fgsl
Copy link

@fgsl fgsl commented Feb 5, 2018

Fixes #7510

@MorrisJobke
Copy link
Member

cc @nextcloud/ldap

Roland Tapken and others added 5 commits February 7, 2018 12:02
Refers to issue nextcloud#8220

user_ldap configured with custom filters for active directory access
(group-member-association is "member"). Then it can happen that the
members of a group contain members that don't belong to the users
available in Nextcloud (the most trivial reason is that the user filter
contains "(!(UserAccountControl:1.2.840.113556.1.4.803:=2))" to exclude
disabled users from being imported).

This can be fixed by applying the ldapUserFilter when resolving the UID
for a DN fetched from the group's member list.

Signed-off-by: Roland Tapken <roland@bitarbeiter.net>
…loud#6962, refs nextcloud#5309

It allows non-logged user to access public pages of applications restricted to a group

Signed-off-by: Julien Veyssier <eneiluj@posteo.net>
…abase."

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Avoids errors when the size exceeds MAX_INT because of the cast to int. Better cast it to float to avoid this.

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member

Copying my comment from #7510:

I would still like to have this properly documented instead of adding another hidden feature that adds more code. You could simply search for "*eyer" to find "Meyer" aswell as "Beyer" in the search field.

@@ -1467,12 +1467,17 @@ private function prepareSearchTerm($term) {
$config = \OC::$server->getConfig();

$allowEnum = $config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes');
$allowMedialSearches = $config->getSystemValue('user_ldap.enable_medial_search', false);
Copy link
Member

Choose a reason for hiding this comment

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

should not be a system value, but applications specific. And per configuration. Add it to OCA\User_LDAP\Configuration and you can read it from $this->connection in the Access class.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, @blizzz, I will change and submit again.

@blizzz
Copy link
Member

blizzz commented Mar 7, 2018

I would still like to have this properly documented instead of adding another hidden feature that adds more code. You could simply search for "*eyer" to find "Meyer" aswell as "Beyer" in the search field.

I am not vetoing this… I'd be OK but not super happy either. @fgsl convince us?

Signed-off-by: Robin Appelman <robin@icewind.nl>
@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Mar 7, 2018
Roland Tapken and others added 11 commits March 7, 2018 12:18
Signed-off-by: Roland Tapken <roland@bitarbeiter.net>
… new File

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
…mit for new File

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
… state

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
rullzer and others added 22 commits March 8, 2018 21:09
Updated popover rules to allow form inputs and added input submit for…
…ontroller

Use session heartbeat to update CSRF token
Besides the extraction some minor adjustments (sorting locators for file
action menu entries to reflect the order of the menu entries in the UI,
moving parametrized locators like "createMenuItemFor" above the locators
that use them and placing "descendantOf" calls always in a new line)
were made too.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This is needed to be able to easily use the actor as a key in an array.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The file list is used in other places besides the Files app (for
example, the File sharing app); in those cases the locators for the file
list elements are the same, but not for the ancestor of the file list.
To make possible to reuse the file list locators in those cases too now
they receive the ancestor to use.

Note that the locators for the file actions menu were not using an
ancestor locator because it is expected that there is only one file
actions menu at a time in the whole page; that may change in the future,
but for the time being it is a valid assumption and thus the ancestor
was not added to those locators in this commit.

Although the locators were generalized the steps themselves still use
the "FilesAppContext::currentSectionMainView" locator as ancestor; the
steps will be generalized in a following commit.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The "FileListContext" provides steps to interact with and check the
behaviour of a file list. However, the "FileListContext" does not know
the right file list ancestor that has to be used by the file list steps,
so until now the file list steps were explicitly wired to the Files app
and they could be used only in that case.

Instead of duplicating the steps with a slightly different name (for
example, "I create a new folder named :folderName in the public shared
folder" instead of "I create a new folder named :folderName") the steps
were generalized; now contexts that "know" that certain file list
ancestor has to be used by the FileListContext steps performed by
certain actor from that point on (until changed again) set it
explicitly. For example, when the current page is the Files app then the
ancestor of the file list is the main view of the current section of the
Files app, but when the current page is a shared link then the ancestor
is set to null (because there will be just one file list, and thus its
ancestor is not relevant to differentiate between instances)

A helper trait, "FileListAncestorSetter", was introduced to reduce the
boilerplate needed to set the file list ancestor from other contexts.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Joas Schilling <coding@schilljs.com>
…-for-permissions-on-public-shared-folders

Add acceptance tests for permissions on public shared folders
…for-user-pagination

Tests for email collaborators pagination
@fgsl
Copy link
Author

fgsl commented Mar 9, 2018

Hi, @blizzz. Sorry, but I feel that awareness of Access::escapeFilterPart() would make all more complicated. After read your post, I thought about it, but, if I really understand, we can fall in something like (not good) idea below:

  1. Create two static attributes for Access class:
private static $escapeFilterPartSearch = null;
private static $escapeFilterPartReplace = null;
  1. Create two static methods for Access class:
private static function getEscapeFilterPartSearch(){
    if (null == self::$escapeFilterPartSearch){
        self::$escapeFilterPartSearch = ($this->connection->ldapMedialSearch, array('\\', '(', ')'), array('*', '\\', '(', ')'));
    }
    return self::$escapeFilterPartSearch;
}
private static function getEscapeFilterPartReplace(){
    if (null == self::$escapeFilterPartReplace){
        self::$escapeFilterPartReplace = ($this->connection->ldapMedialSearch, array('\\\\', '\\(', '\\)'), array('\\\\', '\\(', '\\)'),);
    }
    return self::$escapeFilterPartReplace;
}
  1. Change three last lines of Access::escapeFilterPart() for this:

return $asterisk . str_replace(self::getEscapeFilterPartSearch(), self::getEscapeFilterPartReplace(), $input);

@MorrisJobke
Copy link
Member

@fgsl You merged master into your branch ... maybe a git rebase onto of current master is a lot better to keep the PR clean. See http://morrisjobke.de/2015/12/03/How-to-do-a-git-rebase/ for some steps to do so, otherwise reach out here so we could guide you :)

@fgsl
Copy link
Author

fgsl commented Mar 9, 2018

@fgsl You merged master into your branch ... maybe a git rebase onto of current master is a lot better to keep the PR clean. See http://morrisjobke.de/2015/12/03/How-to-do-a-git-rebase/ for some steps to do so, otherwise reach out here so we could guide you :)

Thank you and I'm sorry, @MorrisJobke. I will follow guidelines.

@fgsl fgsl closed this Mar 9, 2018
@fgsl fgsl deleted the ldap branch March 9, 2018 14:35
@fgsl
Copy link
Author

fgsl commented Mar 9, 2018

@MorrisJobke and @blizzz , I made a mistake and had to submit a new pull request: #8760.

@MorrisJobke
Copy link
Member

@MorrisJobke and @blizzz , I made a mistake and had to submit a new pull request: #8760.

Don't worry :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.