-
Notifications
You must be signed in to change notification settings - Fork 391
feat(auth): Add ability to link a federated ID with the updateUser()
method.
#770
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
Changes from all commits
59d93aa
c79f42d
dfa1fce
fe88272
a04e175
3393af3
ba5361f
a3f8f69
af5d012
37bd4e3
676b4a1
2167091
726cb85
7686113
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -403,6 +403,8 @@ function validateCreateEditRequest(request: any, writeOperationType: WriteOperat | |
phoneNumber: true, | ||
customAttributes: true, | ||
validSince: true, | ||
// Pass linkProviderUserInfo only for updates (i.e. not for uploads.) | ||
linkProviderUserInfo: !uploadAccountRequest, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add deleteProvider as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's already present (line 263) since it was used previously to remove phone auth entries. |
||
// Pass tenantId only for uploadAccount requests. | ||
tenantId: uploadAccountRequest, | ||
passwordHash: uploadAccountRequest, | ||
|
@@ -551,6 +553,12 @@ function validateCreateEditRequest(request: any, writeOperationType: WriteOperat | |
validateProviderUserInfo(providerUserInfoEntry); | ||
}); | ||
} | ||
|
||
// linkProviderUserInfo must be a (single) UserProvider value. | ||
if (typeof request.linkProviderUserInfo !== 'undefined') { | ||
validateProviderUserInfo(request.linkProviderUserInfo); | ||
} | ||
|
||
// mfaInfo is used for importUsers. | ||
// mfa.enrollments is used for setAccountInfo. | ||
// enrollments has to be an array of valid AuthFactorInfo requests. | ||
|
@@ -1306,6 +1314,33 @@ export abstract class AbstractAuthRequestHandler { | |
'Properties argument must be a non-null object.', | ||
), | ||
); | ||
} else if (validator.isNonNullObject(properties.providerToLink)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't the validations at validateCreateEditRequest sufficient? Seems like the validation is added in two places. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Missed this comment, sorry!) IIUC, the parameters are slightly different, so it doesn't quite work. (eg rawId). Some refactoring could be done though... I've added a TODO. |
||
// TODO(rsgowman): These checks overlap somewhat with | ||
// validateProviderUserInfo. It may be possible to refactor a bit. | ||
if (!validator.isNonEmptyString(properties.providerToLink.providerId)) { | ||
throw new FirebaseAuthError( | ||
AuthClientErrorCode.INVALID_ARGUMENT, | ||
'providerToLink.providerId of properties argument must be a non-empty string.'); | ||
} | ||
if (!validator.isNonEmptyString(properties.providerToLink.uid)) { | ||
throw new FirebaseAuthError( | ||
AuthClientErrorCode.INVALID_ARGUMENT, | ||
'providerToLink.uid of properties argument must be a non-empty string.'); | ||
} | ||
} else if (typeof properties.providersToUnlink !== 'undefined') { | ||
if (!validator.isArray(properties.providersToUnlink)) { | ||
throw new FirebaseAuthError( | ||
AuthClientErrorCode.INVALID_ARGUMENT, | ||
'providersToUnlink of properties argument must be an array of strings.'); | ||
} | ||
|
||
properties.providersToUnlink.forEach((providerId) => { | ||
if (!validator.isNonEmptyString(providerId)) { | ||
throw new FirebaseAuthError( | ||
AuthClientErrorCode.INVALID_ARGUMENT, | ||
'providersToUnlink of properties argument must be an array of strings.'); | ||
} | ||
}); | ||
} | ||
|
||
// Build the setAccountInfo request. | ||
|
@@ -1340,13 +1375,25 @@ export abstract class AbstractAuthRequestHandler { | |
// It will be removed from the backend request and an additional parameter | ||
// deleteProvider: ['phone'] with an array of providerIds (phone in this case), | ||
// will be passed. | ||
// Currently this applies to phone provider only. | ||
if (request.phoneNumber === null) { | ||
request.deleteProvider = ['phone']; | ||
request.deleteProvider ? request.deleteProvider.push('phone') : request.deleteProvider = ['phone']; | ||
delete request.phoneNumber; | ||
} else { | ||
// Doesn't apply to other providers in admin SDK. | ||
delete request.deleteProvider; | ||
} | ||
|
||
if (typeof(request.providerToLink) !== 'undefined') { | ||
request.linkProviderUserInfo = deepCopy(request.providerToLink); | ||
delete request.providerToLink; | ||
|
||
request.linkProviderUserInfo.rawId = request.linkProviderUserInfo.uid; | ||
delete request.linkProviderUserInfo.uid; | ||
} | ||
|
||
if (typeof(request.providersToUnlink) !== 'undefined') { | ||
if (!validator.isArray(request.deleteProvider)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this check redundant, considering the condition gets checked at validation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will achieve the same result without the if condition I think:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The user could set this value to anything they like which could interact badly: request.deleteProvider = "foo"
request.providersToUnlink = [ "phone", "email" ]
request.deleteProvider = (request.deleteProvider || []).concat(request.providersToUnlink);
console.log(request.deleteProvider)
> ["foophone", "email"] Such use would be pretty questionable since Thinking about it a bit more though, another alternative would be to just set deleteProvider to undefined (or []) after we take the copy, thus ensuring any value that the user sets can't interfere with this value. I've left it as is for now, though could switch to the alternative if you'd prefer. |
||
request.deleteProvider = []; | ||
} | ||
request.deleteProvider = request.deleteProvider.concat(request.providersToUnlink); | ||
delete request.providersToUnlink; | ||
} | ||
|
||
// Rewrite photoURL to photoUrl. | ||
|
Uh oh!
There was an error while loading. Please reload this page.