enable PKCE by default

[isdnfan]

with #740 PKCE was disabled by default. According to different sources PKCE is more secure and recommended for all sorts of clients:

https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-16#section-2.1.1

Clients MUST prevent injection (replay) of authorization codes into the authorization response by attackers. Public clients MUST use PKCE [RFC7636] to this end. For confidential clients, the use of PKCE [RFC7636] is RECOMMENDED.

in general IdP MUST advertise PKCE support and user_oidc should dynamically adopt rather hard-coding one or another variant:

https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-16#section-2.1.1

Authorization servers MUST provide a way to detect their support for PKCE. To this end, they MUST either (a) publish the element code_challenge_methods_supported in their AS metadata ([RFC8414]) containing the supported PKCE challenge methods (which can be used by the client to detect PKCE support) or (b) provide a deployment-specific way to ensure or determine PKCE support by the AS.

Please reverse use_pkce setting and enable PKCE by default:

'user_oidc' => [
    'use_pkce' => true,
],

in order

• to keep backward compatibility
• use RECOMMENDED config by default
• people could still disable it if required by broken IdP

[edward-ly]

I see a few possible solutions here:

1. Add 'user_oidc' => [ 'use_pkce' => true, ], to the default config.php, in which case there is nothing to be done on the user_oidc side.
2. Default to true if use_pkce is not specified.
3. Default to the value of has_internet_connection if use_pkce is not specified.

If possible, (1) would probably be the best solution, but if not, I would go with (3) as the next best option as PKCE is not a strict requirement for private IdPs. I love to hear what everyone thinks before making a decision, though.

[isdnfan]

@edward-ly: my intention was to change the default in case use_pkce is not defined (case 2 in the list).

Please allow some comments on your list: Option 1: If the setting is defined this is an admin task to provide/adjust right values. Option 1 relies on config.php option which is not really different from now (may or may not exist after upgrades). Option 3 would introduce some "hidden" assumption which is not obvious and IMHO not a good practice.

as described in the issue I would appreciate user_oidc doesn't hardcode PKCE support but rather consume it from IdP. and in case there is some broken IdP the admin should take care of it rather adding a burden to everybody else by setting a bad default.