Skip to content

Test Authentication against FireBase#28

Closed
awinabi wants to merge 1 commit intofreesendmails:masterfrom
awinabi:authentication-specs
Closed

Test Authentication against FireBase#28
awinabi wants to merge 1 commit intofreesendmails:masterfrom
awinabi:authentication-specs

Conversation

@awinabi
Copy link
Contributor

@awinabi awinabi commented Oct 17, 2017

This is the sample code that contains specs that I'm planning to implement for the issues #11, #12, #13, #14. We can use https://github.com/vcr/vcr for canned responses from FireBase to test the various authentication flows.

# refer  spec/controllers/v1/authenticated_controller_spec.rb 

context 'when the to email is invalid' do
  it 'shows json error response'
end

context 'when the to email is valid' do
  it 'checks in FireBase for user existance'
  it 'redirects to authentication success page if user exists in FireBase'
  it 'adds the user to FireBase if user does not exist'
end

@grassiricardo Does this sound good?

* Use VCR responses from Firebase for authentication flows
@awinabi awinabi changed the title Test Authentication against FireBase - sample code Test Authentication against FireBase Oct 18, 2017
@awinabi
Copy link
Contributor Author

awinabi commented Oct 18, 2017

This is complete and ready for review. Thanks!

@grassiricardo
Copy link
Member

@diogopms Could you review this?


def firebase_find_users(email)
if firebase_users && firebase_users.body.present?
firebase_users.body.values.select {|u| u['email'] == email}

Choose a reason for hiding this comment

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

we can use firebase_user_emails function here and then filter email. Additional check can be prevented

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prashuchaudhary I want the whole user JSON here, because I need to compare various other fields in the User hash in the specs. Using firebase_user_emails will give me just the email.

Copy link
Contributor

@diogopms diogopms left a comment

Choose a reason for hiding this comment

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

Can you rebase this code?

@awinabi
Copy link
Contributor Author

awinabi commented Oct 16, 2019

@diogopms Have not looked at this for a while now. Let me rebase and check if the specs are still passing.

@diogopms
Copy link
Contributor

👍 Great

@awinabi
Copy link
Contributor Author

awinabi commented Oct 25, 2019

@diogopms Since I deleted my fork of the repo, I have created a new pull request #41 by applying the same diff.

There are some comments by the sourcelevel-bot which I will look into and possibly fix.

Closing this pull request. Superseded by #41

@awinabi awinabi closed this Oct 25, 2019
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