feat: web UI to delete authorized integration (#12632)

Adds a "Delete" option to the authorized integration list.

## 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

(can be removed for Go 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.

### 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/12632
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>

models/auth/authorized_integration.go

@@ -269,3 +269,29 @@ func ParseAuthorizedIntegrationUI(ui string) (AuthorizedIntegrationUI, error) {
 	}
 	return AuthorizedIntegrationUI(""), fmt.Errorf("invalid authorized integration UI: %q", ui)
 }
+
+// Delete an authorized integration by ID.  Must only succeed if the authorized integration identified is owned by the
+// user provided.
+func DeleteAuthorizedIntegrationByID(ctx context.Context, id, userID int64) error {
+	return db.WithTx(ctx, func(ctx context.Context) error {
+		// Delete doesn't take into account userID, but will be rolled back by the transaction if the user ID isn't
+		// correct.  Needs to occur first due to foreign key.
+		if err := db.DeleteBeans(ctx,
+			&AuthorizedIntegResourceRepo{IntegID: id},
+		); err != nil {
+			return fmt.Errorf("DeleteBeans: %w", err)
+		}
+
+		cnt, err := db.GetEngine(ctx).
+			ID(id).
+			Delete(&AuthorizedIntegration{
+				UserID: userID,
+			})
+		if err != nil {
+			return err
+		} else if cnt != 1 {
+			return fmt.Errorf("authorized integration %d does not exist: %w", id, util.ErrNotExist)
+		}
+		return nil
+	})
+}

models/auth/authorized_integration_test.go

@@ -193,3 +193,37 @@ func TestUpdateAuthorizedIntegration(t *testing.T) {
 	assert.Equal(t, createdUnix, fromDB.CreatedUnix)
 	assert.Equal(t, updatedUnix, fromDB.UpdatedUnix) // not changed -- used to track usage for authentication
 }
+
+func TestDeleteAuthorizedIntegrationByID(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+	t.Run("simple delete", func(t *testing.T) {
+		ai := makeAuthorizedIntegration(t)
+		err := auth_model.DeleteAuthorizedIntegrationByID(t.Context(), ai.ID, ai.UserID)
+		require.NoError(t, err)
+		unittest.AssertNotExistsBean(t, &auth_model.AuthorizedIntegration{ID: ai.ID})
+	})
+
+	t.Run("delete repo-specific", func(t *testing.T) {
+		ai := makeAuthorizedIntegration(t)
+		resRepo1 := &auth_model.AuthorizedIntegResourceRepo{
+			IntegID: ai.ID,
+			RepoID:  1,
+		}
+		err := auth_model.InsertAuthorizedIntegrationResourceRepos(t.Context(), ai.ID,
+			[]*auth_model.AuthorizedIntegResourceRepo{resRepo1})
+		require.NoError(t, err)
+		unittest.AssertCount(t, &auth_model.AuthorizedIntegResourceRepo{IntegID: ai.ID}, 1)
+
+		err = auth_model.DeleteAuthorizedIntegrationByID(t.Context(), ai.ID, ai.UserID)
+		require.NoError(t, err)
+
+		unittest.AssertNotExistsBean(t, &auth_model.AuthorizedIntegration{ID: ai.ID})
+		unittest.AssertCount(t, &auth_model.AuthorizedIntegResourceRepo{IntegID: ai.ID}, 0)
+	})
+
+	t.Run("delete fails on wrong user", func(t *testing.T) {
+		ai := makeAuthorizedIntegration(t)
+		err := auth_model.DeleteAuthorizedIntegrationByID(t.Context(), ai.ID, 300)
+		require.ErrorIs(t, err, util.ErrNotExist)
+	})
+}

options/locale_next/locale_en-US.json

@@ -348,6 +348,9 @@
 	"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.",
 	"settings.authorized_integration.add": "Add authorized integration",
 	"settings.authorized_integration.generic": "Generic JWT Source",
+	"settings.authorized_integration.delete.header": "Delete authorized integration",
+	"settings.authorized_integration.delete.body": "Deleting an authorized integration will revoke access to your account for the integrating application. This is permanent, and cannot be undone. Creating a new authorized integration will not have the same Audience (<code>aud</code>) claim. Continue?",
+	"settings.authorized_integration.deleted": "Authorized integration has been deleted successfully.",
 	"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…",

routers/web/user/setting/authorized_integrations.go

@@ -442,3 +442,12 @@ func repoMultiSelect(ctx *context.Context) {
 	pager.SetDefaultParams(ctx)
 	ctx.Data["Page"] = pager
 }
+
+func DeleteAuthorizedIntegration(ctx *context.Context) {
+	if err := auth_model.DeleteAuthorizedIntegrationByID(ctx, ctx.FormInt64("id"), ctx.Doer.ID); err != nil {
+		ctx.Flash.Error("DeleteAuthorizedIntegrationByID: " + err.Error())
+	} else {
+		ctx.Flash.Success(ctx.Tr("settings.authorized_integration.deleted"))
+	}
+	ctx.JSONRedirect(setting.AppSubURL + "/user/settings/authorized-integrations")
+}

routers/web/web.go

@@ -680,6 +680,7 @@ func registerRoutes(m *web.Route) {
 					Get(web.Bind(user_setting.AuthorizedIntegrationForm{}), user_setting.EditAuthorizedIntegration).
 					Post(web.Bind(user_setting.AuthorizedIntegrationForm{}), user_setting.EditAuthorizedIntegrationPost)
 			})
+			m.Post("/delete", user_setting.DeleteAuthorizedIntegration)
 			m.Get("", user_setting.ListAuthorizedIntegrations)
 		})
 

templates/user/settings/authorized_integrations.tmpl

@@ -39,6 +39,10 @@
 							{{svg "octicon-pencil" 16 "tw-mr-1"}}
 							{{ctx.Locale.Tr "settings.authorized_integration.edit"}}
 						</a>
+						<button class="ui red tiny button delete-button" data-modal-id="delete-authorized-integration" data-url="{{$.Link}}/delete" data-id="{{.ID}}">
+							{{svg "octicon-trash" 16 "tw-mr-1"}}
+							{{ctx.Locale.Tr "settings.delete_token"}}
+						</button>
 					</div>
 				</div>
 			{{else}}
@@ -54,4 +58,15 @@
 	</div>
 </div>
 
+<div class="ui g-modal-confirm delete modal" id="delete-authorized-integration">
+	<div class="header">
+		{{svg "octicon-trash"}}
+		{{ctx.Locale.Tr "settings.authorized_integration.delete.header"}}
+	</div>
+	<div class="content">
+		<p>{{ctx.Locale.Tr "settings.authorized_integration.delete.body"}}</p>
+	</div>
+	{{template "base/modal_actions_confirm"}}
+</div>
+
 {{template "user/settings/layout_footer" .}}

tests/e2e/user-settings.test.e2e.ts

@@ -465,6 +465,16 @@ test('User: Add authorized integration', async ({browser}, workerInfo) => {
   await expect(page.locator('.ui.message.flash-success')).toBeVisible();
   const flashText = await page.locator('.ui.message.flash-success').textContent();
   expect(flashText?.trim()).toBe('Created authorized integration: New Authorized Integration!');
+
+  // Delete the added integration, minimizing left-over test data and also validating the delete UI:
+  await page.goto('/user/settings/authorized-integrations');
+  await page.locator('.flex-item')
+    .filter({has: page.locator('.flex-item-title', {hasText: 'New Authorized Integration!'})})
+    .getByRole('button', {name: 'Delete'}).click();
+  await page.getByRole('button', {name: 'Yes'}).click();
+  await expect(page.locator('.ui.message.flash-success')).toBeVisible();
+  const deleteFlashText = await page.locator('.ui.message.flash-success').textContent();
+  expect(deleteFlashText?.trim()).toBe('Authorized integration has been deleted successfully.');
 });
 
 test('User: Add authorized integration validation error', async ({browser}, workerInfo) => {
@@ -490,4 +500,14 @@ test('User: Add authorized integration validation error', async ({browser}, work
   await expect(page.locator('.ui.message.flash-success')).toBeVisible();
   const flashText = await page.locator('.ui.message.flash-success').textContent();
   expect(flashText?.trim()).toBe('Created authorized integration: Forgot to fill this out!');
+
+  // Delete the added integration, minimizing left-over test data and also validating the delete UI:
+  await page.goto('/user/settings/authorized-integrations');
+  await page.locator('.flex-item')
+    .filter({has: page.locator('.flex-item-title', {hasText: 'Forgot to fill this out!'})})
+    .getByRole('button', {name: 'Delete'}).click();
+  await page.getByRole('button', {name: 'Yes'}).click();
+  await expect(page.locator('.ui.message.flash-success')).toBeVisible();
+  const deleteFlashText = await page.locator('.ui.message.flash-success').textContent();
+  expect(deleteFlashText?.trim()).toBe('Authorized integration has been deleted successfully.');
 });