feat: add correlation identifier to interactions

[mattkelley]

Background

Prior to v7, an Interaction's uid was constant throughout the Authorization Request's series of Interactions. In the changelog for v7.0.0 Panva noted "Every interaction now gets a totally unique identifier, "same grant", which never actually was about grants, or consequent bounces through interaction will now each get a unique identifier." I've interpreted one concern of this change to be rooted in security best practices. For example, if an Interaction's uid updates after each Interaction, and this uid value is reflected in the user-agent's cookie (value and path by default), the effectiveness of stealing a user-agent's interaction cookie is essentially eliminated. I also noticed the default TTLs for Interactions increased from 10m to 1h in the v7.0.0 changelog, it seems if an Interaction can only be used once, its TTL can safely be increased.

What's changing?

I've implemented a new property on the Interaction model called cid which is intended to abbreviate "Correlation Id". This identifier is generated only once when the Authorization is started, and uses the existing nanoid() id generation method, every subsequent Interaction will contain the same cid value.

Why introduce this change?

I have two reasons I'm interested in this change:

I've implemented a single page application for the front-end of the provider implementation - it required a few tricks like loading interaction resume URLs in iframes, and implementing a state transfer endpoint we call GET /status but so far has worked nicely for us. Because our view layer is often not driven by the server, when interaction ids update users can see an error page if they attempt to navigate back / forward in their browser history stack and the Interaction was updated. For example: User navigates from example.com/interaction/123/register to example.com/interaction/456/register/verify, and finally back to example.com/interaction/123/register.
My intention with this change is to update our interactionUrl() config option from

interactions: {
  url(ctx, interaction) { // eslint-disable-line no-unused-vars
    return `/interaction/${interaction.uid}`;
  },
},

to

interactions: {
  url(ctx, interaction) { // eslint-disable-line no-unused-vars
    return `/interaction/${interaction.cid}`;
  },
},

I think for our implementation, and possibly other implementors, using the cid in the cookie's path and interaction base url, while ensuring the cookie's value continues to update on each Interaction is a great balance of security / browsing convenience.

The second intention to this change, and maybe more useful for a wider group of library implementors: Prior to v7, when my team was triaging an issue we'd often ask the question "What's the Interaction Id?". After obtaining this identifier it was easy to go into application logs and see all the requests / Interactions associated to this Interaction.uid. This was useful for us because users would often have many Interactions as they navigated prompts, and other multi-step flows (e.g Forgot Password, OTP Verification, and Registration). Post v7 upgrade, we still ask the question "What's the Interaction Id?", although correlating the Authorization Request's "user journey" requires a bit more work (once a single Interaction id, now many).

Thank you very much for your time and consideration of this change 👋

[panva]

Hi @mattkelley

Would you please strip the PR down to just the bare minimum? It is changing files that it definitely shouldn't.

I'll take a look at it then.

[mattkelley]

Hi @mattkelley

Would you please strip the PR down to just the bare minimum? It is changing files that it definitely shouldn't.

I'll take a look at it then.

Of course! I've amended the commit. Thanks!

[panva]

I think this is ultimately flawed in that it promises to correlate different interactions but only does so if they're part of the same authorization request, the moment there's a step-up later - this correlation identifier is missing.

It is also entirely possible to achieve this through the existing extensibility.

[mattkelley]

It is also entirely possible to achieve this through the existing extensibility.

I tried to come up with a way of doing this before proposing a change, but wasn't sure how to hook into the Interaction model. I looked at implementing a middleware approach but it didn't feel right, as I believe the interaction would have already been saved. How would you go about implementing something like this with the existing extensibility, I could convert that into a recipe instead if you'd prefer 😄

[mattkelley]

One other question about this:

it promises to correlate different interactions but only does so if they're part of the same authorization request, the moment there's a step-up later

What do you mean by "step-up later", I figured all you'd want to do is correlate different interactions that are part of the same authorization request - in a similar fashion to how Interaction uid worked prior to v7. What am I missing?

[panva]

I tried to come up with a way of doing this before proposing a change, but wasn't sure how to hook into the Interaction model.

Overload the provider.Interaction.IN_PAYLOAD getter to return your extra property and then set the property in any of the extension points that have access to ctx and the interaction instance before it gets saved, e.g. the ttl.Interaction configuration function...

I see that's not obvious at first glance, I'll take a look at the this PR again in a bit when I have time.

Details

diff --git a/certification/oidc/configuration.js b/certification/oidc/configuration.js
index a87a4320..c73e9e81 100644
--- a/certification/oidc/configuration.js
+++ b/certification/oidc/configuration.js
@@ -189,6 +189,10 @@ export default {
       .digest('hex');
   },
   ttl: {
+    Interaction(ctx, interaction) {
+      interaction.mystableid = ctx.oidc.entities.Interaction?.mystableid || crypto.randomUUID();
+      return 10 * 60;
+    },
     RegistrationAccessToken: 1 * 24 * 60 * 60,
   },
   clientAuthMethods,
diff --git a/certification/oidc/index.js b/certification/oidc/index.js
index 229b90d0..ca72d7d4 100644
--- a/certification/oidc/index.js
+++ b/certification/oidc/index.js
@@ -30,6 +30,13 @@ try {
 
   const provider = new Provider(ISSUER, { adapter, ...configuration });
 
+  const orig = Object.getOwnPropertyDescriptor(provider.Interaction, 'IN_PAYLOAD').get;
+  Object.defineProperty(provider.Interaction, 'IN_PAYLOAD', {
+    get() {
+      return [...orig(), 'mystableid'];
+    },
+  });
+
   if (GOOGLE_CLIENT_ID) {
     const openid = await import('openid-client'); // eslint-disable-line import/no-unresolved
     const google = await openid.Issuer.discover('https://accounts.google.com/.well-known/openid-configuration');

dismissing for now

[mattkelley]

@panva 🤯 wow this is fantastic - I can't thank you enough. Let me know what you think about moving forward with this PR, or if you'd rather see your approach to extending the Interaction model captured as recipe in the repo. I think a recipe could be helpful for others looking to hook into Interaction model (or any other model), I'm happy to close this PR and add that recipe in a follow up PR.

Thank you again,
Matt