Sort out token authentication

[jeancochrane]

The HMAC hash algo that's supposed to decrypt secret tokens from GitHub isn't working properly. c.f. the webhooks page for the deploy repo, which reports a 401 unauthorized response to the payload:

The hook will return this response if no token matching the hash is present in the secrets file:

https://github.com/jeancochrane/bunny-hook/blob/8dd8d9f39d520beb8876d47b71501620e6ce4876/api/routes.py#L103-L115

The correct token exists on the server, but there are a couple things I can think of that might explain this result:

• The tokens aren't being properly loaded into the Flask context object g on startup
• The HMAC hash that is supposed to reverse the token isn't implemented properly:

https://github.com/jeancochrane/bunny-hook/blob/8dd8d9f39d520beb8876d47b71501620e6ce4876/api/routes.py#L77-L86

For context on how GitHub hashes secret tokens, see Securing your webhooks.

[kalilsn]

@jeancochrane I did some local testing and g.get('tokens', []) always returns [] for me. What's the reason for storing it on the Flask application like that?

If that's not essential, I can open up a PR that just imports the tokens directly in routes.py.

[kalilsn]

I also think we should consider adjusting how this works so that the tokens are in a dictionary:

TOKENS = {'branch_name': 'XXXXXXXXX'}

and then we would check them with:

try:
    if hmac.compare_digest(get_hmac(tokens[branch_name]), post_sig):
             # Payload is good; queue up work 
             payload_json = request.get_json() 
             return queue(payload_json, branch_name) 
except KeyError:
    status = 'No token configured for ' + branch_name

This eliminates the need for the loop that generates a token for each possible branch (potentially costly) and also means that knowing one token doesn't let you build a different project (not super important given how we're going to use this). It's also worth noting that we should use hmac.compare_digest instead of == to prevent timing attacks.

[jeancochrane]

@kalilsn answering inlne:

What's the reason for storing it [secrets] on the Flask application like that?

I vaguely recall having some kind of convoluted fantasy about injecting new secrets into the app dynamically so that the process didn't have to be restarted to add a new app? But I think just importing api.secrets directly makes sense.

I also think we should consider adjusting how this works so that the tokens are in a dictionary

Fully agreed.

It's also worth noting that we should use hmac.compare_digest instead of == to prevent timing attacks

Good catch!

[kalilsn]

That's a good point! Well it would be cool to eventually create a web interface for this to monitor jobs, see logs/error output, and adjust config, and at that point I think storing those tokens in whatever database we use (probably just sqlite) would make sense.