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

replacing duo with google auth #3

Merged
8 commits merged into from
Sep 4, 2016
Merged

replacing duo with google auth #3

8 commits merged into from
Sep 4, 2016

Conversation

ghost
Copy link

@ghost ghost commented Aug 31, 2016

@captn3m0 @venkatvghub

I haven't removed any Duo (old auth) code. Will do once the new Google Auth works perfectly on production.

Also have created a duo_master branch for duo code backup.

* Google
*/
'Google' => array(
'client_id' => '',
Copy link
Contributor

Choose a reason for hiding this comment

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

Pick these from environment variables

@captn3m0
Copy link
Contributor

@rishabhp Is there any way we can get list of google groups in the oauth scope? We'd want to limit app access to a specific google group. (say devs@razorpay).

Alternatively, if that is not possible, we can drop the user-creation from here and only allow users with existing emails in the database to login.

That way, registration is slow (you need to ask an admin), but logins are still fast and we get to limit access.

@ghost
Copy link
Author

ghost commented Aug 31, 2016

@captn3m0 I'll read up on their oauth scopes entirely and let you know.

// - Belong to razorpay.com domain
//
// Then only we'll create a user entry in the system or check for one
if (!$result->verified_email || explode('@', $result->email)[1] !== $_ENV['company_domain']) return App::abort(404);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is buggy. Email can have more than 1 @. Although gmail currently doesn't allow @ in the email address but since it's a third party service we shouldn't rely on them.

@ghost ghost merged commit 0c9ad59 into master Sep 4, 2016
This pull request was closed.
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