Support existing accessToken on OpenIdConnectAuthentication

[ericclaeren]

Hi, I've created a pull request allowing people to:

1. Setup a connection with an existing access token object
2. Add a custom callback to be called when a token is refresh, to process the token.

There's already a similar idea in: #144 (comment)

That PR is fairly old and did not seem to be very active. Also it included some other unrelated improvements, that might makes this hard to merge to master.

The code has some breaking changes in case someone is overriding src/Secure/OpenIdConnectAuthentication.php login() or validateToken()

Summary of improvements

• Allow setting an AccessToken object, through constructor or setter
  • This prevents unnecessary token requests in case the access token is valid.
• Only validate the token on login when:
  • The access token is set/replaced
  • The access token is refreshed when it's expired
• Allows to add your own callback, that will be called when the token is validated, I chose this place, because it's at that point validated by Twinfield that the token is valid. The callable will receive the full AccessToken object and can be processed within the application.
  • In my use case I'm updating my client token, so on every subsequent requests within the validity (1 hour) it won't fetch a new token from twinfield, which saves an additional request.

Previous login flow on every getAuthenticatedClient() call

• Get access token
• Validate access token

New flow

• Has access token
  • Has valid access token
  • Validate access token
• Has no access token -> previous flow
  • Ability to store access token, to continue first flow on subsequent calls.

I've documented the use case in the readme.md file to provide clarity and documentation and wrote tests for this functionality.

Hope this PR can be helpful and if there are questions, don't hesitate to contact me or responding on this PR.

Cheers

[rojtjo]

Thank you for your pull request, it looks great!

I'm just wondering if it would make sense to move these changes to the v4 branch instead. That way we could maybe get rid of the refreshToken property all together as it's already included in the AccessToken.

Let me know what you think!

[ericclaeren]

Hi @rojtjo,

Removing $refeshToken was also one of my initial thoughts, left it in for compatibility.
But definitely agree with you, removing this would make sense.

If you agree, I would prefer to do both, I'm happy porting this code to the v4 branch.
But also think it might be good to introduce this and gather feedback on it.

And I need it in the current v3 branch, to limit the amount of requests, since I'm syncing a rather large data set to Twinfield.
But if you don't want to merge it and hold on to it for v4, that's fine by me, I can still use this PR to patch this package.

[rojtjo]

I guess that makes sense, it's easy to change this for v4 once merged.

[ericclaeren]

Thanks!