fix: mailotp code comparison was failing (#440)

- Added option to disable STARTTLS for smtp dialer
- Added additional e2e test to validate the flow

Signed-off-by: Kush Sharma <[email protected]>

cmd/serve.go

@@ -287,6 +287,7 @@ func buildAPIDependencies(
 			cfg.App.Mailer.SMTPPassword,
 			cfg.App.Mailer.SMTPInsecure,
 			cfg.App.Mailer.Headers,
+			cfg.App.Mailer.TLSPolicy(),
 		)
 		logger.Info("mailer enabled", "host", cfg.App.Mailer.SMTPHost, "port", cfg.App.Mailer.SMTPPort)
 	}

config/sample.config.yaml

@@ -130,6 +130,12 @@ app:
     smtp_insecure: true
     headers:
       from: "[email protected]"
+    # policy to use when establishing a connection. Defaults to mandatory_start.
+    # Possible values are:
+    # opportunistic: Use STARTTLS if the server supports it, otherwise connect without encryption.
+    # mandatory: Always use STARTTLS.
+    # no: Never use STARTTLS.
+    smtp_tls_policy: "mandatory"
 db:
   driver: postgres
   url: postgres://frontier:@localhost:5432/frontier?sslmode=disable

core/authenticate/service.go

@@ -332,7 +332,7 @@ func (s Service) applyMailOTP(ctx context.Context, request RegistrationFinishReq
 		return nil, ErrFlowInvalid
 	}
 
-	if subtle.ConstantTimeCompare([]byte(flow.Nonce), []byte(request.Code)) == 1 {
+	if subtle.ConstantTimeCompare([]byte(flow.Nonce), []byte(request.Code)) == 0 {
 		// avoid brute forcing otp
 		attemptInt := 0
 		if attempts, ok := flow.Metadata[otpAttemptKey]; ok {

core/authenticate/strategy/mail_otp.go

@@ -43,24 +43,39 @@ func NewMailOTP(d mailer.Dialer, subject, body string) *MailOTP {
 // SendMail sends a mail with a one time password embedded link to user's email id
 func (m MailOTP) SendMail(to string) (string, error) {
 	otp := GenerateNonceFromLetters(otpLen, otpLetterRunes)
-	t, err := template.New("body").Parse(m.body)
+
+	tpl := template.New("body")
+	t, err := tpl.Parse(m.body)
 	if err != nil {
 		return "", fmt.Errorf("failed to parse email template: %w", err)
 	}
-	var tpl bytes.Buffer
-	err = t.Execute(&tpl, map[string]string{
+	var tplBuffer bytes.Buffer
+	if err = t.Execute(&tplBuffer, map[string]string{
 		"Otp": otp,
-	})
+	}); err != nil {
+		return "", fmt.Errorf("failed to parse email template: %w", err)
+	}
+	tplBody := tplBuffer.String()
+
+	tpl = template.New("sub")
+	t, err = tpl.Parse(m.subject)
 	if err != nil {
 		return "", fmt.Errorf("failed to parse email template: %w", err)
 	}
+	tplBuffer.Reset()
+	if err = t.Execute(&tplBuffer, map[string]string{
+		"Otp": otp,
+	}); err != nil {
+		return "", fmt.Errorf("failed to parse email template: %w", err)
+	}
+	tplSub := tplBuffer.String()
 
 	//TODO(kushsharma): apply rest of the headers
 	msg := mail.NewMessage()
 	msg.SetHeader("From", m.dialer.FromHeader())
 	msg.SetHeader("To", to)
-	msg.SetHeader("Subject", m.subject)
-	msg.SetBody("text/html", tpl.String())
+	msg.SetHeader("Subject", tplSub)
+	msg.SetBody("text/html", tplBody)
 	msg.SetDateHeader("Date", m.Now())
 	return otp, m.dialer.DialAndSend(msg)
 }

go.mod

@@ -26,6 +26,7 @@ require (
 	github.com/jmoiron/sqlx v1.3.5
 	github.com/lestrrat-go/jwx/v2 v2.0.18
 	github.com/mcuadros/go-defaults v1.2.0
+	github.com/mocktools/go-smtp-mock/v2 v2.1.0
 	github.com/newrelic/go-agent v3.20.2+incompatible
 	github.com/oauth2-proxy/mockoidc v0.0.0-20220308204021-b9169deeb282
 	github.com/ory/dockertest v3.3.5+incompatible

go.sum

@@ -1794,6 +1794,8 @@ github.com/moby/term v0.0.0-20210610120745-9d4ed1856297/go.mod h1:vgPCkQMyxTZ7ID
 github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6/go.mod h1:E2VnQOmVuvZB6UYnnDB0qG5Nq/1tD9acaOpo6xmt0Kw=
 github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0=
 github.com/moby/term v0.5.0/go.mod h1:8FzsFHVUBGZdbDsJw/ot+X+d5HLUbvklYLJ9uGfcI3Y=
+github.com/mocktools/go-smtp-mock/v2 v2.1.0 h1:gGiWqlaMTExk7Id38G2+sWfOelsE+OAqJWAMsAI/654=
+github.com/mocktools/go-smtp-mock/v2 v2.1.0/go.mod h1:n8aNpDYncZHH/cZHtJKzQyeYT/Dut00RghVM+J1Ed94=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=

pkg/mailer/dialer.go

@@ -31,13 +31,13 @@ type DialerImpl struct {
 }
 
 func NewDialerImpl(SMTPHost string, SMTPPort int, SMTPUser string, SMTPPass string,
-	SMTPInsecure bool, headers map[string]string) *DialerImpl {
+	SMTPInsecure bool, headers map[string]string, tlsPolicy mail.StartTLSPolicy) *DialerImpl {
 	d := mail.NewDialer(SMTPHost, SMTPPort, SMTPUser, SMTPPass)
 	d.TLSConfig = &tls.Config{
 		InsecureSkipVerify: SMTPInsecure,
 		ServerName:         SMTPHost,
 	}
-	d.StartTLSPolicy = mail.MandatoryStartTLS
+	d.StartTLSPolicy = tlsPolicy
 	return &DialerImpl{
 		dialer:  d,
 		headers: headers,

pkg/mailer/mailer.go

@@ -1,10 +1,36 @@
 package mailer
 
+import (
+	"strings"
+
+	"gopkg.in/mail.v2"
+)
+
 type Config struct {
 	SMTPHost     string            `yaml:"smtp_host" mapstructure:"smtp_host"`
 	SMTPPort     int               `yaml:"smtp_port" mapstructure:"smtp_port"`
 	SMTPUsername string            `yaml:"smtp_username" mapstructure:"smtp_username"`
 	SMTPPassword string            `yaml:"smtp_password" mapstructure:"smtp_password"`
 	SMTPInsecure bool              `yaml:"smtp_insecure" mapstructure:"smtp_insecure" default:"true"`
 	Headers      map[string]string `yaml:"headers" mapstructure:"headers"`
+
+	// SMTP TLS policy to use when establishing a connection.
+	// Defaults to MandatoryStartTLS.
+	// Possible values are:
+	// opportunistic: Use STARTTLS if the server supports it, otherwise connect without encryption.
+	// mandatory: Always use STARTTLS.
+	// none: Never use STARTTLS.
+	SMTPTLSPolicy string `yaml:"smtp_tls_policy" mapstructure:"smtp_tls_policy" default:"mandatory"`
+}
+
+func (c Config) TLSPolicy() mail.StartTLSPolicy {
+	switch strings.ToLower(c.SMTPTLSPolicy) {
+	case "opportunistic":
+		return mail.OpportunisticStartTLS
+	case "mandatory":
+		return mail.MandatoryStartTLS
+	case "none":
+		return mail.NoStartTLS
+	}
+	return mail.MandatoryStartTLS
 }

sdks/js/packages/core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@raystack/frontier",
-  "version": "0.8.10",
+  "version": "0.8.11",
   "description": "A js library for frontier",
   "sideEffects": false,
   "main": "./dist/index.js",

test/e2e/regression/authentication_test.go

@@ -7,16 +7,23 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"strings"
 	"testing"
 	"time"
 
+	"github.com/raystack/frontier/core/authenticate/strategy"
+	"github.com/raystack/frontier/pkg/mailer"
+	"github.com/raystack/frontier/pkg/server"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/metadata"
+
 	"github.com/golang/protobuf/jsonpb"
 
+	smtpmock "github.com/mocktools/go-smtp-mock/v2"
 	"github.com/oauth2-proxy/mockoidc"
 	"github.com/raystack/frontier/config"
 	"github.com/raystack/frontier/core/authenticate"
 	"github.com/raystack/frontier/pkg/logger"
-	"github.com/raystack/frontier/pkg/server"
 	frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1"
 	"github.com/raystack/frontier/test/e2e/testbench"
 	"github.com/stretchr/testify/suite"
@@ -28,6 +35,8 @@ type AuthenticationRegressionTestSuite struct {
 	apiPort        int
 	mockOIDCServer *mockoidc.MockOIDC
 	callbackPort   int
+
+	smtpServer *smtpmock.Server
 }
 
 func (s *AuthenticationRegressionTestSuite) SetupSuite() {
@@ -47,6 +56,19 @@ func (s *AuthenticationRegressionTestSuite) SetupSuite() {
 	s.mockOIDCServer, err = mockoidc.Run()
 	s.Require().NoError(err)
 
+	s.smtpServer = smtpmock.New(smtpmock.ConfigurationAttr{
+		LogToStdout:       false,
+		LogServerActivity: false,
+	})
+
+	// To start smtpServer use Start() method
+	if err := s.smtpServer.Start(); err != nil {
+		s.Assert().NoError(err)
+	}
+	// Server's port will be assigned dynamically after smtpServer.Start()
+	// for case when portNumber wasn't specified
+	smtpHostAddress, smtpPortNumber := "127.0.0.1", s.smtpServer.PortNumber()
+
 	// mock callback host
 
 	appConfig := &config.Frontier{
@@ -83,6 +105,16 @@ func (s *AuthenticationRegressionTestSuite) SetupSuite() {
 					},
 				},
 				CallbackURLs: []string{fmt.Sprintf("http://localhost:%d/callback", s.callbackPort)},
+				MailOTP: authenticate.MailOTPConfig{
+					Subject: "{{.Otp}}",
+					Body:    "{{.Otp}}",
+				},
+			},
+			Mailer: mailer.Config{
+				SMTPHost:      smtpHostAddress,
+				SMTPPort:      smtpPortNumber,
+				SMTPInsecure:  true,
+				SMTPTLSPolicy: "none",
 			},
 		},
 	}
@@ -96,6 +128,8 @@ func (s *AuthenticationRegressionTestSuite) TearDownSuite() {
 	s.Require().NoError(err)
 	err = s.mockOIDCServer.Shutdown()
 	s.Require().NoError(err)
+	err = s.smtpServer.Stop()
+	s.Require().NoError(err)
 }
 
 func (s *AuthenticationRegressionTestSuite) TestUserSession() {
@@ -167,6 +201,53 @@ func (s *AuthenticationRegressionTestSuite) TestUserSession() {
 		s.Assert().NoError(jsonpb.Unmarshal(userResp.Body, user))
 		s.Assert().Equal(mockoidc.DefaultUser().Email, user.GetUser().Email)
 	})
+	s.Run("3. authenticate a user successfully using mailotp", func() {
+		// start registration flow
+		authResp, err := s.testBench.Client.Authenticate(ctx, &frontierv1beta1.AuthenticateRequest{
+			StrategyName:    strategy.MailOTPAuthMethod,
+			RedirectOnstart: false,
+			ReturnTo:        "",
+			Email:           mockoidc.DefaultUser().Email,
+		})
+		s.Assert().NoError(err)
+		s.Assert().NotNil(authResp.State)
+
+		// check if mail is sent
+		messages := s.smtpServer.Messages()
+		s.Assert().NotEmpty(messages)
+		mailMsg := messages[0].MsgRequest()
+		// extract mail headers
+		mailParts := strings.Split(mailMsg, "\r\n")
+		emailOTP := ""
+		for _, part := range mailParts {
+			if strings.HasPrefix(part, "Subject: ") {
+				emailOTP = strings.TrimPrefix(part, "Subject: ")
+			}
+		}
+		s.Assert().NotEmpty(emailOTP)
+
+		// verify incorrect otp
+		// extract grpc headers
+		md := metadata.MD{}
+		_, err = s.testBench.Client.AuthCallback(ctx, &frontierv1beta1.AuthCallbackRequest{
+			StrategyName: strategy.MailOTPAuthMethod,
+			Code:         "123456",
+			State:        authResp.State,
+		}, grpc.Header(&md))
+		s.Assert().Error(err)
+		s.Assert().Empty(md["gateway-session-id"])
+
+		// verify correct otp
+		// extract grpc headers
+		md = metadata.MD{}
+		_, err = s.testBench.Client.AuthCallback(ctx, &frontierv1beta1.AuthCallbackRequest{
+			StrategyName: strategy.MailOTPAuthMethod,
+			Code:         emailOTP,
+			State:        authResp.State,
+		}, grpc.Header(&md))
+		s.Assert().NoError(err)
+		s.Assert().NotEmpty(md["gateway-session-id"])
+	})
 }
 
 func TestEndToEndAuthenticationRegressionTestSuite(t *testing.T) {