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

Jump to bottom

Add Feature-Policy header #16613

Merged
merged 3 commits into from
Aug 11, 2019
Merged

Add Feature-Policy header #16613

merged 3 commits into from
Aug 11, 2019

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented Jul 30, 2019

No description provided.

@rullzer rullzer added this to the Nextcloud 17 milestone Jul 30, 2019
@rullzer rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 1, 2019
@rullzer
Copy link
Member Author

rullzer commented Aug 1, 2019

Review time :)

This is not all the features. But other things will be added later in steps.

georgehrke
georgehrke previously requested changes Aug 1, 2019
View reviewed changes
Copy link
Member

@georgehrke georgehrke left a comment

Choose a reason for hiding this comment

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

Code looks good, just some minor changes.

To test just make sure that the header is there?

lib/public/AppFramework/Http/EmptyFeaturePolicy.php Outdated Show resolved Hide resolved
lib/public/AppFramework/Http/EmptyFeaturePolicy.php Outdated Show resolved Hide resolved
lib/public/AppFramework/Http/EmptyFeaturePolicy.php Outdated Show resolved Hide resolved
lib/public/AppFramework/Http/EmptyFeaturePolicy.php Outdated Show resolved Hide resolved
lib/public/AppFramework/Http/EmptyFeaturePolicy.php Outdated Show resolved Hide resolved
lib/public/AppFramework/Http/EmptyFeaturePolicy.php Outdated Show resolved Hide resolved
@rullzer
Copy link
Member Author

rullzer commented Aug 1, 2019

Yes check the header is there.
You could check with talk. That should fail hard now until I provide a patch there ;)

ChristophWurst
ChristophWurst reviewed Aug 5, 2019
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

looks good otherwise

lib/private/AppFramework/DependencyInjection/DIContainer.php Outdated Show resolved Hide resolved
lib/private/Security/FeaturePolicy/FeaturePolicyManager.php Outdated Show resolved Hide resolved
lib/public/AppFramework/Http/EmptyFeaturePolicy.php Outdated Show resolved Hide resolved
lib/public/AppFramework/Http/EmptyFeaturePolicy.php Outdated Show resolved Hide resolved
@rullzer rullzer requested a review from georgehrke August 5, 2019 19:34
@rullzer
Copy link
Member Author

rullzer commented Aug 5, 2019

All done @ChristophWurst 😉

ChristophWurst
ChristophWurst approved these changes Aug 6, 2019
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

looks good!

kesselb
kesselb reviewed Aug 6, 2019
View reviewed changes
lib/private/AppFramework/Middleware/Security/FeaturePolicyMiddleware.php
* @return Response
*/
public function afterController($controller, $methodName, Response $response): Response {
$policy = !is_null($response->getFeaturePolicy()) ? $response->getFeaturePolicy() : new FeaturePolicy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$policy = !is_null($response->getFeaturePolicy()) ? $response->getFeaturePolicy() : new FeaturePolicy();
$policy = $response->getFeaturePolicy() ?? new FeaturePolicy();

Could that work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably. But lets leave it for now.

@MorrisJobke MorrisJobke mentioned this pull request Aug 8, 2019
Merged
28 tasks
@rullzer rullzer requested a review from kesselb August 8, 2019 06:35
@rullzer rullzer mentioned this pull request Aug 8, 2019
Merged
georgehrke
georgehrke approved these changes Aug 8, 2019
View reviewed changes
Copy link
Member

@georgehrke georgehrke left a comment

Choose a reason for hiding this comment

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

👍 Tested and works

danxuliu
danxuliu approved these changes Aug 8, 2019
View reviewed changes
Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

Tested (with Talk) and works 👍

@rullzer
Have the OCSBaseResponse call the parent constructor
5d94590 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Add feature policy header
b8c5008 
This adds the events and the classes to modify the feature policy.
It also adds a default restricted feature policy.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Copy link
Member Author

rullzer commented Aug 10, 2019

Pushed tests. Will merge once CI is happy

@rullzer
Add tests
f81817b 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer merged commit 773ce9e into master Aug 11, 2019
@rullzer rullzer deleted the enh/featurepolicy branch August 11, 2019 08:17
@Rello Rello mentioned this pull request Aug 16, 2019
Closed
MorrisJobke added a commit to nextcloud/documentation that referenced this pull request Aug 10, 2020
@MorrisJobke
Document existing events
da43209 
* LoadAdditionalScripts (@rullzer) - nextcloud/server#16641
* LoadViewerEvent (@skjnldsv) - nextcloud/viewer#271
* RegisterDirectEditorEvent (@juliushaertl) - nextcloud/server#17625
* typed events for files scanner (@ChristophWurst) - nextcloud/server#18351
* typed events for group mangement (@ChristophWurst) - nextcloud/server#18350
* AddContentSecurityPolicyEvent (@rullzer) - nextcloud/server#15730
* UserLiveStatusEvent (@georgehrke) - nextcloud/server#21186
* password_policy events (@ChristophWurst) - nextcloud/server#18019
* AddFeaturePolicyEvent (@rullzer) - nextcloud/server#16613
* ShareCreatedEvent (@rullzer) - nextcloud/server#18384
* LoadSettingsScriptsEvent (@blizzz) - nextcloud/server#21475
* flow events (@rullzer) - nextcloud/server#18535

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke MorrisJobke mentioned this pull request Aug 10, 2020
Merged
backportbot-nextcloud bot pushed a commit to nextcloud/documentation that referenced this pull request Aug 10, 2020
@MorrisJobke @backportbot
Document existing events
6ca7040 
* LoadAdditionalScripts (@rullzer) - nextcloud/server#16641
* LoadViewerEvent (@skjnldsv) - nextcloud/viewer#271
* RegisterDirectEditorEvent (@juliushaertl) - nextcloud/server#17625
* typed events for files scanner (@ChristophWurst) - nextcloud/server#18351
* typed events for group mangement (@ChristophWurst) - nextcloud/server#18350
* AddContentSecurityPolicyEvent (@rullzer) - nextcloud/server#15730
* UserLiveStatusEvent (@georgehrke) - nextcloud/server#21186
* password_policy events (@ChristophWurst) - nextcloud/server#18019
* AddFeaturePolicyEvent (@rullzer) - nextcloud/server#16613
* ShareCreatedEvent (@rullzer) - nextcloud/server#18384
* LoadSettingsScriptsEvent (@blizzz) - nextcloud/server#21475
* flow events (@rullzer) - nextcloud/server#18535

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
MorrisJobke added a commit to nextcloud/documentation that referenced this pull request Aug 10, 2020
@MorrisJobke
Document existing events
7e78e7e 
* LoadAdditionalScripts (@rullzer) - nextcloud/server#16641
* LoadViewerEvent (@skjnldsv) - nextcloud/viewer#271
* RegisterDirectEditorEvent (@juliushaertl) - nextcloud/server#17625
* typed events for files scanner (@ChristophWurst) - nextcloud/server#18351
* typed events for group mangement (@ChristophWurst) - nextcloud/server#18350
* AddContentSecurityPolicyEvent (@rullzer) - nextcloud/server#15730
* UserLiveStatusEvent (@georgehrke) - nextcloud/server#21186
* password_policy events (@ChristophWurst) - nextcloud/server#18019
* AddFeaturePolicyEvent (@rullzer) - nextcloud/server#16613
* ShareCreatedEvent (@rullzer) - nextcloud/server#18384
* LoadSettingsScriptsEvent (@blizzz) - nextcloud/server#21475
* flow events (@rullzer) - nextcloud/server#18535

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
MorrisJobke added a commit to nextcloud/documentation that referenced this pull request Aug 10, 2020
@MorrisJobke
Document existing events
6da5114 
* LoadAdditionalScripts (@rullzer) - nextcloud/server#16641
* LoadViewerEvent (@skjnldsv) - nextcloud/viewer#271
* RegisterDirectEditorEvent (@juliushaertl) - nextcloud/server#17625
* typed events for files scanner (@ChristophWurst) - nextcloud/server#18351
* typed events for group mangement (@ChristophWurst) - nextcloud/server#18350
* AddContentSecurityPolicyEvent (@rullzer) - nextcloud/server#15730
* UserLiveStatusEvent (@georgehrke) - nextcloud/server#21186
* password_policy events (@ChristophWurst) - nextcloud/server#18019
* AddFeaturePolicyEvent (@rullzer) - nextcloud/server#16613
* ShareCreatedEvent (@rullzer) - nextcloud/server#18384
* LoadSettingsScriptsEvent (@blizzz) - nextcloud/server#21475
* flow events (@rullzer) - nextcloud/server#18535

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@snyk-bot snyk-bot mentioned this pull request Mar 14, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@georgehrke georgehrke georgehrke approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@danxuliu danxuliu danxuliu approved these changes

@MorrisJobke MorrisJobke Awaiting requested review from MorrisJobke

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@juliushaertl juliushaertl Awaiting requested review from juliushaertl

@kesselb kesselb Awaiting requested review from kesselb

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement security
Projects
None yet
Milestone
Nextcloud 17
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@rullzer @georgehrke @ChristophWurst @kesselb @danxuliu