Skip to content

Commit

Permalink
[BUG] admin oauth2 source required check (#4194)
Browse files Browse the repository at this point in the history
#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 go-gitea/gitea#31246 which I initially overlooked.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4194
Reviewed-by: Earl Warren <[email protected]>
Co-authored-by: oliverpool <[email protected]>
Co-committed-by: oliverpool <[email protected]>
(cherry picked from commit 65f8c22)
  • Loading branch information
oliverpool authored Jun 21, 2024
1 parent 664c8a9 commit 6cef23d
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 6 deletions.
1 change: 0 additions & 1 deletion release-notes/7.0.4/fix/4059.md

This file was deleted.

1 change: 1 addition & 0 deletions release-notes/7.0.5/fix/4059.md
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 4 additions & 5 deletions web_src/js/features/admin/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
Expand All @@ -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');
}
Expand Down

0 comments on commit 6cef23d

Please sign in to comment.