-
Notifications
You must be signed in to change notification settings - Fork 102
feat(backend): tenant signature validation for admin api #3164
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
Merged
njlie
merged 16 commits into
2893/multi-tenancy-v1
from
nl/2928/multi-tenancy-signatures
Dec 17, 2024
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
5a04f4d
feat(auth): tenants table v1
njlie 49e335e
feat(backend): tenant service
njlie 12500cc
feat: use soft delete
njlie 54e62ef
feat: add idp columns to tenant model
njlie 10e2922
feat: pagination tests, push deletedAt to auth api call
njlie 56d51e4
feat: add cache
njlie f7fe54b
feat(backend): tenant signature validation for admin api
njlie 11e91a6
fix: rebase errors
njlie 496df5e
fix: remove admin api secret check from app
njlie a78e1ad
fix: always expect tenant id in request
njlie f9437dc
chore: remove some logs
njlie 1e6a93a
feat: await signature verification, test improvements
njlie 6ae0cf6
fix: better util parameters
njlie 23b121d
fix: add tenant info to apollo context
njlie 8994937
feat: fix integration tests
njlie 39b663c
fix: make tenant required on extended apollo context
njlie 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
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { createHmac } from 'crypto' | |||||
| import { canonicalize } from 'json-canonicalize' | ||||||
| import { IAppConfig } from '../config/app' | ||||||
| import { AppContext } from '../app' | ||||||
| import { Tenant } from '../tenants/model' | ||||||
|
|
||||||
| export function validateId(id: string): boolean { | ||||||
| return validate(id) && version(id) === 4 | ||||||
|
|
@@ -126,7 +127,8 @@ function getSignatureParts(signature: string) { | |||||
| function verifyApiSignatureDigest( | ||||||
| signature: string, | ||||||
| request: AppContext['request'], | ||||||
| config: IAppConfig | ||||||
| adminApiSignatureVersion: number, | ||||||
| secret: string | ||||||
| ): boolean { | ||||||
| const { body } = request | ||||||
| const { | ||||||
|
|
@@ -135,12 +137,12 @@ function verifyApiSignatureDigest( | |||||
| timestamp | ||||||
| } = getSignatureParts(signature as string) | ||||||
|
|
||||||
| if (Number(signatureVersion) !== config.adminApiSignatureVersion) { | ||||||
| if (Number(signatureVersion) !== adminApiSignatureVersion) { | ||||||
| return false | ||||||
| } | ||||||
|
|
||||||
| const payload = `${timestamp}.${canonicalize(body)}` | ||||||
| const hmac = createHmac('sha256', config.adminApiSecret as string) | ||||||
| const hmac = createHmac('sha256', secret) | ||||||
| hmac.update(payload) | ||||||
| const digest = hmac.digest('hex') | ||||||
|
|
||||||
|
|
@@ -171,6 +173,53 @@ async function canApiSignatureBeProcessed( | |||||
| return true | ||||||
| } | ||||||
|
|
||||||
| export interface TenantApiSignatureResult { | ||||||
| tenant: Tenant | ||||||
| isOperator: boolean | ||||||
| } | ||||||
|
|
||||||
| /* | ||||||
| Verifies http signatures by first attempting to replicate it with a secret | ||||||
| associated with a tenant id in the headers. | ||||||
|
|
||||||
| If a tenant secret can replicate the signature, the request is tenanted to that particular tenant. | ||||||
| If the environment admin secret matches the tenant's secret, then it is an operator request with elevated permissions. | ||||||
| If neither can replicate the signature then it is unauthorized. | ||||||
| */ | ||||||
| export async function getTenantFromApiSignature( | ||||||
| ctx: AppContext, | ||||||
| config: IAppConfig | ||||||
| ): Promise<TenantApiSignatureResult | undefined> { | ||||||
| const { headers } = ctx.request | ||||||
| const signature = headers['signature'] | ||||||
| if (!signature) { | ||||||
| return undefined | ||||||
| } | ||||||
|
|
||||||
| const tenantService = await ctx.container.use('tenantService') | ||||||
| const tenantId = headers['tenant-id'] | ||||||
| const tenant = tenantId ? await tenantService.get(tenantId) : undefined | ||||||
|
|
||||||
| if (!tenant) return undefined | ||||||
|
|
||||||
| if (!(await canApiSignatureBeProcessed(signature as string, ctx, config))) | ||||||
| return undefined | ||||||
|
|
||||||
| if ( | ||||||
| tenant.apiSecret && | ||||||
| verifyApiSignatureDigest( | ||||||
| signature as string, | ||||||
| ctx.request, | ||||||
| config.adminApiSignatureVersion, | ||||||
| tenant.apiSecret | ||||||
| ) | ||||||
| ) { | ||||||
| return { tenant, isOperator: tenant.apiSecret === config.adminApiSecret } | ||||||
| } | ||||||
|
|
||||||
| return undefined | ||||||
| } | ||||||
|
|
||||||
| export async function verifyApiSignature( | ||||||
| ctx: AppContext, | ||||||
| config: IAppConfig | ||||||
|
|
@@ -184,5 +233,10 @@ export async function verifyApiSignature( | |||||
| if (!(await canApiSignatureBeProcessed(signature as string, ctx, config))) | ||||||
| return false | ||||||
|
|
||||||
| return verifyApiSignatureDigest(signature as string, ctx.request, config) | ||||||
| return verifyApiSignatureDigest( | ||||||
| signature as string, | ||||||
| ctx.request, | ||||||
| config.adminApiSignatureVersion, | ||||||
| config.adminApiSecret as string | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Since it should already be typed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: but we can pass in either whole of |
||||||
| ) | ||||||
| } | ||||||
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.
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.
should we return false if we can't find the tenant?
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 idea here is to account for non-tenanted requests (e.g. createTenant, listTenants, other pagination requests). But this is moot since our call where the tenant id is to be expected in any request.