change semantics of ENABLE_COLLECTIONS_AUTHX to fail closed and allow * for all collections (#888)

Author: philvarner

CHANGELOG.md

@@ -5,6 +5,15 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [4.1.0] - 2024-04-21
+
+### Changed
+
+- **breaking** Collections Auth features (ENABLE_COLLECTIONS_AUTHX) now fails closed
+  rather than open. If `_collections` is not specified or is empty, caller has no
+  access to collections. Also, adds a special collection name `*` that means access
+  to all collections is granted.
+
 ## [4.0.0] - 2025-04-07
 
 ### Removed

README.md

@@ -1129,6 +1129,11 @@ The endpoints this applies to are:
 The five endpoints of the Transaction Extension do not use this parameter, as there are
 other authorization considerations for these, that are left as future work.
 
+If this behavior is enabled and a `_collections` parameter is not passed or is passed
+with an empty string or empty list, the caller will not have access to any collections.
+When `*` is included in the list of collections (ideally as the only value), the caller
+will have access to all collections.
+
 ## Ingesting Data
 
 STAC Collections and Items are ingested by the `ingest` Lambda function, however this Lambda is not invoked directly by a user, it consumes records from the `stac-server-<stage>-queue` SQS. To add STAC Items or Collections to the queue, publish them to the SNS Topic `stac-server-<stage>-ingest`.

src/lib/api.js

@@ -315,6 +315,11 @@ const extractFields = function (params) {
   return fieldRules
 }
 
+/**
+ * Parse a string or array of IDs into an array of strings or undefined.
+ * @param {string | string[] | undefined} ids - The IDs parameter to parse
+ * @returns {string[] | undefined} Parsed array of ID strings or undefined
+ */
 const parseIds = function (ids) {
   let idsRules
   if (ids) {
@@ -337,14 +342,28 @@ const extractIds = function (params) {
 
 const extractAllowedCollectionIds = function (params) {
   return process.env['ENABLE_COLLECTIONS_AUTHX'] === 'true'
-    ? parseIds(params._collections)
+    ? parseIds(params._collections) || []
     : undefined
 }
 
 const extractCollectionIds = function (params) {
   return parseIds(params.collections)
 }
 
+const filterAllowedCollectionIds = function (allowedCollectionIds, specifiedCollectionIds) {
+  return (
+    Array.isArray(allowedCollectionIds) && !allowedCollectionIds.includes('*')
+  ) ? allowedCollectionIds.filter(
+      (x) => !specifiedCollectionIds || specifiedCollectionIds.includes(x)
+    ) : specifiedCollectionIds
+}
+
+const isCollectionIdAllowed = function (allowedCollectionIds, collectionId) {
+  return !Array.isArray(allowedCollectionIds)
+          || allowedCollectionIds.includes(collectionId)
+          || allowedCollectionIds.includes('*')
+}
+
 export const parsePath = function (inpath) {
   const searchFilters = {
     root: false,
@@ -577,9 +596,7 @@ const searchItems = async function (collectionId, queryParameters, backend, endp
   const ids = extractIds(queryParameters)
   const allowedCollectionIds = extractAllowedCollectionIds(queryParameters)
   const specifiedCollectionIds = extractCollectionIds(queryParameters)
-  const collections = allowedCollectionIds ? allowedCollectionIds.filter(
-    (x) => !specifiedCollectionIds || specifiedCollectionIds.includes(x)
-  ) : specifiedCollectionIds
+  const collections = filterAllowedCollectionIds(allowedCollectionIds, specifiedCollectionIds)
   const limit = extractLimit(queryParameters) || 10
   const page = extractPage(queryParameters)
 
@@ -715,9 +732,28 @@ const aggregate = async function (
   const ids = extractIds(queryParameters)
   const allowedCollectionIds = extractAllowedCollectionIds(queryParameters)
   const specifiedCollectionIds = extractCollectionIds(queryParameters)
-  const collections = allowedCollectionIds ? allowedCollectionIds.filter(
-    (x) => !specifiedCollectionIds || specifiedCollectionIds.includes(x)
-  ) : specifiedCollectionIds
+  const collections = filterAllowedCollectionIds(allowedCollectionIds, specifiedCollectionIds)
+
+  if (Array.isArray(collections) && !collections.length) {
+    if (collectionId) {
+      return new NotFoundError()
+    }
+
+    return {
+      aggregations: [],
+      links: [
+        {
+          rel: 'self',
+          type: 'application/json',
+          href: `${endpoint}/aggregate`
+        },
+        {
+          rel: 'root',
+          type: 'application/json',
+          href: `${endpoint}`
+        }]
+    }
+  }
 
   const searchParams = pickBy({
     datetime,
@@ -991,7 +1027,7 @@ const validateAdditionalProperties = (queryables) => {
 
 const getCollectionQueryables = async (collectionId, backend, endpoint, queryParameters) => {
   const allowedCollectionIds = extractAllowedCollectionIds(queryParameters)
-  if (allowedCollectionIds && !allowedCollectionIds.includes(collectionId)) {
+  if (!isCollectionIdAllowed(allowedCollectionIds, collectionId)) {
     return new NotFoundError()
   }
 
@@ -1008,8 +1044,7 @@ const getCollectionQueryables = async (collectionId, backend, endpoint, queryPar
 }
 
 const getCollectionAggregations = async (collectionId, backend, endpoint, queryParameters) => {
-  const allowedCollectionIds = extractAllowedCollectionIds(queryParameters)
-  if (allowedCollectionIds && !allowedCollectionIds.includes(collectionId)) {
+  if (!isCollectionIdAllowed(extractAllowedCollectionIds(queryParameters), collectionId)) {
     return new NotFoundError()
   }
 
@@ -1155,7 +1190,7 @@ const getCollections = async function (backend, endpoint, queryParameters) {
 
   const allowedCollectionIds = extractAllowedCollectionIds(queryParameters)
   const collections = collectionsOrError.filter(
-    (c) => !allowedCollectionIds || allowedCollectionIds.includes(c.id)
+    (c) => isCollectionIdAllowed(allowedCollectionIds, c.id)
   )
 
   for (const collection of collections) {
@@ -1194,8 +1229,7 @@ const getCollections = async function (backend, endpoint, queryParameters) {
 }
 
 const getCollection = async function (collectionId, backend, endpoint, queryParameters) {
-  const allowedCollectionIds = extractAllowedCollectionIds(queryParameters)
-  if (allowedCollectionIds && !allowedCollectionIds.includes(collectionId)) {
+  if (!isCollectionIdAllowed(extractAllowedCollectionIds(queryParameters), collectionId)) {
     return new NotFoundError()
   }
 
@@ -1224,8 +1258,7 @@ const createCollection = async function (collection, backend) {
 }
 
 const getItem = async function (collectionId, itemId, backend, endpoint, queryParameters) {
-  const allowedCollectionIds = extractAllowedCollectionIds(queryParameters)
-  if (allowedCollectionIds && !allowedCollectionIds.includes(collectionId)) {
+  if (!isCollectionIdAllowed(extractAllowedCollectionIds(queryParameters), collectionId)) {
     return new NotFoundError()
   }
 
@@ -1279,8 +1312,7 @@ const deleteItem = async function (collectionId, itemId, backend) {
 }
 
 const getItemThumbnail = async function (collectionId, itemId, backend, queryParameters) {
-  const allowedCollectionIds = extractAllowedCollectionIds(queryParameters)
-  if (allowedCollectionIds && !allowedCollectionIds.includes(collectionId)) {
+  if (!isCollectionIdAllowed(extractAllowedCollectionIds(queryParameters), collectionId)) {
     return new NotFoundError()
   }
 

tests/system/test-api-collection-items-get.js

@@ -58,6 +58,10 @@ test.before(async (t) => {
   })
 })
 
+test.beforeEach(async (_) => {
+  delete process.env['ENABLE_COLLECTIONS_AUTHX']
+})
+
 test.after.always(async (t) => {
   if (t.context.api) await t.context.api.close()
 })
@@ -98,8 +102,22 @@ test('GET /collections/:collectionId/items with restriction returns filtered col
 
   const path = `collections/${collectionId}/items`
 
+  // _collections undefined
+  t.is((await t.context.api.client.get(path,
+    { resolveBodyOnly: false, throwHttpErrors: false })).statusCode, 404)
+
+  // _collections empty
+  t.is((await t.context.api.client.get(path,
+    { resolveBodyOnly: false,
+      throwHttpErrors: false,
+      searchParams: { _collections: '' },
+    })).statusCode, 404)
+
   t.is((await t.context.api.client.get(path,
-    { resolveBodyOnly: false })).statusCode, 200)
+    {
+      resolveBodyOnly: false,
+      searchParams: { _collections: '*' }
+    })).statusCode, 200)
 
   t.is((await t.context.api.client.get(path,
     {

tests/system/test-api-get-aggregate.js

@@ -43,6 +43,10 @@ test('GET /aggregate with no aggregations param', async (t) => {
   ])
 })
 
+test.beforeEach(async (_) => {
+  delete process.env['ENABLE_COLLECTIONS_AUTHX']
+})
+
 test('GET /aggregate with aggregations param', async (t) => {
   const fixtureFiles = [
     'collection.json',
@@ -515,78 +519,89 @@ test('GET /aggregate with restriction returns filtered collections', async (t) =
 
   const collectionId = 'landsat-8-l1'
 
-  let response = null
-
-  // validate how many items we have total without restricting collections
-  response = await t.context.api.client.get(
-    'aggregate',
-    {
-      searchParams: new URLSearchParams({
-        aggregations: ['total_count']
-      }),
-      resolveBodyOnly: false,
-    }
-  )
-
-  t.is(response.statusCode, 200)
-  t.deepEqual(response.body.aggregations, [{
-    name: 'total_count',
-    data_type: 'integer',
-    value: 4
-  }])
-
-  // get the counts for collectionId without restrictions
-  response = await t.context.api.client.get(
-    'aggregate',
-    {
-      searchParams: new URLSearchParams({
-        aggregations: ['total_count'],
-        collections: [collectionId]
-      }),
-      resolveBodyOnly: false,
-    }
-  )
-
-  t.is(response.statusCode, 200)
-  t.deepEqual(response.body.aggregations, [{
-    name: 'total_count',
-    data_type: 'integer',
-    value: 2
-  }])
-
-  // restrict collections to include the one we just got 2 results for
-
-  const response2 = await t.context.api.client.get(
-    'aggregate',
-    {
-      searchParams: new URLSearchParams({
-        aggregations: ['total_count'],
-        _collections: [collectionId, 'foo', 'bar']
-      }),
-      resolveBodyOnly: false,
-    }
-  )
+  { // _collections not included
+    const r = await t.context.api.client.get(
+      'aggregate',
+      {
+        searchParams: new URLSearchParams({
+          aggregations: ['total_count']
+        }),
+        resolveBodyOnly: false,
+      }
+    )
 
-  t.is(response.statusCode, 200)
-  t.deepEqual(response2.body.aggregations, response.body.aggregations)
+    t.is(r.statusCode, 200)
+    t.deepEqual(r.body.aggregations, [])
+  }
 
-  // restrict collections to a non-existent one with no items, so should be 0 results
-  response = await t.context.api.client.get(
-    'aggregate',
-    {
-      searchParams: new URLSearchParams({
-        aggregations: ['total_count'],
-        collections: [collectionId],
-        _collections: ['not-a-collection']
-      }),
-      resolveBodyOnly: false,
-    }
-  )
+  { // _collections is empty
+    const r = await t.context.api.client.get(
+      'aggregate',
+      {
+        searchParams: new URLSearchParams({
+          aggregations: ['total_count'],
+          collections: []
+        }),
+        resolveBodyOnly: false,
+      }
+    )
+    t.is(r.statusCode, 200)
+    t.deepEqual(r.body.aggregations, [])
+  }
+
+  { // _collections is * for admin -- get the counts for  without restrictions
+    const r = await t.context.api.client.get(
+      'aggregate',
+      {
+        searchParams: new URLSearchParams({
+          aggregations: ['total_count'],
+          _collections: ['*']
+        }),
+        resolveBodyOnly: false,
+      }
+    )
+    t.is(r.statusCode, 200)
+    t.deepEqual(r.body.aggregations, [{
+      name: 'total_count',
+      data_type: 'integer',
+      value: 4
+    }])
+  }
+
+  { // restrict collections to include the one we just got 2 results for
+    const r = await t.context.api.client.get(
+      'aggregate',
+      {
+        searchParams: new URLSearchParams({
+          aggregations: ['total_count'],
+          _collections: [collectionId, 'foo', 'bar']
+        }),
+        resolveBodyOnly: false,
+      }
+    )
+
+    t.is(r.statusCode, 200)
+    t.deepEqual(r.body.aggregations, [{
+      name: 'total_count',
+      data_type: 'integer',
+      value: 2
+    }])
+  }
+
+  { // restrict collections to a non-existent one with no items, so should be 0 results
+    const r = await t.context.api.client.get(
+      'aggregate',
+      {
+        searchParams: new URLSearchParams({
+          aggregations: ['total_count'],
+          collections: [collectionId],
+          _collections: ['not-a-collection']
+        }),
+        resolveBodyOnly: false,
+      }
+    )
 
-  t.is(response.statusCode, 200)
-  t.deepEqual(response.body.aggregations, [{
-    name: 'total_count',
-    data_type: 'integer',
-    value: 0
-  }])
+    t.is(r.statusCode, 200)
+    t.deepEqual(r.body.aggregations, [])
+  }
 })