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

Always renew apppasswords on login #25460

Merged
merged 1 commit into from
Feb 10, 2021
Merged

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented Feb 3, 2021

Else you can end up that you renewed your password (LDAP for example).
But they still don't work because you did not use them before you logged
in.

@rullzer rullzer added bug 3. to review Waiting for reviews labels Feb 3, 2021
@rullzer rullzer added this to the Nextcloud 22 milestone Feb 3, 2021
@rullzer
Copy link
Member Author

rullzer commented Feb 3, 2021

/backport to stable21

@juliusknorr
Copy link
Member

Code makes sense, but still one failure:

  1. Test\Authentication\Token\PublicKeyTokenProviderTest::testUpdatePasswordsNotRequired
    OC\Authentication\Token\PublicKeyTokenMapper::getTokenByUser('myUID'): array was not expected to be called.

/drone/src/lib/private/Authentication/Token/PublicKeyTokenProvider.php:418
/drone/src/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php:578

Else you can end up that you renewed your password (LDAP for example).
But they still don't work because you did not use them before you logged
in.

Signed-off-by: Roeland Jago Douma <[email protected]>
@faily-bot
Copy link

faily-bot bot commented Feb 10, 2021

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 2122: failure

acceptance-header

  • tests/acceptance/features/header.feature:33
Show full log
  Scenario: users from other groups are not seen in the contacts menu when autocompletion is restricted within the same group # /drone/src/tests/acceptance/features/header.feature:33
    Given I am logged in as the admin                                                                                         # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I visit the settings page                                                                                             # SettingsMenuContext::iVisitTheSettingsPage()
    And I open the "Sharing" section of the "Administration" group                                                            # AppNavigationContext::iOpenTheSectionOf()
    And I enable restricting username autocompletion to groups                                                                # SettingsContext::iEnableRestrictingUsernameAutocompletionToGroups()
    And I see that username autocompletion is restricted to groups                                                            # SettingsContext::iSeeThatUsernameAutocompletionIsRestrictedToGroups()
    When I open the Contacts menu                                                                                             # ContactsMenuContext::iOpenTheContactsMenu()
    Then I see that the Contacts menu is shown                                                                                # ContactsMenuContext::iSeeThatTheContactsMenuIsShown()
    And I see that the contact "user0" in the Contacts menu is not shown                                                      # ContactsMenuContext::iSeeThatTheContactInTheContactsMenuIsNotShown()
      Failed asserting that true is false.
    And I see that the contact "admin" in the Contacts menu is not shown                                                      # ContactsMenuContext::iSeeThatTheContactInTheContactsMenuIsNotShown()

@zimmersi
Copy link

zimmersi commented Jul 8, 2021

these changes breaking the webauthn login (not 2FA). NC 21.0.3 and NC 22
The login process itself is working though, but the users are constantly logged out after a short period of time. this affects not only the sessions in the browser, also all sync clients are logged out every few minutes, which makes it unusebale!

after reverting this changes in my NC 21.0.3, everything is working again as expected!

@akhil1508
Copy link
Contributor

akhil1508 commented Dec 22, 2021

There was a major increase in the number of UPDATE queries of the form: UPDATE `authtoken` SET `password` = <redacted> ==' WHERE `id` = <redacted> due to the changes in this PR after updating from 20.0.12 to 21.0.7. To avoid this increase in load on the database, this PR's changes had to be reverted.

  • Stats on day before reverting the changes(one hour time frame):
    before

  • Stats on day after reverting the changes(one hour time frame):
    after

  • We see a decrease after reverting the changes in:

    • Number of UPDATE queries
    • Number of open tables cache
    • Rate of questions
    • Network traffic
    • Thread activity
  • On another smaller nextcloud instance with less than 10 active users, we had ~2k UPDATE queries every minute.

  • This comment is related to the issue and was also the fix for our problem.

@nickvergessen
Copy link
Member

nickvergessen commented Dec 22, 2021

#29682 should fix that
That being said, I'm not aware that 21 is impacted as the faulty other part is not there.

@akhil1508
Copy link
Contributor

#29682 should fix that That being said, I'm not aware that 21 is impacted as the faulty other part is not there.

21 is indeed impacted with high database load. The above screenshots are from a 21.0.7 server.

@nickvergessen
Copy link
Member

Maybe you can create a new issue then as the author of the PR is no longer with the project and therefore commenting on a ~1 year old PR is not the best way forward.

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 bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants