Implement basic authentication #70
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/M
There was a problem hiding this comment.
Thanks for the PR!
I think the header is set incorrectly (it might work, but IMO we should stick to the standard).
Can you please also add a test for detection to this directory.
| `setHeader` toHeader ("authorization", "Basic " <> encodeBasicAuth) | ||
| & L.set rAuthTypesL [] | ||
| where | ||
| encodeBasicAuth = T.decodeUtf8 $ encode $ T.encodeUtf8 $ basicAuthUsername <> ":" <> basicAuthPassword |
There was a problem hiding this comment.
This has to be base64 encoded (at least according to RFC 7617. It is better to ues applyBasicAuth function from http-client instead of setHeader.
There was a problem hiding this comment.
This has to be base64 encoded (at least according to RFC 7617. It is better to ues
applyBasicAuthfunction from http-client instead ofsetHeader.
@akshaymankar So the encode function is from Data.ByteString.Base64 so this encoding-decoding stuff what it does is Text -> ByteString -> Encode to base 64 -> Text. I understand it is quite ugly but it is encoding the username and password as base64.
I opted for converting to ByteString then encode, and then convert to Text again because the library base64-bytestring is already imported and did not want to introduce another library for just doing the base64 encoding.
Will do the test.
There was a problem hiding this comment.
The applyBasicAuth works for raw WAI Requests, but here we are having KubernetesRequest which come from the OpenAPI generated code. I am not sure how I can use the applyBasicAuth here.
There was a problem hiding this comment.
You're right. I totally missed encode in all of this. I think this ok then 👍
|
/assign @akshaymankar |
k8s-ci-robot
added
size/L
|
@akshaymankar Added detect test in 7c3a8f5, let me know if this needs anything else. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akshaymankar, frincon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Basic auth was not supported yet. An example use-case is when using microk8s which by default uses basic auth in its configuration.