Skip to content

users-email - Lowercase the email value on CRUD operations for deta and USERS kv #49

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.

Sign up for GitHub

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

Open
ashishjullia wants to merge 1 commit into
base: main
Choose a base branch
from
Open

users-email - Lowercase the email value on CRUD operations for deta and USERS kv #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions handlers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ module.exports.addUserToTeam = async ({
throw new HTTPError('Invalid access: team admin required', 403)
}

let kvUser = await USERS.get(userEmail, { type: 'json' })
let kvUser = await USERS.get(userEmail.toLowerCase(), { type: 'json' })

@woozyking woozyking Jul 20, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would ignore issues retrospectively, such that if the KV already has the user's email in different cases, USERS.get will have a miss

@ashishjullia ashishjullia Jul 20, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it will but that can be solved by manually making the ones already in kv USERS to lowercase and in deta collection users?

@woozyking woozyking Jul 21, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you resolve this issue in a backward compatible way, without the batch update in the existing deta base?

@ashishjullia ashishjullia Jul 21, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's what I meant to solve this a bulk update is required to the existing USERS and users.

@ashishjullia ashishjullia Jul 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But there is one catch here, a bulk update to lowercase the email can be done but if there is a case where there are two entries for (say) ashishjullia19@gmail.com and ASHISHJULLIA19@GMAIL.COM, then if both have a few teams/stages/key:value(s) then this process can lead to a failure in such cases where either nit-pick the ones that can be discarded from the db means whether to keep ashishjullia19@gmail.com or ASHISHJULLIA19@GMAIL.COM (other making the email lowercase).


if (!kvUser) {
kvUser = await createUser(userEmail, { getKVUser: true })
Expand Down Expand Up @@ -452,7 +452,7 @@ module.exports.removeUserFromTeam = async ({
throw new HTTPError('Invalid access: team admin required', 403)
}

const kvUser = await USERS.get(userEmail, { type: 'json' })
const kvUser = await USERS.get(userEmail.toLowerCase(), { type: 'json' })

if (!kvUser) {
throw new HTTPError('Invalid user: user not found', 400)
Expand Down Expand Up @@ -483,7 +483,7 @@ module.exports.addUserToAdmin = async ({
throw new HTTPError('Invalid access: team admin required', 403)
}

const kvUser = await USERS.get(userEmail, { type: 'json' })
const kvUser = await USERS.get(userEmail.toLowerCase(), { type: 'json' })

if (!kvUser) {
throw new HTTPError('Invalid user: user not found', 400)
Expand Down Expand Up @@ -514,7 +514,7 @@ module.exports.removeUserFromAdmin = async ({
throw new HTTPError('Invalid access: team admin required', 403)
}

const kvUser = await USERS.get(userEmail, { type: 'json' })
const kvUser = await USERS.get(userEmail.toLowerCase(), { type: 'json' })

if (!kvUser) {
throw new HTTPError('Invalid user: user not found', 400)
Expand Down
34 changes: 18 additions & 16 deletions modules/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ const deta = require('./db')

// deta Base - users, for UI use
// TODO: deprecated in favor of getUser below
module.exports.fetchUser = (email) =>
module.exports.fetchUser = (email) =>
deta
.Base('users')
.fetch({ email }, { limit: 1 })
.fetch({ email: email.toLowerCase() }, { limit: 1 })
.then(({ items = [] }) => items[0] || null)


module.exports.getUser = deta.Base('users').get

module.exports.listTeamUsers = ({ team }) =>
Expand All @@ -19,7 +20,7 @@ module.exports.listTeamUsers = ({ team }) =>

module.exports.createUser = async (email, { getKVUser = false } = {}) => {
const user = {
email,
email: email.toLowerCase(),
jwt_uuid: uuidv4(),
otp_secret: uuidv4(),
teams: [],
Expand All @@ -32,10 +33,11 @@ module.exports.createUser = async (email, { getKVUser = false } = {}) => {
throw new Error('jwt_uuid and otp_secret must be different')
}
// TODO: do this "transactionally"
const dbUser = await deta.Base('users').insert(user, email) // email as key
const dbUser = await deta.Base('users').insert(user, email.toLowerCase()) // email as key
// remove deta exclusive fields (such as otp_secret)
const kvUser = { ...dbUser }
delete kvUser.otp_secret
// No need to lowercase the email here as it is already there when user is constructed
await USERS.put(user.email, JSON.stringify(kvUser))

return getKVUser ? kvUser : dbUser
Expand All @@ -51,7 +53,7 @@ module.exports.updateUser = (user) => {
return Promise.all([
// TODO: do this "transactionally"
deta.Base('users').put(user), // deta.Base put(data)
USERS.put(user.email, JSON.stringify(kvUser)), // KV put(key, value)
USERS.put(user.email.toLowerCase(), JSON.stringify(kvUser)), // KV put(key, value)
])
}

Expand All @@ -62,9 +64,9 @@ module.exports.addUserToTeam = ({ user, team }) => {
// TODO: make this "transactional"
// TODO: unify with other user updates e.g. ./user.update
return Promise.all([
users.update({ teams: users.util.append(team) }, user.key),
users.update({ teams: users.util.append(team) }, user.key.toLowerCase()),
USERS.put(
user.email,
user.email.toLowerCase(),
JSON.stringify({
...user,
teams: [...user.teams, team],
Expand All @@ -82,9 +84,9 @@ module.exports.removeUserFromTeam = ({ user, team }) => {
// TODO: make this "transactional"
// TODO: unify with other user updates e.g. ./user.update
return Promise.all([
users.update({ teams, admins }, user.key),
users.update({ teams, admins }, user.key.toLowerCase()),
USERS.put(
user.email,
user.email.toLowerCase(),
JSON.stringify({
...user,
teams,
Expand All @@ -102,9 +104,9 @@ module.exports.addUserToAdmin = ({ user, team }) => {
// TODO: make this "transactional"
// TODO: unify with other user updates e.g. ./user.update
return Promise.all([
users.update({ admins: users.util.append(team) }, user.key),
users.update({ admins: users.util.append(team) }, user.key.toLowerCase()),
USERS.put(
user.email,
user.email.toLowerCase(),
JSON.stringify({
...user,
admins: [...user.admins, team],
Expand All @@ -122,9 +124,9 @@ module.exports.removeUserFromAdmin = ({ user, team }) => {
// TODO: make this "transactional"
// TODO: unify with other user updates e.g. ./user.update
return Promise.all([
users.update({ admins }, user.key),
users.update({ admins }, user.key.toLowerCase()),
USERS.put(
user.email,
user.email.toLowerCase(),
JSON.stringify({
...user,
admins,
Expand All @@ -145,10 +147,10 @@ module.exports.deleteUser = (user) => {
// TODO: what if they're the only admin for a team? Should make someone else admin!
return Promise.all([
// TODO: do this "transactionally"
deta.Base('users').delete(user.key),
USERS.delete(user.email),
deta.Base('users').delete(user.key.toLowerCase()),
USERS.delete(user.email.toLowerCase()),
])
}

// Cloudflare Workers KV - USERS, for CLI use
module.exports.getKVUser = (email) => USERS.get(email, { type: 'json' })
module.exports.getKVUser = (email) => USERS.get(email.toLowerCase(), { type: 'json' })