Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server: replace EnableAuth with AuthSession #251

Merged
merged 1 commit into from
Mar 28, 2024
Merged

Conversation

emersion
Copy link
Owner

Closes: #170

Future work:

  • Drop Session.AuthPlain
  • Drop Server.AuthDisabled

@emersion emersion merged commit c5e530a into master Mar 28, 2024
1 check passed
@emersion emersion deleted the auth-session branch March 28, 2024 15:29
@yusufozturk
Copy link

@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
Copy link

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
Copy link

mgnsk commented Jun 14, 2024

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
Copy link

@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
Copy link

mgnsk commented Jun 15, 2024

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
Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

server: Impossible to disable PLAIN auth
3 participants