From cf087a2f12bbbb9a2eb14d50ad81bfc5b6755bd3 Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Sun, 17 May 2026 18:33:39 +0200 Subject: [PATCH] feat: ability to edit authorized integration in web UI (#12601) Extends the UI introduced in #12558 to have edit capabilities. (not in scope: "Add" for a new Authorized Integration will be the next update to this UI; `create-authorized-integration` CLI is still the only way to create a new record) This PR includes a few refactoring steps. The goal of these steps is to have `services/auth` be a single entrypoint for validating, inserting, or updating an authorized integration. Some logic is moved out of `services/authz` because it is not authorization related, and some is moved out of `services/auth/method` to allow it to be reused during validation without creating a cyclical module dependency. This PR also adds comprehensive validation to the more complex fields in the authorized integration, such as the issuer and claim rules. This validation applies to the `forgejo admin user create-authorized-integration` CLI as well. The visible UI is the same as #12558, but with a "Save" button, and the ability to display errors: ![Screenshot 2026-05-16 at 15-43-20 Authorized Integrations - Forgejo Beyond coding. We Forge](/attachments/ffaf60e2-3652-429b-a815-b339100f05f8) ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests for Go changes - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### Tests for JavaScript changes - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [x] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. - Documentation is on my TODO list and will be completed before release. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12601 Reviewed-by: Andreas Ahlenstorf --- .deadcode-out | 3 + .mockery.yml | 4 +- ...in_user_generate_authorized_integration.go | 19 +- models/auth/authorized_integration.go | 12 + .../authorized_integration_resource_repo.go | 9 + models/auth/authorized_integration_test.go | 18 + modules/json/json.go | 1 + options/locale_next/locale_en-US.json | 11 +- routers/api/actions/oidc.go | 4 +- routers/web/user/setting/access_token.go | 30 +- routers/web/user/setting/access_token_test.go | 4 +- .../user/setting/authorized_integrations.go | 319 ++++++++++- routers/web/web.go | 4 +- services/auth/authorized_integration.go | 344 +++++++++++ .../authorized_integration_oidc.go | 6 +- .../authorized_integration_oidc_test.go | 14 +- services/auth/authorized_integration_test.go | 537 ++++++++++++++++++ services/auth/internal_issuer.go | 67 +++ .../auth/method/authorized_integration.go | 223 +------- .../method/authorized_integration_test.go | 56 +- .../auth/{method/mocks_test.go => mocks.go} | 2 +- services/authz/access_token.go | 4 +- services/authz/authorized_integration.go | 16 - services/authz/authorized_integration_test.go | 69 --- .../settings/authorized_integrations.tmpl | 2 +- .../authorized_integrations/generic/view.tmpl | 10 +- .../authorized_integrations/view_footer.tmpl | 116 +++- .../authorized_integrations/view_head.tmpl | 8 +- tests/e2e/fixtures/authorized_integration.yml | 13 + tests/e2e/user-settings.test.e2e.ts | 190 ++++++- tests/integration/integration_test.go | 4 +- tests/integration/user_test.go | 2 +- web_src/js/features/authorized-integration.js | 2 +- 33 files changed, 1710 insertions(+), 413 deletions(-) create mode 100644 services/auth/authorized_integration.go rename services/auth/{method => }/authorized_integration_oidc.go (89%) rename services/auth/{method => }/authorized_integration_oidc_test.go (97%) create mode 100644 services/auth/authorized_integration_test.go create mode 100644 services/auth/internal_issuer.go rename services/auth/{method/mocks_test.go => mocks.go} (99%) create mode 100644 tests/e2e/fixtures/authorized_integration.yml diff --git a/.deadcode-out b/.deadcode-out index b70be3e800..6ac7bc9b2a 100644 --- a/.deadcode-out +++ b/.deadcode-out @@ -222,6 +222,9 @@ forgejo.org/modules/zstd forgejo.org/routers/web/org MustEnableProjects +forgejo.org/services/auth + RegisterInternalIssuerForTesting + forgejo.org/services/context GetPrivateContext diff --git a/.mockery.yml b/.mockery.yml index 2e9427cd77..29fc880549 100644 --- a/.mockery.yml +++ b/.mockery.yml @@ -4,7 +4,9 @@ packages: forgejo.org/modules/nosql: config: filename: mocks.go # make mocks public so that external packages can use - forgejo.org/services/auth/method: + forgejo.org/services/auth: + config: + filename: mocks.go # make mocks public so that external packages can use forgejo.org/services/authz: config: filename: authorization_reducer_mock.go # make mocks public so that external packages can use diff --git a/cmd/admin_user_generate_authorized_integration.go b/cmd/admin_user_generate_authorized_integration.go index 262f35161d..7c01bc8019 100644 --- a/cmd/admin_user_generate_authorized_integration.go +++ b/cmd/admin_user_generate_authorized_integration.go @@ -12,11 +12,10 @@ import ( "strings" auth_model "forgejo.org/models/auth" - "forgejo.org/models/db" "forgejo.org/models/repo" user_model "forgejo.org/models/user" "forgejo.org/modules/json" - "forgejo.org/services/authz" + auth_service "forgejo.org/services/auth" "github.com/urfave/cli/v3" ) @@ -195,22 +194,8 @@ func runCreateAuthorizedIntegration(ctx context.Context, c *cli.Command) error { for i := range repos { rr[i] = &auth_model.AuthorizedIntegResourceRepo{RepoID: repos[i].ID} } - if err := authz.ValidateAuthorizedIntegration(ai, rr); err != nil { - return err - } - err = db.WithTx(ctx, func(ctx context.Context) error { - if err := auth_model.InsertAuthorizedIntegration(ctx, ai); err != nil { - return err - } - if !allRepos { - if err := auth_model.InsertAuthorizedIntegrationResourceRepos(ctx, ai.ID, rr); err != nil { - return err - } - } - return nil - }) - if err != nil { + if err := auth_service.InsertAuthorizedIntegration(ctx, ai, rr); err != nil { return err } diff --git a/models/auth/authorized_integration.go b/models/auth/authorized_integration.go index 1f1fc52a2d..6079d28341 100644 --- a/models/auth/authorized_integration.go +++ b/models/auth/authorized_integration.go @@ -184,6 +184,18 @@ func InsertAuthorizedIntegration(ctx context.Context, ai *AuthorizedIntegration) return err } +func UpdateAuthorizedIntegration(ctx context.Context, ai *AuthorizedIntegration) error { + // NoAutoTime -- UpdatedUnix is used to track the last used time, don't update it when editing + // AllCols -- ensure ResourceAllRepo can be set to false + rowsImpacted, err := db.GetEngine(ctx).ID(ai.ID).NoAutoTime().AllCols().Update(ai) + if rowsImpacted == 0 { + return fmt.Errorf("authorized integration update affected 0 records: %w", util.ErrNotExist) + } else if rowsImpacted != 1 { + return fmt.Errorf("authorized integration update affected %d records", rowsImpacted) + } + return err +} + // Bump the UpdatedUnix field of this authorized integration to now, tracking when it was last used for authentication. // To reduce database write workload, this is only tracked by one-minute intervals -- the UPDATE statement conditionally // avoids writes. diff --git a/models/auth/authorized_integration_resource_repo.go b/models/auth/authorized_integration_resource_repo.go index 77445b60ae..e6eb544931 100644 --- a/models/auth/authorized_integration_resource_repo.go +++ b/models/auth/authorized_integration_resource_repo.go @@ -54,3 +54,12 @@ func InsertAuthorizedIntegrationResourceRepos(ctx context.Context, aiID int64, r return nil }) } + +func UpdateAuthorizedIntegrationResourceRepos(ctx context.Context, aiID int64, resources []*AuthorizedIntegResourceRepo) error { + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err := db.GetEngine(ctx).Delete(&AuthorizedIntegResourceRepo{IntegID: aiID}); err != nil { + return err + } + return InsertAuthorizedIntegrationResourceRepos(ctx, aiID, resources) + }) +} diff --git a/models/auth/authorized_integration_test.go b/models/auth/authorized_integration_test.go index 447640b38c..c77fb32cc6 100644 --- a/models/auth/authorized_integration_test.go +++ b/models/auth/authorized_integration_test.go @@ -175,3 +175,21 @@ func TestListAuthorizedIntegrationOptions(t *testing.T) { require.NoError(t, err) assert.Empty(t, ais) } + +func TestUpdateAuthorizedIntegration(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + ai := makeAuthorizedIntegration(t) + ai.Name = "Hello, world!" + ai.ResourceAllRepos = false + createdUnix := ai.CreatedUnix + updatedUnix := ai.UpdatedUnix + + require.NoError(t, auth_model.UpdateAuthorizedIntegration(t.Context(), ai)) + + fromDB := unittest.AssertExistsAndLoadBean(t, &auth_model.AuthorizedIntegration{ID: ai.ID}) + assert.Equal(t, "Hello, world!", fromDB.Name) + assert.False(t, fromDB.ResourceAllRepos) + assert.Equal(t, createdUnix, fromDB.CreatedUnix) + assert.Equal(t, updatedUnix, fromDB.UpdatedUnix) // not changed -- used to track usage for authentication +} diff --git a/modules/json/json.go b/modules/json/json.go index 34568c75c6..b422e8fcd5 100644 --- a/modules/json/json.go +++ b/modules/json/json.go @@ -21,6 +21,7 @@ type Encoder interface { // Decoder represents a decoder for json type Decoder interface { Decode(v any) error + DisallowUnknownFields() } // Interface represents an interface to handle json data diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 5ed2c48152..c0dffa174f 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -325,8 +325,9 @@ "settings.authorized_integration.ui.generic": "Generic JWT", "settings.authorized_integration.ui.forgejo_actions_local": "Forgejo Actions (Local)", "settings.authorized_integration.none": "No authorized integrations currently configured.", - "settings.authorized_integration.view": "View", - "settings.authorized_integration.view_page_title": "Authorized Integration %s", + "settings.authorized_integration.edit": "Edit", + "settings.authorized_integration.edit_page_title": "Authorized Integration %s", + "settings.authorized_integration.save": "Save authorized integration", "settings.authorized_integration.field.name": "Name", "settings.authorized_integration.field.description": "Description", "settings.authorized_integration.field.description.placeholder": "Used to publish packages when ...", @@ -336,6 +337,12 @@ "settings.authorized_integration.claims.generic": "Generic JWT Rules", "settings.authorized_integration.perms.title": "Capabilities Permitted", "settings.authorized_integration.copy_audience": "Copy audience to clipboard", + "settings.authorized_integration.name.required": "Authorized integration name is required.", + "settings.authorized_integration.issuer.invalid": "Issuer validation failed: %s", + "settings.authorized_integration.claim_rules.invalid": "Claim Rules validation failed: %s", + "settings.authorized_integration.specified_repos_none": "Authorized integrations with specified repositories must have at least one repository.", + "settings.authorized_integration.specified_repos_and_public_only": "Authorized integrations with specified repositories cannot be combined with the public-only scope.", + "settings.authorized_integration.specified_repos_and_invalid_scope": "Authorized integrations with specified repositories can only be used with the read:issue, write:issue, read:repository, and write:repository scopes.", "webauthn.insert_key": "Insert your security key", "webauthn.sign_in": "Press the button on your security key. If your security key has no button, re-insert it.", "webauthn.press_button": "Please press the button on your security key…", diff --git a/routers/api/actions/oidc.go b/routers/api/actions/oidc.go index 23b6fec16c..991f52b6fc 100644 --- a/routers/api/actions/oidc.go +++ b/routers/api/actions/oidc.go @@ -14,7 +14,7 @@ import ( "forgejo.org/modules/web" web_types "forgejo.org/modules/web/types" actions_service "forgejo.org/services/actions" - auth_method "forgejo.org/services/auth/method" + "forgejo.org/services/auth" "forgejo.org/services/context" ) @@ -67,7 +67,7 @@ func init() { web.RegisterResponseStatusProvider[*OIDCContext](func(req *http.Request) web_types.ResponseStatusProvider { return req.Context().Value(oidcContextKey).(*OIDCContext) }) - auth_method.RegisterInternalIssuer("api/actions", internalIssuer{}) + auth.RegisterInternalIssuer("api/actions", internalIssuer{}) } func OIDCContexter() func(next http.Handler) http.Handler { diff --git a/routers/web/user/setting/access_token.go b/routers/web/user/setting/access_token.go index cdaad287a8..8d7a53d400 100644 --- a/routers/web/user/setting/access_token.go +++ b/routers/web/user/setting/access_token.go @@ -34,13 +34,12 @@ const ( tplAccessTokenEdit base.TplName = "user/settings/access_token_edit" ) -func getSelectedRepos(ctx *context.Context, selectedReposRaw []string) []*repo_model.Repository { +func getSelectedRepos(ctx *context.Context, selectedReposRaw []string) ([]*repo_model.Repository, error) { ownerAndName := make([][2]string, len(selectedReposRaw)) for i, selected := range selectedReposRaw { split := strings.SplitN(selected, "/", 2) // ownername/reponame if len(split) != 2 { - ctx.Error(http.StatusBadRequest, fmt.Sprintf("invalid selected_repo: %s", selected)) - return nil + return nil, fmt.Errorf("invalid selected_repo: %s", selected) } ownerAndName[i] = [2]string{split[0], split[1]} } @@ -54,29 +53,26 @@ func getSelectedRepos(ctx *context.Context, selectedReposRaw []string) []*repo_m cond := repo_model.SearchRepositoryCondition(repoSearch) repos, _, err := repo_model.SearchRepositoryByCondition(ctx, repoSearch, cond, false) if err != nil { - ctx.ServerError("SearchRepositoryByCondition", err) - return nil + return nil, err } else if len(repos) != len(selectedReposRaw) { // One or more of the repositories couldn't be found by search by owner & name. - ctx.Error(http.StatusBadRequest, "GetRepositoryByOwnerAndName") // keep in sync w/ !permission.HasAccess below - return nil + // Keep error in sync w/ ~permission.HasAccess below to avoid data existence probing. + return nil, fmt.Errorf("one or more of the repositories couldn't be found by owner & name") } selectedRepos := make([]*repo_model.Repository, len(selectedReposRaw)) for i, repo := range repos { permission, err := access_model.GetUserRepoPermission(ctx, repo, ctx.Doer) if err != nil { - ctx.ServerError("GetUserRepoPermission", err) - return nil + return nil, err } else if !permission.HasAccess() { // Prevent data existence probing -- ensure this error is the exact same as the (len(repos) != // len(selectedReposRaw)) case above - ctx.Error(http.StatusBadRequest, "GetRepositoryByOwnerAndName") - return nil + return nil, fmt.Errorf("one or more of the repositories couldn't be found by owner & name") } selectedRepos[i] = repo } - return selectedRepos + return selectedRepos, nil } func loadAccessTokenCreateData(ctx *context.Context) { @@ -136,8 +132,9 @@ func loadAccessTokenCreateData(ctx *context.Context) { } else if isPost { selectedReposRaw = postForm.SelectedRepo } - selectedRepos := getSelectedRepos(ctx, selectedReposRaw) - if ctx.Written() { + selectedRepos, err := getSelectedRepos(ctx, selectedReposRaw) + if err != nil { + ctx.Error(http.StatusBadRequest, "getSelectedRepos") return } ctx.Data["SelectedRepos"] = selectedRepos @@ -273,8 +270,9 @@ func AccessTokenCreatePost(ctx *context.Context) { t.Scope = newScope case "repo-specific": t.ResourceAllRepos = false - selectedRepos := getSelectedRepos(ctx, form.SelectedRepo) - if ctx.Written() { + selectedRepos, err := getSelectedRepos(ctx, form.SelectedRepo) + if err != nil { + ctx.Error(http.StatusBadRequest, "getSelectedRepos") return } for _, repo := range selectedRepos { diff --git a/routers/web/user/setting/access_token_test.go b/routers/web/user/setting/access_token_test.go index 21d27a7992..776965175d 100644 --- a/routers/web/user/setting/access_token_test.go +++ b/routers/web/user/setting/access_token_test.go @@ -143,7 +143,7 @@ func TestAccessTokenCreatePost(t *testing.T) { SelectedRepo: []string{"org17/big_test_private_4000000_does_not_exist"}, }) assert.Equal(t, http.StatusBadRequest, resp.Result().StatusCode) - assert.Equal(t, "GetRepositoryByOwnerAndName\n", resp.Body.String()) + assert.Equal(t, "getSelectedRepos\n", resp.Body.String()) }) t.Run("non-visible repo selected via IDOR", func(t *testing.T) { @@ -154,6 +154,6 @@ func TestAccessTokenCreatePost(t *testing.T) { SelectedRepo: []string{"user30/empty"}, // private repo, user2 has no visibility }) assert.Equal(t, http.StatusBadRequest, resp.Result().StatusCode) - assert.Equal(t, "GetRepositoryByOwnerAndName\n", resp.Body.String()) // should be exact same response as "invalid repo selected" case + assert.Equal(t, "getSelectedRepos\n", resp.Body.String()) // should be exact same response as "invalid repo selected" case }) } diff --git a/routers/web/user/setting/authorized_integrations.go b/routers/web/user/setting/authorized_integrations.go index 4e15c8addf..227d95bdf2 100644 --- a/routers/web/user/setting/authorized_integrations.go +++ b/routers/web/user/setting/authorized_integrations.go @@ -4,17 +4,30 @@ package setting import ( + "bytes" "errors" + "fmt" + "io" "net/http" "slices" + "strings" auth_model "forgejo.org/models/auth" "forgejo.org/models/db" + access_model "forgejo.org/models/perm/access" + repo_model "forgejo.org/models/repo" "forgejo.org/modules/base" "forgejo.org/modules/json" + "forgejo.org/modules/log" "forgejo.org/modules/optional" + "forgejo.org/modules/setting" "forgejo.org/modules/util" + "forgejo.org/modules/web" + auth_service "forgejo.org/services/auth" + "forgejo.org/services/authz" "forgejo.org/services/context" + + "xorm.io/builder" ) const ( @@ -38,53 +51,156 @@ func ListAuthorizedIntegrations(ctx *context.Context) { } type AuthorizedIntegrationForm struct { - Name string - Description string - Audience string - Resource string // all, public-only, repo-specific + // Top data in UI, descriptive information about the Authorized Integration: + Name string + Description string + Audience string + + // Middle data in UI, how JWTs are validated by this Authorized Integration: + Issuer string // Future: Issuer is likely to be replaced with more-specific fields on non-generic UIs + ClaimRules string // Future: ClaimRules is only required when aiUI == "generic" + + // Bottom data in the UI, what authorization is permitted by this Authorized Integration: + Resource string // all, public-only, repo-specific + SelectedRepo []string // slice of ownername/reponame for repo-specific ScopeAll bool Scope []string - SelectedRepo []string // slice of ownername/reponame - // Future: ClaimRules is only required when aiUI == "generic" - ClaimRules string - // Future: Issuer is likely to be replaced with more-specific fields on non-generic UIs - Issuer string + // Values used for repo-specific repository multi-select UI, not stored in Authorized Integration: + RepoSearch string + AddSelectedRepo string // add a repo to SelectedRepo + RemoveSelectedRepo string // remove a repo from SelectedRepo + Page int // repo search page + SetPage int // repo search buttons } -func ViewAuthorizedIntegration(ctx *context.Context) { - ctx.Data["Title"] = ctx.Tr("settings.authorized_integrations") - ctx.Data["PageIsSettingsAuthorizedIntegrations"] = true - +func getAuthorizedIntegration(ctx *context.Context) *auth_model.AuthorizedIntegration { aiUIString := ctx.Params("ui") aiUI, err := auth_model.ParseAuthorizedIntegrationUI(aiUIString) if err != nil { ctx.NotFound("ParseAuthorizedIntegrationUI", err) - return + return nil } aiID := ctx.ParamsInt64("id") ai, err := auth_model.GetAuthorizedIntegrationByUI(ctx, ctx.Doer.ID, aiUI, aiID) if errors.Is(err, util.ErrNotExist) { ctx.NotFound("GetAuthorizedIntegrationByUI", err) - return + return nil } else if err != nil { ctx.ServerError("GetAuthorizedIntegrationByUI", err) + return nil + } + + return ai +} + +func EditAuthorizedIntegration(ctx *context.Context) { + ai := getAuthorizedIntegration(ctx) + if ctx.Written() { return } + form := web.GetForm(ctx).(*AuthorizedIntegrationForm) + if form.Audience == "" { // empty GET; first load of the page + repos, err := auth_model.GetRepositoriesAccessibleWithIntegration(ctx, ai.ID) + if err != nil { + ctx.ServerError("GetRepositoriesAccessibleWithIntegration", err) + return + } + + form, err = copyAuthorizedIntegrationToForm(ctx, ai, repos) + if err != nil { + ctx.ServerError("copyAuthorizedIntegrationToForm", err) + return + } + } + ctx.Data["Form"] = form + + EditAuthorizedIntegrationRenderCommon(ctx) +} + +func EditAuthorizedIntegrationPost(ctx *context.Context) { + form := web.GetForm(ctx).(*AuthorizedIntegrationForm) + ctx.Data["Form"] = form // make form available for template render on any error + + ai := getAuthorizedIntegration(ctx) + if ctx.Written() { + return + } + + rr, err := copyFormToAuthorizedIntegration(ctx, form, ai) + if err != nil { + editAuthorizedIntegrationErrorHandler(ctx, err) + return + } + + if err := auth_service.UpdateAuthorizedIntegration(ctx, ai, rr); err != nil { + editAuthorizedIntegrationErrorHandler(ctx, err) + return + } + + ctx.Redirect(setting.AppSubURL + "/user/settings/authorized-integrations") +} + +func editAuthorizedIntegrationErrorHandler(ctx *context.Context, err error) { + var errMissingField *auth_service.MissingFieldError + switch { + case errors.As(err, &errMissingField): + switch errMissingField.Field { + case "Name": + ctx.Data["Err_Name"] = true + ctx.Flash.Error(ctx.Tr("settings.authorized_integration.name.required"), true) + default: + // Unrecognized field; fallback to server error handling. + ctx.ServerError("UpdateAuthorizedIntegration", err) + return + } + EditAuthorizedIntegrationRenderCommon(ctx) + return + case errors.Is(err, auth_service.ErrInvalidIssuer): + ctx.Data["Err_Issuer"] = true + ctx.Flash.Error(ctx.Tr("settings.authorized_integration.issuer.invalid", err.Error()), true) + EditAuthorizedIntegrationRenderCommon(ctx) + return + case errors.Is(err, auth_service.ErrInvalidClaimRules): + ctx.Data["Err_ClaimRules"] = true + ctx.Flash.Error(ctx.Tr("settings.authorized_integration.claim_rules.invalid", err.Error()), true) + EditAuthorizedIntegrationRenderCommon(ctx) + return + case errors.Is(err, authz.ErrSpecifiedReposNone): + ctx.Data["Err_SelectedRepo"] = true + ctx.Flash.Error(ctx.Tr("settings.authorized_integration.specified_repos_none"), true) + EditAuthorizedIntegrationRenderCommon(ctx) + return + case errors.Is(err, authz.ErrSpecifiedReposNoPublicOnly): + ctx.Data["Err_SelectedRepo"] = true + ctx.Flash.Error(ctx.Tr("settings.authorized_integration.specified_repos_and_public_only"), true) + EditAuthorizedIntegrationRenderCommon(ctx) + return + case errors.Is(err, authz.ErrSpecifiedReposInvalidScope): + ctx.Data["Err_SelectedRepo"] = true + ctx.Data["Err_Scope"] = true + ctx.Flash.Error(ctx.Tr("settings.authorized_integration.specified_repos_and_invalid_scope"), true) + EditAuthorizedIntegrationRenderCommon(ctx) + return + } + + ctx.ServerError("UpdateAuthorizedIntegration", err) +} + +func copyAuthorizedIntegrationToForm(ctx *context.Context, ai *auth_model.AuthorizedIntegration, rr []*auth_model.AuthorizedIntegResourceRepo) (*AuthorizedIntegrationForm, error) { form := &AuthorizedIntegrationForm{ Name: ai.Name, Description: ai.Description, Audience: ai.Audience, - Issuer: ai.Issuer, + Issuer: ai.Issuer, // Future: Issuer is only required when ai.UI == "generic" } if ai.ResourceAllRepos { publicOnly, err := ai.Scope.PublicOnly() if err != nil { - ctx.ServerError("PublicOnly", err) - return + return nil, err } if publicOnly { form.Resource = "public-only" @@ -96,23 +212,105 @@ func ViewAuthorizedIntegration(ctx *context.Context) { } form.Scope = ai.Scope.StringSlice() - form.ScopeAll, err = ai.Scope.HasScope(auth_model.AccessTokenScopeAll) + scopeAll, err := ai.Scope.HasScope(auth_model.AccessTokenScopeAll) if err != nil { - ctx.ServerError("HasScope", err) - return + return nil, err } + form.ScopeAll = scopeAll // Future: ClaimRules is only required when aiUI == "generic" claimRulesJSON, err := json.MarshalIndent(ai.ClaimRules, "", " ") if err != nil { - ctx.ServerError("MarshalIndent", err) - return + return nil, err } form.ClaimRules = string(claimRulesJSON) - // FIXME: form.SelectedRepo + form.SelectedRepo = []string{} + if len(rr) != 0 { + repoIDs := make([]int64, len(rr)) + for _, r := range rr { + repoIDs = append(repoIDs, r.RepoID) + } + repos, err := db.GetByIDs(ctx, "id", repoIDs, &repo_model.Repository{}) + if err != nil { + return nil, err + } + for _, r := range rr { + repo := repos[r.RepoID] + // Repos associated with an authorized integration should already be visible to the owner, but it's possible + // that access has changed, such as a removed collaborator on a repo -- don't provide info on that repo if + // so. + permission, err := access_model.GetUserRepoPermission(ctx, repo, ctx.Doer) + if err != nil { + return nil, err + } + if permission.HasAccess() { + form.SelectedRepo = append(form.SelectedRepo, fmt.Sprintf("%s/%s", repo.OwnerName, repo.Name)) + } + } + } - ctx.Data["Form"] = form + return form, nil +} + +func copyFormToAuthorizedIntegration(ctx *context.Context, form *AuthorizedIntegrationForm, ai *auth_model.AuthorizedIntegration) ([]*auth_model.AuthorizedIntegResourceRepo, error) { + ai.Name = form.Name + ai.Description = form.Description + + // ui=Generic, to be refactored later + ai.Issuer = form.Issuer + var claimRules *auth_model.ClaimRules + + reader := bytes.NewReader([]byte(form.ClaimRules)) + decoder := json.NewDecoder(reader) + decoder.DisallowUnknownFields() // prevent typo fields from being ignored to make errors easier to identify + if err := decoder.Decode(&claimRules); err != nil { + return nil, fmt.Errorf("%w: %w", auth_service.ErrInvalidClaimRules, err) + } + // json.Decoder doesn't guarantee that all of the reader is consumed, which can lead to weird situations + // where the UI appears to work correctly if extra content is in the form field, but it won't be parsed, + // misleading users. Detect if anything other than io.EOF comes out of further decodings: + var extra any + if err := decoder.Decode(&extra); err != io.EOF { + if err == nil { + return nil, fmt.Errorf("%w: unexpected trailing content: %s", auth_service.ErrInvalidClaimRules, extra) + } + return nil, fmt.Errorf("%w: error after JSON value: %w", auth_service.ErrInvalidClaimRules, err) + } + ai.ClaimRules = claimRules + + scopeRaw := strings.Join(form.Scope, ",") + + var resourceRepos []*auth_model.AuthorizedIntegResourceRepo + switch form.Resource { + case "all": + ai.ResourceAllRepos = true + case "public-only": + ai.ResourceAllRepos = true + scopeRaw = fmt.Sprintf("%s,%s", scopeRaw, auth_model.AccessTokenScopePublicOnly) + case "repo-specific": + ai.ResourceAllRepos = false + selectedRepos, err := getSelectedRepos(ctx, form.SelectedRepo) + if err != nil { + return nil, err + } + for _, repo := range selectedRepos { + resourceRepos = append(resourceRepos, &auth_model.AuthorizedIntegResourceRepo{RepoID: repo.ID}) + } + } + + scope, err := auth_model.AccessTokenScope(scopeRaw).Normalize() + if err != nil { + return nil, err + } + ai.Scope = scope + + return resourceRepos, nil +} + +func EditAuthorizedIntegrationRenderCommon(ctx *context.Context) { + ctx.Data["Title"] = ctx.Tr("settings.authorized_integrations") + ctx.Data["PageIsSettingsAuthorizedIntegrations"] = true categories := []string{ "activitypub", @@ -130,5 +328,76 @@ func ViewAuthorizedIntegration(ctx *context.Context) { slices.Sort(categories) ctx.Data["Categories"] = categories + repoMultiSelect(ctx) + ctx.HTML(http.StatusOK, tplAuthorizedIntegrationViewGeneric) } + +func repoMultiSelect(ctx *context.Context) { + form := ctx.Data["Form"].(*AuthorizedIntegrationForm) + + if form.AddSelectedRepo != "" { + form.SelectedRepo = append(form.SelectedRepo, form.AddSelectedRepo) + } + if form.RemoveSelectedRepo != "" { + form.SelectedRepo = slices.DeleteFunc( + form.SelectedRepo, + func(r string) bool { return r == form.RemoveSelectedRepo }, + ) + } + + selectedRepos, err := getSelectedRepos(ctx, form.SelectedRepo) + if err != nil { + ctx.Error(http.StatusBadRequest, "getSelectedRepos") + return + } + ctx.Data["SelectedRepos"] = selectedRepos + + repoSearchText := form.RepoSearch + + page := 1 + // Pagination on the repo search has form submit buttons that send the `set_page` param. It's then encoded into the + // page in the hidden input `page` which we fall back to, if anything else causes a form get (eg. adding or removing + // a repo). + if form.SetPage > 0 { + page = form.SetPage + } else if form.Page > 0 { + page = form.Page + } + pageSize := 10 + repoSearch := &repo_model.SearchRepoOptions{ + Actor: ctx.Doer, + Keyword: repoSearchText, + Private: true, + Archived: optional.Some(false), + + // Restrict repositories to those owned by, or collaborated with, by the user. Repo-specific access tokens + // could theoretically be created on any public repository as well, but there wouldn't be much point to that and + // it would really balloon the search results to an impractical number of repos. + OwnerID: ctx.Doer.ID, + + ListOptions: db.ListOptions{ + Page: page, + PageSize: pageSize, + }, + OrderBy: db.SearchOrderByAlphabetically, // match sorting in getSelectedRepos for consistency + } + cond := repo_model.SearchRepositoryCondition(repoSearch) + // Exclude all the repos that are currently in `form.SelectedRepo` from the search, by omitting them from the search + // condition. This prevents the UI from displaying the same repo on the left and right, and maintains the repo + // search and page-size correctly. + for _, selected := range selectedRepos { + cond = cond.And(builder.Neq{"id": selected.ID}) + } + repos, count, err := repo_model.SearchRepositoryByCondition(ctx, repoSearch, cond, false) + if err != nil { + log.Error("SearchRepository: %v", err) + ctx.JSON(http.StatusInternalServerError, nil) + return + } + ctx.Data["Repos"] = repos + + pager := context.NewPagination(int(count), pageSize, page, 3) + pager.SetDefaultParams(ctx) + ctx.Data["Page"] = pager +} diff --git a/routers/web/web.go b/routers/web/web.go index 97d7ddae0f..2345483d9d 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -672,7 +672,9 @@ func registerRoutes(m *web.Route) { }) m.Group("/authorized-integrations", func() { - m.Get("/{ui}/{id}", user_setting.ViewAuthorizedIntegration) + m.Combo("/{ui}/{id}"). + Get(web.Bind(user_setting.AuthorizedIntegrationForm{}), user_setting.EditAuthorizedIntegration). + Post(web.Bind(user_setting.AuthorizedIntegrationForm{}), user_setting.EditAuthorizedIntegrationPost) m.Get("", user_setting.ListAuthorizedIntegrations) }) diff --git a/services/auth/authorized_integration.go b/services/auth/authorized_integration.go new file mode 100644 index 0000000000..cf9260fe06 --- /dev/null +++ b/services/auth/authorized_integration.go @@ -0,0 +1,344 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package auth + +import ( + "bufio" + "bytes" + "context" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "sync" + + auth_model "forgejo.org/models/auth" + "forgejo.org/models/db" + "forgejo.org/modules/cache" + "forgejo.org/modules/hostmatcher" + "forgejo.org/modules/json" + "forgejo.org/modules/log" + "forgejo.org/modules/proxy" + "forgejo.org/modules/setting" + "forgejo.org/services/authz" + + "github.com/gobwas/glob" +) + +var ( + ErrAuthorizedIntegrationBadUI = errors.New("invalid authorized integration UI") + ErrInvalidIssuer = errors.New("invalid issuer") + ErrInvalidClaimRules = errors.New("invalid claim rules") + + // Authorized Integration's HTTP client for remote OIDC metadata and key fetches: + aiHTTPClient *http.Client + initHTTPClient sync.Once + + // Allow mocking / overridding during tests: + GetAuthorizedIntegrationHTTPClient = func() *http.Client { + initHTTPClient.Do(initAuthorizedIntegrationHTTPClient) + return aiHTTPClient + } + GetAuthorizedIntegrationCache = cache.GetCache +) + +// Restrict document size to prevent resource exhaustion attack with a malicious authorized integration; largest +// real-world openid-configuration observed is about 1kB, largest JWKS is 6kB, so for both cases 16kB should be +// sufficient. If this needs to change in the future, it could be moved to a config setting -- but until a reason comes +// up it seems reasonable to keep microscopic settings out-of-sight. +const authorizedIntegrationRequestBodyLimit = int64(16 * 1024) + +func initAuthorizedIntegrationHTTPClient() { + blockList := hostmatcher.ParseSimpleMatchList("authorized_integration.BLOCKED_DOMAINS", setting.AuthorizedIntegration.BlockedDomains) + + allowList := hostmatcher.ParseSimpleMatchList("authorized_integration.ALLOWED_DOMAINS", setting.AuthorizedIntegration.AllowedDomains) + if allowList.IsEmpty() { + // the default policy is that authorized integrations can access external hosts + allowList.AppendBuiltin(hostmatcher.MatchBuiltinExternal) + } + if setting.AuthorizedIntegration.AllowLocalNetworks { + allowList.AppendBuiltin(hostmatcher.MatchBuiltinPrivate) + allowList.AppendBuiltin(hostmatcher.MatchBuiltinLoopback) + } + + aiHTTPClient = &http.Client{ + Timeout: setting.AuthorizedIntegration.RequestTimeout, + Transport: &http.Transport{ + Proxy: proxy.Proxy(), + DialContext: hostmatcher.NewDialContext("authorized_integration", allowList, blockList, setting.Proxy.ProxyURLFixed), + }, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + // It might be possible to come up with some reasonable capability to support redirects -- such as + // keeping them within the same issuer host? -- but there are risks that this can be used for SSRF + // attacks. In the face of those risks, and with a lack of real-world use-cases, disable redirects. + return errors.New("authorized integration: HTTP redirects are disabled") + }, + } +} + +func authorizedIntegrationCacheKey(urlString string) string { + return fmt.Sprintf("auth-int-remote:%s", urlString) +} + +func authorizedIntegrationCacheGetJSON[K any](urlString string, v *K) bool { + conn := GetAuthorizedIntegrationCache() + if conn == nil { + return false + } + + cachedAny := conn.Get(authorizedIntegrationCacheKey(urlString)) + if cachedAny == nil { + return false + } + cachedBytes, ok := cachedAny.([]byte) + if !ok { + cachedString, ok := cachedAny.(string) + if !ok { + log.Error("cached content was not []byte or string, but was %T", cachedAny) + return false + } + cachedBytes = []byte(cachedString) + } + + err := json.Unmarshal(cachedBytes, &v) + if err != nil { + // This error case shouldn't occur, as we only store data in the cache once we're sure we could unmarshal it. + // If it does occur, log and fallback to treating as uncached. + log.Error("failed to Unmarshal cached content: %s", err) + // Caller may reuse `v` in a future unmarshal/decode call, and failure here may have polluted it. + var zeroValue K + *v = zeroValue + return false + } + + return true +} + +func authorizedIntegrationCacheSetJSON(urlString string, buf []byte) { + conn := GetAuthorizedIntegrationCache() + if conn == nil { + return + } + err := conn.Put(authorizedIntegrationCacheKey(urlString), buf, int64(setting.AuthorizedIntegration.CacheTTL.Seconds())) + if err != nil { + log.Error("failed to put cache: %s", err) + } +} + +func AuthorizedIntegrationFetchJSON[K any](urlString string, v *K) error { + parsedURL, err := url.Parse(urlString) + if err != nil { + return fmt.Errorf("failed parsing URL %q: %w", urlString, err) + } + // Fetching openid-connect or JWKS needs to come from a source that is authentic, and therefore only `https` is + // supported. This also protects against a trusted issuer being configured maliciously as `file://` or a JKWS URI + // being `file://` -- the HTTP client won't permit that, but, extra safety doesn't hurt. + if parsedURL.Scheme != "https" { + return fmt.Errorf("unsupported URL scheme: %q", parsedURL.String()) + } + + // Check our cache, save a remote HTTP interaction. + if authorizedIntegrationCacheGetJSON(urlString, v) { + return nil + } + + resp, err := GetAuthorizedIntegrationHTTPClient().Get(parsedURL.String()) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("non-OK response code: %s", resp.Status) + } + + bodyReader := io.LimitReader(resp.Body, authorizedIntegrationRequestBodyLimit) + var buf bytes.Buffer + _, err = io.Copy(bufio.NewWriter(&buf), bodyReader) + if err != nil { + return fmt.Errorf("read from remote error: %w", err) + } + + err = json.Unmarshal(buf.Bytes(), &v) + if err != nil { + // If a decoding error is hit, decorate with information about the limited body size so that it doesn't look + // like the remote server provided an incomplete response. err should be something like `io.UnexpectedEOF` in + // this case, but it actually isn't, so don't bother trying to detect precisely. + return fmt.Errorf("failed to decode (response body restricted to %d bytes): %w", authorizedIntegrationRequestBodyLimit, err) + } + + // Successfully decoded the response -- cache the raw bytes for later access. + authorizedIntegrationCacheSetJSON(urlString, buf.Bytes()) + + return nil +} + +type MissingFieldError struct { + Field string +} + +func (e *MissingFieldError) Error() string { + return fmt.Sprintf("missing field %s", e.Field) +} + +// Validate that an authorized integration's state is valid for creation. For example, that it doesn't have a +// conflicting set of resources (public-only and specific repositories), and other similar checks. +func ValidateAuthorizedIntegration(ai *auth_model.AuthorizedIntegration, repoResources []*auth_model.AuthorizedIntegResourceRepo) error { + if ai.Name == "" { + return &MissingFieldError{Field: "Name"} + } + + switch ai.UI { + case auth_model.AuthorizedIntegrationUIGeneric, + auth_model.AuthorizedIntegrationUIForgejoActionsLocal: + break + default: + return fmt.Errorf("%w: invalid UI: %q", ErrAuthorizedIntegrationBadUI, ai.UI) + } + + internalIssuer := false + for _, ii := range GetInternalIssuers() { + if ai.Issuer == ii.IssuerPlaceholder() { + internalIssuer = true + break + } + } + if !internalIssuer { + if err := validateExternalIssuer(ai.Issuer); err != nil { + return err + } + } + + if err := validateClaimRules(ai.ClaimRules, "root"); err != nil { + return err + } + + return authz.ValidateRepositoryResource(ai.ResourceAllRepos, ai.Scope, len(repoResources)) +} + +// Validate and insert a new authorized integration. +func InsertAuthorizedIntegration(ctx context.Context, ai *auth_model.AuthorizedIntegration, repoResources []*auth_model.AuthorizedIntegResourceRepo) error { + ai.Name = strings.TrimSpace(ai.Name) + ai.Description = strings.TrimSpace(ai.Description) + + if err := ValidateAuthorizedIntegration(ai, repoResources); err != nil { + return err + } + + return db.WithTx(ctx, func(ctx context.Context) error { + if err := auth_model.InsertAuthorizedIntegration(ctx, ai); err != nil { + return err + } + if !ai.ResourceAllRepos { + if err := auth_model.InsertAuthorizedIntegrationResourceRepos(ctx, ai.ID, repoResources); err != nil { + return err + } + } + return nil + }) +} + +func UpdateAuthorizedIntegration(ctx context.Context, ai *auth_model.AuthorizedIntegration, repoResources []*auth_model.AuthorizedIntegResourceRepo) error { + ai.Name = strings.TrimSpace(ai.Name) + ai.Description = strings.TrimSpace(ai.Description) + + if err := ValidateAuthorizedIntegration(ai, repoResources); err != nil { + return err + } + + return db.WithTx(ctx, func(ctx context.Context) error { + if err := auth_model.UpdateAuthorizedIntegration(ctx, ai); err != nil { + return err + } + return auth_model.UpdateAuthorizedIntegrationResourceRepos(ctx, ai.ID, repoResources) + }) +} + +func validateExternalIssuer(issuer string) error { + issuerURL, err := url.Parse(issuer) + if err != nil { + return fmt.Errorf("%w: failed parsing issuer URL: %w", ErrInvalidIssuer, err) + } + + // Checks implemented here a variation of [AuthorizedIntegration.Verify]'s checks on the remote issuer. Where + // possible, if validation changes are made on either implementation, they should be kept in sync with each other. + + issuerOIDCURL := issuerURL.JoinPath(".well-known/openid-configuration") + var oidcConfig AuthorizedIntegrationOpenIDConfiguration + if err := AuthorizedIntegrationFetchJSON(issuerOIDCURL.String(), &oidcConfig); err != nil { + return fmt.Errorf("%w: error when fetching .well-known/openid-configuration from %s: %w", ErrInvalidIssuer, issuerOIDCURL, err) + } + if oidcConfig.Issuer != issuer { + return fmt.Errorf("%w: .well-known/openid-configuration from %s has issuer %q, but input issuer was %q", ErrInvalidIssuer, issuerOIDCURL, oidcConfig.Issuer, issuer) + } else if len(oidcConfig.IDTokenSigningAlgValuesSupported) == 0 { + return fmt.Errorf("%w: .well-known/openid-configuration from %s lacks required field id_token_signing_alg_values_supported", ErrInvalidIssuer, issuerOIDCURL) + } else if oidcConfig.JwksURI == "" { + return fmt.Errorf("%w: .well-known/openid-configuration from %s lacks required field jwks_uri", ErrInvalidIssuer, issuerOIDCURL) + } + + jwksURI, err := url.Parse(oidcConfig.JwksURI) + if err != nil { + return fmt.Errorf("%w: .well-known/openid-configuration from %s has invalid jwks_uri: %w", ErrInvalidIssuer, issuerOIDCURL, err) + } else if jwksURI.Host != issuerURL.Host { + return fmt.Errorf("%w: .well-known/openid-configuration from %s has jwks_uri host mismatch: must be the same as issuer host %q, but was %q", ErrInvalidIssuer, issuerOIDCURL, issuerURL.Host, jwksURI.Host) + } + + var keys AuthorizedIntegrationOpenIDKeys + if err := AuthorizedIntegrationFetchJSON(oidcConfig.JwksURI, &keys); err != nil { + return fmt.Errorf("%w: error when fetching JWKS from %s: %w", ErrInvalidIssuer, oidcConfig.JwksURI, err) + } else if len(keys.Keys) == 0 { + return fmt.Errorf("%w: fetching JWKS from %s had zero keys", ErrInvalidIssuer, oidcConfig.JwksURI) + } + + return nil +} + +func validateClaimRules(cr *auth_model.ClaimRules, path string) error { + if cr == nil { + return fmt.Errorf("%w: claim rules are nil at %s", ErrInvalidClaimRules, path) + } + + for ruleIndex, r := range cr.Rules { + if r.Claim == "" { + return fmt.Errorf("%w: claim is missing at %s[%d]", ErrInvalidClaimRules, path, ruleIndex) + } + switch r.Comparison { + case auth_model.ClaimEqual: + if r.Value == "" { + return fmt.Errorf("%w: claim value missing at %s[%d].value", ErrInvalidClaimRules, path, ruleIndex) + } + case auth_model.ClaimGlob: + if r.Value == "" { + return fmt.Errorf("%w: claim value missing at %s[%d].value", ErrInvalidClaimRules, path, ruleIndex) + } else if _, err := glob.Compile(r.Value); err != nil { + return fmt.Errorf("%w: claim glob invalid at %s[%d].value: %w", ErrInvalidClaimRules, path, ruleIndex, err) + } + case auth_model.ClaimIn: + if len(r.Values) == 0 { + return fmt.Errorf("%w: claim values missing at %s[%d].values", ErrInvalidClaimRules, path, ruleIndex) + } + case auth_model.ClaimGlobIn: + if len(r.Values) == 0 { + return fmt.Errorf("%w: claim values missing at %s[%d].values", ErrInvalidClaimRules, path, ruleIndex) + } + for globIndex, g := range r.Values { + if g == "" { + return fmt.Errorf("%w: claim glob empty string invalid, would match anything, at %s[%d].values[%d]", ErrInvalidClaimRules, path, ruleIndex, globIndex) + } else if _, err := glob.Compile(g); err != nil { + return fmt.Errorf("%w: claim glob invalid at %s[%d].values[%d]: %w", ErrInvalidClaimRules, path, ruleIndex, globIndex, err) + } + } + case auth_model.ClaimNested: + if err := validateClaimRules(r.Nested, fmt.Sprintf("%s.%s", path, r.Claim)); err != nil { + return err + } + default: + return fmt.Errorf("%w: compare %q is not valid at %s[%d]", ErrInvalidClaimRules, r.Comparison, path, ruleIndex) + } + } + + return nil +} diff --git a/services/auth/method/authorized_integration_oidc.go b/services/auth/authorized_integration_oidc.go similarity index 89% rename from services/auth/method/authorized_integration_oidc.go rename to services/auth/authorized_integration_oidc.go index 2e35b63ace..cf2882f257 100644 --- a/services/auth/method/authorized_integration_oidc.go +++ b/services/auth/authorized_integration_oidc.go @@ -1,18 +1,18 @@ // Copyright 2026 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: GPL-3.0-or-later -package method +package auth // Response structure for a JWT issuer's `${iss}/.well-known/openid-configuration` URL endpoint; this is pared down to // the relevant entries for authorized integrations to inspect from the remote issuer. -type openIDConfiguration struct { +type AuthorizedIntegrationOpenIDConfiguration struct { Issuer string `json:"issuer"` JwksURI string `json:"jwks_uri"` IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported"` } // Response structure for a JSON Web Key Set, which is typically read from the JwksURI field of [openIDConfiguration]. -type openIDKeys struct { +type AuthorizedIntegrationOpenIDKeys struct { // Typically map[string]string, for fields like "kty", "alg", "use", "kid", "n", "e", but also string:any for fields // like x5c which are []string. We currently don't parse any fields that aren't string, but we need to Unmarshal // into this field successfully in those cases. diff --git a/services/auth/method/authorized_integration_oidc_test.go b/services/auth/authorized_integration_oidc_test.go similarity index 97% rename from services/auth/method/authorized_integration_oidc_test.go rename to services/auth/authorized_integration_oidc_test.go index b394ab0c01..a427812aa6 100644 --- a/services/auth/method/authorized_integration_oidc_test.go +++ b/services/auth/authorized_integration_oidc_test.go @@ -1,7 +1,7 @@ // Copyright 2026 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: GPL-3.0-or-later -package method +package auth import ( "testing" @@ -232,19 +232,19 @@ const awsJWKS = ` func TestParseOpenIDConfiguration(t *testing.T) { t.Run("Forgejo", func(t *testing.T) { - var retval openIDConfiguration + var retval AuthorizedIntegrationOpenIDConfiguration data := []byte(forgejoOIDC) require.NoError(t, json.Unmarshal(data, &retval)) assert.Equal(t, "https://example.org/api/actions/.well-known/keys", retval.JwksURI) }) t.Run("GitHub", func(t *testing.T) { - var retval openIDConfiguration + var retval AuthorizedIntegrationOpenIDConfiguration data := []byte(githubOIDC) require.NoError(t, json.Unmarshal(data, &retval)) assert.Equal(t, "https://token.actions.githubusercontent.com/.well-known/jwks", retval.JwksURI) }) t.Run("AWS", func(t *testing.T) { - var retval openIDConfiguration + var retval AuthorizedIntegrationOpenIDConfiguration data := []byte(awsOIDC) require.NoError(t, json.Unmarshal(data, &retval)) assert.Equal(t, "https://a103a2cc-b461-473d-84fe-6c4f6d45af88.tokens.sts.global.api.aws/.well-known/jwks.json", retval.JwksURI) @@ -253,19 +253,19 @@ func TestParseOpenIDConfiguration(t *testing.T) { func TestParseJSONWebKeySet(t *testing.T) { t.Run("Forgejo", func(t *testing.T) { - var retval openIDKeys + var retval AuthorizedIntegrationOpenIDKeys data := []byte(forgejoJWKS) require.NoError(t, json.Unmarshal(data, &retval)) assert.Len(t, retval.Keys, 1) }) t.Run("GitHub", func(t *testing.T) { - var retval openIDKeys + var retval AuthorizedIntegrationOpenIDKeys data := []byte(githubJWKS) require.NoError(t, json.Unmarshal(data, &retval)) assert.Len(t, retval.Keys, 4) }) t.Run("AWS", func(t *testing.T) { - var retval openIDKeys + var retval AuthorizedIntegrationOpenIDKeys data := []byte(awsJWKS) require.NoError(t, json.Unmarshal(data, &retval)) assert.Len(t, retval.Keys, 2) diff --git a/services/auth/authorized_integration_test.go b/services/auth/authorized_integration_test.go new file mode 100644 index 0000000000..0dcaf6424b --- /dev/null +++ b/services/auth/authorized_integration_test.go @@ -0,0 +1,537 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package auth + +import ( + "fmt" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "forgejo.org/models/auth" + "forgejo.org/models/unittest" + "forgejo.org/modules/json" + "forgejo.org/modules/jwtx" + "forgejo.org/modules/test" + "forgejo.org/services/authz" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateAuthorizedIntegration(t *testing.T) { + ii := NewMockInternalIssuer(t) + ii.On("IssuerPlaceholder").Return("urn:forgejo:authorized-issuer:internal:test2") + RegisterInternalIssuerForTesting(t, "/fake-jwt-issuer", ii) + + makeValid := func() *auth.AuthorizedIntegration { + return &auth.AuthorizedIntegration{ + Name: "Test authorized integration", + ResourceAllRepos: true, + Scope: auth.AccessTokenScopeReadRepository, + UI: auth.AuthorizedIntegrationUIGeneric, + Issuer: "urn:forgejo:authorized-issuer:internal:test2", + ClaimRules: &auth.ClaimRules{}, + } + } + + t.Run("valid - all access", func(t *testing.T) { + ai := makeValid() + ai.ResourceAllRepos = true + ai.Scope = auth.AccessTokenScopeReadRepository + err := ValidateAuthorizedIntegration(ai, nil) + require.NoError(t, err) + }) + + t.Run("valid - specified repos", func(t *testing.T) { + ai := makeValid() + ai.ResourceAllRepos = false + ai.Scope = auth.AccessTokenScopeReadRepository + resources := []*auth.AuthorizedIntegResourceRepo{{RepoID: 12}} + err := ValidateAuthorizedIntegration(ai, resources) + require.NoError(t, err) + }) + + t.Run("invalid - no specified repos", func(t *testing.T) { + ai := makeValid() + ai.ResourceAllRepos = false + ai.Scope = auth.AccessTokenScopeReadRepository + resources := []*auth.AuthorizedIntegResourceRepo{} + err := ValidateAuthorizedIntegration(ai, resources) + require.ErrorIs(t, err, authz.ErrSpecifiedReposNone) + }) + + t.Run("invalid - specified repos & public-only", func(t *testing.T) { + ai := makeValid() + ai.ResourceAllRepos = false + ai.Scope = auth.AccessTokenScope(strings.Join([]string{string(auth.AccessTokenScopePublicOnly), string(auth.AccessTokenScopeReadRepository)}, ",")) + resources := []*auth.AuthorizedIntegResourceRepo{{RepoID: 12}} + err := ValidateAuthorizedIntegration(ai, resources) + require.ErrorIs(t, err, authz.ErrSpecifiedReposNoPublicOnly) + }) + + t.Run("invalid - specified repos unsupported scopes", func(t *testing.T) { + ai := makeValid() + ai.ResourceAllRepos = false + ai.Scope = auth.AccessTokenScopeReadAdmin + resources := []*auth.AuthorizedIntegResourceRepo{{RepoID: 12}} + err := ValidateAuthorizedIntegration(ai, resources) + require.ErrorIs(t, err, authz.ErrSpecifiedReposInvalidScope) + require.ErrorContains(t, err, string(auth.AccessTokenScopeReadAdmin)) + }) + + t.Run("invalid - missing UI", func(t *testing.T) { + ai := makeValid() + ai.UI = "" + err := ValidateAuthorizedIntegration(ai, nil) + require.ErrorIs(t, err, ErrAuthorizedIntegrationBadUI) + require.ErrorContains(t, err, "invalid UI: \"\"") + }) + + t.Run("invalid - missing name", func(t *testing.T) { + ai := makeValid() + ai.Name = "" + err := ValidateAuthorizedIntegration(ai, nil) + var mfe *MissingFieldError + require.ErrorAs(t, err, &mfe) + assert.Equal(t, "Name", mfe.Field) + }) + + t.Run("invalid - checks external issuer name", func(t *testing.T) { + ai := makeValid() + ai.Issuer = "ftp://example.com/" + err := ValidateAuthorizedIntegration(ai, nil) + require.ErrorIs(t, err, ErrInvalidIssuer) + }) + + t.Run("invalid - checks claims issuer name", func(t *testing.T) { + ai := makeValid() + ai.ClaimRules = &auth.ClaimRules{Rules: []auth.ClaimRule{{}}} + err := ValidateAuthorizedIntegration(ai, nil) + require.ErrorIs(t, err, ErrInvalidClaimRules) + }) +} + +func TestInsertAuthorizedIntegration(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + ii := NewMockInternalIssuer(t) + ii.On("IssuerPlaceholder").Return("urn:forgejo:authorized-issuer:internal:test3") + RegisterInternalIssuerForTesting(t, "/fake-jwt-issuer", ii) + + t.Run("success inserts w/ repos", func(t *testing.T) { + ai := &auth.AuthorizedIntegration{ + UserID: 2, + UI: auth.AuthorizedIntegrationUIGeneric, + ResourceAllRepos: false, + ClaimRules: &auth.ClaimRules{}, + Name: " Magical AI ", + Scope: auth.AccessTokenScopeReadRepository, + Issuer: "urn:forgejo:authorized-issuer:internal:test3", + } + rr := []*auth.AuthorizedIntegResourceRepo{ + { + RepoID: 2, + }, + } + + err := InsertAuthorizedIntegration(t.Context(), ai, rr) + require.NoError(t, err) + + fromDB := unittest.AssertExistsAndLoadBean(t, &auth.AuthorizedIntegration{ID: ai.ID}) + assert.Equal(t, "Magical AI", fromDB.Name) + + // IntegID should have been initialized and the repo-specific record saved + res := unittest.AssertExistsAndLoadBean(t, &auth.AuthorizedIntegResourceRepo{IntegID: ai.ID}) + assert.EqualValues(t, 2, res.RepoID) + }) + + t.Run("validates data", func(t *testing.T) { + ai := &auth.AuthorizedIntegration{ + UserID: 2, + UI: auth.AuthorizedIntegrationUIGeneric, + ResourceAllRepos: false, + ClaimRules: &auth.ClaimRules{}, + Name: " Magical AI ", + Issuer: "urn:forgejo:authorized-issuer:internal:test3", + } + err := InsertAuthorizedIntegration(t.Context(), ai, nil) + require.ErrorIs(t, err, authz.ErrSpecifiedReposNone) + }) +} + +func TestUpdateAuthorizedIntegration(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + ii := NewMockInternalIssuer(t) + ii.On("IssuerPlaceholder").Return("urn:forgejo:authorized-issuer:internal:test4") + RegisterInternalIssuerForTesting(t, "/fake-jwt-issuer", ii) + + prep := func(t *testing.T) (*auth.AuthorizedIntegration, []*auth.AuthorizedIntegResourceRepo) { + ai := &auth.AuthorizedIntegration{ + UserID: 2, + UI: auth.AuthorizedIntegrationUIGeneric, + ResourceAllRepos: false, + ClaimRules: &auth.ClaimRules{}, + Name: " Magical AI ", + Scope: auth.AccessTokenScopeReadRepository, + Issuer: "urn:forgejo:authorized-issuer:internal:test4", + } + rr := []*auth.AuthorizedIntegResourceRepo{ + { + RepoID: 2, + }, + } + err := InsertAuthorizedIntegration(t.Context(), ai, rr) + require.NoError(t, err) + return ai, rr + } + + t.Run("update basic fields", func(t *testing.T) { + ai, rr := prep(t) + ai.Description = "This is the description field." + + err := UpdateAuthorizedIntegration(t.Context(), ai, rr) + require.NoError(t, err) + + fromDB := unittest.AssertExistsAndLoadBean(t, &auth.AuthorizedIntegration{ID: ai.ID}) + assert.Equal(t, "Magical AI", fromDB.Name) + assert.Equal(t, "This is the description field.", fromDB.Description) + unittest.AssertCount(t, &auth.AuthorizedIntegResourceRepo{IntegID: ai.ID}, 1) + }) + + t.Run("update remove resource repos", func(t *testing.T) { + ai, _ := prep(t) + ai.ResourceAllRepos = true + + err := UpdateAuthorizedIntegration(t.Context(), ai, nil) + require.NoError(t, err) + + unittest.AssertCount(t, &auth.AuthorizedIntegResourceRepo{IntegID: ai.ID}, 0) + }) + + t.Run("update add resource repos", func(t *testing.T) { + ai, _ := prep(t) + rr := []*auth.AuthorizedIntegResourceRepo{ + { + RepoID: 2, + }, + { + RepoID: 3, + }, + } + + err := UpdateAuthorizedIntegration(t.Context(), ai, rr) + require.NoError(t, err) + + unittest.AssertCount(t, &auth.AuthorizedIntegResourceRepo{IntegID: ai.ID}, 2) + }) + + t.Run("validates data", func(t *testing.T) { + ai, _ := prep(t) + err := InsertAuthorizedIntegration(t.Context(), ai, nil) + require.ErrorIs(t, err, authz.ErrSpecifiedReposNone) + }) +} + +type ExternalIssuerTester struct { + t *testing.T + jwtSigningKey jwtx.SigningKey + testServer *httptest.Server + resetHTTPClient func() + tweaks []tweak + issuer string +} + +func newEITester(t *testing.T, tweaks ...tweak) *ExternalIssuerTester { + eit := &ExternalIssuerTester{ + t: t, + tweaks: tweaks, + } + + var jwtSigningKey jwtx.SigningKey + var err error + keyPath := filepath.Join(t.TempDir(), "jwt-rsa-2048.priv") + jwtSigningKey, err = jwtx.InitAsymmetricSigningKey(keyPath, "RS256") + require.NoError(t, err) + eit.jwtSigningKey = jwtSigningKey + + eit.testServer = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/actions/.well-known/openid-configuration" { + retval := &AuthorizedIntegrationOpenIDConfiguration{ + Issuer: eit.issuer, + IDTokenSigningAlgValuesSupported: []string{"RS256"}, + JwksURI: fmt.Sprintf("%s/.keys", eit.issuer), + } + for _, tweak := range eit.tweaks { + if tweak, is := tweak.(openIDTweak); is { + tweak(retval) + } + } + err := json.NewEncoder(w).Encode(retval) + require.NoError(t, err) + return + } + if r.URL.Path == "/api/actions/.keys" { + jwk, err := eit.jwtSigningKey.ToJWK() + require.NoError(t, err) + jwk["use"] = "sig" + jwkMapAny := make(map[string]any, len(jwk)) + for k, v := range jwk { + jwkMapAny[k] = v // convert map[string]string -> map[string]any + } + retval := &AuthorizedIntegrationOpenIDKeys{ + Keys: []map[string]any{jwkMapAny}, + } + for _, tweak := range eit.tweaks { + if jwksTweak, is := tweak.(jwksTweak); is { + jwksTweak(retval) + } + } + _ = json.NewEncoder(w).Encode(retval) // no error checking -- some tests abort read + return + } + w.WriteHeader(http.StatusNotFound) + })) + + eit.issuer = fmt.Sprintf("%s/api/actions", eit.testServer.URL) + + // trust TLS cert of our mock client by inserting the test client for our test server into the global aiHTTPClient + eit.resetHTTPClient = test.MockVariableValue( + &GetAuthorizedIntegrationHTTPClient, + func() *http.Client { + return eit.testServer.Client() + }) + + return eit +} + +func (eit *ExternalIssuerTester) close() { + eit.resetHTTPClient() + eit.testServer.Close() +} + +type tweak any + +type openIDTweak func(*AuthorizedIntegrationOpenIDConfiguration) + +type jwksTweak func(*AuthorizedIntegrationOpenIDKeys) + +func TestValidateExternalIssuer(t *testing.T) { + t.Run("valid", func(t *testing.T) { + eit := newEITester(t) + defer eit.close() + err := validateExternalIssuer(eit.issuer) + require.NoError(t, err) + }) + + t.Run("unparseable URL", func(t *testing.T) { + err := validateExternalIssuer("hello? \x7f is this a URL?") + require.ErrorIs(t, err, ErrInvalidIssuer) + require.ErrorContains(t, err, "failed parsing issuer URL") + }) + + t.Run("404 OIDC", func(t *testing.T) { + eit := newEITester(t) + defer eit.close() + err := validateExternalIssuer(eit.issuer + "/wrong-path") + require.ErrorIs(t, err, ErrInvalidIssuer) + require.ErrorContains(t, err, "non-OK response code: 404 Not Found") + }) + + t.Run("mismatched issuer", func(t *testing.T) { + eit := newEITester(t, + openIDTweak(func(oidc *AuthorizedIntegrationOpenIDConfiguration) { + oidc.Issuer = "https://whoops.example.org" + })) + defer eit.close() + err := validateExternalIssuer(eit.issuer) + require.ErrorIs(t, err, ErrInvalidIssuer) + require.ErrorContains(t, err, "has issuer \"https://whoops.example.org\", but input issuer was") + }) + + t.Run("no signing alg issuer", func(t *testing.T) { + eit := newEITester(t, + openIDTweak(func(oidc *AuthorizedIntegrationOpenIDConfiguration) { + oidc.IDTokenSigningAlgValuesSupported = nil + })) + defer eit.close() + err := validateExternalIssuer(eit.issuer) + require.ErrorIs(t, err, ErrInvalidIssuer) + require.ErrorContains(t, err, "lacks required field id_token_signing_alg_values_supported") + }) + + t.Run("no jwks_uri", func(t *testing.T) { + eit := newEITester(t, + openIDTweak(func(oidc *AuthorizedIntegrationOpenIDConfiguration) { + oidc.JwksURI = "" + })) + defer eit.close() + err := validateExternalIssuer(eit.issuer) + require.ErrorIs(t, err, ErrInvalidIssuer) + require.ErrorContains(t, err, "lacks required field jwks_uri") + }) + + t.Run("remote jwks_uri", func(t *testing.T) { + eit := newEITester(t, + openIDTweak(func(oidc *AuthorizedIntegrationOpenIDConfiguration) { + oidc.JwksURI = "https://example.org/.keys" + })) + defer eit.close() + err := validateExternalIssuer(eit.issuer) + require.ErrorIs(t, err, ErrInvalidIssuer) + require.ErrorContains(t, err, "jwks_uri host mismatch") + }) + + t.Run("empty JWKS", func(t *testing.T) { + eit := newEITester(t, + jwksTweak(func(keys *AuthorizedIntegrationOpenIDKeys) { + keys.Keys = nil + })) + defer eit.close() + err := validateExternalIssuer(eit.issuer) + require.ErrorIs(t, err, ErrInvalidIssuer) + require.ErrorContains(t, err, "had zero keys") + }) +} + +func TestValidateClaimRules(t *testing.T) { + t.Run("nil", func(t *testing.T) { + err := validateClaimRules(nil, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim rules are nil") + }) + + t.Run("missing claim", func(t *testing.T) { + err := validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: ""}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim is missing at root[0]") + }) + + t.Run("eq", func(t *testing.T) { + err := validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimEqual, Value: ""}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim value missing at root[0].value") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimEqual, Value: "present"}}, + }, "root") + require.NoError(t, err) + }) + + t.Run("glob", func(t *testing.T) { + err := validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimGlob, Value: ""}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim value missing at root[0].value") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimGlob, Value: "abc["}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim glob invalid at root[0].value") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimGlob, Value: "pre*ent"}}, + }, "root") + require.NoError(t, err) + }) + + t.Run("in", func(t *testing.T) { + err := validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimIn, Values: nil}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim values missing at root[0].values") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimIn, Values: []string{}}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim values missing at root[0].values") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimIn, Values: []string{"1", "2"}}}, + }, "root") + require.NoError(t, err) + }) + + t.Run("glob-in", func(t *testing.T) { + err := validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimGlobIn, Values: nil}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim values missing at root[0].values") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimGlobIn, Values: []string{}}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim values missing at root[0].values") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimGlobIn, Values: []string{"abc", "abc["}}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim glob invalid at root[0].values[1]") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimGlobIn, Values: []string{"1", "2"}}}, + }, "root") + require.NoError(t, err) + }) + + t.Run("nested", func(t *testing.T) { + err := validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{{Claim: "c", Comparison: auth.ClaimNested}}, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim rules are nil at root.c") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{ + { + Claim: "c", + Comparison: auth.ClaimNested, + Nested: &auth.ClaimRules{ + Rules: []auth.ClaimRule{ + { + Claim: "d", + Comparison: auth.ClaimEqual, + }, + }, + }, + }, + }, + }, "root") + require.ErrorIs(t, err, ErrInvalidClaimRules) + require.ErrorContains(t, err, "claim value missing at root.c[0].value") + + err = validateClaimRules(&auth.ClaimRules{ + Rules: []auth.ClaimRule{ + { + Claim: "c", + Comparison: auth.ClaimNested, + Nested: &auth.ClaimRules{ + Rules: []auth.ClaimRule{ + { + Claim: "d", + Comparison: auth.ClaimEqual, + Value: "123", + }, + }, + }, + }, + }, + }, "root") + require.NoError(t, err) + }) +} diff --git a/services/auth/internal_issuer.go b/services/auth/internal_issuer.go new file mode 100644 index 0000000000..547b546dd8 --- /dev/null +++ b/services/auth/internal_issuer.go @@ -0,0 +1,67 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package auth + +import ( + "testing" + + "forgejo.org/modules/jwtx" +) + +var internalIssuers = make(map[string]InternalIssuer) + +// Authorized Integrations can verify the signature of JWTs that the application itself generated without requiring +// remote access, and in a manner that is flexible to changes in [setting.AppURL]. +// +// For example, Forgejo Actions is often used to access Forgejo with a JWT, by setting `enable-openid-connect: true` in +// a workflow. Without any special support for this internal access situation, problems would occur: +// +// 1. Forgejo would need to make an HTTP request to itself to get the valid public key for the JWT, in order to validate +// its signature. This is a waste of resources, and introduces a self-DoS risk. +// +// 2. Forgejo would need to be available via TLS in order for Actions to make service calls to Forgejo with that JWT +// (due to the TLS requirement for public key fetching). +// +// 3. Authorized Integrations would need to be saved with the `issuer` URL of Forgejo. If Forgejo's own +// [setting.AppURL] changed, all the persisted records in the database would become incorrect. +// +// Internal Issuers work by registering a URL suffix like "api/actions". When a JWT is received with an issuer +// matching [setting.AppURL] and the registered URL suffix, then the [InternalIssuer] interface is used to access the +// JWT public key, and the value to be saved in the Authorized Integrations table as the issuer. +func RegisterInternalIssuer(urlSuffix string, internalIssuer InternalIssuer) { + internalIssuers[urlSuffix] = internalIssuer +} + +// Variant of RegisterInternalIssuer which removes the registration impact in test cleanup. +func RegisterInternalIssuerForTesting(t *testing.T, urlSuffix string, internalIssuer InternalIssuer) { + orig, hadOrig := internalIssuers[urlSuffix] + internalIssuers[urlSuffix] = internalIssuer + t.Cleanup(func() { + if hadOrig { + internalIssuers[urlSuffix] = orig + } else { + delete(internalIssuers, urlSuffix) + } + }) +} + +// Retrieve an internal issuer, if one exists, for the provided URL suffix from a JWT token. For example, +// "api/actions". +func GetInternalIssuerByURLSuffix(issuerSuffix string) (InternalIssuer, bool) { + ii, ok := internalIssuers[issuerSuffix] + return ii, ok +} + +// Read access to the registered internal issuers. +func GetInternalIssuers() map[string]InternalIssuer { + return internalIssuers +} + +//mockery:generate: true +type InternalIssuer interface { + // Signing key used to validate a JWT from this internal issuer. + SigningKey() jwtx.SigningKey + // Value to store in [auth_model.AuthorizedIntegration]'s Issuer field to reflect the use of this internal issuer. + IssuerPlaceholder() string +} diff --git a/services/auth/method/authorized_integration.go b/services/auth/method/authorized_integration.go index d9fb0c10cc..be4102020d 100644 --- a/services/auth/method/authorized_integration.go +++ b/services/auth/method/authorized_integration.go @@ -4,31 +4,23 @@ package method import ( - "bufio" - "bytes" "errors" "fmt" - "io" "net/http" "net/url" "slices" "strings" - "sync" "time" auth_model "forgejo.org/models/auth" user_model "forgejo.org/models/user" - "forgejo.org/modules/cache" - "forgejo.org/modules/hostmatcher" - "forgejo.org/modules/json" "forgejo.org/modules/jwtx" "forgejo.org/modules/log" "forgejo.org/modules/optional" - "forgejo.org/modules/proxy" "forgejo.org/modules/setting" "forgejo.org/modules/timeutil" "forgejo.org/modules/util" - "forgejo.org/services/auth" + auth_service "forgejo.org/services/auth" "forgejo.org/services/authz" "github.com/gobwas/glob" @@ -36,59 +28,11 @@ import ( ) var ( - _ auth.Method = &AuthorizedIntegration{} - - aiHTTPClient *http.Client - initHTTPClient sync.Once + _ auth_service.Method = &AuthorizedIntegration{} errParseInternalServer = errors.New("internal server error") - - // Allow mocking / overridding during tests: - GetAuthorizedIntegrationHTTPClient = func() *http.Client { - initHTTPClient.Do(initAuthorizedIntegrationHTTPClient) - return aiHTTPClient - } - getCache = cache.GetCache - - internalIssuers = make(map[string]InternalIssuer) ) -// Authorized Integrations can verify the signature of JWTs that the application itself generated without requiring -// remote access, and in a manner that is flexible to changes in [setting.AppURL]. -// -// For example, Forgejo Actions is often used to access Forgejo with a JWT, by setting `enable-openid-connect: true` in -// a workflow. Without any special support for this internal access situation, problems would occur: -// -// 1. Forgejo would need to make an HTTP request to itself to get the valid public key for the JWT, in order to validate -// its signature. This is a waste of resources, and introduces a self-DoS risk. -// -// 2. Forgejo would need to be available via TLS in order for Actions to make service calls to Forgejo with that JWT -// (due to the TLS requirement for public key fetching). -// -// 3. Authorized Integrations would need to be saved with the `issuer` URL of Forgejo. If Forgejo's own -// [setting.AppURL] changed, all the persisted records in the database would become incorrect. -// -// Internal Issuers work by registering a URL suffix like "/api/actions". When a JWT is received with an issuer -// matching [setting.AppURL] and the registered URL suffix, then the [InternalIssuer] interface is used to access the -// JWT public key, and the value to be saved in the Authorized Integrations table as the issuer. -func RegisterInternalIssuer(urlSuffix string, internalIssuer InternalIssuer) { - internalIssuers[urlSuffix] = internalIssuer -} - -//mockery:generate: true -type InternalIssuer interface { - // Signing key used to validate a JWT from this internal issuer. - SigningKey() jwtx.SigningKey - // Value to store in [auth_model.AuthorizedIntegration]'s Issuer field to reflect the use of this internal issuer. - IssuerPlaceholder() string -} - -// Restrict document size to prevent resource exhaustion attack with a malicious authorized integration; largest -// real-world openid-configuration observed is about 1kB, largest JWKS is 6kB, so for both cases 16kB should be -// sufficient. If this needs to change in the future, it could be moved to a config setting -- but until a reason comes -// up it seems reasonable to keep microscopic settings out-of-sight. -const authorizedIntegrationRequestBodyLimit = int64(16 * 1024) - // Authenticates incoming requests by JWTs that are issued by an authorized integration. Authorized integrations are // stored in the database in the [auth_model.AuthorizedIntegration] table. Once authenticated, the request can perform // actions as the owner of the authorized integration, with limited access defined by the scope and resources stored on @@ -105,15 +49,15 @@ type AuthorizedIntegration struct { fixedTime *time.Time } -func (a *AuthorizedIntegration) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) auth.MethodOutput { +func (a *AuthorizedIntegration) Verify(req *http.Request, w http.ResponseWriter, _ auth_service.SessionStore) auth_service.MethodOutput { hasToken, token := tokenFromAuthorizationBearer(req).Get() if !hasToken { if !a.PermitBasic { - return &auth.AuthenticationNotAttempted{} + return &auth_service.AuthenticationNotAttempted{} } hasBasic, basicToken := tokenFromAuthorizationBasic(req).Get() if !hasBasic { - return &auth.AuthenticationNotAttempted{} + return &auth_service.AuthenticationNotAttempted{} } token = basicToken } @@ -147,10 +91,10 @@ func (a *AuthorizedIntegration) Verify(req *http.Request, w http.ResponseWriter, // Check if there's an internal issuer that matches the JWT's issuer, and if so, change `queryIssuer` to the // internal issuer's placeholder, and store `internalIssuer` for later: queryIssuer := issuer - var internalIssuer InternalIssuer + var internalIssuer auth_service.InternalIssuer issuerSuffix := strings.TrimPrefix(issuer, setting.AppURL) if issuer != issuerSuffix { // TrimPrefix will return a different string when the prefix was present - if ii, ok := internalIssuers[issuerSuffix]; ok { + if ii, ok := auth_service.GetInternalIssuerByURLSuffix(issuerSuffix); ok { internalIssuer = ii queryIssuer = internalIssuer.IssuerPlaceholder() } @@ -183,9 +127,13 @@ func (a *AuthorizedIntegration) Verify(req *http.Request, w http.ResponseWriter, return nil, fmt.Errorf("failed parsing issuer: %w", err) } + // Checks implemented here a variation of validateExternalIssuer used when creating an authorized + // integration. Where possible, if validation changes are made on either implementation, they should be + // kept in sync with each other. + issuerOIDCURL := issuerURL.JoinPath(".well-known/openid-configuration") - var oidcConfig openIDConfiguration - if err := authorizedIntegrationFetchJSON(issuerOIDCURL.String(), &oidcConfig); err != nil { + var oidcConfig auth_service.AuthorizedIntegrationOpenIDConfiguration + if err := auth_service.AuthorizedIntegrationFetchJSON(issuerOIDCURL.String(), &oidcConfig); err != nil { return nil, fmt.Errorf("error when fetching .well-known/openid-configuration from %s: %w", issuerOIDCURL, err) } @@ -206,8 +154,8 @@ func (a *AuthorizedIntegration) Verify(req *http.Request, w http.ResponseWriter, // but until a real-world case comes up where that is needed, this is a safety-first restriction. return nil, fmt.Errorf("jwks_uri host mismatch: must be the same as issuer host %q, but was %q", issuerURL.Host, jwksURI.Host) } - var keys openIDKeys - if err := authorizedIntegrationFetchJSON(oidcConfig.JwksURI, &keys); err != nil { + var keys auth_service.AuthorizedIntegrationOpenIDKeys + if err := auth_service.AuthorizedIntegrationFetchJSON(oidcConfig.JwksURI, &keys); err != nil { return nil, fmt.Errorf("error when fetching JWKS from %s: %w", oidcConfig.JwksURI, err) } @@ -244,18 +192,18 @@ func (a *AuthorizedIntegration) Verify(req *http.Request, w http.ResponseWriter, ) if err != nil && errors.Is(err, errParseInternalServer) { // Errors from parsing marked errParseInternalServer are AuthenticationError, not incorrect creds: - return &auth.AuthenticationError{Error: err} + return &auth_service.AuthenticationError{Error: err} } else if err != nil { - return &auth.AuthenticationAttemptedIncorrectCredential{Error: fmt.Errorf("authorized integration: parse JWT error: %w", err)} + return &auth_service.AuthenticationAttemptedIncorrectCredential{Error: fmt.Errorf("authorized integration: parse JWT error: %w", err)} } else if !parsedToken.Valid { - return &auth.AuthenticationAttemptedIncorrectCredential{Error: errors.New("authorized integration: JWT not valid")} + return &auth_service.AuthenticationAttemptedIncorrectCredential{Error: errors.New("authorized integration: JWT not valid")} } else if authorizedIntegration == nil { // shouldn't be possible, but overly safe - return &auth.AuthenticationError{Error: errors.New("authorized integration: nil authorized integration")} + return &auth_service.AuthenticationError{Error: errors.New("authorized integration: nil authorized integration")} } u, err := user_model.GetUserByID(req.Context(), authorizedIntegration.UserID) if err != nil { - return &auth.AuthenticationError{Error: fmt.Errorf("authorized integration: GetUserByID: %w", err)} + return &auth_service.AuthenticationError{Error: fmt.Errorf("authorized integration: GetUserByID: %w", err)} } if err = authorizedIntegration.UpdateLastUsed(req.Context()); err != nil { @@ -264,17 +212,17 @@ func (a *AuthorizedIntegration) Verify(req *http.Request, w http.ResponseWriter, reducer, err := authz.GetAuthorizationReducerForAuthorizedIntegration(req.Context(), authorizedIntegration) if err != nil { - return &auth.AuthenticationError{Error: fmt.Errorf("authorized integration GetAuthorizationReducerForAuthorizedIntegration: %w", err)} + return &auth_service.AuthenticationError{Error: fmt.Errorf("authorized integration GetAuthorizationReducerForAuthorizedIntegration: %w", err)} } var optionalExp optional.Option[timeutil.TimeStamp] if exp, err := parsedToken.Claims.GetExpirationTime(); err != nil { - return &auth.AuthenticationError{Error: fmt.Errorf("authorized integration GetExpirationTime: %w", err)} + return &auth_service.AuthenticationError{Error: fmt.Errorf("authorized integration GetExpirationTime: %w", err)} } else if exp != nil { optionalExp = optional.Some(timeutil.TimeStamp(exp.Unix())) } - return &auth.AuthenticationSuccess{ + return &auth_service.AuthenticationSuccess{ Result: &authorizedIntegrationAuthenticationResult{ user: u, scope: authorizedIntegration.Scope, @@ -284,131 +232,6 @@ func (a *AuthorizedIntegration) Verify(req *http.Request, w http.ResponseWriter, } } -func initAuthorizedIntegrationHTTPClient() { - blockList := hostmatcher.ParseSimpleMatchList("authorized_integration.BLOCKED_DOMAINS", setting.AuthorizedIntegration.BlockedDomains) - - allowList := hostmatcher.ParseSimpleMatchList("authorized_integration.ALLOWED_DOMAINS", setting.AuthorizedIntegration.AllowedDomains) - if allowList.IsEmpty() { - // the default policy is that authorized integrations can access external hosts - allowList.AppendBuiltin(hostmatcher.MatchBuiltinExternal) - } - if setting.AuthorizedIntegration.AllowLocalNetworks { - allowList.AppendBuiltin(hostmatcher.MatchBuiltinPrivate) - allowList.AppendBuiltin(hostmatcher.MatchBuiltinLoopback) - } - - aiHTTPClient = &http.Client{ - Timeout: setting.AuthorizedIntegration.RequestTimeout, - Transport: &http.Transport{ - Proxy: proxy.Proxy(), - DialContext: hostmatcher.NewDialContext("authorized_integration", allowList, blockList, setting.Proxy.ProxyURLFixed), - }, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - // It might be possible to come up with some reasonable capability to support redirects -- such as - // keeping them within the same issuer host? -- but there are risks that this can be used for SSRF - // attacks. In the face of those risks, and with a lack of real-world use-cases, disable redirects. - return errors.New("authorized integration: HTTP redirects are disabled") - }, - } -} - -func authorizedIntegrationCacheKey(urlString string) string { - return fmt.Sprintf("auth-int-remote:%s", urlString) -} - -func authorizedIntegrationCacheGetJSON[K any](urlString string, v *K) bool { - conn := getCache() - if conn == nil { - return false - } - - cachedAny := conn.Get(authorizedIntegrationCacheKey(urlString)) - if cachedAny == nil { - return false - } - cachedBytes, ok := cachedAny.([]byte) - if !ok { - cachedString, ok := cachedAny.(string) - if !ok { - log.Error("cached content was not []byte or string, but was %T", cachedAny) - return false - } - cachedBytes = []byte(cachedString) - } - - err := json.Unmarshal(cachedBytes, &v) - if err != nil { - // This error case shouldn't occur, as we only store data in the cache once we're sure we could unmarshal it. - // If it does occur, log and fallback to treating as uncached. - log.Error("failed to Unmarshal cached content: %s", err) - // Caller may reuse `v` in a future unmarshal/decode call, and failure here may have polluted it. - var zeroValue K - *v = zeroValue - return false - } - - return true -} - -func authorizedIntegrationCacheSetJSON(urlString string, buf []byte) { - conn := getCache() - if conn == nil { - return - } - err := conn.Put(authorizedIntegrationCacheKey(urlString), buf, int64(setting.AuthorizedIntegration.CacheTTL.Seconds())) - if err != nil { - log.Error("failed to put cache: %s", err) - } -} - -func authorizedIntegrationFetchJSON[K any](urlString string, v *K) error { - parsedURL, err := url.Parse(urlString) - if err != nil { - return fmt.Errorf("failed parsing URL %q: %w", urlString, err) - } - // Fetching openid-connect or JWKS needs to come from a source that is authentic, and therefore only `https` is - // supported. This also protects against a trusted issuer being configured maliciously as `file://` or a JKWS URI - // being `file://` -- the HTTP client won't permit that, but, extra safety doesn't hurt. - if parsedURL.Scheme != "https" { - return fmt.Errorf("unsupported URL scheme: %q", parsedURL.String()) - } - - // Check our cache, save a remote HTTP interaction. - if authorizedIntegrationCacheGetJSON(urlString, v) { - return nil - } - - resp, err := GetAuthorizedIntegrationHTTPClient().Get(parsedURL.String()) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("non-OK response code: %s", resp.Status) - } - - bodyReader := io.LimitReader(resp.Body, authorizedIntegrationRequestBodyLimit) - var buf bytes.Buffer - _, err = io.Copy(bufio.NewWriter(&buf), bodyReader) - if err != nil { - return fmt.Errorf("read from remote error: %w", err) - } - - err = json.Unmarshal(buf.Bytes(), &v) - if err != nil { - // If a decoding error is hit, decorate with information about the limited body size so that it doesn't look - // like the remote server provided an incomplete response. err should be something like `io.UnexpectedEOF` in - // this case, but it actually isn't, so don't bother trying to detect precisely. - return fmt.Errorf("failed to decode (response body restricted to %d bytes): %w", authorizedIntegrationRequestBodyLimit, err) - } - - // Successfully decoded the response -- cache the raw bytes for later access. - authorizedIntegrationCacheSetJSON(urlString, buf.Bytes()) - - return nil -} - // Compare a map[string]any of incoming claims against an array of claim rules. All rules must match successfully or // else an error with the mismatch detail is returned. func (a *AuthorizedIntegration) checkClaims(incomingClaims any, stored *auth_model.ClaimRules) error { diff --git a/services/auth/method/authorized_integration_test.go b/services/auth/method/authorized_integration_test.go index 7ffd686e9b..61b6731375 100644 --- a/services/auth/method/authorized_integration_test.go +++ b/services/auth/method/authorized_integration_test.go @@ -429,7 +429,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("mismatch openid metadata", func(t *testing.T) { ait := newAITester(t, - openIDTweak(func(oidc *openIDConfiguration, _ *AuthorizedIntegrationTester) { + openIDTweak(func(oidc *auth.AuthorizedIntegrationOpenIDConfiguration, _ *AuthorizedIntegrationTester) { oidc.Issuer = "https://whoops.example.org" })) defer ait.close() @@ -451,7 +451,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("signing alg values supported doesn't include in-use alg", func(t *testing.T) { ait := newAITester(t, - openIDTweak(func(oidc *openIDConfiguration, _ *AuthorizedIntegrationTester) { + openIDTweak(func(oidc *auth.AuthorizedIntegrationOpenIDConfiguration, _ *AuthorizedIntegrationTester) { oidc.IDTokenSigningAlgValuesSupported = []string{"WEIRD"} })) defer ait.close() @@ -517,7 +517,7 @@ func TestAuthorizedIntegration(t *testing.T) { require.NoError(t, err) return jwtSigningKey }), - openIDTweak(func(oidc *openIDConfiguration, _ *AuthorizedIntegrationTester) { + openIDTweak(func(oidc *auth.AuthorizedIntegrationOpenIDConfiguration, _ *AuthorizedIntegrationTester) { oidc.IDTokenSigningAlgValuesSupported = []string{alg} }), ) @@ -531,7 +531,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("JWKS", func(t *testing.T) { t.Run("jwks_uri host mismatch", func(t *testing.T) { ait := newAITester(t, - openIDTweak(func(oidc *openIDConfiguration, ait *AuthorizedIntegrationTester) { + openIDTweak(func(oidc *auth.AuthorizedIntegrationOpenIDConfiguration, ait *AuthorizedIntegrationTester) { oidc.JwksURI = "https://whoops.example.org/.keys" })) defer ait.close() @@ -542,7 +542,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("non-HTTPS JWKS address", func(t *testing.T) { ait := newAITester(t, - openIDTweak(func(oidc *openIDConfiguration, ait *AuthorizedIntegrationTester) { + openIDTweak(func(oidc *auth.AuthorizedIntegrationOpenIDConfiguration, ait *AuthorizedIntegrationTester) { oidc.JwksURI = strings.ReplaceAll(ait.testServer.URL, "https://", "http://") })) defer ait.close() @@ -553,7 +553,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("missing key", func(t *testing.T) { ait := newAITester(t, - jwksTweak(func(keys *openIDKeys) { + jwksTweak(func(keys *auth.AuthorizedIntegrationOpenIDKeys) { keys.Keys = []map[string]any{} })) defer ait.close() @@ -564,7 +564,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("alg missing", func(t *testing.T) { ait := newAITester(t, - jwksTweak(func(keys *openIDKeys) { + jwksTweak(func(keys *auth.AuthorizedIntegrationOpenIDKeys) { for k := range keys.Keys { delete(keys.Keys[k], "alg") } @@ -577,7 +577,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("alg mismatch", func(t *testing.T) { ait := newAITester(t, - jwksTweak(func(keys *openIDKeys) { + jwksTweak(func(keys *auth.AuthorizedIntegrationOpenIDKeys) { for k := range keys.Keys { keys.Keys[k]["alg"] = "WEIRD" } @@ -590,7 +590,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("use missing", func(t *testing.T) { ait := newAITester(t, - jwksTweak(func(keys *openIDKeys) { + jwksTweak(func(keys *auth.AuthorizedIntegrationOpenIDKeys) { for k := range keys.Keys { delete(keys.Keys[k], "use") } @@ -603,7 +603,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("use isn't 'sig'", func(t *testing.T) { ait := newAITester(t, - jwksTweak(func(keys *openIDKeys) { + jwksTweak(func(keys *auth.AuthorizedIntegrationOpenIDKeys) { for k := range keys.Keys { keys.Keys[k]["use"] = "enc" } @@ -616,7 +616,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("large JWKS document", func(t *testing.T) { ait := newAITester(t, - jwksTweak(func(keys *openIDKeys) { + jwksTweak(func(keys *auth.AuthorizedIntegrationOpenIDKeys) { var keyContents map[string]any for _, v := range keys.Keys { keyContents = v @@ -657,7 +657,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("cache", func(t *testing.T) { t.Run("miss and store", func(t *testing.T) { c := cache.NewMockCache(t) - defer test.MockVariableValue(&getCache, func() mc.Cache { return c })() + defer test.MockVariableValue(&auth.GetAuthorizedIntegrationCache, func() mc.Cache { return c })() defer test.MockVariableValue(&setting.AuthorizedIntegration.CacheTTL, 10*time.Minute)() var cacheKey string @@ -698,12 +698,12 @@ func TestAuthorizedIntegration(t *testing.T) { var oidcMetadata []byte var jwksData []byte ait := newAITester(t, - openIDTweak(func(oi *openIDConfiguration, ait *AuthorizedIntegrationTester) { + openIDTweak(func(oi *auth.AuthorizedIntegrationOpenIDConfiguration, ait *AuthorizedIntegrationTester) { var err error oidcMetadata, err = json.Marshal(oi) require.NoError(t, err) }), - jwksTweak(func(oi *openIDKeys) { + jwksTweak(func(oi *auth.AuthorizedIntegrationOpenIDKeys) { var err error jwksData, err = json.Marshal(oi) require.NoError(t, err) @@ -714,7 +714,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("cache returns []byte", func(t *testing.T) { c := cache.NewMockCache(t) - defer test.MockVariableValue(&getCache, func() mc.Cache { return c })() + defer test.MockVariableValue(&auth.GetAuthorizedIntegrationCache, func() mc.Cache { return c })() c.On("Get", mock.MatchedBy(func(key string) bool { @@ -732,7 +732,7 @@ func TestAuthorizedIntegration(t *testing.T) { t.Run("cache returns string", func(t *testing.T) { c := cache.NewMockCache(t) - defer test.MockVariableValue(&getCache, func() mc.Cache { return c })() + defer test.MockVariableValue(&auth.GetAuthorizedIntegrationCache, func() mc.Cache { return c })() c.On("Get", mock.MatchedBy(func(key string) bool { @@ -842,7 +842,7 @@ type AuthorizedIntegrationTester struct { testServer *httptest.Server resetHTTPClient func() tweaks []tweak - ii *MockInternalIssuer + ii *auth.MockInternalIssuer } func newAITester(t *testing.T, tweaks ...tweak) *AuthorizedIntegrationTester { @@ -876,7 +876,7 @@ func newAITester(t *testing.T, tweaks ...tweak) *AuthorizedIntegrationTester { ait.testServer = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/api/actions/.well-known/openid-configuration" { - retval := &openIDConfiguration{ + retval := &auth.AuthorizedIntegrationOpenIDConfiguration{ Issuer: ait.dbAI.Issuer, IDTokenSigningAlgValuesSupported: []string{"RS256"}, JwksURI: fmt.Sprintf("%s/.keys", ait.dbAI.Issuer), @@ -898,7 +898,7 @@ func newAITester(t *testing.T, tweaks ...tweak) *AuthorizedIntegrationTester { for k, v := range jwk { jwkMapAny[k] = v // convert map[string]string -> map[string]any } - retval := &openIDKeys{ + retval := &auth.AuthorizedIntegrationOpenIDKeys{ Keys: []map[string]any{jwkMapAny}, } for _, tweak := range ait.tweaks { @@ -913,11 +913,11 @@ func newAITester(t *testing.T, tweaks ...tweak) *AuthorizedIntegrationTester { })) // trust TLS cert of our mock client by inserting the test client for our test server into the global aiHTTPClient - ait.resetHTTPClient = test.MockVariableValue(&aiHTTPClient, ait.testServer.Client()) - // prevent self-initialization of the HTTP client during unit testing -- this means that a real client cant' be - // created and aiHTTPClient will always be nil (other than when mocked), but that's fine because we don't want to do - // external HTTP traffic in these tests - initHTTPClient.Do(func() {}) + ait.resetHTTPClient = test.MockVariableValue( + &auth.GetAuthorizedIntegrationHTTPClient, + func() *http.Client { + return ait.testServer.Client() + }) ait.dbAI = &auth_model.AuthorizedIntegration{ UserID: 2, @@ -956,8 +956,8 @@ func newInternalIssuerAITester(t *testing.T, tweaks ...tweak) *AuthorizedIntegra } innerTweaks = append(innerTweaks, tweaks...) ait := newAITester(t, innerTweaks...) - ii := NewMockInternalIssuer(t) - internalIssuers["/fake-jwt-issuer"] = ii + ii := auth.NewMockInternalIssuer(t) + auth.RegisterInternalIssuerForTesting(t, "/fake-jwt-issuer", ii) ii.On("IssuerPlaceholder").Return("urn:forgejo:authorized-issuer:internal:test1") ii.On("SigningKey").Return(ait.jwtSigningKey) ait.ii = ii @@ -1015,9 +1015,9 @@ type claimTweak func(*flexibleClaims) type aiTweak func(*AuthorizedIntegration) -type openIDTweak func(*openIDConfiguration, *AuthorizedIntegrationTester) +type openIDTweak func(*auth.AuthorizedIntegrationOpenIDConfiguration, *AuthorizedIntegrationTester) -type jwksTweak func(*openIDKeys) +type jwksTweak func(*auth.AuthorizedIntegrationOpenIDKeys) type aiDBTweak func(*auth_model.AuthorizedIntegration) diff --git a/services/auth/method/mocks_test.go b/services/auth/mocks.go similarity index 99% rename from services/auth/method/mocks_test.go rename to services/auth/mocks.go index 46648c8cba..68cbf359c7 100644 --- a/services/auth/method/mocks_test.go +++ b/services/auth/mocks.go @@ -2,7 +2,7 @@ // github.com/vektra/mockery // template: testify -package method +package auth import ( "forgejo.org/modules/jwtx" diff --git a/services/authz/access_token.go b/services/authz/access_token.go index 5a6d876cf6..11b492bc1e 100644 --- a/services/authz/access_token.go +++ b/services/authz/access_token.go @@ -37,7 +37,7 @@ func GetAuthorizationReducerForAccessToken(ctx context.Context, token *auth_mode // resources (public-only and specific repositories), and other similar checks. func ValidateAccessToken(token *auth_model.AccessToken, repoResources []*auth_model.AccessTokenResourceRepo) error { // Other validations may be added here in the future. - return validateRepositoryResource(token.ResourceAllRepos, token.Scope, len(repoResources)) + return ValidateRepositoryResource(token.ResourceAllRepos, token.Scope, len(repoResources)) } var ( @@ -46,7 +46,7 @@ var ( ErrSpecifiedReposInvalidScope = errors.New("specified repository access token: invalid scope") ) -func validateRepositoryResource(resourceAllRepos bool, scope auth_model.AccessTokenScope, numRepoResources int) error { +func ValidateRepositoryResource(resourceAllRepos bool, scope auth_model.AccessTokenScope, numRepoResources int) error { // Access tokens with broad access to all resources don't have any relevant validation rules to apply. if resourceAllRepos { return nil diff --git a/services/authz/authorized_integration.go b/services/authz/authorized_integration.go index 7f789a09df..c463c0aeca 100644 --- a/services/authz/authorized_integration.go +++ b/services/authz/authorized_integration.go @@ -5,14 +5,11 @@ package authz import ( "context" - "errors" "fmt" auth_model "forgejo.org/models/auth" ) -var ErrAuthorizedIntegrationBadUI = errors.New("invalid authorized integration UI") - func GetAuthorizationReducerForAuthorizedIntegration(ctx context.Context, ai *auth_model.AuthorizedIntegration) (AuthorizationReducer, error) { if ai.ResourceAllRepos { if publicOnly, err := ai.Scope.PublicOnly(); err != nil { @@ -34,16 +31,3 @@ func GetAuthorizationReducerForAuthorizedIntegration(ctx context.Context, ai *au } return &SpecificReposAuthorizationReducer{resourceRepos: iface}, nil } - -// Validate that an authorized integration's state is valid for creation. For example, that it doesn't have a -// conflicting set of resources (public-only and specific repositories), and other similar checks. -func ValidateAuthorizedIntegration(ai *auth_model.AuthorizedIntegration, repoResources []*auth_model.AuthorizedIntegResourceRepo) error { - switch ai.UI { - case auth_model.AuthorizedIntegrationUIGeneric, - auth_model.AuthorizedIntegrationUIForgejoActionsLocal: - break - default: - return fmt.Errorf("%w: invalid UI: %q", ErrAuthorizedIntegrationBadUI, ai.UI) - } - return validateRepositoryResource(ai.ResourceAllRepos, ai.Scope, len(repoResources)) -} diff --git a/services/authz/authorized_integration_test.go b/services/authz/authorized_integration_test.go index 787d3560dd..fe3ec697a4 100644 --- a/services/authz/authorized_integration_test.go +++ b/services/authz/authorized_integration_test.go @@ -4,7 +4,6 @@ package authz import ( - "strings" "testing" "forgejo.org/models/auth" @@ -45,71 +44,3 @@ func TestGetAuthorizationReducerForAuthorizedIntegration(t *testing.T) { assert.EqualValues(t, 1, specific.resourceRepos[0].GetTargetRepoID()) }) } - -func TestValidateAuthorizedIntegration(t *testing.T) { - t.Run("valid - all access", func(t *testing.T) { - ai := &auth.AuthorizedIntegration{ - ResourceAllRepos: true, - Scope: auth.AccessTokenScopeReadRepository, - UI: auth.AuthorizedIntegrationUIGeneric, - } - err := ValidateAuthorizedIntegration(ai, nil) - require.NoError(t, err) - }) - - t.Run("valid - specified repos", func(t *testing.T) { - ai := &auth.AuthorizedIntegration{ - ResourceAllRepos: false, - Scope: auth.AccessTokenScopeReadRepository, - UI: auth.AuthorizedIntegrationUIGeneric, - } - resources := []*auth.AuthorizedIntegResourceRepo{{RepoID: 12}} - err := ValidateAuthorizedIntegration(ai, resources) - require.NoError(t, err) - }) - - t.Run("invalid - no specified repos", func(t *testing.T) { - ai := &auth.AuthorizedIntegration{ - ResourceAllRepos: false, - Scope: auth.AccessTokenScopeReadRepository, - UI: auth.AuthorizedIntegrationUIGeneric, - } - resources := []*auth.AuthorizedIntegResourceRepo{} - err := ValidateAuthorizedIntegration(ai, resources) - require.ErrorIs(t, err, ErrSpecifiedReposNone) - }) - - t.Run("invalid - specified repos & public-only", func(t *testing.T) { - ai := &auth.AuthorizedIntegration{ - ResourceAllRepos: false, - Scope: auth.AccessTokenScope(strings.Join([]string{string(auth.AccessTokenScopePublicOnly), string(auth.AccessTokenScopeReadRepository)}, ",")), - UI: auth.AuthorizedIntegrationUIGeneric, - } - resources := []*auth.AuthorizedIntegResourceRepo{{RepoID: 12}} - err := ValidateAuthorizedIntegration(ai, resources) - require.ErrorIs(t, err, ErrSpecifiedReposNoPublicOnly) - }) - - t.Run("invalid - specified repos unsupported scopes", func(t *testing.T) { - ai := &auth.AuthorizedIntegration{ - ResourceAllRepos: false, - Scope: auth.AccessTokenScopeReadAdmin, - UI: auth.AuthorizedIntegrationUIGeneric, - } - resources := []*auth.AuthorizedIntegResourceRepo{{RepoID: 12}} - err := ValidateAuthorizedIntegration(ai, resources) - require.ErrorIs(t, err, ErrSpecifiedReposInvalidScope) - require.ErrorContains(t, err, string(auth.AccessTokenScopeReadAdmin)) - }) - - t.Run("invalid - missing UI", func(t *testing.T) { - ai := &auth.AuthorizedIntegration{ - ResourceAllRepos: false, - Scope: auth.AccessTokenScopeReadAdmin, - } - resources := []*auth.AuthorizedIntegResourceRepo{{RepoID: 12}} - err := ValidateAuthorizedIntegration(ai, resources) - require.ErrorIs(t, err, ErrAuthorizedIntegrationBadUI) - require.ErrorContains(t, err, "invalid UI: \"\"") - }) -} diff --git a/templates/user/settings/authorized_integrations.tmpl b/templates/user/settings/authorized_integrations.tmpl index 868077bbfa..8a04410750 100644 --- a/templates/user/settings/authorized_integrations.tmpl +++ b/templates/user/settings/authorized_integrations.tmpl @@ -31,7 +31,7 @@
{{svg "octicon-pencil" 16 "tw-mr-1"}} - {{ctx.Locale.Tr "settings.authorized_integration.view"}} + {{ctx.Locale.Tr "settings.authorized_integration.edit"}}
diff --git a/templates/user/settings/authorized_integrations/generic/view.tmpl b/templates/user/settings/authorized_integrations/generic/view.tmpl index 070114fe28..8be4068146 100644 --- a/templates/user/settings/authorized_integrations/generic/view.tmpl +++ b/templates/user/settings/authorized_integrations/generic/view.tmpl @@ -4,14 +4,14 @@ {{ctx.Locale.Tr "settings.authorized_integration.claims.generic"}}
-
+
- +
-
- - +
+ + {{template "shared/codemirror_container" .}}
diff --git a/templates/user/settings/authorized_integrations/view_footer.tmpl b/templates/user/settings/authorized_integrations/view_footer.tmpl index 23bd1cb0b7..73c32f7ad4 100644 --- a/templates/user/settings/authorized_integrations/view_footer.tmpl +++ b/templates/user/settings/authorized_integrations/view_footer.tmpl @@ -9,14 +9,14 @@
- +

{{ctx.Locale.Tr "settings.access_token.resource_all_help"}}

- +

{{ctx.Locale.Tr "settings.access_token.resource_public_only_help"}} @@ -25,8 +25,8 @@

-
- +
+

{{ctx.Locale.Tr "settings.access_token.resource_specific_repo_help"}} @@ -36,9 +36,108 @@

+
+
+ + +
+
+ {{ctx.Locale.Tr "settings.access_token.available_repositories"}} +
+ +
+ + {{svg "octicon-search" 16}} + +
+ + {{if eq (len .Repos) 0}} + {{ctx.Locale.Tr "settings.access_token.no_repositories_found"}} + {{else}} +
+ {{range .Repos}} + {{template "user/settings/repo_icon" .}} +
+ {{.FullName}} +
+ + {{end}} +
+ {{end}} + + {{/* Can't use base/paginate template include here because all the pagination links in + that template are simple links. Here, we need to turn them into form + buttons so that we can submit the current form. If a user just changed a value (eg. set + the token name, changed a selected permission) and then clicked a pagination button, the + new value that they changed needs to be submitted. base/paginate would allow preserving + old values from before the change, but not new updates. Implementing here also allows + the use of smaller styling. */}} + {{with .Page.Paginater}} + +
+ +
+ {{end}} +
+ + +
+
{{ctx.Locale.TrPluralString (len .SelectedRepos) "settings.access_token.selected_repositories" (len .SelectedRepos)}}
+ {{if eq (len .SelectedRepos) 0}} +
+ {{ctx.Locale.Tr "settings.access_token.no_repositories_selected"}} +
+ {{else}} +
+ {{range .SelectedRepos}} + + {{template "user/settings/repo_icon" .}} +
+ {{.FullName}} +
+ + {{end}} +
+ {{end}} +
+ +
+
-
+
@@ -52,7 +151,7 @@ {{.}}
- @@ -70,6 +169,11 @@
+
+ +
+ +
diff --git a/templates/user/settings/authorized_integrations/view_head.tmpl b/templates/user/settings/authorized_integrations/view_head.tmpl index a148008cd1..47b3394e6e 100644 --- a/templates/user/settings/authorized_integrations/view_head.tmpl +++ b/templates/user/settings/authorized_integrations/view_head.tmpl @@ -4,17 +4,17 @@

- {{ctx.Locale.Tr "settings.authorized_integration.view_page_title" .Form.Name}} + {{ctx.Locale.Tr "settings.authorized_integration.edit_page_title" .Form.Name}}

-
+
- +
- +
diff --git a/tests/e2e/fixtures/authorized_integration.yml b/tests/e2e/fixtures/authorized_integration.yml new file mode 100644 index 0000000000..6686c460ba --- /dev/null +++ b/tests/e2e/fixtures/authorized_integration.yml @@ -0,0 +1,13 @@ +- + id: 1 + user_id: 2 + ui: generic + scope: write:repository,read:issue + resource_all_repos: true + name: "Example AI" + description: "This is an authorized integration.\nThis example is just for viewing and editing." + issuer: "urn:forgejo:authorized-integrations:actions" # avoid an external dependency on validating an issuer on edit + audience: u:2:7a6a47fb-6252-48b2-b0bb-e39158b11a36 + claim_rules: "{}" + created_unix: 1778959784 + updated_unix: 1778959784 diff --git a/tests/e2e/user-settings.test.e2e.ts b/tests/e2e/user-settings.test.e2e.ts index 1c52b5f9e1..0c58fd998d 100644 --- a/tests/e2e/user-settings.test.e2e.ts +++ b/tests/e2e/user-settings.test.e2e.ts @@ -3,7 +3,7 @@ // web_src/css/{form,user}.css // @watch end -import {expect} from '@playwright/test'; +import {expect, type Page} from '@playwright/test'; import {test, login_user, login} from './utils_e2e.ts'; import {screenshot} from './shared/screenshots.ts'; import {validate_form} from './shared/forms.ts'; @@ -237,3 +237,191 @@ test('User: Add specific repo access token error', async ({browser}, workerInfo) await expect(page.getByRole('combobox', {name: 'repository'})).toHaveValue('read:repository'); await expect(page.getByRole('button', {name: 'Remove org17/big_test_private_4'})).toBeVisible(); }); + +test('User: List authorized integrations', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/authorized-integrations'); + + await expect(page.locator('.flex-item-title')).toContainText('Example AI'); + await expect(page.locator('.flex-item-body')).toContainText('Added on 2026-05-16'); + await expect(page.locator('.flex-item-body')).toContainText('No recent activity'); +}); + +async function validateClaimRules(page: Page, expected: string) { + await expect(async () => { + const internal = await page.evaluate(() => Array.from(window.codeEditors)[0].state.doc.toString()); + expect(internal).toStrictEqual(expected); + }).toPass({timeout: 3000}); + await expect(page.locator('#claim_rules')).toHaveValue(expected); +} + +test('User: View authorized integration', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/authorized-integrations'); + + await page.getByRole('link', {name: 'Edit'}).click(); + + await expect(page.getByRole('textbox', {name: 'Name'})).toHaveValue('Example AI'); + await expect(page.getByRole('textbox', {name: 'Description'})).toHaveValue('This is an authorized integration.\nThis example is just for viewing and editing.'); + await expect(page.getByRole('textbox', {name: 'Audience (aud Claim)'})).toHaveValue('u:2:7a6a47fb-6252-48b2-b0bb-e39158b11a36'); + await expect(page.getByRole('textbox', {name: 'Issuer (iss Claim)'})).toHaveValue('urn:forgejo:authorized-integrations:actions'); + + // Claim rules JSON codemirror editor: + const editor = page.locator('.cm-content'); + await expect(editor).toHaveAttribute('data-language', 'json', {timeout: 3000}); + await validateClaimRules(page, '{\n "rules": null\n}'); + + await expect(page.getByRole('radio', {name: 'All (public, private, and limited)'})).toBeChecked(); + await expect(page.getByRole('radio', {name: 'Public only'})).not.toBeChecked(); + await expect(page.getByRole('radio', {name: 'Specific repositories'})).not.toBeChecked(); + + await expect(page.getByRole('combobox', {name: 'issue'})).toHaveValue('read:issue'); + await expect(page.getByRole('combobox', {name: 'repository'})).toHaveValue('write:repository'); + await expect(page.getByRole('combobox', {name: 'user'})).toHaveValue(''); + await expect(page.getByRole('combobox', {name: 'admin'})).toBeHidden(); // not an admin user +}); + +test('User: Edit authorized integration basic fields', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/authorized-integrations'); + + await page.getByRole('link', {name: 'Edit'}).click(); + + await page.getByRole('textbox', {name: 'Name'}).fill('Example AI (Updated!)'); + await page.getByRole('textbox', {name: 'Description'}).fill('Updated by Edit authorized integration basic field test'); + + await page.getByRole('button', {name: 'Save authorized integration'}).click(); + + // Returns to the list page; validate the updated name is present, and that it isn't marked + // as "used" just because it was edited: + await expect(page.locator('.flex-item-title')).toContainText('Example AI (Updated!)'); + await expect(page.locator('.flex-item-body')).toContainText('Added on 2026-05-16'); + await expect(page.locator('.flex-item-body')).toContainText('No recent activity'); + + // Reopen to check description: + await page.getByRole('link', {name: 'Edit'}).click(); + await expect(page.getByRole('textbox', {name: 'Name'})).toHaveValue('Example AI (Updated!)'); + await expect(page.getByRole('textbox', {name: 'Description'})).toHaveValue('Updated by Edit authorized integration basic field test'); + + // Restore values to avoid affecting other tests and other platforms: + await page.getByRole('textbox', {name: 'Name'}).fill('Example AI'); + await page.getByRole('textbox', {name: 'Description'}).fill('This is an authorized integration.\nThis example is just for viewing and editing.'); + await page.getByRole('button', {name: 'Save authorized integration'}).click(); + await expect(page.locator('.flex-item-title')).toContainText('Example AI'); // ensure save completes and we land on list page +}); + +test('User: Edit authorized integration basic fields validation error', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/authorized-integrations'); + + await page.getByRole('link', {name: 'Edit'}).click(); + await page.getByRole('textbox', {name: 'Name'}).fill('\t'); // trims to empty + await page.getByRole('button', {name: 'Save authorized integration'}).click(); + + await expect(page.locator('.flash-error')).toContainText('Authorized integration name is required.'); + await expect(page.getByRole('textbox', {name: 'Name'}).locator('..')).toHaveClass('required field error'); +}); + +test('User: Edit authorized integration issuer validation error', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/authorized-integrations'); + + await page.getByRole('link', {name: 'Edit'}).click(); + await page.getByRole('textbox', {name: 'Issuer (iss Claim)'}).fill('ftp://example.org'); // designed to hit "unsupported URL scheme" error, no external traffic involved + await page.getByRole('button', {name: 'Save authorized integration'}).click(); + + await expect(page.locator('.flash-error')).toContainText(/Issuer validation failed:/); + await expect(page.getByRole('textbox', {name: 'Issuer (iss Claim)'}).locator('..')).toHaveClass('required field error'); +}); + +test('User: Edit authorized integration claim rules', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/authorized-integrations'); + + await page.getByRole('link', {name: 'Edit'}).click(); + + const editor = page.locator('.cm-content'); + await editor.click(); // Focus codemirror editor + await page.keyboard.press('ControlOrMeta+A'); // select all + await page.keyboard.press('Backspace'); // delete + await page.keyboard.type('{"rules": [{"claim": "sub", "compare": "eq", "value": "a subject"}]}', {delay: 10}); + + await page.getByRole('button', {name: 'Save authorized integration'}).click(); + + // Reopen to check claim rules saved: + await page.getByRole('link', {name: 'Edit'}).click(); + await validateClaimRules(page, '{\n "rules": [\n {\n "claim": "sub",\n "compare": "eq",\n "value": "a subject"\n }\n ]\n}'); + + // Restore values to avoid affecting other tests and other platforms: + await editor.click(); // Focus codemirror editor + await page.keyboard.press('ControlOrMeta+A'); // select all + await page.keyboard.press('Backspace'); // delete + await page.keyboard.type('{"rules": null}', {delay: 10}); + await page.getByRole('button', {name: 'Save authorized integration'}).click(); + await expect(page.locator('.flex-item-title')).toContainText('Example AI'); // ensure save completes and we land on list page +}); + +test('User: Edit authorized integration claim rules validation error', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/authorized-integrations'); + + await page.getByRole('link', {name: 'Edit'}).click(); + + const editor = page.locator('.cm-content'); + await editor.click(); // Focus codemirror editor + await page.keyboard.type('{{{{{{', {delay: 10}); // type some incomplete garbage at the end + await page.getByRole('button', {name: 'Save authorized integration'}).click(); + + await expect(page.locator('.flash-error')).toContainText(/Claim Rules validation failed:/); +}); + +test('User: Edit authorized integration specific repo', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/authorized-integrations'); + + await page.getByRole('link', {name: 'Edit'}).click(); + + // clicking specific repositories will display currently available repositories: + await expect(page.getByText('org17/big_test_private_4')).toBeHidden(); + await page.getByRole('radio', {name: 'Specific repositories'}).click(); + await expect(page.getByText('org17/big_test_private_4')).toBeVisible(); + await expect(page.getByText('user2/commits_search_test')).toBeVisible(); // another repo, will be used to verify search worked + + await page.getByPlaceholder('Search repos…').fill('big_test_private_4'); + await page.getByRole('button', {name: 'Search…'}).click(); + + // verify search results visible: + await expect(page.getByText('org17/big_test_private_4')).toBeVisible(); + await expect(page.getByText('user2/commits_search_test')).toBeHidden(); + + // after performing a search, verify that the name, 'selected repositories', and selected permissions are maintained + await expect(page.getByRole('textbox', {name: 'Name'})).toHaveValue(/^Example AI/); + await expect(page.getByRole('radio', {name: 'Specific repositories'})).toBeChecked(); + await expect(page.getByRole('combobox', {name: 'repository'})).toHaveValue('write:repository'); + + // Add the big_test_private_4 repo. + await page.getByRole('button', {name: 'Add org17/big_test_private_4'}).click(); + await expect(page.getByText('Selected repository (1)')).toBeVisible(); + await expect(page.getByText('org17/big_test_private_4')).toBeVisible(); + + // Remove it to test remove, and then re-add + await page.getByRole('button', {name: 'Remove org17/big_test_private_4'}).click(); + await expect(page.getByText('Selected repositories (0)')).toBeVisible(); + await expect(page.getByText('org17/big_test_private_4')).toBeVisible(); + await page.getByRole('button', {name: 'Add org17/big_test_private_4'}).click(); + + // Save authorized integration + await page.getByRole('button', {name: 'Save authorized integration'}).click(); + + // Reopen to check change to repo-specific was saved: + await page.getByRole('link', {name: 'Edit'}).click(); + await expect(page.getByRole('radio', {name: 'All (public, private, and limited)'})).not.toBeChecked(); + await expect(page.getByRole('radio', {name: 'Public only'})).not.toBeChecked(); + await expect(page.getByRole('radio', {name: 'Specific repositories'})).toBeChecked(); + await expect(page.getByRole('button', {name: 'Remove org17/big_test_private_4'})).toBeVisible(); + + // Restore values to avoid affecting other tests and other platforms: + await page.getByRole('radio', {name: 'All (public, private, and limited)'}).click(); + await page.getByRole('button', {name: 'Save authorized integration'}).click(); + await expect(page.locator('.flex-item-title')).toContainText('Example AI'); // ensure save completes and we land on list page +}); diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 770bc5d1b1..ae594cf9f8 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -42,7 +42,7 @@ import ( "forgejo.org/modules/util" "forgejo.org/modules/web" "forgejo.org/routers" - auth_method "forgejo.org/services/auth/method" + auth_service "forgejo.org/services/auth" "forgejo.org/services/auth/source/remote" app_context "forgejo.org/services/context" "forgejo.org/services/mailer" @@ -831,7 +831,7 @@ func newAITester(t *testing.T, setupAI ...func(*auth.AuthorizedIntegration)) *Au // trust TLS cert of our NewTLSServer by inserting the test client for our test server in as the HTTP client to use ait.resetHTTPClient = test.MockVariableValue( - &auth_method.GetAuthorizedIntegrationHTTPClient, + &auth_service.GetAuthorizedIntegrationHTTPClient, func() *http.Client { return ait.testServer.Client() }) diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 1fae407a37..1177a18801 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -1374,7 +1374,7 @@ func TestAuthorizedIntegrationView(t *testing.T) { assert.Equal(t, "An Authorized Integration created for the test case TestAuthorizedIntegrationView.\nIt's pretty neat.", htmlDoc.doc.Find("textarea[name='description']").Text()) htmlDoc.AssertAttrEqual(t, "#audience", "value", ait.authorizedIntegration.Audience) htmlDoc.AssertAttrEqual(t, "#issuer", "value", ait.authorizedIntegration.Issuer) - assert.Equal(t, "{\n \"rules\": [\n {\n \"claim\": \"custom-claim\",\n \"compare\": \"eq\",\n \"value\": \"custom-claim-value\"\n }\n ]\n}", htmlDoc.doc.Find("textarea[id='content']").Text()) //nolint:testifylint // this isn't a JSON comparison; the formatting here should be exact as it represents the auto-indentation generated by the server + assert.Equal(t, "{\n \"rules\": [\n {\n \"claim\": \"custom-claim\",\n \"compare\": \"eq\",\n \"value\": \"custom-claim-value\"\n }\n ]\n}", htmlDoc.doc.Find("textarea[id='claim_rules']").Text()) //nolint:testifylint // this isn't a JSON comparison; the formatting here should be exact as it represents the auto-indentation generated by the server htmlDoc.AssertElementChecked(t, "#resource-all") htmlDoc.AssertElementSelected(t, "#scope-activitypub option[value='write:activitypub']") assert.Equal(t, 0, htmlDoc.Find("#scope-admin").Length()) // not an admin user diff --git a/web_src/js/features/authorized-integration.js b/web_src/js/features/authorized-integration.js index ca79631226..0b8f4d9b5e 100644 --- a/web_src/js/features/authorized-integration.js +++ b/web_src/js/features/authorized-integration.js @@ -3,5 +3,5 @@ import {createCodemirror} from './codemirror.ts'; export function initAuthorizedIntegrationClaimRuleEditor() { if (!$('.user.authorized-integrations').length) return; - const _promise = createCodemirror($('#content')[0], 'claims.json', {language: 'JSON'}); + const _promise = createCodemirror($('#claim_rules')[0], 'claims.json', {language: 'JSON'}); }