jedireza / frame

:bulb: A user system API starter
https://jedireza.github.io/frame/
MIT License
739 stars 156 forks source link

Why others can delete sessions ?! #188

Closed iraniamir closed 6 years ago

iraniamir commented 6 years ago

Hi in this route everyone can send sessions to delete without checking the session is theirs or not !!

  server.route({
        method: 'DELETE',
        path: '/sessions/my/{id}',
        config: {
            auth: {
                strategy: 'simple'
            },
            pre: [{
                assign: 'current',
                method: function (request, reply) {

                    const currentSession = request.auth.credentials.session._id.toString();

                    if (currentSession === request.params.id) {

                        return reply(Boom.badRequest('Unable to close your current session. You can use logout instead.'));
                    }

                    reply(true);
                }
            }]
        },
        handler: function (request, reply) {

            const id = request.params.id;

            Session.findByIdAndDelete(id, (err, session) => {

                if (err) {
                    return reply(err);
                }

                if (!session) {
                    return reply(Boom.notFound('Document not found.'));
                }

                reply(session);
            });
        }
    });

i suggest replace it with this (to let user just can delete their sessions not for others too) :

    server.route({
        method: 'DELETE',
        path: '/sessions/my/{id}',
        config: {
            auth: {
                strategy: 'simple'
            },
            pre: [{
                assign: 'current',
                method: function (request, reply) {

                    const currentSession = request.auth.credentials.session._id.toString();

                    if (currentSession === request.params.id) {

                        return reply(Boom.badRequest('Unable to close your current session. You can use logout instead.'));
                    }

                    reply(true);
                }
            }]
        },
        handler: function (request, reply) {

            const id = request.params.id;
            const userId = request.auth.credentials.user._id.toString();

            const filter = {
                _id: new ObjectID(id),
                userId
            };

            Session.findOneAndDelete(filter, (err, session) => {

                if (err) {
                    return reply(err);
                }

                if (!session) {
                    return reply(Boom.notFound('Document not found.'));
                }

                reply(session);
            });
        }
    });
jedireza commented 6 years ago

Thanks. A PR is welcome.

gregfrasco commented 6 years ago

Was this fixed in #190? or is there another reason why it is still open?

jedireza commented 6 years ago

Thanks @gregfrasco