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

Add session to endpoints #4068

Closed
RaeesBhatti opened this issue Feb 23, 2022 · 4 comments
Closed

Add session to endpoints #4068

RaeesBhatti opened this issue Feb 23, 2022 · 4 comments

Comments

@RaeesBhatti
Copy link

Describe the problem

Endpoints need to have access to user session to perform actions. Right now, you have to reimplement/import session management in endpoints.

Describe the proposed solution

Why not just expose session as part of Request to endpoints. We are already running getSession for pages. Let's run it for endpoints as well and have the results available in RequestEvent to endpoint handlers. e.g.

export const get: RequestHandler = async function({ url, session }) {
        if (!session.username) {
            status: 401,
            body: { error: "Unauthorized" }
        }
	return {
		body: { message: "ok" },
	};
};

Alternatives considered

No response

Importance

would make my life easier

Additional Information

No response

@PH4NTOMiki
Copy link
Contributor

Well, you have locals for that purpose, cause you're also probably using locals in the getSession too to return session info to the client.

@RaeesBhatti
Copy link
Author

RaeesBhatti commented Feb 24, 2022

I suppose locals could be used for session but having a dedicated getSession function and not passing that session information to endpoints is a missed opportunity. If you're supposed to have the session information figured out in locals during the handle hook, then it doesn't make sense to have a separate getSession function in my opinion.

I understand that a workaround is possible. But I'm advocating for a more explicit way.

@PH4NTOMiki
Copy link
Contributor

getSession gets run after handle hook, and getSession is mostly used to filter what's safe(from locals) to send the client as session, so for ex. you have locals.password
and then in getSession you do this:

export function getSession({ locals }){
  delete locals.password;
  return locals;
}

@Rich-Harris
Copy link
Member

I'm going to close this as wontfix because it's very intentional that session is meant to contain values that are safe to read in the client. By adding session to RequestEvent we make that less clear, and the danger is that people will get confused and start returning unsafe values from getSession.

As mentioned, it's typical for session to be a subset of event.locals — both of these are totally valid:

export const getSession = ({ locals }) => locals.session;
export function getSession({ locals }) {
  // locals.user contains secrets like 'token' or whatever
  return {
    user: locals.user && {
      name: locals.user.name,
      avatar: locals.user.avatar
    }
  };
}

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

3 participants