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

Well Known Support #1607

Closed
mod-flux opened this issue Mar 28, 2021 · 5 comments
Closed

Well Known Support #1607

mod-flux opened this issue Mar 28, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@mod-flux
Copy link

Summary of proposed feature
For OAuth providers that have .well-known configuration endpoints, offer the ability to provide these and have NextAuth dynamically discover and setup against all endpoints and everything that's provided as opposed to hardcoding all the endpoints manually.

Purpose of proposed feature
Dynamic service discovery is always advisable as it means the vendor can change their endpoints and/or configuration and a NextAuth enabled website can pick up the configuration dynamically and continue operating. Furthermore, it takes a lot of work out of manually configuring all of the endpoints which have inherent human error.

Detail about proposed feature
In the provider's options, provide the ability to set a .well-known endpoint and have the NextAuth service set up as per the configuration. Perhaps saving this configuration in the Lambda cache so it doesn't have to be requested every time.

Potential problems
Could conflict with other OAuth provider options. Could also lead to more time on the initial request.

Describe any alternatives you've considered
Manual configuration, which is what I've done to date.

@mod-flux mod-flux added the enhancement New feature or request label Mar 28, 2021
@balazsorban44
Copy link
Member

balazsorban44 commented Mar 28, 2021

#1048 should probably be preceeding this, that would made implementation especially easy. Although the new alternative does not support OAuth 1.x, which is a bummer. If we want to keep it, we might have to have two oauth dependencies, which would increase package size a lot.

Most of the built-in/already pre-configured providers could use this as well, although this should be more of a fallback option in my opinion, to avoid an additional request on each invocation. Maybe we could extract this build-time somehow? If one of the endpoints aren't present or wrong, we could utilize the well-known endpoint to fetch the missing ones and show a warning that tells about the extra request in each invocation.

If we change the oauth package for OAuth 2 and OIDC providers, the provider options shape will probably have to change, as there is no fine overlap there and exposing the client config for both packages might be confusing.

Open for ideas how we could migrate with the least possible amount of breaking changes and confusion for users.

@balazsorban44
Copy link
Member

balazsorban44 commented Apr 25, 2021

So after playing around with #1698, I even think that this could be introduced in a non-breaking change! I got openid-client working with the configuration, no changes needed from the user! 🎉

One downside of .well-known is that there will always be an extra call to that endpoint, whenever you interact with NextAuth. It would make more sense maybe to fetch the config build-time and provide that instead. What do you think?

This could tie in neatly with #1846, where wellKnown could be an OAuthEndpoint type (see #1846 for explanation), and you could use the request method and cache the config however you want for example.

@mod-flux
Copy link
Author

This makes sense and great news @balazsorban44! Could we not cache it on the Lambda similar to how the auto-generated keys work? I don't know the inner workings but atleast that way it would only request per lambda for aslong as it's alive. Failing that, build time seems the most appropriate, as you've highlighted.

@balazsorban44
Copy link
Member

I decided to leave this out from #1698 initially to decrease the scope of the PR. We could address this subsequently in another one later on.

balazsorban44 added a commit that referenced this issue Aug 4, 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](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.
@balazsorban44
Copy link
Member

Landed in next-auth@4.0.0-next.20. For now, I will await any optimization/caching of the endpoint result. We will see if there is actual interest/need for it. openid-client actually already uses lru-cache, so it might not even be needed.

mnphpexpert added a commit to mnphpexpert/next-auth that referenced this issue 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
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants