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

[QA] Relative URL breaks client and mail notification #38888

Closed
gabi18 opened this issue Jun 24, 2021 · 27 comments
Closed

[QA] Relative URL breaks client and mail notification #38888

gabi18 opened this issue Jun 24, 2021 · 27 comments
Assignees
Labels
p2-high Escalation, on top of current planning, release blocker QA:p2

Comments

@gabi18
Copy link

gabi18 commented Jun 24, 2021

Versions:
ownCloud client 2.8.2

cloud.damken.com 10.7 with patch #38639 applied

Steps to reproduce

  1. login to damken.cloud.com as user1
  2. user1 shares a file or folder with user2
  3. start ownCloud client and login as user2
  4. go to 'Activity' 'Server Activity' tab and look for the notifications sent from the server
  5. click on the link

Result:
It's not possible to open the link (URL sent is relative).
Also the 'Decline', 'Accept' and 'Dismiss' buttons don't work

Share-Notification-Client

@gabi18
Copy link
Author

gabi18 commented Jun 24, 2021

The mail notification is broken as well.

Share-Mail-Kopano

@AlexAndBear
Copy link

AlexAndBear commented Jun 24, 2021

@gabi18 you need the latest notification app (or patch owncloud/notifications#333) to get the correct link in the email (this is where the mails are sent).

@micbar @TheOneRing I don't know what to do about the client issue here, client fix?
Where does the client fetch the info ? API Request?

@gabi18
Copy link
Author

gabi18 commented Jun 24, 2021

From client logfile:

06-24 11:50:26:414 [ info sync.httplogger ]:	"36eec34c-ae5c-4d9e-bbd0-448d92c08122: Request: GET https://cloud.damken.com/ocs/v2.php/apps/notifications/api/v1/notifications?format=json Header: { OCS-APIREQUEST: true, Authorization: Bearer [redacted], User-Agent: Mozilla/5.0 (Linux) mirall/2.8.2 (build 4326) (Damken Cloud, opensuse-leap-5.3.18-lp152.78-default ClientArchitecture: x86_64 OsArchitecture: x86_64), Accept: */*, X-Request-ID: 36eec34c-ae5c-4d9e-bbd0-448d92c08122, Cookie: oc_sessionPassphrase=HXI7NqQPzC9sYvtW0Yax%2B8E4yoAHT10ku1Nf1%2B7%2FO%2FfXO71LLd0ziZuCQTESEEjMOGhz8%2BXhYiYbpq2GiJlZxtiwSoHSa8AXSjGvyGWqkagnONEhceSSujVF4hwK74AZ; ocuhsyihz2sp=uq2qejgrdfdmsceneevp6t8tbj, } Data: []"

06-24 11:50:26:664 [ info sync.httplogger ]:	"36eec34c-ae5c-4d9e-bbd0-448d92c08122: Response: GET 200 https://cloud.damken.com/ocs/v2.php/apps/notifications/api/v1/notifications?format=json Header: { Date: Thu, 24 Jun 2021 09:50:26 GMT, Server: Apache/2.4.18 (Ubuntu), Strict-Transport-Security: max-age=15768000, X-Content-Type-Options: nosniff, X-XSS-Protection: 0, X-Robots-Tag: none, X-Frame-Options: SAMEORIGIN, X-Download-Options: noopen, X-Permitted-Cross-Domain-Policies: none, Expires: Thu, 19 Nov 1981 08:52:00 GMT, Cache-Control: no-cache, no-store, must-revalidate, Pragma: no-cache, Content-Security-Policy: default-src 'none';manifest-src 'self';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self';connect-src 'self';media-src 'self', ETag: 60e3994e09aed625e88b62dcb774c3ef, Content-Length: 3368, Keep-Alive: timeout=3, max=200, Connection: Keep-Alive, Content-Type: application/json; charset=utf-8, } Data: [{\"ocs\":{\"meta\":{\"status\":\"ok\",\"statuscode\":200,\"message\":null,\"totalitems\":\"\",\"itemsperpage\":\"\"},\"data\":[{\"notification_id\":1262,\"app\":\"files_sharing\",\"user\":\"gmohr\",\"datetime\":\"2021-06-23T14:59:12+00:00\",\"object_type\":\"local_share\",\"object_id\":\"ocinternal:1934\",\"subject\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" shared \\\"von damken.cloud\\\" with you\",\"message\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" invited you to view \\\"von damken.cloud\\\"\",\"link\":\"\\/index.php\\/f\\/17241526\",\"actions\":[{\"label\":\"Decline\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1934\",\"type\":\"DELETE\",\"primary\":false},{\"label\":\"Accept\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1934\",\"type\":\"POST\",\"primary\":true}],\"icon\":\"\\/core\\/img\\/actions\\/shared.svg\"},{\"notification_id\":1261,\"app\":\"files_sharing\",\"user\":\"gmohr\",\"datetime\":\"2021-06-23T14:58:14+00:00\",\"object_type\":\"local_share\",\"object_id\":\"ocinternal:1933\",\"subject\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" shared \\\"von cloud.damken.com\\\" with you\",\"message\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" invited you to view \\\"von cloud.damken.com\\\"\",\"link\":\"\\/index.php\\/f\\/17241524\",\"actions\":[{\"label\":\"Decline\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1933\",\"type\":\"DELETE\",\"primary\":false},{\"label\":\"Accept\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1933\",\"type\":\"POST\",\"primary\":true}],\"icon\":\"\\/core\\/img\\/actions\\/shared.svg\"},{\"notification_id\":1260,\"app\":\"files_sharing\",\"user\":\"gmohr\",\"datetime\":\"2021-06-23T14:56:17+00:00\",\"object_type\":\"local_share\",\"object_id\":\"ocinternal:1932\",\"subject\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" shared \\\"fuer Gabi\\\" with you\",\"message\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" invited you to view \\\"fuer Gabi\\\"\",\"link\":\"\\/index.php\\/f\\/17241519\",\"actions\":[{\"label\":\"Decline\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1932\",\"type\":\"DELETE\",\"primary\":false},{\"label\":\"Accept\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1932\",\"type\":\"POST\",\"primary\":true}],\"icon\":\"\\/core\\/img\\/actions\\/shared.svg\"},{\"notification_id\":1259,\"app\":\"files_sharing\",\"user\":\"gmohr\",\"datetime\":\"2021-06-23T14:50:38+00:00\",\"object_type\":\"local_share\",\"object_id\":\"ocinternal:1931\",\"subject\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" shared \\\"dreikopftile-3d\\\" with you\",\"message\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" invited you to view \\\"dreikopftile-3d\\\"\",\"link\":\"\\/index.php\\/f\\/16749203\",\"actions\":[{\"label\":\"Decline\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1931\",\"type\":\"DELETE\",\"primary\":false},{\"label\":\"Accept\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1931\",\"type\":\"POST\",\"primary\":true}],\"icon\":\"\\/core\\/img\\/actions\\/shared.svg\"},{\"notification_id\":1213,\"app\":\"files_sharing\",\"user\":\"gmohr\",\"datetime\":\"2021-05-04T22:25:45+00:00\",\"object_type\":\"local_share\",\"object_id\":\"ocinternal:1867\",\"subject\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" shared \\\"tmp\\\" with you\",\"message\":\"\\\"J\\u00fcrgen Weigert (jw)\\\" invited you to view \\\"tmp\\\"\",\"link\":\"\\/index.php\\/f\\/16646906\",\"actions\":[{\"label\":\"Decline\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1867\",\"type\":\"DELETE\",\"primary\":false},{\"label\":\"Accept\",\"link\":\"\\/ocs\\/v1.php\\/apps\\/files_sharing\\/api\\/v1\\/shares\\/pending\\/1867\",\"type\":\"POST\",\"primary\":true}],\"icon\":\"\\/core\\/img\\/actions\\/shared.svg\"}]}}]

Response line above includes the information: "link":"\/index.php\/f\/16749203",

@jnweiger
Copy link
Contributor

jnweiger commented Jun 24, 2021

FYI: There is also

@AlexAndBear
Copy link

Please keep in mind, these changes are dependent!
In order to get rid of errors in any 'Use relative url instead of absoulute url' ticket (in WebUI and Emails), you need the Notifications 0.5.3 app.

@micbar
Copy link
Contributor

micbar commented Jun 25, 2021

@jnweiger @janackermann @TheOneRing We need to know very soon because 10.8.0 is already in Code freeze.

@micbar micbar added p2-high Escalation, on top of current planning, release blocker QA:p2 labels Jun 25, 2021
@AlexAndBear
Copy link

@gabi18 can you confirm that the emails work with notifications app 0.5.3, we tested it and it works

@JammingBen
Copy link
Contributor

JammingBen commented Jun 25, 2021

We will do another update of the notification app to fix the behavior with the client.

-> owncloud/notifications#342
-> owncloud/notifications#343

@gabi18
Copy link
Author

gabi18 commented Jun 28, 2021

@gabi18 can you confirm that the emails work with notifications app 0.5.3, we tested it and it works

I will retest with with notifications app 0.5.3.

@gabi18
Copy link
Author

gabi18 commented Jun 28, 2021

@gabi18 can you confirm that the emails work with notifications app 0.5.3, we tested it and it works

I will retest with with notifications app 0.5.3.

Confirmed that email notification works (has absolute link) with 0.5.3.

@AlexAndBear
Copy link

@gabi18 thank you, with 0.5.4 (still in release phase) client should work again as well, I will inform you :)

@jnweiger
Copy link
Contributor

jnweiger commented Jun 28, 2021

A confusing side note:
Just saw an email notification with a localhost URL:

Subject: Activity notification
To: mailuser <gmohr@owncloud.com>


Hello mailuser,

You are receiving this email because the following things happened at http://localhost/

* admin announced Share Notification - Today at 2:12:48 PM
* admin shared Sharefolder with you - Today at 3:51:07 PM
* You created Sharefolder/crypt-o.mp3 - Today at 3:53:37 PM
* admin removed the share for Sharefolder - Today at 3:56:07 PM
* admin shared Photos with you - Today at 3:56:19 PM

--
ownCloud - A safe home for all your data
https://owncloud.org

I assume, this is due to not having set overwrite.cli.url set correctly, but seem related :-)

Yes, confirmed. This is dependant on overwrite.cli.url in config.php -- after

occ config:system:set overwrite.cli.url --value https://oc-10-8-0beta1-20210625.jw-qa.owncloud.works/owncloud

the next Activity notification looks like this:

Subject: Activity notification
To: mailuser <gmohr@owncloud.com>


Hello mailuser,

You are receiving this email because the following things happened at https://oc-10-8-0beta1-20210625.jw-qa.owncloud.works/owncloud/

* admin removed the share for Photos (2) - Today at 5:07:58 PM
* admin shared ownCloud Manual.pdf with you - Today at 5:08:08 PM

@AlexAndBear
Copy link

I saw this before with notifications 0.5.2, @jnweiger can you confirm this is 0.5.3 regression?

@jnweiger
Copy link
Contributor

jnweiger commented Jun 28, 2021

root@jw-oc-10-8-0beta1-20210625-bg2:/var/www/owncloud/config# occ app:list notifications
Enabled:
  - notifications:
    - Version: 0.5.3
    - Path: /var/www/owncloud/apps/notifications

Yes, this is 0.5.3 🤯 -- the above mails is some kind of summary mail, not the usual direct notification, that @gabi18 was hunting.

@gabi18
Copy link
Author

gabi18 commented Jun 28, 2021

On same server:
Notification_Mailhog

@gabi18
Copy link
Author

gabi18 commented Jun 28, 2021

Just saw another mail with 'localhost' URL for notification created from announcementcenter:

annoucement_mail

@AlexAndBear
Copy link

AlexAndBear commented Jun 28, 2021

@jnweiger can you please check if this happens with 0.5.2 as well ? I guess this is not related to the notifications app and not a regression

@AlexAndBear
Copy link

@gabi18 good or bad?

@AlexAndBear
Copy link

AlexAndBear commented Jun 28, 2021

@gabi18 which version of announcement center ist this ?

@gabi18
Copy link
Author

gabi18 commented Jun 28, 2021

Sorry for confusion, confirmed that after @jnweiger changed the overwrite.cli.url the link is correct.

@gabi18
Copy link
Author

gabi18 commented Jun 28, 2021

correct_URL

@AlexAndBear
Copy link

Okay, but I still wondering why in some cases the URL is correct and in some cases not, I guess not all of these mails are set with a relative url

@jnweiger
Copy link
Contributor

I don't think anything was wrong with these localhost urls. I had forgotten to correctly configure the overwrite.cli.url ---
But giving that a second thought: Is the concept of an overwrite.cli.url a good thing at all? It cannot really work with a multihomed server, can it? (I don't think that is a regression -- but cannot test with notifications 0.5.2 now; the setup is horribly tedious....)

@AlexAndBear
Copy link

TBH I don't have any idea, but I guess these emails are sent via the activity app, and those links are not relative yet, need to investigate

@micbar
Copy link
Contributor

micbar commented Jun 28, 2021

@jnweiger @janackermann cli url is always needed.
Activity mails are sent in the background

Background jobs are started via crontab -> cli
CLI has no awareness of url - no apache involved

So all CLI jobs require a overwrite.cli.url

I consider an owncloud without it as "nonfunctional"

@AlexAndBear
Copy link

@gabi18 the client issue has been fixed with notifications 0.5.4 👍

@gabi18
Copy link
Author

gabi18 commented Jul 5, 2021

Thank you!
Confirmed - tested on 10-8-0beta1 with notifications 0.5.4 installed - that client receives notifications containing an absolute path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-high Escalation, on top of current planning, release blocker QA:p2
Projects
None yet
Development

No branches or pull requests

5 participants