Re: [PATCH b4 2/2] b4: add support for oauth2 SMTP authentication

[Tomas Melin <tomas.melin@vaisala.com>]

Hi,

On 01/02/2026 07:37, Konstantin Ryabitsev wrote:
> On Thu, 29 Jan 2026 11:10:30 +0200, Tomas Melin <tomas.melin@vaisala.com> wrote:
>> diff --git a/src/b4/__init__.py b/src/b4/__init__.py
>> index 3d774f7..0adf5ca 100644
>> --- a/src/b4/__init__.py
>> +++ b/src/b4/__init__.py
>> @@ -2922,6 +2923,16 @@ def git_credential_fill(gitdir: Optional[str], protocol: str, host: str, usernam
>>      return None
>>  
>>  
>> +def git_credential_helper(name: str) -> str:
>> +    if not name:
>> +        raise ValueError("credential helper name cannot be empty")
>> +    name = 'credential-' + name
>> +    ecode, out = git_run_command(None, args=[name])
>> +    if ecode == 0:
> 
> Any reason we're not making use of git_credential_fill() here? I'm not sure we
> can expect correct return here, because credential helpers expect input on
> stdin like:
> 
>   protocol=smtp
>   host=<server>
>   username=<user>
> 
> The existing git_credential_fill() function should already handle this
> correctly, unless I'm mistaken or you have other reasons not to use it.

AFAIU basically they are kindof related but with slightly different
usage. git-credential-outlook will call out the helper and get the token.

Using 'git-credential-something' follows directly from the documented
usage for the git-credential helper
https://github.com/AdityaGarg8/git-credential-email?tab=readme-ov-file#usage
That way, the integration is seamless with git-send-email oauth2 and b4
oauth usage.



> 
>> +        return out.strip()
>> +    return ''
>> +
>> +
>>  def git_get_command_lines(gitdir: Optional[str], args: List[str]) -> List[str]:
>>      ecode, out = git_run_command(gitdir, args)
>>      lines = list()
>> @@ -4050,6 +4061,15 @@ def get_sendemail_config() -> Dict[str, Optional[Union[str, List[str]]]]:
>>      return SENDEMAIL_CONFIG
>>  
>>  
>> +def get_oauth2_token(server: str, smtpuser: str) -> str:
>> +    creds = get_config_from_git(rf'credential.*{server}*')
>> +    access_token = git_credential_helper(creds.get('helper', ''))
> 
> The regex does not look correct here. You probably want to do:
> 
>     rf'credential\.{re.escape(server)}\.*'
> 
> or something like that.
I've tested e.g. with server name smtp://smtp.office365.com:587.
Escaping does sound like correct this to do, need to test it out some more..


> 
>> +    if not access_token:
>> +        raise smtplib.SMTPException(f'Unable to get access token from git credential helper for server {server}')
>> +    auth_string = f'user={smtpuser}\x01auth=Bearer {access_token}\x01\x01'
>> +    return base64.b64encode(auth_string.encode()).decode()
>> +
>> +
>>  def get_smtp(dryrun: bool = False) -> Tuple[Union[smtplib.SMTP, smtplib.SMTP_SSL, List[str], None], str]:
>>      sconfig = get_sendemail_config()
>>      # Limited support for smtp settings to begin with, but should cover the vast majority of cases
>> @@ -4118,6 +4138,11 @@ def get_smtp(dryrun: bool = False) -> Tuple[Union[smtplib.SMTP, smtplib.SMTP_SSL
>>          if smtpauth.lower() == 'none':
>>              return smtp, fromaddr
>>  
>> +        if (sconfig.get('smtpauth').lower() == 'xoauth2'):
>> +            logger.debug('Authenticating to SMTP server using oauth2')
> 
> Two issues here:
> 
> 1. sconfig.get('smtpauth') has no default value. If smtpauth is not set
>    in the config, this returns None and .lower() raises AttributeError.
>    Two lines above, the same key is fetched with a default:
>      smtpauth = str(sconfig.get('smtpauth', ''))
>    You should use the existing `smtpauth` variable instead of re-fetching.
> 
> 2. The parentheses around the condition are unnecessary (minor Python style
>    issue).
> 
>> +            auth_b64 = get_oauth2_token(server, sconfig.get('smtpuser'))
>> +            smtp.docmd('AUTH', 'XOAUTH2 ' + auth_b64)
>> +
>>          auser = str(sconfig.get('smtpuser', ''))
>>          apass = str(sconfig.get('smtppass', ''))
> 
> This is going to not do the right thing.
> 
> After successful XOAUTH2 authentication, there is no return statement.
> Execution falls through to the basic auth block below, which will call
> smtp.login(auser, apass). This will either fail (no password configured)
> or attempt a second, unwanted authentication.

Indeed here the basic auth is a fallback operation. Certainly I can
change it to be exclusive so that it will not attempt basic auth.
But with oauth2 it currently works, if the basic credentials are
available they are autofilled and it proceeds.


> 
> Add `return smtp, fromaddr` after smtp.docmd() to prevent fall-through.

Yes, will add.

> 
> Also: smtp.docmd() returns a (code, message) tuple. The response code
> should be checked -- XOAUTH2 authentication can fail (e.g. expired
> token), and silently ignoring the response means the caller will not
> know authentication failed.
Absolutely, thanks for pointing out. Adding return value check here.


Thanks,
Tomas

> 
>>          if auser and not apass:
>