From 1f54f5b8a4dc427136521014876d64be8fcb3dee Mon Sep 17 00:00:00 2001 From: Miguel Cabrerizo <30386061+doncicuto@users.noreply.github.com> Date: Tue, 30 Apr 2024 09:31:07 +0200 Subject: [PATCH] fix: Unrecognized Authentication Type Error when SMTP LOGIN Auth method is required (#7761) * fix: poc outlook.com now works login auth * fix: remove port arg from smtpAuth * fix: add outlook provider and custom email placeholder * fix: minor typo in contributing docs * fix: use zerrors package * fix: typo for idp and smtp providers --------- Co-authored-by: Max Peintner --- CONTRIBUTING.md | 20 +++---- .../known-smtp-providers-settings.ts | 18 ++++++ .../smtp-provider-routing.module.ts | 1 + .../smtp-provider.component.html | 8 ++- .../smtp-provider/smtp-provider.component.ts | 8 +++ .../smtp-table/smtp-table.component.html | 3 +- console/src/assets/i18n/en.json | 4 +- console/src/assets/images/smtp/outlook.svg | 1 + .../notification/channels/smtp/channel.go | 5 +- .../notification/channels/smtp/plain_auth.go | 22 ------- .../channels/smtp/plain_or_login_auth.go | 57 +++++++++++++++++++ 11 files changed, 107 insertions(+), 40 deletions(-) create mode 100644 console/src/assets/images/smtp/outlook.svg delete mode 100644 internal/notification/channels/smtp/plain_auth.go create mode 100644 internal/notification/channels/smtp/plain_or_login_auth.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6f28dbf042..9c18c920a4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -108,13 +108,13 @@ Please make sure you cover your changes with tests before marking a Pull Request The code consists of the following parts: -| name | description | language | where to find | -| --------------- | --------------------------------------------------------------- | --------------------------------------------------------------------------- | -------------------------------------------------- | -| backend | Service that serves the grpc(-web) and RESTful API | [go](https://go.dev) | [API implementation](./internal/api/grpc) | -| console | Frontend the user interacts with after log in | [Angular](https://angular.io), [Typescript](https://www.typescriptlang.org) | [./console](./console) | +| name | description | language | where to find | +| --------------- | ------------------------------------------------------------------ | --------------------------------------------------------------------------- | -------------------------------------------------- | +| backend | Service that serves the grpc(-web) and RESTful API | [go](https://go.dev) | [API implementation](./internal/api/grpc) | +| console | Frontend the user interacts with after log in | [Angular](https://angular.io), [Typescript](https://www.typescriptlang.org) | [./console](./console) | | login | Server side rendered frontend the user interacts with during login | [go](https://go.dev), [go templates](https://pkg.go.dev/html/template) | [./internal/api/ui/login](./internal/api/ui/login) | -| API definitions | Specifications of the API | [Protobuf](https://developers.google.com/protocol-buffers) | [./proto/zitadel](./proto/zitadel) | -| docs | Project documentation made with docusaurus | [Docusaurus](https://docusaurus.io/) | [./docs](./docs) | +| API definitions | Specifications of the API | [Protobuf](https://developers.google.com/protocol-buffers) | [./proto/zitadel](./proto/zitadel) | +| docs | Project documentation made with docusaurus | [Docusaurus](https://docusaurus.io/) | [./docs](./docs) | Please validate and test the code before you contribute. @@ -129,12 +129,12 @@ We add the label "good first issue" for problems we think are a good starting po We are committed to creating a welcoming and inclusive community for all developers, regardless of their gender identity or expression. To achieve this, we are actively working to ensure that our contribution guidelines are gender-neutral and use inclusive language. -**Use gender-neutral pronouns**: +**Use gender-neutral pronouns**: Don't use gender-specific pronouns unless the person you're referring to is actually that gender. In particular, don't use he, him, his, she, or her as gender-neutral pronouns, and don't use he/she or (s)he or other such punctuational approaches. Instead, use the singular they. -**Choose gender-neutral alternatives**: -Opt for gender-neutral terms instead of gendered ones whenever possible. +**Choose gender-neutral alternatives**: +Opt for gender-neutral terms instead of gendered ones whenever possible. Replace "policeman" with "police officer," "manpower" with "workforce," and "businessman" with "entrepreneur" or "businessperson." **Avoid ableist language**: @@ -194,7 +194,7 @@ make core_unit_test To test the database-connected gRPC API, run PostgreSQL and CockroachDB, set up a ZITADEL instance and run the tests including integration tests: ```bash -export INTEGRATION_DB_FLAVOR="postgres" ZITADEL_MASTERKEY="MasterkeyNeedsToHave32Characters" +export INTEGRATION_DB_FLAVOR="cockroach" ZITADEL_MASTERKEY="MasterkeyNeedsToHave32Characters" docker compose -f internal/integration/config/docker-compose.yaml up --pull always --wait ${INTEGRATION_DB_FLAVOR} make core_integration_test docker compose -f internal/integration/config/docker-compose.yaml down diff --git a/console/src/app/modules/smtp-provider/known-smtp-providers-settings.ts b/console/src/app/modules/smtp-provider/known-smtp-providers-settings.ts index ec6087ea43..c0d8f7e282 100644 --- a/console/src/app/modules/smtp-provider/known-smtp-providers-settings.ts +++ b/console/src/app/modules/smtp-provider/known-smtp-providers-settings.ts @@ -53,6 +53,7 @@ export interface ProviderDefaultSettings { value: string; placeholder: string; }; + senderEmailPlaceholder?: string; image?: string; routerLinkElement: string; } @@ -102,6 +103,7 @@ export const MailjetDefaultSettings: ProviderDefaultSettings = { user: { value: '', placeholder: 'Your Mailjet API key' }, password: { value: '', placeholder: 'Your Mailjet Secret key' }, image: './assets/images/smtp/mailjet.svg', + senderEmailPlaceholder: 'An authorized domain or email address', routerLinkElement: 'mailjet', }; @@ -114,6 +116,7 @@ export const PostmarkDefaultSettings: ProviderDefaultSettings = { user: { value: '', placeholder: 'Your Server API token' }, password: { value: '', placeholder: 'Your Server API token' }, image: './assets/images/smtp/postmark.png', + senderEmailPlaceholder: 'An authorized domain or email address', routerLinkElement: 'postmark', }; @@ -138,6 +141,7 @@ export const MailchimpDefaultSettings: ProviderDefaultSettings = { user: { value: '', placeholder: 'Your Mailchimp primary contact email' }, password: { value: '', placeholder: 'Your Mailchimp Transactional API key' }, image: './assets/images/smtp/mailchimp.svg', + senderEmailPlaceholder: 'An authorized domain or email address', routerLinkElement: 'mailchimp', }; @@ -153,6 +157,19 @@ export const BrevoDefaultSettings: ProviderDefaultSettings = { routerLinkElement: 'brevo', }; +export const OutlookDefaultSettings: ProviderDefaultSettings = { + name: 'outlook.com', + requiredTls: true, + host: 'smtp-mail.outlook.com', + unencryptedPort: 587, + encryptedPort: 587, + user: { value: '', placeholder: 'Your outlook.com email address' }, + password: { value: '', placeholder: 'Your outlook.com password' }, + image: './assets/images/smtp/outlook.svg', + senderEmailPlaceholder: 'Your outlook.com email address', + routerLinkElement: 'outlook', +}; + export const GenericDefaultSettings: ProviderDefaultSettings = { name: 'generic', requiredTls: false, @@ -170,5 +187,6 @@ export const SMTPKnownProviders = [ MailjetDefaultSettings, PostmarkDefaultSettings, SendgridDefaultSettings, + OutlookDefaultSettings, GenericDefaultSettings, ]; diff --git a/console/src/app/modules/smtp-provider/smtp-provider-routing.module.ts b/console/src/app/modules/smtp-provider/smtp-provider-routing.module.ts index 849be82a45..3cebddebf3 100644 --- a/console/src/app/modules/smtp-provider/smtp-provider-routing.module.ts +++ b/console/src/app/modules/smtp-provider/smtp-provider-routing.module.ts @@ -13,6 +13,7 @@ const types = [ { path: 'mailjet', component: SMTPProviderComponent }, { path: 'mailchimp', component: SMTPProviderComponent }, { path: 'brevo', component: SMTPProviderComponent }, + { path: 'outlook', component: SMTPProviderComponent }, ]; const routes: Routes = types.map((value) => { diff --git a/console/src/app/modules/smtp-provider/smtp-provider.component.html b/console/src/app/modules/smtp-provider/smtp-provider.component.html index c23b19f66e..b7ae8d8c63 100644 --- a/console/src/app/modules/smtp-provider/smtp-provider.component.html +++ b/console/src/app/modules/smtp-provider/smtp-provider.component.html @@ -106,7 +106,13 @@ {{ 'SETTING.SMTP.SENDERADDRESS' | translate }} - + diff --git a/console/src/app/modules/smtp-provider/smtp-provider.component.ts b/console/src/app/modules/smtp-provider/smtp-provider.component.ts index c3991a55c8..5a60e2e1a4 100644 --- a/console/src/app/modules/smtp-provider/smtp-provider.component.ts +++ b/console/src/app/modules/smtp-provider/smtp-provider.component.ts @@ -28,6 +28,7 @@ import { MailjetDefaultSettings, PostmarkDefaultSettings, ProviderDefaultSettings, + OutlookDefaultSettings, SendgridDefaultSettings, } from './known-smtp-providers-settings'; @@ -56,6 +57,8 @@ export class SMTPProviderComponent { public firstFormGroup!: UntypedFormGroup; public secondFormGroup!: UntypedFormGroup; + public senderEmailPlaceholder = 'sender@example.com'; + constructor( private service: AdminService, private _location: Location, @@ -91,6 +94,9 @@ export class SMTPProviderComponent { case 'brevo': this.providerDefaultSetting = BrevoDefaultSettings; break; + case 'outlook': + this.providerDefaultSetting = OutlookDefaultSettings; + break; } this.firstFormGroup = this.fb.group({ @@ -106,6 +112,8 @@ export class SMTPProviderComponent { password: [this.providerDefaultSetting?.password.value || ''], }); + this.senderEmailPlaceholder = this.providerDefaultSetting?.senderEmailPlaceholder || 'sender@example.com'; + this.secondFormGroup = this.fb.group({ senderAddress: ['', [requiredValidator]], senderName: ['', [requiredValidator]], diff --git a/console/src/app/modules/smtp-table/smtp-table.component.html b/console/src/app/modules/smtp-table/smtp-table.component.html index ba8ac58518..bbb59d451c 100644 --- a/console/src/app/modules/smtp-table/smtp-table.component.html +++ b/console/src/app/modules/smtp-table/smtp-table.component.html @@ -11,7 +11,8 @@ '/instance/smtpprovider/postmark/create', '/instance/smtpprovider/sendgrid/create', '/instance/smtpprovider/mailchimp/create', - '/instance/smtpprovider/brevo/create' + '/instance/smtpprovider/brevo/create', + '/instance/smtpprovider/outlook/create' ]" [timestamp]="configsResult?.details?.viewTimestamp" [selection]="selection" diff --git a/console/src/assets/i18n/en.json b/console/src/assets/i18n/en.json index 0a541cf418..008e4e29f1 100644 --- a/console/src/assets/i18n/en.json +++ b/console/src/assets/i18n/en.json @@ -1994,7 +1994,7 @@ }, "CREATE": { "TITLE": "Add provider", - "DESCRIPTION": "Select one ore more of the following providers.", + "DESCRIPTION": "Select one or more of the following providers.", "STEPPERTITLE": "Create Provider", "OIDC": { "TITLE": "OIDC Provider", @@ -2264,7 +2264,7 @@ }, "CREATE": { "TITLE": "Add SMTP provider", - "DESCRIPTION": "Select one ore more of the following providers.", + "DESCRIPTION": "Select one or more of the following providers.", "STEPS": { "TITLE": "Add {{ value }} SMTP Provider", "CREATE_DESC_TITLE": "Enter your {{ value }} SMTP settings step by step", diff --git a/console/src/assets/images/smtp/outlook.svg b/console/src/assets/images/smtp/outlook.svg new file mode 100644 index 0000000000..99409ecbc3 --- /dev/null +++ b/console/src/assets/images/smtp/outlook.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/internal/notification/channels/smtp/channel.go b/internal/notification/channels/smtp/channel.go index 3524634765..88f5bb7f6b 100644 --- a/internal/notification/channels/smtp/channel.go +++ b/internal/notification/channels/smtp/channel.go @@ -152,10 +152,7 @@ func (smtpConfig SMTP) smtpAuth(client *smtp.Client, host string) error { return nil } // Auth - auth := unencryptedAuth{ - smtp.PlainAuth("", smtpConfig.User, smtpConfig.Password, host), - } - err := client.Auth(auth) + err := client.Auth(PlainOrLoginAuth(smtpConfig.User, smtpConfig.Password, host)) if err != nil { return zerrors.ThrowInternalf(err, "EMAIL-s9kfs", "could not add smtp auth for user %s", smtpConfig.User) } diff --git a/internal/notification/channels/smtp/plain_auth.go b/internal/notification/channels/smtp/plain_auth.go deleted file mode 100644 index 76fde3ec8f..0000000000 --- a/internal/notification/channels/smtp/plain_auth.go +++ /dev/null @@ -1,22 +0,0 @@ -package smtp - -import ( - "net/smtp" -) - -type unencryptedAuth struct { - smtp.Auth -} - -// PlainAuth returns an Auth that implements the PLAIN authentication -// mechanism as defined in RFC 4616. The returned Auth uses the given -// username and password to authenticate to host and act as identity. -// Usually identity should be the empty string, to act as username. -// -// This reimplementation allows it to work over non-TLS connections - -func (a unencryptedAuth) Start(server *smtp.ServerInfo) (string, []byte, error) { - s := *server - s.TLS = true - return a.Auth.Start(&s) -} diff --git a/internal/notification/channels/smtp/plain_or_login_auth.go b/internal/notification/channels/smtp/plain_or_login_auth.go new file mode 100644 index 0000000000..8e9ca56e00 --- /dev/null +++ b/internal/notification/channels/smtp/plain_or_login_auth.go @@ -0,0 +1,57 @@ +package smtp + +import ( + "bytes" + "net/smtp" + "slices" + + "github.com/zitadel/zitadel/internal/zerrors" +) + +// golang net/smtp SMTP AUTH LOGIN or PLAIN Auth Handler +// Reference: https://gist.github.com/andelf/5118732?permalink_comment_id=4825669#gistcomment-4825669 + +func PlainOrLoginAuth(username, password, host string) smtp.Auth { + return &plainOrLoginAuth{username: username, password: password, host: host} +} + +type plainOrLoginAuth struct { + username string + password string + host string + authMethod string +} + +func (a *plainOrLoginAuth) Start(server *smtp.ServerInfo) (string, []byte, error) { + if server.Name != a.host { + return "", nil, zerrors.ThrowInternal(nil, "SMTP-RRi75", "wrong host name") + } + if !slices.Contains(server.Auth, "PLAIN") { + a.authMethod = "LOGIN" + return a.authMethod, nil, nil + } else { + a.authMethod = "PLAIN" + resp := []byte("\x00" + a.username + "\x00" + a.password) + return a.authMethod, resp, nil + } +} + +func (a *plainOrLoginAuth) Next(fromServer []byte, more bool) ([]byte, error) { + if !more { + return nil, nil + } + + if a.authMethod == "PLAIN" { + // We've already sent everything. + return nil, zerrors.ThrowInternal(nil, "SMTP-AAf43", "unexpected server challenge for PLAIN auth method") + } + + switch { + case bytes.Equal(fromServer, []byte("Username:")): + return []byte(a.username), nil + case bytes.Equal(fromServer, []byte("Password:")): + return []byte(a.password), nil + default: + return nil, zerrors.ThrowInternal(nil, "SMTP-HjW21", "unexpected server challenge") + } +}