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

Make it possible to call save() in decryptSession #161

Merged
merged 4 commits into from
Sep 6, 2022
Merged

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Sep 6, 2022

It seems that in some cases the request is not decorated with the sessionStore. Instead we keep now a reference to the sessionStore directly and access the sessionStore instead of always accessing it through the request-Object.

Checklist

@SimenB
Copy link
Member

SimenB commented Sep 6, 2022

I'm down with the fastify.decorateRequest('sessionStore', { getter: () => sessionStore }) change, but I don't understand why it doesn't work. Can you add a test that fails without?


To me its seems the issue is that the request object is not the real one - it is accessed off of socket.request. Is that from fastify-socket.io? I have no idea, but to me the bug seems to be some other lib creating their own Request objects without copying decorated stuff properly.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 6, 2022

Hmm. let me check

@SimenB
Copy link
Member

SimenB commented Sep 6, 2022

https://github.com/Shrihari-Prakash/fastify-session-bug/blob/58c67e2c2fb4fc3a4ed2cb576aefbbc416954465/index.js#L85 isn't a real (fastify) Request - I don't think this is a bug in @fastify/session (unless it can be reproduced without the socket library)

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 6, 2022

Yeah, i agree with you.

But maybe it makes anyway sense to pass and keep a reference to the sessionStore and use it. Making it this way I have the feeling that we separate it more and request does not need to be a fastifyRequest anymore. I am ok with it that the request is not a fastify request anymore.

Will add now a unit test.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 6, 2022

@SimenB
It seems like we always made it possible to use a custom request object.

test('should decryptSession with custom request object', async (t) => {

But we never tried if we can actually save the session when we use decryptSession.

So extended the unit test.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 6, 2022

@SimenB
Can you make a review please?

@SimenB
Copy link
Member

SimenB commented Sep 6, 2022

should decryptSession with custom request object

Is this a use case we wanna support?

@Uzlopak Uzlopak changed the title Fix #159 Make it possible to call save() in decryptSession Sep 6, 2022
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 6, 2022

I went back to the tag of v6.0.0. And we...ll it seems like it never worked properly.

So this is actually fixing it.

I think there is no harm to make it possible.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 6, 2022

@zekth

It is not documented why decryptSession is deprecated. Only that you once requested to mark it as deprecated.

#53 (review)

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 6, 2022

@SimenB
Thanks

@Uzlopak Uzlopak merged commit a981573 into master Sep 6, 2022
@Uzlopak Uzlopak deleted the fix-159 branch September 6, 2022 14:27
bodinsamuel pushed a commit to specfy/specfy that referenced this pull request Aug 21, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@fastify/session](https://togithub.com/fastify/session) | [`10.2.0`
->
`10.4.0`](https://renovatebot.com/diffs/npm/@fastify%2fsession/10.2.0/10.4.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@fastify%2fsession/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@fastify%2fsession/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@fastify%2fsession/10.2.0/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@fastify%2fsession/10.2.0/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>fastify/session (@&#8203;fastify/session)</summary>

###
[`v10.4.0`](https://togithub.com/fastify/session/releases/tag/v10.4.0)

[Compare
Source](https://togithub.com/fastify/session/compare/v10.3.1...v10.4.0)

#### What's Changed

- fix: make sure to set cookie on manual session saves by
[@&#8203;SimenB](https://togithub.com/SimenB) in
[fastify/session#203

**Full Changelog**:
fastify/session@v10.3.1...v10.4.0

###
[`v10.3.1`](https://togithub.com/fastify/session/releases/tag/v10.3.1)

[Compare
Source](https://togithub.com/fastify/session/compare/v10.3.0...v10.3.1)

#### What's Changed

- ci: only trigger on pushes to main branches by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#198
- build(deps-dev): bump
[@&#8203;types/node](https://togithub.com/types/node) from 18.16.5 to
20.1.0 by [@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#199
- fix: manually persisting the session should make `isModified` return
false by [@&#8203;SimenB](https://togithub.com/SimenB) in
[fastify/session#201

**Full Changelog**:
fastify/session@v10.3.0...v10.3.1

###
[`v10.3.0`](https://togithub.com/fastify/session/releases/tag/v10.3.0)

[Compare
Source](https://togithub.com/fastify/session/compare/v10.2.1...v10.3.0)

#### What's Changed

- build(deps-dev): bump sinon from 13.0.2 to 14.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#88
- update to Fastify v4 by
[@&#8203;mcollina](https://togithub.com/mcollina) in
[fastify/session#89
- feat: Update benchmark. by
[@&#8203;ShogunPanda](https://togithub.com/ShogunPanda) in
[fastify/session#91
- ci: migrate to fastify reusable workflow by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#90
- build(deps-dev): bump fastify-cookie from 5.6.1 to 5.7.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#92
- docs(readme): remove snyk badge by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#93
- chore(.gitignore): use updated skeleton template by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#96
- build(deps-dev): bump
[@&#8203;types/node](https://togithub.com/types/node) from 17.0.45 to
18.0.0 by [@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#97
- build(deps-dev): bump tsd from 0.20.0 to 0.21.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#98
- build(deps-dev): bump tsd from 0.21.0 to 0.22.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#102
- style(types/types.test-d.ts): explicitly declare semicolon by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#103
- refactor: use optional chaining by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#104
- build(deps): bump fastify-plugin from 3.0.1 to 4.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#105
- remove metadata.js by [@&#8203;Uzlopak](https://togithub.com/Uzlopak)
in
[fastify/session#107
- separate lint from npm test script, activate lint and license-check in
ci pipeline by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#108
- refactor unit tests, replace ava with tap, remove undici and
typescript as devDependencies by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#109
- move benchmark to corresponding folder, slim it down by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#110
- tap should only run test.js files by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#115
- Clean up tests by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#117
- Add missing documentation for README.md by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#116
- Simplify shouldSaveSession code a little bit by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#119
- Move `expires` from session to session.cookies by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#120
- refactor store to es6 class by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#122
- simplify callback typing by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#124
- Add cookiePrefix as an option to allow for compatibility with
express-session by [@&#8203;rclmenezes](https://togithub.com/rclmenezes)
in
[fastify/session#113
- perf: improve idgenerator (about 25-30 % faster) by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#123
- chore: remove addDataToSession by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#127
- add autocannon example by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#128
- feat: use
[@&#8203;fastify/cookie-signer-logic](https://togithub.com/fastify/cookie-signer-logic)
for signing and unsigning by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#129
- move signer require into ensureDefaults by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#130
- fix unit test by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#132
- fix lgtm alert by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#133
- Remove session id round 4 by
[@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#134
- simplify getExpires, export getExpires from cookie by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#135
- remove internal used session restore() function by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#136
- Revert "refactor store to es6 class" by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#138
- remove sinon as dev dependency by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#139
- add redis example by [@&#8203;Uzlopak](https://togithub.com/Uzlopak)
in
[fastify/session#142
- add pre-commit by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#145
- add unit test for maxAge by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#144
- create a separate unit test file for memorystore by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#150
- rename secret.test.js to fastifySession.checkOptions.test.js and
refactor it by [@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#151
- prevent encryptedSessionId from being stored in the sessionStore by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#143
- use maxAge over expires by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#152
- build(deps-dev): bump tsd from 0.22.0 to 0.23.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#153
- Resave session cookie if saveUninitialized is false and rolling is
true by [@&#8203;rclmenezes](https://togithub.com/rclmenezes) in
[fastify/session#149
- rename session to fastifySession by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#160
- Make it possible to call save() in decryptSession by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#161
- Implement originalMaxAge by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#162
- Remove deprecation notice for fastify-Instance decryptSession by
[@&#8203;shrihari-prakash](https://togithub.com/shrihari-prakash) in
[fastify/session#164
- add typings for decryptSession by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#165
- simplify cookie handling by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#166
- minor cosmetic change by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#167
- ensure that secure false is not overwritten by null by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#168
- security: verify path of cookie/session by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#169
- build(deps-dev): bump tsd from 0.23.0 to 0.24.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#171
- Little remark fastify-cookie ->
[@&#8203;fastify/cookie](https://togithub.com/fastify/cookie) by
[@&#8203;Luchanso](https://togithub.com/Luchanso) in
[fastify/session#172
- fix rolling location in doc by
[@&#8203;HPieters](https://togithub.com/HPieters) in
[fastify/session#175
- fix: set domain when clearing cookie by
[@&#8203;TheWashiba](https://togithub.com/TheWashiba) in
[fastify/session#174
- fix: align cookie implementation with express-session by
[@&#8203;climba03003](https://togithub.com/climba03003) in
[fastify/session#177
- nodenext compatibility by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[fastify/session#178
- build(deps-dev): bump tsd from 0.24.1 to 0.25.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#179
- types: fix Session interface reference by
[@&#8203;FabianFrank](https://togithub.com/FabianFrank) in
[fastify/session#182
- chore(.gitignore): add clinic by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#183
- chore(.gitignore): add bun lockfile by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/session#185
- build(deps-dev): bump connect-redis from 6.1.3 to 7.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#186
- build(deps-dev): bump tsd from 0.25.0 to 0.27.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#187
- build(deps-dev): bump tsd from 0.27.0 to 0.28.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/session#188
- fix: express session type compatibility by
[@&#8203;ojeytonwilliams](https://togithub.com/ojeytonwilliams) in
[fastify/session#190
- feat: add ignoreFields option to regenerate method by
[@&#8203;RafaelGSS](https://togithub.com/RafaelGSS) in
[fastify/session#189
- fix: separate an actual path from url search part before verification
by [@&#8203;ohchi](https://togithub.com/ohchi) in
[fastify/session#192
- fix: lint by [@&#8203;Eomm](https://togithub.com/Eomm) in
[fastify/session#195
- feat!: stricter types by
[@&#8203;climba03003](https://togithub.com/climba03003) in
[fastify/session#196

#### New Contributors

- [@&#8203;mcollina](https://togithub.com/mcollina) made their first
contribution in
[fastify/session#89
- [@&#8203;ShogunPanda](https://togithub.com/ShogunPanda) made their
first contribution in
[fastify/session#91
- [@&#8203;Uzlopak](https://togithub.com/Uzlopak) made their first
contribution in
[fastify/session#107
- [@&#8203;rclmenezes](https://togithub.com/rclmenezes) made their first
contribution in
[fastify/session#117
- [@&#8203;shrihari-prakash](https://togithub.com/shrihari-prakash) made
their first contribution in
[fastify/session#164
- [@&#8203;Luchanso](https://togithub.com/Luchanso) made their first
contribution in
[fastify/session#172
- [@&#8203;HPieters](https://togithub.com/HPieters) made their first
contribution in
[fastify/session#175
- [@&#8203;TheWashiba](https://togithub.com/TheWashiba) made their first
contribution in
[fastify/session#174
- [@&#8203;FabianFrank](https://togithub.com/FabianFrank) made their
first contribution in
[fastify/session#182
- [@&#8203;ojeytonwilliams](https://togithub.com/ojeytonwilliams) made
their first contribution in
[fastify/session#190
- [@&#8203;RafaelGSS](https://togithub.com/RafaelGSS) made their first
contribution in
[fastify/session#189
- [@&#8203;ohchi](https://togithub.com/ohchi) made their first
contribution in
[fastify/session#192
- [@&#8203;Eomm](https://togithub.com/Eomm) made their first
contribution in
[fastify/session#195

**Full Changelog**:
fastify/session@v8.3.0...v10.3.0

###
[`v10.2.1`](https://togithub.com/fastify/session/releases/tag/v10.2.1)

[Compare
Source](https://togithub.com/fastify/session/compare/v10.2.0...v10.2.1)

#### What's Changed

- fix: separate an actual path from url search part before verification
by [@&#8203;ohchi](https://togithub.com/ohchi) in
[fastify/session#192
- fix: lint by [@&#8203;Eomm](https://togithub.com/Eomm) in
[fastify/session#195

#### New Contributors

- [@&#8203;ohchi](https://togithub.com/ohchi) made their first
contribution in
[fastify/session#192
- [@&#8203;Eomm](https://togithub.com/Eomm) made their first
contribution in
[fastify/session#195

**Full Changelog**:
fastify/session@v10.2.0...v10.2.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 10pm every weekday" in timezone
Europe/Paris, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/specfy/specfy).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi40My4yIiwidXBkYXRlZEluVmVyIjoiMzYuNDMuMiIsInRhcmdldEJyYW5jaCI6ImNob3JlL3Jlbm92YXRlQmFzZUJyYW5jaCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

None yet

2 participants