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: Implement recaptcha verification #2349

Open
wants to merge 12 commits into
base: development
Choose a base branch
from
Open

feat: Implement recaptcha verification #2349

wants to merge 12 commits into from

Conversation

atm1504
Copy link
Member

@atm1504 atm1504 commented Aug 16, 2019

Fixes #2348 , #2319

Changes:

Screenshots for the change:
Will be updated once complete

@atm1504 atm1504 changed the title [WIP]: Implement recaptcha verification feat: Implement recaptcha verification Aug 19, 2019
@auto-label auto-label bot added the feature label Aug 19, 2019
@atm1504
Copy link
Member Author

atm1504 commented Aug 21, 2019

@iamareebjamal should we have this feature. This was implemented as discussed in the Gitter channel, regarding the changes in the server-side script.
But, @liveHarshit shared some comments fossasia/open-event-attendee-android#1943 (comment) where it was discussed that ReCaptcha won't be needed in the android clients. Can you please confirm what should I do?

@iamareebjamal
Copy link
Member

Discuss this with SUSI server guys

@atm1504
Copy link
Member Author

atm1504 commented Aug 22, 2019

Discuss this with SUSI server guys

We had a discussion regarding this in the gitter channel, they said to implement recaptcha.

@atm1504
Copy link
Member Author

atm1504 commented Aug 23, 2019

@iamareebjamal please review it. Also, please tell where to store the API key.

@ci-reporter
Copy link

ci-reporter bot commented Aug 24, 2019

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Circle CI build is failing as of 3ff7a72. Here's the output:

lint check
> Task :app:preBuild UP-TO-DATE

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!


Failed build for de75047
lint check
> Task :app:preBuild UP-TO-DATE
Failed build for 21ed178
lint check
> Task :app:preBuild UP-TO-DATE

This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

@ci-reporter
Copy link

ci-reporter bot commented Aug 24, 2019

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Circle CI build is failing as of 21ed178. Here's the output:

lint check
> Task :app:preBuild UP-TO-DATE

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!


This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

@mariobehling
Copy link
Member

Please fix Travis.

@mariobehling
Copy link
Member

Please provide a screenshot.

Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

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

Same comment as in the other PR: We cannot keep the API key in the app itself. This would be a security issue. It should be added during building of the app using Travis environment variables. Please implement it in that way.

@atm1504 atm1504 mentioned this pull request Sep 19, 2019
@batbrain7
Copy link
Member

@atm1504 please refer to the way youtube API key is stored in the App.

@batbrain7
Copy link
Member

This PR is open for a long time, if anyone wants to take this up, they can

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

Successfully merging this pull request may close these issues.

Add ReCaptcha for second login attempt
4 participants