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

Support requiring user to pass SMS verification to post #108

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

Conversation

PichuChen
Copy link
Contributor

@PichuChen PichuChen commented Apr 16, 2022

Requirement:

- 新增看板權限限制沒有進行 AOTP 者不得發文

Changes:

  1. Implement is_taiwan_phone() which determine if current user is registered phone number.
  2. Insert checking registered phone into const char * postperm_msg(const char *bname).
  3. Introduce a board configuration BRD_NEEDS_SMS_VERIFICATION (0x20000000) for this function.

Screenshots:
TBD, Still try to build without verifydb lib

Remark:

  • Please enable USE_SMS_VERIFICATION and flag USE_VERIFYDB when use this function.

TODO:

  • Review the post or comment checking flow and write the document.

@PichuChen PichuChen changed the title support ban outside Taiwan phone from SMS verification and verifydb Support ban outside Taiwan phone from SMS verification and verifydb Apr 17, 2022
@wens
Copy link
Contributor

wens commented Apr 23, 2022

This should simply be implemented as "require SMS verification to post".

include/pttstruct.h Outdated Show resolved Hide resolved
mbbsd/bbs.c Show resolved Hide resolved
mbbsd/cache.c Outdated Show resolved Hide resolved
@wens wens changed the title Support ban outside Taiwan phone from SMS verification and verifydb Support requiring user to pass SMS verification to post Apr 24, 2022
Copy link
Contributor

@IepIweidieng IepIweidieng left a comment

Choose a reason for hiding this comment

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

I performed manual spelling verification against the code changes and produced some manual reports.

mbbsd/bbs.c Outdated Show resolved Hide resolved
mbbsd/bbs.c Outdated Show resolved Hide resolved
mbbsd/bbs.c Outdated Show resolved Hide resolved
mbbsd/bbs.c Outdated Show resolved Hide resolved
mbbsd/bbs.c Outdated Show resolved Hide resolved
Copy link
Contributor

@hungte hungte left a comment

Choose a reason for hiding this comment

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

Please solve all the review comments

@wens
Copy link
Contributor

wens commented Jun 8, 2023

Rebase, not merge in master branch.

@PichuChen PichuChen force-pushed the pichu.taiwan_phone branch 2 times, most recently from fb63a83 to ab5a1a3 Compare June 8, 2023 13:12
@PichuChen PichuChen changed the title Support requiring user to pass SMS verification to post WIP: Support requiring user to pass SMS verification to post Jun 8, 2023
@PichuChen PichuChen changed the title WIP: Support requiring user to pass SMS verification to post Support requiring user to pass SMS verification to post Jun 8, 2023
@PichuChen PichuChen requested a review from hungte June 8, 2023 14:11
Copy link
Contributor

@IepIweidieng IepIweidieng left a comment

Choose a reason for hiding this comment

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

I found minor spelling issues.

/*
* Check if user phone number is verified by sms
*
* This function check if current user has already register their
Copy link
Contributor

@IepIweidieng IepIweidieng Jun 8, 2023

Choose a reason for hiding this comment

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

check (1st-person or plural or infinitive v.) → checks (3rd-person singular v.)

*
* Please note that, ptt system might have two different users with the same userid,
* but this won't happen at the same time. Therefore. we need to use firstlogin as the second
* parameter. And, ptt only accept phone registrations from Taiwan since July 2021
Copy link
Contributor

Choose a reason for hiding this comment

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

And (conj.) → Also (adv.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, accept (1st-person or plural or infinitive v.) → accepts (3rd-person singular v.).

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.

5 participants