Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug (&fix) - PUT requests from Settings tabs overwrite all other values #404

Closed
taylorwilsdon opened this issue Jul 30, 2024 · 0 comments · Fixed by #405
Closed

Bug (&fix) - PUT requests from Settings tabs overwrite all other values #404

taylorwilsdon opened this issue Jul 30, 2024 · 0 comments · Fixed by #405
Assignees

Comments

@taylorwilsdon
Copy link

taylorwilsdon commented Jul 30, 2024

Issue:

  • When using the Netbird dashboard UI to make changes to the "Settings" tabs, specifically under "Authentication" (/src/modules/settings/AuthenticationTab.tsx) a PUT call is made to /api/accounts/{accountid} that contains only the values from the current screen, which overwrites and blanks out the other values for /api/accounts/{accountid} from Groups & Permissions

  • In the included video "broken.mov" you can see that groups settings are in place and active, then a change is made only to the "expires in" value on the Authentication tab, and the resulting payload does not include values for jwt_allow_groups, jwt_groups_claim_name, jwt_groups_enabled and regular_users_view_blocked.

  • The response payload confirms that those values, previously set, have now been erased. While some APIs do implement a patch-like mechanism for PUT calls, it appears Netbird management does not, and thus the whole object is replaced with the values declared in the PUT call. A solution could be to migrate to a PATCH call, but my proposed change will fix this without changes to the API.

Solution:

  • Establish the other parameters in the state for AuthenticationTab.tsx and include with the PUT request.
  • Attached video "fixed.mov" shows that the API response now contains all the values correctly, and the Groups & Permissions values are not erased from the UI.
broken.mov
fixed.mov

I seem to lack the appropriate permissions to open a PR for this repo, but the diff that fixes this is included below:

diff --git a/src/modules/settings/AuthenticationTab.tsx b/src/modules/settings/AuthenticationTab.tsx
index 11f0235..333590f 100644
--- a/src/modules/settings/AuthenticationTab.tsx
+++ b/src/modules/settings/AuthenticationTab.tsx
@@ -22,6 +22,7 @@ import { useSWRConfig } from "swr";
 import SettingsIcon from "@/assets/icons/SettingsIcon";
 import { useHasChanges } from "@/hooks/useHasChanges";
 import { Account } from "@/interfaces/Account";
+import { isEmpty } from "lodash";
 
 type Props = {
   account: Account;
@@ -41,6 +42,26 @@ export default function AuthenticationTab({ account }: Props) {
     }
   });
 
+    /**
+   * Group Propagation
+   */
+    const [groupsPropagation, setGroupsPropagation] = useState<boolean>(
+      account.settings.groups_propagation_enabled,
+    );
+
+    /**
+     * JWT Group Sync
+     */
+    const [jwtGroupSync, setJwtGroupSync] = useState<boolean>(
+      account.settings.jwt_groups_enabled,
+    );
+    const [jwtGroupsClaimName, setJwtGroupsClaimName] = useState(
+      account.settings.jwt_groups_claim_name,
+    );
+    const [jwtAllowGroups, setJwtAllowGroups] = useState<string[]>(
+      account.settings.jwt_allow_groups,
+    );
+
   /**
    * Login expiration enabled
    */
@@ -87,6 +108,9 @@ export default function AuthenticationTab({ account }: Props) {
   ]);
 
   const saveChanges = async () => {
+    const jwtGroupsEntered =
+    jwtAllowGroups.filter((g) => !isEmpty(g)).length > 0;
+
     const expiration =
       expireInterval === "days"
         ? Number(expiresIn) * 86400
@@ -99,6 +123,12 @@ export default function AuthenticationTab({ account }: Props) {
         .put({
           id: account.id,
           settings: {
+            groups_propagation_enabled: groupsPropagation,
+            jwt_groups_enabled: jwtGroupSync,
+            jwt_groups_claim_name: isEmpty(jwtGroupsClaimName)
+              ? undefined
+              : jwtGroupsClaimName,
+            jwt_allow_groups: jwtGroupsEntered ? jwtAllowGroups : undefined,
             peer_login_expiration_enabled: loginExpiration,
             peer_login_expiration: loginExpiration ? expiration : 86400,
             extra: {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants