Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read token from other sources (like secret/password manager) #382

Closed
tfurf opened this issue Jun 30, 2020 · 7 comments · Fixed by #416
Closed

Read token from other sources (like secret/password manager) #382

tfurf opened this issue Jun 30, 2020 · 7 comments · Fixed by #416
Milestone

Comments

@tfurf
Copy link

tfurf commented Jun 30, 2020

Having the user token stored in plaintext makes the config file not a great candidate for being version controlled. Is there some appetite or view forward on being able to load the tokens dynamically from somewhere else?

For example, I use pass, [1], and normally to load a password I would do something like:

$ pass show work/software/gitlab/token
FooPass123!

In the config, this might be something like,

{
  "host" = "https://www.gitlab.com"
  "load_token" = "pass show work/software/gitlab/token"
}

This relates to #151, #155 , etc.

@zaquestion
Copy link
Owner

Defs support this, and even like the way you suggest using a different (alt) creds key. This way there aren't any backwards compatible issues. One thing we'll need to sort out is the current first time flow which has users create a token and enter directly into lab, which then writes the config. This might be fine, as users who want this can just delete the raw token and place into password managers a accordingly.

@zaquestion
Copy link
Owner

Xpost from #402

a) A way to execute a command to retrieve a token in place of a plaintext one sounds great, so long as we can do it in a backwards compatible way. Even if this mean simply offering an alternative key to provide creds under.

- Me

I think a way to do this would be to read the token to see if it "looks" like a token. There are other cases of this in code where they check to see if a git hash is, in fact, a git hash. If the token is a token then it could be used as straight text. Otherwise let's assume a command is written there. I think I could test with this pass or some other password manager.

- @prarit

This approach seems reasonable as well, provided we're confident we can reliably detect token vs command. I suppose this could be as simple as taking the token and trying a gitlab call (like GetUser) or the inverse, attempting to execute and seeing checking for failure.

@tfurf
Copy link
Author

tfurf commented Aug 28, 2020

IMHO it's better to be explicit than clever --- adding a different key (at least in the beginning) is guaranteed to not cause compatibility issues. Eventually the alternative token behaviour can be merged back into the current token parsing.

In any case, given #406, #407, and #151, is this worth pursuing now or waiting for the dust to settle somewhat?

@zaquestion
Copy link
Owner

We can pursue this now, this should fit in well with the other stuff.

@prarit
Copy link
Collaborator

prarit commented Sep 4, 2020

@tfurf , I'm waiting on the dust from #414 to settle but this feature will nicely fit into the new config work.

diff --git a/internal/config/config.go b/internal/config/config.go
index 8fb699dbf133..6216b45ca352 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -11,6 +11,7 @@ import (
        "net/http"
        "net/url"
        "os"
+       "os/exec"
        "path"
        "strings"
        "syscall"
@@ -230,6 +231,19 @@ func LoadConfig() (string, string, string, string, bool) {
        host = viper.GetString("core.host")
        user = viper.GetString("core.user")
        token = viper.GetString("core.token")
+       if token == "" && viper.GetString("core.load_token") != "" {
+               _token, err := exec.Command(viper.GetString("core.load_token")).Output()
+               if err != nil {
+                       log.Fatal(err)
+               }
+               token = string(_token)
+               // tools like pass and a simple bash script add a '\n' to
+               // their output which confuses the gitlab WebAPI
+               if token[len(token)-1:] == "\n" {
+                       token = strings.TrimSuffix(token, "\n")
+               }
+       }
+
        tlsSkipVerify := viper.GetBool("tls.skip_verify")
        ca_file := viper.GetString("tls.ca_file")
 

@prarit
Copy link
Collaborator

prarit commented Sep 4, 2020

I'm trying to think of a test to write for load_token. The only thing I can think of doing is creating a simple bash script dummy_token.sh

#!/usr/bin/bash
echo "12345abcdef"

and creating a dummy lab config with

load_token = "./dummy_token.sh"

and executing 'lab mr list' in the test repo to see if lab read in the token or not.

@prarit prarit linked a pull request Sep 6, 2020 that will close this issue
@prarit
Copy link
Collaborator

prarit commented Sep 6, 2020

@tfurf , can you checkout #416 and test please? As stated above, I've added a "load_token" config option that will use the contents of "load_token" as an executable, and use that output as the token value.

For example,
load_token = "pass show work/software/gitlab/token"

Please provide your testing input in #416 .

Thanks :)

@zaquestion zaquestion added this to the 0.18.0 milestone Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants