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

Merged
merged 3 commits into from
Nov 9, 2022
Merged

feat(auth): fix httpsig bypass env #735

merged 3 commits into from
Nov 9, 2022

Conversation

njlie
Copy link
Contributor

@njlie njlie commented Nov 8, 2022

Changes proposed in this pull request

Conditionally adds httpsig middleware depending on the setting of the env variable for bypassing httpsig validation.

Context

This conditional used to be in the code but refactors of app.ts have caused it to be removed.

Checklist

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

@github-actions github-actions bot added pkg: auth Changes in the GNAP auth package. type: source Changes business logic labels Nov 8, 2022
wilsonianb
wilsonianb previously approved these changes Nov 8, 2022
Copy link
Contributor

@wilsonianb wilsonianb left a comment

Choose a reason for hiding this comment

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

This should work until:

at which point OpenAPI validation will fail without the signature headers.
See:

@@ -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?

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.

@github-actions github-actions bot added the type: tests Testing related label Nov 8, 2022
@njlie njlie requested a review from wilsonianb November 8, 2022 18:42
wilsonianb
wilsonianb previously approved these changes Nov 8, 2022
})

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.

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.

2 participants