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

Prep Security (make:user) for 6.0 release #984

Closed
weaverryan opened this issue Oct 1, 2021 · 2 comments · Fixed by #1010
Closed

Prep Security (make:user) for 6.0 release #984

weaverryan opened this issue Oct 1, 2021 · 2 comments · Fixed by #1010

Comments

@weaverryan
Copy link
Member

weaverryan commented Oct 1, 2021

Currently, for Symfony 5.x, we still generate (in make:user) deprecated methods (because we need to). For example, if I run make:user but choose NO to needing a password, it generates:

// ...
class User implements UserInterface
{
    // ...

    /**
     * This method can be removed in Symfony 6.0 - is not needed for apps that do not check user passwords.
     *
     * @see PasswordAuthenticatedUserInterface
     */
    public function getPassword(): ?string
    {
        return null;
    }
}

We need to prep MakerBundle before the 6.0 release to be smart enough to not generate these anymore (if you're using 6.0).

@bechir
Copy link
Contributor

bechir commented Oct 3, 2021

I think this is already done, when I did make:user with Symfony 6.0.0-DEV and i type "no" for "hash/check user passwords", It generate this:

<?php

namespace App\Security;

use Symfony\Component\Security\Core\User\UserInterface;

class User implements UserInterface
{
    private $email;

    private $roles = [];

    public function getEmail(): ?string
    {
        return $this->email;
    }

    public function setEmail(string $email): self
    {
        $this->email = $email;

        return $this;
    }

    /**
     * A visual identifier that represents this user.
     *
     * @see UserInterface
     */
    public function getUserIdentifier(): string
    {
        return (string) $this->email;
    }

    /**
     * @deprecated since Symfony 5.3, use getUserIdentifier instead
     */
    public function getUsername(): string
    {
        return (string) $this->email;
    }

    /**
     * @see UserInterface
     */
    public function getRoles(): array
    {
        $roles = $this->roles;
        // guarantee every user at least has ROLE_USER
        $roles[] = 'ROLE_USER';

        return array_unique($roles);
    }

    public function setRoles(array $roles): self
    {
        $this->roles = $roles;

        return $this;
    }

    /**
     * @see UserInterface
     */
    public function eraseCredentials()
    {
        // If you store any temporary, sensitive data on the user, clear it here
        // $this->plainPassword = null;
    }
}

But I have this error after:

Declaration of App\Security\UserProvider::upgradePassword(Symfony\Component\Security\Core\User\UserInterface $user, string $newHashedPassword): void
must be compatible with Symfony\Component\Security\Core\User\PasswordUpgraderInterface::upgradePassword(Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface $user, string $newHashedPassword): void

It's not this?

if (60000 > Kernel::VERSION_ID) {
// Add methods required to fulfill the UserInterface contract
$this->addGetPassword($manipulator, $userClassConfig);
$this->addGetSalt($manipulator, $userClassConfig);
// Symfony >=5.3 uses "@see PasswordAuthenticatedInterface" for getPassword()
if (interface_exists(PasswordAuthenticatedUserInterface::class)) {
$manipulator->addUseStatementIfNecessary(PasswordAuthenticatedUserInterface::class);
}
// Future proof the entity for >= Symfony 6 && the entity will check passwords
if ($userClassConfig->hasPassword() && interface_exists(PasswordAuthenticatedUserInterface::class)) {
$manipulator->addInterface(PasswordAuthenticatedUserInterface::class);
}
return;
}
// Future proof >= Symfony 6.0
if (!$userClassConfig->hasPassword()) {
return;
}

@weaverryan
Copy link
Member Author

Ah, ok, I think you're right. So, there are a few things that we just need to "make sure are correct" on 6.0:

A) If we choose "no password", we should NOT get a getPassword() or getSalt() method. It looks like this may already be correct, though it looks like (from the error above) there may be a problem with the generated user provider (or something).

B) Also based on the above generated code, it looks like it generated a getUsername() method, which is not needed on 6.0.

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 a pull request may close this issue.

2 participants