Skip to content
This repository has been archived by the owner on Jun 13, 2022. It is now read-only.

feat: Breaking changes to SmartAuthProvider #170

Merged
merged 1 commit into from
Dec 15, 2021
Merged

Conversation

jdjkelly
Copy link
Member

  • A new interface replacing the previous type
  • Root keys are simpler
  • Allows explicitly for the expression of Client Credentials flow clients

* A new interface replacing the previous type
* Root keys are simpler
* Allows explicitly for the expression of Client Credentials flow clients
@jdjkelly jdjkelly requested a review from acr13 December 14, 2021 22:10
@jdjkelly jdjkelly self-assigned this Dec 14, 2021
Copy link
Contributor

@acr13 acr13 left a comment

Choose a reason for hiding this comment

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

Couple questions / comments but for the most part looks great


interface ClientCredentialsConfig extends SmartAuthConfig {
grantFlow: "client_credentials"
};
Copy link
Contributor

Choose a reason for hiding this comment

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

for the client_credientials grant flow, are we banking on tokenParams having a scope property?

Copy link
Member Author

Choose a reason for hiding this comment

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

No - if it has it, we will use it. But it doesn't need it.

In most OAuth 2.0 libraries, including simple-oauth2, you get to specify the "default" scope you want to use for a given Authorization Code grant flow request. When you want to support different grant flows with the same config, things get a little trickier because scope is also used in the Credentials Grant flow.

How do they each work?

In the Authorization Code grant flow the scope value is used during the authorization URL creation step (the place we redirect the users to first). scope is commonly used as a global config value that is used for every single authorization URL, but there's no reason that you can't change it per request.

This might make sense for example when you have an app that flexibly lets the user select which scopes they want to give to the app. You could in advance ask the user what they want to give you, before redirecting them to the authorization URL.

The param scope also happens to be used by the Client Credentials grant flow but it's very rarely ever going to make sense to have the same default values. Making a Client Credentials grant flow with scopes that are the same as a user is sort of disjoint from the point of the CC grant flow: to give you application instead of user-level access to some resources. Worse, SMART App Launch as a spec says absolutely nothing about Client Credentials.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, if you set up a SmartAuthProvider with the Client Credentials grant flow, we're going to listen to anything you want to specify in tokenParams but no other defaults.

src/smart-auth/index.ts Show resolved Hide resolved
src/smart-auth/index.ts Show resolved Hide resolved
@jdjkelly jdjkelly changed the title Breaking changes to SmartAuthProvider feat: Breaking changes to SmartAuthProvider Dec 15, 2021
@kodiakhq kodiakhq bot merged commit 967e54f into master Dec 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants