Skip to content

Comments

fix: add check for validatePromise for passwordless environments#1209

Merged
juliusknorr merged 2 commits intonextcloud-libraries:mainfrom
benjaminfrueh:fix/password-confirmation-sso
Nov 6, 2025
Merged

fix: add check for validatePromise for passwordless environments#1209
juliusknorr merged 2 commits intonextcloud-libraries:mainfrom
benjaminfrueh:fix/password-confirmation-sso

Conversation

@benjaminfrueh
Copy link
Contributor

Description

Fixes an error in the password confirmation interceptor when used with Strict mode in passwordless environments, e. g. SSO

Problem

In passwordless environments, the request interceptor exits early because of !isPasswordConfirmationRequired() without creating a validatePromise. The response interceptor then tries to resolve this undefined promise for Strict mode requests, causing a error.

This error prevents proper success feedback. For example, in nextcloud/files_external with SSO it caused multiple POST requests and multiple saved configurations because it could not get the database id from the response.

Solution

Add a check for validatePromise before resolving it. If validatePromise is undefined at this point, it has to be from !isPasswordConfirmationRequired().

@juliusknorr juliusknorr requested review from artonge and nfebe November 4, 2025 09:35
@juliusknorr juliusknorr added bug Something isn't working 3. to review Waiting for reviews labels Nov 4, 2025
Copy link
Collaborator

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find!

@benjaminfrueh benjaminfrueh requested a review from artonge November 4, 2025 15:50
@benjaminfrueh benjaminfrueh force-pushed the fix/password-confirmation-sso branch 3 times, most recently from 6985c61 to 306e1eb Compare November 6, 2025 10:47
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

❌ Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@78c6c23). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/password-confirmation.ts 0.00% 9 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1209   +/-   ##
=======================================
  Coverage        ?   14.82%           
=======================================
  Files           ?        8           
  Lines           ?      263           
  Branches        ?       12           
=======================================
  Hits            ?       39           
  Misses          ?      221           
  Partials        ?        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@benjaminfrueh benjaminfrueh force-pushed the fix/password-confirmation-sso branch 2 times, most recently from bea80ed to 2d4705a Compare November 6, 2025 11:16
@benjaminfrueh benjaminfrueh force-pushed the fix/password-confirmation-sso branch from 2d4705a to dc293ed Compare November 6, 2025 11:31
@juliusknorr juliusknorr merged commit 54965ad into nextcloud-libraries:main Nov 6, 2025
12 checks passed
@susnux
Copy link
Collaborator

susnux commented Nov 6, 2025

/backport to stable5

@backportbot
Copy link

backportbot bot commented Nov 6, 2025

The backport to stable5 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable5
git pull origin stable5

# Create the new backport branch
git checkout -b backport/1209/stable5

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick b1be655b dc293ed7

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/1209/stable5

Error: Failed to check for changes with origin/stable5: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews backport-request bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants