Re: [PATCH v4 1/3] send-email: implement SMTP bearer authentication

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On 23 Apr 2025, at 11:34 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> 
> Aditya Garg <gargaditya08@xxxxxxxx> writes:
> 
>> From: Julian Swagemakers <julian@xxxxxxxxxxxxxxx>
>> 
>> 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:
> 
> I am not familiar with this area, especially with Authen::SASL, so
> I'd appreciate a second eye from other reviewers.

What I've noticed is that other reviewers didn't seem to have reviewed this
patch for more than a year when the original author proposed this patch.
Also, oauth2 is something that is significant in today's world and is definitely
more secure as well.

Nevertheless, your concern is quite valid, but I would also appreciate atleast
pinging the other reviewers who might have the knowledge. 

I'm Ccing Greg, who is credited for this script and the maintainer of the perl module
as well, with a hope to get a positive response.
> 
> Having said that, the documentation is very clearly written, so is
> the above log message.
> 
> Please fold overly long lines the patch adds.  We officially aim for
> 80-column soft limit, and we allow going over it when folding lines
> to stay under it makes the result less readable. But lines added to
> the credential callback to call smtp_bearer_auth() are a way too
> wide, for example [*].
> 
> Footnote [*] The text themselves are not overly wide, but the long
> lines there are primarily due to them deeply indented.  I have to
> wonder if it is a sign that the part of the code needs to be a bit
> better refactored, e.g., by defining the callback sub defined
> elsewhere and passed to Git::credential() call as a variable that
> holds a reference to it, instead of defining an anonymous sub in
> place there, for example.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux