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

Adding Origin-Agent-Cluster #286

Merged
merged 2 commits into from
Jan 17, 2021
Merged

Adding Origin-Agent-Cluster #286

merged 2 commits into from
Jan 17, 2021

Conversation

salesh
Copy link

@salesh salesh commented Jan 17, 2021

Related to #275

Copy link
Member

@EvanHahn EvanHahn left a comment

Choose a reason for hiding this comment

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

Thank you very much for doing this! It looks great but needs a few small changes.

CHANGELOG.md Outdated
@@ -1,5 +1,11 @@
# Changelog

## 4.3.2 - 2021-01-17
Copy link
Member

Choose a reason for hiding this comment

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

Could you change this heading to "unreleased"?

README.md Outdated
@@ -45,6 +45,7 @@ app.use(helmet.noSniff());
app.use(helmet.permittedCrossDomainPolicies());
app.use(helmet.referrerPolicy());
app.use(helmet.xssFilter());
app.use(helmet.originAgentCluster());
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this line because it's not enabled by default.

CHANGELOG.md Outdated

### Added

- `helmet.originAgentCluster`: disabled by default, set `true` enables it
Copy link
Member

Choose a reason for hiding this comment

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

Please change this to:

Suggested change
- `helmet.originAgentCluster`: disabled by default, set `true` enables it
- `helmet.originAgentCluster`: a new middleware for the `Origin-Agent-Cluster` middleware, disabled by default

README.md Outdated
<details>
<summary><code>helmet.originAgentCluster()</code></summary>

`helmet.originAgentCluster` The Origin-Agent-Cluster header provides a mechanism to allow web applications to isolate their origins.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`helmet.originAgentCluster` The Origin-Agent-Cluster header provides a mechanism to allow web applications to isolate their origins.
`helmet.originAgentCluster` sets the `Origin-Agent-Cluster` header, which provides a mechanism to allow web applications to isolate their origins. Read more about it [in the spec](https://whatpr.org/html/6214/origin.html#origin-keyed-agent-clusters).

index.ts Outdated
)
) {
throw new Error(
"Helmet no longer supports `true` as a middleware option, exception is Origin-Agent-Cluster. Remove the property from your options to fix this error."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"Helmet no longer supports `true` as a middleware option, exception is Origin-Agent-Cluster. Remove the property from your options to fix this error."
"Helmet no longer supports `true` as a middleware option, except for Origin-Agent-Cluster. Remove the property from your options to fix this error."

@salesh
Copy link
Author

salesh commented Jan 17, 2021

@EvanHahn
No Evan, thank you very much for giving me the opportunity to learn and contribute - you had written really full and descriptive guide, my part is really small. I had learned a lot about how a helmet works because of this. Thank you

@salesh salesh requested a review from EvanHahn January 17, 2021 16:09
@EvanHahn EvanHahn changed the base branch from main to origin-agent-cluster January 17, 2021 16:50
@EvanHahn EvanHahn merged commit 6a6e109 into helmetjs:origin-agent-cluster Jan 17, 2021
EvanHahn added a commit that referenced this pull request Jan 17, 2021
Co-authored-by: salesh <sasa.cvetkovic@yahoo.com>

See [#286][286] and [#287][287].

[286]: #286
[287]: #287
@EvanHahn
Copy link
Member

Thank you so much! I've made some small cleanups in #287 and will deploy soon.

@EvanHahn
Copy link
Member

This has been published in helmet@4.4.0. Thanks again!

Feel free to add yourself to the contributors list, or let me know your name and URL and I can do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants