-
Notifications
You must be signed in to change notification settings - Fork 14
MANTA-5523 Authorization layer must be aware of bucket scoped accesskeys #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
cneira
wants to merge
38
commits into
master
Choose a base branch
from
accesskey-per-bucket
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 35 commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
3eea637
replicator: replicate accesskeyscope field to Redis
cneira da09e79
sigv4: return bucket scope in verification response
cneira 4f30c64
sts: inherit parent key bucket scope in temporary credentials
cneira 689d3d2
fix: per-bucket access key scope — 5 bugs fixed, unit tests added
cneira fb097a3
style: jsstyle fixes — typeof parens, return parens, block comments
cneira 5cba714
fix: handle scoped permanent key JSON in getUserByAccessKey
cneira 7f0e4aa
fix: return bucketScope from getUserByAccessKey for STS/IAM routing
cneira 91bd2fe
fix: return bucketScope from UFDS fallback path in sigv4 temp credent…
cneira 871937d
fix: replicator preserves bucketScope in temp credential reverse lookup
cneira 1774f55
per-bucket scope hardening: unified Redis format, operator endpoints,…
cneira 62141a1
sigv4: UFDS read-through fallback for permanent key cache miss
cneira 821de34
fix: guard JSON.parse in sigv4, return 403 for invalid keys
cneira 15fca86
sigv4: negative cache for Redis-only permanent credential path
cneira a58015d
sigv4: gate negative cache on !ufds, add debug log
cneira 4952ade
sigv4: replace Object.keys size check with counter
cneira ce3405e
replicator: set UFDS_POLL_INTERVAL default and cache-push documentation
cneira ea62bde
fix up
cneira 1dd2535
fix: use plain default for UFDS poll interval in mahi2 template
cneira b0fed95
Fix UFDS pool connections silently dropping after 90s idle
cneira 716169e
Fix UFDS pool validate() always returning true, raise acquireTimeoutM…
cneira 11b7682
refactor: parse bucket scope once in loadCaller, add cross-path tests…
cneira 0d00819
fix: wire UFDS pool into sigv4 verification for permanent key fallback
cneira 39482e1
refactor: extract shared Redis entry builder for permanent access keys
cneira 55db59c
deps: pin node-ufds to git commit f8ea6ad (v1.9.2, idleTimeout fix)
cneira 3c7d6dc
harden: sigv4 decomposition, durable revoke, write versioning
cneira b560111
sts: use "none" sentinel for unscoped parent key temp credentials
cneira 5dfd1d5
fix: use explicit null check in buildPermanentKey builders
cneira 437a6b7
fix: guard JSON.parse in cachePush and scope-revoke handlers
cneira 24be590
fix: use explicit null check for version in Redis builders
cneira f7096ef
harden: changenumber ceiling check, scope validation, negative
cneira 62fc282
harden: rename handler to keyRevokeHandler, warn on negative
cneira e79e823
fix: complete || null eradication, LDAP filter sanitization,
cneira 44b6986
fix: rewrite legacy format comment to describe rolling upgrade
cneira 55dfb12
cleanup: remove complexity annotations from endpoint JSDoc
cneira b813256
docs: add operator endpoints documentation
cneira 82c0ea9
Catch invalid changenumber
cneira d3d95a3
Fix horizontal white space
cneira 74afe0f
Remove reference to development bugs found.
cneira File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| # Operator Endpoints | ||
|
|
||
| Mahi exposes two internal endpoints for direct Redis manipulation. | ||
| These are on the mahi server port (80) and are not exposed to | ||
| external S3 clients. | ||
|
|
||
| ## POST /cache-push/:accesskeyid | ||
|
|
||
| Writes an access key directly to Redis, bypassing the replicator | ||
| poll interval (~2s). Called by CloudAPI after creating or updating | ||
| keys for immediate availability. | ||
|
|
||
| **Request body:** | ||
|
|
||
| | Field | Type | Required | Description | | ||
| |-------|------|----------|-------------| | ||
| | accesskeysecret | string | yes | Secret access key | | ||
| | ownerUuid | string | yes | Owner account UUID | | ||
| | status | string | no | Key status (default: "Active") | | ||
| | scope | string\|null | no | Scope JSON string or null | | ||
|
|
||
| **Behavior:** | ||
|
|
||
| - Active keys are written in the unified format via the shared | ||
| builder (`redis-accesskey-format.js`), identical to the | ||
| replicator output. | ||
| - Inactive keys are removed from Redis (reverse lookup deleted, | ||
| key entry removed from user record). | ||
| - Uses `Date.now()` as the write version, which is always greater | ||
| than the replicator's changenumber. This ensures cachePush | ||
| always wins over a concurrent replicator write. | ||
| - Idempotent. Repeated calls overwrite the previous entry. | ||
| - Best-effort: failure is logged at warn level by the CloudAPI | ||
| caller but does not block the CloudAPI response. | ||
|
|
||
| **Example:** | ||
|
|
||
| ``` | ||
| curl -X POST http://authcache.coal.joyent.us/cache-push/abc123 \ | ||
| -H 'Content-Type: application/json' \ | ||
| -d '{ | ||
| "accesskeysecret": "tdc_...", | ||
| "ownerUuid": "fe3617d8-...", | ||
| "status": "Active", | ||
| "scope": "{\"version\":1,\"permissions\":[{\"bucket\":\"my-bucket\",\"level\":\"read\"}]}" | ||
| }' | ||
| ``` | ||
|
|
||
| ## POST /key-revoke/:accesskeyid | ||
|
|
||
| Removes an access key from Redis immediately and writes a | ||
| revocation tombstone with a 24-hour TTL. The replicator checks | ||
| for the tombstone before writing a key — if present, the write | ||
| is skipped, making the revocation durable across replication | ||
| cycles. | ||
|
|
||
| Use for emergency revocation of compromised keys without waiting | ||
| for the UFDS delete to propagate through the replicator. | ||
|
|
||
| **Request body:** None required. | ||
|
|
||
| **Behavior:** | ||
|
|
||
| - Deletes the key entry from the user record in Redis. | ||
| - Deletes the reverse lookup at `/accesskey/:accesskeyid`. | ||
| - Writes a tombstone at `/revoked/:accesskeyid` with a 24-hour | ||
| TTL (`SETEX`). | ||
| - Repeated calls renew the tombstone TTL. | ||
| - Returns 404 if the key is not found in Redis. | ||
|
|
||
| **Important:** Revocation is temporary. The replicator will | ||
| re-add the key on its next cycle once the tombstone expires | ||
| (24 hours) if the key still exists in UFDS. To permanently | ||
| revoke a key: | ||
|
|
||
| 1. Call `DELETE /:account/accesskeys/:id` via CloudAPI | ||
| (which deletes from UFDS and calls key-revoke automatically). | ||
| 2. Or: call key-revoke for immediate effect, then delete | ||
| from UFDS within 24 hours. | ||
|
|
||
| **Example:** | ||
|
|
||
| ``` | ||
| curl -X POST http://authcache.coal.joyent.us/key-revoke/abc123 | ||
| ``` | ||
|
|
||
| **Response:** | ||
|
|
||
| ```json | ||
| { | ||
| "revoked": true, | ||
| "accessKeyId": "abc123", | ||
| "userUuid": "fe3617d8-...", | ||
| "tombstoneTtlSeconds": 86400, | ||
| "replicationWarning": "Key removed from Redis cache and revocation tombstone set (86400s TTL). Delete from UFDS via CloudAPI to permanently revoke." | ||
| } | ||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| /* | ||
| * This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. | ||
| */ | ||
|
|
||
| /* | ||
| * Copyright 2026 Edgecast Cloud LLC. | ||
| */ | ||
|
|
||
| /* | ||
| * Canonical Redis entry builders for permanent access keys. | ||
| * | ||
| * Two code paths write permanent key data to Redis: | ||
| * 1. Replicator transforms (UFDS → Redis sync) | ||
| * 2. cachePush endpoint (CloudAPI → Redis shortcut) | ||
| * | ||
| * Both must produce identical structures. This module | ||
| * is the single source of truth for the Redis format | ||
| * so the invariant is enforced by construction, not by | ||
| * convention. | ||
| * | ||
| * Permanent key format (in /uuid/{uuid}.accesskeys): | ||
| * { secret: string, scope: string|null } | ||
| * | ||
| * Reverse lookup format (in /accesskey/{keyId}): | ||
| * { type: "accesskey", accessKeyId, userUuid, | ||
| * credentialType: "permanent", scope: string|null } | ||
| * | ||
| * Scope values for permanent keys: | ||
| * null — key is unscoped (unrestricted access) | ||
| * JSON str — key is scoped (e.g. '{"version":1,...}') | ||
| * "" — preserved as-is; downstream parseScope | ||
| * returns null, causing fail-closed deny | ||
| * | ||
| * Scope values for STS temporary credentials (in sts.js, | ||
| * NOT built by this module): | ||
| * "none" — parent key was explicitly unscoped | ||
| * JSON str — inherited from scoped parent key | ||
| * null — legacy pre-sentinel temp credential | ||
| */ | ||
|
|
||
|
|
||
| /** | ||
| * @brief Build the accesskeys map entry for a permanent key | ||
| * | ||
| * Stored at /uuid/{ownerUuid}.accesskeys[accessKeyId]. | ||
| * | ||
| * @param {string} secret - Secret access key | ||
| * @param {string|null} scope - Scope JSON string or null | ||
| * @param {number} [version] - Write version (changenumber or | ||
| * 0 for unversioned). Used by the replicator and cachePush | ||
| * to prevent stale writes from overwriting newer data. | ||
| * @return {Object} { secret, scope, version } | ||
| */ | ||
| function buildPermanentKeyEntry(secret, scope, version) { | ||
| /* | ||
| * Warn on malformed scope JSON but store the value | ||
| * as-is. Downstream parseScope() will fail to parse | ||
| * it and enforceBucketScope will deny the request | ||
| * (fail-closed). We do NOT replace with null because | ||
| * null means "unrestricted" (fail-open). We do NOT | ||
| * throw because callers are in async callback chains | ||
| * without try/catch guards. | ||
| */ | ||
| if (scope != null && scope !== '') { | ||
| try { | ||
| JSON.parse(scope); | ||
| } catch (e) { | ||
| /* global console */ | ||
| console.error( | ||
| 'buildPermanentKeyEntry: invalid ' + | ||
| 'scope JSON (storing as-is, will ' + | ||
| 'fail-closed on enforcement): ' + | ||
| e.message); | ||
| } | ||
| } | ||
| return ({ | ||
| secret: secret, | ||
| scope: (scope != null) ? scope : null, | ||
| version: (version != null) ? version : 0 | ||
| }); | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * @brief Build the reverse-lookup entry for a permanent key | ||
| * | ||
| * Stored at /accesskey/{accessKeyId}. | ||
| * | ||
| * @param {string} accessKeyId - Access key ID | ||
| * @param {string} userUuid - Owner UUID | ||
| * @param {string|null} scope - Scope JSON string or null | ||
| * @param {number} [version] - Write version (see | ||
| * buildPermanentKeyEntry) | ||
| * @return {Object} Reverse-lookup entry | ||
| */ | ||
| function buildPermanentKeyLookup(accessKeyId, userUuid, scope, version) { | ||
| return ({ | ||
| type: 'accesskey', | ||
| accessKeyId: accessKeyId, | ||
| userUuid: userUuid, | ||
| credentialType: 'permanent', | ||
| scope: (scope != null) ? scope : null, | ||
| version: (version != null) ? version : 0 | ||
| }); | ||
| } | ||
|
|
||
|
|
||
| /* | ||
| * Revocation tombstone constants and helpers. | ||
| * | ||
| * When an operator calls POST /scope-revoke/:accesskeyid, | ||
| * a tombstone key is written to Redis with a TTL. The | ||
| * replicator checks for the tombstone before writing a key | ||
| * to Redis — if present, the write is skipped, making the | ||
| * revocation durable across replication cycles. | ||
| * | ||
| * The tombstone auto-expires after REVOKE_TTL_SECONDS. | ||
| * The operator should delete the key from UFDS (via CloudAPI) | ||
| * before the tombstone expires to make revocation permanent. | ||
| * Repeated scope-revoke calls renew the tombstone TTL. | ||
| */ | ||
| var REVOKE_TTL_SECONDS = 86400; // 24 hours | ||
|
|
||
|
|
||
| /** | ||
| * @brief Build the Redis key path for a revocation tombstone | ||
| * | ||
| * @param {string} accesskeyid - Access key ID | ||
| * @return {string} Redis key path | ||
| */ | ||
| function revokedKeyPath(accesskeyid) { | ||
| return ('/revoked/' + accesskeyid); | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * @brief Build the revocation tombstone value | ||
| * | ||
| * @param {string} userUuid - Owner UUID of the revoked key | ||
| * @return {Object} Tombstone data | ||
| */ | ||
| function buildRevocationTombstone(userUuid) { | ||
| return ({ | ||
| revokedAt: Date.now(), | ||
| userUuid: userUuid | ||
| }); | ||
| } | ||
|
|
||
|
|
||
| module.exports = { | ||
| buildPermanentKeyEntry: buildPermanentKeyEntry, | ||
| buildPermanentKeyLookup: buildPermanentKeyLookup, | ||
| REVOKE_TTL_SECONDS: REVOKE_TTL_SECONDS, | ||
| revokedKeyPath: revokedKeyPath, | ||
| buildRevocationTombstone: buildRevocationTombstone | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I want this key GONE, wouldn't it make more sense to have THIS endpoint, after marking the tombstone, just generate the above DELETE?
Or are there cases people actually want to revoke for 24 hours, figure it out, and maybe let it go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only good thing of doing it in two steps, is that if you made a mistake the next UFDS sync will correct it. But I agree that doing this into steps just is more complex, because if you forget to do the second step they keys will be there again.