Skip to content

FE-6296 Enable scoped database and role with user-provided keys#558

Merged
ptpaterson merged 12 commits intomainfrom
fe-6296-secret-handling
Jan 17, 2025
Merged

FE-6296 Enable scoped database and role with user-provided keys#558
ptpaterson merged 12 commits intomainfrom
fe-6296-secret-handling

Conversation

@ptpaterson
Copy link
Contributor

@ptpaterson ptpaterson commented Jan 13, 2025

Problem

You can provide a scoped key by specifying --secret some_key:child/db:role, but sometimes it is more convenient to split up the auth config into --secret some_key -d child/db -r role. This is especially true if you have a config profile that provides a secret that you sometimes want to scope down, e.g. -p test-db -d child.

Solution

Add middleware that mutates a secret provided with --secret (or whatever secret is set if added by prior middleware, such are --local) to scope the key with the provided database and/or role.

In the case of using --local or the local command, we can remove the special handing of secret, role and database there, since it will be handled for all cases by the new middleware.

Result

For all commands that accept --secret, --database and --role options can now also be provided.

Use of credentials files

Using the user-provided secret will now avoid digging through file-based database keys before falling back on the provided secret. To do this, I updated the getSecret() function to accept argv and short circuit if argv.secret already exists. It was brought up that the getSecret() function is already a bit of a hack, but I kept this function in place to reduce code duplication: the alternative I saw was to replace getSecret() with argv.secret ?? (await getSecret()) everywhere, and that seemed to add more complexity. I still moved the key-refreshing bit into a class method to keep those details out of the logic that forks at provided key vs credentials file key.

The Credentials singletons are still constructed with the container initialization, so we are not avoiding it entirely. Additionally, the check for whether or not to retry 401s with a refreshed key goes through Credentials (see DatabaseKeys.onInvalidCreds() method). A future optimization might be to not initialize the credentials files with the container, but perform the file I/O when called for and memoize for additional calls. But I was trying to avoid anything that invasive with this PR.

Example

❯ ./src/user-entrypoint.mjs query -p test-db -d child --verbosity 5 '42'
[config]: Reading config from /Users/psquared/.fauna/fauna.config.yaml.
[config]: Using profile test-db...
[config]: Applying config: {
    "secret": "fnAF********7Dp5"
}
[config]: Reading config from /Users/psquared/.fauna/fauna.config.yaml.
[config]: Using profile test-db...
[config]: Applying config: {
    "secret": "fnAF********7Dp5"
}
[argv]: {
    "_": [
        "query"
    ],
    "p": "test-db",
    "profile": "test-db",
    "d": "child",
    "database": "child",
    "verbosity": 5,
    "config": "/Users/psquared/.fauna/fauna.config.yaml",
    "secret": "fnAF********7Dp5",
    "color": true,
    "json": false,
    "quiet": false,
    "verbose-component": [],
    "verboseComponent": [],
    "account-url": "https://account.fauna.com",
    "accountUrl": "https://account.fauna.com",
    "user": "default",
    "u": "default",
    "local": false,
    "api-version": "10",
    "v": "10",
    "apiVersion": "10",
    "format": "fql",
    "f": "fql",
    "timeout": 5000,
    "performance-hints": false,
    "performanceHints": false,
    "include": [
        "summary"
    ],
    "$0": "fauna",
    "fql": "42"
}
[argv]: Existing Fauna environment variables: {"FAUNA_CONFIG":"/Users/psquared/.fauna/fauna.config.yaml","FAUNA_PROFILE":"none"}
[argv]: Defaulted url to 'https://db.fauna.com' no --url was provided
[argv]: Applying scope to secret 'fnAF********7Dp5:child:admin', since --database was 'child' with default role 'admin'
[argv]: no --input specified, using [fql]
42

Testing

Added some new tests to cover the scopeSecret middleware. This is really just taking over the existing tests that were covered by applyLocalArg middleware before.

@ptpaterson ptpaterson requested a review from a team as a code owner January 13, 2025 18:24
@ptpaterson ptpaterson requested a review from a team January 14, 2025 18:25
@ptpaterson ptpaterson merged commit 68f87fe into main Jan 17, 2025
4 checks passed
@ptpaterson ptpaterson deleted the fe-6296-secret-handling branch January 17, 2025 14:26
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.

4 participants