UX Optional Password & Captcha for External Registration

[coolaj86]

Re: #4226 and #3837

I see the primary purposes of external accounts as to increase security and convenience (which always go hand-in-hand).

If I have enabled external accounts, I should not require the user to weaken security by forcing them to create a (likely re-used) password.

Likewise, since I've already hand-picked the external account providers that I trust and want to allow, I should not reduce convenience by requiring a secondary captcha.

• Do not require password for external accounts via REQUIRE_EXTERNAL_REGISTRATION_PASSWORD
• Do not require password for external accounts via REQUIRE_EXTERNAL_REGISTRATION_CAPTCHA
• Only reject omitted passwords on forms that use a password
• Do not provide the option for a password when ALLOW_ONLY_EXTERNAL_REGISTRATION

Some other fixes that I want to get in, but may deserve separate PRs:

• Do not require logging in twice when creating account (PR UX treat register like sign in, even oauth #5033)
• Allow resetting (creating) password via email, even when logged in (PR allow current user to reset their own password #5034, Issue Password reset link does not work when logged in #5008)

If you'd like to test it out

My pull request is against master, but I run it as backport to v1.5.1 (includes #5006, #5029, #5033, #5034):

git clone https://github.com/coolaj86/gitea.git gitea.coolaj86 -b v1.5.1-coolaj86
pushd gitea.coolaj86
TAGS="bindata sqlite" make generate all

I would not recommend replacing your existing gitea, but rather creating a symlink so that you can easily switch back if you don't like it. For example, if you keep gitea in /opt/gitea/bin:

rsync -av ./gitea /opt/gitea/bin/gitea-v1.5.1-coolaj86
pushd /opt/gitea/bin
mv gitea gitea-v1.5.1
ln -s gitea-v1.5.1-coolaj86 gitea

I've run a couple of manual tests so far, so I feel comfortable with someone else trying it out. I won't be pushing any additional changes to that branch (such as the upcoming changes to address the empty checkboxes in the issue) until I've tested them in production for myself.

[codecov-io]

Codecov Report

Merging #5029 into master will increase coverage by <.01%.
The diff coverage is 19.51%.

@@            Coverage Diff             @@
##           master    #5029      +/-   ##
==========================================
+ Coverage   41.24%   41.24%   +<.01%     
==========================================
  Files         464      464              
  Lines       62867    62893      +26     
==========================================
+ Hits        25928    25939      +11     
- Misses      33548    33561      +13     
- Partials     3391     3393       +2

Impacted Files	Coverage Δ
modules/auth/user_form.go	42.85% <ø> (ø)	⬆️
modules/setting/service.go	79.16% <100%> (+0.9%)	⬆️
routers/user/auth.go	12.53% <15.38%> (+0.27%)	⬆️
models/gpg_key.go	55.83% <0%> (-0.84%)	⬇️
routers/repo/view.go	43.25% <0%> (+1.01%)	⬆️
models/unit.go	67.56% <0%> (+5.4%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 855ebbd...dd63dfc. Read the comment docs.

[lafriks]

What is reason REQUIRE_EXTERNAL_REGISTRATION_CAPTCHA why no just reuse captcha setting that is already present?

[coolaj86]

My view is that Captcha is the kind of thing you implement for local logins, not the kind of thing you enforce on delegated logins that you already trust, which already do that type of validation.

For my instance (and I think most people would feel this way), I want the greatest protection from bots with the least friction for new users and I think local-only captcha strikes the right balance.

[coolaj86]

(that said, some people may want it for some reason, so I didn’t want to nix it entirely)

[strk]

Is "OpenID" considered "External Registration" ? Sorry but I'm a bit confused about the state of oAuth2 vs. OpenID(2) implementations

[coolaj86]

TL;DR

• OpenID(1): No change introduced by this PR
• OpenID(2)/OIDC: Same changes as OAuth2

Explanation

I haven't changed the behavior of OpenID(1) and it follows a different flow from OAuth2 in the code.

As such I don't think that these changes affect the OpenID(1) flow. I don't want to pursue changing the OpenID(1) flow as part of this PR (it was never very well adopted and doesn't provide a lot of value in my opinion), but I'd be open to updating it as well later on.

OpenID(2) aka OIDC aka OpenID Connect is actually not related to OpenID in any way. It's a strict subset of OAuth2 that actually defines important things like data format (JSON vs urlencoded), endpoint URLs, etc. It's not very well designed for personal use, in my opinion, so it doesn't really meet the original goal of OpenID (IndieAuth is probably better for that), but it is far better than OAuth2 for enterprise-style use and maintains perfect OAuth2 compatibility (which isn't hard seeing how OAuth2 doesn't define any of those things so there's nothing much to be "compatible" with other than the generic concept of "there's some handshakes").

[stale]

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.

[nickho]

Thanks for working on this @solderjs !
I tested your PR on a setup with :

• local VM on Ubuntu and gitea on https
• our corporate AzureAD as an OIDC provider

The workflow is working fine for me. Clicking the link "Connect With OpenId Connect" on the login page redirect to our AzureAD that redirect to the "Add Account Recovery Info". If i enter a username, then everything is working fine inside Gitea.

I do confirm ALLOW_ONLY_EXTERNAL_REGISTRATION is working, but you need to also to put DISABLE_REGISTRATION to false.

I think something is missaligned on the UI but is it an issue with #5006 ?

Keep up the good work.

[stale]

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.

[strk]

Is this work still ongoing ? I'd love to see OpenID-Connect with discovery enabled on try.gitea.io ...

[lunny]

Please resolve the conflict

[zeripath]

@lunny @solderjs conflict resolved.

[techknowlogick]

Suggested change

	Service.RequireExternalRegistrationCaptcha = sec.Key("REQUIRE_EXTERNAL_REGISTRATION_CAPTCHA").MustBool()
	Service.RequireExternalRegistrationCaptcha = sec.Key("REQUIRE_EXTERNAL_REGISTRATION_CAPTCHA").MustBool(Service.EnableCaptcha)

[lunny]

replaced by #6606