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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from
Mar 8, 2019
Merged

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

merged 8 commits into from
Mar 8, 2019

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Mar 7, 2019

backport of #14464

Roland Tapken and others added 8 commits March 7, 2019 22:36
@blizzz
user_ldap: Filter groups after nexted groups
fe169b0 
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 <roland@bitarbeiter.net>
@blizzz
user_ldap: really resolve nested groups
0997fd9 
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 <roland@bitarbeiter.net>
@blizzz
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 <roland@bitarbeiter.net>
@blizzz
Fixed unit test: groupsMatchFilter will not be called multiple times …
3384e25 
…anymore.

Signed-off-by: Roland Tapken <roland@bitarbeiter.net>
@blizzz
fix nested group retrieval also for 2 other cases
a6c0d53 
and also consolidate logic in one method

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
with LDAP server set offline, config cannot be controlled via ocs any…
3ea24fa 
…more

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
add missing config bits to integration tests
5c21e94 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
remove unused use statement
a358c4d 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz added this to the Nextcloud 15.0.6 milestone Mar 7, 2019
@faily-bot
Copy link

faily-bot bot commented Mar 7, 2019

🤖 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 "test@nextcloud.com"       # 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 "test@nextcloud.com"   # UsersSettingsContext::iSeeThatTheFieldOfUserIs()
      Failed asserting that two strings are equal.
      --- Expected
      +++ Actual
      @@ @@
      -''
      +'test@nextcloud.com'

rullzer
rullzer approved these changes Mar 8, 2019
View reviewed changes
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

🐘

@MorrisJobke MorrisJobke merged commit e957b0a into stable15 Mar 8, 2019
@MorrisJobke MorrisJobke deleted the backport/14464/stable15 branch March 8, 2019 16:31
@blizzz blizzz mentioned this pull request Mar 15, 2019
Closed
@rullzer rullzer mentioned this pull request Mar 29, 2019
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rullzer rullzer rullzer approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@Cybso Cybso Awaiting requested review from Cybso

@MorrisJobke MorrisJobke Awaiting requested review from MorrisJobke

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@kesselb kesselb Awaiting requested review from kesselb

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug feature: ldap
Projects
None yet
Milestone
Nextcloud 15.0.6
Development

Successfully merging this pull request may close these issues.
4 participants
@blizzz @rullzer @ChristophWurst @MorrisJobke