search

awdeorio / mailmerge

A simple, command line mail merge tool.
MIT License
140 stars 41 forks source link

Add support for 2FA OAuth authentication #150

Closed robstewart57 closed 1 year ago

robstewart57 commented 1 year ago

This issue is to track a new feature of supporting OAuth based authentication.

I have a functional prototype of such an implementation in my wip-xoauth branch: https://github.com/awdeorio/mailmerge/compare/develop...robstewart57:mailmerge:wip-xoauth

This commit https://github.com/awdeorio/mailmerge/commit/79b006ee6c2cb065f0dd35609d58d0b473f9bd51 adds the feature.

I've tested this with Microsoft Outlook, using 2FA, and it successfully sends email via OAuth.

Two comments:

  1. It is using the password for... prompt (i.e. self.password) to obtain the user's OAuth token. However, a token isn't a password, so either self.token is added to sendmail_client.py in addition to self.password, or the wording is changed in the interactive prompt e.g. from "password" to "password/token".
  2. It access an OAuth token that would be created by a 3rd party tool that would use the tenant_id, client_id and client_secret. E.g I'm using oauth2ms, then I'm calling oauth2ms to obtain the token to input into the password prompt for mailmerge. A more sophisticated implementation of OAuth support in mailmerge would re-implement (or reuse as a library) the functionality of oauth2ms to dynamically create an Oauth token from those three values. This would add complexity, so perhaps it's more desirable to keep this outside of mailmerge, i.e. as I've implemented it in my wip-xoauth branch.
awdeorio commented 1 year ago

This looks like a great start!

wording is changed in the interactive prompt e.g. from "password" to "password/token".

Perhaps adding a sample server config with some comments about this would work.

E.g I'm using oauth2ms, then I'm calling oauth2ms to obtain the token to input into the password prompt

Seems reasonable to me. The process would just need to be documented so others could replicate it. Maybe this would also be OK in the comments of the sample server config.

How about updating the sample server config and then making a pull request as a next step? From there, I can help with adding a unit test.

robstewart57 commented 1 year ago

@awdeorio I've created a PR with the implementation, a sample server config and documentation about how to use it:

https://github.com/awdeorio/mailmerge/pull/152