Skip to content

Add an example for using auth for logging in #174

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 18 commits into from
Mar 23, 2025

Conversation

sc68cal
Copy link
Contributor

@sc68cal sc68cal commented Mar 10, 2025

This adds a small example for using authentication and signing in a user. This will be linked to from the jetzig.dev documentation, for how to use the authentication middleware

@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 10, 2025

This will require adding a postgres database to the CI and calling the database creation and migration commands. I can add that to the CI configuration, once I read up a bit on https://docs.github.com/en/actions/use-cases-and-examples/using-containerized-services/creating-postgresql-service-containers

@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 10, 2025

If this is not desired, I can also just paste the code into the documentation, in the middware auth section.

@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 10, 2025

Looking at https://github.com/marketplace/actions/setup-postgresql-for-linux-macos-windows since the CI system uses all three platforms

@bobf
Copy link
Contributor

bobf commented Mar 10, 2025

@sc68cal Nice, this looks good. Feel free to push changes to add postgres, I've been wanting to have database coverage in the demo app for a while now.

@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 10, 2025

I'm running it locally and hitting an issue with the tests where they fail on a PG error.

[FAIL] src.app.views.format::"index (html)" (PG)[LEAKED] 
[FAIL] src.app.views.format::"get (html)" (PG)[LEAKED] 
[FAIL] src.app.views.format::"get (json)" (PG)[LEAKED] 
[FAIL] src.app.views.quotes::"get" (PG)[LEAKED] 
[FAIL] src.app.views.cache::"index" (PG)[LEAKED] 
[FAIL] src.app.views.anti_csrf::"post with missing token" (PG)[LEAKED] 
[FAIL] src.app.views.anti_csrf::"post with invalid token" (PG)[LEAKED] 
[FAIL] src.app.views.anti_csrf::"post with valid token but missing expected params" (PG)[LEAKED] 
[FAIL] src.app.views.anti_csrf::"post with valid token and expected params" (PG)[LEAKED] 
[FAIL] src.app.views.anti_csrf::"index" (PG)[LEAKED] 
[FAIL] src.app.views.markdown::"index" (PG)[LEAKED] 
[FAIL] src.app.views.render_template::"index" (PG)[LEAKED] 
[FAIL] src.app.views.file_upload::"index" (PG)[LEAKED] 
[FAIL] src.app.views.file_upload::"post" (PG)[LEAKED] 
[FAIL] src.app.views.kvstore::"index" (PG)[LEAKED] 
[FAIL] src.app.views.params::"post query params" (PG)[LEAKED] 
[FAIL] src.app.views.params::"post json" (PG)[LEAKED] 

The error from PG is

┌───────────────────────────────────────────┐
│ FAILURE: src.app.views.anti_csrf::"index" │
├───────────────────────────────────────────┘
│ connect error: sorry, too many clients already

Now, I am running podman and running a postgres container locally. I think it's because the container by default has a very small max connections. Let me adjust it and retry.

@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 10, 2025

Ok, so running it with 200 max connections gets it down to only 4 tests fail with too many connections error but that's a lot of connections. Does jetquery do connection pooling? Or do we need to stick something like pgcat in front of it?

podman run -p 127.0.0.1:5432:5432 -e POSTGRES_PASSWORD=test -e POSTGRES_DB=test -d postgres -c max_connections=200

@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 10, 2025

Tests pass if the postgres container gets configured with -c max_connections=400

That's a lot of connections.

@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 10, 2025

Discovered JETQUERY_POOL_SIZE - setting it to 1 gets all tests to pass when running the standard postgres container from dockerhub. I did try values 10,5, 2 and those did not pass.

sc68cal added 3 commits March 10, 2025 17:53
I always mess this up when moving from shell to YAML configs
Let jetzig create it via the commands
@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 10, 2025

All this code is extracted from an actual project I am building with jetzig, with all the extra bits like CSS styling and such stripped out, to give a bare bones example.

I will add some tests that creates a user and exercises the POST to log them in, in my next coding session.

@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 10, 2025

And why yes, I was coding at 1AM local time fiddling with all this, as you can see from the database migration.

@sc68cal
Copy link
Contributor Author

sc68cal commented Mar 11, 2025

OK. I'm at a good stopping point here, at least for this PR. I am perfectly fine with this being squashed into a single commit, or I can respin this PR to combine the commits into a more logical sequence if required.

I have some lingering concerns around JETQUERY_POOL_SIZE - there seems to be something not quite working properly. I have created a postgres container with max_connections cranked very high, and a JETQUERY_POOL_SIZE of 10 and the tests will fail with a too many connections error. I am probably going to open an issue to investigate this further, but it's at a point where it's not relevant to this PR's overall objective.

@sc68cal sc68cal marked this pull request as ready for review March 11, 2025 01:40
sc68cal added 5 commits March 10, 2025 21:49
This allows us to use database.zig for CI, while still creating a blank
database.zig for new projects that are created via jetzig init
@bobf
Copy link
Contributor

bobf commented Mar 20, 2025

@sc68cal I'm going to get this merged this weekend ! Thank you for all the work you've done on it so far. ❤️

bobf and others added 2 commits March 23, 2025 13:28
Adjust test, fix memory leak in hashPassword, deinit repo on test app…
@bobf bobf merged commit 2c52792 into jetzig-framework:main Mar 23, 2025
3 checks passed
@bobf
Copy link
Contributor

bobf commented Mar 23, 2025

Nice ! Finally we have database coverage in CI.

@sc68cal sc68cal deleted the login_example branch April 1, 2025 04:22
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.

2 participants