From 8673addaf04d564fd0eea3ab1dca7980652558ab Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Mon, 29 May 2023 18:19:01 +0200 Subject: [PATCH] OAuth2: remove variants Microsoft also accept the same protocol used for Google servers --- auth.go | 3 +- client.go | 32 +++------------ client_test.go | 52 ----------------------- smtp/auth_xoauth2.go | 57 +++----------------------- smtp/smtp_test.go | 98 +++++++++++++++++++++++++++++++------------- 5 files changed, 83 insertions(+), 159 deletions(-) diff --git a/auth.go b/auth.go index 66754a3..e7477a2 100644 --- a/auth.go +++ b/auth.go @@ -20,7 +20,8 @@ const ( // SMTPAuthCramMD5 is the "CRAM-MD5" SASL authentication mechanism as described in RFC 4954 SMTPAuthCramMD5 SMTPAuthType = "CRAM-MD5" - // SMTPAuthXOAUTH2 is the "XOAUTH2" SASL authentication mechanism + // SMTPAuthXOAUTH2 is the "XOAUTH2" SASL authentication mechanism. + // https://developers.google.com/gmail/imap/xoauth2-protocol SMTPAuthXOAUTH2 SMTPAuthType = "XOAUTH2" ) diff --git a/client.go b/client.go index 0c3225c..6b9baf1 100644 --- a/client.go +++ b/client.go @@ -32,9 +32,6 @@ const ( // DefaultTLSMinVersion is the minimum TLS version required for the connection // Nowadays TLS1.2 should be the sane default DefaultTLSMinVersion = tls.VersionTLS12 - - // DefaultXOAuth2Variant is the default XOAuth2 variant - DefaultXOAuth2Variant = smtp.XOAuth2VariantGoogle ) // DSNMailReturnOption is a type to define which MAIL RET option is used when a DSN @@ -138,9 +135,6 @@ type Client struct { // user is the SMTP AUTH username user string - // xoauthVariant sets the client to use the provided XOAuth2Variant variant - xoauthVariant smtp.XOAuth2Variant - // dl enables the debug logging on the SMTP client dl bool @@ -199,12 +193,11 @@ var ( // NewClient returns a new Session client object func NewClient(h string, o ...Option) (*Client, error) { c := &Client{ - cto: DefaultTimeout, - host: h, - port: DefaultPort, - tlsconfig: &tls.Config{ServerName: h, MinVersion: DefaultTLSMinVersion}, - tlspolicy: DefaultTLSPolicy, - xoauthVariant: DefaultXOAuth2Variant, + cto: DefaultTimeout, + host: h, + port: DefaultPort, + tlsconfig: &tls.Config{ServerName: h, MinVersion: DefaultTLSMinVersion}, + tlspolicy: DefaultTLSPolicy, } // Set default HELO/EHLO hostname @@ -422,14 +415,6 @@ func WithDialContextFunc(f DialContextFunc) Option { } } -// WithXOAuth2Variant tells the client to use the provided XOAuth2 variant -func WithXOAuth2Variant(v smtp.XOAuth2Variant) Option { - return func(c *Client) error { - c.xoauthVariant = v - return nil - } -} - // TLSPolicy returns the currently set TLSPolicy as string func (c *Client) TLSPolicy() string { return c.tlspolicy.String() @@ -445,11 +430,6 @@ func (c *Client) SetTLSPolicy(p TLSPolicy) { c.tlspolicy = p } -// SetXOAuth2Variant overrides the current XOAuth2Variant with the given XOAuth2Variant value -func (c *Client) SetXOAuth2Variant(v smtp.XOAuth2Variant) { - c.xoauthVariant = v -} - // SetSSL tells the Client wether to use SSL or not func (c *Client) SetSSL(s bool) { c.ssl = s @@ -682,7 +662,7 @@ func (c *Client) auth() error { if !strings.Contains(sat, string(SMTPAuthXOAUTH2)) { return ErrXOauth2AuthNotSupported } - c.sa = smtp.XOAuth2Auth(c.user, c.pass, c.xoauthVariant) + c.sa = smtp.XOAuth2Auth(c.user, c.pass) default: return fmt.Errorf("unsupported SMTP AUTH type %q", c.satype) } diff --git a/client_test.go b/client_test.go index 846924b..8bad1e0 100644 --- a/client_test.go +++ b/client_test.go @@ -101,7 +101,6 @@ func TestNewClientWithOptions(t *testing.T) { }, {"WithUsername()", WithUsername("test"), false}, {"WithPassword()", WithPassword("test"), false}, - {"WithXOAuth2Variant()", WithXOAuth2Variant(smtp.XOAuth2VariantMicrosoft), false}, {"WithDSN()", WithDSN(), false}, {"WithDSNMailReturnType()", WithDSNMailReturnType(DSNMailReturnFull), false}, {"WithDSNMailReturnType() wrong option", WithDSNMailReturnType("FAIL"), true}, @@ -391,57 +390,6 @@ func TestSetSMTPAuth(t *testing.T) { } } -// TestWithXOAuth2Variant tests the WithXOAuth2Variant() option for the NewClient() method -func TestWithXOAuth2Variant(t *testing.T) { - tests := []struct { - name string - value smtp.XOAuth2Variant - want string - sf bool - }{ - {"Variant: Google", smtp.XOAuth2VariantGoogle, smtp.XOAuth2VariantGoogle.String(), false}, - {"Variant: Microsoft", smtp.XOAuth2VariantMicrosoft, smtp.XOAuth2VariantMicrosoft.String(), false}, - {"Variant: Invalid", -1, "Unknown XOAuth2 variant", true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c, err := NewClient(DefaultHost, WithXOAuth2Variant(tt.value)) - if err != nil && !tt.sf { - t.Errorf("failed to create new client: %s", err) - return - } - if c.xoauthVariant.String() != tt.want { - t.Errorf("failed to set XOAuth2 variant. Want: %s, got: %s", tt.want, c.xoauthVariant) - } - }) - } -} - -// TestSetXOAuth2Variant tests the SetXOAuth2Variant method for the Client object -func TestSetXOAuth2Variant(t *testing.T) { - tests := []struct { - name string - value smtp.XOAuth2Variant - want smtp.XOAuth2Variant - }{ - {"Google", smtp.XOAuth2VariantGoogle, smtp.XOAuth2VariantGoogle}, - {"Google", smtp.XOAuth2VariantMicrosoft, smtp.XOAuth2VariantMicrosoft}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c, err := NewClient(DefaultHost) - if err != nil { - t.Errorf("failed to create new client: %s", err) - return - } - c.SetXOAuth2Variant(tt.value) - if c.xoauthVariant != tt.want { - t.Errorf("failed to set XOAuthVariant. Expected %s, got: %s", tt.want, c.xoauthVariant) - } - }) - } -} - // TestWithDSN tests the WithDSN method for the Client object func TestWithDSN(t *testing.T) { c, err := NewClient(DefaultHost, WithDSN()) diff --git a/smtp/auth_xoauth2.go b/smtp/auth_xoauth2.go index f0c12bd..dade048 100644 --- a/smtp/auth_xoauth2.go +++ b/smtp/auth_xoauth2.go @@ -1,71 +1,24 @@ -// SPDX-FileCopyrightText: Copyright (c) 2022-2023 The go-mail Authors +// SPDX-FileCopyrightText: Copyright (c) 2023 The go-mail Authors // // SPDX-License-Identifier: MIT package smtp -import "fmt" - -// XOAuth2Variant describes a int alias for the different XOAuth2 variants we support -type XOAuth2Variant int - -// Supported XOAuth2 variants -const ( - // XOAuth2VariantGoogle is the Google variant for "XOAUTH2" SASL authentication mechanism. - // https://developers.google.com/gmail/imap/xoauth2-protocol - XOAuth2VariantGoogle XOAuth2Variant = iota - - // XOAuth2VariantMicrosoft is the Microsoft variant for "XOAUTH2" SASL authentication mechanism. - // https://learn.microsoft.com/en-us/exchange/client-developer/legacy-protocols/how-to-authenticate-an-imap-pop-smtp-application-by-using-oauth - XOAuth2VariantMicrosoft -) - -// String is a standard method to convert a XOAuth2Variant into a printable format -func (v XOAuth2Variant) String() string { - switch v { - case XOAuth2VariantGoogle: - return "Google" - case XOAuth2VariantMicrosoft: - return "Microsoft" - default: - return "Unknown XOAuth2 variant" - } -} - type xoauth2Auth struct { username, token string - variant XOAuth2Variant } -func XOAuth2Auth(username, token string, variant XOAuth2Variant) Auth { - return &xoauth2Auth{username, token, variant} -} - -func (a *xoauth2Auth) getToken() []byte { - return []byte("user=" + a.username + "\x01" + "auth=Bearer " + a.token + "\x01\x01") +func XOAuth2Auth(username, token string) Auth { + return &xoauth2Auth{username, token} } func (a *xoauth2Auth) Start(_ *ServerInfo) (string, []byte, error) { - switch a.variant { - case XOAuth2VariantGoogle: - return "XOAUTH2", a.getToken(), nil - case XOAuth2VariantMicrosoft: - return "XOAUTH2", nil, nil - default: - return "", nil, fmt.Errorf("unsupported XOAuth2 variant %d", a.variant) - } + return "XOAUTH2", []byte("user=" + a.username + "\x01" + "auth=Bearer " + a.token + "\x01\x01"), nil } func (a *xoauth2Auth) Next(_ []byte, more bool) ([]byte, error) { if more { - switch a.variant { - case XOAuth2VariantGoogle: - return []byte(""), nil - case XOAuth2VariantMicrosoft: - return a.getToken(), nil - default: - return nil, fmt.Errorf("unsupported XOAuth2 variant %d", a.variant) - } + return []byte(""), nil } return nil, nil } diff --git a/smtp/smtp_test.go b/smtp/smtp_test.go index aaea98f..e09cdf3 100644 --- a/smtp/smtp_test.go +++ b/smtp/smtp_test.go @@ -70,19 +70,12 @@ var authTests = []authTest{ []bool{false, false}, }, { - XOAuth2Auth("username", "token", XOAuth2VariantGoogle), + XOAuth2Auth("username", "token"), []string{""}, "XOAUTH2", []string{"user=username\x01auth=Bearer token\x01\x01", ""}, []bool{false}, }, - { - XOAuth2Auth("username", "token", XOAuth2VariantMicrosoft), - []string{""}, - "XOAUTH2", - []string{"", "user=username\x01auth=Bearer token\x01\x01"}, - []bool{false}, - }, } func TestAuth(t *testing.T) { @@ -207,34 +200,83 @@ func TestAuthLogin(t *testing.T) { } } -func TestXOAuth2VariantToString(t *testing.T) { - v := XOAuth2VariantGoogle - if val := v.String(); val != "Google" { - t.Errorf("got %q; want Google", val) +func TestXOAuthOK(t *testing.T) { + server := []string{ + "220 Fake server ready ESMTP", + "250-fake.server", + "250-AUTH XOAUTH2", + "250 8BITMIME", + "235 2.7.0 Accepted", } - v = XOAuth2VariantMicrosoft - if val := v.String(); val != "Microsoft" { - t.Errorf("got %q; want Microsoft", val) + var wrote strings.Builder + var fake faker + fake.ReadWriter = struct { + io.Reader + io.Writer + }{ + strings.NewReader(strings.Join(server, "\r\n")), + &wrote, } - v = XOAuth2Variant(-1) - if val := v.String(); val != "Unknown XOAuth2 variant" { - t.Errorf("got %q; want Unknown XOAuth2 variant", val) + + c, err := NewClient(fake, "fake.host") + if err != nil { + t.Fatalf("NewClient: %v", err) + } + defer c.Close() + + auth := XOAuth2Auth("user", "token") + err = c.Auth(auth) + if err != nil { + t.Fatalf("XOAuth2 error: %v", err) + } + // the Next method returns a nil response. It must not be sent. + // The client request must end with the authentication. + if !strings.HasSuffix(wrote.String(), "AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIHRva2VuAQE=\r\n") { + t.Fatalf("got %q; want AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIHRva2VuAQE=\r\n", wrote.String()) } } -func TestXOAuth2UnsupportendVariant(t *testing.T) { - serverInfo := &ServerInfo{ - Name: "servename", - Auth: []string{"XOAUTH2"}, +func TestXOAuth2Error(t *testing.T) { + serverResp := []string{ + "220 Fake server ready ESMTP", + "250-fake.server", + "250-AUTH XOAUTH2", + "250 8BITMIME", + "334 eyJzdGF0dXMiOiI0MDAiLCJzY2hlbWVzIjoiQmVhcmVyIiwic2NvcGUiOiJodHRwczovL21haWwuZ29vZ2xlLmNvbS8ifQ==", + "535 5.7.8 Username and Password not accepted", + "221 2.0.0 closing connection", } - auth := &xoauth2Auth{"username", "token", XOAuth2Variant(-1)} - _, _, err := auth.Start(serverInfo) - if err == nil { - t.Error("expected error for auth.Start() using an unsupported variant") + var wrote strings.Builder + var fake faker + fake.ReadWriter = struct { + io.Reader + io.Writer + }{ + strings.NewReader(strings.Join(serverResp, "\r\n")), + &wrote, } - _, err = auth.Next(nil, true) + + c, err := NewClient(fake, "fake.host") + if err != nil { + t.Fatalf("NewClient: %v", err) + } + defer c.Close() + + auth := XOAuth2Auth("user", "token") + err = c.Auth(auth) if err == nil { - t.Error("expected error for auth.Next() using an unsupported variant") + t.Fatal("expected auth error, got nil") + } + client := strings.Split(wrote.String(), "\r\n") + if len(client) != 5 { + t.Fatalf("unexpected number of client requests got %d; want 5", len(client)) + } + if client[1] != "AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIHRva2VuAQE=" { + t.Fatalf("got %q; want AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIHRva2VuAQE=", client[1]) + } + // the Next method returns an empty response. It must be sent + if client[2] != "" { + t.Fatalf("got %q; want empty response", client[2]) } }