Request for extra docs/ guidance for upgrading from v5.x -> v6.x (API surface)

[webketje]

Hi there, I have been using openid-client for a gov. website for over 3 years (with Typescript) and had been struggling for days to upgrade to 6.x. I understand a lot of the global client config options have been made endpoint-specific and/or optional as request params and that gives more fine-grained control. I would love to see a small migration example for a simple code grant login flow, even if it's just a before - after comparison v5/v6 because the entire surface API has dramatically changed (mourning the substitution of TokenSet with TokenEndpointResponse & TokenEndpointResponseHelpers, and especially the new loose typing of the parameters param of build* methods, as parameters: URLSearchParams | Record<string, string> gives you 0 hint about what kind of param is expected, a.o.).

In the end I got the basic login/logout flows working again but it would have been much easier if there was a "Migrating from v5 to v6" section in the docs or in the release: https://github.com/panva/openid-client/releases/tag/v6.0.0, that shows you more or less how the flows and especially the public API has been remapped, eg:

• TS: TokenSet -> TokenEndpointResponse & TokenEndpointResponseHelpers
• tokenSet.expires_at -> TokenEndpointResponse.expires_in VERY IMPORTANT CHANGE
• client.callbackParams(req) + client.callback -> client.authorizationCodeGrant
• TS: a lot of primitives/ simple objects have been recast as JsonValue, this is causing a lot of issues passing around strings, and especially with unit test mocks (resorting to hacks like as JsonValue)
• TS: the Request param (for building URLs) is unfortunately incompatible with Express.js requests

I am also under the impression that error names/codes changed, I had to alter passing the baseURL for redirect_uri's whereas that used to be auto-concatenated? But I may be wrong here.

[panva]

TS: a lot of primitives/ simple objects have been recast as JsonValue, this is causing a lot of issues passing around strings

can you elaborate on this please?

TS: the Request param (for building URLs) is unfortunately incompatible with Express.js requests

You mean Node.js's http.IncomingMessage is incompatible with the Fetch API's Request ;) No I get it, that's why passing URLs around instead of Request is still a needed option to have at hand.

mourning the substitution of TokenSet with TokenEndpointResponse & TokenEndpointResponseHelpers

Interesting, I dread ever introducing TokenSet to begin with and am happy to see it gone in favour of actual protocol response representations with much simpler helpers.

tokenSet.expires_at -> TokenEndpointResponse.expires_in VERY IMPORTANT CHANGE

expires_at was a getter posing as a response parameter which doesn't in reality exist on an OAuth 2.0 Token Endpoint Response. It was a silly thing to have to begin with.

I had to alter passing the baseURL for redirect_uri's whereas that used to be auto-concatenated? But I may be wrong here.

I haven't got the slightest clue what this may refer to, any details are welcome.

[webketje]

I don't doubt the new version is cleaner (or that expires_in makes more sense than than expires_at), and you're right that some type inflexibility comes from Express.js/Node rather than openid-client. I'm sharing the v5->6 upgrade is a lot of rediscovery, trial and error. The v6 release post did mention the changes very high-level, but having some extra API/types-level info in the form of a v5->6 docs section would have much eased that. Typing the parameters param of build* methods such as buildAuthorizationUrl, as | AuthorizationUrlParams (instead of | Record<string, string>) would assist with intellisense

Screenshot

Interesting, I dread ever introducing TokenSet to begin with and am happy to see it gone in favour of actual protocol response representations with much simpler helpers.

TS: a lot of primitives/ simple objects have been recast as JsonValue, this is causing a lot of issues passing around strings

can you elaborate on this please?

IMO TokenSet makes (as much) sense in the "client app" context, as the new nomenclature in the "oauth network" context, so idm.
The reason I wrote this is actually due to a TS error, it looks like both oauth4webapi TokenEndpointResponse and openid-client TokenEndpointResponseHelpers have a somehow conflicting claims property. I can mock them separately but not with &.

Screenshot

I had to alter passing the baseURL for redirect_uri's whereas that used to be auto-concatenated? But I may be wrong here.

I haven't got the slightest clue what this may refer to, any details are welcome.

Nvm, this is partially related to how request params are passed (v5: in client constructor as redirect_uris and post_logout_redirect_uris <-> v6 directly as param (=better, I agree) on the request), but mostly specific to the structure of my wrapping lib.

As I do the upgrade, I'll document here that RPError kind of maps to the new ClientError and OPError has been split in 3 types of error.

[panva]

I can mock them separately but not with &.

Ah yeah, that rings a bell. In the client code I do this to attach the helpers...

function getHelpers(response: oauth.TokenEndpointResponse) {
  // ...
}

function addHelpers(
  response: oauth.TokenEndpointResponse,
): asserts response is typeof response & TokenEndpointResponseHelpers {
  Object.defineProperties(response, getHelpers(response))
}

Don't really have better answer. There needs to be an index signature and by definition of it coming from a JSON.parsed response it is bound to be JsonValue... ¯\_(ツ)_/¯