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

Google sign-in client ID setting regularly fails to save #15974

Closed
noahmoss opened this issue May 8, 2021 · 3 comments · Fixed by #16506
Closed

Google sign-in client ID setting regularly fails to save #15974

noahmoss opened this issue May 8, 2021 · 3 comments · Fixed by #16506
Assignees
Labels
Administration/Auth Google Auth, LDAP, pw+email login Priority:P2 Average run of the mill bug .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Milestone

Comments

@noahmoss
Copy link
Member

noahmoss commented May 8, 2021

Describe the bug
When trying to set the client ID field for the Google sign-in settings, it fails about half the time. Failures manifest as a page refresh after clicking "save changes", and the old value still being present in the client ID field. This isn't deterministic, since sometimes I'll retry saving a value 2-3 times before it succeeds. It also doesn't have any bearing on whether the value is a valid client ID.

Browser console also logs error updating setting every time this form is submitted, regardless of whether saving succeeded or not.

To Reproduce
Steps to reproduce the behavior:

  1. Go to the Admin Panel
  2. Click on Authentication
  3. Under "Sign in with Google" click "Configure"
  4. Put any value into the first text box and click "Save Changes"
  5. If the value was saved successfully after the page refreshes, retry step 4 with different values

Expected behavior
Value should save successfully every time the form is submitted.

Information about your Metabase Installation:
Firefox 88.0.1 on MacOS Big Sur, using an H2 database

@noahmoss noahmoss added Type:Bug Product defects .Needs Triage labels May 8, 2021
@noahmoss
Copy link
Member Author

noahmoss commented May 8, 2021

On further investigation this appears to be a frontend bug, possibly specific to Firefox. The PUT requests seem to be aborted with the message NS_BINDING_ABORTED in the network tab. Might be because a page refresh is being triggered which aborts the request if it happens too quickly.

image

@flamber
Copy link
Contributor

flamber commented May 8, 2021

Very related to #11659, which is probably caused by old form style and a race-condition.
See #13137 for historic report too.

@nemanjaglumac
Copy link
Member

nemanjaglumac commented May 19, 2021

From my experience, it accepts the update when you first navigate to the page (also, on page refresh, which is basically the same thing). If you try to update the value without leaving the page, the error occurs.

One more important thing I noticed. On successful tries, the request is
PUT /api/setting/google-auth-client-id (204)

While on the failed ones it sends the request to:
PUT /api/setting/ (500)

nemanjaglumac added a commit that referenced this issue May 19, 2021
@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label May 19, 2021
@noahmoss noahmoss added this to the 0.40 milestone Jun 9, 2021
nemanjaglumac added a commit that referenced this issue Jun 25, 2021
* Add a timeout to the `saveSettings` function

* Update and unskip the test for #15974

* Put `Google` SSO test in its own `describe` block

* Update the test
This was referenced May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Administration/Auth Google Auth, LDAP, pw+email login Priority:P2 Average run of the mill bug .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants