Skip to content

Commit

Permalink
feat(permissions): Restrict user and profile details based on allowed…
Browse files Browse the repository at this point in the history
… fields
  • Loading branch information
becevka authored and chungthuang committed Nov 12, 2024
1 parent 7041cf5 commit 75fd97c
Show file tree
Hide file tree
Showing 9 changed files with 196 additions and 12 deletions.
7 changes: 7 additions & 0 deletions config/middlewares.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,11 @@ module.exports = [
'strapi::favicon',
'strapi::public',
'plugin::users-permissions.jwt',
{
name: 'global::restrict-user-details',
config: {
allowedUserFields: ['id', 'username', 'profile'],
allowedProfileFields: ['id', 'profilePictureUrl', 'displayName'],
}
}
];
3 changes: 3 additions & 0 deletions init/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ const createUserWithAuthenticatedRole = async (roleId, user) => {
);
} else {
result = await strapi.plugins["users-permissions"].services.user.add(user);
if (user.profile) {
await strapi.service("api::profile.profile").create({data: {user: result.id, ...user.profile}});
}
}
return result.id;
};
Expand Down
14 changes: 13 additions & 1 deletion init/config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const { profile } = require("console");

module.exports = {
google: {
enabled: true,
Expand Down Expand Up @@ -25,6 +27,11 @@ module.exports = {
password: "k2OSo9Td",
confirmed: true,
blocked: null,
profile: {
displayName: "Tester One",
bio: "I am a tester 1",
profilePictureUrl: "https://i.imgur.com/123.jpg",
},
professionalRole: {
"category": "Development",
"name": "Fullstack Developer"
Expand All @@ -37,10 +44,15 @@ module.exports = {
password: "k2OSo9Td",
confirmed: true,
blocked: null,
profile: {
displayName: "Tester Two",
bio: "I am a tester 2",
profilePictureUrl: "https://i.imgur.com/124.jpg",
},
professionalRole: {
"category": "Product / UX",
"name": "UX Researcher"
}
},
},
interests: ["CSS", "HTML", "JavaScript", "Python"],
project: {
Expand Down
114 changes: 114 additions & 0 deletions src/middlewares/restrict-user-details.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/***
* Main algorithm here: take the response and introspect all attributes, if any attribute is a relation, we take that object and introspect its attributes.
* When relation points to a user or profile, we replace that object with shrunk version, including only allowed fields.
* Using private fields in Strapi Schema is not an option, as they are also not available for the user calling `/api/users/me` or `/api/users/:selfId`.
* The goal of this middleware is to restrict those fields for non-owner users.
*
* Metadata is not properly documented in Strapi.
* It is a Mapa Map-alike object where key is a modelName and value is MetaModel, which contains almost all the same info you can find under `content-types/schema.json`.
* See https://github.com/strapi/strapi/blob/develop/packages/core/database/src/types/index.ts#L216 and https://github.com/strapi/strapi/blob/develop/packages/core/database/src/metadata/metadata.ts
*/

module.exports = (config, { strapi }) => {
return async (ctx, next) => {
await next();
try {
// Only need to filter if there is a response body
if (ctx.request.method === "GET" && ctx.request.url.includes("/api/") && ctx.response.body) {
const handler = ctx.state.route.handler; // for example: "user.me" or "api::profile.profile.findOne"
const user = ctx.state.user;
const idx = handler.lastIndexOf(".");
const modelName = handler.substring(0, idx); // for example: "user" or "api::profile.profile"
const action = handler.substring(idx + 1); // for example: "me" or "findOne"
const metadata = strapi.db.metadata;

const fieldsRestricter = new FieldsRestricter(metadata, config);

ctx.response.body = fieldsRestricter.restrictFields(ctx.response.body, modelName, action, user);
}
} catch (error) {
console.error(error);
}
};
};

class FieldsRestricter {
constructor(metadata, config) {
this.metadata = metadata;
this.config = config;
}

findAndRestrictUserDetails(item, modelName) {
if (modelName === "user") {
return this.restrictNonWrappedItem(item, this.config.allowedUserFields);
}
if (modelName === "plugin::users-permissions.user") {
return this.restrictWrappedItem(item, this.config.allowedUserFields);
}
if (modelName === "api::profile.profile") {
return this.restrictWrappedItem(item, this.config.allowedProfileFields);
}
if (!this.metadata.has(modelName)) {
return item;
}
const model = this.metadata.get(modelName);
if (model && item?.data) {
if (Array.isArray(item.data)) {
item.data.forEach((element) => {
this.reviewModelAttributes(element.attributes, model);
});
} else {
this.reviewModelAttributes(item.data.attributes, model);
}
}
return item;
}

reviewModelAttributes(item, model) {
for (let attribute in model.attributes) {
const attributeData = model.attributes[attribute];
if (attributeData.type === "relation" && item[attribute]) {
item[attribute] = this.findAndRestrictUserDetails(item[attribute], attributeData.target);
}
}
}

restrictWrappedItem(item, allowedFields) {
const restrictedItem = { data: { id: item.data.id, attributes: {} } };
for (const field of allowedFields) {
if (field === "profile" && item.data.attributes[field]?.data) {
restrictedItem.data.attributes[field] = this.restrictWrappedItem(item.data.attributes[field], this.config.allowedProfileFields);
} else if (item.data.attributes[field]) {
restrictedItem.data.attributes[field] = item.data.attributes[field];
}
}
return restrictedItem;
}

restrictNonWrappedItem(item, allowedFields) {
const restrictedItem = {};
for (const field of allowedFields) {
if (field === "profile" && item[field]) {
restrictedItem[field] = this.restrictNonWrappedItem(item[field], this.config.allowedProfileFields);
} else if (item[field]) {
restrictedItem[field] = item[field];
}
}
return restrictedItem;
}

restrictFields(response, modelName, action, user) {
if (modelName === "user" && action === "me") {
return response;
}
if (modelName === "user" && action === 'findOne' && response?.id === user.id) {
return response;
}
if (modelName === "api::profile.profile" && action === 'findOne' && response?.data?.attributes?.user?.data?.id === user.id) {
return response;
}
return this.findAndRestrictUserDetails(response, modelName);
}
}


3 changes: 2 additions & 1 deletion tests/global-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { PostgreSqlContainer } from '@testcontainers/postgresql';
async function globalSetup() {
process.env.NODE_ENV = 'test';
process.env.FRONTEND_URL = 'not_used';
process.env.URL = 'http://localhost:1337';

if (process.env.TEST_CONTAINER === 'true') {
const container = await new PostgreSqlContainer().start();
Expand All @@ -20,7 +21,7 @@ async function globalSetup() {
global.strapiInstance = strapiInstance;

const context = await request.newContext({
baseURL: 'http://localhost:1337',
baseURL: process.env.URL,
});
const response = await context.post('/api/auth/local', {
data: {
Expand Down
19 changes: 19 additions & 0 deletions tests/idea-space.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,25 @@ test.describe('/api/comments', () => {

});

test.describe('user details access', () => {

test("should not allow access to most of user fields", async ({ request }) => {
const idea = await api(request).getData(`api/idea-cards/${ideaId}?populate[ideaOwner][populate]&populate[author][populate]&populate[comments][populate][user][populate]=profile`);
const commentUser = idea.attributes.comments.data[0].attributes.user.data.attributes;
expect(commentUser.username).toBe(user.username);
expect(commentUser.email).toBeUndefined();
expect(commentUser.job).toBeUndefined();
expect(commentUser.discordUsername).toBeUndefined();
const profile = commentUser.profile.data;
expect(profile.id).toBe(1);
expect(profile.attributes.profilePictureUrl).toBe(user.profile.profilePictureUrl);
expect(profile.attributes.displayName).toBe(user.profile.displayName);
expect(profile.attributes.bio).toBeUndefined();
});

});


/*
test.describe('/api/comments', () => {
Expand Down
16 changes: 8 additions & 8 deletions tests/notification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ test.describe('/api/notification', () => {
involveLevel: "minimum",
status: "workshopping",
});

// Capture the idea ID for this notification test
notificationIdeaId = newIdea.id;

Expand All @@ -30,8 +30,8 @@ test.describe('/api/notification', () => {

// Filter the notification related to the new idea
const notification = notifications.data.find(
(item) =>
item.attributes.event.data.attributes.entityId === newIdea.id &&
(item) =>
item.attributes.event.data.attributes.entityId === newIdea.id &&
item.attributes.event.data.attributes.entityType === "IdeaCard"
);

Expand All @@ -58,8 +58,8 @@ test.describe('/api/notification', () => {

// Filter the notification related to the comment using the comment ID
const notification = notifications.data.reverse().find(
(item) =>
item.attributes.event?.data?.attributes?.entityId === newCommentId &&
(item) =>
item.attributes.event?.data?.attributes?.entityId === newCommentId &&
item.attributes.event?.data?.attributes?.entityType === "Comment"
);

Expand Down Expand Up @@ -103,11 +103,11 @@ test.describe('/api/notification', () => {
user: myUser,
},
});

const notifications = await api(request).get("/api/notifications?populate=event,user");
const notification = notifications.data.find(
item => item.attributes.event.data.attributes.entityId === testId &&
item.attributes.event.data.attributes.entityType === "IdeaCard"
item.attributes.event.data.attributes.entityType === "IdeaCard"
);

// Check that the notification exists and has correct attributes
Expand Down Expand Up @@ -155,7 +155,7 @@ test.describe('/api/notification', () => {
const notification = notifications.data.find(
item => (item.attributes.event.data.attributes.entityId === testId && item.attributes.event.data.attributes.entityType === "IdeaCard"
));

expect(notification).toBeUndefined();

const error = await api(request).get(`/api/notifications/${newNotification.id}?populate=*`, 403);
Expand Down
22 changes: 22 additions & 0 deletions tests/profile.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { test, expect } from '@playwright/test';
import { api } from './utils';
import * as config from '../init/config';

test.describe('/api/profiles', () => {

test("should show profile by id if allowed", async ({ request }) => {
const profile = await api(request).get("/api/profiles/1?populate=*");
expect(profile.data.id).toBe(1);
expect(profile.data.attributes.displayName).toBe(config.user.profile.displayName);
expect(profile.data.attributes.profilePictureUrl).toBe(config.user.profile.profilePictureUrl);
expect(profile.data.attributes.bio).toBe(config.user.profile.bio);
});

test("should not show profile details id if other", async ({ request }) => {
const profile = await api(request).get("/api/profiles/2?populate=*");
expect(profile.data.id).toBe(2);
expect(profile.data.attributes.displayName).toBe(config.user2.profile.displayName);
expect(profile.data.attributes.profilePictureUrl).toBe(config.user2.profile.profilePictureUrl);
expect(profile.data.attributes.bio).toBeUndefined();
});
});
10 changes: 8 additions & 2 deletions tests/user.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ test.describe('/api/users', () => {
expect(user.confirmed).toBe(config.user.confirmed);
});

test.fixme("should not show user by id if other", async ({ request }) => {
await api(request).get("/api/users/2", 403);
test("should not show user details id if other", async ({ request }) => {
const user = await api(request).get("/api/users/2?populate=profile");
expect(user.username).toBe(config.user2.username);
expect(user.email).toBeUndefined();
expect(user.confirmed).toBeUndefined();
expect(user.profile.profilePictureUrl).toBe(config.user2.profile.profilePictureUrl);
expect(user.profile.displayName).toBe(config.user2.profile.displayName);
expect(user.profile.bio).toBeUndefined();
});
});

0 comments on commit 75fd97c

Please sign in to comment.