Skip to content
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

feat(auth): secure admin api with hmac signatures #2709

Merged
merged 9 commits into from
Jun 11, 2024

Conversation

njlie
Copy link
Contributor

@njlie njlie commented May 7, 2024

Changes proposed in this pull request

  • Added signature header verification to Auth Server Admin API

Context

Closes #2704.

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass
  • Bruno collection updated

@github-actions github-actions bot added type: source Changes business logic pkg: auth Changes in the GNAP auth package. labels May 7, 2024
Copy link

netlify bot commented May 7, 2024

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit eec672b
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/66623d2e094c56000808d87b

bruno/collections/Rafiki/scripts.js Outdated Show resolved Hide resolved
bruno/collections/Rafiki/scripts.js Outdated Show resolved Hide resolved
packages/auth/src/shared/utils.ts Outdated Show resolved Hide resolved
packages/auth/src/config/app.ts Outdated Show resolved Hide resolved
@njlie njlie force-pushed the nl/2704/auth-admin-api-security branch from 7ea0902 to 5bd359b Compare May 16, 2024 21:42
@github-actions github-actions bot added the type: tests Testing related label May 21, 2024
@njlie njlie force-pushed the nl/2704/auth-admin-api-security branch from f75f811 to cc173b6 Compare May 22, 2024 21:03
BlairCurrey
BlairCurrey previously approved these changes May 23, 2024

async function canApiSignatureBeProcessed(
signature: string,
ctx: Context,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be good to type this Context such that we get the types for the services like redis and logger.

const key = `signature:${signature}`
const op = redis.multi()
op.set(key, signature)
op.expire(key, signature)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the expiry take in a num as the second argument?

},
'time differential'
)
if (currentTime - signatureTime > ttlMilliseconds) return false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add a test for this as well

@mkurapov mkurapov merged commit 8c601a5 into main Jun 11, 2024
42 checks passed
@mkurapov mkurapov deleted the nl/2704/auth-admin-api-security branch June 11, 2024 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: auth Changes in the GNAP auth package. type: source Changes business logic type: tests Testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Secure Auth Server Admin API with signatures
4 participants