Skip to content

SMTP-Auth broken in gitea 1.18.0+rc0 #21744

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

Closed
smainz opened this issue Nov 9, 2022 · 24 comments · Fixed by #21767
Closed

SMTP-Auth broken in gitea 1.18.0+rc0 #21744

smainz opened this issue Nov 9, 2022 · 24 comments · Fixed by #21767
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug
Milestone

Comments

@smainz
Copy link

smainz commented Nov 9, 2022

Description

There are two problems with smtp auth in gitea:

  1. One can create an Authentication Source in the web UI, but you can not edit it by clicking the pencil button. On doing so the 500 page is shown and in the logs you get this
gitea1.18 | 2022/11/09 20:23:38 [636bfe33] router: completed GET /user/events for 87.123.115.202:0, 200 OK in 7182.9ms @ events/events.go:19(events.Events)
gitea1.18 | 2022/11/09 20:23:38 ...s/context/context.go:219:HTML() [D] [636bfe3a] Template: admin/auth/edit
gitea1.18 | 2022/11/09 20:23:38 ...s/context/context.go:232:HTML() [E] [636bfe3a] Render failed: template: admin/auth/edit:199:58: executing "admin/auth/edit" at <$cfg.Host>: can't evaluate field Host in type convert.Conversion
gitea1.18 | 2022/11/09 20:23:38 ...s/context/context.go:219:HTML() [D] [636bfe3a] Template: status/500
gitea1.18 | 2022/11/09 20:23:38 [636bfe3a] router: completed GET /admin/auths/1 for 87.123.115.202:0, 500 Internal Server Error in 7.2ms @ admin/auths.go:320(admin.EditAuthSource)
  1. Users can not authenticate using the smtp authentication source (even in an upgraded install where smtp-auth used to work flawlessly)
    When a user tries to log in, an empty page is shown.
    In the logs one can see this:
gitea1.18 | 2022/11/09 20:12:49 ...ers/web/auth/auth.go:221:SignInPost() [E] [636bfbb1] UserSignIn: dial tcp :587: connect: connection refused
gitea1.18 | 2022/11/09 20:12:49 [636bfbb1] router: completed POST /user/login for 87.123.115.202:0, 0  in 1.8ms @ auth/auth.go:177(auth.SignInPost)

I checked these things:

  • Problem happenes with a fresh install and my upgraded install
  • Problem does not happen with gitea 1.17.3 (fresh install and upgraded from previous versions)
  • Smtp-Server is reachable and can be used to authenticate (works for gitea 1.17.3)
  • Username and password are correct (used keepass to auto-type in 1.18-rc0 and 1.17.3)
  • no customization at all, just set up via docker, added authorization source, added user

Gitea Version

1.18-rc0 (docker image gitea:latest)

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

not relevant

Operating System

Linux (docker)

How are you running Gitea?

Used this docker-compose.yml

version: "3"

networks:
  gitea:
    external: false

services:
  server:
    image: gitea/gitea:latest
    container_name: gitea1.18
    environment:
      - USER_UID=1002
      - USER_GID=1002
    restart: always
    expose:
      - 22
      - 3000
    networks:
      - gitea
    volumes:
      - ./gitea:/data
#      - /home/git/.ssh:/data/git/.ssh
      - /etc/timezone:/etc/timezone:ro
      - /etc/localtime:/etc/localtime:ro
    ports:
      - "127.0.0.1:2222:22"

docker image ls:

REPOSITORY                  TAG           IMAGE ID       CREATED         SIZE
...
gitea/gitea                 latest        54e1c09fe090   2 weeks ago     255MB
...

Database

SQLite

@smainz smainz added the type/bug label Nov 9, 2022
@smainz smainz changed the title SMTP-Auth broen in gitea 1.18.0+rc0 SMTP-Auth broken in gitea 1.18.0+rc0 Nov 9, 2022
@techknowlogick
Copy link
Member

Thank you for this report. Which SMTP server are you using? I suspect it's an issue with Gitea itself, but still would be helpful in case of replicating issue.

@smainz
Copy link
Author

smainz commented Nov 9, 2022

I am using a selfhosted mailcow: https://mailcow.email

If you need an account for testing, I can provide one.

@picassochan
Copy link

same issue with office365 SMTP auth.

@wxiaoguang
Copy link
Contributor

I will propose a fix for it.

@wxiaoguang wxiaoguang added this to the 1.18.0 milestone Nov 10, 2022
@wxiaoguang wxiaoguang added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Nov 10, 2022
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 10, 2022

If you could build Gitea by yourselves, feel free to take a try for PRs:

  • 21767 (for 1.19-dev)
  • 21768 (for 1.18-rc)

@smainz
Copy link
Author

smainz commented Nov 10, 2022

Thanks for the fix. I will give it a try later today.

And praise to backups ...

techknowlogick pushed a commit that referenced this issue Nov 10, 2022
Backport #21767

The purpose of #18982 is to improve the SMTP mailer, but there were some
unrelated changes made to the SMTP auth in
d60c438

This PR reverts these unrelated changes, fix #21744

Co-authored-by: Lunny Xiao <[email protected]>
techknowlogick pushed a commit that referenced this issue Nov 10, 2022
The purpose of #18982 is to improve the SMTP mailer, but there were some
unrelated changes made to the SMTP auth in
d60c438

This PR reverts these unrelated changes, fix #21744
fsologureng pushed a commit to fsologureng/gitea that referenced this issue Nov 11, 2022
The purpose of go-gitea#18982 is to improve the SMTP mailer, but there were some
unrelated changes made to the SMTP auth in
go-gitea@d60c438

This PR reverts these unrelated changes, fix go-gitea#21744
@smainz
Copy link
Author

smainz commented Nov 12, 2022

I still owe a feedback: The change works for me.

Thank you

@PaulBol
Copy link

PaulBol commented Nov 14, 2022

Thanks for discovering and solving the issue. It would be great to try this fix. Will there be another 1.18.0 release candidate that I can install?

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Nov 14, 2022

Thanks for discovering and solving the issue. It would be great to try this fix. Will there be another 1.18.0 release candidate that I can install?

You could use the HEAD build from the 1.18 branch. At this stage it should be more stable than r0

@PaulBol
Copy link

PaulBol commented Nov 14, 2022

Thanks for the suggestion @eeyrjmr! I didn't know such builds were available but found them at https://dl.gitea.io/gitea/main now.

@smainz
Copy link
Author

smainz commented Nov 14, 2022

