ubccpsc / classy

Course management and automation.
MIT License
14 stars 23 forks source link

Webhook check fails for github.com #164

Closed nickbradley closed 6 years ago

nickbradley commented 6 years ago

Using the DNS lookup value for api.github.com might not work since it seems the webhook can be sent from a different host:

portal      | <T> 2018-11-22 19:09:03: AutoTestRouteHandler::githubWebhook(..) - start
portal      | <T> 2018-11-22 19:09:03: AutoTestRouteHandler::isWebhookFromGitHub(..) - start; x-forwarded from: 192.30.252.45
portal      | <E> 2018-11-22 19:09:03: AutoTestRouteHandler::isWebhookFromGitHub(..) - rejected: Webhook did not originate from GitHub; request addr: 192.30.252.45 !== expected addr: 185.199.111.153
portal      | <E> 2018-11-22 19:09:03: AutoTestRouteHandler::githubWebhook(..) - ERROR: Webhook did not originate from GitHub; request addr: 192.30.252.45 !== expected addr: 185.199.111.153
portal      | <E> 2018-11-22 19:09:03: AutoTestRoutes::handleError(..) - ERROR: Error processing webhook: Webhook did not originate from GitHub; request addr: 192.30.252.45 !== expected addr: 185.199.111.153

This was a legitimate webhook from the SECapstone org.

rtholmes commented 6 years ago

Really just need to update the webhooks to use a secret value; any thoughts on the right .env value to use @nickbradley ?

rtholmes commented 6 years ago

I'm surprised this is happening on sdmm; this should have been caught by the fulfill(true)?

const ghAPI = config.getProp(ConfigKey.githubAPI);
            if (ghAPI.indexOf('github.com') > 0) {
                Log.info('AutoTestRouteHandler::isWebhookFromGitHub(..) - accepted; host is github.com');
                return fulfill(true);
            }
rtholmes commented 6 years ago

See #138 ; this will fix this issue when it is rolled out.