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

FEAT: Prevent self question #91

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

Conversation

yogiprsetya
Copy link
Contributor

Prevent self question

Description

We don't know what the free tier will be like in the future, it's better if we make DB writes more efficient.

@vercel
Copy link

vercel bot commented Oct 16, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @mazipan on Vercel.

@mazipan first needs to authorize it.

@yogiprsetya yogiprsetya marked this pull request as ready for review October 16, 2023 14:29
@mazipan mazipan changed the title Prevent self question FEAT: Prevent self question Oct 16, 2023
@@ -80,6 +84,15 @@ export function QuestionForm({ owner }: { owner: UserProfile }) {
if (process.env.NODE_ENV === 'development') {
await sendQuestion(owner?.slug || '', data.q, 'development')
} else {
if (isLogin && user?.email?.includes(owner.slug)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Hmmm, actually user can update the slug after first login.
So detecting by matching with the email is misslead.

Copy link
Owner

Choose a reason for hiding this comment

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

The correct one is by matching the uid.
But for the public page, I already strip the uid from the response for security purposes 😂

Copy link
Owner

Choose a reason for hiding this comment

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

This feature seems quite hard to solve for now.

Copy link
Owner

Choose a reason for hiding this comment

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

One approach that I can think is:

  • Passing authorization token, if user logged in
  • In the server API:
    • Get session by the token
    • Get uid from the session we get (if exist)
    • if the uid is the same with the owner of the question, you can reject it.

Seems complicated and adding at least 1 API call to the session DB

Choose a reason for hiding this comment

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

hi sir, how about comparing slug from browser URL and slug from account info?, we can do this on file src/modules/PublicQuestionPage/QuestionForm.tsx ?

when owner of the page click Kirim Pertanyaan button, just show a dialog to tell that owner of the page cannot ask to themself

Copy link
Owner

@mazipan mazipan Oct 22, 2023

Choose a reason for hiding this comment

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

Oh yes, it's possible I guess 🎉

Just can we also adding a way to submit question for our self?
In some cases, let say for testing, maybe we need that 🙈

For initial, can we enable the feature on the process.env.NODE_ENV === 'production' only? Or adding new env var NEXT_PUBLIC_ENABLE_SELF_SUBMISSSION

@vercel
Copy link

vercel bot commented Oct 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tanyaaja ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 16, 2023 10:07pm

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.

3 participants