public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] send-email: add oauth2 support and fix outlook breaking threads
@ 2025-04-22 15:23 Aditya Garg
  2025-04-22 15:23 ` [PATCH v3 1/3] send-email: implement SMTP bearer authentication Aditya Garg
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Aditya Garg @ 2025-04-22 15:23 UTC (permalink / raw)
  To: Julian Swagemakers, git, Junio C Hamano; +Cc: M Hickford, sandals, Shengyu Qu

Hi all!

This patch series includes three changes:

1. It adds support for Oauth2 authentication, which is now compulsory by Microsoft.
   This patch has been rebased to the latest version from the original version
   at https://lore.kernel.org/git/20250125190131.48717-1-julian@swagemakers.org/

2. The second patch makes the script reply to the message id set by the outlook,
   since outlook has its own proprietary way to handle message ids,
   and does not allow user to set their own. As a result, threads were breaking.

3. The final patch adds a new option to generate passwords like OAuth2 tokens.
   This is useful for users who want to use a script which generates tokens for
   OAuth2 authentication.

Detailed description of each patch has been done in the respective patches

BTW, I am sending this series using the patched send-email by these patches from
Outlook!

v2: Fix errors flagged by the CI
v3: Add third patch to generate passwords like OAuth2 tokens

Aditya Garg (2):
  send-email: retrieve Message-ID from outlook SMTP server
  send-email: add option to generate passswords like OAuth2 tokens

Julian Swagemakers (1):
  send-email: implement SMTP bearer authentication

 Documentation/git-send-email.adoc | 13 ++++-
 git-send-email.perl               | 90 ++++++++++++++++++++++++++++++-
 2 files changed, 101 insertions(+), 2 deletions(-)

-- 
2.49.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/3] send-email: implement SMTP bearer authentication
  2025-04-22 15:23 [PATCH v3 0/3] send-email: add oauth2 support and fix outlook breaking threads Aditya Garg
@ 2025-04-22 15:23 ` Aditya Garg
  2025-04-22 15:23 ` [PATCH v3 2/3] send-email: retrieve Message-ID from outlook SMTP server Aditya Garg
  2025-04-22 15:23 ` [PATCH v3 3/3] send-email: add option to generate passswords like OAuth2 tokens Aditya Garg
  2 siblings, 0 replies; 10+ messages in thread
From: Aditya Garg @ 2025-04-22 15:23 UTC (permalink / raw)
  To: Julian Swagemakers, git, Junio C Hamano; +Cc: M Hickford, sandals, Shengyu Qu

From: Julian Swagemakers <julian@swagemakers.org>

Manually send SMTP AUTH command for auth type OAUTHBEARER and XOAUTH2.
This is necessary since they are currently not supported by the Perls
Authen::SASL module.

The bearer token needs to be passed in as the password. This can be done
with git-credential-oauth[0] after minor modifications[1]. Which will
allow using git send-email with Gmail and oauth2 authentication:

    [credential]
        helper = cache --timeout 7200    # two hours
        helper = oauth
    [sendemail]
        smtpEncryption = tls
        smtpServer = smtp.gmail.com
        smtpUser = example@gmail.com
        smtpServerPort = 587
        smtpauth = OAUTHBEARER

As well as Office 365 accounts:

    [credential]
        helper = cache --timeout 7200   # two hours
        helper = oauth
    [sendemail]
        smtpEncryption = tls
        smtpServer = smtp.office365.com
        smtpUser = example@example.com
        smtpServerPort = 587
        smtpauth = XOAUTH2

[0] https://github.com/hickford/git-credential-oauth
[1] https://github.com/hickford/git-credential-oauth/issues/48

Tested-by: M Hickford <mirth.hickford@gmail.com>
Signed-off-by: Julian Swagemakers <julian@swagemakers.org>
Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 Documentation/git-send-email.adoc |  5 ++-
 git-send-email.perl               | 64 ++++++++++++++++++++++++++++++-
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc
index 7f223db42d..1bf75c060d 100644
--- a/Documentation/git-send-email.adoc
+++ b/Documentation/git-send-email.adoc
@@ -213,7 +213,10 @@ SMTP server and if it is supported by the utilized SASL library, the mechanism
 is used for authentication. If neither 'sendemail.smtpAuth' nor `--smtp-auth`
 is specified, all mechanisms supported by the SASL library can be used. The
 special value 'none' maybe specified to completely disable authentication
-independently of `--smtp-user`
+independently of `--smtp-user`. Specifying `OAUTHBEARER` or `XOAUTH2` will
+bypass SASL negotiation and force bearer authentication. In this case the
+bearer token must be provided with `--smtp-pass` or using a credential helper
+and `--smtp-encryption=tls` must be set.
 
 --smtp-pass[=<password>]::
 	Password for SMTP-AUTH. The argument is optional: If no
diff --git a/git-send-email.perl b/git-send-email.perl
index 1f613fa979..a6cafda29c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1398,6 +1398,63 @@ sub smtp_host_string {
 	}
 }
 
+sub generate_oauthbearer_string {
+	# This will generate the oauthbearer string used for authentication.
+	#
+	# "n,a=" {User} ",^Ahost=" {Host} "^Aport=" {Port} "^Aauth=Bearer " {Access Token} "^A^A
+	#
+	# The first part `n,a=" {User} ",` is the gs2 header described in RFC5801.
+	# * gs2-cb-flag `n` -> client does not support CB
+	# * gs2-authzid `a=" {User} "`
+	#
+	# The second part are key value pairs containing host, port and auth as
+	# described in RFC7628.
+	#
+	# https://datatracker.ietf.org/doc/html/rfc5801
+	# https://datatracker.ietf.org/doc/html/rfc7628
+	my $username = shift;
+	my $token = shift;
+	return "n,a=$username,\001port=$smtp_server_port\001auth=Bearer $token\001\001";
+}
+
+sub generate_xoauth2_string {
+	# "user=" {User} "^Aauth=Bearer " {Access Token} "^A^A"
+	# https://developers.google.com/gmail/imap/xoauth2-protocol#initial_client_response
+	my $username = shift;
+	my $token = shift;
+	return "user=$username\001auth=Bearer $token\001\001";
+}
+
+sub smtp_bearer_auth {
+	my $username = shift;
+	my $token = shift;
+	my $auth_string;
+	if ($smtp_encryption ne "tls") {
+		# As described in RFC7628 TLS is required and will be enforced
+		# at this point.
+		#
+		# https://datatracker.ietf.org/doc/html/rfc7628#section-3
+		die sprintf(__("For %s TLS is required."), $smtp_auth);
+	}
+	if ($smtp_auth eq "OAUTHBEARER") {
+		$auth_string = generate_oauthbearer_string($username, $token);
+	} elsif ($smtp_auth eq "XOAUTH2") {
+		$auth_string = generate_xoauth2_string($username, $token);
+	}
+	my $encoded_auth_string = MIME::Base64::encode($auth_string, "");
+	$smtp->command("AUTH $smtp_auth $encoded_auth_string\r\n");
+	use Net::Cmd qw(CMD_OK);
+	if ($smtp->response() == CMD_OK){
+		return 1;
+	} else {
+		# Send dummy request on authentication failure according to rfc7628.
+		# https://datatracker.ietf.org/doc/html/rfc7628#section-3.2.3
+		$smtp->command(MIME::Base64::encode("\001"));
+		$smtp->response();
+		return 0;
+	}
+}
+
 # Returns 1 if authentication succeeded or was not necessary
 # (smtp_user was not specified), and 0 otherwise.
 
@@ -1436,7 +1493,12 @@ sub smtp_auth_maybe {
 
 		# catch all SMTP auth error in a unified eval block
 		eval {
-			if ($smtp_auth) {
+			if (defined $smtp_auth && ($smtp_auth eq "OAUTHBEARER" || $smtp_auth eq "XOAUTH2")) {
+				# Since Authen:SASL does not support XOAUTH2 nor OAUTHBEARER we will
+				# manually authenticate for these types. The password field should
+				# contain the auth token at this point.
+				$result = smtp_bearer_auth($cred->{'username'}, $cred->{'password'});
+			} elsif ($smtp_auth) {
 				my $sasl = Authen::SASL->new(
 					mechanism => $smtp_auth,
 					callback => {
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/3] send-email: retrieve Message-ID from outlook SMTP server
  2025-04-22 15:23 [PATCH v3 0/3] send-email: add oauth2 support and fix outlook breaking threads Aditya Garg
  2025-04-22 15:23 ` [PATCH v3 1/3] send-email: implement SMTP bearer authentication Aditya Garg
@ 2025-04-22 15:23 ` Aditya Garg
  2025-04-22 22:05   ` Junio C Hamano
  2025-04-22 15:23 ` [PATCH v3 3/3] send-email: add option to generate passswords like OAuth2 tokens Aditya Garg
  2 siblings, 1 reply; 10+ messages in thread
From: Aditya Garg @ 2025-04-22 15:23 UTC (permalink / raw)
  To: Julian Swagemakers, git, Junio C Hamano; +Cc: M Hickford, sandals, Shengyu Qu

Outlook does not accept the Message-ID header in the email body. Instead
it saves it in its own proprietary X-Microsoft-Original-Message-ID
header and a random Message-ID is set my the server. As a result,
replying to threads does not work.

The $smtp->message variable in this script for outlook is something like
this:

2.0.0 OK <Message-ID> [Hostname=Some-hostname]

This contains the Message-ID set by Microsoft in the first <>.

This patch retrieves the Message-ID from this server response
and sets it in the email headers instead of using the self generated one.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 git-send-email.perl | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index a6cafda29c..216b23caa5 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1799,6 +1799,17 @@ sub send_message {
 			$smtp->datasend("$line") or die $smtp->message;
 		}
 		$smtp->dataend() or die $smtp->message;
+
+		# Retrieve the Message-ID from the server response in case of Outlook
+		if ($smtp_server eq 'smtp.office365.com' || $smtp_server eq 'smtp-mail.outlook.com') {
+			if ($smtp->message =~ /<([^>]+)>/) {
+				$message_id = "<$1>";
+				printf __("Outlook: Retrieved Message-ID: %s\n"), $message_id;
+			} else {
+				warn __("Warning: Could not retrieve Message-ID from server response.\n");
+			}
+		}
+
 		$smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
 	}
 	if ($quiet) {
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/3] send-email: add option to generate passswords like OAuth2 tokens
  2025-04-22 15:23 [PATCH v3 0/3] send-email: add oauth2 support and fix outlook breaking threads Aditya Garg
  2025-04-22 15:23 ` [PATCH v3 1/3] send-email: implement SMTP bearer authentication Aditya Garg
  2025-04-22 15:23 ` [PATCH v3 2/3] send-email: retrieve Message-ID from outlook SMTP server Aditya Garg
@ 2025-04-22 15:23 ` Aditya Garg
  2025-04-22 22:20   ` Junio C Hamano
  2 siblings, 1 reply; 10+ messages in thread
From: Aditya Garg @ 2025-04-22 15:23 UTC (permalink / raw)
  To: Julian Swagemakers, git, Junio C Hamano; +Cc: M Hickford, sandals, Shengyu Qu

Some email providers like outlook allow only OAuth2 tokens to be used
for authentication. This commit adds an option to generate OAuth2 tokens
using scripts like M365-IMAP[1]. This option is similar to passwordeval
in msmtp.

Example usage:

[sendemail]
    smtpEncryption = tls
    smtpServer = smtp.office365.com
    smtpUser = someone@outlook.com
    smtpServerPort = 587
    smtpauth = XOAUTH2
    smtpPassEval = cd /workspaces/codespaces-blank/M365-IMAP && python3 ./refresh_token.py

Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
 Documentation/git-send-email.adoc |  8 ++++++++
 git-send-email.perl               | 15 +++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc
index 1bf75c060d..b41c807960 100644
--- a/Documentation/git-send-email.adoc
+++ b/Documentation/git-send-email.adoc
@@ -230,6 +230,14 @@ or on the command line. If a username has been specified (with
 specified (with `--smtp-pass` or `sendemail.smtpPass`), then
 a password is obtained using 'git-credential'.
 
+--smtp-passeval[=<command>]::
+	Generate password or OAuth2 token for SMTP AUTH. The argument is
+	optional. If specified, it will use the output of any password
+	or OAuth2 token generated using the command specified.
++
+Note that it will override any existing password specified using
+`--smtp-user` or a `sendemail.smtpUser`.
+
 --no-smtp-auth::
 	Disable SMTP authentication. Short hand for `--smtp-auth=none`
 
diff --git a/git-send-email.perl b/git-send-email.perl
index 216b23caa5..49de6a048e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -59,6 +59,8 @@ sub usage {
     --smtp-server-port      <int>  * Outgoing SMTP server port.
     --smtp-user             <str>  * Username for SMTP-AUTH.
     --smtp-pass             <str>  * Password for SMTP-AUTH; not necessary.
+    --smtp-passeval         <str>  * Path to script or a command to generate
+                                     password like OAuth2 token for SMTP-AUTH.
     --smtp-encryption       <str>  * tls or ssl; anything else disables.
     --smtp-ssl                     * Deprecated. Use '--smtp-encryption ssl'.
     --smtp-ssl-cert-path    <str>  * Path to ca-certificates (either directory or file).
@@ -280,6 +282,7 @@ sub do_edit {
 my ($auto_8bit_encoding);
 my ($compose_encoding);
 my ($sendmail_cmd);
+my ($smtp_authpasseval);
 my ($mailmap_file, $mailmap_blob);
 # Variables with corresponding config settings & hardcoded defaults
 my ($debug_net_smtp) = 0;		# Net::SMTP, see send_message()
@@ -316,6 +319,7 @@ sub do_edit {
     "smtppass" => \$smtp_authpass,
     "smtpdomain" => \$smtp_domain,
     "smtpauth" => \$smtp_auth,
+    "smtppasseval" => \$smtp_authpasseval,
     "smtpbatchsize" => \$batch_size,
     "smtprelogindelay" => \$relogin_delay,
     "to" => \@config_to,
@@ -516,6 +520,7 @@ sub config_regexp {
 		    "smtp-server-port=s" => \$smtp_server_port,
 		    "smtp-user=s" => \$smtp_authuser,
 		    "smtp-pass:s" => \$smtp_authpass,
+		    "smtp-passeval=s" => \$smtp_authpasseval,
 		    "smtp-ssl" => sub { $smtp_encryption = 'ssl' },
 		    "smtp-encryption=s" => \$smtp_encryption,
 		    "smtp-ssl-cert-path=s" => \$smtp_ssl_cert_path,
@@ -1463,6 +1468,16 @@ sub smtp_auth_maybe {
 		return 1;
 	}
 
+	# If smtpPassEval is set, run the user specified command to get the password
+	if (defined $smtp_authpasseval) {
+		printf __("Executing smtpPassEval: %s\n"), $smtp_authpasseval;
+		chomp(my $generated_password = `$smtp_authpasseval 2>&1`);
+		if ($? != 0) {
+			die sprintf(__("Failed to execute smtpPassEval: %s\n"), $smtp_authpasseval);
+		}
+		$smtp_authpass = $generated_password;
+	}
+
 	# Workaround AUTH PLAIN/LOGIN interaction defect
 	# with Authen::SASL::Cyrus
 	eval {
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] send-email: retrieve Message-ID from outlook SMTP server
  2025-04-22 15:23 ` [PATCH v3 2/3] send-email: retrieve Message-ID from outlook SMTP server Aditya Garg
@ 2025-04-22 22:05   ` Junio C Hamano
  2025-04-23  3:22     ` Aditya Garg
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2025-04-22 22:05 UTC (permalink / raw)
  To: Aditya Garg; +Cc: Julian Swagemakers, git, M Hickford, sandals, Shengyu Qu

Aditya Garg <gargaditya08@live.com> writes:

> Outlook does not accept the Message-ID header in the email body. Instead
> it saves it in its own proprietary X-Microsoft-Original-Message-ID
> header and a random Message-ID is set my the server. As a result,
> replying to threads does not work.
>
> The $smtp->message variable in this script for outlook is something like
> this:
>
> 2.0.0 OK <Message-ID> [Hostname=Some-hostname]
>
> This contains the Message-ID set by Microsoft in the first <>.
>
> This patch retrieves the Message-ID from this server response
> and sets it in the email headers instead of using the self generated one.

Hmph.

send_message calls gen_header as the first thing.  This prepares the
usual From:/To:/Subject:/Date:/Message-ID: lines and returns the
header text as well as recipient addresses broken out into different
classes, among other things.

> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---
>  git-send-email.perl | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index a6cafda29c..216b23caa5 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1799,6 +1799,17 @@ sub send_message {

And before these pre-context lines, the composed $header that
contains the message_id has already been sent by calling datasend()
method on the smtp object.  After that, we are ...

>  			$smtp->datasend("$line") or die $smtp->message;
>  		}

... sending the body of the e-mail here.

So it is not clear to me how ovewriting the $message_id variable
after the message with $header with "Message-ID:" line that
contained the ID we generated has already given to the SMTP server.
What the code is doing certainly contradicts with what the proposed
log message explains it does, i.e.

	... sets it in the email headers instead of using ...

It would affect the message-ID that is used by subsequent messages
when they are sent as replies to this message.  I do not think we
computed the header (the In-Reply-To: field) for the next message
at this point of the code, and I can well believe that mucking with
the $message variable at this point would make the next message
correctly a response to this one.

Perhaps you meant that Outlook DISCARDS the Message-ID: field in the
message it was instructed to send out, and INSERTS its own?  Then I
can see how this patch would improve the situation, but the last
paragraph in the proposed log message needs to be rewritten.


	After sending a message, retrieve the message-ID the Outlook
	server assigned to the message and store it in $message_id
	variable; this value will be used when next and subsequent
	message are sent as replies to the message, preserving the
	threading of the messages.

or something.

> +		# Retrieve the Message-ID from the server response in case of Outlook
> +		if ($smtp_server eq 'smtp.office365.com' || $smtp_server eq 'smtp-mail.outlook.com') {

Perhaps a small helper sub

	sub is_outlook {
		my ($host) = @_;
		return ($host eq '...' ||
			$host eq '...');
	}

would make it easier to maintain (and read).

> +			if ($smtp->message =~ /<([^>]+)>/) {
> +				$message_id = "<$1>";
> +				printf __("Outlook: Retrieved Message-ID: %s\n"), $message_id;

Is this worth reporting, or more or less a leftover debugging aid?

The fact that we retrieved (as opposed to "could not retrieve" case)
may have been significant during the development of this feature,
but to end-users who see this message, retrieved is not at all
interesting.  What is interesting to them is that the server gave
your message some random Message-ID you've never heard of, so while
it may make sense to report what that ID is, perhaps 

	Outlook reassigned Messsage-ID: %s

might be more meaningful to them.  rewritten, reassigned, used,
there may be even better verbs, but the point is that the subject of
that sentence is the outlook smtp serveron the other end of the
connection (as opposed to the actor of "retrived" is the program
running on our end).

> +			} else {
> +				warn __("Warning: Could not retrieve Message-ID from server response.\n");
> +			}
> +		}
> +
>  		$smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
>  	}
>  	if ($quiet) {

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] send-email: add option to generate passswords like OAuth2 tokens
  2025-04-22 15:23 ` [PATCH v3 3/3] send-email: add option to generate passswords like OAuth2 tokens Aditya Garg
@ 2025-04-22 22:20   ` Junio C Hamano
  2025-04-23  3:10     ` Aditya Garg
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2025-04-22 22:20 UTC (permalink / raw)
  To: Aditya Garg; +Cc: Julian Swagemakers, git, M Hickford, sandals, Shengyu Qu

Aditya Garg <gargaditya08@live.com> writes:

> +--smtp-passeval[=<command>]::
> +	Generate password or OAuth2 token for SMTP AUTH. The argument is
> +	optional. If specified, it will use the output of any password
> +	or OAuth2 token generated using the command specified.
> ++
> +Note that it will override any existing password specified using
> +`--smtp-user` or a `sendemail.smtpUser`.

If the argument is optional, we should explain what the behaviour is
when the optional argument is omitted, as well as how the given
argument is used.  You are doing only the latter, but not the former.

Shouldn't the "command" be mandatory, if the option is used?  I do
not quite see how these invocations

	git-send-email ... --smtp-passeval ...
	git-send-email ... --smtp-passeval= ...

that do not specify the command to be used is useful.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] send-email: add option to generate passswords like OAuth2 tokens
  2025-04-22 22:20   ` Junio C Hamano
@ 2025-04-23  3:10     ` Aditya Garg
  0 siblings, 0 replies; 10+ messages in thread
From: Aditya Garg @ 2025-04-23  3:10 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Julian Swagemakers, git@vger.kernel.org, M Hickford,
	sandals@crustytoothpaste.net, Shengyu Qu



> On 23 Apr 2025, at 3:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Aditya Garg <gargaditya08@live.com> writes:
> 
>> +--smtp-passeval[=<command>]::
>> +    Generate password or OAuth2 token for SMTP AUTH. The argument is
>> +    optional. If specified, it will use the output of any password
>> +    or OAuth2 token generated using the command specified.
>> ++
>> +Note that it will override any existing password specified using
>> +`--smtp-user` or a `sendemail.smtpUser`.
> 
> If the argument is optional, we should explain what the behaviour is
> when the optional argument is omitted, as well as how the given
> argument is used.  You are doing only the latter, but not the former.
> 

Argument is not optional. It's a left over line I had copied from above for the sake of formatting. My bad here.

> Shouldn't the "command" be mandatory, if the option is used?  I do
> not quite see how these invocations
> 
>    git-send-email ... --smtp-passeval ...
>    git-send-email ... --smtp-passeval= ...
> 
> that do not specify the command to be used is useful.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] send-email: retrieve Message-ID from outlook SMTP server
  2025-04-22 22:05   ` Junio C Hamano
@ 2025-04-23  3:22     ` Aditya Garg
  2025-04-23 15:34       ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Aditya Garg @ 2025-04-23  3:22 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Julian Swagemakers, git@vger.kernel.org, M Hickford,
	sandals@crustytoothpaste.net, Shengyu Qu



> On 23 Apr 2025, at 3:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Aditya Garg <gargaditya08@live.com> writes:
> 
>> Outlook does not accept the Message-ID header in the email body. Instead
>> it saves it in its own proprietary X-Microsoft-Original-Message-ID
>> header and a random Message-ID is set my the server. As a result,
>> replying to threads does not work.
>> 
>> The $smtp->message variable in this script for outlook is something like
>> this:
>> 
>> 2.0.0 OK <Message-ID> [Hostname=Some-hostname]
>> 
>> This contains the Message-ID set by Microsoft in the first <>.
>> 
>> This patch retrieves the Message-ID from this server response
>> and sets it in the email headers instead of using the self generated one.
> 
> Hmph.
> 
> send_message calls gen_header as the first thing.  This prepares the
> usual From:/To:/Subject:/Date:/Message-ID: lines and returns the
> header text as well as recipient addresses broken out into different
> classes, among other things.
> 
>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>> ---
>> git-send-email.perl | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>> 
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index a6cafda29c..216b23caa5 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -1799,6 +1799,17 @@ sub send_message {
> 
> And before these pre-context lines, the composed $header that
> contains the message_id has already been sent by calling datasend()
> method on the smtp object.  After that, we are ...
> 
>>            $smtp->datasend("$line") or die $smtp->message;
>>        }
> 
> ... sending the body of the e-mail here.
> 
> So it is not clear to me how ovewriting the $message_id variable
> after the message with $header with "Message-ID:" line that
> contained the ID we generated has already given to the SMTP server.
> What the code is doing certainly contradicts with what the proposed
> log message explains it does, i.e.
> 
>    ... sets it in the email headers instead of using ...
> 
> It would affect the message-ID that is used by subsequent messages
> when they are sent as replies to this message.  I do not think we
> computed the header (the In-Reply-To: field) for the next message
> at this point of the code, and I can well believe that mucking with
> the $message variable at this point would make the next message
> correctly a response to this one.
> 
> Perhaps you meant that Outlook DISCARDS the Message-ID: field in the
> message it was instructed to send out, and INSERTS its own?  Then I
> can see how this patch would improve the situation, but the last
> paragraph in the proposed log message needs to be rewritten.

Exactly. We don't care what message id the script is sending here. We just
send the first mail, and then retrieve the message ID that was set by outlook.

Then we change the variable so that In-reply-to and References work properly.

>    After sending a message, retrieve the message-ID the Outlook
>    server assigned to the message and store it in $message_id
>    variable; this value will be used when next and subsequent
>    message are sent as replies to the message, preserving the
>    threading of the messages.
> 
> or something.
> 

I'll change the log message, although it seemed clear to me.

>> +        # Retrieve the Message-ID from the server response in case of Outlook
>> +        if ($smtp_server eq 'smtp.office365.com' || $smtp_server eq 'smtp-mail.outlook.com') {
> 
> Perhaps a small helper sub
> 
>    sub is_outlook {
>        my ($host) = @_;
>        return ($host eq '...' ||
>            $host eq '...');
>    }
> 
> would make it easier to maintain (and read).

Ok

> 
>> +            if ($smtp->message =~ /<([^>]+)>/) {
>> +                $message_id = "<$1>";
>> +                printf __("Outlook: Retrieved Message-ID: %s\n"), $message_id;
> 
> Is this worth reporting, or more or less a leftover debugging aid?
> 
> The fact that we retrieved (as opposed to "could not retrieve" case)
> may have been significant during the development of this feature,
> but to end-users who see this message, retrieved is not at all
> interesting.  What is interesting to them is that the server gave
> your message some random Message-ID you've never heard of, so while
> it may make sense to report what that ID is, perhaps
> 
>    Outlook reassigned Messsage-ID: %s
> 
> might be more meaningful to them.  rewritten, reassigned, used,
> there may be even better verbs, but the point is that the subject of
> that sentence is the outlook smtp serveron the other end of the
> connection (as opposed to the actor of "retrived" is the program
> running on our end).

I'll change the message, and having some message about this IMO
should be there.
> 
>> +            } else {
>> +                warn __("Warning: Could not retrieve Message-ID from server response.\n");
>> +            }
>> +        }
>> +
>>        $smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
>>    }
>>    if ($quiet) {

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] send-email: retrieve Message-ID from outlook SMTP server
  2025-04-23  3:22     ` Aditya Garg
@ 2025-04-23 15:34       ` Junio C Hamano
  2025-04-23 16:24         ` Aditya Garg
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2025-04-23 15:34 UTC (permalink / raw)
  To: Aditya Garg
  Cc: Julian Swagemakers, git@vger.kernel.org, M Hickford,
	sandals@crustytoothpaste.net, Shengyu Qu

Aditya Garg <gargaditya08@live.com> writes:

>> On 23 Apr 2025, at 3:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> 
>> Aditya Garg <gargaditya08@live.com> writes:
>> 
>>> Outlook does not accept the Message-ID header in the email body. Instead
>>> it saves it in its own proprietary X-Microsoft-Original-Message-ID
>>> header and a random Message-ID is set my the server. As a result,
>>> replying to threads does not work.
>>> 
>>> The $smtp->message variable in this script for outlook is something like
>>> this:
>>> 
>>> 2.0.0 OK <Message-ID> [Hostname=Some-hostname]
>>> 
>>> This contains the Message-ID set by Microsoft in the first <>.
>>> 
>>> This patch retrieves the Message-ID from this server response
>>> and sets it in the email headers instead of using the self generated one.
>> 
>> Hmph.
>> 
>> send_message calls gen_header as the first thing.  This prepares the
>> usual From:/To:/Subject:/Date:/Message-ID: lines and returns the
>> header text as well as recipient addresses broken out into different
>> classes, among other things.
>> 
>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>>> ---
>>> git-send-email.perl | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>> 
>>> diff --git a/git-send-email.perl b/git-send-email.perl
>>> index a6cafda29c..216b23caa5 100755
>>> --- a/git-send-email.perl
>>> +++ b/git-send-email.perl
>>> @@ -1799,6 +1799,17 @@ sub send_message {
>> 
>> And before these pre-context lines, the composed $header that
>> contains the message_id has already been sent by calling datasend()
>> method on the smtp object.  After that, we are ...
>> 
>>>            $smtp->datasend("$line") or die $smtp->message;
>>>        }
>> 
>> ... sending the body of the e-mail here.
>> 
>> So it is not clear to me how ovewriting the $message_id variable
>> after the message with $header with "Message-ID:" line that
>> contained the ID we generated has already given to the SMTP server.
>> What the code is doing certainly contradicts with what the proposed
>> log message explains it does, i.e.
>> 
>>    ... sets it in the email headers instead of using ...
>> 
>> It would affect the message-ID that is used by subsequent messages
>> when they are sent as replies to this message.  I do not think we
>> computed the header (the In-Reply-To: field) for the next message
>> at this point of the code, and I can well believe that mucking with
>> the $message variable at this point would make the next message
>> correctly a response to this one.
>> 
>> Perhaps you meant that Outlook DISCARDS the Message-ID: field in the
>> message it was instructed to send out, and INSERTS its own?  Then I
>> can see how this patch would improve the situation, but the last
>> paragraph in the proposed log message needs to be rewritten.
>
> Exactly. We don't care what message id the script is sending here. We just
> send the first mail, and then retrieve the message ID that was set by outlook.
>
>
> Then we change the variable so that In-reply-to and References work properly.
>
>>    After sending a message, retrieve the message-ID the Outlook
>>    server assigned to the message and store it in $message_id
>>    variable; this value will be used when next and subsequent
>>    message are sent as replies to the message, preserving the
>>    threading of the messages.
>> 
>> or something.
>> 
>
> I'll change the log message, although it seemed clear to me.

Sounds good.  What confused me was "and sets it in the email
headers" in the proposed log message looked as if it was referring
to the Message-ID: field of the message we retrieved the ID Outlook
would assign to it, i.e. as if the order of events were (0) we grab
an ID from Outlook, (1) we compose the header, embedding the ID in
its Message-ID: field, (2) we send the result out.  What happens in
reality, if I understand you correctly, is that (0) we compose the
header, with our message-ID in its Message-ID: field, (1) Outlook
discard our Message-ID: field, replaces it with its own, (2) we can
learn the ID Outlook used, and (3) we'll pretend as if that ID is
what we gave the message we just sent in the first place, which
would be used in In-Reply-To: field of the messages sent as replies
to it, but it was unclear from the description which one you meant.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] send-email: retrieve Message-ID from outlook SMTP server
  2025-04-23 15:34       ` Junio C Hamano
@ 2025-04-23 16:24         ` Aditya Garg
  0 siblings, 0 replies; 10+ messages in thread
From: Aditya Garg @ 2025-04-23 16:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Julian Swagemakers, git@vger.kernel.org, M Hickford,
	sandals@crustytoothpaste.net, Shengyu Qu



> On 23 Apr 2025, at 9:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Aditya Garg <gargaditya08@live.com> writes:
> 
>>>> On 23 Apr 2025, at 3:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> 
>>> Aditya Garg <gargaditya08@live.com> writes:
>>> 
>>>> Outlook does not accept the Message-ID header in the email body. Instead
>>>> it saves it in its own proprietary X-Microsoft-Original-Message-ID
>>>> header and a random Message-ID is set my the server. As a result,
>>>> replying to threads does not work.
>>>> 
>>>> The $smtp->message variable in this script for outlook is something like
>>>> this:
>>>> 
>>>> 2.0.0 OK <Message-ID> [Hostname=Some-hostname]
>>>> 
>>>> This contains the Message-ID set by Microsoft in the first <>.
>>>> 
>>>> This patch retrieves the Message-ID from this server response
>>>> and sets it in the email headers instead of using the self generated one.
>>> 
>>> Hmph.
>>> 
>>> send_message calls gen_header as the first thing.  This prepares the
>>> usual From:/To:/Subject:/Date:/Message-ID: lines and returns the
>>> header text as well as recipient addresses broken out into different
>>> classes, among other things.
>>> 
>>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>>>> ---
>>>> git-send-email.perl | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>> 
>>>> diff --git a/git-send-email.perl b/git-send-email.perl
>>>> index a6cafda29c..216b23caa5 100755
>>>> --- a/git-send-email.perl
>>>> +++ b/git-send-email.perl
>>>> @@ -1799,6 +1799,17 @@ sub send_message {
>>> 
>>> And before these pre-context lines, the composed $header that
>>> contains the message_id has already been sent by calling datasend()
>>> method on the smtp object.  After that, we are ...
>>> 
>>>>           $smtp->datasend("$line") or die $smtp->message;
>>>>       }
>>> 
>>> ... sending the body of the e-mail here.
>>> 
>>> So it is not clear to me how ovewriting the $message_id variable
>>> after the message with $header with "Message-ID:" line that
>>> contained the ID we generated has already given to the SMTP server.
>>> What the code is doing certainly contradicts with what the proposed
>>> log message explains it does, i.e.
>>> 
>>>   ... sets it in the email headers instead of using ...
>>> 
>>> It would affect the message-ID that is used by subsequent messages
>>> when they are sent as replies to this message.  I do not think we
>>> computed the header (the In-Reply-To: field) for the next message
>>> at this point of the code, and I can well believe that mucking with
>>> the $message variable at this point would make the next message
>>> correctly a response to this one.
>>> 
>>> Perhaps you meant that Outlook DISCARDS the Message-ID: field in the
>>> message it was instructed to send out, and INSERTS its own?  Then I
>>> can see how this patch would improve the situation, but the last
>>> paragraph in the proposed log message needs to be rewritten.
>> 
>> Exactly. We don't care what message id the script is sending here. We just
>> send the first mail, and then retrieve the message ID that was set by outlook.
>> 
>> 
>> Then we change the variable so that In-reply-to and References work properly.
>> 
>>>   After sending a message, retrieve the message-ID the Outlook
>>>   server assigned to the message and store it in $message_id
>>>   variable; this value will be used when next and subsequent
>>>   message are sent as replies to the message, preserving the
>>>   threading of the messages.
>>> 
>>> or something.
>>> 
>> 
>> I'll change the log message, although it seemed clear to me.
> 
> Sounds good.  What confused me was "and sets it in the email
> headers" in the proposed log message looked as if it was referring
> to the Message-ID: field of the message we retrieved the ID Outlook
> would assign to it, i.e. as if the order of events were (0) we grab
> an ID from Outlook, (1) we compose the header, embedding the ID in
> its Message-ID: field, (2) we send the result out.  What happens in
> reality, if I understand you correctly, is that (0) we compose the
> header, with our message-ID in its Message-ID: field, (1) Outlook
> discard our Message-ID: field, replaces it with its own, (2) we can
> learn the ID Outlook used, and (3) we'll pretend as if that ID is
> what we gave the message we just sent in the first place, which
> would be used in In-Reply-To: field of the messages sent as replies
> to it, but it was unclear from the description which one you meant.

You understood correctly. I've already sent a v4 with detailed description as well
> 
> Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-04-23 16:24 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-22 15:23 [PATCH v3 0/3] send-email: add oauth2 support and fix outlook breaking threads Aditya Garg
2025-04-22 15:23 ` [PATCH v3 1/3] send-email: implement SMTP bearer authentication Aditya Garg
2025-04-22 15:23 ` [PATCH v3 2/3] send-email: retrieve Message-ID from outlook SMTP server Aditya Garg
2025-04-22 22:05   ` Junio C Hamano
2025-04-23  3:22     ` Aditya Garg
2025-04-23 15:34       ` Junio C Hamano
2025-04-23 16:24         ` Aditya Garg
2025-04-22 15:23 ` [PATCH v3 3/3] send-email: add option to generate passswords like OAuth2 tokens Aditya Garg
2025-04-22 22:20   ` Junio C Hamano
2025-04-23  3:10     ` Aditya Garg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox