feat: sector_identifier_uri can be used without pairwise subject_type

BREAKING CHANGE: `sector_identifier_uri` is now verified regardless of client's `subject_type` when provided.
panva · Feb 28, 2021 · 202e4c5 · 202e4c5
1 parent 59d6c52
commit 202e4c5
Show file tree

Hide file tree

Showing 8 changed files with 15 additions and 19 deletions.
diff --git a/lib/actions/introspection.js b/lib/actions/introspection.js
@@ -169,10 +169,10 @@ module.exports = function introspectionAction(provider) {
         ctx.body.sub = token.accountId;
         if (token.clientId !== ctx.oidc.client.clientId) {
           const client = await Client.find(token.clientId);
-          if (client.sectorIdentifier) {
+          if (client.subjectType === 'pairwise') {
             ctx.body.sub = await pairwiseIdentifier(ctx, ctx.body.sub, client);
           }
-        } else if (ctx.oidc.client.sectorIdentifier) {
+        } else if (ctx.oidc.client.subjectType === 'pairwise') {
           ctx.body.sub = await pairwiseIdentifier(ctx, ctx.body.sub, ctx.oidc.client);
         }
       }

diff --git a/lib/helpers/claims.js b/lib/helpers/claims.js
@@ -68,7 +68,7 @@ module.exports = function getClaims(provider) {
         }
       }
 
-      if (this.client.sectorIdentifier && claims.sub) {
+      if (this.client.subjectType === 'pairwise' && claims.sub) {
         claims.sub = await pairwiseIdentifier(this.ctx, claims.sub, this.client);
       }
 

diff --git a/lib/helpers/client_schema.js b/lib/helpers/client_schema.js
@@ -332,10 +332,6 @@ module.exports = function getSchema(provider) {
         }
       }
 
-      if (this.sector_identifier_uri !== undefined && this.subject_type !== 'pairwise') {
-        this.sector_identifier_uri = undefined;
-      }
-
       // SECTOR IDENTIFIER VALIDATION
       sectorIdentifier({
         subjectType: this.subject_type,

diff --git a/lib/helpers/interaction_policy/prompts/login.js b/lib/helpers/interaction_policy/prompts/login.js
@@ -58,7 +58,7 @@ module.exports = () => new Prompt(
       return Check.REQUEST_PROMPT;
     }
 
-    if (oidc.client.sectorIdentifier) {
+    if (oidc.client.subjectType === 'pairwise') {
       sub = await instance(oidc.provider).configuration('pairwiseIdentifier')(ctx, sub, oidc.client);
     }
 
@@ -81,7 +81,7 @@ module.exports = () => new Prompt(
       return Check.REQUEST_PROMPT;
     }
 
-    if (oidc.client.sectorIdentifier) {
+    if (oidc.client.subjectType === 'pairwise') {
       sub = await instance(oidc.provider).configuration('pairwiseIdentifier')(ctx, sub, oidc.client);
     }
 

diff --git a/lib/helpers/sector_identifier.js b/lib/helpers/sector_identifier.js
@@ -5,9 +5,7 @@ const { InvalidClientMetadata } = require('./errors');
 module.exports = ({ sectorIdentifierUri, redirectUris, subjectType }) => {
   if (subjectType === 'pairwise') {
     if (!sectorIdentifierUri) {
-      const { 0: host, length } = redirectUris
-        .map((uri) => new URL(uri).host)
-        .filter((value, index, self) => self.indexOf(value) === index);
+      const { 0: host, length } = [...new Set(redirectUris.map((uri) => new URL(uri).host))];
 
       if (length === 0) {
         throw new InvalidClientMetadata('sector_identifier_uri is required when redirect_uris hosts are not available');
@@ -19,9 +17,7 @@ module.exports = ({ sectorIdentifierUri, redirectUris, subjectType }) => {
 
       return host;
     }
-
-    return new URL(sectorIdentifierUri).host;
   }
 
-  return undefined;
+  return sectorIdentifierUri ? new URL(sectorIdentifierUri).host : undefined;
 };
diff --git a/lib/models/formats/jwt.js b/lib/models/formats/jwt.js
@@ -103,7 +103,7 @@ module.exports = (provider, { opaque }) => {
       if (sub) {
         const { client } = this;
         assert(client && client.clientId === clientId);
-        if (client.sectorIdentifier) {
+        if (client.subjectType === 'pairwise') {
           const pairwiseIdentifier = instance(provider).configuration('pairwiseIdentifier');
           sub = await pairwiseIdentifier(ctx, sub, client);
         }

diff --git a/lib/models/formats/paseto.js b/lib/models/formats/paseto.js
@@ -74,7 +74,7 @@ module.exports = (provider, { opaque }) => {
       if (sub) {
         const { client } = this;
         assert(client && client.clientId === clientId);
-        if (client.sectorIdentifier) {
+        if (client.subjectType === 'pairwise') {
           const pairwiseIdentifier = instance(provider).configuration('pairwiseIdentifier');
           sub = await pairwiseIdentifier(ctx, sub, client);
         }

diff --git a/test/pairwise/pairwise_clients.test.js b/test/pairwise/pairwise_clients.test.js
@@ -72,7 +72,11 @@ describe('pairwise features', () => {
     });
 
     context('sector_identifier_uri is provided', () => {
-      it('is ignored unless pairwise subject_type is used', function () {
+      it('is not ignored even without subject_type=pairwise', function () {
+        nock('https://foobar.example.com')
+          .get('/file_of_redirect_uris')
+          .reply(200, j(['https://client.example.com/cb', 'https://another.example.com/forum/cb']));
+
         return i(this.provider).clientAdd({
           client_id: 'client',
           client_secret: 'secret',
@@ -81,7 +85,7 @@ describe('pairwise features', () => {
           subject_type: 'public',
         }).then((client) => {
           expect(client).to.be.ok;
-          expect(client.sectorIdentifier).to.eq(undefined);
+          expect(client.sectorIdentifier).to.eq('foobar.example.com');
         });
       });