OIDC: Look to support PKCE and/or nonce #4368

New Issue

OVERLORD · 2026-02-05T08:41:48+03:00

OVERLORD commented

2026-02-05 08:41:48 +03:00

Originally created by @ssddanbrown on GitHub (Dec 19, 2023).

With the intention to provide an extra layer of security.
Looks like there may be some overlap between these potential options, so may need to assess benefits relative to support and requirement by Identity providers.

Relevant articles:

https://danielfett.de/2020/05/16/pkce-vs-nonce-equivalent-or-not/

Originally created by @ssddanbrown on GitHub (Dec 19, 2023). With the intention to provide an extra layer of security. Looks like there may be some overlap between these potential options, so may need to assess benefits relative to support and requirement by Identity providers. Relevant articles: - https://danielfett.de/2020/05/16/pkce-vs-nonce-equivalent-or-not/

OVERLORD added the 🛠️ Enhancement 🚪 Authentication 🔒 Security 🏭 Back-End labels 2026-02-05 08:41:48 +03:00

OVERLORD closed this issue

2026-02-05 08:41:49 +03:00

OVERLORD commented

2026-02-05 08:41:54 +03:00

@tumbl3w33d commented on GitHub (Jan 2, 2024):

Hi Dan, I heard about this issue from a colleague and while I don't have the detailed knowledge myself, I hang out in a channel with auth oracles and tried to gather some information. I will just add some hypotheses to be verified:

the nonce only protects the id_token
PKCE protects the entire code exchange including the access token
PKCE is needed if you don't have a confidential client, else the code exchange is insecure as you can retrieve the id token with just the code which can leak via access logs
OAuth 2.1 will make PKCE mandatory to implement
nonce is to be verified by the client but the server cannot enforce this security measure, while with PKCE the server can refuse to play with the client, because you can refuse the request to /token if it doesn't contain the PKCE verifier
here is an explanation of nuances between nonce and pkce

If you have more questions about that topic I can recommend the community channel of kanidm. Kani is the idp I use and the community and maintainers have strong knowledge about all the auth things.

@tumbl3w33d commented on GitHub (Jan 2, 2024): Hi Dan, I heard about this issue from a colleague and while I don't have the detailed knowledge myself, I hang out in a channel with auth oracles and tried to gather some information. I will just add some hypotheses to be verified: * the nonce only protects the id_token * PKCE protects the entire code exchange including the access token * PKCE is needed if you don't have a confidential client, else the code exchange is insecure as you can retrieve the id token with just the `code` which can leak via access logs * OAuth 2.1 will make PKCE mandatory to implement * nonce is to be verified by the client but the server cannot enforce this security measure, while with PKCE the server can refuse to play with the client, because you can refuse the request to `/token` if it doesn't contain the PKCE verifier * [here is an explanation](https://www.ietf.org/archive/id/draft-ietf-oauth-v2-1-09.html#authorization_codes) of nuances between nonce and pkce If you have more questions about that topic I can recommend the [community channel of kanidm](https://kanidm.com/community/). Kani is the idp I use and the community and maintainers have strong knowledge about all the auth things.

OVERLORD commented

2026-02-05 08:41:58 +03:00

@ssddanbrown commented on GitHub (Jan 25, 2024):

Thanks for the valuable insight @tumbl3w33d.
It looks like PKCE is the way to go and would cover the same (and greater) concerns than nonce does.

I've just been reading through the PKCE RFC.
It looks like we should be able to add this into BookStack by default without optional control, which would be nice, although I'd need to test that across a variety my of test auth systems/platforms to be sure that they do play nice as per the RFC, and don't create compatibility issues (Always suspicious of Microsoft Azure doing something awkward).

We'll need to be sure to update our OIDC guidance if adding by default, to advise enforcing PKCE on the auth system server side, otherwise it's of limited benefit.

@ssddanbrown commented on GitHub (Jan 25, 2024): Thanks for the valuable insight @tumbl3w33d. It looks like PKCE is the way to go and would cover the same (and greater) concerns than `nonce` does. I've just been reading through the [PKCE RFC](https://datatracker.ietf.org/doc/html/rfc7636). It looks like we should be able to add this into BookStack by default without optional control, which would be nice, although I'd need to test that across a variety my of test auth systems/platforms to be sure that they do play nice as per the RFC, and don't create compatibility issues (Always suspicious of Microsoft Azure doing something awkward). We'll need to be sure to update our OIDC guidance if adding by default, to advise enforcing PKCE on the auth system server side, otherwise it's of limited benefit.

OVERLORD commented

2026-02-05 08:42:09 +03:00

@ssddanbrown commented on GitHub (Jan 25, 2024):

PR now open for adding PKCE, targeted for the next feature release: https://github.com/BookStackApp/BookStack/pull/4804

@ssddanbrown commented on GitHub (Jan 25, 2024): PR now open for adding PKCE, targeted for the next feature release: https://github.com/BookStackApp/BookStack/pull/4804

OVERLORD commented

2026-02-05 08:42:12 +03:00

@ssddanbrown commented on GitHub (Jan 27, 2024):

Alrighty, #4804 is now merged.
Tested a bunch of OIDC auth systems there. Surprisingly most did not seem to provide options to force PKCE, but all supported PKCE and failed when invalid PKCE data was provided.
Maybe enforcing PKCE is as important as I originally thought.

Either way, there were no issues with enabling by default, and it's now in as an enhanced layer of security for OIDC where supported.
It will be included as part of the next feature release.

@ssddanbrown commented on GitHub (Jan 27, 2024): Alrighty, #4804 is now merged. Tested a bunch of OIDC auth systems there. Surprisingly most did not seem to provide options to force PKCE, but all supported PKCE and failed when invalid PKCE data was provided. Maybe enforcing PKCE is as important as I originally thought. Either way, there were no issues with enabling by default, and it's now in as an enhanced layer of security for OIDC where supported. It will be included as part of the next feature release.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/BookStack#4368