From 4ccfd298273ca8c6c64a24355e5e8873ac36b95e Mon Sep 17 00:00:00 2001 From: Harry Mellor <19981378+HMellor@users.noreply.github.com> Date: Sat, 22 Apr 2023 23:41:18 +0100 Subject: [PATCH] Simplify Firestore and improve permissions (#24) --- README.md | 83 ++++++++++++++++------------------ js/auctions.js | 118 ++++++++++++++++++++----------------------------- js/popups.js | 45 +++++++------------ 3 files changed, 102 insertions(+), 144 deletions(-) diff --git a/README.md b/README.md index a7f97b1e..38904edc 100644 --- a/README.md +++ b/README.md @@ -64,48 +64,45 @@ Head to your project's console and click on Authentication in the menu on the le Setting up the database is a little more involved so here are the steps you must take: - Head to your project's console and click on Database in the menu on the left. Then click on `Create database` (the mode you start in does not matter because we are about to set proper rules anyway). - Then chose which region you want your Firestore to be stored (check the server locations [here](https://firebase.google.com/docs/firestore/locations) if there are multiple in your region). -- Head to the `Rules` tab and paste the rules below. -``` -rules_version = '2'; -service cloud.firestore { - match /databases/{database}/documents { - function isAdmin() { - return get(/databases/$(database)/documents/users/$(request.auth.uid)).data.admin == "insert long random secret string" - } - function isDocumentOverWrite() { - return request.resource.data.keys().hasOnly(resource.data.keys()) - } - function isFieldOverWrite() { - return request.resource.data[request.resource.data.keys()[0]].keys().hasOnly(resource.data[request.resource.data.keys()[0]].keys()) - } - function isLoggedIn() { - return exists(/databases/$(database)/documents/users/$(request.auth.uid)) - } - // Make sure the uid of the requesting user matches name of the user - // document. The wildcard expression {userId} makes the userId variable - // available in rules. - match /users/{userId} { - allow read, update, delete: if request.auth != null && request.auth.uid == userId; - allow create: if request.auth != null; - } - match /auction-live/{items} { - allow get, list: if true; - allow create, delete: if isAdmin(); - allow update: if isAdmin() || isLoggedIn(); //&& !isFieldOverWrite(); - } - match /auction-store/{item} { - allow get, list: if false; - allow create, delete: if isAdmin(); - allow update: if isAdmin() || isLoggedIn() && !isDocumentOverWrite(); +- Head to the `Rules` tab and paste the following rules: + ``` + rules_version = '2'; + service cloud.firestore { + match /databases/{database}/documents { + // Checks that new data doesn't overwrite or delete an existing bid + function isFieldOverWrite() { + let editedKeys = request.resource.data.diff(resource.data); + return editedKeys.changedKeys().union(editedKeys.removedKeys()).size() > 0 + } + // Checks user has anonymous account and has "signed up" (i.e. provided a displayName) + function isLoggedIn() { + return request.auth != null && exists(/databases/$(database)/documents/users/$(request.auth.uid)) + } + // Checks the user is logged in and if their user data contains the admin password + function isAdmin() { + return isLoggedIn() && get(/databases/$(database)/documents/users/$(request.auth.uid)).data.admin == "insert long random secret string" + } + // Make sure the uid of the requesting user matches name of the user + // document. The wildcard expression {userId} makes the userId variable + // available in rules. + match /users/{userId} { + allow read, update, delete: if isAdmin() || request.auth != null && request.auth.uid == userId; + allow create: if request.auth != null && request.auth.uid == userId; + } + // Auction can always be read, updated only if the user is logged in and + // isn't overwiting someone else's bid, and created or deleted by admins + match /auction/items { + allow get, list: if true; + allow update : if isAdmin() || isLoggedIn() && !isFieldOverWrite() + allow create, delete: if isAdmin(); + } } } -} -``` + ``` - These rules state that: - - Users (admin or otherwise) can only create user documents, ensuring that no one but you can see the users names (a privacy measure). - - The auction live document may be read by anyone and only updated if the user is logged in (or if the user is an admin). This document is what your clients will fetch the current state of the auction from. Therefore no real names are stored here, only the bid amount and the user's UID (which is randomly generated by Firebase and is completely non-identifying to any prying eyes). - - The auction store documents may not be read by anyone and only be written to if the user is logged in and the requested write would not overwrite any previous bids (or if the user is an admin). These documents serve as both a backup copy of the auction that cannot be meddled with and a directory of bid information containing user names for your eyes only. -- You may notice that the `isFieldOverWrite()` call for the `auction-live` rule is commented out. This is because it only works for a single auction and thus, in its current state, would be fatal to implement in the auction. If you are able to make it work correctly I would appreciate it if you let me know your solution either in a pull request or in a comment on the [relevant issue](https://github.com/HMellor/auction-website/issues/5). + - Users can only read and write to their own user data, keeping usernames private. + - The auction document may be read by anyone and only updated if the user is logged in and is not modifying or deleting existing bids. This document is what your clients will fetch the current state of the auction from. No usernames are stored here, only the bid amount and the user's UID (which is randomly generated by Firebase and is completely non-identifying to any prying eyes). + - Admins can access all auction and user data. ### Creating an admin account and initialising your auctions The final step in setting up your auction is to create an admin account and use it to initialise your auctions. @@ -119,9 +116,7 @@ To create an admin account: To initialise the auctions: - With the device you used to create your admin account, head to your website. -- Open the developer console (F12) and type the following into the console at the bottom: -``` -resetAll() -``` -- This will wipe all documents in the `auction-live` and `auction-store` collections and create auctions with the titles, info and reserve prices you defined earlier (as long as you are admin). +- Open the developer console (F12) and enter `resetAll()` into the console. +- This will revert the entire auction to the initial state specified in `js/firebase.js` (as long as you are admin), be careful with this one! +- You can also reset individual items using the `resetItem(i)` function. - Your auction is now ready. diff --git a/js/auctions.js b/js/auctions.js index 9fa862f4..e48afb55 100644 --- a/js/auctions.js +++ b/js/auctions.js @@ -1,6 +1,6 @@ // Imports import { auth, db } from "./firebase.js"; -import { doc, setDoc, updateDoc, writeBatch, onSnapshot } from "https://www.gstatic.com/firebasejs/9.20.0/firebase-firestore.js"; +import { doc, setDoc, getDoc, updateDoc, deleteField, onSnapshot } from "https://www.gstatic.com/firebasejs/9.20.0/firebase-firestore.js"; // For a real auction, set this to false let demoAuction = true; @@ -14,10 +14,10 @@ function generateRandomAuctionData() { "https://random-data-api.com/api/name/random_name", { size: auctions.length }, (data) => { - data.forEach((elem, i) => { + data.forEach((elem, i) => { cards[i].querySelector(".title").innerText = elem.name cards[i].dataset.bsTitle = elem.name - }); + }); } ); // Random lorem ipsum cat descriptions @@ -72,8 +72,7 @@ export function setClocks() { if (demoAuction) { auctions[i].endTime = new Date(auctions[i].endTime).setDate(now.getDate() + 1) // add 1 day document.getElementById("auction-" + i).parentElement.remove() - resetLive(i); - resetStore(i); + resetItem(i); auctionGrid = document.getElementById("auction-grid"); auctionCard = generateAuctionCard(i); auctionGrid.appendChild(auctionCard); @@ -101,10 +100,10 @@ function generateAuctionCard(auction) { let card = document.createElement("div"); card.classList.add("card"); - card.dataset.bsTitle=auction.title - card.dataset.bsDetail=auction.detail - card.dataset.bsPrimaryImage=auction.primaryImage - card.dataset.bsSecondaryImage=auction.secondaryImage + card.dataset.bsTitle = auction.title + card.dataset.bsDetail = auction.detail + card.dataset.bsPrimaryImage = auction.primaryImage + card.dataset.bsSecondaryImage = auction.secondaryImage card.id = "auction-" + auction.idx col.appendChild(card); @@ -168,8 +167,8 @@ function generateAuctionCard(auction) { let infoButton = document.createElement("button"); infoButton.type = "button" infoButton.classList.add("btn", "btn-secondary") - infoButton.dataset.bsToggle="modal" - infoButton.dataset.bsTarget="#info-modal" + infoButton.dataset.bsToggle = "modal" + infoButton.dataset.bsTarget = "#info-modal" infoButton.innerText = "Info"; buttonGroup.appendChild(infoButton); @@ -177,8 +176,8 @@ function generateAuctionCard(auction) { bidButton.type = "button" bidButton.classList.add("btn", "btn-primary") bidButton.innerText = "Submit bid"; - bidButton.dataset.bsToggle="modal" - bidButton.dataset.bsTarget="#bid-modal" + bidButton.dataset.bsToggle = "modal" + bidButton.dataset.bsTarget = "#bid-modal" buttonGroup.appendChild(bidButton); return col @@ -201,15 +200,21 @@ function numberWithCommas(x) { export function dataListener() { // Listen for updates in active auctions - onSnapshot(doc(db, "auction-live", "items"), (doc) => { - console.log("Database read from dataListener()") - let data = doc.data() - for (let key in data) { - let cb = document.getElementById("current-bid-" + Number(key)) - let bids = data[key] + onSnapshot(doc(db, "auction", "items"), (doc) => { + console.debug("dataListener() read from auction/items") + // Parse flat document data into structured Object + let data = {} + for (const [key, details] of Object.entries(doc.data())) { + let [item, bid] = key.split("_").map(i => Number(i.match(/\d+/))) + data[item] = data[item] || {} + data[item][bid] = details.amount + } + // Use structured Object to populate the "Current bid" for each item + for (const [item, bids] of Object.entries(data)) { + let cb = document.getElementById(`current-bid-${item}`) // Extract bid data - let bidCount = (Object.keys(bids).length - 1) / 2 - let currPound = Number.parseFloat(bids["bid" + bidCount]).toFixed(2) + let bidCount = Object.keys(bids).length - 1 + let currPound = bids[bidCount].toFixed(2) // Check if the user is winning if (auth.currentUser) { let userWinning = bids["bid" + bidCount + "-user"] == auth.currentUser.uid @@ -220,60 +225,33 @@ export function dataListener() { }) } -function resetLive(i) { - const docRef = doc(db, "auction-live", "items"); - let itemId = i.toString().padStart(5, "0") - updateDoc(docRef, { - [itemId]: { - bid0: auctions[i].startingPrice, - } - }) - console.log("Database write from resetLive()") -} - -function resetAllLive() { - console.log("Resetting live tracker") - for (let i = 0; i < auctions.length; i++) { - resetLive(i); - } -} - -function resetStore(i) { - let itemId = i.toString().padStart(5, "0") - const docRef = doc(db, "auction-store", itemId); - setDoc(docRef, { - bid0: { - bidder: String(i), - amount: auctions[i].startingPrice, - time: Date().substring(0, 24) - } +function resetItem(i) { + const docRef = doc(db, "auction", "items") + const itemId = `item${i.toString().padStart(5, "0")}` + // Find all bids for item i + let initialState = {} + getDoc(docRef).then((doc) => { + console.debug("resetItem() read from auction/items") + let keys = Object.keys(doc.data()).sort() + keys.filter((key) => key.includes(itemId)).forEach((key, idx) => { + // Mark all except bid00000 to be deleted + initialState[key] = idx ? deleteField() : { amount: auctions[i].startingPrice } + }) + }).then(() => { + updateDoc(docRef, initialState) + console.debug("resetItem() write to from auction/items") }) - console.log("Database write from resetStore()") } -function resetAllStore() { - console.log("Resetting auction storage") - const batch = writeBatch(db); +function resetAll() { + let initialState = {} for (let i = 0; i < auctions.length; i++) { - let itemId = i.toString().padStart(5, "0") - let currentItem = doc(db, "auction-store", itemId); - batch.set(currentItem, { - bid0: { - bidder: String(i), - amount: auctions[i].startingPrice, - time: Date().substring(0, 24) - } - }) + let field = `item${i.toString().padStart(5, "0")}_bid00000` + initialState[field] = { amount: auctions[i].startingPrice } } - batch.commit() - console.log(auctions.length + " database writes from resetAllStore()") -} - -function resetAll() { - resetAllLive(); - resetAllStore(); + setDoc(doc(db, "auction", "items"), initialState) + console.debug("resetAll() write to auction/items") } +window.resetItem = resetItem window.resetAll = resetAll -window.resetAllLive = resetAllLive -window.resetAllStore = resetAllStore diff --git a/js/popups.js b/js/popups.js index 74daacd8..92e21107 100644 --- a/js/popups.js +++ b/js/popups.js @@ -49,6 +49,7 @@ function signUp() { let user = auth.currentUser; updateProfile(user, { displayName: username.value }) setDoc(doc(db, "users", user.uid), { name: username.value, admin: false }) + console.debug("signUp() write to users/${auth.currentUser.uid}") authButton.innerText = "Sign out" document.getElementById('username-display').innerText = "Hi " + username.value username.classList.add("is-valid") @@ -77,9 +78,9 @@ bidModal.addEventListener("show.bs.modal", (event) => { // Focus the amount input once bidModal is visible bidModal.addEventListener('shown.bs.modal', () => { // If not logged in, open signUpModal instead - if (authButton.innerText == "Sign in") { + if (authButton.innerText == "Sign in") { bidModalObject.hide() - signUpModalObject.show() + signUpModalObject.show() } else { bidModalInput.focus() } @@ -124,36 +125,20 @@ function placeBid() { amountElement.classList.add("is-invalid") bidModalSubmit.removeAttribute('disabled', ''); } else { - // Checking bid amount - // Get item and user info - let user = auth.currentUser; - let itemId = i.toString().padStart(5, "0") - // Documents to check and write to - const liveRef = doc(db, "auction-live", "items"); - const storeRef = doc(db, "auction-store", itemId); - // Check live document - getDoc(liveRef).then(function (doc) { - console.log("Database read from placeBid()") - let thisItem = doc.data()[itemId]; - let bids = (Object.keys(thisItem).length - 1) / 2 - let currentBid = thisItem["bid" + bids] + // Check auction database + let docRef = doc(db, "auction", "items"); + getDoc(docRef).then(function (doc) { + console.debug("placeBid() read from auction/items") + let data = doc.data() + let itemId = `item${i.toString().padStart(5, "0")}` + let bids = Object.keys(data).filter((key) => key.includes(itemId)) + let bidId = `bid${(bids.length).toString().padStart(5, "0")}` + let currentBid = data[bids[bids.length - 1]].amount if (amount >= 1 + currentBid) { - let keyStem = itemId + ".bid" + (bids + 1) - updateDoc(liveRef, { - [keyStem + "-uid"]: user.uid, - [keyStem]: amount, - }) - console.log("Database write from placeBid()") - let storeKey = "bid" + (bids + 1) - updateDoc(storeRef, { - [storeKey]: { - "bidder-username": user.displayName, - "bidder-uid": user.uid, - "amount": amount, - time: Date().substring(0, 24) - } + updateDoc(docRef, { + [`${itemId}_${bidId}`]: { amount: amount, uid: auth.currentUser.uid }, }) - console.log("Database write from placeBid()") + console.debug("placeBid() write to auction/items") amountElement.classList.add("is-valid") amountElement.classList.remove("is-invalid") setTimeout(() => {