Fix webhooks for users with multiple API keys #3151

indirect · 2022-07-17T09:28:21Z

The vast majority of the 34,000 failed delayed jobs stored in the database are all exceptions caused by user.api_key b 8000 eing nil. That is in fact the expected and desired state now that users has_many :api_keys, so this PR adds tests to make sure there won't be exceptions and then uses the first API key it can find to generate an auth header for the webhook. We should probably use a dedicated webhook secret at some point, but for now this will at least stop the ongoing exceptions.

The backtrace that led me here was:

no implicit conversion of nil into String                                                                                                                                        
/app/app/jobs/notifier.rb:11:in `+'                                                                                                                                              
/app/app/jobs/notifier.rb:11:in `authorization'                                                                                                                                  
/app/app/jobs/notifier.rb:21:in `block in perform'                                                                                                                               
/usr/local/bundle/gems/timeout-0.3.0/lib/timeout.rb:179:in `block in timeout'                                                                                                    
/usr/local/bundle/gems/timeout-0.3.0/lib/timeout.rb:36:in `block in catch'                                                                                                       
/usr/local/bundle/gems/timeout-0.3.0/lib/timeout.rb:36:in `catch'                                                                                                                
/usr/local/bundle/gems/timeout-0.3.0/lib/timeout.rb:36:in `catch'                                                                                                                
/usr/local/bundle/gems/timeout-0.3.0/lib/timeout.rb:188:in `timeout'                                                                                                             
/app/app/jobs/notifier.rb:33:in `timeout'                                                                                                                                        
/app/app/jobs/notifier.rb:15:in `perform'

this resolves several exceptions that would occur if, for example, `user.api_key` was nil. which it is now, by default.

indirect added 2 commits July 17, 2022 02:18

failing tests for users with no api key, or many

d442385

use the first available api key to auth webhooks

e5c9361

this resolves several exceptions that would occur if, for example, `user.api_key` was nil. which it is now, by default.

indirect merged commit c69107e into master Jul 18, 2022

indirect deleted the webhooks-multiple-api-keys branch July 18, 2022 00:52

rubygems-org-shipit bot temporarily deployed to staging July 18, 2022 15:09 Inactive

rubygems-org-shipit bot temporarily deployed to production July 20, 2022 12:48 Inactive

tristandunn mentioned this pull request Aug 11, 2025

Improve webhook verification. #5901

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix webhooks for users with multiple API keys #3151

Fix webhooks for users with multiple API keys #3151

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Fix webhooks for users with multiple API keys #3151

Fix webhooks for users with multiple API keys #3151

Uh oh!

Conversation

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant