Feat/improve api tokens #269
Conversation
jonathanverner
force-pushed
the
feat/improve-api-tokens
branch
from
7e8be80
to
a21afd7
Compare
Some users might prefer to keep their passwords/api_tokens in a keychain so as not to expose them unencrypted in an ENV variable or a config file. This commit introduces an `api_token_cmd` config key (and a corresponding `JIRA_API_TOKEN_CMD` environment variable) which, if set, specifies a command to run whose output will provide the value of the api token. This can be set to, e.g., `kwallet-query -r my.jira.server.com kdewallet` for the KDE KWallet, or an appropriate command for other keychains or password managers. See also Issue ankitpokhrel#247.
It seems PAT tokens (for on-premise JIRA servers) don't use basic auth but rather set the Authorization header to "Bearer " + token. See ankitpokhrel#268.
jonathanverner
force-pushed
the
feat/improve-api-tokens
branch
from
a21afd7
to
b3fa28e
Compare
There was a problem hiding this comment.
Hi @jonathanverner, Thank you for working on this 🙏🏻
I added a few comments and I think the implementation can be simplified a bit. Let me know what you think.
| @@ -222,7 +225,11 @@ func (c *Client) request(ctx context.Context, method, endpoint string, body []by | |||
| req.Header.Set(k, v) | |||
| } | |||
|
|
|||
| req.SetBasicAuth(c.login, c.token) | |||
| if c.bearer { | |||
| req.Header.Set("Authorization", "Bearer "+c.token[4:]) | |||
There was a problem hiding this comment.
c.token[4:] Why do we need to ignore initial bytes in the token?
| return viper.GetString("api_token_pat_cmd"), true, true | ||
| default: | ||
| return "", false, false | ||
| } |
There was a problem hiding this comment.
I was thinking if we can do this a bit differently.
- It might be better to use same
JIRA_API_TOKENenv for both basic and bearer auth. The common usecase would be either to use basic or bearer. - A new env
JIRA_AUTH_TYPEwhich could specify whether it is abearerorbasicauth (basic by default). - A new env
JIRA_AUTH_CMDwhich will be invoked to get the token if it is set.
Also, I wouldn't add tokens directly in the config file. One of the idea behind config file for me was also to be able to share it with colleagues or commit it in vcs and storing tokens in the config defeats that purpose.
|
Closing due to inactivity. The branch has diverged a bit and some part of this PR is done in #327. Please feel free to comment or open a new PR for further discussion. |
This pull requests contains two improvements to API TOKEN handling: