emersion / go-smtp

📤 An SMTP client & server library written in Go
MIT License
1.72k stars 216 forks source link

server: replace EnableAuth with AuthSession #251

Closed emersion closed 6 months ago

emersion commented 7 months ago

Closes: https://github.com/emersion/go-smtp/issues/170

Future work:

yusufozturk commented 3 months ago

@emersion is there any documentation or test update to see how we can upgrade from older versions to this version?

image

Our build job was broken after package upgrades.

yusufozturk commented 3 months ago

Just feedback:

Maybe consider keeping old API methods for a while but display a deprecated warning in the console. Link the old methods to the new ones so that the old methods serve only as a compatibility bridge. This way, users can see how they should upgrade their code.

Thanks for this great library and your efforts to keep it up-to-date. @emersion

mgnsk commented 3 months ago

I recently went through the upgrade. Basically, the server no longer cares about authentication, it is up to sessions to implement either Session or AuthSession.

The server checks if the session implements AuthSession and AuthMechanisms() returns a non-empty slice and advertises the AUTH capability if so. Similarly, if the session implements AuthSession, it calls AuthSession.Auth() during the AUTH command.

To avoid implementing two different sessions, we only implemented a single session that always implements AuthSession and manually returns ErrAuthUnknownMechanism from its Auth() method if no mechanisms are enabled.

yusufozturk commented 3 months ago

@mgnsk Thanks!

Can you share some code blocks how do you implement a single session? For the session, I think I need to change Backend implementation but I don't know how I can do that. Thanks!

mgnsk commented 3 months ago

Can you share some code blocks how do you implement a single session? For the session, I think I need to change Backend implementation but I don't know how I can do that. Thanks!

The most safe way would be to implement unauthenticated and authenticated sessions separately:

// Unauthenticated session.
type session struct{}

func (s *session) Mail(from string, opts *smtp.MailOptions) error {
    // Handle MAIL.
}

func NewSession() smtp.Session {
    return &session{}
}

// Authenticated session.
type authSession struct {
    mechanisms      []string
    isAuthenticated bool
}

func (s *authSession) AuthMechanisms() []string {
    return s.mechanisms
}

func (s *authSession) Auth(mech string) (sasl.Server, error) {
    // return sasl.server for mech
    // in the sasl.Server callback set s.isAuthenticated = true on success
}

func (s *authSession) Mail(from string, opts *smtp.MailOptions) error {
    if !s.isAuthenticated {
        return smtp.ErrAuthRequired
    }

    // Handle MAIL.
}

func NewAuthSession(mechanisms []string) smtp.AuthSession {
    return &authSession{
        mechanisms: mechanisms,
    }
}

The backend could look something like this where it returns either of the sessions, depending on if any auth mechanisms were specified:

type backend struct {
    mechanisms []string
}

func (b *backend) NewSession(c *smtp.Conn) (smtp.Session, error) {
    if len(b.mechanisms) > 0 {
        return NewAuthSession(b.mechanisms), nil 
    }

    return NewSession(), nil
}

func NewBackend(mechanisms []string) smtp.Backend {
    return &backend{
        mechanisms: mechanisms,
    }
}

If you only need authenticated sessions, then only implement smtp.AuthSession. If you need to handle both, it is safest to implement separate sessions but you could also shoehorn everything into a single smtp.AuthSession at the cost of greatly increasing complexity and testing burden. For example:

func (s *authSession) Auth(mech string) (sasl.Server, error) {
    if len(s.mechanisms) == 0 {
        return smtp.ErrAuthUnsupported
    }

    // return sasl.server for mech
    // in the sasl.Server callback set s.isAuthenticated = true on success
}

func (s *authSession) Mail(from string, opts *smtp.MailOptions) error {
    if len(s.mechanisms) > 0 && !s.isAuthenticated {
        return smtp.ErrAuthRequired
    }

    // Handle MAIL.
}
emersion commented 3 months ago

The release notes have a high-level overview of the breaking changes: https://github.com/emersion/go-smtp/releases/tag/v0.21.0

I considered leaving the old API as deprecated but it wasn't possible to do so properly without a lot of churn. Keep in mind that I'm maintaining this library during my free time as a volunteer.