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 secure view using using IShare attributes #245

Merged
merged 13 commits into from
May 15, 2019
Merged

Add secure view using using IShare attributes #245

merged 13 commits into from
May 15, 2019

Conversation

mrow4a
Copy link
Contributor

@mrow4a mrow4a commented Jan 13, 2019 •
edited
Loading

  • Requires View-only dav plugin using IShare attributes core#33994 to be finished in order to use extra permissions and "dav can-download" permission.
  • Add admin setting to enable secure view in collabora
  • Make sure migration and compatibility with older versions is solved (old temporary token table is dropped)

secure-view-feature

@codecov
Copy link

codecov bot commented Jan 13, 2019

Codecov Report

Merging #245 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #245      +/-   ##
===========================================
- Coverage      5.39%   5.38%   -0.01%     
  Complexity      284     284              
===========================================
  Files            14      14              
  Lines          1113    1115       +2     
===========================================
  Hits             60      60              
- Misses         1053    1055       +2
Impacted Files Coverage Δ Complexity Δ
lib/Db/Wopi.php 0% <0%> (ø) 9 <3> (ø) ⬇️
lib/AppInfo/Application.php 58.62% <0%> (-0.69%) 25 <0> (ø)
lib/Controller/DocumentController.php 1.83% <0%> (-0.01%) 116 <0> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e8652c...0c8c905. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 13, 2019 •
edited
Loading

Codecov Report

Merging #245 into master will decrease coverage by 0.09%.
The diff coverage is 1.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             master    #245     +/-   ##
==========================================
- Coverage      5.39%   5.29%   -0.1%     
- Complexity      284     308     +24     
==========================================
  Files            14      14             
  Lines          1113    1190     +77     
==========================================
+ Hits             60      63      +3     
- Misses         1053    1127     +74
Impacted Files Coverage Δ Complexity Δ
lib/HookHandler.php 0% <0%> (ø) 3 <2> (+2) ⬆️
lib/Controller/DocumentController.php 1.63% <0%> (-0.21%) 131 <51> (+15)
lib/Helper.php 0% <0%> (ø) 34 <5> (+1) ⬆️
lib/Storage.php 0% <0%> (ø) 25 <0> (ø) ⬇️
lib/AppConfig.php 0% <0%> (ø) 10 <4> (+3) ⬆️
lib/Controller/SettingsController.php 0% <0%> (ø) 19 <14> (+5) ⬆️
lib/Db/Wopi.php 0% <0%> (ø) 5 <5> (-4) ⬇️
lib/AppInfo/Application.php 56.84% <30%> (-2.47%) 27 <1> (+2)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 556ba67...adab4dc. Read the comment docs.

DeepDiver1975
DeepDiver1975 requested changes Jan 14, 2019
View reviewed changes
js/viewer/extrapermissions.js Outdated Show resolved Hide resolved
js/viewer/extrapermissions.js Outdated Show resolved Hide resolved
lib/AppInfo/Application.php Outdated Show resolved Hide resolved
@mrow4a mrow4a force-pushed the secure-view branch 4 times, most recently from d001503 to b6eac81 Compare January 20, 2019 17:56
@mrow4a mrow4a changed the title [WIP] Add secure view using extra share permission Add secure view using extra share permission Jan 20, 2019
@PVince81 PVince81 requested a review from VicDeo January 25, 2019 16:15
@PVince81 PVince81 requested a review from timar February 1, 2019 09:14
@mrow4a mrow4a force-pushed the secure-view branch 2 times, most recently from 2069b91 to 3447b9e Compare February 3, 2019 23:21
@mrow4a mrow4a force-pushed the secure-view branch 2 times, most recently from d4f50d4 to 05f07ae Compare March 5, 2019 21:18
DeepDiver1975
DeepDiver1975 requested changes Mar 6, 2019
View reviewed changes
js/viewer/attributes.js Outdated Show resolved Hide resolved
DeepDiver1975
DeepDiver1975 requested changes Mar 11, 2019
View reviewed changes
js/admin.js Show resolved Hide resolved
PVince81
PVince81 reviewed Apr 1, 2019
View reviewed changes
lib/AppConfig.php Outdated Show resolved Hide resolved
PVince81
PVince81 suggested changes Apr 2, 2019
View reviewed changes
Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

see comments

js/viewer/attributes.js Outdated Show resolved Hide resolved
js/viewer/attributes.js Outdated Show resolved Hide resolved
js/viewer/attributes.js Outdated Show resolved Hide resolved
js/documents.js Show resolved Hide resolved
js/documents.js Show resolved Hide resolved
lib/Controller/DocumentController.php Outdated Show resolved Hide resolved
templates/admin.php Outdated Show resolved Hide resolved
appinfo/Migrations/Version20190310162809.php Show resolved Hide resolved
@mrow4a mrow4a force-pushed the secure-view branch 4 times, most recently from 1dc56ac to 589dbd2 Compare April 17, 2019 20:00
@mrow4a
Copy link
Contributor Author

mrow4a commented Apr 17, 2019

@PVince81 can you recheck? shall we have some QA by product team?

PVince81
PVince81 reviewed Apr 18, 2019
View reviewed changes
js/admin.js Outdated Show resolved Hide resolved
@mrow4a mrow4a force-pushed the secure-view branch 2 times, most recently from a82e48a to c1f96ee Compare April 29, 2019 17:28
PVince81
PVince81 approved these changes Apr 30, 2019
View reviewed changes
Copy link
Contributor

@PVince81 PVince81 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 fine to me.

@mrow4a can you confirm that this works with your latest core changes ?

@micbar micbar assigned timar and unassigned DeepDiver1975 and mrow4a May 3, 2019
@mrow4a
Copy link
Contributor Author

mrow4a commented May 5, 2019

@PVince81 @micbar Tested on oc10.2.0RC2:

  • upgrade from Colabora 2.1.2 to SecView (2.2.0) and all works smooth and temp table gets recreated with new schema
  • All required featureset is working

@mrow4a
Copy link
Contributor Author

mrow4a commented May 7, 2019 •
edited
Loading

@PVince81 @pmaier1 can we consider this PR ready to merge? @DeepDiver1975 your approval blocks the PR

@PVince81
Copy link
Contributor

PVince81 commented May 9, 2019

I consider this ready to merge.

Not sure if we should wait for approval from @timar

@pmaier1
Copy link

pmaier1 commented May 10, 2019

Please merge this. Then we can do translations and release officially. THX!

@mrow4a
Copy link
Contributor Author

mrow4a commented May 13, 2019

@pmaier1 please consult @timar and @PVince81 for merging

@PVince81
Copy link
Contributor

  • BUG: clicking on "Enable secure view" text in the settings doesn't tick checkbox. need to use the "label" tag properly. it works for other options. (can be fixed in a separate PR)

@PVince81 PVince81 merged commit 8fe5a5d into master May 15, 2019
@delete-merged-branch delete-merged-branch bot deleted the secure-view branch May 15, 2019 11:08
@phil-davis phil-davis mentioned this pull request Apr 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmaier1 pmaier1 pmaier1 left review comments

@PVince81 PVince81 PVince81 approved these changes

@VicDeo VicDeo Awaiting requested review from VicDeo

@timar timar Awaiting requested review from timar

@DeepDiver1975 DeepDiver1975 Awaiting requested review from DeepDiver1975

Assignees

@timar timar

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mrow4a @PVince81 @pmaier1 @DeepDiver1975 @timar