Skip to content

Commit

Permalink
Issue #3043: add support for setting the samesite flag for cookies
Browse files Browse the repository at this point in the history
for the OTOBO cookies. The default value is 'lax'.
But that can be changed with the SysConfig setting SessionSameSite.
The attribute is set automatically for all Cookies that are added
with the help of the method Kernel::Output::HTML::Layout::SetCookie().
  • Loading branch information
bschmalhofer committed Mar 1, 2024
1 parent 61444e0 commit 52aaee4
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 9 deletions.
11 changes: 11 additions & 0 deletions Kernel/Config/Files/XML/Framework.xml
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,17 @@
<Item ValueType="String" ValueRegex="^[0-9]{1,8}$">57600</Item>
</Value>
</Setting>
<Setting Name="SessionSameSite" Required="1" Valid="1">
<Description Translatable="1">Defines the value of the SameSite attribute of the OTOBO session cookies. Used in otobo.psgi.</Description>
<Navigation>Core::Session</Navigation>
<Value>
<Item ValueType="Select" SelectedID="Lax">
<Item ValueType="Option" Value="Strict" Translatable="1">Strict</Item>
<Item ValueType="Option" Value="Lax" Translatable="1">Lax</Item>
<Item ValueType="Option" Value="None" Translatable="1">None</Item>
</Item>
</Value>
</Setting>
<Setting Name="SessionMaxIdleTime" Required="1" Valid="1">
<Description Translatable="1">Sets the inactivity time (in seconds) to pass before a session is killed and a user is logged out.</Description>
<Navigation>Core::Session</Navigation>
Expand Down
28 changes: 21 additions & 7 deletions Kernel/Output/HTML/Layout.pm
Original file line number Diff line number Diff line change
Expand Up @@ -694,10 +694,10 @@ sub Redirect {
&& $ConfigObject->Get('SessionUseCookie')
)
{
for ( sort keys $Self->{SetCookies}->%* ) {
for my $Key ( sort keys $Self->{SetCookies}->%* ) {

# make a copy because we might need $Self->{SetCookies} later on
my %Ingredients = $Self->{SetCookies}->{$_}->%*;
my %Ingredients = $Self->{SetCookies}->{$Key}->%*;
my $Name = delete $Ingredients{name};

# the method 'cookies' is in lower case because we use Plack::Response directly
Expand Down Expand Up @@ -1707,10 +1707,10 @@ sub _AddHeadersToResponseObject {
&& $ConfigObject->Get('SessionUseCookie')
)
{
for ( sort keys $Self->{SetCookies}->%* ) {
for my $Key ( sort keys $Self->{SetCookies}->%* ) {

# make a copy because we might need $Self->{SetCookies} later on
my %Ingredients = $Self->{SetCookies}->{$_}->%*;
my %Ingredients = $Self->{SetCookies}->{$Key}->%*;
my $Name = delete $Ingredients{name};
$ResponseObject->Cookies->{$Name} = \%Ingredients;
}
Expand Down Expand Up @@ -4089,10 +4089,10 @@ sub CustomerLogin {
&& $ConfigObject->Get('SessionUseCookie')
)
{
for ( sort keys $Self->{SetCookies}->%* ) {
for my $Key ( sort keys $Self->{SetCookies}->%* ) {

# make a copy because we might need $Self->{SetCookies} later on
my %Ingredients = $Self->{SetCookies}->{$_}->%*;
my %Ingredients = $Self->{SetCookies}->{$Key}->%*;
my $Name = delete $Ingredients{name};
$ResponseObject->Cookies->{$Name} = \%Ingredients;
}
Expand Down Expand Up @@ -6684,11 +6684,25 @@ sub SetCookie {
}
}

# Get the configured samesite.
# Declare whethers browser should send the cookie to another domain.
# Other protocol counts as another domain.
# The default is 'lax'. There is no override via the parameter array.
my $SameSite;
{
my $ConfigObject = $Kernel::OM->Get('Kernel::Config');
$SameSite = lc $ConfigObject->Get('SessionSameSite') // '';
if ( $SameSite ne 'none' && $SameSite ne 'strict' ) {
$SameSite = 'lax';
}
}

$Self->{SetCookies}->{ $Param{Key} } = {
name => $Param{Key},
value => $Param{Value},
expires => $Param{Expires},
secure => $Param{Secure} || '',
secure => $Param{Secure} || '',
samesite => $SameSite,
httponly => $Param{HTTPOnly} || '',
path => '/' . ( $Param{Path} // '' ),
};
Expand Down
16 changes: 15 additions & 1 deletion Kernel/System/Web/Request.pm
Original file line number Diff line number Diff line change
Expand Up @@ -486,11 +486,25 @@ sub SetCookie {

$Param{Path} ||= '';

# Get the configured samesite.
# Declare whethers browser should send the cookie to another domain.
# Other protocol counts as another domain.
# The default is 'lax'. There is no override via the parameter array.
my $SameSite;
{
my $ConfigObject = $Kernel::OM->Get('Kernel::Config');
$SameSite = lc $ConfigObject->Get('SessionSameSite') // '';
if ( $SameSite ne 'none' && $SameSite ne 'strict' ) {
$SameSite = 'lax';
}
}

return {
name => $Param{Key},
value => $Param{Value},
expires => $Param{Expires},
secure => $Param{Secure} || '',
secure => $Param{Secure} || '',
samesite => $SameSite,
httponly => $Param{HTTPOnly} || '',
path => '/' . ( $Param{Path} // '' ),
};
Expand Down
2 changes: 1 addition & 1 deletion bin/psgi-bin/otobo.psgi
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ my $RedirectOtoboApp = sub {
$Res->redirect($Redirect);

# send the PSGI response arrayref
return $Res->finalize();
return $Res->finalize;
};

# Check whether PublicFrontend::Active is on. If so serve the public interface.
Expand Down

0 comments on commit 52aaee4

Please sign in to comment.