Skip to content

Conversation

@solracsf
Copy link
Member

@solracsf solracsf commented Oct 11, 2025

Possible fix for #1214

  • Prefer matching kid if present
  • Otherwise, select the first key compatible with the expected kty

Testing Notes (not in this PR):

  • Recommend adding future PHPUnit tests with various JWT and JWKS combinations, including kid present/absent and multiple key types (RSA, EC, OKP).

@solracsf solracsf requested a review from julien-nc October 11, 2025 06:51
@solracsf solracsf marked this pull request as draft October 11, 2025 07:15
@solracsf

This comment has been minimized.

@solracsf solracsf marked this pull request as ready for review October 11, 2025 15:52
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

Lgtm. Thanks a lot!
I'll rebase and merge.

@julien-nc julien-nc enabled auto-merge October 13, 2025 12:15
@julien-nc julien-nc merged commit 0c15a8f into main Oct 13, 2025
42 checks passed
@julien-nc julien-nc deleted the handleMissingKid branch October 13, 2025 12:39
@julien-nc julien-nc mentioned this pull request Oct 15, 2025
@joshtrichards
Copy link
Member

Is the supported algorithm list being used by this new check complete / up-to-date?

Looks like it may be missing EdDSA:

https://help.nextcloud.com/t/updated-openid-connect-breaks-using-eddsa-algo/234499

https://www.rfc-editor.org/rfc/rfc8037

@whisperdancer
Copy link

Any movement forward on fixing this? Thanks

@joshtrichards
Copy link
Member

@whisperdancer #1236 + #1254 should fix the matter. If you're able to test, that would be appreciated. Comment on #1254 with your results.

@whisperdancer
Copy link

Thanks @joshtrichards. I checked for app update and none available. Happy to test it, but unsure how to get the fix without an app update. Kindly advise. Thank you.

@joshtrichards
Copy link
Member

Until release you'd have to apply the patch(es) manually from the PRs to the app:

https://docs.nextcloud.com/server/latest/admin_manual/issues/applying_patch.html

@solracsf
Copy link
Member Author

@whisperdancer new app version is out.
Create an issue if needed, thanks 👍🏼

@whisperdancer
Copy link

Thank you! Tested and works great now :) Much appreciate the quick fix!!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants