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

Hook fixes #123

Merged
merged 3 commits into from Nov 3, 2017
Jump to file or symbol
Failed to load files and symbols.
+32 −14
Diff settings

Always

Just for now

Next

Use SiteURL as base if webhook is relative

  • Loading branch information...
biilmann committed Nov 2, 2017
commit 304eb701835980d6557b1b9aadc616c5bd06fef8
View
@@ -145,7 +145,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
}
if config.Webhook.HasEvent("signup") {
if err := triggerHook(SignupEvent, user, instanceID, config.Webhook.Secret, &config.Webhook); err != nil {
if err := triggerHook(SignupEvent, user, instanceID, config); err != nil {
return err
}
a.db.UpdateUser(user)
@@ -199,7 +199,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
}
if config.Webhook.HasEvent("signup") {
if err := triggerHook(SignupEvent, user, instanceID, config.Webhook.Secret, &config.Webhook); err != nil {
if err := triggerHook(SignupEvent, user, instanceID, config); err != nil {
return err
}
a.db.UpdateUser(user)
@@ -209,7 +209,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
user.Confirm()
} else {
if config.Webhook.HasEvent("login") {
if err := triggerHook(LoginEvent, user, instanceID, config.Webhook.Secret, &config.Webhook); err != nil {
if err := triggerHook(LoginEvent, user, instanceID, config); err != nil {
return err
}
a.db.UpdateUser(user)
View
@@ -34,11 +34,13 @@ func TestSignupHookSendInstanceID(t *testing.T) {
}))
defer svr.Close()
config := &conf.WebhookConfig{
URL: svr.URL,
config := &conf.Configuration{
Webhook: conf.WebhookConfig{
URL: svr.URL,
},
}
require.NoError(t, triggerHook(SignupEvent, user, "myinstance", "", config))
require.NoError(t, triggerHook(SignupEvent, user, "myinstance", config))
assert.Equal(t, 1, callCount)
}
View
@@ -7,6 +7,7 @@ import (
"net"
"net/http"
"net/http/httptrace"
"net/url"
"time"
jwt "github.com/dgrijalva/jwt-go"
@@ -140,10 +141,24 @@ func closeBody(rsp *http.Response) {
}
}
func triggerHook(event HookEvent, user *models.User, instanceID, jwtSecret string, hconfig *conf.WebhookConfig) error {
if hconfig.URL == "" {
func triggerHook(event HookEvent, user *models.User, instanceID string, config *conf.Configuration) error {
if config.Webhook.URL == "" {
return nil
}
hookURL, err := url.Parse(config.Webhook.URL)
if err != nil {
return err

This comment has been minimized.

@rybit

rybit Nov 2, 2017

Member

Might be worth using an errors.Wrapf so we can distinguish the two URL parses apart

}
if !hookURL.IsAbs() {
siteURL, err := url.Parse(config.SiteURL)
if err != nil {
return err
}
hookURL.Scheme = siteURL.Scheme
hookURL.Host = siteURL.Host
hookURL.User = siteURL.User
}
payload := struct {
Event HookEvent `json:"event"`
@@ -159,8 +174,8 @@ func triggerHook(event HookEvent, user *models.User, instanceID, jwtSecret strin
return internalServerError("Failed to serialize the data for signup webhook").WithInternalError(err)
}
w := Webhook{
WebhookConfig: hconfig,
jwtSecret: jwtSecret,
WebhookConfig: &config.Webhook,
jwtSecret: config.Webhook.Secret,
instanceID: instanceID,
claims: &jwt.StandardClaims{
IssuedAt: time.Now().Unix(),
@@ -169,6 +184,7 @@ func triggerHook(event HookEvent, user *models.User, instanceID, jwtSecret strin
},
payload: data,
}
w.URL = hookURL.String()
body, err := w.trigger()
defer func() {
View
@@ -100,7 +100,7 @@ func (a *API) signupNewUser(ctx context.Context, params *SignupParams, aud strin
user.SetRole(config.JWT.DefaultGroupName)
if config.Webhook.HasEvent("validate") {
if err := triggerHook(ValidateEvent, user, instanceID, config.Webhook.Secret, &config.Webhook); err != nil {
if err := triggerHook(ValidateEvent, user, instanceID, config); err != nil {
return nil, err
}
}
View
@@ -76,7 +76,7 @@ func (a *API) ResourceOwnerPasswordGrant(ctx context.Context, w http.ResponseWri
}
if config.Webhook.HasEvent("login") {
if err := triggerHook(LoginEvent, user, instanceID, config.Webhook.Secret, &config.Webhook); err != nil {
if err := triggerHook(LoginEvent, user, instanceID, config); err != nil {
return err
}
a.db.UpdateUser(user)
View
@@ -87,7 +87,7 @@ func (a *API) signupVerify(ctx context.Context, params *VerifyParams) (*models.U
}
if config.Webhook.HasEvent("signup") {
if err := triggerHook(SignupEvent, user, instanceID, config.Webhook.Secret, &config.Webhook); err != nil {
if err := triggerHook(SignupEvent, user, instanceID, config); err != nil {
return nil, err
}
a.db.UpdateUser(user)
@@ -111,7 +111,7 @@ func (a *API) recoverVerify(ctx context.Context, params *VerifyParams) (*models.
user.Recover()
if !user.IsConfirmed() {
if config.Webhook.HasEvent("signup") {
if err := triggerHook(SignupEvent, user, instanceID, config.Webhook.Secret, &config.Webhook); err != nil {
if err := triggerHook(SignupEvent, user, instanceID, config); err != nil {
return nil, err
}
a.db.UpdateUser(user)
ProTip! Use n and p to navigate between commits in a pull request.