Skip to content

Support registered clients in static Session ctor - #4388

Open
NSeydoux wants to merge 11 commits into
mainfrom
SDK-3455/support-non-url-client-id
Open

Support registered clients in static Session ctor #4388
NSeydoux wants to merge 11 commits into
mainfrom
SDK-3455/support-non-url-client-id

Conversation

@NSeydoux

@NSeydoux NSeydoux commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Session.fromAuthorizationRequestState can now be used with a statically registered client. A NodeJS client that has been registered can authenticate to the OpenID Provider when going through the Authorization Code flow. Previously, only Solid-OIDC URL-based client identifiers were supported.

New feature description

Checklist

  • All acceptance criteria are met.
  • Relevant documentation, if any, has been written/updated. N/A
  • The changelog has been updated, if applicable.
  • New functions/types have been exported in index.ts, if applicable. N/A
  • Commits in this PR are minimal and have descriptive commit messages.

@NSeydoux
Support registered clients in static Session ctor
4b48e40 
`Session.fromAuthorizationRequestState` can now be used with a statically registered client. A NodeJS client that has been registered can authenticate to the OpenID Provider when going through the Authorization Code flow. Previously, only Solid-OIDC URL-based client identifiers were supported.
@NSeydoux
Move client secret to a separate argument
ddee388 
The client id fits naturally in the authorization request state, and the
client secret does not, so it's better to add it as a separate arg
rather than overloading the input type and require type gymnastics.
@NSeydoux
Fix build and lint
87ecaba
@NSeydoux
Emit maps
4a536f5 
This adds the declaration maps to the emitted code, to help navigate to
the source definitions across packages.
@NSeydoux
fixup! Emit maps
0d3f6ec
NSeydoux
NSeydoux commented Sep 9, 2026
View reviewed changes
Comment thread tsconfig.build.json
"module": "preserve",
"strict": true,
"declaration": true,
"declarationMap": true,

@NSeydoux NSeydoux Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is not directly related to the changes in this PR, but it makes navigating the code easier (it was long overdue).

NSeydoux
NSeydoux commented Sep 9, 2026
View reviewed changes
Comment thread packages/core/src/sessionInfo/ISessionInfoManager.ts
webId?: string;
}

export type SessionManagerAuthorizationState = AuthorizationRequestState & {

@NSeydoux NSeydoux Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Enforce the supported states in the type to prevent invalid state being representable.

@NSeydoux
NSeydoux marked this pull request as ready for review September 9, 2026 15:38
@NSeydoux
NSeydoux requested a review from a team as a code owner September 9, 2026 15:38
@NSeydoux
NSeydoux deployed to ESS PodSpaces September 9, 2026 16:04 — with GitHub Actions Active
jholleran
jholleran reviewed Sep 9, 2026
View reviewed changes
Comment thread packages/node/examples/server/.env.example

# Solid-OIDC Client identifier for the application.
CLIENT_ID="https://storage.inrupt.com/d70fd154-de71-4627-89f3-9f9515950f7a/client_ids/ca999ecd-7951-4982-b7eb-d4376985f978"
# For a statically registered client, set the CLIENT_ID and CLIENT_SECRET to the credentials issued by the Identity Provider

@jholleran jholleran Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we have to set a CLIENT_TYPE? It would be a bit more explicit if we had to.

jholleran
jholleran reviewed Sep 9, 2026
View reviewed changes
Comment thread packages/node/src/Session.ts
Comment on lines +206 to +212
} else if (hasSecret) {
state = {
...authorizationRequestState,
keepAlive: false,
clientType: "static",
clientSecret,
};

@jholleran jholleran Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A follow on from: https://github.com/inrupt/solid-client-authn-js/pull/4388/changes#r3970848217 - it might be more extensible if we avoid this logic and have a proper type setting.

@NSeydoux NSeydoux Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, we track this internally anyway. However, I'd prefer to do this change separately, so that it is applied consistently across all the relevant APIs (which goes beyond the scope of this PR): the client type should be set before the authorization request (so in the login function), and the same change should be applied to the browser package. I'll create the follow-up ticket.

@NSeydoux
NSeydoux requested a review from jholleran September 11, 2026 12:05
@NSeydoux
NSeydoux deployed to ESS PodSpaces September 15, 2026 14:54 — with GitHub Actions Active
@NSeydoux
NSeydoux deployed to ESS PodSpaces September 15, 2026 14:54 — with GitHub Actions Active
@NSeydoux
NSeydoux deployed to ESS PodSpaces September 15, 2026 14:54 — with GitHub Actions Active
@NSeydoux
NSeydoux added this pull request to the merge queue Sep 17, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jholleran jholleran jholleran approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@NSeydoux @jholleran