Skip to content
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

Feature: Login with Session Cookie #207

Merged
merged 7 commits into from
Jul 31, 2024

Conversation

devdot
Copy link
Contributor

@devdot devdot commented Jun 5, 2024

This package already handles session cookies for authentication between requests. With these few changes, the session cookie string can be extracted (to be stored elsewhere) and re-used at a later time. The suggested login flow follows that of authentication by login token, verifying the session cookie through an attempted call to whoami.

I've tested this against my own CT instance, I'm not sure about the official integration tests of this package though (I'll try to provide tests).

@devdot devdot marked this pull request as ready for review June 5, 2024 11:50
@devdot
Copy link
Contributor Author

devdot commented Jul 24, 2024

@DumbergerL any chance this can get merged?

DumbergerL added a commit to devdot/churchtools-api that referenced this pull request Jul 29, 2024
@DumbergerL
Copy link
Contributor

@devdot I'm sorry that I've kept you waiting so long for an answer. The code looks very good so far, but I can't get the integration test “testAuthWithSessionCookie” to work.

Does the test run with your ChurchTools instance? To test this you have to rename the file “integration.example.ini” to “integration.ini” and enter your credentials and adjust the api_url in the “integration-test-data.json”.

@DumbergerL DumbergerL added the help wanted Extra attention is needed label Jul 30, 2024
@devdot
Copy link
Contributor Author

devdot commented Jul 30, 2024

@DumbergerL thank you!

There was an error in the integration test that is now fixed (at least it is now running successfully on my end - thanks for the hints).

I've added a few more details to the implementation and fixed some issues that I came across while testing this new feature in another project. Let me know if anything is unclear to you!

This was referenced Jul 30, 2024
@DumbergerL
Copy link
Contributor

Nice one @devdot! Thank you very much!

@DumbergerL DumbergerL merged commit d8bc6d2 into 5pm-HDH:master Jul 31, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants