Skip to content
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

DO NOT MERGE - Add a separate function for calling getAccessToken #138

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vcua-mobify
Copy link
Contributor

DO NOT MERGE

To support both public and private clients as well as 3rd party IDPs, we could export a function that allows for more flexibility in calling SLAS /token endpoint.

Thinking is that if a refresh token is included, we can assume the user wants a refresh token login. This grant type supersedes all other types of request.

If no refresh token, check if the user provides a code verifier. If yes, assume the user is using a SLAS public client (only public clients need to provide a code verifier as the PKCE)

If no code verifier, we can assume the user is using a SLAS private client. If a code is still provided, set grant_type to authorization_code. This is used for registered user federated login.

If no code is provided, assume we are logging in a guest user via SLAS private client and set grant_type to client_credentials

@vcua-mobify vcua-mobify requested a review from a team as a code owner November 28, 2023 00:25

return await getAccessToken(slasClient, parameters, credentials);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

commerce-sdk-isomorphic assumes we are using a SLAS public client

When using a SLAS private client, we want to skip the call to /authorize. We can immediately call the /token endpoint

* Wrapper for the /token endpoint. Depending on the parameters provided, will set up different grant types.
* @param slasClient a configured instance of the ShopperLogin SDK client
*/
export async function getAccessToken(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a catch-all wrapper for the getAccessToken endpoint that can handle refresh tokens, pkce (public client) logins, and client-credentials (private client) logins.

Later on, we could refactor the other helper methods to call this rather than setting up the /token call for a specific case in many places.

const grantType = parameters.refreshToken ? 'refresh_token'
: parameters.codeVerifier ? 'authorization_code_pkce'
: parameters.code ? 'authorization_code'
: 'client_credentials'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parameters we provide to /token affect the grant_type and what the endpoint ultimately does.

I think the order of this conditional is the correct priority order for how to handle scenarios where multiple parameters corresponding to different grant_types are provided.

credentials?: {
client_id: string;
client_secret: string;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the PWA Kit, we're using a proxy to intercept the call to SLAS and inject the client secret there. So PWA Kit does not set this arg.

However, this arg is here to enable non-PWA Kit projects that may want to use a private client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant