>>=y=v>>>24,p-=y,!(16&(y=v>>>16&255))){if(0==(64&y)){v=_[(65535&v)+(d&(1<>>=y,p-=y,(y=s-a)>3,d&=(1<<(p-=w<<3))-1,e.next_in=n,e.next_out=s,e.avail_in=n>>24&255)+(e>>>8&65280)+((65280&e)<<8)+((255&e)<<24)}function s(){this.mode=0,this.last=!1,this.wrap=0,this.havedict=!1,this.flags=0,this.dmax=0,this.check=0,this.total=0,this.head=null,this.wbits=0,this.wsize=0,this.whave=0,this.wnext=0,this.window=null,this.hold=0,this.bits=0,this.length=0,this.offset=0,this.extra=0,this.lencode=null,this.distcode=null,this.lenbits=0,this.distbits=0,this.ncode=0,this.nlen=0,this.ndist=0,this.have=0,this.next=null,this.lens=new I.Buf16(320),this.work=new I.Buf16(288),this.lendyn=null,this.distdyn=null,this.sane=0,this.back=0,this.was=0}function a(e){var t;return e&&e.state?(t=e.state,e.total_in=e.total_out=t.total=0,e.msg="",t.wrap&&(e.adler=1&t.wrap),t.mode=P,t.last=0,t.havedict=0,t.dmax=32768,t.head=null,t.hold=0,t.bits=0,t.lencode=t.lendyn=new I.Buf32(n),t.distcode=t.distdyn=new I.Buf32(i),t.sane=1,t.back=-1,N):U}function o(e){var t;return e&&e.state?((t=e.state).wsize=0,t.whave=0,t.wnext=0,a(e)):U}function h(e,t){var r,n;return e&&e.state?(n=e.state,t<0?(r=0,t=-t):(r=1+(t>>4),t<48&&(t&=15)),t&&(t<8||15=s.wsize?(I.arraySet(s.window,t,r-s.wsize,s.wsize,0),s.wnext=0,s.whave=s.wsize):(n<(i=s.wsize-s.wnext)&&(i=n),I.arraySet(s.window,t,r-n,i,s.wnext),(n-=i)?(I.arraySet(s.window,t,r-n,n,0),s.wnext=n,s.whave=s.wsize):(s.wnext+=i,s.wnext===s.wsize&&(s.wnext=0),s.whave>>8&255,r.check=B(r.check,E,2,0),l=u=0,r.mode=2;break}if(r.flags=0,r.head&&(r.head.done=!1),!(1&r.wrap)||(((255&u)<<8)+(u>>8))%31){e.msg="incorrect header check",r.mode=30;break}if(8!=(15&u)){e.msg="unknown compression method",r.mode=30;break}if(l-=4,k=8+(15&(u>>>=4)),0===r.wbits)r.wbits=k;else if(k>r.wbits){e.msg="invalid window size",r.mode=30;break}r.dmax=1<>8&1),512&r.flags&&(E[0]=255&u,E[1]=u>>>8&255,r.check=B(r.check,E,2,0)),l=u=0,r.mode=3;case 3:for(;l<32;){if(0===o)break e;o--,u+=n[s++]<>>8&255,E[2]=u>>>16&255,E[3]=u>>>24&255,r.check=B(r.check,E,4,0)),l=u=0,r.mode=4;case 4:for(;l<16;){if(0===o)break e;o--,u+=n[s++]<>8),512&r.flags&&(E[0]=255&u,E[1]=u>>>8&255,r.check=B(r.check,E,2,0)),l=u=0,r.mode=5;case 5:if(1024&r.flags){for(;l<16;){if(0===o)break e;o--,u+=n[s++]<>>8&255,r.check=B(r.check,E,2,0)),l=u=0}else r.head&&(r.head.extra=null);r.mode=6;case 6:if(1024&r.flags&&(o<(d=r.length)&&(d=o),d&&(r.head&&(k=r.head.extra_len-r.length,r.head.extra||(r.head.extra=new Array(r.head.extra_len)),I.arraySet(r.head.extra,n,s,d,k)),512&r.flags&&(r.check=B(r.check,n,d,s)),o-=d,s+=d,r.length-=d),r.length))break e;r.length=0,r.mode=7;case 7:if(2048&r.flags){if(0===o)break e;for(d=0;k=n[s+d++],r.head&&k&&r.length<65536&&(r.head.name+=String.fromCharCode(k)),k&&d>9&1,r.head.done=!0),e.adler=r.check=0,r.mode=12;break;case 10:for(;l<32;){if(0===o)break e;o--,u+=n[s++]<>>=7&l,l-=7&l,r.mode=27;break}for(;l<3;){if(0===o)break e;o--,u+=n[s++]<>>=1)){case 0:r.mode=14;break;case 1:if(j(r),r.mode=20,6!==t)break;u>>>=2,l-=2;break e;case 2:r.mode=17;break;case 3:e.msg="invalid block type",r.mode=30}u>>>=2,l-=2;break;case 14:for(u>>>=7&l,l-=7&l;l<32;){if(0===o)break e;o--,u+=n[s++]<>>16^65535)){e.msg="invalid stored block lengths",r.mode=30;break}if(r.length=65535&u,l=u=0,r.mode=15,6===t)break e;case 15:r.mode=16;case 16:if(d=r.length){if(o>>=5,l-=5,r.ndist=1+(31&u),u>>>=5,l-=5,r.ncode=4+(15&u),u>>>=4,l-=4,286>>=3,l-=3}for(;r.have<19;)r.lens[A[r.have++]]=0;if(r.lencode=r.lendyn,r.lenbits=7,S={bits:r.lenbits},x=T(0,r.lens,0,19,r.lencode,0,r.work,S),r.lenbits=S.bits,x){e.msg="invalid code lengths set",r.mode=30;break}r.have=0,r.mode=19;case 19:for(;r.have>>16&255,b=65535&C,!((_=C>>>24)<=l);){if(0===o)break e;o--,u+=n[s++]<>>=_,l-=_,r.lens[r.have++]=b;else{if(16===b){for(z=_+2;l>>=_,l-=_,0===r.have){e.msg="invalid bit length repeat",r.mode=30;break}k=r.lens[r.have-1],d=3+(3&u),u>>>=2,l-=2}else if(17===b){for(z=_+3;l>>=_)),u>>>=3,l-=3}else{for(z=_+7;l>>=_)),u>>>=7,l-=7}if(r.have+d>r.nlen+r.ndist){e.msg="invalid bit length repeat",r.mode=30;break}for(;d--;)r.lens[r.have++]=k}}if(30===r.mode)break;if(0===r.lens[256]){e.msg="invalid code -- missing end-of-block",r.mode=30;break}if(r.lenbits=9,S={bits:r.lenbits},x=T(D,r.lens,0,r.nlen,r.lencode,0,r.work,S),r.lenbits=S.bits,x){e.msg="invalid literal/lengths set",r.mode=30;break}if(r.distbits=6,r.distcode=r.distdyn,S={bits:r.distbits},x=T(F,r.lens,r.nlen,r.ndist,r.distcode,0,r.work,S),r.distbits=S.bits,x){e.msg="invalid distances set",r.mode=30;break}if(r.mode=20,6===t)break e;case 20:r.mode=21;case 21:if(6<=o&&258<=h){e.next_out=a,e.avail_out=h,e.next_in=s,e.avail_in=o,r.hold=u,r.bits=l,R(e,c),a=e.next_out,i=e.output,h=e.avail_out,s=e.next_in,n=e.input,o=e.avail_in,u=r.hold,l=r.bits,12===r.mode&&(r.back=-1);break}for(r.back=0;g=(C=r.lencode[u&(1<>>16&255,b=65535&C,!((_=C>>>24)<=l);){if(0===o)break e;o--,u+=n[s++]<>v)])>>>16&255,b=65535&C,!(v+(_=C>>>24)<=l);){if(0===o)break e;o--,u+=n[s++]<>>=v,l-=v,r.back+=v}if(u>>>=_,l-=_,r.back+=_,r.length=b,0===g){r.mode=26;break}if(32&g){r.back=-1,r.mode=12;break}if(64&g){e.msg="invalid literal/length code",r.mode=30;break}r.extra=15&g,r.mode=22;case 22:if(r.extra){for(z=r.extra;l>>=r.extra,l-=r.extra,r.back+=r.extra}r.was=r.length,r.mode=23;case 23:for(;g=(C=r.distcode[u&(1<>>16&255,b=65535&C,!((_=C>>>24)<=l);){if(0===o)break e;o--,u+=n[s++]<>v)])>>>16&255,b=65535&C,!(v+(_=C>>>24)<=l);){if(0===o)break e;o--,u+=n[s++]<>>=v,l-=v,r.back+=v}if(u>>>=_,l-=_,r.back+=_,64&g){e.msg="invalid distance code",r.mode=30;break}r.offset=b,r.extra=15&g,r.mode=24;case 24:if(r.extra){for(z=r.extra;l>>=r.extra,l-=r.extra,r.back+=r.extra}if(r.offset>r.dmax){e.msg="invalid distance too far back",r.mode=30;break}r.mode=25;case 25:if(0===h)break e;if(d=c-h,r.offset>d){if((d=r.offset-d)>r.whave&&r.sane){e.msg="invalid distance too far back",r.mode=30;break}p=d>r.wnext?(d-=r.wnext,r.wsize-d):r.wnext-d,d>r.length&&(d=r.length),m=r.window}else m=i,p=a-r.offset,d=r.length;for(hd?(m=R[T+a[v]],A[I+a[v]]):(m=96,0),h=1<>S)+(u-=h)]=p<<24|m<<16|_|0,0!==u;);for(h=1<>=1;if(0!==h?(E&=h-1,E+=h):E=0,v++,0==--O[b]){if(b===w)break;b=t[r+a[v]]}if(k>>7)]}function U(e,t){e.pending_buf[e.pending++]=255&t,e.pending_buf[e.pending++]=t>>>8&255}function P(e,t,r){e.bi_valid>d-r?(e.bi_buf|=t<>d-e.bi_valid,e.bi_valid+=r-d):(e.bi_buf|=t<>>=1,r<<=1,0<--t;);return r>>>1}function Z(e,t,r){var n,i,s=new Array(g+1),a=0;for(n=1;n<=g;n++)s[n]=a=a+r[n-1]<<1;for(i=0;i<=t;i++){var o=e[2*i+1];0!==o&&(e[2*i]=j(s[o]++,o))}}function W(e){var t;for(t=0;t>1;1<=r;r--)G(e,s,r);for(i=h;r=e.heap[1],e.heap[1]=e.heap[e.heap_len--],G(e,s,1),n=e.heap[1],e.heap[--e.heap_max]=r,e.heap[--e.heap_max]=n,s[2*i]=s[2*r]+s[2*n],e.depth[i]=(e.depth[r]>=e.depth[n]?e.depth[r]:e.depth[n])+1,s[2*r+1]=s[2*n+1]=i,e.heap[1]=i++,G(e,s,1),2<=e.heap_len;);e.heap[--e.heap_max]=e.heap[1],function(e,t){var r,n,i,s,a,o,h=t.dyn_tree,u=t.max_code,l=t.stat_desc.static_tree,f=t.stat_desc.has_stree,c=t.stat_desc.extra_bits,d=t.stat_desc.extra_base,p=t.stat_desc.max_length,m=0;for(s=0;s<=g;s++)e.bl_count[s]=0;for(h[2*e.heap[e.heap_max]+1]=0,r=e.heap_max+1;r<_;r++)p<(s=h[2*h[2*(n=e.heap[r])+1]+1]+1)&&(s=p,m++),h[2*n+1]=s,u>=7;n>>=1)if(1&r&&0!==e.dyn_ltree[2*t])return o;if(0!==e.dyn_ltree[18]||0!==e.dyn_ltree[20]||0!==e.dyn_ltree[26])return h;for(t=32;t>>3,(s=e.static_len+3+7>>>3)<=i&&(i=s)):i=s=r+5,r+4<=i&&-1!==t?J(e,t,r,n):4===e.strategy||s===i?(P(e,2+(n?1:0),3),K(e,z,C)):(P(e,4+(n?1:0),3),function(e,t,r,n){var i;for(P(e,t-257,5),P(e,r-1,5),P(e,n-4,4),i=0;i>>8&255,e.pending_buf[e.d_buf+2*e.last_lit+1]=255&t,e.pending_buf[e.l_buf+e.last_lit]=255&r,e.last_lit++,0===t?e.dyn_ltree[2*r]++:(e.matches++,t--,e.dyn_ltree[2*(A[r]+u+1)]++,e.dyn_dtree[2*N(t)]++),e.last_lit===e.lit_bufsize-1},r._tr_align=function(e){P(e,2,3),L(e,m,z),function(e){16===e.bi_valid?(U(e,e.bi_buf),e.bi_buf=0,e.bi_valid=0):8<=e.bi_valid&&(e.pending_buf[e.pending++]=255&e.bi_buf,e.bi_buf>>=8,e.bi_valid-=8)}(e)}},{"../utils/common":41}],53:[function(e,t,r){"use strict";t.exports=function(){this.input=null,this.next_in=0,this.avail_in=0,this.total_in=0,this.output=null,this.next_out=0,this.avail_out=0,this.total_out=0,this.msg="",this.state=null,this.data_type=2,this.adler=0}},{}],54:[function(e,t,r){(function(e){!function(r,n){"use strict";if(!r.setImmediate){var i,s,t,a,o=1,h={},u=!1,l=r.document,e=Object.getPrototypeOf&&Object.getPrototypeOf(r);e=e&&e.setTimeout?e:r,i="[object process]"==={}.toString.call(r.process)?function(e){process.nextTick(function(){c(e)})}:function(){if(r.postMessage&&!r.importScripts){var e=!0,t=r.onmessage;return r.onmessage=function(){e=!1},r.postMessage("","*"),r.onmessage=t,e}}()?(a="setImmediate$"+Math.random()+"$",r.addEventListener?r.addEventListener("message",d,!1):r.attachEvent("onmessage",d),function(e){r.postMessage(a+e,"*")}):r.MessageChannel?((t=new MessageChannel).port1.onmessage=function(e){c(e.data)},function(e){t.port2.postMessage(e)}):l&&"onreadystatechange"in l.createElement("script")?(s=l.documentElement,function(e){var t=l.createElement("script");t.onreadystatechange=function(){c(e),t.onreadystatechange=null,s.removeChild(t),t=null},s.appendChild(t)}):function(e){setTimeout(c,0,e)},e.setImmediate=function(e){"function"!=typeof e&&(e=new Function(""+e));for(var t=new Array(arguments.length-1),r=0;r Error Fetching user data`
throw(e)
}
- if (curUser.username !== undefined) {
- loadApp();
- } else {
- el.innerHTML += ` No account for ${window.oidcWorker.token}`
- }
+ loadApp();
}
catch (e) {
- el.innerHTML += ` `
+ const ta = document.createElement('textarea')
+ ta.className = 'sm-bootstrap-error'
+ ta.setAttribute('wrap', 'off')
+ ta.rows = 24
+ ta.cols = 80
+ ta.style.fontSize = '10px'
+ ta.readOnly = true
+ ta.value = JSON.stringify(STIGMAN.serializeError(e), null, 2)
+ el.appendChild(document.createElement('br'))
+ el.appendChild(document.createElement('br'))
+ el.appendChild(ta)
}
}
@@ -269,7 +275,7 @@ async function loadApp () {
}
catch (e) {
- Ext.get( 'indicator' ).dom.innerHTML = e.message
+ Ext.get( 'indicator' ).dom.innerHTML = SM.he(e.message)
}
} //end loadApp()
diff --git a/client/src/js/workers/oidc-worker.js b/client/src/js/workers/oidc-worker.js
index 4c22c6186..e0edba7a5 100644
--- a/client/src/js/workers/oidc-worker.js
+++ b/client/src/js/workers/oidc-worker.js
@@ -79,6 +79,10 @@ async function exchangeCodeForToken({ code, codeVerifier, clientId = ENV.clientI
async function initialize(options) {
if (!initialized) {
initialized = true
+ const parsedRedirectUri = new URL(options.redirectUri)
+ if (!parsedRedirectUri.protocol.startsWith('http')) {
+ return { success: false, error: `Invalid redirectUri scheme: ${parsedRedirectUri.protocol}` }
+ }
redirectUri = options.redirectUri
ENV = options.env || null
@@ -108,10 +112,10 @@ async function getStatus() {
}
function logout() {
- return {
- success: true,
- redirect: oidcConfiguration.end_session_endpoint
+ if (!oidcConfiguration.end_session_endpoint) {
+ return { success: false, error: 'Logout not available' }
}
+ return { success: true, redirect: oidcConfiguration.end_session_endpoint }
}
async function onMessage(e) {
@@ -201,8 +205,20 @@ function validateOidcConfiguration() {
} else if (ENV.strictPkce && !oidcConfiguration.code_challenge_methods_supported?.includes('S256')) {
result.success = false
result.error = 'OP does not advertise PKCE and STIGMAN_CLIENT_STRICT_PKCE=true'
+ } else if (oidcConfiguration.end_session_endpoint) {
+ try {
+ const parsed = new URL(oidcConfiguration.end_session_endpoint)
+ if (!parsed.protocol.startsWith('http')) {
+ console.warn(logPrefix, 'end_session_endpoint has invalid scheme, logout will be unavailable:', oidcConfiguration.end_session_endpoint)
+ oidcConfiguration.end_session_endpoint = null
+ }
+ }
+ catch {
+ console.warn(logPrefix, 'end_session_endpoint is not a valid URL, logout will be unavailable:', oidcConfiguration.end_session_endpoint)
+ oidcConfiguration.end_session_endpoint = null
+ }
}
- return result
+ return result
}
function getScopeStr() {
diff --git a/docs/installation-and-setup/data-and-permissions.rst b/docs/installation-and-setup/data-and-permissions.rst
index 91d94a130..c633804b9 100644
--- a/docs/installation-and-setup/data-and-permissions.rst
+++ b/docs/installation-and-setup/data-and-permissions.rst
@@ -174,7 +174,21 @@ STIG Manager recognizes two "privileges" that can be granted to users via config
Users with the **create_collection** privilege can create new Collections of their own, but are otherwise ordinary users.
-Users with the **admin** privilege must explicitly invoke the "elevate" parameter in queries to the API to make use of their privilege. In our reference UI, this parameter is sent when certain "Application Management" functions are invoked, such as importing new Reference STIGs, requesting a list of all Collections, or creating a new Grant in a Collection they do not otherwise have access to.
+Users with the **admin** privilege may explicitly invoke the ``elevate`` parameter in API requests to act as a privileged principal. The elevation mechanism is designed so that an admin user does not need a separate privileged account on the identity provider — the same account is used, and the user opts into elevated mode on a per-request basis.
+
+When a request includes ``?elevate=true``, it is governed by the elevation access model rather than by any Collection Grant the user may also hold. Elevation is scoped exclusively to Collection management and application administration operations:
+
+- Enumerate, create, and delete Collections
+- Read and modify a Collection's name and description
+- Create, modify, and delete Grants on any Collection, assigning any Role to any User or User Group (without supplying an ACL)
+- Manage Users and User Groups
+
+Elevation does **not** grant access to collection content. An elevated admin cannot read or write Reviews, access Asset or STIG checklist data, or modify a Collection's settings, labels, metadata, or Grant ACLs — even with ``?elevate=true`` supplied. These operations require a Collection Grant and are performed via normal (non-elevated) requests.
+
+In the reference UI, the ``elevate`` parameter is sent when "Application Management" functions are invoked, such as importing new Reference STIGs, listing all Collections, or creating a Grant in a Collection the admin does not otherwise have access to.
+
+.. note::
+ An elevated admin can create a Grant giving themselves any Role in any Collection. This is intentional: it avoids requiring admins who also need content access to maintain a second OIDC account. The accepted control is that **every elevated request — including self-grant operations — has its complete request and response bodies written to the application log**, regardless of whether the request succeeds. Administrators responsible for deploying STIG Manager should ensure elevated-request log entries are retained and reviewed.
These **privileges** must be present in the token presented to the API in order to be successfully invoked.
diff --git a/docs/installation-and-setup/envvars.csv b/docs/installation-and-setup/envvars.csv
index c6319ddaf..2bc135405 100644
--- a/docs/installation-and-setup/envvars.csv
+++ b/docs/installation-and-setup/envvars.csv
@@ -15,8 +15,12 @@
| If necessary, the passphrase that decrypts the PEM encoded Server private key used for TLS. Additionally requires setting ``STIGMAN_API_TLS_CERT_FILE`` to enable TLS.","API"
"STIGMAN_CLASSIFICATION","| **Default** ``U``
| Sets the classification banner, if any. Available values: ``NONE`` ``U`` ``CUI`` ``C`` ``S`` ``TS`` ``SCI`` ","API, Client"
+"STIGMAN_CLIENT_ADMIN_TIMEOUT","| **Default** ``0``
+| The maximum time (in minutes) a user with admin privileges can be inactive in the web client before discarding their access token and requiring reauthorization. Activity is defined as mouse click, keypress, or scrolling in any tab or window of a same-origin browsing context group. Set to zero to disable idle detection.","Client"
"STIGMAN_CLIENT_API_BASE","| **Default** ``api``
| The base URL for Client requests to the API relative to ``window.location`` ","Client"
+"STIGMAN_CLIENT_CONSOLE_MODE","| **Default** ``production``
+| The console mode of the web client, setting to ``development`` enables console logging which is otherwise disabled","Client"
"STIGMAN_CLIENT_DIRECTORY","| **Default** ``./clients``
| The location of the web client files, relative to the API source directory. Note that if running source from a clone of the GitHub repository, the client is located at `../../clients` relative to the API directory. ","API, Client"
"STIGMAN_CLIENT_DISABLED","| **Default** ``false``
@@ -27,8 +31,6 @@
| A space separated list of OAuth2 scopes to request in addition to ``stig-manager:stig`` ``stig-manager:stig:read`` ``stig-manager:collection`` ``stig-manager:user`` ``stig-manager:user:read`` ``stig-manager:op``. Some OIDC providers (Okta) generate a refresh token only if the scope ``offline_access`` is requested","Client"
"STIGMAN_CLIENT_ID","| **Default** ``stig-manager``
| The OIDC clientId of the web client","Client"
-"STIGMAN_CLIENT_ADMIN_TIMEOUT","| **Default** ``0``
-| The maximum time (in minutes) a user with admin privileges can be inactive in the web client before discarding their access token and requiring reauthorization. Activity is defined as mouse click, keypress, or scrolling in any tab or window of a same-origin browsing context group. Set to zero to disable idle detection.","Client"
"STIGMAN_CLIENT_OIDC_PROVIDER","| **Default** Value of ``STIGMAN_OIDC_PROVIDER``
| Client override of the base URL of the OIDC provider issuing signed JWTs for the API. The string ``/.well-known/openid-configuration`` will be appended by the client when fetching metadata.","Client "
"STIGMAN_CLIENT_REAUTH_ACTION","| **Default** ``popup``
diff --git a/docs/reference/index.rst b/docs/reference/index.rst
index 84b375adf..cf26c6ece 100644
--- a/docs/reference/index.rst
+++ b/docs/reference/index.rst
@@ -119,14 +119,15 @@ This is a glossary with definitions for terms like :term:`Asset`:
For each Collection they are granted access to, Users can have one of 4 :term:`Roles ` , providing different capabilities and default access to your Collection. See :ref:`roles-and-access` for more information.
Users can also be given one of 2 **Privileges** on the STIG Manager system. These privileges can be administered in your Authentication Provider (such as Keycloak):
- * Collection Creator: Gives the User the ability to create their own Collections in STIG Manager.
- * Administrator (Application Manager): Gives the user elevated access to STIG Manager via the "Application Management" node of the Nav Tree. The Administrator Privilege allows the User to:
-
+ * Collection Creator: Gives the User the ability to create their own Collections in STIG Manager.
+ * Administrator (Application Manager): Gives the user the ability to invoke elevated access via the "Application Management" node of the Nav Tree. The Administrator Privilege allows the User to:
+
* Import new STIGs into STIG Manager, as well as Delete them.
- * Create and Alter Collections, and view their metadata.
- * Create and Alter Users, and view their metadata.
- * Import and Export Application Data. An experimental feature that will export all the Collection data in STIG Manager
- * The Administrator privilege does not by itself provide access to any Collection, however, they can Grant themselves access to any Collection in STIG Manager via the Application Manager interface.
+ * Enumerate, Create, and Delete Collections, and view a Collection's name and description.
+ * Create and modify Grants (without ACLs) on any Collection.
+ * Create and Alter Users and User Groups.
+ * Import and Export Application Data.
+ * The Administrator privilege does not grant access to collection content. An admin cannot read or write Reviews, access Asset or STIG checklist data, or modify a Collection's settings, labels, metadata, or Grant ACLs without holding a Collection Grant. These operations require a normal (non-elevated) request governed by a Grant.
User Group
A named collection of Users that can be granted access to a Collection as a single entity. User Groups can be created and modified in the User Groups interface available to Application Managers. User Groups are available to all Collection Owners and Managers for use in the Grants panel. See :ref:`roles-and-access` for more information.
diff --git a/test/api/mocha/data/collection/collectionPost.test.js b/test/api/mocha/data/collection/collectionPost.test.js
index 051df3de5..b93d60825 100644
--- a/test/api/mocha/data/collection/collectionPost.test.js
+++ b/test/api/mocha/data/collection/collectionPost.test.js
@@ -42,7 +42,7 @@ describe('POST - Collection - not all tests run for all iterations', function ()
it("Create a Collection and test projections",async function () {
const post = JSON.parse(JSON.stringify(requestBodies.createCollection))
post.name = "testCollection" + random
- const res = await utils.executeRequest(`${config.baseUrl}/collections?elevate=${distinct.canElevate}&projection=grants&projection=labels&projection=assets&projection=owners&projection=statistics&projection=stigs`, 'POST', iteration.token, post)
+ const res = await utils.executeRequest(`${config.baseUrl}/collections?projection=grants&projection=labels&projection=assets&projection=owners&projection=statistics&projection=stigs`, 'POST', iteration.token, post)
if(distinct.canCreateCollection === false){
expect(res.status).to.eql(403)
return
@@ -138,7 +138,7 @@ describe('POST - Collection - not all tests run for all iterations', function ()
}
const post = JSON.parse(JSON.stringify(requestBodies.collectionWithNoSettings))
post.name = post.name + random
- const res = await utils.executeRequest(`${config.baseUrl}/collections?elevate=${distinct.canElevate}&projection=grants&projection=labels&projection=assets&projection=owners&projection=statistics&projection=stigs`, 'POST', iteration.token, post)
+ const res = await utils.executeRequest(`${config.baseUrl}/collections?projection=grants&projection=labels&projection=assets&projection=owners&projection=statistics&projection=stigs`, 'POST', iteration.token, post)
if(distinct.canCreateCollection === false){
expect(res.status).to.eql(403)
return
@@ -197,7 +197,7 @@ describe('POST - Collection - not all tests run for all iterations', function ()
maxReviews: 10
},
}
- const res = await utils.executeRequest(`${config.baseUrl}/collections?elevate=${distinct.canElevate}&projection=grants&projection=labels&projection=assets&projection=owners&projection=statistics&projection=stigs`, 'POST', iteration.token, post)
+ const res = await utils.executeRequest(`${config.baseUrl}/collections?projection=grants&projection=labels&projection=assets&projection=owners&projection=statistics&projection=stigs`, 'POST', iteration.token, post)
if(distinct.canCreateCollection === false){
expect(res.status).to.eql(403)
return
@@ -221,7 +221,7 @@ describe('POST - Collection - not all tests run for all iterations', function ()
const post = requestBodies.createCollectionWithTestGroup
let uuid = uuidv4().slice(0, 10)
post.name = "testCollection" + uuid
- const res = await utils.executeRequest(`${config.baseUrl}/collections?elevate=${distinct.canElevate}&projection=grants`, 'POST', iteration.token, post)
+ const res = await utils.executeRequest(`${config.baseUrl}/collections?projection=grants`, 'POST', iteration.token, post)
if(distinct.canCreateCollection === false){
expect(res.status).to.eql(403)
return
@@ -236,7 +236,7 @@ describe('POST - Collection - not all tests run for all iterations', function ()
const post = JSON.parse(JSON.stringify(requestBodies.createCollection))
post.grants.push(post.grants[0])
post.name = "TEST" + utils.getUUIDSubString()
- const res = await utils.executeRequest(`${config.baseUrl}/collections?elevate=${distinct.canElevate}`, 'POST', iteration.token, post)
+ const res = await utils.executeRequest(`${config.baseUrl}/collections`, 'POST', iteration.token, post)
if(distinct.canCreateCollection === false){
expect(res.status).to.eql(403)
return
@@ -248,7 +248,7 @@ describe('POST - Collection - not all tests run for all iterations', function ()
it("should throw SmError.UnprocessableError due to duplicate name exists ",async function () {
const post = JSON.parse(JSON.stringify(requestBodies.createCollection))
post.name = "testCollection" + random
- const res = await utils.executeRequest(`${config.baseUrl}/collections?elevate=${distinct.canElevate}&projection=grants&projection=labels&projection=assets&projection=owners&projection=statistics&projection=stigs`, 'POST', iteration.token, post)
+ const res = await utils.executeRequest(`${config.baseUrl}/collections?projection=grants&projection=labels&projection=assets&projection=owners&projection=statistics&projection=stigs`, 'POST', iteration.token, post)
if(distinct.canCreateCollection === false){
expect(res.status).to.eql(403)
return
diff --git a/test/api/mocha/integration/collection.test.js b/test/api/mocha/integration/collection.test.js
index c7dc6d3ee..916050468 100644
--- a/test/api/mocha/integration/collection.test.js
+++ b/test/api/mocha/integration/collection.test.js
@@ -1064,7 +1064,7 @@ describe('POST - exportToCollection - /collections/{collectionId}/export-to/{dst
let exportedAssetResults
let exportedAssetStatuses
it('Merge provided properties with a Collection Copy', async () => {
- const res = await utils.executeRequest(`${config.baseUrl}/collections/${reference.scrapCollection.collectionId}?elevate=true`, 'PATCH', user.token, {
+ const res = await utils.executeRequest(`${config.baseUrl}/collections/${reference.scrapCollection.collectionId}`, 'PATCH', user.token, {
"metadata": {
"pocName": "poc2Patched",
"pocEmail": "pocEmail@email.com",
@@ -1260,7 +1260,7 @@ describe('POST - exportToCollection - /collections/{collectionId}/export-to/{dst
expect(res.body[0].metrics.statuses, "comparing source asset to exported asset statuses").to.eql(expectedStatuses);
})
it('Merge provided properties with a Collection Copy 2', async () => {
- const res = await utils.executeRequest(`${config.baseUrl}/collections/${reference.scrapCollection.collectionId}?elevate=true`, 'PATCH', user.token, {
+ const res = await utils.executeRequest(`${config.baseUrl}/collections/${reference.scrapCollection.collectionId}`, 'PATCH', user.token, {
"metadata": {
"pocName": "poc2Patched",
"pocEmail": "pocEmail@email.com",
@@ -1718,12 +1718,6 @@ describe('PUT - setStigAssetsByCollectionUser - /collections/{collectionId}/gran
it('Add restricted user to collection Y', async () => {
const res = await utils.executeRequest(`${config.baseUrl}/collections/83?elevate=true&projection=grants`, 'PATCH', user.token, {
- "metadata": {
- "pocName": "poc2Patched",
- "pocEmail": "pocEmail@email.com",
- "pocPhone": "12342",
- "reqRar": "true"
- },
"grants": [
{
"userId": "87",
diff --git a/test/api/mocha/integration/deleteHandling.test.js b/test/api/mocha/integration/deleteHandling.test.js
index c9b5b2488..36567187a 100644
--- a/test/api/mocha/integration/deleteHandling.test.js
+++ b/test/api/mocha/integration/deleteHandling.test.js
@@ -23,7 +23,7 @@ describe('DELETE - deleteAsset - /assets/{assetId} - DELETE - deleteCollection -
let deletedCollection = null
it('Create a Collection in order to delete it', async () => {
- const res = await utils.executeRequest(`${config.baseUrl}/collections?elevate=true&projection=grants&projection=labels`, 'POST', user.token, {
+ const res = await utils.executeRequest(`${config.baseUrl}/collections?projection=grants&projection=labels`, 'POST', user.token, {
"name": "TEST_"+ utils.getUUIDSubString(),
"description": "Collection TEST description",
"settings": {
diff --git a/test/api/mocha/security/reviewCrossCollectionWrite.test.js b/test/api/mocha/security/reviewCrossCollectionWrite.test.js
new file mode 100644
index 000000000..89a85d235
--- /dev/null
+++ b/test/api/mocha/security/reviewCrossCollectionWrite.test.js
@@ -0,0 +1,460 @@
+/**
+ * Security Regression Tests: Unauthorized Cross-Collection Review Write (Finding 1)
+ *
+ * VULNERABILITY SUMMARY
+ * ---------------------
+ * postReviewsByAsset (POST /collections/{collectionId}/reviews/{assetId})
+ * putReviewByAssetRule (PUT /collections/{collectionId}/reviews/{assetId}/{ruleId})
+ * patchReviewByAssetRule (PATCH /collections/{collectionId}/reviews/{assetId}/{ruleId})
+ *
+ * All three handlers verify that the caller holds a grant on the collectionId in the
+ * URL path, and that the assetId exists. None verify that the asset belongs to that
+ * collection.
+ *
+ * ReviewService.putReviewsByAsset builds cteGrant with:
+ * WHERE a.assetId = @assetId
+ * with no AND a.collectionId = @collectionId predicate (ReviewService.js:1000-1011).
+ * For non-Restricted callers (roleId > 1) the ACL join in cteGrant is a LEFT JOIN,
+ * so it returns rules for any asset regardless of which collection owns it.
+ * The write succeeds: reviews are inserted or updated in the review table for the
+ * victim asset using the attacker's collection's validation settings.
+ *
+ * A secondary enabler in patchReviewByAssetRule: the pre-write existence check
+ * (Review.js:202-206) calls getReviews with filter: {assetId, ruleId} and no
+ * collectionId. This allows a review in the victim collection to satisfy the
+ * "review must exist to be patched" gate (Review.js:207), enabling the PATCH
+ * write path when the asset is in a foreign collection.
+ *
+ * ATTACK SCENARIO
+ * ---------------
+ * - Collection X (collectionId: 21) — attacker's collection; attacker (lvl2) has Full grant
+ * - Collection Y (collectionId: 83) — victim collection; attacker has NO grant
+ * - Asset 153 — belongs to Collection Y; has VPN_SRG_TEST STIG mapped and an
+ * existing submitted review for ruleId SV-106179r1_rule
+ * - Attacker — user "lvl2" (userId: 21), Full grant on Collection X (21) only;
+ * no grant on Collection Y (83)
+ *
+ * The attacker issues POST, PUT, or PATCH to a URL using Collection X's collectionId
+ * but Asset 153's assetId (which belongs to Collection Y).
+ *
+ * CORRECT BEHAVIOUR (after fix)
+ * ------------------------------
+ * After verifying the caller's grant on the URL collectionId, the API must verify
+ * that the assetId belongs to that collection. If it does not, the request must be
+ * rejected with 403 before any write occurs.
+ *
+ * HOW THESE TESTS FAIL TODAY / PASS AFTER FIX
+ * --------------------------------------------
+ * Today: POST, PUT, and PATCH all succeed (HTTP 200/201). The write-impact tests
+ * verify this by reading the victim asset's review via admin token after the
+ * attack and confirming the review was mutated — the test asserts it was NOT
+ * mutated, so it fails.
+ * After fix: the API returns 403, no write occurs, the admin read-back confirms the
+ * original review is unchanged, and all assertions pass.
+ */
+
+import { config } from '../testConfig.js'
+import * as utils from '../utils/testUtils.js'
+import reference from '../referenceData.js'
+import { expect } from 'chai'
+
+// ---------------------------------------------------------------------------
+// Actors
+// ---------------------------------------------------------------------------
+
+// The attacker: Full grant (roleId 2) on Collection X (21).
+// Has grants on collections 1 and 21 only — NO grant on Collection Y (83).
+const attacker = {
+ name: 'lvl2',
+ userId: '21',
+ token:
+ 'eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJGSjg2R2NGM2pUYk5MT2NvNE52WmtVQ0lVbWZZQ3FvcXRPUWVNZmJoTmxFIn0.eyJleHAiOjE4NjQ3MDkwNzQsImlhdCI6MTY3MDU2ODI3NSwiYXV0aF90aW1lIjoxNjcwNTY4Mjc0LCJqdGkiOiIwM2Y0OWVmYy1jYzcxLTQ3MTItOWFjNy0xNGY5YzZiNDc1ZGEiLCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjgwODAvYXV0aC9yZWFsbXMvc3RpZ21hbiIsImF1ZCI6WyJyZWFsbS1tYW5hZ2VtZW50IiwiYWNjb3VudCJdLCJzdWIiOiJjMTM3ZDYzNy1mMDU2LTRjNzItOWJlZi1lYzJhZjdjMWFiYzciLCJ0eXAiOiJCZWFyZXIiLCJhenAiOiJzdGlnLW1hbmFnZXIiLCJub25jZSI6IjQ5MzY5ZTdmLWEyZGYtNDkxYS04YjQ0LWEwNDJjYWYyMzhlYyIsInNlc3Npb25fc3RhdGUiOiJjNmUyZTgyNi0xMzMzLTRmMDctOTc4OC03OTQxMGM5ZjJkMDYiLCJhY3IiOiIwIiwicmVhbG1fYWNjZXNzIjp7InJvbGVzIjpbImRlZmF1bHQtcm9sZXMtc3RpZ21hbiJdfSwicmVzb3VyY2VfYWNjZXNzIjp7InJlYWxtLW1hbmFnZW1lbnQiOnsicm9sZXMiOlsidmlldy11c2VycyIsInF1ZXJ5LWdyb3VwcyIsInF1ZXJ5LXVzZXJzIl19LCJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLCJ2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6Im9wZW5pZCBzdGlnLW1hbmFnZXI6Y29sbGVjdGlvbiBzdGlnLW1hbmFnZXI6c3RpZzpyZWFkIHN0aWctbWFuYWdlcjp1c2VyOnJlYWQgc3RpZy1tYW5hZ2VyOmNvbGxlY3Rpb246cmVhZCIsInNpZCI6ImM2ZTJlODI2LTEzMzMtNGYwNy05Nzg4LTc5NDEwYzlmMmQwNiIsIm5hbWUiOiJsdmwyIiwicHJlZmVycmVkX3VzZXJuYW1lIjoibHZsMiIsImdpdmVuX25hbWUiOiJsdmwyIn0.F1i8VVLNkVsaW9i83vbVyB9eFiSxX_9ZpR6K7Zs0r7pKOCMJnSOHeKIHrlMO4hW8DrbmSRrkrrXExwNtw6zUsuH8_1uxx-SVUkaQyHEMfbx1_TstkTOFcjxIWqtlVvwPIt-DlTpQ_IFuby8wDAIxUvNwogn2OoybzAy1CDMcpIA'
+}
+
+// The restricted attacker: Restricted grant (roleId 1) on Collection X (21) via group membership
+// (userId=85, belongs to userGroupId=1 which holds grantId=32, roleId=1 on collectionId=21).
+// No grant on Collection Y (83).
+const restrictedAttacker = {
+ name: 'lvl1',
+ userId: '85',
+ token:
+ 'eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJGSjg2R2NGM2pUYk5MT2NvNE52WmtVQ0lVbWZZQ3FvcXRPUWVNZmJoTmxFIn0.eyJleHAiOjE4NjQ3MDg5ODQsImlhdCI6MTY3MDU2ODE4NCwiYXV0aF90aW1lIjoxNjcwNTY4MTg0LCJqdGkiOiIxMDhmMDc2MC0wYmY5LTRkZjEtYjE0My05NjgzNmJmYmMzNjMiLCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjgwODAvYXV0aC9yZWFsbXMvc3RpZ21hbiIsImF1ZCI6WyJyZWFsbS1tYW5hZ2VtZW50IiwiYWNjb3VudCJdLCJzdWIiOiJlM2FlMjdiOC1kYTIwLTRjNDItOWRmOC02MDg5ZjcwZjc2M2IiLCJ0eXAiOiJCZWFyZXIiLCJhenAiOiJzdGlnLW1hbmFnZXIiLCJub25jZSI6IjE0ZmE5ZDdkLTBmZTAtNDQyNi04ZmQ5LTY5ZDc0YTZmMzQ2NCIsInNlc3Npb25fc3RhdGUiOiJiNGEzYWNmMS05ZGM3LTQ1ZTEtOThmOC1kMzUzNjJhZWM0YzciLCJhY3IiOiIxIiwicmVhbG1fYWNjZXNzIjp7InJvbGVzIjpbImRlZmF1bHQtcm9sZXMtc3RpZ21hbiJdfSwicmVzb3VyY2VfYWNjZXNzIjp7InJlYWxtLW1hbmFnZW1lbnQiOnsicm9sZXMiOlsidmlldy11c2VycyIsInF1ZXJ5LWdyb3VwcyIsInF1ZXJ5LXVzZXJzIl19LCJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLCJ2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6Im9wZW5pZCBzdGlnLW1hbmFnZXI6Y29sbGVjdGlvbiBzdGlnLW1hbmFnZXI6c3RpZzpyZWFkIHN0aWctbWFuYWdlcjp1c2VyOnJlYWQgc3RpZy1tYW5hZ2VyOmNvbGxlY3Rpb246cmVhZCIsInNpZCI6ImI0YTNhY2YxLTlkYzctNDVlMS05OGY4LWQzNTM2MmFlYzRjNyIsIm5hbWUiOiJyZXN0cmljdGVkIiwicHJlZmVycmVkX3VzZXJuYW1lIjoibHZsMSIsImdpdmVuX25hbWUiOiJyZXN0cmljdGVkIn0.OqLARi5ILt3j2rMikXy0ECTTqjWco0-CrMwzE88gUv2i8rVO9kMgVsXbtPk2L2c9NNNujnxqg7QIr2_sqA51saTrZHvzXcsT8lBruf74OubRMwcTQqJap-COmrzb60S7512k0WfKTYlHsoCn_uAzOb9sp8Trjr0NksU8OXCElDU'
+}
+
+// ---------------------------------------------------------------------------
+// Fixture identifiers
+// ---------------------------------------------------------------------------
+
+// Attacker's collection — attacker holds a Full grant here
+const attackerCollectionId = '21' // Collection X
+
+// Victim collection — attacker has NO grant here
+const victimCollectionId = '83' // Collection Y
+
+// Victim asset — belongs to Collection Y (83)
+// Has VPN_SRG_TEST mapped; seed review (reviewId 13) exists for victimRuleId
+// with detail "test\nvisible to lvl1" and status submitted
+const victimAssetId = '153'
+
+// Rule present in VPN_SRG_TEST, mapped to victimAsset via stig_asset_map
+const victimRuleId = 'SV-106179r1_rule'
+
+// The seed review detail — used to confirm the review was NOT mutated after fix
+const seedDetail = 'test\nvisible to lvl1'
+
+// Attacker-controlled content — used to confirm mutation in the unfixed case
+const attackerDetail = 'ATTACKER WROTE THIS VIA CROSS-COLLECTION WRITE'
+
+// ---------------------------------------------------------------------------
+// Helpers
+// ---------------------------------------------------------------------------
+
+// Read the victim review via admin token — source of truth for write-impact checks
+async function getVictimReview () {
+ return utils.executeRequest(
+ `${config.baseUrl}/collections/${victimCollectionId}/reviews/${victimAssetId}/${victimRuleId}`,
+ 'GET',
+ config.adminToken
+ )
+}
+
+// ---------------------------------------------------------------------------
+
+describe('Security Regression: Unauthorized Cross-Collection Review Write (Finding 1)', () => {
+
+ // -------------------------------------------------------------------------
+ // Sanity checks — confirm the prerequisite fixture state is correct.
+ // These must pass both before and after the fix.
+ // -------------------------------------------------------------------------
+ describe('Fixture sanity checks', () => {
+
+ before(async function () {
+ await utils.loadAppData()
+ })
+
+ it('attacker (lvl2) has no access to victim asset 153 via GET — confirming no grant on Collection Y', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/assets/${victimAssetId}`,
+ 'GET',
+ attacker.token
+ )
+ expect(res.status).to.equal(403)
+ })
+
+ it('attacker (lvl2) can access Collection X (21) — confirming their grant is active', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}`,
+ 'GET',
+ attacker.token
+ )
+ expect(res.status).to.equal(200)
+ })
+
+ it('seed review exists on victim asset 153 with expected detail text', async () => {
+ const res = await getVictimReview()
+ expect(res.status).to.equal(200)
+ expect(res.body).to.have.property('detail', seedDetail)
+ })
+ })
+
+ // -------------------------------------------------------------------------
+ // POST /collections/{collectionId}/reviews/{assetId}
+ //
+ // Attack: collectionId = 21 (Collection X, attacker has Full grant)
+ // assetId = 153 (belongs to Collection Y, attacker has NO grant)
+ //
+ // CURRENT BEHAVIOUR (bug): HTTP 200, review written to victim asset.
+ // EXPECTED BEHAVIOUR (fix): HTTP 403, no write occurs.
+ // -------------------------------------------------------------------------
+ describe('POST /collections/{collectionId}/reviews/{assetId} — cross-collection write', () => {
+
+ beforeEach(async function () {
+ await utils.loadAppData()
+ })
+
+ it('SECURITY: POST must return 403 when assetId belongs to a different collection than collectionId', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${victimAssetId}`,
+ 'POST',
+ attacker.token,
+ [{ ruleId: victimRuleId, result: 'pass', detail: attackerDetail, comment: 'attacker comment' }]
+ )
+ expect(res.status,
+ 'Expected 403: asset 153 belongs to Collection Y (83), not Collection X (21). ' +
+ 'The API must reject writes that cross collection boundaries. ' +
+ 'If this is 200, the vulnerability is present.'
+ ).to.equal(403)
+ })
+
+ it('SECURITY: POST cross-collection attack must not mutate the victim review (write-impact verification)', async () => {
+ await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${victimAssetId}`,
+ 'POST',
+ attacker.token,
+ [{ ruleId: victimRuleId, result: 'pass', detail: attackerDetail, comment: 'attacker comment' }]
+ )
+
+ // Read the victim review via admin to check whether it was actually written.
+ // After fix: the POST was rejected (403), so the review is unchanged.
+ // Today (bug): the POST succeeded, so the detail has been overwritten.
+ const adminRes = await getVictimReview()
+ expect(adminRes.status).to.equal(200)
+ expect(adminRes.body.detail,
+ 'The victim review detail must not have been modified by the cross-collection POST. ' +
+ `Expected the seed value "${seedDetail}" to be unchanged. ` +
+ 'If the detail was overwritten, the unauthorized write succeeded.'
+ ).to.equal(seedDetail)
+ })
+ })
+
+ // -------------------------------------------------------------------------
+ // PUT /collections/{collectionId}/reviews/{assetId}/{ruleId}
+ //
+ // CURRENT BEHAVIOUR (bug): HTTP 200/201, review written to victim asset.
+ // EXPECTED BEHAVIOUR (fix): HTTP 403, no write occurs.
+ // -------------------------------------------------------------------------
+ describe('PUT /collections/{collectionId}/reviews/{assetId}/{ruleId} — cross-collection write', () => {
+
+ beforeEach(async function () {
+ await utils.loadAppData()
+ })
+
+ it('SECURITY: PUT must return 403 when assetId belongs to a different collection than collectionId', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${victimAssetId}/${victimRuleId}`,
+ 'PUT',
+ attacker.token,
+ { result: 'pass', detail: attackerDetail, comment: 'attacker comment', status: 'saved' }
+ )
+ expect(res.status,
+ 'Expected 403: asset 153 belongs to Collection Y (83), not Collection X (21). ' +
+ 'If this is 200, the vulnerability is present.'
+ ).to.equal(403)
+ })
+
+ it('SECURITY: PUT cross-collection attack must not mutate the victim review (write-impact verification)', async () => {
+ await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${victimAssetId}/${victimRuleId}`,
+ 'PUT',
+ attacker.token,
+ { result: 'pass', detail: attackerDetail, comment: 'attacker comment', status: 'saved' }
+ )
+
+ const adminRes = await getVictimReview()
+ expect(adminRes.status).to.equal(200)
+ expect(adminRes.body.detail,
+ 'The victim review detail must not have been modified by the cross-collection PUT. ' +
+ `Expected the seed value "${seedDetail}" to be unchanged.`
+ ).to.equal(seedDetail)
+ })
+ })
+
+ // -------------------------------------------------------------------------
+ // PATCH /collections/{collectionId}/reviews/{assetId}/{ruleId}
+ //
+ // The PATCH path has an additional enabler: the pre-write existence check
+ // (Review.js:202-206) calls getReviews without a collectionId filter.
+ // This allows the victim asset's existing review to satisfy the
+ // "review must exist to be patched" gate, enabling the write path.
+ //
+ // CURRENT BEHAVIOUR (bug): HTTP 200, review patched on victim asset.
+ // EXPECTED BEHAVIOUR (fix): HTTP 403, no write occurs.
+ // -------------------------------------------------------------------------
+ describe('PATCH /collections/{collectionId}/reviews/{assetId}/{ruleId} — cross-collection write', () => {
+
+ beforeEach(async function () {
+ await utils.loadAppData()
+ })
+
+ it('SECURITY: PATCH must return 403 when assetId belongs to a different collection than collectionId', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${victimAssetId}/${victimRuleId}`,
+ 'PATCH',
+ attacker.token,
+ { detail: attackerDetail }
+ )
+ expect(res.status,
+ 'Expected 403: asset 153 belongs to Collection Y (83), not Collection X (21). ' +
+ 'The pre-write existence check must not satisfy itself using a review from a ' +
+ 'foreign collection. If this is 200, the vulnerability is present.'
+ ).to.equal(403)
+ })
+
+ it('SECURITY: PATCH cross-collection attack must not mutate the victim review (write-impact verification)', async () => {
+ await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${victimAssetId}/${victimRuleId}`,
+ 'PATCH',
+ attacker.token,
+ { detail: attackerDetail }
+ )
+
+ const adminRes = await getVictimReview()
+ expect(adminRes.status).to.equal(200)
+ expect(adminRes.body.detail,
+ 'The victim review detail must not have been modified by the cross-collection PATCH. ' +
+ `Expected the seed value "${seedDetail}" to be unchanged.`
+ ).to.equal(seedDetail)
+ })
+ })
+
+ // -------------------------------------------------------------------------
+ // Restricted-role attacker — incidentally blocked today, must remain blocked after fix.
+ //
+ // When the attacker's grant roleId === 1 (Restricted), cteGrant in
+ // putReviewsByAsset uses an INNER JOIN on cteAclEffective (ReviewService.js:1006).
+ // cteAclEffective is built from the attacker's grant IDs in Collection X. The
+ // victim asset's stig_asset_map entries have saId values that never appear in
+ // Collection X's ACL, so cteGrant returns zero rules, every incoming review gets
+ // error = 'no grant for this asset/ruleId', and nothing is committed.
+ //
+ // These tests confirm that the Restricted path is blocked both before and after
+ // the fix, and that the fix does not accidentally change this behaviour.
+ // The tests should PASS today and continue to PASS after the fix.
+ // -------------------------------------------------------------------------
+ describe('Restricted-role attacker (lvl1) — blocked by ACL INNER JOIN, must stay blocked', () => {
+
+ beforeEach(async function () {
+ await utils.loadAppData()
+ })
+
+ it('Restricted attacker: POST returns 403 and does not mutate victim review', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${victimAssetId}`,
+ 'POST',
+ restrictedAttacker.token,
+ [{ ruleId: victimRuleId, result: 'pass', detail: attackerDetail, comment: 'restricted attacker comment' }]
+ )
+ // The membership check (added by the Finding 1 fix) fires before putReviewsByAsset
+ // is called, so the Restricted user gets 403 — the same as the Full-role attacker.
+ expect(res.status).to.equal(403)
+
+ const adminRes = await getVictimReview()
+ expect(adminRes.body.detail).to.equal(seedDetail)
+ })
+
+ it('Restricted attacker: PUT returns non-2xx and does not mutate victim review', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${victimAssetId}/${victimRuleId}`,
+ 'PUT',
+ restrictedAttacker.token,
+ { result: 'pass', detail: attackerDetail, comment: 'restricted attacker comment', status: 'saved' }
+ )
+ expect(res.status).to.equal(403)
+
+ const adminRes = await getVictimReview()
+ expect(adminRes.body.detail).to.equal(seedDetail)
+ })
+
+ it('Restricted attacker: PATCH returns 403 and does not mutate victim review', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${victimAssetId}/${victimRuleId}`,
+ 'PATCH',
+ restrictedAttacker.token,
+ { detail: attackerDetail }
+ )
+ // 403 is required in all cases — not 404.
+ //
+ // Today (before fix): the PATCH pre-write read (Review.js:202-206) calls getReviews
+ // without collectionId. For a Restricted caller the ACL join is an INNER JOIN, so
+ // the victim review is invisible to that read — currentReviews is empty and the
+ // handler throws NotFoundError (404). This is incorrect: a 404 tells the attacker
+ // that no review exists for this rule on this asset from the perspective of their
+ // ACL, which leaks review state across a collection boundary they cannot access.
+ //
+ // After fix: the asset-collection membership check must fire BEFORE the pre-write
+ // existence read, so the attacker receives 403 regardless of whether a review exists
+ // on the victim asset. This eliminates the 403/404 oracle.
+ expect(res.status,
+ 'Expected 403 — not 404. A 404 leaks review state across a collection boundary: ' +
+ 'it reveals whether a review exists for this rule on this asset, which is information ' +
+ 'the caller has no grant to access. The collection-membership check must run before ' +
+ 'the pre-write existence read.'
+ ).to.equal(403)
+
+ const adminRes = await getVictimReview()
+ expect(adminRes.body.detail).to.equal(seedDetail)
+ })
+ })
+
+ // -------------------------------------------------------------------------
+ // Negative controls — confirm the fix does not block legitimate same-collection writes.
+ // These tests must pass both before and after the fix.
+ // -------------------------------------------------------------------------
+ describe('Negative controls — legitimate same-collection writes must still succeed', () => {
+
+ before(async function () {
+ await utils.loadAppData()
+ })
+
+ // Full-role user (lvl2): asset 42 belongs to Collection X (21)
+ it('Full-role (lvl2): POST reviews to asset 42 in Collection X succeeds', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${reference.testAsset.assetId}`,
+ 'POST',
+ attacker.token,
+ [{ ruleId: reference.testCollection.ruleId, result: 'pass', detail: 'legitimate post from lvl2', comment: 'comment' }]
+ )
+ expect(res.status).to.equal(200)
+ })
+
+ it('Full-role (lvl2): PUT review to asset 42 in Collection X succeeds', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${reference.testAsset.assetId}/${reference.testCollection.ruleId}`,
+ 'PUT',
+ attacker.token,
+ { result: 'pass', detail: 'legitimate write from lvl2', comment: 'legitimate comment', status: 'saved' }
+ )
+ expect(res.status).to.equal(200)
+ expect(res.body).to.have.property('ruleId', reference.testCollection.ruleId)
+ expect(res.body).to.have.property('assetId', reference.testAsset.assetId)
+ })
+
+ it('Full-role (lvl2): PATCH review on asset 42 in Collection X succeeds', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${reference.testAsset.assetId}/${reference.testCollection.ruleId}`,
+ 'PATCH',
+ attacker.token,
+ { detail: 'legitimate patch from lvl2' }
+ )
+ expect(res.status).to.equal(200)
+ expect(res.body).to.have.property('ruleId', reference.testCollection.ruleId)
+ })
+
+ // Restricted-role user (lvl1): rw access via ACL rule —
+ // grantId=32 grants rw to label 'test-label-lvl1' (clId=2) + VPN_SRG_TEST.
+ // Asset 42 carries that label and has VPN_SRG_TEST mapped, so lvl1 has
+ // legitimate rw access to VPN_SRG_TEST rules on asset 42 within Collection X.
+ it('Restricted-role (lvl1): POST review to ACL-granted asset 42 / VPN_SRG_TEST in Collection X succeeds', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${reference.testAsset.assetId}`,
+ 'POST',
+ restrictedAttacker.token,
+ [{ ruleId: reference.testCollection.ruleId, result: 'pass', detail: 'legitimate post from lvl1', comment: 'comment' }]
+ )
+ expect(res.status).to.equal(200)
+ })
+
+ it('Restricted-role (lvl1): PUT review to ACL-granted asset 42 / VPN_SRG_TEST in Collection X succeeds', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${reference.testAsset.assetId}/${reference.testCollection.ruleId}`,
+ 'PUT',
+ restrictedAttacker.token,
+ { result: 'pass', detail: 'legitimate write from lvl1', comment: 'legitimate comment', status: 'saved' }
+ )
+ expect(res.status).to.equal(200)
+ expect(res.body).to.have.property('ruleId', reference.testCollection.ruleId)
+ expect(res.body).to.have.property('assetId', reference.testAsset.assetId)
+ })
+
+ it('Restricted-role (lvl1): PATCH review on ACL-granted asset 42 / VPN_SRG_TEST in Collection X succeeds', async () => {
+ const res = await utils.executeRequest(
+ `${config.baseUrl}/collections/${attackerCollectionId}/reviews/${reference.testAsset.assetId}/${reference.testCollection.ruleId}`,
+ 'PATCH',
+ restrictedAttacker.token,
+ { detail: 'legitimate patch from lvl1' }
+ )
+ expect(res.status).to.equal(200)
+ expect(res.body).to.have.property('ruleId', reference.testCollection.ruleId)
+ })
+ })
+})
diff --git a/test/api/mocha/utils/testUtils.js b/test/api/mocha/utils/testUtils.js
index d8ea7e26a..4c584d67d 100644
--- a/test/api/mocha/utils/testUtils.js
+++ b/test/api/mocha/utils/testUtils.js
@@ -193,7 +193,7 @@ const createTempCollection = async (collectionPost) => {
}
}
- const res = await fetch(`${baseUrl}/collections?elevate=true&projection=grants&projection=labels`, {
+ const res = await fetch(`${baseUrl}/collections?projection=grants&projection=labels`, {
method: 'POST',
headers: {
Authorization: `Bearer ${adminToken}`,
From 806dee5ac24bc94382c552bea7d8bac07f505696 Mon Sep 17 00:00:00 2001
From: cd-rite <61710958+cd-rite@users.noreply.github.com>
Date: Fri, 24 Apr 2026 02:45:26 -0400
Subject: [PATCH 4/7] deps: update @nuwcdivnpt/stig-manager-client-modules to
v1.6.7 and upgrade uuid to v14.0.0 (#2030)
---
client/src/js/modules/package-lock.json | 8 ++++----
client/src/js/modules/package.json | 2 +-
test/api/package-lock.json | 17 ++++-------------
test/api/package.json | 5 +++--
test/state/package-lock.json | 12 ++++++++----
test/state/package.json | 3 ++-
test/unit/package-lock.json | 12 ++++++++----
test/unit/package.json | 3 ++-
8 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/client/src/js/modules/package-lock.json b/client/src/js/modules/package-lock.json
index fb1d02f0f..ea455cc16 100644
--- a/client/src/js/modules/package-lock.json
+++ b/client/src/js/modules/package-lock.json
@@ -5,7 +5,7 @@
"packages": {
"": {
"dependencies": {
- "@nuwcdivnpt/stig-manager-client-modules": "^1.6.6",
+ "@nuwcdivnpt/stig-manager-client-modules": "^1.6.7",
"chart.js": "^4.4.2",
"serialize-error": "^11.0.0"
}
@@ -16,9 +16,9 @@
"integrity": "sha512-fuscdXJ9G1qb7W8VdHi+IwRqij3lBkosAm4ydQtEmbY58OzHXqQhvlxqEkoz0yssNVn38bcpRWgA9PP+OGoisw=="
},
"node_modules/@nuwcdivnpt/stig-manager-client-modules": {
- "version": "1.6.6",
- "resolved": "https://registry.npmjs.org/@nuwcdivnpt/stig-manager-client-modules/-/stig-manager-client-modules-1.6.6.tgz",
- "integrity": "sha512-gqiHLeblktGkJLEnUxBhcfWLeKQ23jFaua+hIMPLINf4MwOIzECshgBlHB8IcK+YqphiK5wuplNS/2glPWLeNw==",
+ "version": "1.6.7",
+ "resolved": "https://registry.npmjs.org/@nuwcdivnpt/stig-manager-client-modules/-/stig-manager-client-modules-1.6.7.tgz",
+ "integrity": "sha512-g1/wm/sjod+B61NIQz7u0E85TwWMOm+e0aWxDa4PIbmUlU29j8yQvrh9xVCkoMnenz/hCmK9AJUh8pRAs5Jj3g==",
"license": "MIT"
},
"node_modules/chart.js": {
diff --git a/client/src/js/modules/package.json b/client/src/js/modules/package.json
index 806799525..19f3bf663 100644
--- a/client/src/js/modules/package.json
+++ b/client/src/js/modules/package.json
@@ -1,6 +1,6 @@
{
"dependencies": {
- "@nuwcdivnpt/stig-manager-client-modules": "^1.6.6",
+ "@nuwcdivnpt/stig-manager-client-modules": "^1.6.7",
"chart.js": "^4.4.2",
"serialize-error": "^11.0.0"
}
diff --git a/test/api/package-lock.json b/test/api/package-lock.json
index fa1016d99..267426438 100644
--- a/test/api/package-lock.json
+++ b/test/api/package-lock.json
@@ -9,7 +9,7 @@
"version": "1.0.0",
"license": "ISC",
"dependencies": {
- "@nuwcdivnpt/stig-manager-client-modules": "^1.6.6",
+ "@nuwcdivnpt/stig-manager-client-modules": "^1.6.7",
"chai": "^5.1.2",
"chai-datetime": "^1.8.1",
"deep-equal-in-any-order": "^2.0.6",
@@ -125,9 +125,9 @@
"license": "MIT"
},
"node_modules/@nuwcdivnpt/stig-manager-client-modules": {
- "version": "1.6.6",
- "resolved": "https://registry.npmjs.org/@nuwcdivnpt/stig-manager-client-modules/-/stig-manager-client-modules-1.6.6.tgz",
- "integrity": "sha512-gqiHLeblktGkJLEnUxBhcfWLeKQ23jFaua+hIMPLINf4MwOIzECshgBlHB8IcK+YqphiK5wuplNS/2glPWLeNw==",
+ "version": "1.6.7",
+ "resolved": "https://registry.npmjs.org/@nuwcdivnpt/stig-manager-client-modules/-/stig-manager-client-modules-1.6.7.tgz",
+ "integrity": "sha512-g1/wm/sjod+B61NIQz7u0E85TwWMOm+e0aWxDa4PIbmUlU29j8yQvrh9xVCkoMnenz/hCmK9AJUh8pRAs5Jj3g==",
"license": "MIT"
},
"node_modules/@pkgjs/parseargs": {
@@ -883,15 +883,6 @@
"marge": "bin/cli.js"
}
},
- "node_modules/mochawesome/node_modules/uuid": {
- "version": "8.3.2",
- "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",
- "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==",
- "dev": true,
- "bin": {
- "uuid": "dist/bin/uuid"
- }
- },
"node_modules/ms": {
"version": "2.1.3",
"resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz",
diff --git a/test/api/package.json b/test/api/package.json
index 77b98d069..c4a8c119a 100644
--- a/test/api/package.json
+++ b/test/api/package.json
@@ -11,7 +11,7 @@
"license": "ISC",
"type": "module",
"dependencies": {
- "@nuwcdivnpt/stig-manager-client-modules": "^1.6.6",
+ "@nuwcdivnpt/stig-manager-client-modules": "^1.6.7",
"chai": "^5.1.2",
"chai-datetime": "^1.8.1",
"deep-equal-in-any-order": "^2.0.6",
@@ -23,7 +23,8 @@
},
"overrides": {
"serialize-javascript": "^7.0.5",
- "diff": "^8.0.3"
+ "diff": "^8.0.3",
+ "uuid": "^14.0.0"
},
"devDependencies": {
"mochawesome": "^7.1.3"
diff --git a/test/state/package-lock.json b/test/state/package-lock.json
index 9230d8e9e..76c750662 100644
--- a/test/state/package-lock.json
+++ b/test/state/package-lock.json
@@ -1325,12 +1325,16 @@
}
},
"node_modules/uuid": {
- "version": "8.3.2",
- "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",
- "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==",
+ "version": "14.0.0",
+ "resolved": "https://registry.npmjs.org/uuid/-/uuid-14.0.0.tgz",
+ "integrity": "sha512-Qo+uWgilfSmAhXCMav1uYFynlQO7fMFiMVZsQqZRMIXp0O7rR7qjkj+cPvBHLgBqi960QCoo/PH2/6ZtVqKvrg==",
+ "funding": [
+ "https://github.com/sponsors/broofa",
+ "https://github.com/sponsors/ctavan"
+ ],
"license": "MIT",
"bin": {
- "uuid": "dist/bin/uuid"
+ "uuid": "dist-node/bin/uuid"
}
},
"node_modules/validator": {
diff --git a/test/state/package.json b/test/state/package.json
index b0adf3355..32a3ac873 100644
--- a/test/state/package.json
+++ b/test/state/package.json
@@ -5,7 +5,8 @@
},
"overrides": {
"serialize-javascript": "^7.0.5",
- "diff": "^8.0.3"
+ "diff": "^8.0.3",
+ "uuid": "^14.0.0"
},
"dependencies": {
"chai": "^5.2.0",
diff --git a/test/unit/package-lock.json b/test/unit/package-lock.json
index 7085f9a4d..b3b1ccac3 100644
--- a/test/unit/package-lock.json
+++ b/test/unit/package-lock.json
@@ -1132,12 +1132,16 @@
}
},
"node_modules/uuid": {
- "version": "8.3.2",
- "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",
- "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==",
+ "version": "14.0.0",
+ "resolved": "https://registry.npmjs.org/uuid/-/uuid-14.0.0.tgz",
+ "integrity": "sha512-Qo+uWgilfSmAhXCMav1uYFynlQO7fMFiMVZsQqZRMIXp0O7rR7qjkj+cPvBHLgBqi960QCoo/PH2/6ZtVqKvrg==",
+ "funding": [
+ "https://github.com/sponsors/broofa",
+ "https://github.com/sponsors/ctavan"
+ ],
"license": "MIT",
"bin": {
- "uuid": "dist/bin/uuid"
+ "uuid": "dist-node/bin/uuid"
}
},
"node_modules/which": {
diff --git a/test/unit/package.json b/test/unit/package.json
index 777660b28..e0126738f 100644
--- a/test/unit/package.json
+++ b/test/unit/package.json
@@ -5,7 +5,8 @@
},
"overrides": {
"serialize-javascript": "^7.0.5",
- "diff": "^8.0.3"
+ "diff": "^8.0.3",
+ "uuid": "^14.0.0"
},
"dependencies": {
"chai": "^5.2.0",
From a84cef81751fa89c597ccf1198b1dd4bbf8a434a Mon Sep 17 00:00:00 2001
From: cd-rite <61710958+cd-rite@users.noreply.github.com>
Date: Sun, 26 Apr 2026 23:36:22 -0400
Subject: [PATCH 5/7] chore: update version to 1.6.9 and add release notes for
new features and improvements (#2031)
---
api/source/package-lock.json | 4 ++--
api/source/package.json | 2 +-
release-notes.rst | 18 ++++++++++++++++++
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/api/source/package-lock.json b/api/source/package-lock.json
index 71711db01..f48ee26ec 100644
--- a/api/source/package-lock.json
+++ b/api/source/package-lock.json
@@ -1,12 +1,12 @@
{
"name": "stig-management-api",
- "version": "1.6.8",
+ "version": "1.6.9",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "stig-management-api",
- "version": "1.6.8",
+ "version": "1.6.9",
"license": "MIT",
"dependencies": {
"ajv": "^8.17.1",
diff --git a/api/source/package.json b/api/source/package.json
index 6d9d2a81e..a732c6e03 100644
--- a/api/source/package.json
+++ b/api/source/package.json
@@ -1,6 +1,6 @@
{
"name": "stig-management-api",
- "version": "1.6.8",
+ "version": "1.6.9",
"description": "An API for managing evaluations of Security Technical Implementation Guide (STIG) assessments.",
"main": "index.js",
"scripts": {
diff --git a/release-notes.rst b/release-notes.rst
index 6d3d405e5..2f07d83e2 100644
--- a/release-notes.rst
+++ b/release-notes.rst
@@ -1,3 +1,21 @@
+1.6.9
+-------
+
+Changes:
+
+ - (API) Added guard to prevent elevated requests from modifying collection ``settings``, ``labels``, or ``metadata`` on create/replace/update
+ - (API) Simplified asset collection retrieval in controllers
+ - (API) Refactored JWKS cache error logging
+ - (API) Replaced direct string interpolation in SQL query construction with parameterized binds in MetricsService and JobService
+ - (UI) New ``STIGMAN_CLIENT_CONSOLE_MODE`` environment variable to suppress console output in non-development environments
+ - (UI) Various escaping and DOM insertion improvements across multiple SM components
+ - (UI) Updated OIDC worker initialization
+ - (Docs) Clarified data and permissions documentation for elevated actions
+ - (Tests) Added regression tests for cross-collection write access; updated test utilities and collection test fixtures to align with API behavior
+ - (Dependencies) Update ``fast-xml-parser`` to v5.7.1 and remove the ``uuid`` runtime dependency from the API
+ - (Dependencies) Update ``@nuwcdivnpt/stig-manager-client-modules`` to v1.6.7
+ - (Dependencies) Various security and maintenance updates
+
1.6.8
-------
From 3073fd1df001337007d55fb14be2a47fec0aa392 Mon Sep 17 00:00:00 2001
From: cd-rite <61710958+cd-rite@users.noreply.github.com>
Date: Wed, 29 Apr 2026 16:59:26 -0400
Subject: [PATCH 6/7] test: enhance timeout handling and logging in various
test cases (#2033)
Co-authored-by: cd-rite
---
test/state/mocha/bootstrap.test.js | 61 +++++++++++++----------
test/state/mocha/db.test.js | 63 ++++++++++--------------
test/state/mocha/jwks.test.js | 11 +++--
test/state/mocha/lib.js | 29 +++++++++++
test/state/mocha/oidc.test.js | 45 ++++++++---------
test/state/mocha/tokenValidation.test.js | 18 ++++---
test/utils/mockOidc.js | 22 ++++-----
7 files changed, 137 insertions(+), 112 deletions(-)
diff --git a/test/state/mocha/bootstrap.test.js b/test/state/mocha/bootstrap.test.js
index 4170dbeb7..9325ac365 100644
--- a/test/state/mocha/bootstrap.test.js
+++ b/test/state/mocha/bootstrap.test.js
@@ -21,8 +21,9 @@ describe('Boot with no dependencies', function () {
})
after(async function () {
- await api.stop()
- addContext(this, {title: 'api-log', value: api.logRecords})
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
+ if (api) addContext(this, {title: 'api-log', value: api.logRecords})
})
describe('GET /op/state', function () {
@@ -113,10 +114,11 @@ describe('Boot with both dependencies', function () {
})
after(async function () {
- await api.stop()
- await mysql.stop()
- await oidc.stop()
- addContext(this, {title: 'api-log', value: api.logRecords})
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
+ if (mysql) await mysql.stop().catch(() => {})
+ if (oidc) await oidc.stop().catch(() => {})
+ if (api) addContext(this, {title: 'api-log', value: api.logRecords})
})
describe('GET /op/state', function () {
@@ -127,7 +129,7 @@ describe('Boot with both dependencies', function () {
expect(res.body.dependencies).to.eql({db: true, oidc: true})
})
})
-
+
describe('GET /op/configuration', function () {
it('should return 200 when dependencies are available', async function () {
const res = await simpleRequest(`${apiOrigin}/api/op/configuration`)
@@ -189,10 +191,11 @@ describe('Boot with old mysql', function () {
})
after(async function () {
- await api.stop()
- await mysql.stop()
- await oidc.stop()
- addContext(this, {title: 'api-log', value: api.logRecords})
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
+ if (mysql) await mysql.stop().catch(() => {})
+ if (oidc) await oidc.stop().catch(() => {})
+ if (api) addContext(this, {title: 'api-log', value: api.logRecords})
})
describe('exit code', function () {
@@ -249,10 +252,11 @@ describe('Boot with insecure kid - allow insecure tokens false', function () {
})
after(async function () {
- await api.stop()
- await mysql.stop()
- await oidc.stop()
- addContext(this, {title: 'api-log', value: api.logRecords})
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
+ if (mysql) await mysql.stop().catch(() => {})
+ if (oidc) await oidc.stop().catch(() => {})
+ if (api) addContext(this, {title: 'api-log', value: api.logRecords})
})
describe('exit code', function () {
@@ -312,10 +316,11 @@ describe('Boot without insecure kid - request with insecure token' , function ()
})
after(async function () {
- await api.stop()
- await mysql.stop()
- await oidc.stop()
- addContext(this, {title: 'api-log', value: api.logRecords})
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
+ if (mysql) await mysql.stop().catch(() => {})
+ if (oidc) await oidc.stop().catch(() => {})
+ if (api) addContext(this, {title: 'api-log', value: api.logRecords})
})
describe('GET /op/state', function () {
@@ -361,10 +366,11 @@ describe('Boot with STIGMAN_JWKS_CACHE_MAX_AGE out of range', function () {
})
after(async function () {
- await mysql.stop()
- await oidc.stop()
- await api.stop()
- addContext(this, {title: 'api-log', value: api.logRecords})
+ this.timeout(60000)
+ if (mysql) await mysql.stop().catch(() => {})
+ if (oidc) await oidc.stop().catch(() => {})
+ if (api) await api.stop().catch(() => {})
+ if (api) addContext(this, {title: 'api-log', value: api.logRecords})
})
describe('Mimimum value enforced', function () {
@@ -382,7 +388,8 @@ describe('Boot with STIGMAN_JWKS_CACHE_MAX_AGE out of range', function () {
})
})
after(async function () {
- await api.stop()
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
})
it('should return minimum oauth.maxCacheAge (1)', async function () {
const configLog = api.logRecords.filter(r => r.type === 'configuration')[0]
@@ -405,7 +412,8 @@ describe('Boot with STIGMAN_JWKS_CACHE_MAX_AGE out of range', function () {
})
})
after(async function () {
- await api.stop()
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
})
it('should return maximum oauth.maxCacheAge (35791)', async function () {
const configLog = api.logRecords.filter(r => r.type === 'configuration')[0]
@@ -428,7 +436,8 @@ describe('Boot with STIGMAN_JWKS_CACHE_MAX_AGE out of range', function () {
})
})
after(async function () {
- await api.stop()
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
})
it('should return default oauth.maxCacheAge (10)', async function () {
const configLog = api.logRecords.filter(r => r.type === 'configuration')[0]
diff --git a/test/state/mocha/db.test.js b/test/state/mocha/db.test.js
index a611c439a..ce4be4bab 100644
--- a/test/state/mocha/db.test.js
+++ b/test/state/mocha/db.test.js
@@ -1,5 +1,5 @@
import { expect } from 'chai'
-import { getPorts, spawnApiPromise, spawnMySQL, simpleRequest, execIpTables } from './lib.js'
+import { getPorts, spawnApiPromise, spawnMySQL, simpleRequest, execIpTables, waitForLog } from './lib.js'
import MockOidc from '../../utils/mockOidc.js'
import addContext from 'mochawesome/addContext.js'
@@ -10,16 +10,7 @@ describe('DB outage: shutdown', function () {
let mysql
let oidc
- async function waitLogEvent(type, count = 1) {
- let seen = 0
- return new Promise((resolve) => {
- api.logEvents.on(type, function (log) {
- seen++
- if (seen >= count) resolve(log)
- })
- })
- }
-
+
before(async function () {
this.timeout(60000)
oidc = new MockOidc({keyCount: 1, includeInsecureKid: false})
@@ -47,10 +38,11 @@ describe('DB outage: shutdown', function () {
})
after(async function () {
- await api.stop()
- await mysql.stop()
- await oidc.stop()
- addContext(this, {title: 'api-log', value: api.logRecords})
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
+ if (mysql) await mysql.stop().catch(() => {})
+ if (oidc) await oidc.stop().catch(() => {})
+ if (api) addContext(this, {title: 'api-log', value: api.logRecords})
})
describe('DB up', function () {
@@ -64,8 +56,10 @@ describe('DB outage: shutdown', function () {
})
describe('DB shutdown', function () {
+ let logMark
before(async function () {
this.timeout(30000)
+ logMark = api.logRecords.length
await mysql.stop()
console.log(' mysql shutdown')
})
@@ -74,21 +68,23 @@ describe('DB outage: shutdown', function () {
const res = await simpleRequest(`${apiOrigin}/api/op/state`)
expect(res.status).to.equal(200)
expect(res.body.currentState).to.equal('unavailable')
- expect(res.body.dependencies).to.eql({db: false, oidc: true})
+ expect(res.body.dependencies).to.eql({db: false, oidc: true})
})
it('should log retry fail', async function () {
this.timeout(30000)
console.log(' wait for log: restore (2)')
- const log = await waitLogEvent('restore', 2)
+ const log = await waitForLog(api, 'restore', {count: 2, since: logMark})
expect(log.data.message).to.equal(`connect ECONNREFUSED 127.0.0.1:${dbPort}`)
})
})
describe('DB restarted', function() {
+ let logMark
before( async function() {
this.timeout(30000)
console.log(' try mysql restart')
+ logMark = api.logRecords.length
mysql = await spawnMySQL({tag: '8.0.24', port: dbPort})
console.log(' ✔ mysql restarted')
})
@@ -96,7 +92,7 @@ describe('DB outage: shutdown', function () {
it('should return state "available"', async function () {
this.timeout(60000)
console.log(' wait for log: state-changed')
- const log = await waitLogEvent('state-changed')
+ const log = await waitForLog(api, 'state-changed', {since: logMark})
expect(log.data.currentState).to.equal('available')
expect(log.data.previousState).to.equal('unavailable')
const res = await simpleRequest(`${apiOrigin}/api/op/state`)
@@ -112,16 +108,6 @@ describe('DB outage: network/host down', function () {
let mysql
let oidc
- async function waitLogEvent(type, count = 1) {
- let seen = 0
- return new Promise((resolve) => {
- api.logEvents.on(type, function (log) {
- seen++
- if (seen >= count) resolve(log)
- })
- })
- }
-
before(async function () {
this.timeout(60000)
oidc = new MockOidc({keyCount: 1, includeInsecureKid: false})
@@ -149,10 +135,11 @@ describe('DB outage: network/host down', function () {
})
after(async function () {
- await api.stop()
- await mysql.stop()
- await oidc.stop()
- addContext(this, {title: 'api-log', value: api.logRecords})
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
+ if (mysql) await mysql.stop().catch(() => {})
+ if (oidc) await oidc.stop().catch(() => {})
+ if (api) addContext(this, {title: 'api-log', value: api.logRecords})
})
describe('Network/host up', function () {
@@ -166,33 +153,37 @@ describe('DB outage: network/host down', function () {
})
describe('Network/host down', function () {
+ let logMark
before(async function () {
+ logMark = api.logRecords.length
execIpTables(`-A OUTPUT -p tcp --dport ${dbPort} -j DROP`)
console.log(' iptables dropping packets')
})
it('should return state "unavailable"', async function () {
this.timeout(30000)
console.log(' wait for log: state-changed')
- const log = await waitLogEvent('state-changed')
+ const log = await waitForLog(api, 'state-changed', {since: logMark})
expect(log.data.currentState).to.equal('unavailable')
expect(log.data.previousState).to.equal('available')
const res = await simpleRequest(`${apiOrigin}/api/op/state`)
expect(res.status).to.equal(200)
expect(res.body.currentState).to.equal('unavailable')
- expect(res.body.dependencies).to.eql({db: false, oidc: true})
+ expect(res.body.dependencies).to.eql({db: false, oidc: true})
})
it('should log retry fail', async function () {
this.timeout(45000)
console.log(' wait for log: restore (2)')
- const log = await waitLogEvent('restore', 2)
+ const log = await waitForLog(api, 'restore', {count: 2, since: logMark})
expect(log.data.message).to.equal('connect ETIMEDOUT')
})
})
describe('Network/host up', function() {
+ let logMark
before( async function() {
this.timeout(30000)
+ logMark = api.logRecords.length
execIpTables(`-D OUTPUT -p tcp --dport ${dbPort} -j DROP`)
console.log(' iptables accepting packets')
})
@@ -200,7 +191,7 @@ describe('DB outage: network/host down', function () {
it('should return state "available"', async function () {
this.timeout(60000)
console.log(' wait for log: state-changed')
- const log = await waitLogEvent('state-changed')
+ const log = await waitForLog(api, 'state-changed', {since: logMark})
expect(log.data.currentState).to.equal('available')
expect(log.data.previousState).to.equal('unavailable')
const res = await simpleRequest(`${apiOrigin}/api/op/state`)
diff --git a/test/state/mocha/jwks.test.js b/test/state/mocha/jwks.test.js
index 113abb8e2..c429348d1 100644
--- a/test/state/mocha/jwks.test.js
+++ b/test/state/mocha/jwks.test.js
@@ -15,7 +15,7 @@ describe('JWKS Tests', function () {
const {apiPort, dbPort, oidcPort, apiOrigin, oidcOrigin} = getPorts(54020)
before(async function () {
- this.timeout(30000)
+ this.timeout(60000)
oidc = new MockOidc({keyCount: 1, includeInsecureKid: false})
tokens.rotation0 = oidc.getToken({username: 'prerotation', privileges:['create_collection']}) // default privileges
oidc.rotateKeys({keyCount: 1, includeInsecureKid: false})
@@ -46,10 +46,11 @@ describe('JWKS Tests', function () {
})
after(async function () {
- await api.stop()
- await mysql.stop()
- await oidc.stop()
- addContext(this, {title: 'api-log', value: api.logRecords})
+ this.timeout(60000)
+ if (api) await api.stop().catch(() => {})
+ if (mysql) await mysql.stop().catch(() => {})
+ if (oidc) await oidc.stop().catch(() => {})
+ if (api) addContext(this, {title: 'api-log', value: api.logRecords})
})
describe('Create user according to token', function () {
diff --git a/test/state/mocha/lib.js b/test/state/mocha/lib.js
index 73302ecb4..6baceedc3 100644
--- a/test/state/mocha/lib.js
+++ b/test/state/mocha/lib.js
@@ -113,6 +113,35 @@ export function spawnApi ({
}
}
+/**
+ * Resolves with a log record matching `type` once `count` such records have
+ * been seen at index >= `since` in `api.logRecords`. Race-safe: counts past
+ * records so the helper still resolves if the trigger fired before the caller
+ * awaited. Capture `api.logRecords.length` into `since` *before* triggering
+ * the action that produces the event(s).
+ * @param {Object} api - Result of spawnApiPromise; must have logRecords + logEvents.
+ * @param {string} type - The log record type to wait for.
+ * @param {Object} [opts]
+ * @param {number} [opts.count=1] - Number of matching records before resolving.
+ * @param {number} [opts.since=0] - Index in api.logRecords at which to start counting.
+ * @param {(log: Object) => boolean} [opts.predicate] - Optional filter applied to each record.
+ * @returns {Promise