-
Notifications
You must be signed in to change notification settings - Fork 535
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
Adds support for hidden private keys in server tenants #23379
Open
dhr-verma
wants to merge
28
commits into
microsoft:main
Choose a base branch
from
dhr-verma:vermadhr/keylessAccessWork
base: main
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.
Conversation
This file contains 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
…to vermadhr/keylessAccessWork
github-actions
bot
added
area: server
Server related issues (routerlicious)
base: main
PRs targeted against main branch
labels
Dec 19, 2024
dhr-verma
requested review from
znewton,
yunho-microsoft,
pradeepvairamani,
manishgargd,
zhangxin511 and
zhajie
and removed request for
Copilot
December 19, 2024 19:58
manishgargd
reviewed
Dec 19, 2024
server/routerlicious/packages/routerlicious-base/src/riddler/api.ts
Outdated
Show resolved
Hide resolved
manishgargd
reviewed
Dec 19, 2024
znewton
reviewed
Dec 19, 2024
server/routerlicious/packages/routerlicious-base/src/riddler/api.ts
Outdated
Show resolved
Hide resolved
manishgargd
reviewed
Dec 19, 2024
manishgargd
reviewed
Dec 19, 2024
manishgargd
reviewed
Dec 19, 2024
server/routerlicious/packages/routerlicious-base/src/riddler/tenantManager.ts
Show resolved
Hide resolved
manishgargd
reviewed
Dec 19, 2024
server/routerlicious/packages/routerlicious-base/src/riddler/tenantManager.ts
Outdated
Show resolved
Hide resolved
manishgargd
reviewed
Dec 19, 2024
server/routerlicious/packages/routerlicious-base/src/riddler/tenantManager.ts
Outdated
Show resolved
Hide resolved
manishgargd
reviewed
Dec 19, 2024
server/routerlicious/packages/routerlicious-base/src/riddler/tenantManager.ts
Outdated
Show resolved
Hide resolved
manishgargd
reviewed
Dec 20, 2024
server/routerlicious/packages/routerlicious-base/src/riddler/tenantManager.ts
Show resolved
Hide resolved
…to vermadhr/keylessAccessWork
…Access a mandatory prop
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
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.
Description
This PR adds support for storing private keys in the tenants DB collection. These private keys will be used to sign tokens that are acquired for keyless access. These are never shared with the client and will be rotated based on their rotation schedule. It also adds a new claim in the fluid access token -
fluidRelayKeylessAccess
. This claim will be used to determine how to validate the access token - with the shared keys or the private keys. This allows to keep access tokens to be backward compatible while also supporting keyless access. It does introduce some new APIs and modifies existing APIs:/tenants/:id/keys
: Adds a new query parameter -getPrivateKeys
. When set totrue
, it returns the privateKeys for the tenant. By default, this flag is set to false./tenants/:id/keylessAccess
: Toggles theenableKeylessAccess
flag for a tenant./tenants/:id/key
: Refreshes the requested tenant key. WhenrefreshPrivateKey
istrue
it will refresh the private keys but does not return the refreshed keys. This is because key refreshes of the private keys will not be exposed to clients./tenants/:id?
: Creates a fluid tenant. This request accepts a new parameter in the body -enableKeylessAccess
. When this is set to true, a tenant with support for private keys will be created. By default, this is false.To support these, the following changes are done:
ITenantConfig
interface to have two new mandatory props -enableKeylessAccess
andenableKeyAccess
. These are computed at runtime by thetenantManager
. Since these are not stored in the DB these can safely be made mandatory without breaking backward compatability.ITenantPrivateKeys
. The workflows essentially are the same as before but they include some new flags which helps riddler determine what keys to get/refresh/use for validation. The changes also include creating different caching keys for shared and private keys.fluidRelayKeylessAccess
which will be set totrue
when generating a keyless access token.tenantManager.ts
. This ensures the existing APIs work as before. It also tests the new functionality. To further test these changes, I ran the server locally using Docker.Breaking Changes
This adds a bunch of functionalities to support private hidden keys. However, these should not break existing changes.
Reviewer Guidance
getTenantKeys
rather than creating new interfaces.TODO: Add changesets