ספריית Sequelize: איך למנוע שליחת שדות מסויימות לצד לקוח

yossiz

תיאור הבעיה

על פי רוב בממשקי API, האובייקטים שה-API שולח משקפים במידה רבה את המבנה של ה-DB. עד כדי כך, שלרוב התשובה של ה-API הוא פשוט ייצוג JSON של שורות ב-DB.
מתכנתים עצלנים כמו רוב עמך (כולל אני) בד"כ פשוט עונים לבקשות API עם תבנית זו:

שאילתת DB לקבל את הישויות הרלוונטיות
הרצת לוגיקה על היישויות, כמו בדיקת הרשאות, או עדכון נתונים וכו'
סיריאליזציה של ישות ה-DB שרלוונטי לתשובה ל-JSON, ושליחה לקליינט

הבעיה היא שקורה הרבה שהשדות שנצרכים עבור שלב 2 שונים מהשדות שרוצים לשלוח בשלב 3

למשל בבקשת login בד"כ שולפים שורה שמייצגת משתמש, ועבור שלב 2 רוצים לבדוק שהסיסמה נכונה, ולכן שולפים את השדה שמייצג את ההאש של הסיסמה, אבל בשלב 3 בד"כ לא רוצים לשלוח את זה חזרה לקליינט

עוד דוגמה: אפליקציה ששומרת מפתח API של המשתמש כדי להריץ עבורו פעולות מול צד שלישי, רוצה לשלוף את המידע הזה מה-DB עבור שלב 2 כדי לטפל בבקשות שונות, אבל לא רוצה לשלוח את זה חזרה לקליינט

וכהנה רבות

מצו"ב קוד קצר לטיפול נוח בנושא זה עבור משתמשי Sequelize

הסבר על הפתרון

בספריית Sequelize, כאשר הופכים מודל ל-JSON, מנוע JS משתמש מתחת למכסה בפונקציית Sequelize.Model.prototype.toJSON
הרעיון הוא שנוסיף שדה סטטית להגדרת המודלים שלנו שמכיל מערך של שדות שצריך תמיד להשמיט מהסיריאליזציה, ונכתוב toJSON מותאם אישית שלנו שיכבד רשימה זו
כדי שזה יעבוד גם על מודלים מקוננים, נקרא לפונקצייה זו בצורה ריקורסיבית על מודלים מקוננים

הנה הקוד

// Override the builtin `toJSON` method to allow hiding certain fields from the client
// To hide a field, add it to the `hidden` array on the model's prototype
// We recursively call `toJSON` on all included models
Sequelize.Model.prototype.toJSON = function () {
  const includes = this._options.includeNames ?? [];
  const hiddenKeys = this.hidden ?? [];
  const values = Object.assign({}, this.get());
  for (const hiddenKey of hiddenKeys) {
    delete values[hiddenKey];
  }
  for (const include of includes) {
    if (Array.isArray(values[include])) {
      values[include] = values[include].map((value) => value.toJSON());
    } else if (typeof values[include].toJSON === 'function') {
      values[include] = values[include].toJSON();
    }
  }
  return values;
};

והנה הגדרת מודל לדוגמה:

const User = sequelize.define(
  'User',
  {
    firstName: {
      type: DataTypes.STRING,
      allowNull: false,
    },
    lastName: {
      type: DataTypes.STRING,
    },
    passwordHash: {
      type: DataTypes.STRING,
    }
  });

User.prototype.hidden = ['passwordHash']

או אם אתה מעדיף קלאסים:

class User extends Model {
  static hidden = ['passwordHash']
}

User.init(
  {
    firstName: {
      type: DataTypes.STRING,
      allowNull: false,
    },
    lastName: {
      type: DataTypes.STRING,
    },
    passwordHash: {
      type: DataTypes.STRING,
    }
  },
  {
    sequelize,
    modelName: 'User',
  },
);

נ.ב. הקוד נכתב עבור sequelize v6. אני משתמש בשדה פנימית של Model (ה-_options.includeNames). אין הבטחה שהקוד ימשיך לעבוד בגירסאות הבאות של sequelize

עוד הערה:
אני לא חושב שקוד זה מספיק טוב לאפליקציה רצינית
נראה לי שאפליקציה רצינית אמורה להפריד בין הייצוג הפנימי לאובייקטים החיצוניים שנשלחים לקליינט, ולהשתמש בשכבה שיודע להמיר בין אובייקטים פנימיים לאובקטים "חיצוניים"
בפרוייקט אחד שלי השתמשתי ב-https://github.com/typestack/class-transformer לצורך זה

dovid

לאובייקט שחושפים ללקוח קוראים DTO או ViewModel.
בC# מתמודדים עם זה על ידי אטריביוטים ש"מסתירים" מאפיינים נבחרים, או מחלקה נפרדת עם מילוי על ידי Auto Mapper.
במקרים רבים שליחה עיוורת של האובייקט ממסד הנתונים חושפת דברים שקשה לחשוב עליהם, כמו למשל כמות הלקוחות לפי הID רץ וכולי.

yossiz

@dovid כתב בספריית Sequelize: איך למנוע שליחת שדות מסויימות לצד לקוח:

על ידי Auto Mapper

אתה מתכוון לזה?

dovid

שאלה דומה בסטאק:
https://stackoverflow.com/questions/27972271/sequelize-dont-return-password
אני חושב שזו דוגמה טובה לבלגן ה"איך" ששורר בnodejs.
בעוד בC# יש עומס מרגיז של שיטות וכללי עצוב, בנוד חסר מאוד כאלו.
אגב הרבה לא שמים לב למשהו שיותר מסוכן - הכיוון ההפוך של קבלה מלקוח,
כשמקבלים מהלקוח אובייקט יש סכנה יותר הפוכה שנקראת Mass Assignment או Overposting שזה מתי שהקוד בצד שרת מקבל אובייקט ולמשל מעדכן על פיו את מסד הנתונים בדינמיות, אך בעוד לקליינט מותר לערוך רק את כתובותו האישית, הוא מוסיף שדות שהוא בכלל לא אמור להיות יכול לשנות (את שמם הוא מנחש או יודע דרך הבעיה של @yossiz).
בexpress למשל, מצוי שרואים קוד בסגנון הזה:

app.post('/update', async (req, res) => {
   if(req.body.id != req.seesion.user.id) return res.status(403);
   await connection.query('UPDATE users SET ? WHERE id = ?',  [req.body, req.body.id]);
   ...
});

הקוד הזה שהוא ממש קסום מבחינת פשטותו, הוא פרצת אבטחה בה משתמש יכול לשנות כל שדה, כולל את הID (ל1 למשל ואז הוא בטוח מנהל...)....
ראיתי מלא קוד דומה בפרודקשיין! יש פה טענה קצת על ספריית mysql שעשתה חיים מידי יפים בלי שום התרעה או אזהרה בתיעוד, אבל בnosql המצב הרבה יותר חמור.
בנוסף, בגלל שאין טיפוסיות קשוחה בJS המשתמש יכול לנצל את זה ולשלוח טיפוס שמצד אחד ייתמך במסד נתונים כדי שלא תהיה שגיאה בהכנסה או העדכון, אבל יהיו לכך השלכות לא טובות לגבי הלוגיקה בהמשך. אפילו מחלקה בנויה יפה בtypeScript לא תעזור פה שהרי בזמן ריצה הוא JS והוא מסכים הכל.
יש ספריות לזה, כמו express-validator אבל מי משתמש בה ביחס לכלל? זה אחת הבעיות של הטכנולוגיות המדליקות והחופשיות, אין שמה נורמות בסיסיות מרוב שרוצים לתת את ההגה למפתח.
מאמרים על Mass Assignment בנוד:
https://snyk.io/blog/avoiding-mass-assignment-node-js/
https://knowledge-base.secureflag.com/vulnerabilities/inadequate_input_validation/mass_assignment_nodejs.html

avi rz

@dovid כתב בספריית Sequelize: איך למנוע שליחת שדות מסויימות לצד לקוח:

כמו למשל כמות הלקוחות לפי הID רץ וכולי.

נו ו.. לכן לא לשלוח id איזה תחליף יכול להיות לו?
אם בתכלת כל הזיהוי של הלקוח הוא לפי ה id כי כל דבר יכול להתחלף (טלפון, מייל, ת.ז. וכו') או להיות כפול (במקרה שאין לי בעיה שיהיו 2 חשבונות ללקוח מסוים)
אז פשוט ליצור עוד id לייצר מחרוזת סתם? זה מכביד על הניהול.
ובפרט מי שמשתמש ב sql זה id רץ וכל ה joinים עושים לפי זה.

dovid

@avi-rz לא אמרתי לא לשלוח ID, אמרתי לשים לב מתי לשלוח אותו.
יש המון ישויות שהצד לקוח לא מעדכן או שגם אם הוא מעדכן לא צריך בשביל זה ID ישיר.
לפעמים חייבים לעשות מה שאמרת ולהסתיר את הID על ידי עמודה נוספת של זיהוי GUID (פעם כתבתי על פתרונות אחרים https://tchumim.com/topic/13821/תרגיל-מתמטי-של-הסתרת-מזהה-רץ)

ivrtikshoret

@dovid כתב בספריית Sequelize: איך למנוע שליחת שדות מסויימות לצד לקוח:

app.post('/update', async (req, res) => {
if(req.body.id != req.seesion.user.id) return res.status(403);
await connection.query('UPDATE users SET ? WHERE id = ?', [req.body, req.body.id]);
...
});

לא הבנתי למה זה פירצת אבטחה אתה הרי בודק לפני כן אם זה שווה ל seesion אז איך הלקוח יכול לשנות id?

yossiz

@ivrtikshoret טכנית אתה צודק לגבי הקוד המדוייק ש@dovid הביא. אבל הנקודה הכללית עדיין עומדת וקיימת. כלומר ששם אי אפשר לשנות את שדה id ספציפית כי יש בדיקה לזה. גם בלי הבדיקה, הרי הרשומה שמעדכנים היא הרשומה עם ה-id ששלחת ב-body, אז תמיד זה יהיה זהה. אבל עדיין ייתכן שיש שדות רגישות אחרות. כמו"כ בהרבה מקרים פרמטר id שמצביע על המזהה של הרשומה שצריך לעדכן נשלח בנפרד לאובייקט העדכון ואז הבדיקה לא תועיל.

dovid

@ivrtikshoret באמת שגיתי בכתיבה והתכוונתי למשפט האחרון של @yossiz,
אבל גם במקרה הזה הוא התוקף יכול לשלוח שני פרמטרים, אחד id בקטן תקין עם המזהה הנכון שלו, והשני ID עם המספר 1, הSQL לא רגיש לרישיות.
דוקא השאלה שלך מחדדת איך המתכנת יכול לפספס OVERPOSTING.

ivrtikshoret

@dovid בזה אתה צודק מאה אחוז,
מה שאני עושה בד"כ, אני לא מעביר את כל ה body, אלא שולף מה body רק את מה שאני רוצה שיוכלו לשלוח ל db

dovid

@dovid כתב בספריית Sequelize: איך למנוע שליחת שדות מסויימות לצד לקוח:

אבל גם במקרה הזה הוא התוקף יכול לשלוח שני פרמטרים, אחד id בקטן תקין עם המזהה הנכון שלו, והשני ID עם המספר 1, הSQL לא רגיש לרישיות.

@yossiz העיר לי באישי שמשפט הsql שיווצר ייראה כזה:

SET ID = 1, id = 565, ...

בעוד בsql server זה מוציא שגיאה של עמודה כפולה, בmysql זה עובר בשקט - ההשמה האחרונה מנצחת. אז התוקף צריך לסדר נכון את האובייקט...

תחומים - פורום חרדי מקצועי

ספריית Sequelize: איך למנוע שליחת שדות מסויימות לצד לקוח

תיאור הבעיה

הסבר על הפתרון

הנה הקוד