Repository navigation
feat(errors): typed secret-read refusals in all four SDKs (409 SecretUnavailable*, error code field) - #59
Open
aibeshkov wants to merge 4 commits into
Open
feat(errors): typed secret-read refusals in all four SDKs (409 SecretUnavailable*, error code field)#59aibeshkov wants to merge 4 commits into
aibeshkov wants to merge 4 commits into
Conversation
…MAP-1) The server now answers a value read the secret's own state refuses with 409 SecretExpired / SecretSuspended / SecretMaxReadsExhausted, or 403 SecretAccessOutsideSchedule, instead of 500. Expose the stable code on the base error and raise SecretUnavailable* for the three 409 codes; the schedule 403 stays Forbidden* with an accurate message. Local message text, never the server body. Session: ERRMAP-1 Signed-off-by: Andrei Beshkov <andrey.beshkov@gmail.com>
…ce on a ternary Signed-off-by: Andrei Beshkov <andrey.beshkov@gmail.com>
…nstructor throws it) Signed-off-by: Andrei Beshkov <andrey.beshkov@gmail.com>
Contributor
Author
|
Coordinator review (opus) at 08cf046: problems
Code itself looks right and is backward compatible:
Merge order: after keyorix #3175. It is safe either way, because an unknown 409 or code falls back to the generic error. |
keyorix main requires MFA by default (ADR-112), so the contract suites' password login stopped at the MFA step. A bootstrap script now creates the admin, enrols TOTP from the returned seed, completes the real MFA login and mints a short-lived PAT that every SDK suite uses. Each suite also asserts a password-only login still returns an MFA challenge. The role-less machine token expectation accepts the current empty list as well as 403. Signed-off-by: Andrei Beshkov <andrey.beshkov@gmail.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
STACKED ON keyorixhq/keyorix#3175 — merge after it (it is the server change; this PR is backward compatible either way).
NEEDS ANDREI (.github): this PR now also touches
.github/workflows/contract.yml(one new step, "Bootstrap contract admin (MFA stays on)", plus a header comment). Please review that file.This is the SDK half of keyorixhq/keyorix#3175 (ERRMAP-1). The server now answers a value read that the secret's own state refuses with a client error and a stable code, instead of 500:
errorSecretAccessOutsideScheduleSecretExpiredSecretSuspendedSecretMaxReadsExhaustedThe change is the same in all four SDKs and backward compatible:
APIError.CodeKeyorixError.codeKeyorixError.errorCode(notcode, which Node reserves for system errors)KeyorixException.getErrorCode()SecretUnavailableError(Go, Python, Node) andSecretUnavailableException(Java).*APIError. In the other SDKs it subclasses the base error.ForbiddenError/ForbiddenException, so existing handlers keep working. Its message now says "the secret is outside its access schedule" instead of "token lacks permission".Tests
Each SDK adds a test that drives a by-ref value read against a stub answering each refusal. It asserts the type, the code, a local message, and that the server text is never echoed. A test for an unrelated 409 is included where the SDK has one.
go vet ./... && go test ./...: ok.TestErrorForResponse_SecretReadRefusalspasses.python3 -m unittest test_keyorix:Ran 21 tests ... OK.node test.js:59 passed, 0 failed.Tests run: 50, Failures: 0, Errors: 0, Skipped: 7(ContractTest skips without a server, as in the unit job).BUILD SUCCESS.The contract tests pin a server ref (
.keyorix-server-ref) and do not exercise these codes yet.Contract harness fix (SDK-CONTRACT-1)
The contract job was red for every SDK PR: keyorix main enforces MFA by default (
security.require_mfa, ADR-112), so the suites' password login stopped at the MFA step. MFA is not turned off anywhere (norequire_mfa:false, no server change).scripts/contract_bootstrap.py(new, run once by the workflow):/system/init, password login (setup-only session),/auth/mfa/enroll, TOTP computed from the returned seed (test-only RFC 6238 helper),/auth/mfa/activate, password login,/auth/mfa/verify, then mints a 24 h PAT. The PAT goes to$GITHUB_ENVasKEYORIX_CONTRACT_ADMIN_TOKENwith::add-mask::; it is never printed. The PAT is unscoped because the suites create projects, secrets, machine identities and PATs, and a scoped PAT cannot mint PATs.KEYORIX_CONTRACT_ADMIN_TOKENinstead of logging in with a password.mfa_requiredplus a challenge and no session token. Each makes one login call, because a challenge login keeps its rate-limit slot (10 per 15 min per IP).~/.m2), so it is verified by CI only.Session: ERRMAP-1
Session: SDK-CONTRACT-1