From 65f8c22cc75c3b43d627e80d91e067ce0da97c95 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Fri, 21 Jun 2024 06:21:37 +0000 Subject: [PATCH] [BUG] admin oauth2 source required check (#4194) #4059 was unfortunately incomplete: some custom_url fields are currently shown, even if they are not used by the provider. Moreover the `Use Custom URLs Instead of Default URLs` is always checked by default. Manual testing: - go to http://localhost:3000/admin/auths - click on `Add authentication source` - Choose `Authentication type`: `OAuth2` - Choose `OAuth2 provider`: `GitLab` - verify that the `Use Custom URLs Instead of Default URLs` option is **initially unchecked** - enable the `Use Custom URLs Instead of Default URLs` checkbox - verify that only the fields "Authorize", "Token" and "Profile" URLs are shown (no "Email URL", nor "Tenant"). - Switch the `OAuth2 provider` to `Azure AD v2` - verify that the `Use Custom URLs Instead of Default URLs` option is **initially checked** - verify that only the field "Tenant" is shown (with the default "organizations"). ![image](/attachments/0e2b1508-861c-4b0e-ae6a-6eb24ce94911) Note: this is loosely based on the upstream fix https://github.com/go-gitea/gitea/pull/31246 which I initially overlooked. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4194 Reviewed-by: Earl Warren Co-authored-by: oliverpool Co-committed-by: oliverpool --- release-notes/7.0.4/fix/4059.md | 1 - release-notes/7.0.5/fix/4059.md | 1 + web_src/js/features/admin/common.js | 9 ++++----- 3 files changed, 5 insertions(+), 6 deletions(-) delete mode 100644 release-notes/7.0.4/fix/4059.md create mode 100644 release-notes/7.0.5/fix/4059.md diff --git a/release-notes/7.0.4/fix/4059.md b/release-notes/7.0.4/fix/4059.md deleted file mode 100644 index 202ca4d33c..0000000000 --- a/release-notes/7.0.4/fix/4059.md +++ /dev/null @@ -1 +0,0 @@ -Wrongfully hidden "Use Custom URLs Instead of Default URLs" checkbox on Authentication Source Administration page. diff --git a/release-notes/7.0.5/fix/4059.md b/release-notes/7.0.5/fix/4059.md new file mode 100644 index 0000000000..359e517506 --- /dev/null +++ b/release-notes/7.0.5/fix/4059.md @@ -0,0 +1 @@ +Authentication Source Administration page wrongfully handled the "Custom URLs Instead of Default URLs" checkbox (missing checkbox, irrelevant fields) [#4059](https://codeberg.org/forgejo/forgejo/pulls/4059) [#4194](https://codeberg.org/forgejo/forgejo/pulls/4194) \ No newline at end of file diff --git a/web_src/js/features/admin/common.js b/web_src/js/features/admin/common.js index c2f3cde821..1a5bd6e490 100644 --- a/web_src/js/features/admin/common.js +++ b/web_src/js/features/admin/common.js @@ -78,10 +78,9 @@ export function initAdminCommon() { default: { const customURLSettings = document.getElementById(`${provider}_customURLSettings`); if (!customURLSettings) break; - if (customURLSettings.getAttribute('data-required')) { - document.getElementById('oauth2_use_custom_url')?.setAttribute('checked', 'checked'); - } - if (customURLSettings.getAttribute('data-available')) { + const customURLRequired = (customURLSettings.getAttribute('data-required') === 'true'); + document.getElementById('oauth2_use_custom_url').checked = customURLRequired; + if (customURLRequired || customURLSettings.getAttribute('data-available') === 'true') { showElem('.oauth2_use_custom_url'); } } @@ -103,7 +102,7 @@ export function initAdminCommon() { if (applyDefaultValues) { document.getElementById(`oauth2_${custom}`).value = customInput.value; } - if (customInput.getAttribute('data-available')) { + if (customInput.getAttribute('data-available') === 'true') { for (const input of document.querySelectorAll(`.oauth2_${custom} input`)) { input.setAttribute('required', 'required'); }