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.

Sign up for GitHub

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): fix httpsig bypass env #735

Merged
merged 3 commits into from
Nov 9, 2022
Merged

feat(auth): fix httpsig bypass env #735

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba95ed7
feat(auth): fix httpsig bypass env
njlie Nov 8, 2022
cef5e6c
feat: remove old bypass
njlie Nov 8, 2022
b322638
chore: remove appContainer array from tests
njlie Nov 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions packages/auth/src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ export class App {
path: '/',
method: HttpMethod.POST
}),
grantInitiationHttpsigMiddleware,
this.config.bypassSignatureValidation
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 existing bypass be removed?

rafiki/packages/auth/src/signature/middleware.ts

Lines 25 to 28 in d808c85

if (config.bypassSignatureValidation) {
// bypass
return true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is pretty redundant now, yeah.

? (ctx, next) => next()
: grantInitiationHttpsigMiddleware,
grantRoutes.create
)

Expand All @@ -206,7 +208,9 @@ export class App {
path: '/continue/{id}',
method: HttpMethod.POST
}),
grantContinueHttpsigMiddleware,
this.config.bypassSignatureValidation
? (ctx, next) => next()
: grantContinueHttpsigMiddleware,
grantRoutes.continue
)

Expand All @@ -217,7 +221,9 @@ export class App {
path: '/token/{id}',
method: HttpMethod.POST
}),
tokenHttpsigMiddleware,
this.config.bypassSignatureValidation
? (ctx, next) => next()
: tokenHttpsigMiddleware,
accessTokenRoutes.rotate
)

Expand All @@ -228,7 +234,9 @@ export class App {
path: '/token/{id}',
method: HttpMethod.DELETE
}),
tokenHttpsigMiddleware,
this.config.bypassSignatureValidation
? (ctx, next) => next()
: tokenHttpsigMiddleware,
accessTokenRoutes.revoke
)

Expand Down
49 changes: 0 additions & 49 deletions packages/auth/src/signature/middleware.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,55 +405,6 @@ describe('Signature Service', (): void => {
scope.isDone()
})

test('middleware succeeds if BYPASS_SIGNATURE_VALIDATION is true with bad signature', async (): Promise<void> => {
const altDeps = await initIocContainer({
...Config,
bypassSignatureValidation: true
})

const altContainer = await createTestApp(altDeps)
appContainers.push(altContainer)
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also revert appContainers to appContainer
https://github.com/interledger/rafiki/pull/683/files#diff-9c6c5d794c9dfa6afb4129ccc2ffb701bcdecb70308c63c7e4d615e8d903859c


nock(KEY_REGISTRY_ORIGIN)
.get(keyPath)
.reply(200, {
jwk: testClientKey.jwk,
client: TEST_CLIENT
} as ClientKey)

const ctx = await createContextWithSigHeaders(
{
headers: {
Accept: 'application/json'
},
url: '/',
method: 'POST'
},
{},
{
client: {
display: TEST_CLIENT_DISPLAY,
key: {
proof: 'httpsig',
jwk: testClientKey.jwk
}
}
},
privateKey,
altDeps
)

ctx.headers['signature'] = 'wrong-signature'

await grantInitiationHttpsigMiddleware(ctx, next)

expect(ctx.response.status).toEqual(200)
expect(next).toHaveBeenCalled()

// TODO: https://github.com/interledger/rafiki/issues/656
// scope.done()
})

test('middleware fails if client is invalid', async (): Promise<void> => {
const ctx = await createContextWithSigHeaders(
{
Expand Down
6 changes: 0 additions & 6 deletions packages/auth/src/signature/middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,6 @@ async function verifySigAndChallenge(
clientKey: JWKWithRequired,
ctx: HttpSigContext
): Promise<boolean> {
const config = await ctx.container.use('config')
if (config.bypassSignatureValidation) {
// bypass
return true
}

const sig = ctx.headers['signature'] as string
const sigInput = ctx.headers['signature-input'] as string
const challenge = sigInputToChallenge(sigInput, ctx)
Expand Down