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.

Sign up for GitHub

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

Jump to bottom

fix: Do not build encrypted password if there is none #51130

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: Do not build encrypted password if there is none #51130

wants to merge 1 commit into from
+49 −5

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Feb 28, 2025 •
edited
Loading

This fixes a regression from #48915 where user backends without passwords would store an encrypted empty password for new app passwords.

While they first work, after 5 minutes the password is checked and cannot be validated as there is none so the token expired.

Checklist

@juliusknorr juliusknorr marked this pull request as ready for review February 28, 2025 11:11
@juliusknorr
Copy link
Member Author

/backport to stable31

@juliusknorr
Copy link
Member Author

/backport to stable28

@juliusknorr
Copy link
Member Author

/backport to stable29

@juliusknorr
Copy link
Member Author

/backport to stable30

provokateurin
provokateurin requested changes Feb 28, 2025
View reviewed changes
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Sounds like something that requires a test to avoid regressions.

pabzm
pabzm approved these changes Feb 28, 2025
View reviewed changes
Copy link
Contributor

@pabzm pabzm left a comment

Choose a reason for hiding this comment

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

Apart from the minor style suggestion it looks good to me! 👍

@susnux susnux added this to the Nextcloud 32 milestone Mar 2, 2025
@juliusknorr juliusknorr force-pushed the fix/credential-passwordless-auth branch from 666a017 to 777cd94 Compare March 6, 2025 08:31
@juliusknorr juliusknorr requested a review from a team as a code owner March 6, 2025 08:31
@juliusknorr juliusknorr requested review from icewind1991 and removed request for a team March 6, 2025 08:31
@juliusknorr
Copy link
Member Author

Pushed a unit test

@provokateurin provokateurin enabled auto-merge March 6, 2025 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@provokateurin provokateurin provokateurin approved these changes

@pabzm pabzm pabzm approved these changes

@blizzz blizzz Awaiting requested review from blizzz

@artonge artonge Awaiting requested review from artonge

@yemkareems yemkareems Awaiting requested review from yemkareems

@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 is a code owner automatically assigned from nextcloud/server-backend

Assignees
No one assigned
Labels
3. to review Waiting for reviews backport-request bug feature: authentication
Projects
None yet
Milestone
Nextcloud 32
Development

Successfully merging this pull request may close these issues.
4 participants
@juliusknorr @provokateurin @pabzm @susnux