OIDC implementation - #2960
OIDC implementation#2960
Conversation
…o jasperweyne-openid
- Made oidc config more generic to not be overly reliant on the library based upon learnings from saml2 auth. - Removed any settings that are redundant or not deemed required for initial implementation. - Reduced some methods down where not needed. - Renamed OpenID to OIDC - Updated .env.example.complete to align with all options and their defaults Related to #2169
- Removed uneeded custom refresh or logout actions for OIDC. - Restructured how the services and guards are setup for external auth systems. SAML2 and OIDC now directly share a lot more logic. - Renamed any OpenId references to OIDC or OpenIdConnect - Removed non-required CSRF excemption for OIDC Not tested, Come to roadblock due to lack of PHP8 support in upstream dependancies. Certificate was deemed to be non-valid on every test attempt due to changes in PHP8.
- Got basic signing support and structure checking done. - Need to run through actual claim checking before providing details back to app.
- Validation of all key/token elements now in place. - Signing key system updated to work with jwk-style array or with file:// path to pem key.
- To continue coverage and spec cases next.
|
Some of your dependencies you added here have overlapping logic/functionalities, and you might be able to abstract some heavy lifting of handling keys and doing checks may be outsourced to other/better dependencies. The library phpseclib/phpseclib seems to have some overlap with lcobucci/jwt (sub dependency of league/oauth2-client). I've personally not used phpseclib but am not / was not impressed by lcobucci/jwt (though the v4 is much better, but it took a while to release). We've adopted the following which helped us get rid of more boilerplate key retrieval/matching logic and signature & claim checks. It might help to reduce logic here too. Especially the checking a token against a keyset url is powerful and supported algorithms is pretty useful. Of course make your own judgement, I must admit I've not been able to go through all code in detail. |
|
Thanks for the input @sg3s. It may be that old diffs are showing or something, I've been slowly changing the implementation of that from the original PR hence there has been some overlapping dependencies. Should be cleaned-up as of last night though.
I don't see I had issues with library versions, or libraries having larger than desired dependency chains or questionable maintenance & testing coverage. When I got into things, the slice of JWT handing that we need to take into account of for OIDC seemed quite focused, with the spec being clear about validation required. Therefore I'm taking the approach of adding the OIDC & JWT layer myself with It's pretty much there now, Just fleshing out testing coverage. Basic auto-discovery is now possible, and key handling has been reworked to run via jwk or a key file as per the original PR. |
|
My bad, I might have assumed where lcobucci/jwt came from and I did go through things by commit. Certainly making sure dependencies aren't too much of a burden is very important. Looking forward to give this a spin soon :) P.S. Another thought that crossed my mind is that you might want to let users configure which login method is primary/preferred & in case of social login/SAML/OIDC you could auto-redirect to the issuer/authority when clicking login - but that is a separe thing. |
Tested oidc system on okta, Keycloak & Auth0
3 participants
Continuation of #2169
TODO
References
For Docs