Skip to content

Adjust for PrimaryKey session. #124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 5, 2025
Merged

Adjust for PrimaryKey session. #124

merged 2 commits into from
May 5, 2025

Conversation

dereuromark
Copy link
Contributor

Resolves #123

Also cleaned up PHPStan and CS.

@dereuromark
Copy link
Contributor Author

dereuromark commented Apr 22, 2025

With this it actually works out of the box for me with the new Auth plugin and PrimaryKeySession:
https://github.com/dereuromark/cakephp-tinyauth/blob/master/src/Authenticator/PrimaryKeySessionAuthenticator.php

For me all tests pass locally.

@ADmad
Copy link
Owner

ADmad commented Apr 22, 2025

The change is fine, just need test coverage for the new option

@dereuromark
Copy link
Contributor Author

It doesnt look like it has any such tests yet regarding user table access, neither with array or Entity.
As such I tried to add specifically for id case, but I cannot even get passed the getUser() missing from the Table, since there was also never a callback test or so.

I wonder how to best achieve this given these circumstances.

@dereuromark
Copy link
Contributor Author

Tried this
dereuromark/cakephp-social-auth@master...dereuromark:cakephp-social-auth:tests

And I can see it being in the session correctly before the middleware redirects
But the test case itself doesn't reflect that for some reason

@dereuromark
Copy link
Contributor Author

dereuromark commented Apr 30, 2025

ping @ADmad
I can confirm that it works
Pushed the tmp branches on my toolbox

@ADmad ADmad merged commit d0b5257 into ADmad:master May 5, 2025
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authenticator plugin adaptation
2 participants