-
Notifications
You must be signed in to change notification settings - Fork 70
208 Integrity-check for the correlation ID #218
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
208 Integrity-check for the correlation ID #218
Conversation
There is already a secret key shared among nodes:
What about re-using it here? |
apps/rig/lib/rig/connection/codec.ex
Outdated
@doc "Turn a pid into an url-encoded string." | ||
@spec serialize(pid) :: binary | ||
def serialize(pid) do | ||
key = Confex.fetch_env!(:rig, Rig.Connection.Codec)[:secret_key] |
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.
I think I'd prefer use Rig.Config, [:secret_key]
instead, to be consistent with the other modules and because it ensures that secret_key
is set on startup.
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.
I've applied the change you suggested, but instead of :secret_key
I've used name :codec_secret_key
, because secret_key
is used elsewhere as alias for jwt_secret_key
as far as I understand. I'm using NODE_COOKIE
env var.
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.
well :secret_key
in that context refers to the key used in the configuration for this module, which means there can't be a name clash here. That said, :codec_secret_key
is fine, too.
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.
I would also mention this change in CHANGELOG.md
It is important that
Having 32 characters 'a-zA-Z0-9' is base64 decodable, so I think we're fine. |
I've done that. |
Description
Closes #208
What to look out for