Skip to content

MANTA-5522 Add lib/scope-schema.js as the single source of truth for bucket scope schema#11

Open
cneira wants to merge 8 commits intomasterfrom
accesskey-per-bucket
Open

MANTA-5522 Add lib/scope-schema.js as the single source of truth for bucket scope schema#11
cneira wants to merge 8 commits intomasterfrom
accesskey-per-bucket

Conversation

@cneira
Copy link
Copy Markdown
Contributor

@cneira cneira commented Apr 28, 2026

Add a shared bucket scope validation module and two new endpoints for bucket scoped access keys:

  • cachePush : Push a key directly to mahi/authcache redis cache, the intent is that the newly created access key can be used right away and bypass UFDS replication delay.

  • scopeRevoke: Does the opposite, it inmediately deletes a key from the redis cache.

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

cneira and others added 5 commits April 22, 2026 20:11
and MahiClient.prototype.scopeRevoke for immediate key deletion.
Both are best-effort with warn-level logging on failure.
Single source of truth for per-bucket access key scope
constants, validation, and pattern matching. Exported via
index.js as scopeSchema. Consumed by sdc-cloudapi directly
and by manta-buckets-api via local copy.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Match the mahi server endpoint rename.  The method name
scopeRevoke() is unchanged (JS API, not REST path).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Parse and validate opts.scope using scope-schema before
sending to the mahi server.  Prevents malformed scope
strings from poisoning the Redis cache and corrupting
authorization decisions for affected access keys.

Null/undefined scope (unrestricted key) is still accepted.

AI-generated code.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove redundant local require('querystring') that shadowed
the module-level qs variable (JSL warning).  Strip trailing
whitespace in client.js.  Parenthesize typeof expressions in
scope-schema.js per jsstyle.

AI-generated code.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cneira cneira changed the title Allow bucket-scoped access keys MANTA-5522 Add lib/scope-schema.js as the single source of truth for bucket scope schema Apr 28, 2026
@cneira cneira requested a review from a team April 28, 2026 16:49
Copy link
Copy Markdown

@danmcd danmcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checkpoint, and I shouldn't revisit here until I've completely looked at TritonDataCenter/mahi#30 .

Comment thread index.js

module.exports = require('./lib/client.js');
var client = require('./lib/client.js');
var scopeSchema = require('./lib/scope-schema.js');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the coordination problem due to the "microservices" nature of current Manta & Triton?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the scope-schema to be in sync between consumers, so we share this here.

Comment thread lib/client.js
Copy link
Copy Markdown
Member

@travispaul travispaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please bump the version in package.json too

Comment thread lib/scope-schema.js
* @param {Object} scope - Scope envelope object
* @return {Object} {valid, scope, error}
*/
function validateScope(scope) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to have a couple of unit tests for this validator.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests added for this function.

Copy link
Copy Markdown

@danmcd danmcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems okay, but my too-narrow complaint holds here as well. Unsure why Claude emits it like this.

Comment thread lib/client.js
cneira added 3 commits May 4, 2026 13:47
Cover validateScope shape rejection, envelope rejection,
per-entry rejection, and success path. Smoke-test
isValidBucketPattern, matchBucketPattern, and parseScope.
@cneira cneira requested review from danmcd and travispaul May 4, 2026 20:09
Copy link
Copy Markdown

@danmcd danmcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still too-few-columns in some cases, I think.

Comment thread lib/client.js
err: err,
accesskeyid: accesskeyid
}, 'scopeRevoke: mahi call failed' +
' (non-fatal)');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, wouldn't this line and the prior on join together so it looks like:

        if (err) {                  
            if (self.http.log) {
                self.http.log.warn({
                    err: err,
                    accesskeyid: accesskeyid
                }, 'scopeRevoke: mahi call failed (non-fatal)');
            }

Comment thread lib/scope-schema.js
scope: scopeJson,
error: null
});
return ({valid: true, scope: scopeJson, error: null});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one actually makes sense to stay on different lines. SOme of the other ones above are more what I had in mind.

Comment thread lib/scope-schema.js
error: 'scope: permissions[' + i +
'].bucket: invalid characters' +
' or wildcard position'
'].bucket: invalid characters' + ' or wildcard position'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, stuff like this is what I was talking about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants