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

Make check for empty trusted proxies more strict #14606

Merged
merged 2 commits into from
Mar 21, 2019

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Mar 9, 2019

Follow-Up #14261

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@MorrisJobke MorrisJobke force-pushed the bugfix/stricter-check-trusted-proxy branch from 9e914bc to 3855d78 Compare March 20, 2019 11:18
@MorrisJobke
Copy link
Member

Rebased to check if there are any CI failures.

@nextcloud nextcloud deleted a comment from faily-bot bot Mar 20, 2019
@MorrisJobke
Copy link
Member

@kesselb What is the status here?

@MorrisJobke MorrisJobke added the 2. developing Work in progress label Mar 20, 2019
@kesselb
Copy link
Contributor Author

kesselb commented Mar 20, 2019

Looks good to me 🤣 $trustedProxies is always an array so this check would be more precise.

@kesselb kesselb added enhancement 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 20, 2019
@kesselb kesselb requested review from rullzer and MorrisJobke March 20, 2019 13:21
@kesselb kesselb added this to the Nextcloud 16 milestone Mar 20, 2019
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Code makes sense 👍

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@faily-bot
Copy link

faily-bot bot commented Mar 20, 2019

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 17189: failure

TESTS=acceptance, TESTS-ACCEPTANCE=apps

  • tests/acceptance/features/apps.feature:43
Show full log
  Scenario: Enable an app bundle                                          # /drone/src/github.com/nextcloud/server/tests/acceptance/features/apps.feature:43
    Given I act as Jane                                                   # ActorContext::iActAs()
    And I am logged in as the admin                                       # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the Apps management                                        # SettingsMenuContext::iOpenTheAppsManagement()
    And I open the "App bundles" section                                  # AppNavigationContext::iOpenTheSection()
    When I enable all apps from the "Enterprise bundle"                   # AppsManagementContext::iEnableAllAppsFromThe()
    Then I see that the "Auditing / Logging" app has been enabled         # AppsManagementContext::iSeeThatTheAppHasBeenEnabled()
      Disable button in the app list for Auditing / Logging could not be found after 100 seconds (NoSuchElementException)
    And I see that the "LDAP user and group backend" app has been enabled # AppsManagementContext::iSeeThatTheAppHasBeenEnabled()

@MorrisJobke
Copy link
Member

  • tests/acceptance/features/apps.feature:43

"Fixed" with #14774 (caused by #14578)

@MorrisJobke MorrisJobke merged commit aee0b76 into master Mar 21, 2019
@MorrisJobke MorrisJobke deleted the bugfix/stricter-check-trusted-proxy branch March 21, 2019 08:15
@MorrisJobke MorrisJobke mentioned this pull request Mar 21, 2019
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants