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(api): Update documentation to reflect security header support #8167

Merged
merged 1 commit into from
Apr 25, 2018

Conversation

dcramer
Copy link
Member

@dcramer dcramer commented Apr 24, 2018

Switch to the new security header reporting endpoint for CSP and expose Expect-CT as a first-class citizen.

Fixes GH-7005

@dcramer
Copy link
Member Author

dcramer commented Apr 24, 2018

Index

screen shot 2018-04-24 at 9 27 40 am

Expect-CT Details

screen shot 2018-04-24 at 9 27 51 am

Security Header Exposure

screen shot 2018-04-24 at 9 21 53 am

@dcramer
Copy link
Member Author

dcramer commented Apr 24, 2018

Going to make a few small tweaks to Report URI box (and fix getDynamicText as its wrong)

@dcramer
Copy link
Member Author

dcramer commented Apr 24, 2018

screen shot 2018-04-24 at 9 52 41 am

@dcramer dcramer force-pushed the feat/security-headers-settings branch from 9706b25 to ed39418 Compare April 24, 2018 16:53

componentWillMount() {
super.componentWillMount();
this.props.setProjectNavSection('settings');
Copy link
Member

Choose a reason for hiding this comment

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

This might be needed for old project settings, not sure

Copy link
Member Author

Choose a reason for hiding this comment

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

ya I might as well keep it since I kept it elsewhere


<CodeBlock>{this.getInstructions()}</CodeBlock>

<TextBlockNoMargin css={{marginTop: 30}}>
Copy link
Member

Choose a reason for hiding this comment

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

you can have noMargin as a prop

return [
{
name: 'Content Security Policy',
description: 'foo bar',
Copy link
Member

Choose a reason for hiding this comment

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

what's this for?

<HeaderName>{name}</HeaderName>
</Box>
<Button to={url} priority="primary">
Instructions
Copy link
Member

Choose a reason for hiding this comment

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

t()

</Panel>

<Panel>
<PanelHeader>{'Supported Formats'}</PanelHeader>
Copy link
Member

Choose a reason for hiding this comment

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

t()

import ExternalLink from '../../../components/externalLink';
import {Panel, PanelBody, PanelHeader} from '../../../components/panels';
import PreviewFeature from '../../../components/previewFeature';
import ReportURI, {getSecurityDsn} from './reportUri';
Copy link
Member

Choose a reason for hiding this comment

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

casing is inconsistent w/ acronyms

{
name: 'Content Security Policy',
description: 'foo bar',
url: `/settings/${orgId}/${projectId}/security-headers/csp/`,
Copy link
Member

Choose a reason for hiding this comment

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

if this is accessed in old settings, they'll be forwarded to new settings

Copy link
Member Author

Choose a reason for hiding this comment

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

how much do we care? what is the other solution?

Copy link
Member

Choose a reason for hiding this comment

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

You could try recreateRoute('csp/', this.props)

@dcramer dcramer force-pushed the feat/security-headers-settings branch 4 times, most recently from 62d51a7 to f4e2bb6 Compare April 24, 2018 19:12
@dcramer
Copy link
Member Author

dcramer commented Apr 24, 2018

Cleaned up a lot of this based on feedback and expanded test coverage

@benvinegar
Copy link
Contributor

Could we consider adding short form/acronyms to these links:

  • Content Security Policy (CSP)
  • Certificate Transparency (Expect-CT)

I ask because I think it's more common to talk about these in terms of their abbreviations instead of the full sentence, e.g. no one's asked us to add "Certificate Transparency" support. I think it would be more clear to someone quickly scanning this page.

Switch to the new security header reporting endpoint for CSP and expose Expect-CT as a first-class citizen.
@dcramer dcramer force-pushed the feat/security-headers-settings branch from f4e2bb6 to 0ee6c66 Compare April 25, 2018 16:21
@dcramer
Copy link
Member Author

dcramer commented Apr 25, 2018

As @mattrobenolt mentioned the other day, we're sitll not fully rendering interfaces here. We'll likely just expose a generic JSON interface for this and simply render the raw key/value bits.

Same thing for HPKP support (#8185)

@dcramer dcramer merged commit be2c488 into master Apr 25, 2018
@dcramer dcramer deleted the feat/security-headers-settings branch April 25, 2018 17:10
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI support for security reports.
3 participants