SK-2679: Add JSON object context support for Conditional Data Access#293
SK-2679: Add JSON object context support for Conditional Data Access#293samsternberg wants to merge 3 commits intomainfrom
Conversation
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
The ctx claim in bearer tokens and signed data tokens previously only accepted a String, which meant structured CEL expressions like request.context.role == 'admin' could not be satisfied. Add setCtx(Map<String, Object>) overloads to BearerToken and SignedDataTokens builders so the JWT ctx claim is serialized as a nested JSON object. Also add setContext(Map) and getContextAsObject() to Credentials for use with the high-level Skyflow client. All changes are backwards compatible — existing setCtx(String) and setContext(String) APIs are unchanged. Refs SK-2679 Co-Authored-By: Claude <noreply@anthropic.com>
samsternberg
force-pushed
the
feature/SK-2679-conditional-data-access-ctx-map-support
branch
from
67c6762 to
c683024
Compare
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
| return sb.toString(); | ||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") |
There was a problem hiding this comment.
Why is @SuppressWarnings("unchecked") needed here? Can you add a comment explaining this?
There was a problem hiding this comment.
Good catch — rather than just adding a comment, I refactored to address the root cause. Added a typed getContextAsMap() getter to Credentials (which owns the invariant via its typed setters), so call sites in Utils.generateBearerToken no longer need a cast at all. The @SuppressWarnings now lives in Credentials directly next to the field and setters that guarantee the type is safe. See commit 4f83032.
| import com.skyflow.vault.tokens.TokenizeRequest; | ||
|
|
||
| public class Validations { | ||
| private static final Pattern CTX_MAP_KEY_PATTERN = Pattern.compile("^[a-zA-Z0-9_]+$"); |
There was a problem hiding this comment.
Use constants instead hard-coded strings
There was a problem hiding this comment.
Fixed — moved the regex to Constants.CTX_MAP_KEY_REGEX (same pattern as API_KEY_REGEX) and updated Validations to reference it. See commit 364b8bb.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…d cast in Utils The suppression now lives in Credentials next to the typed setters that guarantee the invariant, keeping call sites in Utils cast-free. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Closing in favour of #302, which re-applies these changes cleanly on top of main after SK-2777 landed and caused conflicts. |
2 participants
Summary
setCtx(Map<String, Object>)overloads toBearerTokenandSignedDataTokensbuilders so the JWTctxclaim can be a nested JSON object (required for Conditional Data Access CEL expressions likerequest.context.role == 'admin')setContext(Map<String, Object>)andgetContextAsObject()toCredentialsfor high-level Skyflow client usagesetCtx(String)andsetContext(String)APIs unchangedChanges
BearerToken.java,SignedDataTokens.java— widen internalctxfield toObject, addMapoverloads to buildersCredentials.java— addsetContext(Map)setter andgetContextAsObject()getter (preservesgetContext()returningString)Utils.java— dispatch context to correctsetCtxoverload based on typeValidations.java— validate emptyMapcontext same as emptyStringBearerTokenTests,SignedDataTokensTests,CredentialsTestsBearerTokenGenerationWithContextExampleandSignedTokenGenerationExamplewith Map context examplesReplaces #291 (which was created from a fork and couldn't run CI).
Test plan
mvn compile— cleanmvn test -Dtest=BearerTokenTests,SignedDataTokensTests,CredentialsTests— 51 tests, 0 failuresRefs SK-2679, DOCU-1438