From 623bb81bb95ee5738197955850df308cc1f45528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20G=C3=B3ra?= <108656216+dawidgora@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:30:30 +0200 Subject: [PATCH 1/2] fix(releases): generate notes for initial tag (#37697) Fixes https://github.com/go-gitea/gitea/issues/37286 Automatic release notes for the first release in a repository were empty when there was no previous tag. Before this change, the release notes generator used the tag name to build the changelog link, but reused that state for pull request collection. When `PreviousTag` was empty, the PR collection logic did not scan a useful commit range, so merged pull requests were omitted from the generated notes. This pull request fixes that by decoupling the internal PR collection range from the rendered changelog link: - when a previous tag exists, behavior stays unchanged - when no previous tag exists, release notes collect merged pull requests from the full reachable history up to the target tag - the displayed full changelog link for the first release still uses the existing `/commits/tag/{tag}` format Tests were updated to cover: - generating notes for a repository with no previous tags - including merged pull requests before the first tag - preserving existing behavior when a previous tag exists --- services/release/notes.go | 49 ++++++++++++++++++++++------- services/release/notes_test.go | 56 ++++++++++++++++++++++++++++------ 2 files changed, 84 insertions(+), 21 deletions(-) diff --git a/services/release/notes.go b/services/release/notes.go index b8baeb9620..e62335c98b 100644 --- a/services/release/notes.go +++ b/services/release/notes.go @@ -10,6 +10,7 @@ import ( "slices" "strings" + "gitea.dev/models/db" issues_model "gitea.dev/models/issues" repo_model "gitea.dev/models/repo" user_model "gitea.dev/models/user" @@ -32,18 +33,23 @@ func GenerateReleaseNotes(ctx context.Context, repo *repo_model.Repository, gitR return "", err } - if opts.PreviousTag == "" { - // no previous tag, usually due to there is no tag in the repo, use the same content as GitHub - content := fmt.Sprintf("**Full Changelog**: %s/commits/tag/%s\n", repo.HTMLURL(ctx), util.PathEscapeSegments(opts.TagName)) - return content, nil + isFirstRelease, err := isFirstRelease(ctx, repo.ID) + if err != nil { + return "", fmt.Errorf("isFirstRelease: %w", err) } - baseCommit, err := gitRepo.GetCommit(opts.PreviousTag) - if err != nil { + baseCommitID := "" + if opts.PreviousTag != "" { + baseCommit, err := gitRepo.GetCommit(opts.PreviousTag) + if err != nil { + return "", util.ErrorWrapTranslatable(util.ErrNotExist, "repo.release.generate_notes_tag_not_found", opts.TagName) + } + baseCommitID = baseCommit.ID.String() + } else if !isFirstRelease { return "", util.ErrorWrapTranslatable(util.ErrNotExist, "repo.release.generate_notes_tag_not_found", opts.TagName) } - commits, err := gitRepo.CommitsBetweenIDs(headCommit.ID.String(), baseCommit.ID.String()) + commits, err := gitRepo.CommitsBetweenIDs(headCommit.ID.String(), baseCommitID) if err != nil { return "", fmt.Errorf("CommitsBetweenIDs: %w", err) } @@ -58,10 +64,27 @@ func GenerateReleaseNotes(ctx context.Context, repo *repo_model.Repository, gitR return "", err } - content := buildReleaseNotesContent(ctx, repo, opts.TagName, opts.PreviousTag, prs, contributors, newContributors) + fullChangelogURL := "" + if isFirstRelease { + // Keep the first-release changelog link aligned with GitHub, while collecting PRs from full history. + fullChangelogURL = fmt.Sprintf("%s/commits/tag/%s", repo.HTMLURL(ctx), util.PathEscapeSegments(opts.TagName)) + } + + content := buildReleaseNotesContent(ctx, repo, opts.TagName, opts.PreviousTag, prs, contributors, newContributors, fullChangelogURL) return content, nil } +func isFirstRelease(ctx context.Context, repoID int64) (bool, error) { + count, err := db.Count[repo_model.Release](ctx, repo_model.FindReleasesOptions{ + RepoID: repoID, + IncludeDrafts: false, + }) + if err != nil { + return false, err + } + return count == 0, nil +} + func resolveHeadCommit(gitRepo *git.Repository, tagName, tagTarget string) (*git.Commit, error) { ref := tagName if !gitRepo.IsTagExist(tagName) { @@ -107,7 +130,7 @@ func collectPullRequestsFromCommits(ctx context.Context, repoID int64, commits [ return prs, nil } -func buildReleaseNotesContent(ctx context.Context, repo *repo_model.Repository, tagName, baseRef string, prs []*issues_model.PullRequest, contributors []*user_model.User, newContributors []*issues_model.PullRequest) string { +func buildReleaseNotesContent(ctx context.Context, repo *repo_model.Repository, tagName, baseRef string, prs []*issues_model.PullRequest, contributors []*user_model.User, newContributors []*issues_model.PullRequest, fullChangelogURL string) string { var builder strings.Builder builder.WriteString("## What's Changed\n") @@ -136,8 +159,12 @@ func buildReleaseNotesContent(ctx context.Context, repo *repo_model.Repository, } builder.WriteString("**Full Changelog**: ") - compareURL := fmt.Sprintf("%s/compare/%s...%s", repo.HTMLURL(ctx), util.PathEscapeSegments(baseRef), util.PathEscapeSegments(tagName)) - fmt.Fprintf(&builder, "[%s...%s](%s)", baseRef, tagName, compareURL) + if fullChangelogURL != "" { + builder.WriteString(fullChangelogURL) + } else { + compareURL := fmt.Sprintf("%s/compare/%s...%s", repo.HTMLURL(ctx), util.PathEscapeSegments(baseRef), util.PathEscapeSegments(tagName)) + fmt.Fprintf(&builder, "[%s...%s](%s)", baseRef, tagName, compareURL) + } builder.WriteByte('\n') return builder.String() } diff --git a/services/release/notes_test.go b/services/release/notes_test.go index 2922da424b..3fb3b7c553 100644 --- a/services/release/notes_test.go +++ b/services/release/notes_test.go @@ -21,13 +21,14 @@ import ( func TestGenerateReleaseNotes(t *testing.T) { unittest.PrepareTestEnv(t) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - gitRepo, err := gitrepo.OpenRepository(t.Context(), repo) - require.NoError(t, err) - t.Run("ChangeLogsWithPRs", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + gitRepo, err := gitrepo.OpenRepository(t.Context(), repo) + require.NoError(t, err) + t.Cleanup(func() { gitRepo.Close() }) + mergedCommit := "90c1019714259b24fb81711d4416ac0f18667dfa" - createMergedPullRequest(t, repo, mergedCommit, 5) + createMergedPullRequest(t, repo, mergedCommit, 5, "Release notes test pull request") content, err := GenerateReleaseNotes(t.Context(), repo, gitRepo, GenerateReleaseNotesOptions{ TagName: "v1.2.0", @@ -50,16 +51,51 @@ func TestGenerateReleaseNotes(t *testing.T) { }) t.Run("NoPreviousTag", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 16}) + gitRepo, err := gitrepo.OpenRepository(t.Context(), repo) + require.NoError(t, err) + t.Cleanup(func() { gitRepo.Close() }) + + createMergedPullRequest(t, repo, "69554a64c1e6030f051e5c3f94bfbd773cd6a324", 5, "Initial tag PR 1") + createMergedPullRequest(t, repo, "27566bd5738fc8b4e3fef3c5e72cce608537bd95", 4, "Initial tag PR 2") + createMergedPullRequest(t, repo, "5099b81332712fe655e34e8dd63574f503f61811", 8, "Initial tag PR 3") + content, err := GenerateReleaseNotes(t.Context(), repo, gitRepo, GenerateReleaseNotesOptions{ - TagName: "v1.2.0", - TagTarget: "DefaultBranch", + TagName: "v0.1.0", + TagTarget: repo.DefaultBranch, }) require.NoError(t, err) - assert.Equal(t, "**Full Changelog**: https://try.gitea.io/user2/repo1/commits/tag/v1.2.0\n", content) + + assert.Contains(t, content, "## What's Changed\n") + assert.Contains(t, content, "* Initial tag PR 1 in [#") + assert.Contains(t, content, "* Initial tag PR 2 in [#") + assert.Contains(t, content, "* Initial tag PR 3 in [#") + assert.Contains(t, content, "\n## Contributors\n") + assert.Contains(t, content, "* @user5\n") + assert.Contains(t, content, "* @user4\n") + assert.Contains(t, content, "* @user8\n") + assert.Contains(t, content, "\n## New Contributors\n") + assert.Contains(t, content, "* @user5 made their first contribution in [#") + assert.Contains(t, content, "* @user4 made their first contribution in [#") + assert.Contains(t, content, "* @user8 made their first contribution in [#") + assert.Contains(t, content, "**Full Changelog**: https://try.gitea.io/user2/repo16/commits/tag/v0.1.0\n") + }) + + t.Run("EmptyPreviousTagWithExistingTags", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + gitRepo, err := gitrepo.OpenRepository(t.Context(), repo) + require.NoError(t, err) + t.Cleanup(func() { gitRepo.Close() }) + + _, err = GenerateReleaseNotes(t.Context(), repo, gitRepo, GenerateReleaseNotesOptions{ + TagName: "v1.2.0", + TagTarget: repo.DefaultBranch, + }) + require.Error(t, err) }) } -func createMergedPullRequest(t *testing.T, repo *repo_model.Repository, mergeCommit string, posterID int64) *issues_model.PullRequest { +func createMergedPullRequest(t *testing.T, repo *repo_model.Repository, mergeCommit string, posterID int64, title string) *issues_model.PullRequest { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: posterID}) issue := &issues_model.Issue{ @@ -67,7 +103,7 @@ func createMergedPullRequest(t *testing.T, repo *repo_model.Repository, mergeCom Repo: repo, Poster: user, PosterID: user.ID, - Title: "Release notes test pull request", + Title: title, Content: "content", } From 735e940a6148b7cbe0ee58ed1508856e84a82327 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Wed, 3 Jun 2026 18:50:47 +0200 Subject: [PATCH 2/2] fix(oauth2): not respecting claims before second login (#37874) fixes defect where claims where only applies on login but not during account linking making only the second login take them into account fixes: https://github.com/go-gitea/gitea/issues/32566 --- routers/web/auth/oauth.go | 7 - routers/web/auth/oauth_signin_sync.go | 9 ++ tests/integration/auth_oauth2_test.go | 203 ++++++++++++++++++++++++-- 3 files changed, 196 insertions(+), 23 deletions(-) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 7bed3523ed..f7d9c3c34a 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -188,10 +188,6 @@ func SignInOAuthCallback(ctx *context.Context) { source := authSource.Cfg.(*oauth2.Source) - isAdmin, isRestricted := getUserAdminAndRestrictedFromGroupClaims(source, &gothUser) - u.IsAdmin = isAdmin.ValueOrDefault(user_service.UpdateOptionField[bool]{FieldValue: false}).FieldValue - u.IsRestricted = isRestricted.ValueOrDefault(setting.Service.DefaultUserIsRestricted) - linkAccountData := &LinkAccountData{authSource.ID, gothUser} if setting.OAuth2Client.AccountLinking == setting.OAuth2AccountLinkingDisabled { linkAccountData = nil @@ -373,9 +369,6 @@ func handleOAuth2SignIn(ctx *context.Context, authSource *auth.Source, u *user_m opts.IsActive = optional.Some(true) } - // Update GroupClaims - opts.IsAdmin, opts.IsRestricted = getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) - if oauth2Source.GroupTeamMap != "" || oauth2Source.GroupTeamMapRemoval { if err := source_service.SyncGroupsToTeams(ctx, u, groups, groupTeamMapping, oauth2Source.GroupTeamMapRemoval); err != nil { ctx.ServerError("SyncGroupsToTeams", err) diff --git a/routers/web/auth/oauth_signin_sync.go b/routers/web/auth/oauth_signin_sync.go index a939a0e71e..f5ec66e006 100644 --- a/routers/web/auth/oauth_signin_sync.go +++ b/routers/web/auth/oauth_signin_sync.go @@ -14,6 +14,7 @@ import ( asymkey_service "gitea.dev/services/asymkey" "gitea.dev/services/auth/source/oauth2" "gitea.dev/services/context" + user_service "gitea.dev/services/user" "github.com/markbates/goth" ) @@ -50,6 +51,14 @@ func oauth2SignInSync(ctx *context.Context, authSourceID int64, u *user_model.Us } } + // sync user flags (admin/restricted) + isAdmin, isRestricted := getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) + if isAdmin.Has() || isRestricted.Has() { + if err = user_service.UpdateUser(ctx, u, &user_service.UpdateOptions{IsAdmin: isAdmin, IsRestricted: isRestricted}); err != nil { + log.Error("Unable to sync OAuth2 user admin or restricted status %s: %v", gothUser.Provider, err) + } + } + err = oauth2UpdateSSHPubIfNeed(ctx, authSource, &gothUser, u) if err != nil { log.Error("Unable to sync OAuth2 SSH public key %s: %v", gothUser.Provider, err) diff --git a/tests/integration/auth_oauth2_test.go b/tests/integration/auth_oauth2_test.go index bcd4981149..3bb3d56e7b 100644 --- a/tests/integration/auth_oauth2_test.go +++ b/tests/integration/auth_oauth2_test.go @@ -54,7 +54,7 @@ func TestMigrateAzureADV2ToOIDC(t *testing.T) { ) // The fake OIDC server issues tokens containing both sub and oid claims, mirroring what Azure AD v2.0 returns. - srv := newFakeOIDCServer(t, subValue, oidValue) + srv := newFakeOIDCServer(t, FakeOIDCConfig{Sub: subValue, OID: oidValue}) // --- Step 1: Establish the legacy Azure AD V2 state --- // Create an azureadv2 auth source. In production this would have been the source used before the migration. @@ -138,7 +138,7 @@ func TestOIDCIgnoresStaleExternalLoginLinks(t *testing.T) { setup := func(t *testing.T, sourceName, sub, userName, email string) (*auth_model.Source, *user_model.User) { t.Helper() - srv := newFakeOIDCServerWithProfile(t, sub, sub+"-oid", email, "OIDC Test User") + srv := newFakeOIDCServer(t, FakeOIDCConfig{Sub: sub, OID: sub + "-oid", Email: email, Name: "OIDC Test User"}) addOAuth2Source(t, sourceName, oauth2.Source{ Provider: "openidConnect", ClientID: "test-client-id", @@ -191,14 +191,27 @@ func TestOIDCIgnoresStaleExternalLoginLinks(t *testing.T) { }) } -// newFakeOIDCServer starts an httptest.Server that implements the minimum OIDC endpoints needed to complete a sign-in flow: -func newFakeOIDCServer(t *testing.T, sub, oid string) *httptest.Server { - return newFakeOIDCServerWithProfile(t, sub, oid, sub+"@example.com", "OIDC Test User") +// FakeOIDCConfig holds configuration for the fake OIDC server used in tests. +type FakeOIDCConfig struct { + Sub string + OID string + Email string + Name string + Groups []string } -func newFakeOIDCServerWithProfile(t *testing.T, sub, oid, email, name string) *httptest.Server { +// newFakeOIDCServer starts a httptest.Server that implements the minimum OIDC endpoints needed to complete a sign-in flow +func newFakeOIDCServer(t *testing.T, cfg FakeOIDCConfig) *httptest.Server { t.Helper() + // Set defaults for backward compatibility with existing tests + if cfg.Email == "" { + cfg.Email = cfg.Sub + "@example.com" + } + if cfg.Name == "" { + cfg.Name = "OIDC Test User" + } + var srv *httptest.Server srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -212,11 +225,18 @@ func newFakeOIDCServerWithProfile(t *testing.T, sub, oid, email, name string) *h }) case "/token": // returns an ID token with both "sub" and "oid" claims so tests can verify which one ends up as ExternalID claims := map[string]any{ - "iss": srv.URL, - "aud": "test-client-id", - "exp": time.Now().Add(time.Hour).Unix(), - "sub": sub, - "oid": oid, + "iss": srv.URL, + "aud": "test-client-id", + "exp": time.Now().Add(time.Hour).Unix(), + "sub": cfg.Sub, + "email": cfg.Email, + "name": cfg.Name, + } + if cfg.OID != "" { + claims["oid"] = cfg.OID + } + if cfg.Groups != nil { + claims["groups"] = cfg.Groups } payload, _ := json.Marshal(claims) header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"none"}`)) @@ -232,11 +252,18 @@ func newFakeOIDCServerWithProfile(t *testing.T, sub, oid, email, name string) *h }) case "/userinfo": // sub MUST match the id_token sub; goth rejects mismatches. - _ = json.NewEncoder(w).Encode(map[string]any{ - "sub": sub, - "email": email, - "name": name, - }) + response := map[string]any{ + "sub": cfg.Sub, + "email": cfg.Email, + "name": cfg.Name, + } + if cfg.OID != "" { + response["oid"] = cfg.OID + } + if cfg.Groups != nil { + response["groups"] = cfg.Groups + } + _ = json.NewEncoder(w).Encode(response) default: http.NotFound(w, r) } @@ -264,3 +291,147 @@ func doOIDCSignIn(t *testing.T, sourceName string) { callbackURL := fmt.Sprintf("/user/oauth2/%s/callback?code=test-code&state=%s", sourceName, url.QueryEscape(state)) session.MakeRequest(t, NewRequest(t, "GET", callbackURL), http.StatusSeeOther) } + +// newOIDCSource is a helper function to create a configured OAuth2 source for testing +func newOIDCSource(srv *httptest.Server, withAdmin, withRestricted bool) oauth2.Source { + src := oauth2.Source{ + Provider: "openidConnect", + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + OpenIDConnectAutoDiscoveryURL: srv.URL + "/.well-known/openid-configuration", + GroupClaimName: "groups", + } + if withAdmin { + src.AdminGroup = "admins" + } + if withRestricted { + src.RestrictedGroup = "restricted-users" + } + return src +} + +// TestOAuth2GroupClaimsAppliedOnFirstLogin verifies that group claims from OAuth2/OIDC +// are correctly applied to newly created users on the first login +func TestOAuth2GroupClaimsAppliedOnFirstLogin(t *testing.T) { + defer tests.PrepareTestEnv(t)() + // Enable auto-registration to ensure first login creates user with group claims + defer test.MockVariableValue(&setting.OAuth2Client.EnableAutoRegistration, true)() + // Use sub claim as username for deterministic user naming + defer test.MockVariableValue(&setting.OAuth2Client.Username, setting.OAuth2UsernameUserid)() + + tt := []struct { + Name string + IsAdmin bool + IsRestricted bool + SourceName string + }{ + { + Name: "user in both admin and restricted groups", + IsAdmin: true, + IsRestricted: true, + SourceName: "test-group-claims", + }, + { + Name: "no groups", + IsAdmin: false, + IsRestricted: false, + SourceName: "test-no-groups", + }, + } + for _, tc := range tt { + t.Run(tc.Name, func(t *testing.T) { + // Set up OIDC server with group claims + srv := newFakeOIDCServer(t, FakeOIDCConfig{ + Sub: tc.SourceName, + Email: tc.SourceName + "@example.com", + Name: "Test User", + Groups: []string{"admins", "restricted-users"}, + }) + + // Ensure it's the first login so no user in database + unittest.AssertNotExistsBean(t, &user_model.User{Name: tc.SourceName}) + + addOAuth2Source(t, tc.SourceName, newOIDCSource(srv, tc.IsAdmin, tc.IsRestricted)) + + doOIDCSignIn(t, tc.SourceName) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: tc.SourceName}) + assert.Equal(t, tc.IsAdmin, user.IsAdmin) + assert.Equal(t, tc.IsRestricted, user.IsRestricted) + assert.Equal(t, auth_model.OAuth2, user.LoginType) + }) + } +} + +// TestOAuth2GroupClaimsManualLinking tests that group claims are applied correctly +// when a user goes through the manual linking flow (auto-registration disabled). +func TestOAuth2GroupClaimsManualLinking(t *testing.T) { + defer tests.PrepareTestEnv(t)() + // Disable auto-registration to force manual linking flow + defer test.MockVariableValue(&setting.OAuth2Client.EnableAutoRegistration, false)() + defer test.MockVariableValue(&setting.Service.AllowOnlyInternalRegistration, false)() + + tt := []struct { + Name string + IsAdmin bool + IsRestricted bool + SourceName string + }{ + { + Name: "user in both admin and restricted groups", + IsAdmin: true, + IsRestricted: true, + SourceName: "test-group-claims-manual-linking", + }, + { + Name: "no groups", + IsAdmin: false, + IsRestricted: false, + SourceName: "test-no-groups-manual-linking", + }, + } + + for _, tc := range tt { + t.Run(tc.Name, func(t *testing.T) { + srv := newFakeOIDCServer(t, FakeOIDCConfig{ + Sub: tc.SourceName, + Email: tc.SourceName + "@example.com", + Name: "Manual User", + Groups: []string{"admins", "restricted-users"}, + }) + addOAuth2Source(t, tc.SourceName, newOIDCSource(srv, tc.IsAdmin, tc.IsRestricted)) + unittest.AssertNotExistsBean(t, &user_model.User{Name: tc.SourceName}) + session := emptyTestSession(t) + resp := session.MakeRequest(t, NewRequest(t, "GET", "/user/oauth2/"+tc.SourceName), http.StatusTemporaryRedirect) + + location := resp.Header().Get("Location") + u, err := url.Parse(location) + require.NoError(t, err) + state := u.Query().Get("state") + require.NotEmpty(t, state, "redirect to OIDC provider must include state") + + callbackURL := fmt.Sprintf("/user/oauth2/%s/callback?code=test-code&state=%s", tc.SourceName, url.QueryEscape(state)) + session.MakeRequest(t, NewRequest(t, "GET", callbackURL), http.StatusSeeOther) + + // Submit the form to create a new account + linkAccountResp := session.MakeRequest(t, NewRequest(t, "GET", "/user/link_account"), http.StatusOK) + // Verify we're on the link account page + assert.Contains(t, linkAccountResp.Body.String(), "link_account") + + // Use NewRequestWithValues to POST form data (no CSRF needed in tests) + // Field names are lowercase in HTML forms: user_name, email, password, retype + req := NewRequestWithValues(t, "POST", "/user/link_account_signup", map[string]string{ + "user_name": tc.SourceName, + "email": tc.SourceName + "@example.com", + "password": "", // AllowOnlyExternalRegistration means no password needed + "retype": "", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: tc.SourceName}) + assert.Equal(t, tc.IsAdmin, user.IsAdmin) + assert.Equal(t, tc.IsRestricted, user.IsRestricted) + assert.Equal(t, auth_model.OAuth2, user.LoginType) + }) + } +}