search

pglombardo / PasswordPusher

🔐 Securely share sensitive information with automatic expiration & deletion after a set number of views or duration. Track who, what and when with full audit logs.
https://docs.pwpush.com
Apache License 2.0
2.02k stars 343 forks source link

Deleting Push with Last View yields Error 500 #2522

Open Viajaz opened 1 week ago

Viajaz commented 1 week ago

🐛 Bug Report

Deleting a password push that is already on it's last view will yield HTTP error 500 on pglombardo/pwpush-public-gateway:stable

🔬 How To Reproduce

Steps to reproduce the behavior:

  1. Create Password Push with 5 days and 1 view
  2. View Password Push with pglombardo/pwpush-public-gateway:stable
  3. Delete the Push

Environment

Where are you running/using Password Pusher?

If applicable, what version of Password Pusher?: 1.45.4

📈 Expected behavior

Probably "We apologize but this secret link has expired."

pglombardo commented 1 week ago

Hey @Viajaz - I've been a bit underwater. I'll take a look at this soon.

Viajaz commented 1 week ago

Let me know if I can provide more information. Sorry it's a bit light on details but, as far as I can tell, it seems to be just that simple to reproduce.

pglombardo commented 5 days ago

Just reproduced the error now:

# Logfile created on 2024-09-18 12:33:43 +0000 by logger.rb/v1.6.1
E, [2024-09-23T12:54:00.273596 #39] ERROR -- : [2b11ce46-9262-4883-927f-e1a7ae248ebe]
[2b11ce46-9262-4883-927f-e1a7ae248ebe] NoMethodError (undefined method `root_url' for an instance of PasswordsController):
[2b11ce46-9262-4883-927f-e1a7ae248ebe]
[2b11ce46-9262-4883-927f-e1a7ae248ebe] app/controllers/passwords_controller.rb:293:in `block (2 levels) in destroy'
[2b11ce46-9262-4883-927f-e1a7ae248ebe] app/controllers/passwords_controller.rb:292:in `destroy'
[2b11ce46-9262-4883-927f-e1a7ae248ebe] app/controllers/concerns/set_locale.rb:9:in `set_locale'
pglombardo commented 4 days ago

Fix released in v1.45.9 which is building now. Thanks @Viajaz!

Viajaz commented 4 days ago

I switched to :latest for both containers and attempted to reproduce, unfortunately, still error 500 for me.

Confirmed in logs that pwpush-public-gateway is running 1.45.9:

Password Pusher: migrating database to latest...
Password Pusher Version: 1.45.9
Password Pusher: precompiling assets for customisations...
Password Pusher Version: 1.45.9

@pglombardo

pglombardo commented 4 days ago

How are you deleting the push? Via API or UI?

Viajaz commented 4 days ago

It was the UI, however, when I went to perform another reproduction, and collect some more HTTP request details for you, the error is now gone. I don't have an explanation, it was the same test instance that I had used 8 hours before this post and I checked to ensure the version was the latest when posting last.

Regardless, I guess problem is fixed? @pglombardo

pglombardo commented 4 days ago

No I think you are right. There is another code path that can give this error. I need a better fix.

magaiverpr commented 3 days ago

Hello,

Testing here, If I delete a push sended to a Public gateway, the page dont redirect to the page "you link expired", its runs eternal. BUT the link is deleted, if I refresh manually the page (press F5), the link was gone.

pglombardo commented 2 days ago

When a push is deleted/expired, if anonymous, you get redirected back to the "expired" page. If you are logged in and the push owner/creator, you get sent to the audit log.

Audit log doesn't exist on the public gateway. My theory is that the problem is somewhere in this redirect logic.

If you both test in an incognito window, does the problem still occur?

I'll get back on this soon.

magaiverpr commented 2 days ago

When a push is deleted/expired, if anonymous, you get redirected back to the "expired" page. If you are logged in and the push owner/creator, you get sent to the audit log.

Audit log doesn't exist on the public gateway. My theory is that the problem is somewhere in this redirect logic.

If you both test in an incognito window, does the problem still occur?

I'll get back on this soon.

I have 2 enviroments here: DEV and PROD.

Only PROD is using the gateway.

Tested in DEV enviroment without public gateway and incognito mode in another browser: image

Testing in PROD mode with public gateway and incognito mode in another browser: image

Forget to say, I am not using audit mode nor login

Viajaz commented 2 days ago

There's an argument to be made about the need to even show a Delete button if this was the last view.