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

Disable unnecessary OpenID/OAuth2 elements #18491

Merged
merged 3 commits into from
Feb 9, 2022

Conversation

pboguslawski
Copy link
Contributor

@pboguslawski pboguslawski commented Jan 31, 2022

This mod fixes disabling unnecessary OpenID/OAuth2 elements.

Related: #13129
Author-Change-Id: IB#1115256

This mod fixes disabling unnecessary OpenID elements.

Related: go-gitea#13129
Author-Change-Id: IB#1115256
m.Post("/link_account_signup", bindIgnErr(forms.RegisterForm{}), auth.LinkAccountPostRegister)
m.Group("/link_account", func() {
m.Get("", auth.LinkAccount)
}, openIDSignInEnabled)
Copy link
Contributor

Choose a reason for hiding this comment

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

openIDSignInEnabled can be added to the Get and Post arguments, no need to go all fancy with Group here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just tested that

m.Get("/", Home, openIDSignInEnabled)

does not forbid access to / when openIDSignInEnabled is disabled but

m.Group("/", func() {
		m.Get("", Home)
	}, openIDSignInEnabled)

does. Seems that simply adding openIDSignInEnabled to Get arguments does not work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

m.Get("/", openIDSignInEnabled, Home)

works ok - will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 31, 2022
Fixes: 8faceac
Related: go-gitea#18491 (review)
Author-Change-Id: IB#1115256
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 1, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 2, 2022
Copy link
Member

@lunny lunny left a comment

Choose a reason for hiding this comment

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

LinkAccount is not only for openID, OAuth2 could also use that. This is totally wrong.

@pboguslawski pboguslawski changed the title Disable unnecessary OpenID elements Disable unnecessary OpenID/OAuth2 elements Feb 2, 2022
@pboguslawski
Copy link
Contributor Author

LinkAccount is not only for openID, OAuth2 could also use that. This is totally wrong.

Fixed in 513303b.

@techknowlogick techknowlogick added this to the 1.17.0 milestone Feb 2, 2022
@zeripath zeripath merged commit c917f2d into go-gitea:main Feb 9, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 10, 2022
* giteaofficial/main:
  Fix issue with docker-rootless shimming script (go-gitea#18690)
  tests: remove redundant comparison in repo dump/restore (go-gitea#18660)
  [skip ci] Updated translations via Crowdin
  Disable unnecessary OpenID/OAuth2 elements (go-gitea#18491)
  Add apply-patch, basic revert and cherry-pick functionality (go-gitea#17902)
  C preprocessor colors improvement (go-gitea#18671)
  Update object repo with the migrated repository (go-gitea#18684)
@pboguslawski pboguslawski deleted the main-IB#1115256 branch February 11, 2022 10:29
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
This mod fixes disabling unnecessary OpenID elements.

Related: go-gitea#13129
Author-Change-Id: IB#1115256
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants