-
Notifications
You must be signed in to change notification settings - Fork 107
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
Encrypted payload support #263
Conversation
cipher.encrypt | ||
cipher.key = key | ||
cipher.iv = initialization_vector | ||
json = ActiveSupport::JSON.encode(payload).gsub('<', '\u003C') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure that this gsub is not necessary as the results are encrypted and Base64'd anyway. However retaining it for now.
cipher.iv = initialization_vector | ||
json = ActiveSupport::JSON.encode(payload).gsub('<', '\u003C') | ||
encrypted = initialization_vector + cipher.update(json) + cipher.final + cipher.auth_tag | ||
Base64.encode64(encrypted).gsub("\n", "\\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just so that the result can be interpolated as JavaScript without a syntax error caused by line breaks.
|
||
ENCRYPTED_MODE_SETTINGS_WHITELIST = [:app_id, :session_duration, :widget, :custom_launcher_selector, :hide_default_launcher, :alignment, :horizontal_padding, :vertical_padding] | ||
|
||
def initialize(secret, initialization_vector, options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just encapsulate the IV generation (through securerandom) and drop it from the list of the function args?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It helps to able to Dependency Inject it so that we can specify fixed IVs for things like testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that it is optional, we fall back to SecureRandom https://github.com/intercom/intercom-rails/pull/263/files#diff-a86f56ee7fcbd0a1aac0f6800d8c0435R9
looks good! Failing specs because of ruby 1.9.3. we will need to upgrade intercom-ruby to a new major version to fix this before we ship this PR. |
FYI: Circle is not failing because of ruby version |
@Skaelv I'm pretty sure that the Circle failure is the same thing. This repo doesn't have a |
Just merged my PR if you want to rebase |
No description provided.