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

feat: improve OAuth provider configuration #2411

Merged
merged 40 commits into from
Aug 4, 2021

Conversation

balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Jul 20, 2021

This touches on all OAuth providers, so there is a big potential for breaking by default. We have let new providers be added for contributors' specific needs, but from now on, we will require a more strict default on all new providers, so the basic behavior is predictable for everyone.
⚠ Unfortunately, we will not have the capacity to test each and every provider that has been added to the default providers, but we will do our best to test the most popular ones. (@ndom91 has worked on setting up the infrastructure for this). If you wish to make sure that the provider you are using will stay working, please reach out with your concerns and tell us how can you help us test that particular provider in the future. 🙏

That said, I will try my best to not break ANY of the currently built-in providers, or at least make the migration super easy. So hopefully, you won't have to change anything. It will most probably affect you if you defined a custom provider though.

We will monitor the default configuration much more closely, so the behavior will be more consistent across providers by default.

Closes #1846, Closes #1605, Closes #1607

BREAKING CHANGES:

Basecamp provider is removed. See the explanation here

ALL OAuth providers' profile callback is expected to only return these fields by default from now on: id, name, email, and image at most. Any of these missing values should be set to null.

The following new options are available:

  1. authorization (replaces authorizationUrl, authorizationParams, scope)
  2. token replaces (accessTokenUrl, headers, params)
  3. userinfo (replaces profileUrl)

These three options map nicely to the OAuth spec's three endpoints for

  1. initiating the login flow
  2. retrieve OAuth tokens
  3. retrieve user information

They all take the form of EndpointHandler:

type EndpointRequest<C, R> = (
  context: C & {
    /** `openid-client` Client */
    client: Client
    /** Provider is passed for convenience, ans also contains the `callbackUrl`. */
    provider: OAuthConfig & {
      signinUrl: string
      callbackUrl: string
    }
  }
) => Awaitable<R>

/** Gives granular control of the request to the given endpoint */
type AdvancedEndpointHandler<P extends UrlParams, C, R> = {
  /** Endpoint URL. Can contain parameters. Optionally, you can use `params`*/
  url?: string
  /** These will be prepended to the `url` */
  params?: P
  /**
   * Control the corresponding OAuth endpoint request completely.
   * Useful if your provider relies on some custom behavior
   * or it diverges from the OAuth spec.
   *
   * - ⚠ **This is an advanced option.**
   * You should **try to avoid using advanced options** unless you are very comfortable using them.
   */
  request?: EndpointRequest<C, R>
}

/** Either an URL (containing all the parameters) or an object with more granular control. */
type EndpointHandler<P extends UrlParams, C = any, R = any> =
  | string
  | AdvancedEndpointHandler<P, C, R>

In case of authorization, the EndpointHandler can define the params as AuthorizationParameters

Note: authorization does not implement request yet. We will have to see if there is demand for it.

From now on, instead of using the ... spread operator when adding a new built-in provider, the user is expected to add options as a property at the end of the default config. This way, we can deep merge the user config with the default one. This is needed to let the user do something like this:

MyProvider({
  clientId: "",
  clientSecret: "",
  authorization: { params: {scope: ""} }
})

So even if the default config defines anything in authorization, only the user-defined parts will be overridden.

@vercel
Copy link

vercel bot commented Jul 20, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/FS2wPp4RxZw7DAQER6hYNvf9oSzZ
✅ Preview: https://next-auth-git-feat-rfc-oauth-provider-config-nextauthjs.vercel.app

@github-actions github-actions bot added core Refers to `@auth/core` providers TypeScript Issues relating to TypeScript labels Jul 20, 2021
@vercel vercel bot temporarily deployed to Preview July 20, 2021 23:22 Inactive
@vercel vercel bot temporarily deployed to Preview July 20, 2021 23:27 Inactive
@vercel vercel bot temporarily deployed to Preview July 22, 2021 20:33 Inactive
@vercel vercel bot temporarily deployed to Preview August 1, 2021 12:45 Inactive
@vercel vercel bot temporarily deployed to Preview August 4, 2021 22:25 Inactive
@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2021

Codecov Report

Merging #2411 (67fa328) into next (f06e4d2) will decrease coverage by 0.22%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #2411      +/-   ##
==========================================
- Coverage   11.74%   11.52%   -0.23%     
==========================================
  Files          83       83              
  Lines        1294     1319      +25     
  Branches      358      370      +12     
==========================================
  Hits          152      152              
- Misses        956      971      +15     
- Partials      186      196      +10     
Impacted Files Coverage Δ
src/lib/merge.js 0.00% <0.00%> (ø)
src/providers/42.js 0.00% <0.00%> (ø)
src/providers/apple.js 0.00% <0.00%> (ø)
src/providers/atlassian.js 0.00% <ø> (ø)
src/providers/auth0.js 0.00% <ø> (ø)
src/providers/azure-ad-b2c.js 0.00% <0.00%> (ø)
src/providers/azure-ad.js 0.00% <0.00%> (ø)
src/providers/battlenet.js 0.00% <0.00%> (ø)
src/providers/box.js 0.00% <ø> (ø)
src/providers/bungie.js 0.00% <ø> (ø)
... and 48 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f06e4d2...67fa328. Read the comment docs.

@vercel vercel bot temporarily deployed to Preview August 4, 2021 22:27 Inactive
@valstu
Copy link

valstu commented Sep 6, 2021

Note: authorization does not implement request yet. We will have to see if there is demand for it.

This is exactly what I would need 😅

I need to take all the params from the initial authorization request and send them as JWS inside request query params.

@balazsorban44
Copy link
Member Author

please open a new issue/feature request with a detailed description, preferably with code examples, alternatives you have considered, etc.

Comment on lines -139 to +140
scope: "identify",
authorization: { params: { scope: "identify" } },

Choose a reason for hiding this comment

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

I'm using beta.2 and this doesn't seem to work, whatever I do the scope in the URL remains as scope=identify%20email.

I do not want to know user's their email.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please open an issue/discussion and attach a reproduction. This is not the right place. You are commenting on a test file. 👀

mnphpexpert added a commit to mnphpexpert/next-auth that referenced this pull request Sep 2, 2024
> This touches on all OAuth providers, so there is a big potential for breaking by default. We have let new providers be added for contributors' specific needs, but from now on, we will require a more strict default on all new providers, so the basic behavior is predictable for everyone.
⚠ Unfortunately, we will not have the capacity to test each and every provider that has been added to the default providers, but we will do our best to test the most popular ones. (@ndom91 has worked on setting up the infrastructure for this). If you wish to make sure that the provider you are using will stay working, please reach out with your concerns and tell us how can you help us test that particular provider in the future. 🙏

That said, I will try my best to not break ANY of the currently built-in providers, or at least make the migration super easy. So hopefully, you won't have to change anything. It will most probably affect you if you defined a custom provider though.

We will monitor the default configuration much more closely, so the behavior will be more consistent across providers by default.

Closes nextauthjs#1846, Closes nextauthjs#1605, Closes nextauthjs#1607

BREAKING CHANGES:

Basecamp provider is removed. See the explanation [here](https://github.com/basecamp/api/blob/master/sections/authentication.md#on-authenticating-users-via-oauth)

**ALL** OAuth providers' `profile` callback is expected to only return these fields by default from now on: `id`, `name`, `email`, and `image` at most. Any of these missing values should be set to `null`.

The following new options are available:
1. `authorization` (replaces `authorizationUrl`, `authorizationParams`, `scope`)
2. `token` replaces (`accessTokenUrl`, `headers`, `params`)
3. `userinfo` (replaces `profileUrl`)

These three options map nicely to the OAuth spec's three endpoints for
1. initiating the login flow
2. retrieve OAuth tokens
3. retrieve user information

They all take the form of `EndpointHandler`:
```ts
type EndpointRequest<C, R> = (
  context: C & {
    /** `openid-client` Client */
    client: Client
    /** Provider is passed for convenience, ans also contains the `callbackUrl`. */
    provider: OAuthConfig & {
      signinUrl: string
      callbackUrl: string
    }
  }
) => Awaitable<R>

/** Gives granular control of the request to the given endpoint */
type AdvancedEndpointHandler<P extends UrlParams, C, R> = {
  /** Endpoint URL. Can contain parameters. Optionally, you can use `params`*/
  url?: string
  /** These will be prepended to the `url` */
  params?: P
  /**
   * Control the corresponding OAuth endpoint request completely.
   * Useful if your provider relies on some custom behavior
   * or it diverges from the OAuth spec.
   *
   * - ⚠ **This is an advanced option.**
   * You should **try to avoid using advanced options** unless you are very comfortable using them.
   */
  request?: EndpointRequest<C, R>
}

/** Either an URL (containing all the parameters) or an object with more granular control. */
type EndpointHandler<P extends UrlParams, C = any, R = any> =
  | string
  | AdvancedEndpointHandler<P, C, R>
```

In case of `authorization`, the `EndpointHandler` can define the `params` as [`AuthorizationParameters`](https://github.com/panva/node-openid-client/blob/51dc47d9ac619b71cd1c983b0be750a12bbae008/types/index.d.ts#L108-L143)

> Note: `authorization` does not implement `request` yet. We will have to see if there is demand for it.

From now on, instead of using the `...` spread operator when adding a new built-in provider, the user is expected to add `options` as a property at the end of the default config. This way, we can deep merge the user config with the default one. This is needed  to let the user do something like this:

```js
MyProvider({
  clientId: "",
  clientSecret: "",
  authorization: { params: {scope: ""} }
})
```
So even if the default config defines anything in `authorization`, only the user-defined parts will be overridden.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Refers to `@auth/core` documentation Relates to documentation providers test Related to testing TypeScript Issues relating to TypeScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants