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

[Fix] oauth not working because of email array #9173

Merged
merged 3 commits into from
Dec 19, 2017

Conversation

geekgonecrazy
Copy link
Contributor

@RocketChat/core

Pretty much no oauth integration supports doing something like emails[0].address to grab the email address out of the /api/v1/me response. So grabbing the email and surfacing as a property

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-9173 December 18, 2017 22:47 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-9173 December 18, 2017 22:49 Inactive
@graywolf336 graywolf336 added this to the 0.60.0 milestone Dec 19, 2017
@graywolf336 graywolf336 merged commit 80c367b into develop Dec 19, 2017
@graywolf336 graywolf336 deleted the fix/single-email-expected-for-oauth branch December 19, 2017 05:53
@sampaiodiego
Copy link
Member

I would suggest to at least get a verified email from array

@geekgonecrazy
Copy link
Contributor Author

geekgonecrazy commented Dec 19, 2017

@sampaiodiego yeah I was wondering about that... I thought about doing: me.emails.find((email) => return email.verified)

Seems like we shouldn't let people oauth that don't have verified email. But at this stage it wouldn't stop that. Because it just continues on with out email right now.

@sampaiodiego
Copy link
Member

Seems like we shouldn't let people oauth that don't have verified email. But at this stage it wouldn't stop that. Because it just continues on with out email right now.

I think this is the correct behavior.. we should not prevent an user to OAuth if it has no verified email but we should not return an unverified email as well.

@geekgonecrazy
Copy link
Contributor Author

Yeah that makes sense. We should only bubble up verified email Opened #9183

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.

4 participants