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

Feature/701: Finish recrypt #728

Merged
merged 45 commits into from
Jul 18, 2023
Merged

Feature/701: Finish recrypt #728

merged 45 commits into from
Jul 18, 2023

Conversation

lkleisa
Copy link
Collaborator

@lkleisa lkleisa commented Jun 26, 2023

No description provided.

@lkleisa lkleisa linked an issue Jun 26, 2023 that may be closed by this pull request
ENCRYPTABLES: Finish recrypt #701
Closed
18 tasks
@lkleisa lkleisa mentioned this pull request Jun 26, 2023
Closed
18 tasks
@lkleisa lkleisa force-pushed the feature/701/recrypt-finish branch from a16b3b4 to 6cdd706 Compare June 28, 2023 11:16
@mtnstar mtnstar force-pushed the master branch 2 times, most recently from 520ebe8 to 151e944 Compare June 28, 2023 14:05
@lkleisa lkleisa self-assigned this Jul 3, 2023
@lkleisa lkleisa force-pushed the feature/701/recrypt-finish branch 3 times, most recently from 8bdf3d5 to 1d1b052 Compare July 5, 2023 07:14
@lkleisa lkleisa marked this pull request as ready for review July 5, 2023 09:27
@lkleisa lkleisa requested a review from mtnstar July 5, 2023 09:37
@mtnstar
Copy link
Contributor

mtnstar commented Jul 5, 2023 •
edited
Loading

image

maybe place lock in front? make lock icon green?

reduce text?: Encrypted with AES256IV/256bit

same for the edit form ... make this more consistent and pretty 🥳

mtnstar
mtnstar suggested changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@mtnstar mtnstar left a comment

Choose a reason for hiding this comment

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

another huge PR we're going to merge hopefully soon!

some comments to check.

especially the test cases for the transferred enryptables vs. recryption need some improvement.

will have another look on the PR for your next review request

@lkleisa lkleisa force-pushed the feature/701/recrypt-finish branch 2 times, most recently from 38717a0 to bbacb09 Compare July 7, 2023 14:02
@lkleisa
Copy link
Collaborator Author

lkleisa commented Jul 7, 2023

The icon in encryptable/show with the lock needs a new design, otherwise feedback is implemented

@lkleisa lkleisa requested a review from mtnstar July 7, 2023 14:11
@lkleisa lkleisa force-pushed the feature/701/recrypt-finish branch from d3c6cfd to 24c3b14 Compare July 10, 2023 06:41
@mtnstar
Copy link
Contributor

mtnstar commented Jul 10, 2023 •
edited
Loading

nice ... looks way better now !

image

image

could we make this look a bit similar, the edit and read element? at least the same icon?
I like the lock with the tick a lot !

let's have a look together tomorrow !

the text for the used encryption algorithm could be even shorter ... just leave the 256 bits ....
Encrypted with AES256IV is fine ... 256 defines the key length already !

mtnstar
mtnstar suggested changes Jul 10, 2023
View reviewed changes
Copy link
Contributor

@mtnstar mtnstar left a comment

Choose a reason for hiding this comment

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

almost there !
check the feedback ... and let us discuss the changes first ... will be ready for merging soon, I'm sure !

@lkleisa lkleisa force-pushed the feature/701/recrypt-finish branch 2 times, most recently from 6527c02 to 4a89e14 Compare July 12, 2023 07:32
@lkleisa lkleisa requested a review from mtnstar July 12, 2023 14:39
@mtnstar mtnstar mentioned this pull request Jul 17, 2023
Closed
lkleisa added 22 commits July 17, 2023 16:13
@lkleisa @mtnstar
Remove unused fixture transferred_file
984e980
@lkleisa @mtnstar
Add new encryption algorithm to fabricator
6b07c4f
@lkleisa @mtnstar
Use other translation in encryptable form
1b27a1c
@lkleisa @mtnstar
Add route to team index
6eeeac9
@lkleisa @mtnstar
Remove unused code
a561291
@lkleisa @mtnstar
Receive transferred encryptable before recrypt
b0f652b
@lkleisa @mtnstar
Write tests for receive transferred encryptables before recrypt
50b0de5
@lkleisa @mtnstar
Adjust tests for both algorithm types
a05d5c6
@lkleisa @mtnstar
Implement feedback
f25e3f0
@lkleisa @mtnstar
Use bootstrap spinner
d4a0920
@lkleisa @mtnstar
Split up tests in encryptables_controller_spec
1b97f72
@lkleisa @mtnstar
Fix backend spec
5801658
@lkleisa @mtnstar
Beautify encrypt_with_encryption_class method
027a703
@lkleisa @mtnstar
Beautify lock icon on encryptable show
c2530c0
@lkleisa @mtnstar
Implement feedback
769b7af
@lkleisa @mtnstar
Adjust lock in encryptable show
6783d7c
@lkleisa @mtnstar
Use fabricator for transferred files
bcfc720
@lkleisa @mtnstar
Change css class of attribute field
4ae736d
@lkleisa @mtnstar
Make rubocop happy
80fb9ef
@lkleisa @mtnstar
Fix rebase problems with credential sharing
30d9aa2
@lkleisa @mtnstar
Remove rubocop disable
bb06de4
@lkleisa @mtnstar
Remove encryption algorithm and bitsize from serializer
f8ee95d
@mtnstar mtnstar force-pushed the feature/701/recrypt-finish branch from 87ef27b to f8ee95d Compare July 17, 2023 14:14
mtnstar
mtnstar suggested changes Jul 18, 2023
View reviewed changes
spec/controllers/api/encryptables_controller_spec.rb
@@ -805,12 +830,13 @@ def create_file
file
end

def prepare_transferred_encryptable
Copy link
Contributor

Choose a reason for hiding this comment

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

use new Fabricator for this ... check spec/fabricators/...

spec/controllers/api/encryptables_controller_spec.rb

encryptable_file = prepare_transferred_encryptable
encryptable_file = prepare_transferred_encryptable(Crypto::Symmetric::Aes256)
Copy link
Contributor

Choose a reason for hiding this comment

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

use fabricator !

spec/fabricators/encryptables/credentials_fabricator.rb
@@ -5,12 +5,66 @@
# See the COPYING file at the top-level directory or at
# https://github.com/puzzle/cryptopus.

Copy link
Contributor

Choose a reason for hiding this comment

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

move this fabricator to spec/fabricators/encrytable_fabricator.rb

@mtnstar mtnstar merged commit 4f4e59d into master Jul 18, 2023
@mtnstar mtnstar deleted the feature/701/recrypt-finish branch July 18, 2023 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtnstar mtnstar mtnstar requested changes

Assignees

@lkleisa lkleisa

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ENCRYPTABLES: Finish recrypt
2 participants
@lkleisa @mtnstar