TRITON-2513 - Update Access Keys to better support Manta S3 #12

travispaul · 2025-10-24T19:38:45Z

Add support for a description field on access keys.
Add support for updating an access key's description and status.
Added an updated field that is updated when either description or status are modified.
Add prefix and checksum to accesskeysecret inspired by suggestions from Github's Secret scanning partner program (even though we are unlikely to join this program.)
Replace usage of nodeunit's ifError function with assert-plus' ifError (See: ifError doesn't throw caolan/nodeunit#151).
Use non-blocking version of crypto.randomBytes().
Move existing (and new) accesskey unit tests into a separate file.

Confirmed make check and make tests pass. The unit tests will pass with sdc-ufds version (7.5.0) with changes from TritonDataCenter/sdc-ufds/pull/29

The unit tests will also mostly pass with the current version of sdc-ufds (v7.4.1) though some validation tests in this repo will fail as this older version is less strict about the status and description properties. Meaning this client is compatible with both versions (not that it necessarily needs to be.)

Related PRs:

- Add support for description field on access keys - Add support for udpating an access key's description and status - Add prefix and checksum to accesskeysecret - Replace usege of nodeunit's ifError with assert-plus' ifError (See: caolan/nodeunit#151)

- Eexport DEFAULT_PREFIX and DEFAULT_BYTE_LENGTH. - Patch up some fields that may be missing on older records as a convience for clients.

danmcd

Catching up on the incremental change here.

danmcd

Got distracted by Intel ucode, have some clarifying questions, one of which is critical, and one nit.

lib/accesskey.js

lib/index.js

test/accesskey.test.js

danmcd · 2025-11-17T16:20:37Z

package-lock.json

+        "rimraf": "~2.4.0"
+      }
+    },
+    "nan": {


This will prevent OS-8701/MANTA-5487 classes of problems right?

Unfortunately, not in this case because this module wouldn't commonly be used at the highest level of the dependency tree. It does help the next person working on this module and running tests but its not nearly as helpful as other places.

To avoid OS-8701/MANTA-5487 issues we really want package-lock.json files in the top level modules, like sdc-ufds, and sdc-cloudapi. We also want to use npm ci instead of npm install as there's cases where npm install will still re-resolve version ranges. Even without using npm ci having a package-lock.json in those repos reduces the frequency of a OS-8701/MANTA-5487 happening again.

OH... I'm getting confused. HERE it shouldn't be an issue because nobody's using this app with v0.10 anymore I'm guessing?

If this IS getting used with v0.10, we'll need to put it up in package.json I'm guessing, or higher up here (I'll indicate where).

When sdc-ufds and sdc-cloudapi come up I'll keep an eye on that.

Sorry, I think I muddied things up a bit too and didn't accurately address your question.

it shouldn't be an issue because nobody's using this app with v0.10 anymore

Correct, if something was using v0.10 with this module the package-lock.json here wouldn't help (or hurt) them anyway, we'd still need to solve that in the same way we did in OS-8701/MANTA-5487.

danmcd

Pardon my nan panic. I've resolved all other conversations except for that one and a new one. All I need on those two is that I'm now understanding things and not further muddying the waters.

I'll need to go over index.js and accesskey.test.js once more.

lib/accesskey.js

danmcd · 2025-11-17T18:23:54Z

package-lock.json

+        "rimraf": "~2.4.0"
+      }
+    },
+    "nan": {


OH... I'm getting confused. HERE it shouldn't be an issue because nobody's using this app with v0.10 anymore I'm guessing?

If this IS getting used with v0.10, we'll need to put it up in package.json I'm guessing, or higher up here (I'll indicate where).

package-lock.json

danmcd · 2025-11-17T18:26:01Z

package-lock.json

+        "rimraf": "~2.4.0"
+      }
+    },
+    "nan": {


When sdc-ufds and sdc-cloudapi come up I'll keep an eye on that.

danmcd

Code seems okay, but looking through the tests, I have some silly questions. I may be missing the plot too.

Do access keys have lifetimes?
This may be more a node-triton or sdc-ufds issue, but does the testing here need to cover users trying to grab things that aren't theirs?

travispaul · 2025-11-17T22:09:59Z

@danmcd,

* Do access keys have lifetimes?

These access keys do not have a lifetime and do not expire on their own. I considered giving them an expiration but opted against it to align with AWS. We'll need to support temporary tokens for the S3 Manta functionality separately too, which would cover the use case for keys that can expire.

There's an argument to be made that "We're not AWS" and/or those reasons aren't good enough and we want the keys to automatically expire too.

* This may be more a node-triton or sdc-ufds issue, but does the testing here need to cover users trying to grab things that aren't theirs?

This module assumes that the user IDs supplied are from a trusted source such as cloudapi or sdc-ufds. I did add some some tests here which creates a key for a user and two subusers and confirms that only their specific keys are returned when queried. I can add another user with two subusers and do a cross check against both accounts too to cover all the bases.

danmcd

Thank you for the extra test with two (possibly mutually-hostile) users. I can't see anything else immediately wrong here.

cneira

Let me know if it's too much work to avoid the usage of Buffer.alloc that we need to start exploring bumping up the mahi version of node.

lib/accesskey.js

Also fix some inconsistent 2 vs 4 space indentiation usage.

travispaul · 2025-11-24T15:19:04Z

@danmcd commit c1da5c0 dismissed your approval. When/if you have some more cycles could you re-review?

danmcd

Re-up based on the new play-nice-with-v.0.10.x changes.

lib/accesskey.js

travispaul mentioned this pull request Oct 24, 2025

TRITON-2513 - Update Access Keys to better support Manta S3 TritonDataCenter/sdc-ufds#29

Closed

travispaul force-pushed the TRITON-2513 branch from 590843b to cf5cd46 Compare October 24, 2025 21:18

Export defaults params, patch up older records.

3e0ffc4

- Eexport DEFAULT_PREFIX and DEFAULT_BYTE_LENGTH. - Patch up some fields that may be missing on older records as a convience for clients.

travispaul mentioned this pull request Oct 27, 2025

TRITON-2513 - Update Access Keys to better support Manta S3 TritonDataCenter/sdc-cloudapi#153

Open

travispaul added 3 commits October 28, 2025 15:01

Add listActiveAccessKeys convenience function

6fed73e

Add additional subuser test

d0beab1

updateAccessKey should return the updated key

8b64c76

danmcd reviewed Oct 28, 2025

View reviewed changes

travispaul marked this pull request as ready for review October 28, 2025 21:16

cneira self-requested a review October 29, 2025 11:32

travispaul mentioned this pull request Oct 29, 2025

MANTA-5471 - Support accesskey status updates TritonDataCenter/mahi#26

Merged

travispaul requested a review from a team November 17, 2025 13:59

travispaul mentioned this pull request Nov 17, 2025

TRITON-2510 Add triton accesskeys commands to manage access keys. TritonDataCenter/node-triton#350

Open

danmcd reviewed Nov 17, 2025

View reviewed changes

cneira previously approved these changes Nov 17, 2025

View reviewed changes

Fix copyright comment

cf6fcc9

travispaul dismissed cneira’s stale review via cf6fcc9 November 17, 2025 17:25

danmcd reviewed Nov 17, 2025

View reviewed changes

Add cross account checks

c8f93c1

danmcd previously approved these changes Nov 18, 2025

View reviewed changes

cneira requested changes Nov 18, 2025

View reviewed changes

lib/accesskey.js Outdated Show resolved Hide resolved

Restore Node v0.10 support

c1da5c0

Also fix some inconsistent 2 vs 4 space indentiation usage.

travispaul dismissed danmcd’s stale review via c1da5c0 November 18, 2025 21:58

cneira approved these changes Nov 26, 2025

View reviewed changes

danmcd approved these changes Nov 26, 2025

View reviewed changes

lib/accesskey.js Show resolved Hide resolved

travispaul merged commit 4dadfe2 into master Nov 26, 2025
1 check passed

TRITON-2513 - Update Access Keys to better support Manta S3 #12

TRITON-2513 - Update Access Keys to better support Manta S3 #12

Uh oh!

Conversation

travispaul commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danmcd Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

travispaul Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

danmcd Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

danmcd Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

travispaul Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danmcd Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danmcd Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

travispaul commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

cneira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

travispaul commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danmcd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

travispaul commented Oct 24, 2025 •

edited

Loading

travispaul commented Nov 17, 2025 •

edited

Loading

travispaul commented Nov 24, 2025 •

edited

Loading