-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Node broker implementation #5550
Conversation
…tion-library-for-js into node-extensions-nativeBrokerPlugin
…tion-library-for-js into node-extensions-nativeBrokerPlugin
…tion-library-for-js into node-extensions-nativeBrokerPlugin
…tion-library-for-js into node-extensions-nativeBrokerPlugin
* @param request | ||
* @returns | ||
*/ | ||
async signOut(request: SignOutRequest): Promise<void> { |
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.
Slightly worried if this could be mistaken for an API doing a service signout (end session). I suppose the name was chosen to align with X.. (likely lacking context, apologies)
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.
signOut
API has the same name across all MSAL APIs except formsal-browser
. What do you have in mind?
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.
@sameerag didn't know that! My worry is, this API only does a "local" signout, it doesn't end the session with IdP. We probably should emphasize this in API description as it's reasonable to expect this to be misunderstood.
samples/msal-node-samples/auth-code-cli-brokered-app/src/index.js
Outdated
Show resolved
Hide resolved
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.
LGTM -minor comments
...remainingProperties, | ||
clientId: this.config.auth.clientId, | ||
scopes: request.scopes || OIDC_DEFAULT_SCOPES, | ||
redirectUri: `${Constants.HTTP_PROTOCOL}${Constants.LOCALHOST}`, |
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.
Basic q: Is it set this way as we do not need it for the native calls?
Co-authored-by: Doğan Erişen <v-derisen@microsoft.com>
…tion-library-for-js into msal-node-brokering
…osoft-authentication-library-for-js into msal-node-brokering
🎉 Handy links: |
Exposes nativeBrokerPlugin to enable token acquisition through the system broker via msal-node-extensions and msal-runtime --------- Co-authored-by: Thomas Norling <thnorlin@microsoft.com> Co-authored-by: Doğan Erişen <v-derisen@microsoft.com>
Exposes nativeBrokerPlugin to enable token acquisition through the system broker via msal-node-extensions and msal-runtime --------- Co-authored-by: Thomas Norling <thnorlin@microsoft.com> Co-authored-by: Doğan Erişen <v-derisen@microsoft.com>
Exposes nativeBrokerPlugin to enable token acquisition through the system broker via msal-node-extensions and msal-runtime