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

Added keycloak integration test #342

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ricklambrechts
Copy link
Contributor

@ricklambrechts ricklambrechts commented Oct 24, 2022

I currently have a running keycloak service.

Part of #340

Next things todo:

  • Create or import a basic realm
  • Create a test script that uses this package and connects to the realm
  • Serve the test script
  • Create cypress test to check if authentication flow is working

@ricklambrechts
Copy link
Contributor Author

@azmeuk @DeepDiver1975

First integration test 🎉
Let me know what you think.

Things to improve:

  • Import realms/users/clients to Keycloak instead of making them one by one
  • Create small application (with something like https://www.slimframework.com/) with multiple endpoints for the different clients
  • Add additional test cases

Copy link
Collaborator

@azmeuk azmeuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for this! ❤️

I am wondering: is cypress needed here?
Wouldn't PHP testcases with requests against keycloak in PHPUnit be enough? I don't really know cypress, so I fear this would make an additional difficulty for contributors.
What do you think?

- name: Run simple test application
run: |
cd .github/workflows/keycloak
make serve-test-application > /dev/null 2>&1 &
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why redirecting the outputs to /dev/null? When everything is fine you surely don't need it, but this might be useful to debug when things are broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could indeed save the output as a kind of debug log., thanks.

@ricklambrechts
Copy link
Contributor Author

Thanks!

I think that multiple integration tests are really usefull to check if the package can load the openid-configuration correctly at first, than a user really can login at the identity provider and after that check if the application can load the user info endpoint.

The package redirects the user to the login screen of the identity provider, after successful login the user is redirected to the sample code where the user name is shown. With Cypress, selenium or another tools we can fully test this flow.

@DeepDiver1975
Copy link
Collaborator

I have to agree with @azmeuk - adding cypress here is a bit of an overkill.

From my understanding we don't need to test full flows but only parts of it - meaning dedicated calls to keycloak and assert the response.

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.

3 participants