Skip to content

Use hostmatcher for openid allow/block list. Disable OpenID by default and added default black list for OpenID#36729

Open
lunny wants to merge 7 commits intogo-gitea:mainfrom
lunny:lunny/disable_openid_by_default
Open

Use hostmatcher for openid allow/block list. Disable OpenID by default and added default black list for OpenID#36729
lunny wants to merge 7 commits intogo-gitea:mainfrom
lunny:lunny/disable_openid_by_default

Conversation

@lunny
Copy link
Copy Markdown
Member

@lunny lunny commented Feb 24, 2026

  • Move OpenID settings from service section, and switches allow/block logic to hostmatcher
    with a default blocklist for localhost/private/link-local addresses to prevent SSRF (modules/setting/openid.go:12).
  • Disable OpenID SignUp/SignIn by default
  • Updates OpenID login flow to use the new allowlist/blocklist checks and wires routes/templates to the new settings
    (routers/web/auth/openid.go:47).
  • Refreshes the example config comments to describe host matcher behavior and default values (custom/conf/app.example.ini:1638).
  • Adds tests for OpenID settings and OpenID web flow (modules/setting/openid_test.go:1, routers/web/auth/openid_test.go:1).

Partially generated by Codex 5.2

@lunny lunny added the type/bug label Feb 24, 2026
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 24, 2026
@github-actions github-actions bot added modifies/go Pull requests that update Go code docs-update-needed The document needs to be updated synchronously labels Feb 24, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

The install lock problem is caused by #36641. Will be fixed by #36735

@wxiaoguang wxiaoguang marked this pull request as draft February 24, 2026 09:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors OpenID authentication settings and introduces security improvements by disabling OpenID by default and adding a default blocklist to prevent SSRF attacks. The changes migrate OpenID-related settings from the Service struct to a dedicated OpenID struct, replace POSIX regex-based URI filtering with host-based matching using the hostmatcher package, and establish default security protections for OpenID provider URIs.

Changes:

  • Disabled OpenID sign-in by default (changed from enabled during installation to explicitly disabled)
  • Introduced a default blocklist (localhost, loopback, private networks, IPv6 link-local) to prevent SSRF attacks
  • Migrated OpenID settings from setting.Service to dedicated setting.OpenID struct with allowlist/blocklist using hostmatcher.HostMatchList

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
modules/setting/openid.go New file implementing dedicated OpenID settings module with default SSRF-preventing blocklist and hostmatcher-based allow/blocklists
modules/setting/openid_test.go Test coverage for OpenID settings loading including default blocklist, allowlist, and custom blocklist scenarios
modules/setting/service.go Removed OpenID-related fields and loadOpenIDSetting function from Service struct (moved to dedicated module)
routers/web/auth/openid.go Updated URI validation to use hostname-based matching via hostmatcher instead of regex, fixed terminology (whitelist→allowlist, blacklist→blocklist)
routers/web/auth/openid_test.go Test coverage for allowedOpenIDURI function covering allowlist priority, blocklist filtering, and invalid URI handling
routers/web/web.go Updated middleware to reference setting.OpenID.EnableSignIn and setting.OpenID.EnableSignUp
routers/install/install.go Updated install handler to read/write OpenID settings from new setting.OpenID location
modules/web/middleware/data.go Updated common template context data to use setting.OpenID.EnableSignIn
services/auth/sspi.go Updated SSPI authentication to reference setting.OpenID.EnableSignIn
custom/conf/app.example.ini Updated documentation for OpenID settings including new default blocklist, changed default value, and improved descriptions
Comments suppressed due to low confidence (3)

custom/conf/app.example.ini:1656

  • Typo in comment: "rhw" should be "the". The sentence should read "Do not include to rely on the DISABLE_REGISTRATION setting".
;; Do not include to rely on rhw DISABLE_REGISTRATION setting

routers/web/auth/openid.go:68

  • Typo in comment: "expliclty" should be "explicitly".
	// A blocklist match expliclty forbids

routers/web/auth/openid_test.go:1

  • Copyright year should be 2026 instead of 2024 to match the current year and other newly added files in this codebase.
// Copyright 2024 The Gitea Authors. All rights reserved.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lunny lunny added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Mar 1, 2026
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Mar 1, 2026
@lunny lunny marked this pull request as ready for review March 1, 2026 21:18
@lunny lunny changed the title Disable OpenID by default. Added default black list for OpenID Use hostmatcher for openid allow/block list. Disable OpenID by default and added default black list for OpenID Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-update-needed The document needs to be updated synchronously lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants