Skip to content

feat: add user authentication (#2)#59

Open
martinydeAI wants to merge 7 commits into
developfrom
feature/issue-2-user-login
Open

feat: add user authentication (#2)#59
martinydeAI wants to merge 7 commits into
developfrom
feature/issue-2-user-login

Conversation

@martinydeAI

Copy link
Copy Markdown
Collaborator

Summary

Implements #2 end-to-end: the application's first persistent identity, the form-login flow that gates it, the supporting console tooling, and the tests that hold all of it at 100 % coverage.

Backend

  • Bundles installed: symfony/orm-pack (Doctrine ORM + migrations + bundle), symfony/security-bundle, plus symfony/maker-bundle and doctrine/doctrine-fixtures-bundle as dev-only deps.
  • Database wired to MariaDB. The recipe defaulted to Postgres; reverted the docker-compose.yml additions and the generated compose.override.yaml, dropped the Postgres-specific bits from config/packages/doctrine.yaml, and set DATABASE_URL in .env to point at the in-stack mariadb service. Granted the db user privileges on db_test* so PHPUnit's test DB works.
  • App\Entity\User — email, hashed password, roles. Repository implements PasswordUpgraderInterface for transparent rehashing.
  • App\Security\UserManager — single seam owning persistence + hashing. Throws \DomainException on duplicate email / missing user and \InvalidArgumentException on empty password. Per project conventions the controller and commands stay thin and delegate here.
  • security.yamlapp_user_provider (entity provider on email), form_login + declarative logout on the main firewall, /login and /logout publicly accessible.
  • Initial migration Version20260611124347.php creates the user table (id, email unique, roles JSON, password). Migration applied locally and against the test DB.

Login flow

  • App\Controller\SecurityController/login renders the form using Symfony Security's AuthenticationUtils; /logout is intercepted by the firewall.
  • templates/security/login.html.twig — extends base.html.twig, uses the <twig:Eyebrow> component, CSRF token, and translation keys.
  • Translation keys under security.login.* added to translations/messages.da.yaml.

Tooling

  • app:user:create <email> <password> — creates a new user via UserManager.
  • app:user:change-password <email> <password> — rotates an existing user's password.
  • App\DataFixtures\UserFixtures — seeds alice@example.test and bob@example.test, both with password password, via UserManager (same code path the commands use).

Tests — 32 cases, 70 assertions, 100 % coverage

  • tests/Support/ResetsDatabaseSchemaTrait — drops and recreates the schema from ORM metadata in setUp. Shared by every DB-touching test class.
  • tests/Security/UserManagerTest — happy paths and four exception branches.
  • tests/Repository/UserRepositoryTest — covers upgradePassword for the supported and unsupported cases.
  • tests/Command/UserCreateCommandTest and UserChangePasswordCommandTest — CommandTester wrappers, success + failure paths.
  • tests/DataFixtures/UserFixturesTest — verifies the fixture loads both users.
  • tests/Controller/SecurityControllerTest — full HTTP flow: page renders, successful login redirects to / with token populated, failed login redirects back with no token, /logout clears the session. Also unit-tests the logout method's defensive throw.

Docs

  • README.md — new Creating the first user subsection covering migration, fixture load, and the two console commands.
  • CHANGELOG.md[Unreleased] / Added entry referencing User login #2.

Out of scope

  • Password reset, email verification, OAuth/SSO (explicitly excluded by the issue).
  • Roles beyond ROLE_USER (the framework's implicit guarantee is enough for now).
  • The active, domainManager, name fields from feat: add User entity with active / domainManager / name fields #45 — they'll land in a follow-up PR with their own migration (per the answer to the scope question on this PR's setup).

Local verification

task site-install
task console -- doctrine:migrations:migrate -n
task console -- doctrine:fixtures:load -n
# Visit /login, sign in as alice@example.test / password
task test-coverage

Test plan

  • task test-coverage → 100 %, 32 tests, 70 assertions.
  • task coding-standards-check → green across PHP, Twig, YAML, JS, CSS, Markdown, Composer.
  • /login renders, accepts credentials, redirects to / on success.
  • /logout clears the session and redirects to /.
  • app:user:create + app:user:change-password succeed and surface domain errors.

Closes #2

🤖 Generated with Claude Code

martinydeAI and others added 7 commits June 11, 2026 19:07
Wires up the application's first persistent identity and the login flow it gates. Installs Doctrine ORM + migrations + Symfony Security + dev-only MakerBundle and DoctrineFixturesBundle; switches doctrine.yaml to MariaDB and sets DATABASE_URL to the in-stack mariadb service.

App surface: App\Entity\User (email, hashed password, roles), UserRepository with PasswordUpgraderInterface, App\Security\UserManager service that owns persistence + hashing, App\Controller\SecurityController exposing /login and /logout (form_login + declarative logout in security.yaml), Twig login form under templates/security/, and Danish translation keys under security.login.*. Two console commands sit on UserManager: app:user:create and app:user:change-password. App\DataFixtures\UserFixtures seeds alice@example.test and bob@example.test (password `password`) for local dev.

Tests: 32 cases, 70 assertions, 100% coverage. UserManager unit-tested via KernelTestCase + a schema-reset trait; UserRepository's upgradePassword covered for the happy path and the foreign-user rejection; commands exercised through CommandTester; SecurityController's full /login + /logout + failed-login flow exercised through WebTestCase. Tests share tests/Support/ResetsDatabaseSchemaTrait which drops + recreates the db_test schema from ORM metadata in setUp.

Docs: README gains a "Creating the first user" subsection covering migration, fixture load, and the two console commands. CHANGELOG entry under [Unreleased] / Added references #2.

Closes #2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PHPUnit's test environment uses Symfony's dbname_suffix to talk to a separate test database (db_test, optionally db_test_paratest_N). The itkdev/mariadb image only grants MYSQL_USER on MYSQL_DATABASE, so the test runner hits 'Access denied for user db@% to database db_test' on a fresh container.

Mount .docker/mariadb/init/ as /docker-entrypoint-initdb.d/ so the included GRANT runs on first container initialisation. The wildcard is escaped as db\_test% so it only matches db_test... not unrelated names like dbXtest.

Local devs with an already-initialised mariadb volume can either recreate the container (task compose -- down -v && task site-install) or apply the grant once manually.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds task db-prepare-test which re-applies the GRANT init SQL and ensures db_test exists. test and test-coverage depend on it so any local dev whose mariadb data volume predates the new /docker-entrypoint-initdb.d mount can run tests without first recreating the container. The grant is idempotent and only touches privileges — no data is read or written.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The earlier init script only granted privileges on db_test* but never created the database. CI runs vendor/bin/phpunit directly (not task test), so the db-prepare-test target couldn't backfill the CREATE either, and the Tests workflow failed with 'Unknown database db_test'. Folds CREATE DATABASE IF NOT EXISTS db_test into the init SQL so both fresh CI containers and the local db-prepare-test target reach the same state. The doctrine:database:create call in db-prepare-test is now redundant and removed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds doctrine:migrations:migrate as the final step of site-install so a fresh check-out's database schema lands automatically. Drops the manual 'Apply the database schema' snippet from the README's user-creation section since the schema is now in place by the time anyone reaches it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 'Creating the first user' section doesn't need to remind readers that site-install ran migrations — that's documented in the section above. Removes the explanatory paragraph and leaves just the commands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@symfony enables phpdoc_align with align=vertical, which padded @param and @return so columns lined up and pushed descriptions onto extra wrapped lines. Override to align=left and rewrite the verbose blurbs in the user-auth code so each tag fits on one line.

Net 46 lines removed across UserManager, the two console commands, SecurityController, UserFixtures, and DevTemplateMarkerNodeVisitor (the last one fell out of the same cs-fixer pass).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@martinyde martinyde left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a scraped user entity with login functionality, commands related to user management and a few fixtures for good measure. The user entity is not complete with this PR and we have not yet implemented any restrictions based on user session or role. I have added some comments to pinpoint my uncertainties in regards to our usual user/login implementations.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this the correct approach to ensure a database for testing?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with our standards in regards to doctrine config, so this might need corrections

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with our standards regarding security config

$command = $application->find('app:user:change-password');
$this->tester = new CommandTester($command);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is our document/commneting guidelines regarding test methods?


<input type="hidden" name="_csrf_token" data-controller="csrf-protection" value="{{ csrf_token('authenticate') }}">

<button type="submit"

@martinyde martinyde Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should the button element be a separate template? @yepzdk

@yepzdk yepzdk Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@martinyde I think we would benefit from having a template for buttons, with variables for size, variant etc.
You mentioned Symfony UX earlier, maybe we should utilize it? https://symfony.com/bundles/ux-twig-component/current/index.html#component-basics

<form action="{{ path('app_login') }}" method="post" class="grid gap-4">
<label class="grid gap-1 text-sm" for="inputEmail">
<span class="font-medium text-ink">{{ 'security.login.email_label'|trans }}</span>
<input id="inputEmail"

@martinyde martinyde Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should the input field be a separate template, maybe including the label wrapper ? @yepzdk

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, same as for the buttons. Label should maybe be its own component?

class="rounded-lg border border-line bg-surface px-3 py-2 text-base text-ink focus:outline-none focus:ring-2 focus:ring-primary/40">
</label>

<label class="grid gap-1 text-sm" for="inputPassword">

@martinyde martinyde Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same question as above? @yepzdk

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think label should be its own component, so we can change it in one place.

) {
}

/**

@martinyde martinyde Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In my opinion this is the right amount of commenting for a method, unless it's a standard controller method and maybe a test (see other comment).

Comment thread .php-cs-fixer.dist.php

$config->setRules([
'@Symfony' => true,
// Override the Symfony default that vertically aligns @param / @return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a preference i added, that (in my opinion). makes comments more readable.

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.

User login

3 participants