[stable15] resolve user and groups in nested groups first before filtering the results #14591

blizzz · 2019-03-07T21:43:43Z

backport of #14464

Currently groupsMatchFilter is called before nested groups are resolved. This basicly breaks this feature since it is not possible to inherit membership in a group from another group. Minimal example: Group filter: (&(objectClass=group),(cn=nextcloud)) Nested groups: enabled cn=nextcloud,ou=Nextcloud,ou=groups,dn=company,dn=local objectClass: group cn=IT,ou=groups,dn=company,dn=local objectClass: group memberOf: cn=nextcloud,ou=Nextcloud,ou=groups,dn=company,dn=local cn=John Doe,ou=users,dn=company,dn=local objectClass: person memberOf: cn=IT,ou=groups,dn=company,dn=local Since 'cn=IT,ou=groups,dn=company,dn=local' doesn't match the group filter, John wouldn't be a member of group 'nextcloud'. This patch fixes this by filtering the groups after all nested groups have been collected. If nested groups is disabled the result will be the same as without this patch. Signed-off-by: Roland Tapken <[email protected]>

The previous patch fixed the problem only for one level of indirection because groupsMatchFilter() had been applied on each recursive call (and thus there would be no second level if the first level fails the check). This new implementation replaces the recursive call with a stack that iterates all nested groups before filtering with groupsMatchFilter(). Signed-off-by: Roland Tapken <[email protected]>

Nested groups are now cached in a CappedMemoryCache object to reduce queries to the LDAP backend. Signed-off-by: Roland Tapken <[email protected]>

…anymore. Signed-off-by: Roland Tapken <[email protected]>

and also consolidate logic in one method Signed-off-by: Arthur Schiwon <[email protected]>

…more Signed-off-by: Arthur Schiwon <[email protected]>

Signed-off-by: Arthur Schiwon <[email protected]>

faily-bot · 2019-03-07T22:43:29Z

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 16862: failure

TESTS=acceptance, TESTS-ACCEPTANCE=app-comments

cancelled - typically means that the tests took longer than the drone CI allows them to run

TESTS=acceptance, TESTS-ACCEPTANCE=app-files

tests/acceptance/features/app-files.feature:79
tests/acceptance/features/app-files.feature:133
tests/acceptance/features/app-files.feature:185

Show full log

  Scenario: show recent files for a second time                               # /drone/src/github.com/nextcloud/server/tests/acceptance/features/app-files.feature:79
    Given I am logged in                                                      # LoginPageContext::iAmLoggedIn()
    And I open the "Recent" section                                           # AppNavigationContext::iOpenTheSection()
    And I see that the current section is "Recent"                            # AppNavigationContext::iSeeThatTheCurrentSectionIs()
    And I open the "All files" section                                        # AppNavigationContext::iOpenTheSection()
    And I see that the current section is "All files"                         # AppNavigationContext::iSeeThatTheCurrentSectionIs()
    And I create a new folder named "Folder just created"                     # FileListContext::iCreateANewFolderNamed()
    When I open the "Recent" section                                          # AppNavigationContext::iOpenTheSection()
    Then I see that the current section is "Recent"                           # AppNavigationContext::iSeeThatTheCurrentSectionIs()
    Then I see that the file list contains a file named "Folder just created" # FileListContext::iSeeThatTheFileListContainsAFileNamed()
      Row for file Folder just created in file list could not be found after 100 seconds (NoSuchElementException)

  Scenario: show deleted files for a second time                      # /drone/src/github.com/nextcloud/server/tests/acceptance/features/app-files.feature:133
    Given I am logged in                                              # LoginPageContext::iAmLoggedIn()
    And I open the "Deleted files" section                            # AppNavigationContext::iOpenTheSection()
    And I see that the current section is "Deleted files"             # AppNavigationContext::iSeeThatTheCurrentSectionIs()
    And I open the "All files" section                                # AppNavigationContext::iOpenTheSection()
    And I see that the current section is "All files"                 # AppNavigationContext::iSeeThatTheCurrentSectionIs()
    And I delete "welcome.txt"                                        # FileListContext::iDelete()
    When I open the "Deleted files" section                           # AppNavigationContext::iOpenTheSection()
    Then I see that the current section is "Deleted files"            # AppNavigationContext::iSeeThatTheCurrentSectionIs()
    Then I see that the file list contains a file named "welcome.txt" # FileListContext::iSeeThatTheFileListContainsAFileNamed()
      Row for file welcome.txt in file list could not be found after 100 seconds (NoSuchElementException)

  Scenario: copy a selection to another folder                                       # /drone/src/github.com/nextcloud/server/tests/acceptance/features/app-files.feature:185
    Given I am logged in                                                             # LoginPageContext::iAmLoggedIn()
    And I create a new folder named "Folder"                                         # FileListContext::iCreateANewFolderNamed()
    And I create a new folder named "Not selected folder"                            # FileListContext::iCreateANewFolderNamed()
    And I create a new folder named "Destination"                                    # FileListContext::iCreateANewFolderNamed()
    When I select "welcome.txt"                                                      # FileListContext::iSelect()
    And I select "Folder"                                                            # FileListContext::iSelect()
    And I start the move or copy operation for the selected files                    # FileListContext::iStartTheMoveOrCopyOperationForTheSelectedFiles()
    And I select "Destination" in the file picker                                    # FilePickerContext::iSelectInTheFilePicker()
    And I copy to the last selected folder in the file picker                        # FilePickerContext::iCopyToTheLastSelectedFolderInTheFilePicker()
    Then I enter in the folder named "Destination"                                   # FileListContext::iEnterInTheFolderNamed()
    And I see that the file list contains a file named "welcome.txt"                 # FileListContext::iSeeThatTheFileListContainsAFileNamed()
      Row for file welcome.txt in file list could not be found after 100 seconds (NoSuchElementException)
    And I see that the file list contains a file named "Folder"                      # FileListContext::iSeeThatTheFileListContainsAFileNamed()
    And I see that the file list does not contain a file named "Not selected folder" # FileListContext::iSeeThatTheFileListDoesNotContainAFileNamed()
    And I open the Files app                                                         # FilesAppContext::iOpenTheFilesApp()
    And I see that the file list contains a file named "welcome.txt"                 # FileListContext::iSeeThatTheFileListContainsAFileNamed()
    And I see that the file list contains a file named "Folder"                      # FileListContext::iSeeThatTheFileListContainsAFileNamed()
    And I see that the file list contains a file named "Not selected folder"         # FileListContext::iSeeThatTheFileListContainsAFileNamed()

TESTS=acceptance, TESTS-ACCEPTANCE=app-files-sharing

cancelled - typically means that the tests took longer than the drone CI allows them to run

TESTS=acceptance, TESTS-ACCEPTANCE=users

tests/acceptance/features/users.feature:116
tests/acceptance/features/users.feature:137

Show full log

  Scenario: change display name                                        # /drone/src/github.com/nextcloud/server/tests/acceptance/features/users.feature:116
    Given I act as Jane                                                # ActorContext::iActAs()
    And I am logged in as the admin                                    # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                       # SettingsMenuContext::iOpenTheUserSettings()
    And I see that the list of users contains the user user0           # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
    And I see that the displayName of user0 is user0                   # UsersSettingsContext::iSeeThatTheFieldOfUserIs()
    When I set the displayName for user0 to user1                      # UsersSettingsContext::iSetTheFieldForUserTo()
    And I see that the displayName cell for user user0 is done loading # UsersSettingsContext::iSeeThatTheCellForUserIsDoneLoading()
    Then I see that the displayName of user0 is user1                  # UsersSettingsContext::iSeeThatTheFieldOfUserIs()
      Failed asserting that two strings are equal.
      --- Expected
      +++ Actual
      @@ @@
      -'user0'
      +'user1'

  Scenario: change email                                               # /drone/src/github.com/nextcloud/server/tests/acceptance/features/users.feature:137
    Given I act as Jane                                                # ActorContext::iActAs()
    And I am logged in as the admin                                    # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                       # SettingsMenuContext::iOpenTheUserSettings()
    And I see that the list of users contains the user user0           # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
    And I see that the mailAddress of user0 is ""                      # UsersSettingsContext::iSeeThatTheFieldOfUserIs()
    When I set the mailAddress for user0 to "[email protected]"       # UsersSettingsContext::iSetTheFieldForUserTo()
    And I see that the mailAddress cell for user user0 is done loading # UsersSettingsContext::iSeeThatTheCellForUserIsDoneLoading()
    Then I see that the mailAddress of user0 is "[email protected]"   # UsersSettingsContext::iSeeThatTheFieldOfUserIs()
      Failed asserting that two strings are equal.
      --- Expected
      +++ Actual
      @@ @@
      -''
      +'[email protected]'

rullzer

🐘

Roland Tapken and others added 8 commits March 7, 2019 22:36

Reduce queries to LDAP by caching nested groups

0a469e8

Nested groups are now cached in a CappedMemoryCache object to reduce queries to the LDAP backend. Signed-off-by: Roland Tapken <[email protected]>

Fixed unit test: groupsMatchFilter will not be called multiple times …

3384e25

…anymore. Signed-off-by: Roland Tapken <[email protected]>

fix nested group retrieval also for 2 other cases

a6c0d53

and also consolidate logic in one method Signed-off-by: Arthur Schiwon <[email protected]>

with LDAP server set offline, config cannot be controlled via ocs any…

3ea24fa

…more Signed-off-by: Arthur Schiwon <[email protected]>

add missing config bits to integration tests

5c21e94

Signed-off-by: Arthur Schiwon <[email protected]>

remove unused use statement

a358c4d

Signed-off-by: Arthur Schiwon <[email protected]>

blizzz added bug 3. to review Waiting for reviews feature: ldap labels Mar 7, 2019

blizzz added this to the Nextcloud 15.0.6 milestone Mar 7, 2019

blizzz requested review from ChristophWurst, Cybso, MorrisJobke, juliusknorr, kesselb and rullzer March 7, 2019 21:43

ChristophWurst approved these changes Mar 8, 2019

View reviewed changes

rullzer approved these changes Mar 8, 2019

View reviewed changes

MorrisJobke merged commit e957b0a into stable15 Mar 8, 2019

MorrisJobke deleted the backport/14464/stable15 branch March 8, 2019 16:31

blizzz mentioned this pull request Mar 15, 2019

LDAP Nested Groups doesn't work with Active Directory #14711

Closed

rullzer mentioned this pull request Mar 29, 2019

15.0.6 RC 1 #14909

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[stable15] resolve user and groups in nested groups first before filtering the results #14591

[stable15] resolve user and groups in nested groups first before filtering the results #14591

Uh oh!

blizzz commented Mar 7, 2019

Uh oh!

faily-bot bot commented Mar 7, 2019

Uh oh!

rullzer left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

[stable15] resolve user and groups in nested groups first before filtering the results #14591

[stable15] resolve user and groups in nested groups first before filtering the results #14591

Uh oh!

Conversation

blizzz commented Mar 7, 2019

Uh oh!

faily-bot bot commented Mar 7, 2019

Status of 16862: failure

TESTS=acceptance, TESTS-ACCEPTANCE=app-comments

TESTS=acceptance, TESTS-ACCEPTANCE=app-files

TESTS=acceptance, TESTS-ACCEPTANCE=app-files-sharing

TESTS=acceptance, TESTS-ACCEPTANCE=users

Uh oh!

rullzer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants