schema-registry
schema-registry copied to clipboard
Decode URL's user info before setting auth header
Commit 789621d added possibility to set http basic auth header in the schema registry client by putting them on the URL (as in https://user:password@host:port). However, if the credentials contain special characters, they must be percent-encoded:
https://tools.ietf.org/html/rfc3986#section-2.1
This patch makes schema registry client respect percent encoded credentials by decoding them before calculating the base64 representation. Please note, that the decoding step happens independently of the specific credentials provider, so other ways to supply credentials might be affected as well. Currently, all ways to set credentials use the user info form "user:pass", so this should not be harmful.
It looks like @i7c hasn't signed our Contributor License Agreement, yet.
The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence. Wikipedia
You can read and sign our full Contributor License Agreement here.
Once you've signed reply with [clabot:check] to prove it.
Appreciation of efforts,
clabot
Please note, that if merged, you also need to percent-encode credentials set via 'schema.registry.basic.auth.user.info'. If this is not wanted, we should maybe consider using completely different means to set the credentials (independetly of any "user info" form).
[clabot:check]
@confluentinc It looks like @i7c just signed our Contributor License Agreement. :+1:
Always at your service,
clabot
I can hopefully wrap this up tomorrow and we can do another round of review. :)
Hi again,
my reworked pull request is out for discussion. I split it in three commits, because I’m doing three steps which you may individually decide if you like them or not.
- the common base class and actual URL decoding, which was the main concern for the original PR
- providing the auth header directly from the credential provider. I’m not sure if this is a good way to go, but since the providers are only meant for basic auth, I do not see any harm at least.
- another way of specifying credentials without URL encoding
I have not yet tested the code against an actual schema registry. I will do so if you think the patch series is going in an acceptable direction.
Looking forward to getting feedback! :)
@i7c schema.registry.basic.auth.user.info is part of the configs. So, I don't see a need to decode the credentials there. The only place we potentially might need decoding is when the credentials are provided via the URL config itself. In which case, we should just be able to do the decode just in the UrlBasicAuthCredentialProvider. WDYT?
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.