mirror of
https://github.com/zitadel/zitadel.git
synced 2025-08-11 20:47:32 +00:00
fix: cancel notifications on missing channels and configurable (twilio) error codes (#9185)
# Which Problems Are Solved If a notification channel was not present, notification workers would retry to the max attempts. This leads to unnecessary load. Additionally, a client noticed bad actors trying to abuse SMS MFA. # How the Problems Are Solved - Directly cancel the notification on: - a missing channel and stop retries. - any `4xx` errors from Twilio Verify # Additional Changes None # Additional Context reported by customer
This commit is contained in:
@@ -9,11 +9,16 @@ import (
|
|||||||
verify "github.com/twilio/twilio-go/rest/verify/v2"
|
verify "github.com/twilio/twilio-go/rest/verify/v2"
|
||||||
"github.com/zitadel/logging"
|
"github.com/zitadel/logging"
|
||||||
|
|
||||||
|
"github.com/zitadel/zitadel/internal/eventstore"
|
||||||
"github.com/zitadel/zitadel/internal/notification/channels"
|
"github.com/zitadel/zitadel/internal/notification/channels"
|
||||||
"github.com/zitadel/zitadel/internal/notification/messages"
|
"github.com/zitadel/zitadel/internal/notification/messages"
|
||||||
"github.com/zitadel/zitadel/internal/zerrors"
|
"github.com/zitadel/zitadel/internal/zerrors"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
aggregateTypeNotification = "notification"
|
||||||
|
)
|
||||||
|
|
||||||
func InitChannel(config Config) channels.NotificationChannel {
|
func InitChannel(config Config) channels.NotificationChannel {
|
||||||
client := twilio.NewRestClientWithParams(twilio.ClientParams{Username: config.SID, Password: config.Token})
|
client := twilio.NewRestClientWithParams(twilio.ClientParams{Username: config.SID, Password: config.Token})
|
||||||
logging.Debug("successfully initialized twilio sms channel")
|
logging.Debug("successfully initialized twilio sms channel")
|
||||||
@@ -30,13 +35,19 @@ func InitChannel(config Config) channels.NotificationChannel {
|
|||||||
|
|
||||||
resp, err := client.VerifyV2.CreateVerification(config.VerifyServiceSID, params)
|
resp, err := client.VerifyV2.CreateVerification(config.VerifyServiceSID, params)
|
||||||
|
|
||||||
|
// In case of any client error (4xx), we should not retry sending the verification code
|
||||||
|
// as it would be a waste of resources and could potentially result in a rate limit.
|
||||||
var twilioErr *twilioClient.TwilioRestError
|
var twilioErr *twilioClient.TwilioRestError
|
||||||
if errors.As(err, &twilioErr) && twilioErr.Code == 60203 {
|
if errors.As(err, &twilioErr) && twilioErr.Status >= 400 && twilioErr.Status < 500 {
|
||||||
// If there were too many attempts to send a verification code (more than 5 times)
|
userID, notificationID := userAndNotificationIDsFromEvent(twilioMsg.TriggeringEvent)
|
||||||
// without a verification check, even retries with backoff might not solve the problem.
|
logging.WithFields(
|
||||||
// Instead, let the user initiate the verification again (e.g. using "resend code")
|
"error", twilioErr.Message,
|
||||||
// https://www.twilio.com/docs/api/errors/60203
|
"status", twilioErr.Status,
|
||||||
logging.WithFields("error", twilioErr.Message, "code", twilioErr.Code).Warn("twilio create verification error")
|
"code", twilioErr.Code,
|
||||||
|
"instanceID", twilioMsg.TriggeringEvent.Aggregate().InstanceID,
|
||||||
|
"userID", userID,
|
||||||
|
"notificationID", notificationID).
|
||||||
|
Warn("twilio create verification error")
|
||||||
return channels.NewCancelError(twilioErr)
|
return channels.NewCancelError(twilioErr)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -65,3 +76,24 @@ func InitChannel(config Config) channels.NotificationChannel {
|
|||||||
return nil
|
return nil
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func userAndNotificationIDsFromEvent(event eventstore.Event) (userID, notificationID string) {
|
||||||
|
aggID := event.Aggregate().ID
|
||||||
|
|
||||||
|
// we cannot cast to the actual event type because of circular dependencies
|
||||||
|
// so we just check the type...
|
||||||
|
if event.Aggregate().Type != aggregateTypeNotification {
|
||||||
|
// in case it's not a notification event, we can directly return the aggregate ID (as it's a user event)
|
||||||
|
return aggID, ""
|
||||||
|
}
|
||||||
|
// ...and unmarshal the event data from the notification event into a struct that contains the fields we need
|
||||||
|
var data struct {
|
||||||
|
Request struct {
|
||||||
|
UserID string `json:"userID"`
|
||||||
|
} `json:"request"`
|
||||||
|
}
|
||||||
|
if err := event.Unmarshal(&data); err != nil {
|
||||||
|
return "", aggID
|
||||||
|
}
|
||||||
|
return data.Request.UserID, aggID
|
||||||
|
}
|
||||||
|
@@ -8,6 +8,7 @@ import (
|
|||||||
"github.com/zitadel/logging"
|
"github.com/zitadel/logging"
|
||||||
|
|
||||||
"github.com/zitadel/zitadel/internal/eventstore"
|
"github.com/zitadel/zitadel/internal/eventstore"
|
||||||
|
zchannels "github.com/zitadel/zitadel/internal/notification/channels"
|
||||||
"github.com/zitadel/zitadel/internal/notification/messages"
|
"github.com/zitadel/zitadel/internal/notification/messages"
|
||||||
"github.com/zitadel/zitadel/internal/notification/templates"
|
"github.com/zitadel/zitadel/internal/notification/templates"
|
||||||
"github.com/zitadel/zitadel/internal/query"
|
"github.com/zitadel/zitadel/internal/query"
|
||||||
@@ -27,7 +28,9 @@ func generateEmail(
|
|||||||
emailChannels, config, err := channels.Email(ctx)
|
emailChannels, config, err := channels.Email(ctx)
|
||||||
logging.OnError(err).Error("could not create email channel")
|
logging.OnError(err).Error("could not create email channel")
|
||||||
if emailChannels == nil || emailChannels.Len() == 0 {
|
if emailChannels == nil || emailChannels.Len() == 0 {
|
||||||
return zerrors.ThrowPreconditionFailed(nil, "PHONE-w8nfow", "Errors.Notification.Channels.NotPresent")
|
return zchannels.NewCancelError(
|
||||||
|
zerrors.ThrowPreconditionFailed(nil, "MAIL-w8nfow", "Errors.Notification.Channels.NotPresent"),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
recipient := user.VerifiedEmail
|
recipient := user.VerifiedEmail
|
||||||
if lastEmail {
|
if lastEmail {
|
||||||
@@ -67,7 +70,9 @@ func generateEmail(
|
|||||||
}
|
}
|
||||||
return webhookChannels.HandleMessage(message)
|
return webhookChannels.HandleMessage(message)
|
||||||
}
|
}
|
||||||
return zerrors.ThrowPreconditionFailed(nil, "MAIL-83nof", "Errors.Notification.Channels.NotPresent")
|
return zchannels.NewCancelError(
|
||||||
|
zerrors.ThrowPreconditionFailed(nil, "MAIL-83nof", "Errors.Notification.Channels.NotPresent"),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
func mapNotifyUserToArgs(user *query.NotifyUser, args map[string]interface{}) map[string]interface{} {
|
func mapNotifyUserToArgs(user *query.NotifyUser, args map[string]interface{}) map[string]interface{} {
|
||||||
|
@@ -7,6 +7,7 @@ import (
|
|||||||
"github.com/zitadel/logging"
|
"github.com/zitadel/logging"
|
||||||
|
|
||||||
"github.com/zitadel/zitadel/internal/eventstore"
|
"github.com/zitadel/zitadel/internal/eventstore"
|
||||||
|
zchannels "github.com/zitadel/zitadel/internal/notification/channels"
|
||||||
"github.com/zitadel/zitadel/internal/notification/messages"
|
"github.com/zitadel/zitadel/internal/notification/messages"
|
||||||
"github.com/zitadel/zitadel/internal/notification/senders"
|
"github.com/zitadel/zitadel/internal/notification/senders"
|
||||||
"github.com/zitadel/zitadel/internal/notification/templates"
|
"github.com/zitadel/zitadel/internal/notification/templates"
|
||||||
@@ -33,7 +34,9 @@ func generateSms(
|
|||||||
smsChannels, config, err := channels.SMS(ctx)
|
smsChannels, config, err := channels.SMS(ctx)
|
||||||
logging.OnError(err).Error("could not create sms channel")
|
logging.OnError(err).Error("could not create sms channel")
|
||||||
if smsChannels == nil || smsChannels.Len() == 0 {
|
if smsChannels == nil || smsChannels.Len() == 0 {
|
||||||
return zerrors.ThrowPreconditionFailed(nil, "PHONE-w8nfow", "Errors.Notification.Channels.NotPresent")
|
return zchannels.NewCancelError(
|
||||||
|
zerrors.ThrowPreconditionFailed(nil, "PHONE-w8nfow", "Errors.Notification.Channels.NotPresent"),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
recipient := user.VerifiedPhone
|
recipient := user.VerifiedPhone
|
||||||
if lastPhone {
|
if lastPhone {
|
||||||
@@ -85,5 +88,7 @@ func generateSms(
|
|||||||
}
|
}
|
||||||
return webhookChannels.HandleMessage(message)
|
return webhookChannels.HandleMessage(message)
|
||||||
}
|
}
|
||||||
return zerrors.ThrowPreconditionFailed(nil, "PHONE-w8nfow", "Errors.Notification.Channels.NotPresent")
|
return zchannels.NewCancelError(
|
||||||
|
zerrors.ThrowPreconditionFailed(nil, "PHONE-83nof", "Errors.Notification.Channels.NotPresent"),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user