8000 Breaks up public API (#6) by rjz · Pull Request #7 · rjz/githubhook · GitHub
[go: up one dir, main page]

Skip to content

Breaks up public API (#6) #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Breaks up public API (#6)
Separating `Parse` into separate `New` and `SignedBy` methods will allow
consumers to provisionally extract hook content from an HTTP request
(via `New`) before verifyingt that the hook signature meets
expectations.
  • Loading branch information
rjz committed Aug 5, 2016
commit 424280528717a50ef3a8866f9d7b62aaf885291f
43 changes: 21 additions & 22 deletions githubhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,37 +10,38 @@ import (
"strings"
)

// Hook describes an inbound github webhook
type Hook struct {
Signature string
Event string
Id string
Payload []byte
}

const signaturePrefix = "sha1="
const signatureLength = 45 // len(SignaturePrefix) + len(hex(sha1))

func signBody(secret, body []byte) []byte {
computed := hmac.New(sha1.New, secret)
computed.Write(body)
return []byte(computed.Sum(nil))
}

func verifySignature(secret []byte, signature string, body []byte) bool {

const signaturePrefix = "sha1="
const signatureLength = 45 // len(SignaturePrefix) + len(hex(sha1))

if len(signature) != signatureLength || !strings.HasPrefix(signature, signaturePrefix) {
// SignedBy checks that the provided secret matches the hook Signature
func (h *Hook) SignedBy(secret []byte) bool {
if len(h.Signature) != signatureLength || !strings.HasPrefix(h.Signature, signaturePrefix) {
return false
}

actual := make([]byte, 20)
hex.Decode(actual, []byte(signature[5:]))
hex.Decode(actual, []byte(h.Signature[5:]))

return hmac.Equal(signBody(secret, body), actual)
return hmac.Equal(signBody(secret, h.Payload), actual)
}

func Parse(secret []byte, req *http.Request) (*Hook, error) {
hook := Hook{}

// New extracts a Hook from an incoming http.Request
func New(req *http.Request) (hook *Hook, err error) {
hook = new(Hook)
if !strings.EqualFold(req.Method, "POST") {
return nil, errors.New("Unknown method!")
}
Expand All @@ -57,17 +58,15 @@ func Parse(secret []byte, req *http.Request) (*Hook, error) {
return nil, errors.New("No event Id!")
}

body, err := ioutil.ReadAll(req.Body)

if err != nil {
return nil, err
}
hook.Payload, err = ioutil.ReadAll(req.Body)
return
}

if !verifySignature(secret, hook.Signature, body) {
return nil, errors.New("Invalid signature")
// Parse extracts and verifies a hook against a secret
func Parse(secret []byte, req *http.Request) (hook *Hook, err error) {
hook, err = New(req)
if err == nil && !hook.SignedBy(secret) {
err = errors.New("Invalid signature")
}

hook.Payload = body

return &hook, nil
return
}
13 changes: 9 additions & 4 deletions githubhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ func expectErrorMessage(t *testing.T, msg string, err error) {
}
}

func expectNewError(t *testing.T, msg string, r *http.Request) {
_, err := New(r)
expectErrorMessage(t, msg, err)
}

func expectParseError(t *testing.T, msg string, r *http.Request) {
_, err := Parse([]byte(testSecret), r)
expectErrorMessage(t, msg, err)
Expand All @@ -33,25 +38,25 @@ func signature(body string) string {

func TestNonPost(t *testing.T) {
r, _ := http.NewRequest("GET", "/path", nil)
expectParseError(t, "Unknown method!", r)
expectNewError(t, "Unknown method!", r)
}

func TestMissingSignature(t *testing.T) {
r, _ := http.NewRequest("POST", "/path", nil)
expectParseError(t, "No signature!", r)
expectNewError(t, "No signature!", r)
}

func TestMissingEvent(t *testing.T) {
r, _ := http.NewRequest("POST", "/path", nil)
r.Header.Add("x-hub-signature", "bogus signature")
expectParseError(t, "No event!", r)
expectNewError(t, "No event!", r)
}

func TestMissingEventId(t *testing.T) {
r, _ := http.NewRequest("POST", "/path", nil)
r.Header.Add("x-hub-signature", "bogus signature")
r.Header.Add("x-github-event", "bogus event")
expectParseError(t, "No event Id!", r)
expectNewError(t, "No event Id!", r)
}

func TestInvalidSignature(t *testing.T) {
Expand Down
0