feat(config): option to configure a helper to lookup the token #1359
Conversation
3479b9c to
8ecf559
Compare
Codecov Report
@@ Coverage Diff @@
## master #1359 +/- ##
==========================================
- Coverage 80.21% 79.98% -0.24%
==========================================
Files 73 73
Lines 3801 4021 +220
==========================================
+ Hits 3049 3216 +167
- Misses 752 805 +53
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Hi @nejch , to fix commitlinting issues, I have squashed my commits and created better commit messages. |
There was a problem hiding this comment.
Thanks a lot for this feature, this is really really neat :) People have requested native keyring support but I think this might be even more flexible as it can be used with any backend/CLI client like vault etc.
I've just added a few comments, sorry it's just a very user-facing feature so I thought it'd be good to agree on the approach from the start.
Also I think this deserves a mention in the docs, would you mind adding a section in https://github.com/python-gitlab/python-gitlab/blob/master/docs/cli-usage.rst?
Codecov Report
@@ Coverage Diff @@
## master #1359 +/- ##
=========================================
Coverage ? 80.10%
=========================================
Files ? 73
Lines ? 4061
Branches ? 0
=========================================
Hits ? 3253
Misses ? 808
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Hi @nejch, yes I am fine with your changes. This is a good compromise
between convenience and security. Thanks :)
Nejc Habjan ***@***.***> schrieb am So., 18. Apr. 2021, 12:11:
… ***@***.**** commented on this pull request.
------------------------------
In gitlab/config.py
<#1359 (comment)>
:
> @@ -150,6 +151,16 @@ def __init__(
except Exception:
pass
+ for attr in ("job_token", "http_password", "private_token", "oauth_token"):
+ value = getattr(self, attr)
+ prefix = "lookup:"
+ if isinstance(value, str) and value.lower().strip().startswith(prefix):
+ helper = value[len(prefix) :].strip()
+ value = (
+ subprocess.check_output(helper, shell=True).decode("utf-8").strip()
Thank you for the changes @klorenz <https://github.com/klorenz> and sorry
for the delay! I'll be merging in a minute.
I really liked the convenience of your initial implementation, and I don't
want to block this PR any longer so I just pushed a small change (I
credited you as the author still) - I went for a compromise where
shell=False is still used and recommend wrappers for any scripts that
require piping etc. I hope this is ok with you.
I also expanded the docs and wrapped the call in a try/except block for
people to have a meaningful erorr message on the CLI. See 91ffb8e
<91ffb8e>,
let me know if you want to revisit this in a follow-up.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1359 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAE2WZADPHXO37LIOFOMXWLTJKV6JANCNFSM4YWO4IXA>
.
|
I have stored my personal access token with all other secrets in UNIX password store.
For not having the token stored on the disk, I have implemented the option to let tokens be
looked up from a helper program.
Example:
whith a helper script
~/bin/pass-helper.sh: