Skip to content

Added the way to activate remember me in the new authentication system #15464

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

Closed
wants to merge 1 commit into from

Conversation

parijke
Copy link
Contributor

@parijke parijke commented Jun 24, 2021

I had to search a while for myself not understanding why I did not have a REMEMBERME cookie when I activated the new system. So maybe handy to add it.

I had to search a while for myself not understanding why I did not have a REMEMBERME cookie when I activated the new system. So maybe handy to add it.
@@ -167,6 +167,27 @@ this:
The user will then automatically be logged in on subsequent visits while
the cookie remains valid.

Beware that in the new Authenitaction System you have to set the RememberMeBadge()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Beware that in the new Authenitaction System you have to set the RememberMeBadge()
Beware that in the new authenitaction system you have to set the ``RememberMeBadge()``

@@ -167,6 +167,27 @@ this:
The user will then automatically be logged in on subsequent visits while
the cookie remains valid.

Beware that in the new Authenitaction System you have to set the RememberMeBadge()
in the authenticate method of the authenticator, like:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
in the authenticate method of the authenticator, like:
in the ``authenticate()`` method of the authenticator::

Comment on lines +173 to +189
.. code-block:: php

public function authenticate(Request $request): PassportInterface
{
$email = $request->request->get('email', '');

$request->getSession()->set(Security::LAST_USERNAME, $email);

return new Passport(
new UserBadge($email),
new PasswordCredentials($request->request->get('password', '')),
[
new CsrfTokenBadge('authenticate', $request->get('_csrf_token')),
new RememberMeBadge(),
]
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.. code-block:: php
public function authenticate(Request $request): PassportInterface
{
$email = $request->request->get('email', '');
$request->getSession()->set(Security::LAST_USERNAME, $email);
return new Passport(
new UserBadge($email),
new PasswordCredentials($request->request->get('password', '')),
[
new CsrfTokenBadge('authenticate', $request->get('_csrf_token')),
new RememberMeBadge(),
]
);
}
public function authenticate(Request $request): PassportInterface
{
$email = $request->request->get('email', '');
$request->getSession()->set(Security::LAST_USERNAME, $email);
return new Passport(
new UserBadge($email),
new PasswordCredentials($request->request->get('password', '')),
[
new CsrfTokenBadge('authenticate', $request->get('_csrf_token')),
new RememberMeBadge(),
]
);
}

Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Some comments

@@ -167,6 +167,27 @@ this:
The user will then automatically be logged in on subsequent visits while
the cookie remains valid.

Beware that in the new Authenitaction System you have to set the RememberMeBadge()

Choose a reason for hiding this comment

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

Little spelling error :)

Suggested change
Beware that in the new Authenitaction System you have to set the RememberMeBadge()
Beware that in the new authentication system you have to set the ``RememberMeBadge()``

@javiereguiluz
Copy link
Member

@wouterj do you agree with the proposed changes here? I see that in your PR #15503 you didn't update the security/remember_me.rst file, so I'm curious. Thanks!

@wouterj
Copy link
Member

wouterj commented Jan 7, 2022

Hi there! There has been some unfortunate delay with documenting the new remember me system.

I've proposed a complete update of the remember docs now: #16376 I've cherry-picked your commit in that PR, as it's just me being late on this PR that avoided this commit to make it into the docs repo in the first place.
Thanks for taking the time to not only figure out what was missing your case, but also proposing a doc change to help others @parijke!

@wouterj wouterj closed this Jan 7, 2022
wouterj added a commit that referenced this pull request Jan 19, 2022
…dojulien, parijke, wouterj)

This PR was merged into the 5.3 branch.

Discussion
----------

[Security] Document the new remember me system

Fixes #15721, fixes #16149
Replaces #15464, replaces #15893
Ref #15908

In Symfony 5.3, along with the new authentication system, we also somewhat silently introduced a new remember me system. Time to update the remember me docs with all new features :)

This PR also includes 2 commits from open PRs in this repository. We've been very late with this one, and community members have already invested time in contributing necessary changes to the docs before. They deserve the credits :)

`@javiereguiluz` I'm sorry for not informing you of this work before, I see you just merged a third PR that is replaced by this one.

Commits
-------

5ac2d26 Document the new remember me system
2df2275 Added the way to activate remember me in the new authentication system
d36b949 Update remember_me.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants