Skip to content

Fixes #42 - Add Google External Auth provider #69

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

Merged

Conversation

mtargonski
Copy link
Contributor

No description provided.

…m 2 places now (registration + external auth provider). Replaced EmailTempllates to extension methods on emailmessagedto. Extracted startup methods into submethods grouping them
@mtargonski
Copy link
Contributor Author

PR for issue #42

@mtargonski mtargonski changed the title Issues/42 external providers for id s4 Add Google External Auth provider Nov 30, 2019
@mtargonski mtargonski requested a review from enkodellc November 30, 2019 21:05
@mtargonski mtargonski marked this pull request as ready for review November 30, 2019 21:05
@mtargonski
Copy link
Contributor Author

mtargonski commented Nov 30, 2019

Fixes #42 when merged

@mtargonski mtargonski added the 🔍 Ready for Review Pull Request is not reviewed yet label Dec 3, 2019
@mtargonski mtargonski changed the title Add Google External Auth provider Fixes #42 - Add Google External Auth provider Dec 3, 2019
@enkodellc
Copy link
Owner

Ha, just when I got your code merged and I want to sign up with google to get my API key I get an error from Google: Couldn't initialize warning A network error occurred and the request could not be completed. I will try later.

@enkodellc
Copy link
Owner

Well I got it running locally and tested quite a bit. Then I went to publish to test live and I got an error:
Assets file 'E:\Websites\BlazorBoilerplate\src\BlazorBoilerplate.Server\obj\project.assets.json' doesn't have a target for '.NETCoreApp,Version=v3.0'

Which led me here:
aspnet/AspNetKatana#251
https://docs.microsoft.com/en-us/dotnet/core/compatibility/2.2-3.0

I am done for the night, Might try and submit a PR for your PR...

@enkodellc
Copy link
Owner

@Acid12 I merged some changes. Everything looked pretty solid.

  • I noticed that some of my test google users had issues with invalid username's because it was using my Google First Name and Last Name with a space in it so I needed to do some additional changes for those test cases.
  • I do like separating out some of the startup functionality. It was easier for this PR to keep it together for the merge. Let's do that in a separate PR?
  • I agree we would not need the Confirm Email for External Auth.
  • I could not publish still but will try a few different things today. Did you try publishing your PR?
  • I wanted to publish so I could create my own Google API and test on my demo / live site to see if I could get rid of the success page and make some changes to the error page.

Hopefully I can get it to publish as we have both have time into this PR and I think it is a good example. Maybe if we cannot we do Twitter Login instead of Google.

@enkodellc
Copy link
Owner

Well I was able to get it published but then got an "undefined 'map'" javascript error after putting on the server. Will continue to debug.

@enkodellc enkodellc merged commit 055d579 into enkodellc:master Dec 6, 2019
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Merged Pull Request has been merged successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants