ISSUE_218_Support_single_page_app_browser_clients_using_authorization_code_and_pkce

[ioigoume]

218

[cicnavi]

@ioigoume thanks for the contribution! @pradtke will you be reviewing this? A few hints:

• phpunit tests (currently) won't show you if there are any problems with the client management UI. For example, there is 'ArgumentCountError: Too few arguments to function SimpleSAML\Module\oidc\Forms\ClientForm::__construct()...' error when I try to edit the client. So, you will also have to manually test the client forms yourself.
• you can also run command 'composer pre-commit' which will also run phpcs and psalm, in addition to phpunit. There are some issues there...
• I would strongly advise to use mocks instead of real dependencies in unit tests. For example, you use real Session instead of mock, and this actually forced you to tackle with globals like '$_SERVER['REQUEST_URI'] = '/dummy''...
• I guess that this PR won't introduce breaking changes. If so, we can merge this into master which is at v5 level and make this available earlier
• If this PR is not yet ready for review, mark it as draft

[ioigoume]

• composer pre-commit

Hi @cicnavi thank you for the comments. I will address the changes and push the fixes.

[pradtke]

Thank you @cicnavi and @ioigoume . I'll run a few tests tomorrow and then I can merge it in.

[cicnavi]

I hurried up with merging stuff to wip-version-6 bc Tim announced more static analysis related commits coming next week 😅... There were some collisions which I had to solve when I merged to this branch, but I think it should be ok....

[pradtke]

@cicnavi I'm ready to merge this. I just want to confirm the target is wip-version-6 and do you have a preference between doing a squash commit or merge commit? I wasn't sure if you care about commit log noise. If no preference I'll make this a squash commit.

[cicnavi]

Yes, squash to v6.