Skip to content

Regenerate session ID on login#7829

Merged
Alkarex merged 6 commits intoFreshRSS:edgefrom
Inverle:regenerate-id-on-login
Aug 30, 2025
Merged

Regenerate session ID on login#7829
Alkarex merged 6 commits intoFreshRSS:edgefrom
Inverle:regenerate-id-on-login

Conversation

@Inverle
Copy link
Member

@Inverle Inverle commented Aug 15, 2025

Follow-up to #7762

@Inverle Inverle marked this pull request as ready for review August 15, 2025 17:34
@Inverle Inverle marked this pull request as draft August 15, 2025 17:45
@Inverle Inverle marked this pull request as ready for review August 15, 2025 18:20
@Inverle
Copy link
Member Author

Inverle commented Aug 15, 2025

works properly now

return;
}
$lifetime = session_get_cookie_params()['lifetime'];
setcookie('FreshRSS', $newId, $lifetime, self::getCookieDir(), '', Minz_Request::isHttps(), true);
Copy link
Member

@Alkarex Alkarex Aug 16, 2025

Choose a reason for hiding this comment

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

Please pass 'FreshRSS' as parameter $name to keep the same convention as the related functions just below

Copy link
Member Author

Choose a reason for hiding this comment

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

Made some other changes too: a8dcc22

@Alkarex
Copy link
Member

Alkarex commented Aug 18, 2025

Can it wait till after the release of 1.27.0?

@Alkarex Alkarex added this to the 1.28.0 milestone Aug 18, 2025
@Inverle
Copy link
Member Author

Inverle commented Aug 18, 2025

Probably

@Alkarex Alkarex merged commit 200eafb into FreshRSS:edge Aug 30, 2025
1 check passed
@Inverle Inverle deleted the regenerate-id-on-login branch August 30, 2025 19:40
@Inverle
Copy link
Member Author

Inverle commented Aug 30, 2025

Forgot to also apply this when creating user or deleting account

Not critical but will make a PR later
(because giveAccess() checks if passwordHash is the same in the session and for bcrypt it's always different so no risk of session fixation here - assumption from earlier testing)

@Alkarex Alkarex modified the milestones: 1.28.0, 1.27.1 Sep 2, 2025
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.

2 participants