mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
125 stars 41 forks source link

Throttle site permission add-ons generation #8652

Closed diox closed 2 years ago

diox commented 2 years ago

Follow-up for https://github.com/mozilla/addons/issues/8544 : let's make submission of the site permission generator form go through rate limiting.

AlexandraMoga commented 2 years ago

I've verified this issue on stage and I have the following results:

  1. If I'm trying to resend the submission request by refreshing the Confirmation page and clicking Resend, I'm now being sent back to the generation form instead of remaining on the Confirmation page: firefox_vwHL7qROer

  2. If I'm trying to send several consecutive requests (with different origins) per minute with the same user, I'm able to successfully submit 2, while the 3d one is no longer sent; this is different from what we have for the normal submissions, where the rate limit kicks in on the 4th submission attempt within the same minute.

I've also noticed the following issues with the second scenario:

@diox I can file follow up issues for each of these scenarios if needed.

diox commented 2 years ago

there is no error message displayed in the form when the rate limit was reached; In the usual submission form we have this error message: "You have submitted too many uploads recently. Please try again after some time"

Oops, my bad. The message is generated but not shown, I'll fix it.

the activity logs for site-permission add-on generation are not matching the usual logs we capture for normal add-on submissions (...)

That's weird, do file a follow-up for that one please, I'll investigate. Because the add-on gets created in a separate task I might need to make some tweaks to the way we log it.

AlexandraMoga commented 2 years ago

That's weird, do file a follow-up for that one please, I'll investigate. Because the add-on gets created in a separate task I might need to make some tweaks to the way we log it. Filed mozilla/addons#8655

I also have an update for the incorrect throttling logs I've mentioned in my previous comment. Initially, I thought the form allowed only two successful add-on requests per user/minute and that user throttling activity logs were duplicated, but I think I understand now why that was the case. Here are the exact steps I've made and what I now think to be the logic behind the requests:

So, basically, each Resend counts as a request, although we do not generate an add-on when that happens.

Alternatively, if I open a fresh site-permission generation form each time I want to request a new add-on, the rate limiting works as expected, i.e. I can submit 3 addons per user/minute, while the forth request is throttled: image

diox commented 2 years ago

So, basically, each Resend counts as a request, although we do not generate an add-on when that happens.

Ah yes, the throttling happens before anything else. That's fine.

AlexandraMoga commented 2 years ago

Error message is visible now: image

This issue is verified fixed on stage.