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

SignIn form disabled when reverse proxy auth is enabled #18601

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pboguslawski
Copy link
Contributor

SignIn form should not be enabled when users are authenticated
with reverse proxy.

Author-Change-Id: IB#1115398

SignIn form should not be enabled when users are authenticated
with reverse proxy.

Author-Change-Id: IB#1115398
routers/web/web.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 4, 2022
Co-authored-by: silverwind <me@silverwind.io>
@stale
Copy link

stale bot commented Apr 17, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Apr 17, 2022
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Apr 17, 2022
@stale stale bot removed the issue/stale label Apr 17, 2022
pboguslawski added a commit to ibpl/gitea that referenced this pull request Oct 5, 2022
Disabled registration notification e-mail in reverse proxy mode to avoid
spamming with invalid instructions (such accounts are created by admin
in exteral systems not during self registration and no gitea passwords
are used for auth).

Related: go-gitea#18601
Author-Change-Id: IB#1122610
@lunny lunny added the type/enhancement An improvement of existing functionality label Mar 27, 2023
@lunny lunny added this to the 1.20.0 milestone Mar 27, 2023
@@ -240,6 +240,9 @@ func NewFuncMap() []template.FuncMap {
"DisableImportLocal": func() bool {
return !setting.ImportLocalPaths
},
"DisableReverseProxyAuth": func() bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's better to use EnableReverseProxyAuth?

@pboguslawski
Copy link
Contributor Author

DisableGitHooks
DisableWebhooks
DisableImportLocal
...so why not DisableReverseProxyAuth?

@wxiaoguang
Copy link
Contributor

As a personal/private usage, these changes are good.

As a general feature for all users, I am not sure whether these changes are complete.

For example, there are other handlers besides "login": "openid", "sign_up", "oauth2", etc.

To make it a general feature for all users (avoid any misuse or risk), I think there should be a whole design/plan for the Account System first.

Just my personal opinion, correct me if I was wrong.

@pboguslawski
Copy link
Contributor Author

As a personal/private usage, these changes are good.
As a general feature for all users, I am not sure whether these changes are complete.

Disabling unused auth stuff is good for professional usage also IHMO.

To make it a general feature for all users (avoid any misuse or risk), I think there should be a whole design/plan for the Account System first.

Gitea should implement pluging-based auth system and plugin-based authz system IHMO and not focus on specific auth soluition (like tokens/passwords, etc.). Separate auth/authz plugin config for front and API probably if separate set of APIs is necessary. Not easy to implement probably so consider spliiting this idea to separate thread to discuss and implement. This mod would be obsolete if signing screen was part of "password auth plugin".

@wxiaoguang wxiaoguang removed this from the 1.20.0 milestone May 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants