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

[#21] Implement Document Encryption that returns EDEKs #27

Merged
merged 32 commits into from
Aug 9, 2019

Conversation

clintfred
Copy link
Contributor

@clintfred clintfred commented Aug 1, 2019

see #21

Changelog

  • DocumentAdvancedOps::document_encrypt_unmanaged function added for advanced use cases where the calling application wants to manage both the encrypted data and the associated edeks instead of using the IronCore service for EDEK management.

TODO

  • - sync up with latest edek proto definition
  • - fix android-64 build (disabled for now)
  • - convert remaining TODOs to other issues

@clintfred
Copy link
Contributor Author

Android 64 build is failing as it is cross compiled and doesn't allow writing to the src/ tree. This is problematic as our build.rs generates edeks.rs at build time.

cross-rs/cross#145 (comment) may contain some helpful information if we decide to continue generating proto -> rs at ironoxide build time. Deferring this until we decide on a general way to handle .proto files.

@clintfred clintfred changed the title [WIP] [#21] Implement Document Encryption that returns EDEKs. [#21] Implement Document Encryption that returns EDEKs. Aug 1, 2019
@clintfred clintfred changed the title [#21] Implement Document Encryption that returns EDEKs. [#21] Implement Document Encryption that returns EDEKs Aug 1, 2019
build.rs Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/requests.rs Outdated Show resolved Hide resolved
src/proto/mod.rs Outdated Show resolved Hide resolved
src/document.rs Outdated Show resolved Hide resolved
src/document.rs Outdated Show resolved Hide resolved
src/document.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@coltfred coltfred left a comment

Choose a reason for hiding this comment

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

Responding to some responses.

src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/document.rs Outdated Show resolved Hide resolved
proto/transform.proto Outdated Show resolved Hide resolved
src/internal/mod.rs Show resolved Hide resolved
src/internal/mod.rs Outdated Show resolved Hide resolved
src/internal/rest.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Show resolved Hide resolved
Copy link
Member

@giarc3 giarc3 left a comment

Choose a reason for hiding this comment

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

Just comments on comments

src/document.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
@clintfred clintfred requested a review from skeet70 August 6, 2019 22:27
Copy link
Member

@skeet70 skeet70 left a comment

Choose a reason for hiding this comment

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

Pending a bit more cleanup of TODO style comments into issues or actual changes, I think it looks good.

src/internal/mod.rs Show resolved Hide resolved
Copy link
Member

@coltfred coltfred left a comment

Choose a reason for hiding this comment

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

Besides the structure and creation of DocumentEncryptUnmanagedResult, I think we're close.

src/document/advanced.rs Show resolved Hide resolved
proto/transform.proto Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Show resolved Hide resolved
src/internal/document_api/mod.rs Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
@@ -211,32 +212,209 @@ fn doc_create_with_policy_grants() -> Result<(), IronOxideErr> {
}

#[test]
fn doc_create_with_explicit_grants() {
let sdk = init_sdk();
fn doc_edek_encrypt_with_policy_grants() -> Result<(), IronOxideErr> {
Copy link
Member

Choose a reason for hiding this comment

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

This test name should reflect the name changes.

tests/document_ops.rs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@clintfred clintfred left a comment

Choose a reason for hiding this comment

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

responses

src/document/advanced.rs Show resolved Hide resolved
proto/transform.proto Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Outdated Show resolved Hide resolved
src/internal/document_api/mod.rs Show resolved Hide resolved
src/internal/document_api/mod.rs Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
Copy link
Member

@coltfred coltfred left a comment

Choose a reason for hiding this comment

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

I'd still like to see that one test case with the policy simplified, but thanks for fixing up the creation of the Unmanaged result to make the java and scala bindings better.

@clintfred clintfred merged commit 448920f into master Aug 9, 2019
@clintfred clintfred deleted the 21-edek-encrypt branch August 9, 2019 14:51
clintfred added a commit that referenced this pull request Aug 9, 2019
  - DocumentAdvancedOps::document_encrypt_unmanaged function added for advanced use cases where the calling application wants to manage both the encrypted data and the associated edeks instead of using the IronCore service for EDEK management.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants