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

ARTCipherParams and ARTCrypto doc comments #1439

Closed
wants to merge 8 commits into from
Closed

ARTCipherParams and ARTCrypto doc comments #1439

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
385ff3d
Added docstrings for `ARTCipherParams`
maratal Jun 30, 2022
d693aa1
Added docstrings for `ARTCrypto`
maratal Jun 30, 2022
fd86ccd
Params name fixed
maratal Jun 30, 2022
b87ee16
Removed "" for CBC as per the canonical table.
maratal Jul 7, 2022
3e0875b
Return statement for `ARTCrypto` updated
maratal Jul 7, 2022
3cce613
Updated descriptions for both `generateRandomKey...` methods
maratal Jul 7, 2022
4511851
Removed unnecessary return type placement.
maratal Jul 7, 2022
0deb0a0
Added backticks for `AES` and `CBC`
maratal Jul 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions Source/ARTCrypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,20 @@ NS_ASSUME_NONNULL_BEGIN
@end

/**
ARTCipherParams contains configuration options for a channel cipher, including algorithm, mode, key length and key.
Ably client libraries currently support AES with CBC, PKCS#7 with a default key length of 256 bits. All implementations also support AES128.
The ARTCipherParams object contains properties to configure encryption for a channel.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The ARTCipherParams object contains properties to configure encryption for a channel.
The `ARTCipherParams` object contains properties to configure encryption for a channel.

@tbedford @m-hulbert I’d suggest that this sort of formatting should be in the canonical documentation, what do you think?

Copy link

Choose a reason for hiding this comment

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

Agree with the formatting. It is already styled as code in the canonical docs though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will link to the page that user is already on. Is this what was expected? @lawrence-forooghian

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The ARTCipherParams object contains properties to configure encryption for a channel.
An ARTCipherParams instance contains properties to configure encryption for a channel.

Copy link
Collaborator Author

@maratal maratal Jul 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe

The ARTCipherParams properties allows you to configure encryption for a channel.

@lawrence-forooghian @m-hulbert

*/
@interface ARTCipherParams : NSObject <ARTCipherParamsCompatible>

/// The algorithm to use for encryption. Only `AES` is supported.
@property (readonly, strong, nonatomic) NSString *algorithm;

/// The private key used to encrypt and decrypt payloads.
@property (readonly, strong, nonatomic) NSData *key;

/// The length of the key in bits; for example 128 or 256.
@property (readonly, nonatomic) NSUInteger keyLength;

/// The cipher mode. Only `CBC` is supported.
@property (readonly, getter=getMode) NSString *mode;

- (instancetype)init UNAVAILABLE_ATTRIBUTE;
Expand All @@ -44,11 +51,30 @@ NS_ASSUME_NONNULL_BEGIN

@end

/**
The ARTCrypto object ensures that message payloads are encrypted, can never be decrypted by Ably, and can only be decrypted by other clients that share the same secret symmetric key.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The ARTCrypto object ensures that message payloads are encrypted, can never be decrypted by Ably, and can only be decrypted by other clients that share the same secret symmetric key.
The `ARTCrypto` object ensures that message payloads are encrypted, can never be decrypted by Ably, and can only be decrypted by other clients that share the same secret symmetric key.

@tbedford @m-hulbert as I mentioned elsewhere in this PR, I think this sort of formatting should be part of the canonical docs?

Copy link

Choose a reason for hiding this comment

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

Agree with the change and this is also part of the canonical table already.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The ARTCrypto object ensures that message payloads are encrypted, can never be decrypted by Ably, and can only be decrypted by other clients that share the same secret symmetric key.
The ARTCrypto class ensures that message payloads are encrypted, can never be decrypted by Ably, and can only be decrypted by other clients that share the same secret symmetric key.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure whether I agree with this description entirely. I think that the description of how encryption works in Ably is a good one, but it's not the ARTCrypto class itself that's responsible for performing the encryption. It’s more of a utility class, which helps you to create the parameters used to configure encryption elsewhere in the SDK.

Copy link

Choose a reason for hiding this comment

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

Is something like the following more accurate?

The `ARTCrypto` class enables you to set the parameters required for configuring the encryption of message payloads.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about

The ARTCrypto class enables you to generate the parameters required for configuring the encryption of message payloads.

@lawrence-forooghian @m-hulbert

*/
@interface ARTCrypto : NSObject

+ (ARTCipherParams *)getDefaultParams:(NSDictionary *)values;
/**
Retrieves, or optionally sets, the `ARTCipherParams` for the channel.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think I agree with this description. My understanding is that getDefaultParams is a factory method for creating an ARTCipherParams instance. I don't think it queries or manipulates a channel in any way (bear in mind that this is a class method, so there isn't even a "the channel" to speak of here).

Copy link

Choose a reason for hiding this comment

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

Is this more accurate?

Creates an `ARTCipherParams` object to use for encryption.

@param params Overrides the default parameters. A suitable key must be provided as a minimum.
@return An `ARTCipherParams` object, using the default values for any field not supplied.
*/
+ (ARTCipherParams *)getDefaultParams:(NSDictionary *)params;

/**
Generates a random key to be used in the encryption of the channel.
@param keyLength The length of the key, in bits, to be generated.
@return The key as a binary data.
*/
+ (NSData *)generateRandomKey:(NSUInteger)keyLength;

/**
Same as `+generateRandomKey:`, but with the default key length of the default algorithm: for AES this is 256 bits.
@return The key as a binary data.
*/
+ (NSData *)generateRandomKey;
+ (NSData *)generateRandomKey:(NSUInteger)length;

@end

Expand Down