Skip to content

feat(auth): github OAuth provider option in login #986

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.

Sign up for GitHub

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
merged 5 commits into from
Jul 25, 2020
Merged

feat(auth): github OAuth provider option in login #986

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3db699c
test case to assert apple provider | created const to hold the lower …
davidepalazzo Jul 24, 2020
d296a4b
Added microsoft & apple unions to the Credentials type
davidepalazzo Jul 24, 2020
a4a696b
also added yahoo.com as auth provider
davidepalazzo Jul 24, 2020
72e0fe5
included apple & github to auth.spec
davidepalazzo Jul 24, 2020
21feb23
Edited auth doc to reflect the provider list
davidepalazzo Jul 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
test case to assert apple provider | created const to hold the lower … 
…case provider name | check for apple.com | pass down providerName to OAuthProvider when microsoft or apple | added apple to supportedAuthProviders
  • Loading branch information
@davidepalazzo
davidepalazzo committed Jul 24, 2020
commit 3db699cf2c0f9e7e01f5bec217da43df26bdabd1
3 changes: 2 additions & 1 deletion src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,8 @@ export const supportedAuthProviders = [
'github',
'twitter',
'facebook',
'microsoft.com'
'microsoft.com',
'apple.com'
]

/**
Expand Down
11 changes: 8 additions & 3 deletions src/utils/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,13 @@ function createAuthProvider(firebase, providerName, scopes) {
// TODO: Verify scopes are valid before adding
// TODO: Validate parameter inputs

if (providerName.toLowerCase() === 'microsoft.com') {
const provider = new firebase.auth.OAuthProvider('microsoft.com')
const lowerCaseProviderName = providerName.toLowerCase()

if (
lowerCaseProviderName === 'microsoft.com' ||
Copy link
Owner

@prescottprue prescottprue Jul 25, 2020

Choose a reason for hiding this comment

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

Most likely going to update this to be an array instead of multiple if conditions since that will also make it easy to add to the list:

['microsoft.com', 'apple.com', 'yahoo.com'].includes(lowerCaseProviderName)

lowerCaseProviderName === 'apple.com'
) {
const provider = new firebase.auth.OAuthProvider(providerName)
return provider
}

Expand All @@ -38,7 +43,7 @@ function createAuthProvider(firebase, providerName, scopes) {

// Handle providers without scopes
if (
providerName.toLowerCase() === 'twitter' ||
lowerCaseProviderName === 'twitter' ||
typeof provider.addScope !== 'function'
) {
return provider
Expand Down
6 changes: 6 additions & 0 deletions test/unit/utils/auth.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ describe('Utils: Auth', () => {
).to.include.keys('method')
})

it('apple.com provider', () => {
expect(
getLoginMethodAndParams(firebase, { provider: 'apple.com' })
).to.include.keys('method')
})

it('token', () => {
expect(
getLoginMethodAndParams(firebase, { token: 'asdf' })
Expand Down