From c69e8cde7a5f0512d668ca73c5fc85cabf716d7e Mon Sep 17 00:00:00 2001 From: Michael Jerger Date: Mon, 7 Jul 2025 14:29:57 +0200 Subject: [PATCH 1/2] chore(ci): temporarily disable flaky ActivityPub related tests (#8395) As discussed in forgejo/forgejo#8274 A workaround till analysis of test timeouts helped us to fix the root cause. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8395 Reviewed-by: Earl Warren Co-authored-by: Michael Jerger Co-committed-by: Michael Jerger --- .../integration/api_activitypub_actor_test.go | 77 -------- .../api_activitypub_person_test.go | 114 ------------ .../api_activitypub_repository_test.go | 174 ------------------ .../api_federation_httpsig_test.go | 82 --------- tests/integration/repo_settings_test.go | 65 ------- 5 files changed, 512 deletions(-) delete mode 100644 tests/integration/api_activitypub_actor_test.go delete mode 100644 tests/integration/api_activitypub_person_test.go delete mode 100644 tests/integration/api_activitypub_repository_test.go delete mode 100644 tests/integration/api_federation_httpsig_test.go diff --git a/tests/integration/api_activitypub_actor_test.go b/tests/integration/api_activitypub_actor_test.go deleted file mode 100644 index 42232bd640..0000000000 --- a/tests/integration/api_activitypub_actor_test.go +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2024 The Forgejo Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package integration - -import ( - "fmt" - "net/http" - "net/url" - "strconv" - "testing" - - "forgejo.org/modules/forgefed" - "forgejo.org/modules/setting" - "forgejo.org/modules/test" - "forgejo.org/routers" - "forgejo.org/services/contexttest" - "forgejo.org/services/federation" - "forgejo.org/tests" - - ap "github.com/go-ap/activitypub" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestActivityPubActor(t *testing.T) { - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - defer tests.PrepareTestEnv(t)() - - req := NewRequest(t, "GET", "/api/v1/activitypub/actor") - resp := MakeRequest(t, req, http.StatusOK) - assert.Contains(t, resp.Body.String(), "@context") - - var actor ap.Actor - err := actor.UnmarshalJSON(resp.Body.Bytes()) - require.NoError(t, err) - - assert.Equal(t, ap.ApplicationType, actor.Type) - assert.Equal(t, "ghost", actor.PreferredUsername.String()) - keyID := actor.GetID().String() - assert.Regexp(t, "activitypub/actor$", keyID) - assert.Regexp(t, "activitypub/actor/inbox$", actor.Inbox.GetID().String()) - - pubKey := actor.PublicKey - assert.NotNil(t, pubKey) - publicKeyID := keyID + "#main-key" - assert.Equal(t, pubKey.ID.String(), publicKeyID) - - pubKeyPem := pubKey.PublicKeyPem - assert.NotNil(t, pubKeyPem) - assert.Regexp(t, "^-----BEGIN PUBLIC KEY-----", pubKeyPem) -} - -func TestActorNewFromKeyId(t *testing.T) { - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - - onGiteaRun(t, func(t *testing.T, u *url.URL) { - ctx, _ := contexttest.MockAPIContext(t, "/api/v1/activitypub/actor") - sut, err := federation.NewActorIDFromKeyID(ctx.Base, fmt.Sprintf("%sapi/v1/activitypub/actor#main-key", u)) - require.NoError(t, err) - - port, err := strconv.ParseUint(u.Port(), 10, 16) - require.NoError(t, err) - - assert.Equal(t, forgefed.ActorID{ - ID: "actor", - HostSchema: "http", - Path: "api/v1/activitypub", - Host: setting.Domain, - HostPort: uint16(port), - UnvalidatedInput: fmt.Sprintf("http://%s:%d/api/v1/activitypub/actor", setting.Domain, port), - IsPortSupplemented: false, - }, sut) - }) -} diff --git a/tests/integration/api_activitypub_person_test.go b/tests/integration/api_activitypub_person_test.go deleted file mode 100644 index ca3bc844d7..0000000000 --- a/tests/integration/api_activitypub_person_test.go +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package integration - -import ( - "fmt" - "net/http" - "net/url" - "testing" - - "forgejo.org/models/db" - "forgejo.org/models/unittest" - user_model "forgejo.org/models/user" - "forgejo.org/modules/activitypub" - "forgejo.org/modules/setting" - "forgejo.org/modules/test" - "forgejo.org/routers" - "forgejo.org/tests" - - ap "github.com/go-ap/activitypub" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestActivityPubPerson(t *testing.T) { - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - - mock := test.NewFederationServerMock() - federatedSrv := mock.DistantServer(t) - defer federatedSrv.Close() - - onGiteaRun(t, func(t *testing.T, localUrl *url.URL) { - defer test.MockVariableValue(&setting.AppURL, localUrl.String())() - - localUserID := 2 - localUserName := "user2" - localUserURL := fmt.Sprintf("%sapi/v1/activitypub/user-id/%d", localUrl, localUserID) - - // distantURL := federatedSrv.URL - // distantUser15URL := fmt.Sprintf("%s/api/v1/activitypub/user-id/15", distantURL) - - cf, err := activitypub.GetClientFactory(db.DefaultContext) - require.NoError(t, err) - - c, err := cf.WithKeysDirect(db.DefaultContext, mock.Persons[0].PrivKey, - mock.Persons[0].KeyID(federatedSrv.URL)) - require.NoError(t, err) - - // Unsigned request - t.Run("UnsignedRequest", func(t *testing.T) { - req := NewRequest(t, "GET", localUserURL) - MakeRequest(t, req, http.StatusBadRequest) - }) - - t.Run("SignedRequestValidation", func(t *testing.T) { - // Signed request - resp, err := c.GetBody(localUserURL) - require.NoError(t, err) - - var person ap.Person - err = person.UnmarshalJSON(resp) - require.NoError(t, err) - - assert.Equal(t, ap.PersonType, person.Type) - assert.Equal(t, localUserName, person.PreferredUsername.String()) - assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%d$", localUserID), person.GetID()) - assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%d/inbox$", localUserID), person.Inbox.GetID().String()) - - assert.NotNil(t, person.PublicKey) - assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%d#main-key$", localUserID), person.PublicKey.ID) - - assert.NotNil(t, person.PublicKey.PublicKeyPem) - assert.Regexp(t, "^-----BEGIN PUBLIC KEY-----", person.PublicKey.PublicKeyPem) - }) - }) -} - -func TestActivityPubMissingPerson(t *testing.T) { - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - defer tests.PrepareTestEnv(t)() - - req := NewRequest(t, "GET", "/api/v1/activitypub/user-id/999999999") - resp := MakeRequest(t, req, http.StatusNotFound) - assert.Contains(t, resp.Body.String(), "user does not exist") -} - -func TestActivityPubPersonInbox(t *testing.T) { - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - - onGiteaRun(t, func(t *testing.T, u *url.URL) { - defer test.MockVariableValue(&setting.AppURL, u.String())() - user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - - user1url := u.JoinPath("/api/v1/activitypub/user-id/1").String() + "#main-key" - cf, err := activitypub.GetClientFactory(db.DefaultContext) - require.NoError(t, err) - c, err := cf.WithKeys(db.DefaultContext, user1, user1url) - require.NoError(t, err) - user2inboxurl := u.JoinPath("/api/v1/activitypub/user-id/2/inbox").String() - - // Signed request succeeds - resp, err := c.Post([]byte{}, user2inboxurl) - require.NoError(t, err) - assert.Equal(t, http.StatusNoContent, resp.StatusCode) - - // Unsigned request fails - req := NewRequest(t, "POST", user2inboxurl) - MakeRequest(t, req, http.StatusBadRequest) - }) -} diff --git a/tests/integration/api_activitypub_repository_test.go b/tests/integration/api_activitypub_repository_test.go deleted file mode 100644 index 14ea1a4b66..0000000000 --- a/tests/integration/api_activitypub_repository_test.go +++ /dev/null @@ -1,174 +0,0 @@ -// Copyright 2024, 2025 The Forgejo Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package integration - -import ( - "fmt" - "net/http" - "net/url" - "testing" - "time" - - "forgejo.org/models/db" - "forgejo.org/models/forgefed" - "forgejo.org/models/unittest" - "forgejo.org/models/user" - "forgejo.org/modules/activitypub" - forgefed_modules "forgejo.org/modules/forgefed" - "forgejo.org/modules/setting" - "forgejo.org/modules/test" - "forgejo.org/routers" - "forgejo.org/tests" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestActivityPubRepository(t *testing.T) { - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - - mock := test.NewFederationServerMock() - federatedSrv := mock.DistantServer(t) - defer federatedSrv.Close() - - onGiteaRun(t, func(t *testing.T, u *url.URL) { - repositoryID := 2 - - cf, err := activitypub.GetClientFactory(db.DefaultContext) - require.NoError(t, err) - - c, err := cf.WithKeysDirect(db.DefaultContext, mock.Persons[0].PrivKey, - mock.Persons[0].KeyID(federatedSrv.URL)) - require.NoError(t, err) - - resp, err := c.GetBody(fmt.Sprintf("%sapi/v1/activitypub/repository-id/%d", u, repositoryID)) - require.NoError(t, err) - assert.Contains(t, string(resp), "@context") - - var repository forgefed_modules.Repository - err = repository.UnmarshalJSON(resp) - require.NoError(t, err) - - assert.Regexp(t, fmt.Sprintf("activitypub/repository-id/%d$", repositoryID), repository.GetID().String()) - }) -} - -func TestActivityPubMissingRepository(t *testing.T) { - defer tests.PrepareTestEnv(t)() - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&setting.Federation.SignatureEnforced, false)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - - repositoryID := 9999999 - req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/activitypub/repository-id/%d", repositoryID)) - resp := MakeRequest(t, req, http.StatusNotFound) - assert.Contains(t, resp.Body.String(), "repository does not exist") -} - -func TestActivityPubRepositoryInboxValid(t *testing.T) { - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - - mock := test.NewFederationServerMock() - federatedSrv := mock.DistantServer(t) - defer federatedSrv.Close() - - onGiteaRun(t, func(t *testing.T, u *url.URL) { - repositoryID := 2 - timeNow := time.Now().UTC() - - cf, err := activitypub.GetClientFactory(db.DefaultContext) - require.NoError(t, err) - - c, err := cf.WithKeysDirect(db.DefaultContext, mock.Persons[0].PrivKey, - mock.Persons[0].KeyID(federatedSrv.URL)) - require.NoError(t, err) - - repoInboxURL := u.JoinPath(fmt.Sprintf("/api/v1/activitypub/repository-id/%d/inbox", repositoryID)).String() - - activity1 := []byte(fmt.Sprintf( - `{"type":"Like",`+ - `"startTime":"%s",`+ - `"actor":"%s/api/v1/activitypub/user-id/15",`+ - `"object":"%s"}`, - timeNow.Format(time.RFC3339), - federatedSrv.URL, u.JoinPath(fmt.Sprintf("/api/v1/activitypub/repository-id/%d", repositoryID)).String())) - t.Logf("activity: %s", activity1) - resp, err := c.Post(activity1, repoInboxURL) - - require.NoError(t, err) - assert.Equal(t, http.StatusNoContent, resp.StatusCode) - - federationHost := unittest.AssertExistsAndLoadBean(t, &forgefed.FederationHost{HostFqdn: "127.0.0.1"}) - federatedUser := unittest.AssertExistsAndLoadBean(t, &user.FederatedUser{ExternalID: "15", FederationHostID: federationHost.ID}) - unittest.AssertExistsAndLoadBean(t, &user.User{ID: federatedUser.UserID}) - - // A like activity by a different user of the same federated host. - activity2 := []byte(fmt.Sprintf( - `{"type":"Like",`+ - `"startTime":"%s",`+ - `"actor":"%s/api/v1/activitypub/user-id/30",`+ - `"object":"%s"}`, - // Make sure this activity happens later then the one before - timeNow.Add(time.Second).Format(time.RFC3339), - federatedSrv.URL, u.JoinPath(fmt.Sprintf("/api/v1/activitypub/repository-id/%d", repositoryID)).String())) - t.Logf("activity: %s", activity2) - resp, err = c.Post(activity2, repoInboxURL) - - require.NoError(t, err) - assert.Equal(t, http.StatusNoContent, resp.StatusCode) - - federatedUser = unittest.AssertExistsAndLoadBean(t, &user.FederatedUser{ExternalID: "30", FederationHostID: federationHost.ID}) - unittest.AssertExistsAndLoadBean(t, &user.User{ID: federatedUser.UserID}) - - // The same user sends another like activity - otherRepositoryID := 3 - otherRepoInboxURL := u.JoinPath(fmt.Sprintf("/api/v1/activitypub/repository-id/%d/inbox", otherRepositoryID)).String() - activity3 := []byte(fmt.Sprintf( - `{"type":"Like",`+ - `"startTime":"%s",`+ - `"actor":"%s/api/v1/activitypub/user-id/30",`+ - `"object":"%s"}`, - // Make sure this activity happens later then the ones before - timeNow.Add(time.Second*2).Format(time.RFC3339), - federatedSrv.URL, u.JoinPath(fmt.Sprintf("/api/v1/activitypub/repository-id/%d", otherRepositoryID)).String())) - t.Logf("activity: %s", activity3) - resp, err = c.Post(activity3, otherRepoInboxURL) - - require.NoError(t, err) - assert.Equal(t, http.StatusNoContent, resp.StatusCode) - - federatedUser = unittest.AssertExistsAndLoadBean(t, &user.FederatedUser{ExternalID: "30", FederationHostID: federationHost.ID}) - unittest.AssertExistsAndLoadBean(t, &user.User{ID: federatedUser.UserID}) - - // Replay activity2. - resp, err = c.Post(activity2, repoInboxURL) - require.NoError(t, err) - assert.Equal(t, http.StatusNotAcceptable, resp.StatusCode) - }) -} - -func TestActivityPubRepositoryInboxInvalid(t *testing.T) { - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&setting.Federation.SignatureEnforced, false)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - - onGiteaRun(t, func(t *testing.T, u *url.URL) { - apServerActor := user.NewAPServerActor() - repositoryID := 2 - - cf, err := activitypub.GetClientFactory(db.DefaultContext) - require.NoError(t, err) - - c, err := cf.WithKeys(db.DefaultContext, apServerActor, apServerActor.KeyID()) - require.NoError(t, err) - - repoInboxURL := u.JoinPath(fmt.Sprintf("/api/v1/activitypub/repository-id/%d/inbox", repositoryID)).String() - activity := []byte(`{"type":"Wrong"}`) - resp, err := c.Post(activity, repoInboxURL) - require.NoError(t, err) - assert.Equal(t, http.StatusNotAcceptable, resp.StatusCode) - }) -} diff --git a/tests/integration/api_federation_httpsig_test.go b/tests/integration/api_federation_httpsig_test.go deleted file mode 100644 index a8deaa315f..0000000000 --- a/tests/integration/api_federation_httpsig_test.go +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright 2025 The Forgejo Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package integration - -import ( - "fmt" - "net/http" - "net/url" - "testing" - - "forgejo.org/models/db" - "forgejo.org/models/forgefed" - "forgejo.org/models/unittest" - "forgejo.org/models/user" - "forgejo.org/modules/activitypub" - "forgejo.org/modules/setting" - "forgejo.org/modules/test" - "forgejo.org/routers" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestFederationHttpSigValidation(t *testing.T) { - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())() - - onGiteaRun(t, func(t *testing.T, u *url.URL) { - userID := 2 - userURL := fmt.Sprintf("%sapi/v1/activitypub/user-id/%d", u, userID) - - user1 := unittest.AssertExistsAndLoadBean(t, &user.User{ID: 1}) - - clientFactory, err := activitypub.GetClientFactory(db.DefaultContext) - require.NoError(t, err) - - apClient, err := clientFactory.WithKeys(db.DefaultContext, user1, user1.KeyID()) - require.NoError(t, err) - - // Unsigned request - t.Run("UnsignedRequest", func(t *testing.T) { - req := NewRequest(t, "GET", userURL) - MakeRequest(t, req, http.StatusBadRequest) - }) - - // Signed request - t.Run("SignedRequest", func(t *testing.T) { - resp, err := apClient.Get(userURL) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, resp.StatusCode) - }) - - // HACK HACK HACK: the host part of the URL gets set to which IP forgejo is - // listening on, NOT localhost, which is the Domain given to forgejo which - // is then used for eg. the keyID all requests - applicationKeyID := fmt.Sprintf("%sapi/v1/activitypub/actor#main-key", setting.AppURL) - actorKeyID := fmt.Sprintf("%sapi/v1/activitypub/user-id/1#main-key", setting.AppURL) - - // Check for cached public keys - t.Run("ValidateCaches", func(t *testing.T) { - host, err := forgefed.FindFederationHostByKeyID(db.DefaultContext, applicationKeyID) - require.NoError(t, err) - assert.NotNil(t, host) - assert.True(t, host.PublicKey.Valid) - - _, user, err := user.FindFederatedUserByKeyID(db.DefaultContext, actorKeyID) - require.NoError(t, err) - assert.NotNil(t, user) - assert.True(t, user.PublicKey.Valid) - }) - - // Disable signature validation - defer test.MockVariableValue(&setting.Federation.SignatureEnforced, false)() - - // Unsigned request - t.Run("SignatureValidationDisabled", func(t *testing.T) { - req := NewRequest(t, "GET", userURL) - MakeRequest(t, req, http.StatusOK) - }) - }) -} diff --git a/tests/integration/repo_settings_test.go b/tests/integration/repo_settings_test.go index 63cc5332bc..6b467d78b2 100644 --- a/tests/integration/repo_settings_test.go +++ b/tests/integration/repo_settings_test.go @@ -6,21 +6,16 @@ package integration import ( "fmt" "net/http" - "strings" "testing" "forgejo.org/models/db" - "forgejo.org/models/forgefed" git_model "forgejo.org/models/git" repo_model "forgejo.org/models/repo" unit_model "forgejo.org/models/unit" "forgejo.org/models/unittest" user_model "forgejo.org/models/user" - fm "forgejo.org/modules/forgefed" "forgejo.org/modules/optional" "forgejo.org/modules/setting" - "forgejo.org/modules/test" - "forgejo.org/modules/validation" gitea_context "forgejo.org/services/context" repo_service "forgejo.org/services/repository" user_service "forgejo.org/services/user" @@ -311,63 +306,3 @@ func TestProtectedBranch(t *testing.T) { unittest.AssertCount(t, &git_model.ProtectedBranch{RuleName: "master", RepoID: repo.ID}, 1) }) } - -func TestRepoFollowing(t *testing.T) { - defer tests.PrepareTestEnv(t)() - defer test.MockVariableValue(&setting.Federation.Enabled, true)() - - mock := test.NewFederationServerMock() - federatedSrv := mock.DistantServer(t) - defer federatedSrv.Close() - - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1, OwnerID: user.ID}) - session := loginUser(t, user.Name) - - t.Run("Add a following repo", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - link := fmt.Sprintf("/%s/settings", repo.FullName()) - - req := NewRequestWithValues(t, "POST", link, map[string]string{ - "_csrf": GetCSRF(t, session, link), - "action": "federation", - "following_repos": fmt.Sprintf("%s/api/v1/activitypub/repository-id/1", federatedSrv.URL), - }) - session.MakeRequest(t, req, http.StatusSeeOther) - - // Verify it was added. - federationHost := unittest.AssertExistsAndLoadBean(t, &forgefed.FederationHost{HostFqdn: "127.0.0.1"}) - unittest.AssertExistsAndLoadBean(t, &repo_model.FollowingRepo{ - ExternalID: "1", - FederationHostID: federationHost.ID, - }) - }) - - t.Run("Star a repo having a following repo", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - repoLink := fmt.Sprintf("/%s", repo.FullName()) - link := fmt.Sprintf("%s/action/star", repoLink) - req := NewRequestWithValues(t, "POST", link, map[string]string{ - "_csrf": GetCSRF(t, session, repoLink), - }) - - session.MakeRequest(t, req, http.StatusOK) - - // Verify distant server received a like activity - like := fm.ForgeLike{} - err := like.UnmarshalJSON([]byte(mock.LastPost)) - if err != nil { - t.Errorf("Error unmarshalling ForgeLike: %q", err) - } - if isValid, err := validation.IsValid(like); !isValid { - t.Errorf("ForgeLike is not valid: %q", err) - } - activityType := like.Type - object := like.Object.GetLink().String() - isLikeType := activityType == "Like" - isCorrectObject := strings.HasSuffix(object, "/api/v1/activitypub/repository-id/1") - if !isLikeType || !isCorrectObject { - t.Error("Activity is not a like for this repo") - } - }) -} From cee2aae4cacd9caab98a4cbb273aab1cf0d0137b Mon Sep 17 00:00:00 2001 From: oliverpool Date: Mon, 7 Jul 2025 18:04:00 +0200 Subject: [PATCH 2/2] fix: corrupted wiki unit default permission (#8234 follow-up) (#8258) Closes #8119, follow-up of #8234 #8234 "only" fixed the case when a new wiki setting was applied. However it lacked 2 aspects: - fixing the already corrupted unit permission in the database (required a migration) - fixing the API route Both aspects should now be covered. Additionally, I commented out the unused `UnitAccessMode` and indicated that they are only used for the wiki-unit (hopefully saving some time to future code-readers). ### Testing - go to a commit before #8234 (e.g. 285f66b782491d05e35aa81a6e1a0fede85069b4) - create 3 repositories - save the wiki settings of the second, without any change - set the wiki of the third to be globally writable - verify the `default_permissions` column in the database, of the unit `5`: 0, 2, 3 - stop Forgejo, switch to this PR and start Forgejo - verify that the logs writes `Migration[35]: Fix wiki unit default permission` - verify the `default_permissions` column in the database, of the unit `5`: 0, 0, 3 Before: ![2025-06-23-150055.png](/attachments/1eb92507-b8b4-422c-921c-4296a91172e7) After: ![2025-06-23-150853.png](/attachments/c7b53397-54fe-487d-89da-e10b26538cde) - [x] I did not document these changes and I do not expect someone else to do it. - [x] I do not want this change to show in the release notes. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8258 Reviewed-by: Earl Warren Co-authored-by: oliverpool Co-committed-by: oliverpool --- models/forgejo_migrations/migrate.go | 2 + models/forgejo_migrations/v36.go | 55 ++++++++++++++++++++++++++++ models/repo/repo_unit.go | 17 +++++---- models/repo/repo_unit_test.go | 4 +- routers/api/v1/repo/repo.go | 2 +- 5 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 models/forgejo_migrations/v36.go diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index 50391ff650..fcea69d23f 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -109,6 +109,8 @@ var migrations = []*Migration{ NewMigration("Add `notify-email` column to `action_run` table", AddNotifyEmailToActionRun), // v34 -> v35 NewMigration("Noop because of https://codeberg.org/forgejo/forgejo/issues/8373", NoopAddIndexToActionRunStopped), + // v35 -> v36 + NewMigration("Fix wiki unit default permission", FixWikiUnitDefaultPermission), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v36.go b/models/forgejo_migrations/v36.go new file mode 100644 index 0000000000..1a798147cf --- /dev/null +++ b/models/forgejo_migrations/v36.go @@ -0,0 +1,55 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "xorm.io/xorm" +) + +func FixWikiUnitDefaultPermission(x *xorm.Engine) error { + // Type is Unit's Type + type Type int + + // Enumerate all the unit types + const ( + TypeInvalid Type = iota // 0 invalid + TypeCode // 1 code + TypeIssues // 2 issues + TypePullRequests // 3 PRs + TypeReleases // 4 Releases + TypeWiki // 5 Wiki + TypeExternalWiki // 6 ExternalWiki + TypeExternalTracker // 7 ExternalTracker + TypeProjects // 8 Projects + TypePackages // 9 Packages + TypeActions // 10 Actions + ) + + // RepoUnitAccessMode specifies the users access mode to a repo unit + type UnitAccessMode int + + const ( + // UnitAccessModeUnset - no unit mode set + UnitAccessModeUnset UnitAccessMode = iota // 0 + // UnitAccessModeNone no access + UnitAccessModeNone // 1 + // UnitAccessModeRead read access + UnitAccessModeRead // 2 + // UnitAccessModeWrite write access + UnitAccessModeWrite // 3 + ) + _ = UnitAccessModeNone + _ = UnitAccessModeWrite + + type RepoUnit struct { + DefaultPermissions UnitAccessMode `xorm:"NOT NULL DEFAULT 0"` + } + _, err := x.Where("type = ?", TypeWiki). + Where("default_permissions = ?", UnitAccessModeRead). + Cols("default_permissions"). + Update(RepoUnit{ + DefaultPermissions: UnitAccessModeUnset, + }) + return err +} diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go index c11ad70627..e50f79e945 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -41,27 +41,30 @@ func (err ErrUnitTypeNotExist) Unwrap() error { } // RepoUnitAccessMode specifies the users access mode to a repo unit +// Only UnitAccessModeWrite is used by the wiki, to mark it as instance-writable type UnitAccessMode int const ( // UnitAccessModeUnset - no unit mode set UnitAccessModeUnset UnitAccessMode = iota // 0 + // UnitAccessModeNone no access - UnitAccessModeNone // 1 + // UnitAccessModeNone UnitAccessMode = 1 // UnitAccessModeRead read access - UnitAccessModeRead // 2 + // UnitAccessModeRead UnitAccessMode = 2 + // UnitAccessModeWrite write access - UnitAccessModeWrite // 3 + UnitAccessModeWrite UnitAccessMode = 3 ) func (mode UnitAccessMode) ToAccessMode(modeIfUnset perm.AccessMode) perm.AccessMode { switch mode { case UnitAccessModeUnset: return modeIfUnset - case UnitAccessModeNone: - return perm.AccessModeNone - case UnitAccessModeRead: - return perm.AccessModeRead + // case UnitAccessModeNone: + // return perm.AccessModeNone + // case UnitAccessModeRead: + // return perm.AccessModeRead case UnitAccessModeWrite: return perm.AccessModeWrite default: diff --git a/models/repo/repo_unit_test.go b/models/repo/repo_unit_test.go index a1964519bd..3d6d408fcb 100644 --- a/models/repo/repo_unit_test.go +++ b/models/repo/repo_unit_test.go @@ -34,8 +34,8 @@ func TestActionsConfig(t *testing.T) { } func TestRepoUnitAccessMode(t *testing.T) { - assert.Equal(t, perm.AccessModeNone, UnitAccessModeNone.ToAccessMode(perm.AccessModeAdmin)) - assert.Equal(t, perm.AccessModeRead, UnitAccessModeRead.ToAccessMode(perm.AccessModeAdmin)) + // assert.Equal(t, perm.AccessModeNone, UnitAccessModeNone.ToAccessMode(perm.AccessModeAdmin)) + // assert.Equal(t, perm.AccessModeRead, UnitAccessModeRead.ToAccessMode(perm.AccessModeAdmin)) assert.Equal(t, perm.AccessModeWrite, UnitAccessModeWrite.ToAccessMode(perm.AccessModeAdmin)) assert.Equal(t, perm.AccessModeRead, UnitAccessModeUnset.ToAccessMode(perm.AccessModeRead)) } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 3d6a40e9ab..42385d54a6 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -861,7 +861,7 @@ func updateRepoUnits(ctx *context.APIContext, owner string, repo *repo_model.Rep if *opts.GloballyEditableWiki { wikiPermissions = repo_model.UnitAccessModeWrite } else { - wikiPermissions = repo_model.UnitAccessModeRead + wikiPermissions = repo_model.UnitAccessModeUnset } }