-
Notifications
You must be signed in to change notification settings - Fork 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
Sm/acccesstoken works as username #344
Conversation
@@ -758,8 +759,7 @@ export class AuthInfo extends AsyncCreatable<AuthInfo.Options> { | |||
this.fields.username = this.options.username || getString(options, 'username') || undefined; | |||
|
|||
// If the username is an access token, use that for auth and don't persist | |||
const accessTokenMatch = isString(this.fields.username) && this.fields.username.match(/^(00D\w{12,15})![.\w]*$/); | |||
if (accessTokenMatch) { | |||
if (isString(this.fields.username) && sfdc.matchesAccessToken(this.fields.username)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used in multiple places, so moved to the sfdc utils
@@ -758,8 +759,7 @@ export class AuthInfo extends AsyncCreatable<AuthInfo.Options> { | |||
this.fields.username = this.options.username || getString(options, 'username') || undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is last || undefined
necessary, given getString either returns the property or undefined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the || undefined
suffix looks unnecessary based on how getString
works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TS complains:
Type 'Optional<string | null>' is not assignable to type 'string | undefined'.
Type 'null' is not assignable to type 'string | undefined'.
going to leave it for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh right. getString
returns string | null
not string | undefined
. null !== undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With some suggestions.
@@ -758,8 +759,7 @@ export class AuthInfo extends AsyncCreatable<AuthInfo.Options> { | |||
this.fields.username = this.options.username || getString(options, 'username') || undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the || undefined
suffix looks unnecessary based on how getString
works
@W-7885939@
By fixing this accessToken issue in Core, it might also solve some other things like