feat: Add mtls authentication for client certificate auth
#615
Conversation
|
@ankitpokhrel Here is a first pass at |
markhatch
changed the title
mtls authentication for client certificate authmtls authentication for client certificate auth
There was a problem hiding this comment.
Hi @markhatch,
Apologies for late response and thank you for working on this!
I added some comments. Most of them are nitpicks. Looks good otherwise! Great Work 🎉
|
Think I've resolved most of the comments, let me know if it looks good! |
There was a problem hiding this comment.
Hi @markhatch, apologies again for the late response.
This looks good overall, just added few comments. I will try to get this merged for the next release.
Thank you for working on this!
| qs := &survey.Select{ | ||
| Message: "Authentication type:", | ||
| Help: "basic (login) or mtls (client certs)?", | ||
| Options: []string{"basic", "mtls"}, |
There was a problem hiding this comment.
Should here be bearer as well?
There was a problem hiding this comment.
I believe that bearer is only used in for InstallationType cloud
| @@ -226,7 +263,7 @@ func (c *Client) request(ctx context.Context, method, endpoint string, body []by | |||
|
|
|||
| if c.authType == AuthTypeBearer { | |||
| req.Header.Add("Authorization", "Bearer "+c.token) | |||
| } else { | |||
| } else if c.authType == AuthTypeBasic { | |||
There was a problem hiding this comment.
Current default auth type is basic (if nothing is set in config/env). Looks like this might break that?
There was a problem hiding this comment.
Sorry been a while - but I believe this change was made so that the login/token are provided only if authType is AuthTypeBasic.
If authType is Bearer you add the authorization header.
If authType is Basic you add the login/token
If authType is mtls, you don't need to modify the request.
I don't believe the default auth type has changed, and will depend on how the user configured authType.
* Persist auth type in config file * Update `jira init` to configure `mtls` * Update README with instructions
There was a problem hiding this comment.
I wasn't able to test this fully but looks good overall. Thank you @markhatch
jira initto configuremtlsDiscussion: #603