Skip to content
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.

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

prevent not-owned slugs to be open from domain #18

Closed
wants to merge 1 commit into from

Conversation

f
Copy link

@f f commented Jun 8, 2020

No description provided.

Copy link
Owner

@stephenou stephenou left a comment

Choose a reason for hiding this comment

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

Hey @f (nice username btw),

Thanks for sharing this on Reddit and putting up a PR!

Just looked at the code and left an inline comment.


// Prevent other users' pages to be loaded
if (url.pathname.includes("loadPageChunk")) {
if (!body.includes(NOTION_MAIL)) {
Copy link
Owner

Choose a reason for hiding this comment

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

this is an interesting approach! i like it.

i want to make this more granular though.

loadPageChunk returns several maps: block, space, notion_user, collection_view, and collection. so it's not guaranteed that if the email exists in the response text, the page is created by the user of that email.

but certainly this is on the right track though. i think the correct solution may be (await response.json).recordMap.notion_user.email !== MY_EMAIL. i will look further into this and report back :)

Copy link

Choose a reason for hiding this comment

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

recordMap can have multiple users with different id's (If you duplicate a page template, that's the situation). On a different scenario, I might have a multiple user Notion page and might have no contribution on some of the pages (hence my email won't even exist on those pages). I'll send my solution to this.

Copy link
Author

Choose a reason for hiding this comment

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

this is an interesting approach! i like it.

i want to make this more granular though.

loadPageChunk returns several maps: block, space, notion_user, collection_view, and collection. so it's not guaranteed that if the email exists in the response text, the page is created by the user of that email.

but certainly this is on the right track though. i think the correct solution may be (await response.json).recordMap.notion_user.email !== MY_EMAIL. i will look further into this and report back :)

I thought parsing JSON would cost a bit, so I just checked plaintext. But this can be improved. I'll think on another solution.

Copy link

Choose a reason for hiding this comment

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

A bit convoluted, but I've been using the code below, where NOTION_EMAILS is an array of emails that can have pages that the custom domain can map to.

const data = JSON.parse(body)
if (!NOTION_EMAILS.includes(data.recordMap.notion_user[Object.keys(data.recordMap.notion_user)[0]].value.email)) {
    throw new Error('page not owned by correct user')
}

Copy link

Choose a reason for hiding this comment

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

Fyi, as of notion api v3, what I provided no longer works as the recordMap returned by /loadPageChunk no longer contains the notion_user field

Copy link
Owner

Choose a reason for hiding this comment

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

yeah, unfortunately this doesn't work anymore because the space map is no longer in recordMap in the response of loadPageChunk.

would appreciate it if anyone has a better idea in terms of how to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants