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

[9.x] Add custom segments on "remember me" for session rebuild #42316

Merged
merged 3 commits into from
May 10, 2022

Conversation

PaolaRuby
Copy link
Contributor

@PaolaRuby PaolaRuby commented May 9, 2022

Closes #42271

Actually Recaller only admit 3 segments, id,token,hash,
If we create a custom session class extending SessionGuard, or we change the remember me cookie on login, we need a way to get the extra data from recaller, this is useful when you have a multi company login, or when needs an extra id/value to rebuild the session,

Example: company is a manyToMany relation with user, on login user pick wich company use for session, on "remember me" laravel rebuild the user session but not the company, of course i can create another cookie for rebuild the picked company or another company selector after session rebuild, but would be great if the recaller can handle that extra id/ids

keeps same functionality, but adds option for more segments, really small not breaking change


#42271 (comment)

How are you setting these custom segments?

Login, custom session guard, or auth macro

Why do you need them at all?

Rebuild the picked login company on session with remember me

Why not store them in a database table associated with the user?

User can be logged on two or more companies at the same time(using different browsers, or with incognito mode, different computers, etc)


@taylorotwell
Copy link
Member

How are you actually setting the custom segments? Can you provide a code example?

@PaolaRuby
Copy link
Contributor Author

PaolaRuby commented May 9, 2022

@taylorotwell thanks for answer
#42262 (comment)
on a custom guard extending SessionGuard , overwriting methods, and creating a new recaller

protected function queueRecallerCookie(AuthenticatableContract $user)  {
    $this->getCookieJar()->queue($this->createRecaller(
        $user->getAuthIdentifier().'|'.$user->getRememberToken().'|'.$user->getAuthPassword().
            '|'.session('company_id', $this->request->input('company_id'))
    ));
}
protected function userFromRecaller($recaller){      
      $user = parent::userFromRecaller($recaller);
      if ($user && $this->viaRemember && !($recaller->customSegments()[0] ?? false)) {
            return;
      }
      $this->session->put(['company_id' => $recaller->customSegments()[0]]);
      return $user;
  }

I can remove all the overwriting with this change, and use a middleware and an auth macro

Maybe, is there a better option for this?

@taylorotwell
Copy link
Member

How can you remove all that code? This PR gives no way to define custom segments - only retrieve them?

@PaolaRuby
Copy link
Contributor Author

PaolaRuby commented May 9, 2022

This PR gives no way to define custom segments - only retrieve them?

yes, it is true, i think the same, but i'm trying to make the smaller change for the framework, i can remove the extending with an macro for renew the cookie, without this change it is imposible because if there is more than 3 segments "remember me" stop working(count($segments) === 3 always must be 3)

protected function hasAllSegments()
{
$segments = explode('|', $this->recaller);
return count($segments) === 3 && trim($segments[0]) !== '' && trim($segments[1]) !== '';

@korkoshko
Copy link
Contributor

Hmm, maybe just define the count of segments as a constant or a property in Recaller? This allow to extend it and do something like:

class SomeRecaller extends Recaller
{
    public static $segments = 4;

    public function someId(): ?string
    {
        return explode('|', $this->recaller, static::$segments)[3] ?? null;
    }
}

@PaolaRuby
Copy link
Contributor Author

PaolaRuby commented May 9, 2022

@korkoshko yes, it is what i have now, but i'm trying to avoid all the extending recaller.php, sessionguard.php, adding a service provider for the new guard, and others changes with this only change, i tested changing vendor and works

@PaolaRuby
Copy link
Contributor Author

PaolaRuby commented May 9, 2022

@korkoshko you forget about overwriting also

protected function hasAllSegments()
{
$segments = explode('|', $this->recaller);
return count($segments) === 3 && trim($segments[0]) !== '' && trim($segments[1]) !== '';

count($segments) === 3 makes stop "remember me" on more than 4 segments
And
public function hash()
{
return explode('|', $this->recaller, 3)[2];
}

There is a static 3, that merge third segment with the fourth

@korkoshko
Copy link
Contributor

@PaolaRuby When the count of segments will be constant or a property in Recaller (parent class), we won't need to override this method.

 protected function hasAllSegments()
    {
        $segments = explode('|', $this->recaller);

        return count($segments) === static::$segments && trim($segments[0]) !== '' && trim($segments[1]) !== '';
    }

@angeljqv
Copy link
Contributor

angeljqv commented May 9, 2022

+1 for this

This PR gives no way to define custom segments

Would be great some changes on SessionGuard class like a segments resolver on login for the custom segments

@PaolaRuby
Copy link
Contributor Author

PaolaRuby commented May 9, 2022

public static $segments = 4;

@korkoshko maybe, still, i'm waiting for Taylor, maybe he close this, the whole "extending" is what I'm trying to avoid

@PaolaRuby
Copy link
Contributor Author

PaolaRuby commented May 9, 2022

@angeljqv @taylorotwell i can add a "session keys setter" on SessionGuard for define segments solution, like

protected static $recallCustomSegments = [];
public function setRecallCustomSegments(array $keys){
    self:$recallCustomSegments = $keys;
}
protected function queueRecallerCookie(AuthenticatableContract $user)
{
   $this->getCookieJar()->queue($this->createRecaller(
      $user->getAuthIdentifier().'|'.$user->getRememberToken().'|'.$user->getAuthPassword().$this->getCustomSegments()
   ));
}
protected function getCustomSegments()
{
     $segments = [];
     foreach (self::$recallCustomSegments as $segment) {
         $segments[$segment] = $this->session->get($segment, '');
     }
     return $segments ? '|'.implode('|', $segments) : '';
}
// on userFromRecaller
if ($user && $this->viaRemember) {
    $customSegments = $recaller->customSegments();
    foreach (self::$recallCustomSegments as $i => $segment) {
       $this->session->put([$segment => $customSegments[$i] ?? null]);
    }
}

now on auth service provider

auth()->setRecallCustomSegments(['company_id']);

Also i can add it from a config on config/auth.php

@korkoshko
Copy link
Contributor

@PaolaRuby btw, what if you need to change the company id for an already logged in user?

@PaolaRuby
Copy link
Contributor Author

PaolaRuby commented May 9, 2022

@korkoshko just refresh the cookie with the new id, look at this for example
remember-me-custom-expiration-to-your-laravel

auth()->getCookieJar()->queue(
   auth()->getCookieJar()->make(auth()->getRecallerName(), 'pipe_segments', auth()->getRememberDuration())
);

or with a macro, many ways

@taylorotwell taylorotwell merged commit 89d500f into laravel:9.x May 10, 2022
@taylorotwell
Copy link
Member

Refactored this to just a plain segments method - you can do whatever you want from there.

@PaolaRuby
Copy link
Contributor Author

@taylorotwell thank you very much,
that works the same for me

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.

How to add extra data on session guard recaller
4 participants