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

Clarify/refactor session handling #303

Closed
molomby opened this issue Sep 12, 2018 · 0 comments
Closed

Clarify/refactor session handling #303

molomby opened this issue Sep 12, 2018 · 0 comments

Comments

@molomby
Copy link
Member

molomby commented Sep 12, 2018

Based on some discussions (and a heap of assumptions on my part) I think there are a few problems with how we handling sessions internally. Specifically:

  • We currently add a user property to the express request object. This assumes/ignores the list key of the authenticated list.
  • We eagerly load the authenticated item on each request.
    • Often request won't need this at all (eg. if the request is reading from a list with a list-level access control, we only need the authenticated ID, which we have already). This these cases this is a completely unnecessary DB round trip.
    • Other times, there'll be something like an like item-level access control function, that does require the item (or, at least, KS doesn't know if it needs it or not). We can detect these case and lazy-load the data then. (This relates to ACL and hook stuff.)
  • When we load the auth'ed item (as above), we load the whole thing. This could also often be unnecessary. Imagine for example, user items that had a large JSON blob representing their profile info or activity history. This doesn't need to be loaded from the DB for every authenticated request.
    • Maybe we can specify a list of fields where the auth strategy is configured?

@jesstelford, @JedWatson: What are your thoughts on this? Grab me at HQ to discus?

@molomby molomby added this to the M1: Prototype milestone Sep 13, 2018
@MadeByMike MadeByMike modified the milestones: M1: Prototype, MVP Jul 23, 2019
@MadeByMike MadeByMike removed this from the Roadmap milestone Sep 19, 2019
@stale stale bot added the needs-review label Nov 18, 2019
@stale stale bot removed the needs-review label Jun 1, 2020
@keystonejs keystonejs deleted a comment from stale bot Jun 1, 2020
@bladey bladey closed this as completed Apr 8, 2021
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

No branches or pull requests

4 participants