Does the HEAD of 1.18 correspond to this image: `gitea/gitea:1.18-dev?
So I do not have to run my home built image.

Thanks

@PaulBol
Copy link

PaulBol commented Nov 14, 2022

Wait, that's a good question! Maybe https://dl.gitea.io/gitea/1.18 is better? I imagine this is built from release/v1.18.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Nov 14, 2022

The merged PRs are in:

@PaulBol
Copy link

PaulBol commented Nov 24, 2022

Installed the latest version 1.18.0+rc0-29-g16772ffde from https://dl.gitea.io/gitea/1.18.

Sending a test email results in

Failed to send a testing email to '<address>': gomail: could not send email 1: failed to establish network connection to SMTP server: dial tcp :0: connect: connection refused

Does that mean I haven't received the fix for this issue or is anything else wrong?

@PaulBol
Copy link

PaulBol commented Nov 24, 2022

UPDATE: I was able to resolve the issue by replacing HOST = hostname:port with SMTP_ADDR and SMTP_PORT in gitea.ini.

@smainz
Copy link
Author

smainz commented Nov 25, 2022

@PaulBol Good you found the solution. These settings have been changed recently.

Just as a not as others might stumble upon this ussue to:

This issue does not relate to sending emails from gitea, but to authenticating users using an SMTP-Server as external authentication mechanism.

@zeripath
Copy link
Contributor

@PaulBol

UPDATE: I was able to resolve the issue by replacing HOST = hostname:port with SMTP_ADDR and SMTP_PORT in gitea.ini.

I don't understand why you needed to do this. The configuration should fallback to HOST if SMTP_ADDR is absent.

@PaulBol
Copy link

PaulBol commented Nov 26, 2022

@zeripath - I had made some other changes while trying to get the configuration to work. However, I double-checked that it boils done to HOST vs. SMTP_ADDR+SMTP_PORT.

Old mailer configuration - not working ("failed to establish network connection to SMTP server...")

[mailer]
ENABLED        = true
HOST           = mailout.provider.com:587
FROM           = [email protected]
USER           = <username>
PASSWD         = <password>

New mailer configuration - works

[mailer]
ENABLED        = true
SMTP_ADDR      = mailout.provider.com
SMTP_PORT      = 587
FROM           = [email protected]
USER           = <username>
PASSWD         = <password>

@PaulBol
Copy link

PaulBol commented Nov 26, 2022

I found that the Mailer Configuration box differs considerably.

Old
grafik

New
grafik

Doesn't this look like the settings aren't read correctly with a HOST value?

@zeripath
Copy link
Contributor

Ah! The fallbacks won't work due to a mistaken assumption by the author of the PR.

@PaulBol
Copy link

PaulBol commented Nov 26, 2022

Alright, I see. Thanks for checking. "startls" might be a typo, by the way.

@zeripath
Copy link
Contributor

yup I've just spotted that!

I'm just about to push up a PR fixing the deprecation fall backs.

zeripath added a commit to zeripath/gitea that referenced this issue Nov 26, 2022
Unfortunately the fallback configuration code for [mailer] that were
added in go-gitea#18982 are incorrect. When you read a value from an ini
section that key is added. This leads to a failure of the fallback
mechanism. Further there is also a spelling mistake in the startTLS
configuration.

This PR restructures the mailer code to first map the deprecated
settings on to the new ones - and then use ini.MapTo to map those
on to the struct with additional validation as necessary.

Ref go-gitea#21744

Signed-off-by: Andrew Thornton <[email protected]>
@PaulBol
Copy link

PaulBol commented Nov 26, 2022

Awesome how you identified and resolved the issue in 30 minutes or so. I hope to work on a Go project some day and learn how the language works.

@zeripath
Copy link
Contributor

It helps that I really do know this codebase - but assuming that you know any imperative language then go is extremely easy to learn.

zeripath added a commit that referenced this issue Nov 27, 2022
Unfortunately the fallback configuration code for [mailer] that were
added in #18982 are incorrect. When you read a value from an ini section
that key is added. This leads to a failure of the fallback mechanism.
Further there is also a spelling mistake in the startTLS configuration.

This PR restructures the mailer code to first map the deprecated
settings on to the new ones - and then use ini.MapTo to map those on to
the struct with additional validation as necessary.

Ref #21744

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit to zeripath/gitea that referenced this issue Nov 27, 2022
Backport go-gitea#21945

Unfortunately the fallback configuration code for [mailer] that were
added in go-gitea#18982 are incorrect. When you read a value from an ini section
that key is added. This leads to a failure of the fallback mechanism.
Further there is also a spelling mistake in the startTLS configuration.

This PR restructures the mailer code to first map the deprecated
settings on to the new ones - and then use ini.MapTo to map those on to
the struct with additional validation as necessary.

Ref go-gitea#21744

Signed-off-by: Andrew Thornton <[email protected]>
zeripath added a commit that referenced this issue Nov 27, 2022
Backport #21945

Unfortunately the fallback configuration code for [mailer] that were
added in #18982 are incorrect. When you read a value from an ini section
that key is added. This leads to a failure of the fallback mechanism.
Further there is also a spelling mistake in the startTLS configuration.

This PR restructures the mailer code to first map the deprecated
settings on to the new ones - and then use ini.MapTo to map those on to
the struct with additional validation as necessary.

Ref #21744

Signed-off-by: Andrew Thornton <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants