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

[Bug]: Share by mail password expiration not changing after requesting password #31951

Closed
4 of 8 tasks
PVince81 opened this issue Apr 12, 2022 · 5 comments
Closed
4 of 8 tasks
Labels
1. to develop Accepted and waiting to be taken care of bug feature: sharing
Milestone

Comments

@PVince81
Copy link
Member

⚠️ This issue respects the following points: ⚠️

  • This is a bug, not a question or a configuration/webserver/proxy issue.
  • This issue is not already reported on Github (I've searched it).
  • Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
  • I agree to follow Nextcloud's Code of Conduct.

Bug description

The expiration of the password needs to bet adjusted after requesting a new password.

Before requesting:

MariaDB [nextcloud]> select id, fc.name, share_type, password_expiration_time from oc_share s left join oc_filecache fc on s.file_source=fc.fileid;
+----+----------+------------+--------------------------+
| id | name     | share_type | password_expiration_time |
+----+----------+------------+--------------------------+
|  1 | testpre  |          4 | NULL                     |
|  2 | testpost |          4 | 2022-04-12 20:44:11      |
+----+----------+------------+--------------------------+
2 rows in set (0.000 sec)

After requesting for both:

MariaDB [nextcloud]> select id, fc.name, share_type, password_expiration_time from oc_share s left join oc_filecache fc on s.file_source=fc.fileid;
+----+----------+------------+--------------------------+
| id | name     | share_type | password_expiration_time |
+----+----------+------------+--------------------------+
|  1 | testpre  |          4 | NULL                     |
|  2 | testpost |          4 | 2022-04-11 20:44:11      |
+----+----------+------------+--------------------------+
2 rows in set (0.000 sec)

The expiration date was not set for the first link and the second one stayed the same and was not refreshed to a future time.

Steps to reproduce

  1. Setup stable23
  2. Setup mailhog docker + configure SMTP to receive emails
  3. Create a folder "testpre"
  4. Share "testpre" by email with "vincent@example.org"
  5. Open the three dots and set a password "test123"
  6. Log out
  7. Upgrade to master
  8. Login as admin again
  9. Create a folder "testpost"
  10. Share "testpost" by email with "vincent@example.org"
  11. Open the three dots and set a password "test123"
  12. Log out
  13. select * from oc_share and take note
  14. Open the link in the email for "testpre"
  15. Request a new password by entering "vincent@example.org"
  16. Open the link in the email for "testpost"
  17. Change the expiration date in oc_share to one day before
  18. Request a new password by entering "vincent@example.org"
  19. Check the database again

Expected behavior

In the table oc_share, before requesting a password:

  • "testpre" has no password expiration date set
  • "testpost" has a password expiration date set

In the table oc_share, after requesting a password:

  • "testpre" and "testpost" should both have a password expiration set to a recent date instead of being stuck on the old state

Installation method

No response

Operating system

No response

PHP engine version

No response

Web server

No response

Database engine version

No response

Is this bug present after an update or on a fresh install?

No response

Are you using the Nextcloud Server Encryption module?

No response

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

No response

List of activated Apps

Enabled:
  - accessibility: 1.10.0
  - activity: 2.16.0
  - cloud_federation_api: 1.7.0
  - comments: 1.14.0
  - contactsinteraction: 1.5.0
  - dashboard: 7.4.0
  - dav: 1.22.0
  - federatedfilesharing: 1.14.0
  - federation: 1.14.0
  - files: 1.19.0
  - files_external: 1.16.1
  - files_sharing: 1.16.2
  - files_trashbin: 1.14.0
  - files_versions: 1.17.0
  - lookup_server_connector: 1.12.0
  - nextcloud_announcements: 1.13.0
  - oauth2: 1.12.0
  - photos: 1.6.0
  - provisioning_api: 1.14.0
  - recommendations: 1.3.0
  - serverinfo: 1.14.0
  - settings: 1.6.0
  - sharebymail: 1.14.0
  - survey_client: 1.12.0
  - systemtags: 1.14.0
  - theming: 1.15.0
  - twofactor_backupcodes: 1.13.0
  - updatenotification: 1.14.0
  - user_status: 1.4.0
  - viewer: 1.8.0
  - weather_status: 1.4.0
  - workflowengine: 2.6.0
Disabled:
  - admin_audit
  - bruteforcesettings
  - calendar
  - contacts
  - deck
  - email_template_example-0.0.1
  - encryption
  - files_3d
  - files_accesscontrol
  - files_antivirus
  - files_automatedtagging
  - files_downloadlimit
  - files_retention
  - files_texteditor
  - files_versions_s3
  - groupfolders
  - guests
  - mail
  - music
  - officeonline
  - password_policy: 1.14.0
  - previewgenerator
  - ransomware_protection
  - registration
  - richdocuments
  - shareimporter
  - sharepermissions
  - spreed
  - support: 1.7.0
  - suspicious_login
  - terms_of_service
  - testing
  - twofactor_totp
  - user_ldap
  - user_migration
  - user_saml
  - workflow_script

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

stable23 was 8ec5401
master on df14579 which includes #31220

@PVince81
Copy link
Member Author

oh, it even looks like that after requesting the password, even the "password" column value doesn't change and so the password received in the email doesn't work.

maybe something is wrong with the logic that touches the database

@PVince81
Copy link
Member Author

the code for updating expiration is here https://github.com/nextcloud/server/blob/master/apps/files_sharing/lib/Controller/ShareAPIController.php#L1186 for when the password is changed in the web UI

however the code that sets expiration date is not called when requesting a password: https://github.com/nextcloud/server/blob/master/apps/files_sharing/lib/Controller/ShareController.php#L229
it goes straight to the manager

so perhaps we need to move the method that generates the expiration to the manager ?

I can't say whether this worked before since the PR had evolved quite a bit

@PVince81
Copy link
Member Author

or: move the expiration value setting to the ShareByMail provider every time the password changes, regardless whether set by the user or generated.

has this been considered before ? @StCyr

@PVince81
Copy link
Member Author

PR here: #31965

@StCyr
Copy link
Contributor

StCyr commented Apr 14, 2022

Thanks for fixing the issue @PVince81

Your change makes the initial password (at share's creation, via the ShareAPIController) permanent, but I think it's OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug feature: sharing
Projects
None yet
Development

No branches or pull requests

2 participants