lucia-auth / lucia

Authentication, simple and clean
https://lucia-auth.com
BSD Zero Clause License
9.51k stars 488 forks source link

[Bug]: Key getting deleted from mongo db once logged in #412

Closed dikshantrajput closed 1 year ago

dikshantrajput commented 1 year ago

Package

lucia-auth

Package version

0.9.0

Describe the bug

I created a user using auth.createUser and keys are successfully generated in my mongo db but when I logged into the account using auth.useKey() Post login, my key getting deleted from MongoDB keys collection. So I am not able to log in again using same user as key is not present and lucia throws error saving auth invalid key id

The expected behaviour is that the keys should never be deleted as it is a persistent type key.

Below are my code for creating user and useKey:

await auth.createUser({
    primaryKey: {
        providerId: 'email',
        providerUserId: email,
        password
    },
    attributes: {
        username,
        firstname: undefined,
        lastname: undefined,
        avatar: undefined,
        email: email.toLowerCase(),
        role: 'ADMIN',
        resetRequestedAt: undefined,
        resetToken: undefined
    }
});
await auth.useKey('email', email, password);

System info

I am using windows 11 with node version 16.15.0 with npm version 8.5.5 as package manager and vite version 4.1.4 as my dev server.

Also I am using these all packages for lucia auth:

"@lucia-auth/adapter-mongoose": "^0.6.0",
"@lucia-auth/oauth": "^0.7.0",
"@lucia-auth/sveltekit": "^0.6.10",
"lucia-auth": "^0.9.0",

Reproduction

No response

Relevant log output

No response

Additional information

While debugging, I had found the origin of bug inside lucia-auth.

The complete path to file is:

lucia-auth > auth > index.js > class Auth > useKey method > shouldDataBeDeleted function

const isPersistent = data.expires === null;

This line is causing the issue as my expires on key is undefined and you are just checking for null values.

Isn't it be checked for any undefined, null values? something like this?

const isPersistent = ! data.expires

pilcrowonpaper commented 1 year ago

I don't think the database should be returning undefined in the first place? @SkepticMystic

SkepticMystic commented 1 year ago

@dikshantrajput please share your mongoose model setup, and the auth export in the lucia.ts file

dikshantrajput commented 1 year ago

@SkepticMystic This is my lucia.ts file


export const auth = lucia({
    adapter: adapter(mongoose),

    //for production & cloned dev environment
    env: dev ? 'DEV' : 'PROD',
    autoDatabaseCleanup: true,
    transformUserData: (userData) => {
        return {
            userId: userData.id,
            email: userData.email,
            role: userData.role,
            username: userData.username,

            firstname: userData.firstname,
            lastname: userData.lastname,
            avatar: userData.avatar,

            resetRequestedAt: userData.resetRequestedAt,
            resetToken: userData.resetToken
        };
    }
});

export type Auth = typeof auth;

This is my mongoose model setup

//key model
import mongoose from 'mongoose';

const KeySchema = new mongoose.Schema(
    {
        _id: {
            type: String
        },
        user_id: {
            type: String,
            required: true
        },

        // Not strictly required by Lucia, but we'll be using it
        hashed_password: String,
        primary: {
            type: Boolean,
            required: true
        }
    },
    { _id: false }
);

export const Key = mongoose.models.key ?? mongoose.model('key', KeySchema);

//user schema
const UserSchema = new mongoose.Schema(
    {
        // Will be generated by Lucia
        _id: {
            type: String
        },

        // Other User Properties
        email: String,
        role: String,
        username: String,
        firstname: String,
        lastname: String,
        avatar: String,
        resetRequestedAt: String,
        resetToken: String,
        expiresAt: Date,
    },

    // Let Lucia handle the _id field
    { _id: false }
);
SkepticMystic commented 1 year ago

@pilcrowOnPaper my guess is, because there is no expires field on the keys (which is a valid option, right?), the doc returned from mongo doesn't have that field at all. So when isPersistent is defined, data.expires is undefined because it isn't there to begin with. Wouldn't it work to check null or undefined? Is it recommended to always pass an expires field, even if they never expire?

pilcrowonpaper commented 1 year ago

I never intended the expires column to be optional since it leads the bug like this. Would defining an expire column with a default value of null in the schema fix the issue?

Or rather this:

db.key.update({'expires': {$exists : false}}, {$set: {'expires': null}})
SkepticMystic commented 1 year ago

That would work, yes. So add a default: null value to the Key schema, and also update existing keys using the snippet you posted

dikshantrajput commented 1 year ago

@SkepticMystic Providing default value to key schema doesn't work

expires: {
    type: Number,
    default: null,
}

giving this error while creating user

image
SkepticMystic commented 1 year ago

@dikshantrajput you need to tell the Mongoose schema that null is an allowed value. It can be number or null. The easiest way to do that is change type to be Mongoose.Types.Mixed, or something like that. I'm on mobile, but the Mongoose docs will tell you how to make it a mixed type

dikshantrajput commented 1 year ago

Yup I figured it out. Sorry forgot to post it up here. Mb

pilcrowonpaper commented 1 year ago

Sorry for the lack of guide on this, added this to the migration guide for the upcoming release!